Fix update proxy contracts with delete (#1012)

contracts/icp/context-config/.env

@@ -2,7 +2,7 @@
 # DFX CANISTER ENVIRONMENT VARIABLES
 DFX_VERSION='0.24.2'
 DFX_NETWORK='local'
-CANISTER_ID_CONTEXT_CONTRACT='bw4dl-smaaa-aaaaa-qaacq-cai'
-CANISTER_ID='bw4dl-smaaa-aaaaa-qaacq-cai'
-CANISTER_CANDID_PATH='/Users/alen/www/calimero/core/contracts/icp/context-config/context_contract.did'
+CANISTER_ID_CONTEXT_CONTRACT='bkyz2-fmaaa-aaaaa-qaaaq-cai'
+CANISTER_ID='bkyz2-fmaaa-aaaaa-qaaaq-cai'
+CANISTER_CANDID_PATH='/Users/alen/www/calimero/core/contracts/icp/context-config/./res/calimero_context_config_icp.did'
 # END DFX CANISTER ENVIRONMENT VARIABLES

contracts/icp/context-proxy/dfx.json

@@ -1,19 +1,19 @@
 {
   "canisters": {
     "proxy_contract": {
-      "package": "calimero_context_proxy_icp",
+      "package": "calimero-context-proxy-icp",
       "candid": "./res/calimero_context_proxy_icp.did",
       "type": "rust"
     },
     "mock_ledger": {
       "type": "rust",
-      "package": "mock_ledger",
+      "package": "calimero-mock-ledger-icp",
       "candid": "./mock/ledger/res/calimero_mock_ledger_icp.did",
       "path": "mock/ledger"
     },
     "mock_external": {
       "type": "rust",
-      "package": "mock_external",
+      "package": "calimero-mock-external-icp",
       "candid": "./mock/external/res/calimero_mock_external_icp.did",
       "path": "mock/external"
     }

contracts/icp/context-proxy/res/calimero_context_proxy_icp.did

@@ -7,6 +7,7 @@ type ICProposalAction = variant {
   SetNumApprovals : record { num_approvals : nat32 };
   SetContextValue : record { key : blob; value : blob };
   Transfer : record { receiver_id : principal; amount : nat };
+  DeleteProposal : record { proposal_id : blob };
   SetActiveProposalsLimit : record { active_proposals_limit : nat32 };
   ExternalFunctionCall : record {
     receiver_id : principal;
@@ -35,8 +36,8 @@ service : (blob, principal) -> {
   get_proposal_approvals_with_signer : (blob) -> (
       vec ICProposalApprovalWithSigner,
     ) query;
-  get_proposal_approvers : (blob) -> (opt vec blob) query;
   mutate : (ICSigned) -> (Result);
   proposal : (blob) -> (opt ICProposal) query;
+  proposal_approvers : (blob) -> (opt vec blob) query;
   proposals : (nat64, nat64) -> (vec ICProposal) query;
 }

contracts/icp/context-proxy/src/mutate.rs

@@ -87,8 +87,32 @@ async fn execute_proposal(proposal_id: &ProposalId) -> Result<(), String> {
                 receiver_id,
                 method_name,
                 args,
-                deposit: _,
+                deposit,
             } => {
+                // If there's a deposit, transfer it first
+                if deposit > 0 {
+                    let ledger_id = with_state(|contract| contract.ledger_id.clone());
+
+                    let transfer_args = TransferArgs {
+                        memo: Memo(0),
+                        amount: Tokens::from_e8s(
+                            deposit
+                                .try_into()
+                                .map_err(|e| format!("Amount conversion error: {}", e))?,
+                        ),
+                        fee: Tokens::from_e8s(10_000), // Standard fee is 0.0001 ICP
+                        from_subaccount: None,
+                        to: AccountIdentifier::new(&receiver_id, &Subaccount([0; 32])),
+                        created_at_time: None,
+                    };
+
+                    let _: (Result<u64, TransferError>,) =
+                        ic_cdk::call(Principal::from(ledger_id), "transfer", (transfer_args,))
+                            .await
+                            .map_err(|e| format!("Transfer failed: {:?}", e))?;
+                }
+
+                // Then make the actual cross-contract call
                 let args_bytes = candid::encode_one(args)
                     .map_err(|e| format!("Failed to encode args: {}", e))?;
 
@@ -137,6 +161,7 @@ async fn execute_proposal(proposal_id: &ProposalId) -> Result<(), String> {
                     contract.context_storage.insert(key, value);
                 });
             }
+            ICProposalAction::DeleteProposal { proposal_id: _ } => {}
         }
     }
 
@@ -155,6 +180,14 @@ async fn internal_create_proposal(
         return Err("proposal cannot have empty actions".to_string());
     }
 
+    // Check if the proposal contains a delete action
+    for action in &proposal.actions {
+        if let ICProposalAction::DeleteProposal { proposal_id } = action {
+            remove_proposal(proposal_id);
+            return Ok(None);
+        }
+    }
+
     with_state_mut(|contract| {
         let num_proposals = contract
             .num_proposals_pk
@@ -223,6 +256,7 @@ fn validate_proposal_action(action: &ICProposalAction) -> Result<(), String> {
             }
         }
         ICProposalAction::SetContextValue { .. } => {}
+        ICProposalAction::DeleteProposal { .. } => {}
     }
     Ok(())
 }

contracts/icp/context-proxy/src/query.rs

@@ -52,7 +52,7 @@ pub fn get_confirmations_count(proposal_id: ICRepr<ProposalId>) -> Option<ICProp
 }
 
 #[ic_cdk::query]
-pub fn get_proposal_approvers(proposal_id: ICRepr<ProposalId>) -> Option<Vec<ICRepr<SignerId>>> {
+pub fn proposal_approvers(proposal_id: ICRepr<ProposalId>) -> Option<Vec<ICRepr<SignerId>>> {
     with_state(|contract| {
         contract
             .approvals

contracts/icp/context-proxy/tests/integration.rs

@@ -50,7 +50,7 @@ fn create_and_verify_proposal(
     canister: Principal,
     signer_sk: &SigningKey,
     proposal: ICProposal,
-) -> Result<ICProposalWithApprovals, String> {
+) -> Result<Option<ICProposalWithApprovals>, String> {
     let request = ICProxyMutateRequest::Propose { proposal };
 
     let signed_request = create_signed_request(signer_sk, request);
@@ -70,8 +70,7 @@ fn create_and_verify_proposal(
                     .map_err(|e| format!("Failed to decode response: {}", e))?;
 
             match result {
-                Ok(Some(proposal_with_approvals)) => Ok(proposal_with_approvals),
-                Ok(None) => Err("No proposal returned".to_string()),
+                Ok(proposal_with_approvals) => Ok(proposal_with_approvals),
                 Err(e) => Err(e),
             }
         }
@@ -1098,3 +1097,238 @@ fn test_proposal_execution_external_call() {
         _ => panic!("Unexpected response type"),
     }
 }
+
+#[test]
+fn test_proposal_execution_external_call_with_deposit() {
+    let mut rng = rand::thread_rng();
+
+    let ProxyTestContext {
+        pic,
+        proxy_canister,
+        mock_external,
+        author_sk,
+        context_canister,
+        context_id,
+        mock_ledger,
+        ..
+    } = setup();
+
+    let author_pk = author_sk.verifying_key();
+    let author_id = author_pk.rt().expect("infallible conversion");
+
+    let signer2_sk = SigningKey::from_bytes(&rng.gen());
+    let signer2_pk = signer2_sk.verifying_key();
+    let signer2_id = signer2_pk.rt().expect("infallible conversion");
+
+    let signer3_sk = SigningKey::from_bytes(&rng.gen());
+    let signer3_pk = signer3_sk.verifying_key();
+    let signer3_id = signer3_pk.rt().expect("infallible conversion");
+
+    let proposal_id = rng.gen::<[_; 32]>().rt().expect("infallible conversion");
+
+    // Create external call proposal
+    let deposit_amount = 1_000_000;
+    let test_args = "01020304".to_string(); // Test arguments as string
+    let proposal = ICProposal {
+        id: proposal_id,
+        author_id,
+        actions: vec![ICProposalAction::ExternalFunctionCall {
+            receiver_id: mock_external,
+            method_name: "test_method".to_string(),
+            args: test_args.clone(),
+            deposit: deposit_amount,
+        }],
+    };
+
+    // Create and verify initial proposal
+    let _ = create_and_verify_proposal(&pic, proxy_canister, &author_sk, proposal);
+
+    let context_members = vec![
+        signer2_pk.rt().expect("infallible conversion"),
+        signer3_pk.rt().expect("infallible conversion"),
+    ];
+
+    let _ = add_members_to_context(
+        &pic,
+        context_canister,
+        context_id,
+        &author_sk,
+        context_members,
+    );
+
+    // Add approvals to trigger execution
+    for (signer_sk, signer_id) in [(signer2_sk, signer2_id), (signer3_sk, signer3_id)] {
+        let approval = ICProposalApprovalWithSigner {
+            signer_id,
+            proposal_id,
+            added_timestamp: get_time_nanos(&pic),
+        };
+
+        let request = ICProxyMutateRequest::Approve { approval };
+        let signed_request = create_signed_request(&signer_sk, request);
+
+        let response = pic
+            .update_call(
+                proxy_canister,
+                Principal::anonymous(),
+                "mutate",
+                candid::encode_one(signed_request).unwrap(),
+            )
+            .expect("Failed to approve proposal");
+
+        match response {
+            WasmResult::Reply(bytes) => {
+                let result: Result<Option<ICProposalWithApprovals>, String> =
+                    candid::decode_one(&bytes).expect("Failed to decode response");
+
+                if let Ok(None) = result {
+                    // Proposal was executed, verify it's gone
+                    let query_response = pic
+                        .query_call(
+                            proxy_canister,
+                            Principal::anonymous(),
+                            "proposal",
+                            candid::encode_one(proposal_id).unwrap(),
+                        )
+                        .expect("Query failed");
+
+                    match query_response {
+                        WasmResult::Reply(bytes) => {
+                            let stored_proposal: Option<ICProposal> = candid::decode_one(&bytes)
+                                .expect("Failed to decode stored proposal");
+                            assert!(
+                                stored_proposal.is_none(),
+                                "Proposal should be removed after execution"
+                            );
+                        }
+                        WasmResult::Reject(msg) => panic!("Query rejected: {}", msg),
+                    }
+                }
+            }
+            WasmResult::Reject(msg) => panic!("Approval rejected: {}", msg),
+        }
+    }
+
+    // Verify the transfer was executed by checking mock ledger balance
+    let args = AccountBalanceArgs {
+        account: AccountIdentifier::new(&mock_external, &Subaccount([0; 32])),
+    };
+
+    let response = pic
+        .query_call(
+            mock_ledger,
+            Principal::anonymous(),
+            "account_balance",
+            candid::encode_one(args).unwrap(),
+        )
+        .expect("Failed to query balance");
+
+    match response {
+        WasmResult::Reply(bytes) => {
+            let balance: Tokens = candid::decode_one(&bytes).expect("Failed to decode balance");
+            let gas_fee = 10_000;
+            assert_eq!(
+                balance.e8s(),
+                MOCK_LEDGER_BALANCE.with(|b| *b.borrow()) - deposit_amount as u64 - gas_fee as u64,
+                "External contract should have received the deposit"
+            );
+        }
+        WasmResult::Reject(msg) => panic!("Balance query rejected: {}", msg),
+    }
+
+    // Verify the external call was executed
+    let response = pic
+        .query_call(
+            mock_external,
+            Principal::anonymous(),
+            "get_calls",
+            candid::encode_args(()).unwrap(),
+        )
+        .expect("Query failed");
+
+    match response {
+        WasmResult::Reply(bytes) => {
+            let calls: Vec<Vec<u8>> = candid::decode_one(&bytes).expect("Failed to decode calls");
+            assert_eq!(calls.len(), 1, "Should have exactly one call");
+
+            // Decode the Candid-encoded argument
+            let received_args: String =
+                candid::decode_one(&calls[0]).expect("Failed to decode call arguments");
+            assert_eq!(received_args, test_args, "Call arguments should match");
+        }
+        _ => panic!("Unexpected response type"),
+    }
+}
+
+#[test]
+fn test_delete_proposal() {
+    let mut rng = rand::thread_rng();
+
+    let ProxyTestContext {
+        pic,
+        proxy_canister,
+        author_sk,
+        ..
+    } = setup();
+
+    let author_pk = author_sk.verifying_key();
+    let author_id = author_pk.rt().expect("infallible conversion");
+
+    // First create a proposal that we'll want to delete
+    let target_proposal_id = rng.gen::<[_; 32]>().rt().expect("infallible conversion");
+    let target_proposal = ICProposal {
+        id: target_proposal_id,
+        author_id,
+        actions: vec![ICProposalAction::SetNumApprovals { num_approvals: 2 }],
+    };
+
+    // Create and verify target proposal
+    let target_proposal_result =
+        create_and_verify_proposal(&pic, proxy_canister, &author_sk, target_proposal)
+            .expect("Target proposal creation should succeed");
+    assert!(
+        target_proposal_result.is_some(),
+        "Target proposal should be created"
+    );
+
+    // Create delete proposal
+    let delete_proposal_id = rng.gen::<[_; 32]>().rt().expect("infallible conversion");
+    let delete_proposal = ICProposal {
+        id: delete_proposal_id,
+        author_id,
+        actions: vec![ICProposalAction::DeleteProposal {
+            proposal_id: target_proposal_id,
+        }],
+    };
+
+    // Execute delete proposal immediately
+    let delete_proposal_result =
+        create_and_verify_proposal(&pic, proxy_canister, &author_sk, delete_proposal)
+            .expect("Delete proposal execution should succeed");
+    assert!(
+        delete_proposal_result.is_none(),
+        "Delete proposal should execute immediately"
+    );
+
+    // Verify target proposal no longer exists
+    let query_response = pic
+        .query_call(
+            proxy_canister,
+            Principal::anonymous(),
+            "proposal",
+            candid::encode_one(target_proposal_id).unwrap(),
+        )
+        .expect("Query failed");
+
+    match query_response {
+        WasmResult::Reply(bytes) => {
+            let stored_proposal: Option<ICProposal> =
+                candid::decode_one(&bytes).expect("Failed to decode stored proposal");
+            assert!(
+                stored_proposal.is_none(),
+                "Target proposal should be deleted"
+            );
+        }
+        WasmResult::Reject(msg) => panic!("Query rejected: {}", msg),
+    }
+}