Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix update proxy contracts with delete #1012

Merged
merged 7 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions contracts/icp/context-config/.env
Original file line number Diff line number Diff line change
Expand Up @@ -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
Empty file modified contracts/icp/context-config/deploy_devnet.sh
100644 → 100755
Empty file.
6 changes: 3 additions & 3 deletions contracts/icp/context-proxy/dfx.json
Original file line number Diff line number Diff line change
@@ -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"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
36 changes: 35 additions & 1 deletion contracts/icp/context-proxy/src/mutate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))?;

Expand Down Expand Up @@ -137,6 +161,7 @@ async fn execute_proposal(proposal_id: &ProposalId) -> Result<(), String> {
contract.context_storage.insert(key, value);
});
}
ICProposalAction::DeleteProposal { proposal_id: _ } => {}
}
}

Expand All @@ -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
Expand Down Expand Up @@ -223,6 +256,7 @@ fn validate_proposal_action(action: &ICProposalAction) -> Result<(), String> {
}
}
ICProposalAction::SetContextValue { .. } => {}
ICProposalAction::DeleteProposal { .. } => {}
}
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/icp/context-proxy/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
240 changes: 237 additions & 3 deletions contracts/icp/context-proxy/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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),
}
}
Expand Down Expand Up @@ -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),
}
}
Loading
Loading