fix: sign contract should be allowed by agents

docs/canisters/deferred.md

@@ -7,7 +7,7 @@
   - [Roles](#roles)
   - [API](#api)
     - [register\_contract](#register_contract)
-    - [admin\_sign\_contract](#admin_sign_contract)
+    - [sign\_contract](#sign_contract)
     - [get\_contract](#get_contract)
     - [get\_token](#get_token)
     - [get\_signed\_contracts](#get_signed_contracts)
@@ -69,7 +69,7 @@ A Contract is identified by the following properties
 On the deferred canister the following roles exists:
 
 - **Custodian**: administrator of the canister, following the DIP721 standard. It can administrate the canister and sign contracts.
-- **Agent**: role for agencies. Agent can create contracts, but he cannot sign them.
+- **Agent**: role for agencies. Agents can create contracts, and operate only on contracts created by their agency.
 
 ## API
 
@@ -81,9 +81,9 @@ Register a contract with the provided data.
 
 **The contract value** MUST be **multiple of installments**.
 
-### admin_sign_contract
+### sign_contract
 
-Approve and sign an existing contract. Once signed, the contract's tokens can be sold on the marketplace.
+Approve and sign an existing contract. Once signed, the contract's tokens can be sold on the marketplace. Only an admin or an agent can sign the contract.
 
 ### get_contract
 

integration-tests/src/client/deferred.rs

@@ -39,13 +39,13 @@ impl<'a> DeferredClient<'a> {
         contract_id
     }
 
-    pub fn admin_sign_contract(&self, id: Nat) -> DeferredResult<()> {
+    pub fn sign_contract(&self, id: Nat) -> DeferredResult<()> {
         let res: DeferredResult<()> = self
             .env
             .update(
                 self.env.deferred_id,
                 admin(),
-                "admin_sign_contract",
+                "sign_contract",
                 Encode!(&id).unwrap(),
             )
             .unwrap();

integration-tests/tests/http/deferred.rs

@@ -120,7 +120,7 @@ fn init_contract(env: &TestEnv) -> ID {
     assert_eq!(contract_id, 0_u64);
 
     // sign contract
-    let res = deferred_client.admin_sign_contract(contract_id.clone());
+    let res = deferred_client.sign_contract(contract_id.clone());
     assert!(res.is_ok());
 
     contract_id

integration-tests/tests/inspect/deferred.rs

@@ -1,5 +1,7 @@
 use candid::{Encode, Nat};
-use did::deferred::{ContractRegistration, ContractType, DeferredResult, GenericValue, Seller};
+use did::deferred::{
+    Agency, ContractRegistration, ContractType, DeferredResult, GenericValue, Seller,
+};
 use did::ID;
 use dip721::NftError;
 use integration_tests::actor::{admin, alice, bob};
@@ -473,6 +475,62 @@ fn test_should_inspect_register_contract_expired() {
     assert!(result.is_err());
 }
 
+#[test]
+#[serial_test::serial]
+fn test_should_inspect_sign_contract() {
+    let env = TestEnv::init();
+    let client = DeferredClient::new(&env);
+
+    let agent = bob();
+    // give bob an agency
+    client.admin_register_agency(
+        agent,
+        Agency {
+            name: "Bob's agency".to_string(),
+            address: "Via Delle Botteghe Scure".to_string(),
+            city: "Rome".to_string(),
+            region: "Lazio".to_string(),
+            zip_code: "00100".to_string(),
+            country: "Italy".to_string(),
+            continent: did::deferred::Continent::Europe,
+            email: "email".to_string(),
+            website: "website".to_string(),
+            mobile: "mobile".to_string(),
+            vat: "vat".to_string(),
+            agent: "agent".to_string(),
+            logo: None,
+        },
+    );
+
+    let registration_data = ContractRegistration {
+        r#type: ContractType::Sell,
+        sellers: vec![Seller {
+            principal: alice(),
+            quota: 100,
+        }],
+        buyers: vec![bob()],
+        value: 400_000,
+        currency: "EUR".to_string(),
+        installments: 400_000 / 100,
+        properties: vec![(
+            "contract:address".to_string(),
+            GenericValue::TextContent("via roma 10".to_string()),
+        )],
+        expiration: None,
+    };
+
+    let contract_id = client.register_contract(agent, registration_data).unwrap();
+
+    let result: anyhow::Result<DeferredResult<ID>> = env.update(
+        env.deferred_id,
+        agent,
+        "sign_contract",
+        Encode!(&contract_id).unwrap(),
+    );
+
+    assert!(result.is_ok());
+}
+
 #[test]
 #[serial_test::serial]
 fn test_should_inspect_burn() {
@@ -499,7 +557,7 @@ fn test_should_inspect_burn() {
     let contract_id = client
         .register_contract(admin(), registration_data)
         .unwrap();
-    assert!(client.admin_sign_contract(contract_id.clone()).is_ok());
+    assert!(client.sign_contract(contract_id.clone()).is_ok());
 
     // transfer token to buyer
     let token_id = Nat::from(1_u64);

integration-tests/tests/use_case/buy_marketplace_nft.rs

@@ -137,7 +137,7 @@ fn setup_contract_marketplace(env: &TestEnv) -> ID {
     assert_eq!(contract_id, 0_u64);
 
     // sign contract
-    let res = deferred_client.admin_sign_contract(contract_id.clone());
+    let res = deferred_client.sign_contract(contract_id.clone());
     assert!(res.is_ok());
 
     contract_id

integration-tests/tests/use_case/increment_contract_value.rs

@@ -33,7 +33,7 @@ fn test_as_seller_i_can_set_the_contract_buyers() {
         .unwrap();
 
     // sign contract
-    let res = deferred_client.admin_sign_contract(contract_id.clone());
+    let res = deferred_client.sign_contract(contract_id.clone());
     assert!(res.is_ok());
 
     // increment contract value

integration-tests/tests/use_case/register_agency.rs

@@ -61,7 +61,7 @@ fn test_should_register_agency_and_be_able_to_create_contract() {
     assert!(signed_contract.is_empty());
 
     // sign contract
-    let res = deferred_client.admin_sign_contract(Nat::from(0_u64));
+    let res = deferred_client.sign_contract(Nat::from(0_u64));
     assert!(res.is_ok());
 
     // agency could remove himself

integration-tests/tests/use_case/register_contract_buyers.rs

@@ -33,7 +33,7 @@ fn test_as_seller_i_can_set_the_contract_buyers() {
         .unwrap();
 
     // sign contract
-    let res = deferred_client.admin_sign_contract(contract_id.clone());
+    let res = deferred_client.sign_contract(contract_id.clone());
     assert!(res.is_ok());
 
     // update buyers

integration-tests/tests/use_case/register_sell_contract.rs

@@ -65,7 +65,7 @@ fn test_as_seller_i_can_register_a_sell_contract() {
     assert!(signed_contract.is_empty());
 
     // sign contract
-    let res = deferred_client.admin_sign_contract(Nat::from(0_u64));
+    let res = deferred_client.sign_contract(Nat::from(0_u64));
     assert!(res.is_ok());
 
     // get contract

integration-tests/tests/use_case/reserve_reward_pool.rs

@@ -64,7 +64,7 @@ fn test_should_reserve_a_reward_pool_on_ekoke() {
         .is_ok());
 
     // sign contract
-    let res = deferred_client.admin_sign_contract(contract_id);
+    let res = deferred_client.sign_contract(contract_id);
     assert!(res.is_ok());
 
     // verify reward

integration-tests/tests/use_case/update_contract_property.rs

@@ -44,7 +44,7 @@ fn test_should_update_contract_property() {
         .register_contract(admin(), registration_data)
         .unwrap();
 
-    let res = deferred_client.admin_sign_contract(contract_id.clone());
+    let res = deferred_client.sign_contract(contract_id.clone());
     assert!(res.is_ok());
 
     // call update_contract_property

src/declarations/deferred/deferred.did

@@ -294,7 +294,6 @@ service : (DeferredInitData) -> {
   admin_set_ekoke_ledger_canister : (principal) -> ();
   admin_set_marketplace_canister : (principal) -> ();
   admin_set_role : (principal, Role) -> ();
-  admin_sign_contract : (nat) -> (Result);
   approve : (principal, nat) -> (Result_1);
   balance_of : (principal) -> (Result_1) query;
   burn : (nat) -> (Result_1);
@@ -324,6 +323,7 @@ service : (DeferredInitData) -> {
   set_logo : (text) -> ();
   set_name : (text) -> ();
   set_symbol : (text) -> ();
+  sign_contract : (nat) -> (Result);
   stats : () -> (Stats) query;
   supported_interfaces : () -> (vec SupportedInterface) query;
   symbol : () -> (opt text) query;

src/declarations/deferred/deferred.did.d.ts

@@ -295,7 +295,6 @@ export interface _SERVICE {
   'admin_set_ekoke_ledger_canister' : ActorMethod<[Principal], undefined>,
   'admin_set_marketplace_canister' : ActorMethod<[Principal], undefined>,
   'admin_set_role' : ActorMethod<[Principal, Role], undefined>,
-  'admin_sign_contract' : ActorMethod<[bigint], Result>,
   'approve' : ActorMethod<[Principal, bigint], Result_1>,
   'balance_of' : ActorMethod<[Principal], Result_1>,
   'burn' : ActorMethod<[bigint], Result_1>,
@@ -328,6 +327,7 @@ export interface _SERVICE {
   'set_logo' : ActorMethod<[string], undefined>,
   'set_name' : ActorMethod<[string], undefined>,
   'set_symbol' : ActorMethod<[string], undefined>,
+  'sign_contract' : ActorMethod<[bigint], Result>,
   'stats' : ActorMethod<[], Stats>,
   'supported_interfaces' : ActorMethod<[], Array<SupportedInterface>>,
   'symbol' : ActorMethod<[], [] | [string]>,

src/declarations/deferred/deferred.did.js

@@ -339,7 +339,6 @@ export const idlFactory = ({ IDL }) => {
     'admin_set_ekoke_ledger_canister' : IDL.Func([IDL.Principal], [], []),
     'admin_set_marketplace_canister' : IDL.Func([IDL.Principal], [], []),
     'admin_set_role' : IDL.Func([IDL.Principal, Role], [], []),
-    'admin_sign_contract' : IDL.Func([IDL.Nat], [Result], []),
     'approve' : IDL.Func([IDL.Principal, IDL.Nat], [Result_1], []),
     'balance_of' : IDL.Func([IDL.Principal], [Result_1], ['query']),
     'burn' : IDL.Func([IDL.Nat], [Result_1], []),
@@ -397,6 +396,7 @@ export const idlFactory = ({ IDL }) => {
     'set_logo' : IDL.Func([IDL.Text], [], []),
     'set_name' : IDL.Func([IDL.Text], [], []),
     'set_symbol' : IDL.Func([IDL.Text], [], []),
+    'sign_contract' : IDL.Func([IDL.Nat], [Result], []),
     'stats' : IDL.Func([], [Stats], ['query']),
     'supported_interfaces' : IDL.Func(
         [],

src/deferred/deferred.did

@@ -294,7 +294,6 @@ service : (DeferredInitData) -> {
   admin_set_ekoke_ledger_canister : (principal) -> ();
   admin_set_marketplace_canister : (principal) -> ();
   admin_set_role : (principal, Role) -> ();
-  admin_sign_contract : (nat) -> (Result);
   approve : (principal, nat) -> (Result_1);
   balance_of : (principal) -> (Result_1) query;
   burn : (nat) -> (Result_1);
@@ -324,6 +323,7 @@ service : (DeferredInitData) -> {
   set_logo : (text) -> ();
   set_name : (text) -> ();
   set_symbol : (text) -> ();
+  sign_contract : (nat) -> (Result);
   stats : () -> (Stats) query;
   supported_interfaces : () -> (vec SupportedInterface) query;
   symbol : () -> (opt text) query;

src/deferred/src/app.rs

@@ -169,8 +169,8 @@ impl Deferred {
     }
 
     /// Sign contract and mint tokens
-    pub async fn admin_sign_contract(contract_id: ID) -> DeferredResult<()> {
-        if !Inspect::inspect_is_custodian(caller()) {
+    pub async fn sign_contract(contract_id: ID) -> DeferredResult<()> {
+        if !Inspect::inspect_sign_contract(caller(), &contract_id) {
             ic_cdk::trap("Unauthorized");
         }
 
@@ -639,7 +639,7 @@ mod test {
             Deferred::admin_get_unsigned_contracts(),
             vec![Nat::from(0_u64)]
         );
-        assert!(Deferred::admin_sign_contract(0_u64.into()).await.is_ok());
+        assert!(Deferred::sign_contract(0_u64.into()).await.is_ok());
         assert_eq!(Deferred::get_signed_contracts(), vec![Nat::from(0_u64)]);
         assert_eq!(Deferred::total_supply(), Nat::from(10_u64));
     }
@@ -661,7 +661,7 @@ mod test {
             expiration: Some("2048-01-01".to_string()),
         };
         assert_eq!(Deferred::register_contract(contract).unwrap(), 0_u64);
-        assert!(Deferred::admin_sign_contract(0_u64.into()).await.is_ok());
+        assert!(Deferred::sign_contract(0_u64.into()).await.is_ok());
 
         // increment value
         assert!(Deferred::increment_contract_value(0_u64.into(), 50, 10)

src/deferred/src/app/inspect.rs

@@ -22,8 +22,14 @@ impl Inspect {
         RolesManager::is_agent(caller)
     }
 
+    /// Inspect whether can sign contract
+    pub fn inspect_sign_contract(caller: Principal, contract_id: &ID) -> bool {
+        Inspect::inspect_is_custodian(caller)
+            || Inspect::inspect_is_agent_for_contract(caller, contract_id).is_ok()
+    }
+
     /// Returns whether caller is agent of the canister and agent for the contracts
-    fn inspect_is_agent_for_contract(
+    pub fn inspect_is_agent_for_contract(
         caller: Principal,
         contract_id: &ID,
     ) -> DeferredResult<Contract> {
@@ -139,7 +145,7 @@ impl Inspect {
         };
 
         if !Self::inspect_is_custodian(caller)
-            && !Self::inspect_is_agent_for_contract(caller, id).is_ok()
+            && Self::inspect_is_agent_for_contract(caller, id).is_err()
             && !contract.is_seller(&caller)
         {
             Err(DeferredError::Unauthorized)
@@ -221,7 +227,7 @@ impl Inspect {
         contract: ID,
     ) -> DeferredResult<Contract> {
         if !Self::inspect_is_custodian(caller)
-            && !Self::inspect_is_agent_for_contract(caller, &contract).is_ok()
+            && Self::inspect_is_agent_for_contract(caller, &contract).is_err()
         {
             return Err(DeferredError::Unauthorized);
         }
@@ -770,4 +776,36 @@ mod test {
             Principal::management_canister()
         ));
     }
+
+    #[test]
+    fn test_should_inspect_sign_contract() {
+        let admin = alice();
+        assert!(RolesManager::set_custodians(vec![admin]).is_ok());
+        // set agent
+        let agent = bob();
+        let agency = mock_agency();
+        let other_agent = Principal::management_canister();
+        Agents::insert_agency(agent, agency.clone());
+        RolesManager::give_role(agent, Role::Agent);
+
+        let mut other_agency = agency.clone();
+        other_agency.name = "other".to_string();
+        Agents::insert_agency(other_agent, other_agency.clone());
+        RolesManager::give_role(other_agent, Role::Agent);
+
+        let contract_id = 0;
+        let contract = test_utils::with_mock_contract(contract_id, 1, |contract| {
+            contract.agency = Some(agency);
+        });
+        assert!(ContractStorage::insert_contract(contract).is_ok());
+
+        // admin
+        assert!(Inspect::inspect_sign_contract(admin, &contract_id.into()));
+
+        // agent
+        assert_eq!(
+            Inspect::inspect_sign_contract(other_agent, &contract_id.into()),
+            false
+        );
+    }
 }

src/deferred/src/inspect.rs

@@ -24,6 +24,10 @@ fn inspect_message_impl() {
 
     let check_result = match method.as_str() {
         method if method.starts_with("admin_") => Inspect::inspect_is_custodian(caller()),
+        "sign_contract" => {
+            let (id,) = api::call::arg_data::<(ID,)>();
+            Inspect::inspect_sign_contract(caller(), &id)
+        }
         "set_logo" | "set_name" | "set_symbol" | "set_custodians" => {
             Inspect::inspect_is_custodian(caller())
         }

src/deferred/src/lib.rs

@@ -41,8 +41,8 @@ pub fn register_contract(data: ContractRegistration) -> DeferredResult<Nat> {
 
 #[update]
 #[candid_method(update)]
-pub async fn admin_sign_contract(contract_id: ID) -> DeferredResult<()> {
-    Deferred::admin_sign_contract(contract_id).await
+pub async fn sign_contract(contract_id: ID) -> DeferredResult<()> {
+    Deferred::sign_contract(contract_id).await
 }
 
 #[update]