From cd6e08fc78a5da39246878d1eb58db7c894ebb23 Mon Sep 17 00:00:00 2001 From: Dorin Marian Iancu Date: Tue, 6 Feb 2024 15:19:59 +0200 Subject: [PATCH 1/6] multisig improvements again --- contracts/multisig/src/action.rs | 3 +- contracts/multisig/src/multisig.rs | 34 +++++++++++++++---- contracts/multisig/src/multisig_perform.rs | 22 +++++++++++- contracts/multisig/src/multisig_sign.rs | 23 ++++++++++--- .../multisig/tests/multisig_blackbox_test.rs | 4 +-- .../multisig/wasm-multisig-full/src/lib.rs | 9 +++-- contracts/multisig/wasm/src/lib.rs | 9 +++-- 7 files changed, 84 insertions(+), 20 deletions(-) diff --git a/contracts/multisig/src/action.rs b/contracts/multisig/src/action.rs index fc9d84ea..0ae3fded 100644 --- a/contracts/multisig/src/action.rs +++ b/contracts/multisig/src/action.rs @@ -3,7 +3,7 @@ use multiversx_sc::{ types::{BigUint, CodeMetadata, ManagedAddress, ManagedBuffer, ManagedVec}, }; -use crate::multisig_state::ActionId; +use crate::multisig_state::{ActionId, GroupId}; multiversx_sc::derive_imports!(); @@ -57,6 +57,7 @@ pub struct ActionFullInfo { pub action_id: ActionId, pub action_data: Action, pub signers: ManagedVec>, + pub group_id: GroupId, } #[cfg(test)] diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index 63a58c6c..f082ecaf 100644 --- a/contracts/multisig/src/multisig.rs +++ b/contracts/multisig/src/multisig.rs @@ -9,7 +9,7 @@ pub mod multisig_state; pub mod user_role; use action::ActionFullInfo; -use multisig_state::ActionId; +use multisig_state::{ActionId, GroupId}; use user_role::UserRole; multiversx_sc::imports!(); @@ -46,7 +46,7 @@ pub trait Multisig: #[upgrade] fn upgrade(&self, quorum: usize, board: MultiValueEncoded) { - self.init(quorum, board) + self.init(quorum, board); } /// Allows the contract to receive funds even if it is marked as unpayable in the protocol. @@ -72,9 +72,11 @@ pub trait Multisig: action_id, action_data, signers: self.get_action_signers(action_id), + group_id: self.group_for_action(action_id).get(), }); } } + result } @@ -87,10 +89,10 @@ pub trait Multisig: fn user_role(&self, user: ManagedAddress) -> UserRole { let user_id = self.user_mapper().get_user_id(&user); if user_id == 0 { - UserRole::None - } else { - self.user_id_to_role(user_id).get() + return UserRole::None; } + + self.user_id_to_role(user_id).get() } /// Lists all users that can sign actions. @@ -117,6 +119,7 @@ pub trait Multisig: } } } + result } @@ -124,12 +127,31 @@ pub trait Multisig: /// Any signatures that the action received must first be removed, via `unsign`. /// Otherwise this endpoint would be prone to abuse. #[endpoint(discardAction)] - fn discard_action(&self, action_id: ActionId) { + fn discard_action_endpoint(&self, action_id: ActionId) { let (_, caller_role) = self.get_caller_id_and_role(); require!( caller_role.can_discard_action(), "only board members and proposers can discard actions" ); + + self.discard_action(action_id); + } + + /// Discard all the actions in the given group + #[endpoint(discardBatch)] + fn discard_batch(&self, group_id: GroupId) { + let (_, caller_role) = self.get_caller_id_and_role(); + require!( + caller_role.can_discard_action(), + "only board members and proposers can discard actions" + ); + + for action_id in self.action_groups(group_id).iter() { + self.discard_action(action_id); + } + } + + fn discard_action(&self, action_id: ActionId) { require!( self.get_action_valid_signer_count(action_id) == 0, "cannot discard action with valid signatures" diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index 1b89e183..2cb28a02 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -1,6 +1,6 @@ use crate::{ action::{Action, ActionFullInfo, GasLimit}, - multisig_state::ActionId, + multisig_state::{ActionId, GroupId}, user_role::UserRole, }; @@ -115,6 +115,25 @@ pub trait MultisigPerformModule: self.perform_action(action_id) } + /// Perform all the actions in the given group + #[endpoint(performBatch)] + fn perform_batch(&self, group_id: GroupId) { + let (_, caller_role) = self.get_caller_id_and_role(); + require!( + caller_role.can_perform_action(), + "only board members and proposers can perform actions" + ); + + for action_id in self.action_groups(group_id).iter() { + require!( + self.quorum_reached(action_id), + "quorum has not been reached" + ); + + let _ = self.perform_action(action_id); + } + } + fn perform_action(&self, action_id: ActionId) -> OptionalValue { let action = self.action_mapper().get(action_id); @@ -122,6 +141,7 @@ pub trait MultisigPerformModule: action_id, action_data: action.clone(), signers: self.get_action_signers(action_id), + group_id: self.group_for_action(action_id).get(), }); // clean up storage diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index a9f34574..685a666f 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -1,4 +1,4 @@ -use crate::multisig_state::ActionId; +use crate::multisig_state::{ActionId, GroupId}; multiversx_sc::imports!(); @@ -58,14 +58,29 @@ pub trait MultisigSignModule: /// Actions that are left with no valid signatures can be then deleted to free up storage. #[endpoint] fn unsign(&self, action_id: ActionId) { + let (caller_id, caller_role) = self.get_caller_id_and_role(); + require!(caller_role.can_sign(), "only board members can un-sign"); + + self.unsign_action(action_id, caller_id); + } + + /// Unsign all actions in the given group + #[endpoint(unsignBatch)] + fn unsign_batch(&self, group_id: GroupId) { + let (caller_id, caller_role) = self.get_caller_id_and_role(); + require!(caller_role.can_sign(), "only board members can un-sign"); + + for action_id in self.action_groups(group_id).iter() { + self.unsign_action(action_id, caller_id); + } + } + + fn unsign_action(&self, action_id: ActionId, caller_id: usize) { require!( !self.action_mapper().item_is_empty_unchecked(action_id), "action does not exist" ); - let (caller_id, caller_role) = self.get_caller_id_and_role(); - require!(caller_role.can_sign(), "only board members can un-sign"); - let _ = self.action_signer_ids(action_id).swap_remove(&caller_id); } diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index 6300508f..e00afba5 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -377,7 +377,7 @@ fn test_change_quorum() { state.world.sc_call( ScCallStep::new() .from(BOARD_MEMBER_ADDRESS_EXPR) - .call(state.multisig_contract.discard_action(action_id)) + .call(state.multisig_contract.discard_action_endpoint(action_id)) .expect(TxExpect::user_error( "str:cannot discard action with valid signatures", )), @@ -393,7 +393,7 @@ fn test_change_quorum() { state.world.sc_call( ScCallStep::new() .from(BOARD_MEMBER_ADDRESS_EXPR) - .call(state.multisig_contract.discard_action(action_id)), + .call(state.multisig_contract.discard_action_endpoint(action_id)), ); // try sign discarded action diff --git a/contracts/multisig/wasm-multisig-full/src/lib.rs b/contracts/multisig/wasm-multisig-full/src/lib.rs index a4aa2a8d..e6d03c89 100644 --- a/contracts/multisig/wasm-multisig-full/src/lib.rs +++ b/contracts/multisig/wasm-multisig-full/src/lib.rs @@ -5,9 +5,9 @@ //////////////////////////////////////////////////// // Init: 1 -// Endpoints: 33 +// Endpoints: 36 // Async Callback: 1 -// Total number of exported functions: 35 +// Total number of exported functions: 38 #![no_std] #![allow(internal_features)] @@ -22,7 +22,8 @@ multiversx_sc_wasm_adapter::endpoints! { init => init upgrade => upgrade deposit => deposit - discardAction => discard_action + discardAction => discard_action_endpoint + discardBatch => discard_batch getQuorum => quorum getNumBoardMembers => num_board_members getNumProposers => num_proposers @@ -41,9 +42,11 @@ multiversx_sc_wasm_adapter::endpoints! { signAndPerform => sign_and_perform signBatchAndPerform => sign_batch_and_perform unsign => unsign + unsignBatch => unsign_batch signed => signed quorumReached => quorum_reached performAction => perform_action_endpoint + performBatch => perform_batch dnsRegister => dns_register getPendingActionFullInfo => get_pending_action_full_info userRole => user_role diff --git a/contracts/multisig/wasm/src/lib.rs b/contracts/multisig/wasm/src/lib.rs index b372a340..7d667377 100644 --- a/contracts/multisig/wasm/src/lib.rs +++ b/contracts/multisig/wasm/src/lib.rs @@ -5,9 +5,9 @@ //////////////////////////////////////////////////// // Init: 1 -// Endpoints: 25 +// Endpoints: 28 // Async Callback: 1 -// Total number of exported functions: 27 +// Total number of exported functions: 30 #![no_std] #![allow(internal_features)] @@ -22,7 +22,8 @@ multiversx_sc_wasm_adapter::endpoints! { init => init upgrade => upgrade deposit => deposit - discardAction => discard_action + discardAction => discard_action_endpoint + discardBatch => discard_batch getQuorum => quorum getNumBoardMembers => num_board_members getNumProposers => num_proposers @@ -41,9 +42,11 @@ multiversx_sc_wasm_adapter::endpoints! { signAndPerform => sign_and_perform signBatchAndPerform => sign_batch_and_perform unsign => unsign + unsignBatch => unsign_batch signed => signed quorumReached => quorum_reached performAction => perform_action_endpoint + performBatch => perform_batch dnsRegister => dns_register ) } From de093d25111aacb479a53be1c7105ba93ea88654 Mon Sep 17 00:00:00 2001 From: Dorin Marian Iancu Date: Tue, 6 Feb 2024 15:22:57 +0200 Subject: [PATCH 2/6] fix test --- contracts/multisig/scenarios/changeQuorum.scen.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/multisig/scenarios/changeQuorum.scen.json b/contracts/multisig/scenarios/changeQuorum.scen.json index 0ccc305d..2b407b62 100644 --- a/contracts/multisig/scenarios/changeQuorum.scen.json +++ b/contracts/multisig/scenarios/changeQuorum.scen.json @@ -65,7 +65,7 @@ [ "u32:1", "u8:4|u32:3", - "u32:2|address:alice|address:bob" + "u32:2|address:alice|address:bob|u32:0" ] ], "status": "" From 4e2105756689c88a1f29579bb3c29cb152df29d4 Mon Sep 17 00:00:00 2001 From: Dorin Marian Iancu Date: Tue, 6 Feb 2024 15:29:24 +0200 Subject: [PATCH 3/6] fix mapper being used in different places --- contracts/multisig/src/multisig.rs | 5 +++++ contracts/multisig/src/multisig_perform.rs | 5 +++++ contracts/multisig/src/multisig_sign.rs | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index f082ecaf..97fc7ca7 100644 --- a/contracts/multisig/src/multisig.rs +++ b/contracts/multisig/src/multisig.rs @@ -146,7 +146,12 @@ pub trait Multisig: "only board members and proposers can discard actions" ); + let mut action_ids = ManagedVec::::new(); for action_id in self.action_groups(group_id).iter() { + action_ids.push(action_id); + } + + for action_id in &action_ids { self.discard_action(action_id); } } diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index 2cb28a02..2f8e39b8 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -124,7 +124,12 @@ pub trait MultisigPerformModule: "only board members and proposers can perform actions" ); + let mut action_ids = ManagedVec::::new(); for action_id in self.action_groups(group_id).iter() { + action_ids.push(action_id); + } + + for action_id in &action_ids { require!( self.quorum_reached(action_id), "quorum has not been reached" diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index 685a666f..f59208fc 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -70,7 +70,12 @@ pub trait MultisigSignModule: let (caller_id, caller_role) = self.get_caller_id_and_role(); require!(caller_role.can_sign(), "only board members can un-sign"); + let mut action_ids = ManagedVec::::new(); for action_id in self.action_groups(group_id).iter() { + action_ids.push(action_id); + } + + for action_id in &action_ids { self.unsign_action(action_id, caller_id); } } From b0b2c5f35e5f01d0c36190cb809c4bb758a7dc39 Mon Sep 17 00:00:00 2001 From: Dorin Marian Iancu Date: Tue, 6 Feb 2024 15:33:31 +0200 Subject: [PATCH 4/6] fixes after review --- contracts/multisig/src/action.rs | 2 +- contracts/multisig/src/multisig.rs | 13 ++++--------- contracts/multisig/src/multisig_perform.rs | 13 ++++--------- contracts/multisig/src/multisig_sign.rs | 13 ++++--------- 4 files changed, 13 insertions(+), 28 deletions(-) diff --git a/contracts/multisig/src/action.rs b/contracts/multisig/src/action.rs index 0ae3fded..726a0888 100644 --- a/contracts/multisig/src/action.rs +++ b/contracts/multisig/src/action.rs @@ -55,9 +55,9 @@ impl Action { #[derive(TopEncode, TypeAbi)] pub struct ActionFullInfo { pub action_id: ActionId, + pub group_id: GroupId, pub action_data: Action, pub signers: ManagedVec>, - pub group_id: GroupId, } #[cfg(test)] diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index 97fc7ca7..ea22036f 100644 --- a/contracts/multisig/src/multisig.rs +++ b/contracts/multisig/src/multisig.rs @@ -9,7 +9,7 @@ pub mod multisig_state; pub mod user_role; use action::ActionFullInfo; -use multisig_state::{ActionId, GroupId}; +use multisig_state::ActionId; use user_role::UserRole; multiversx_sc::imports!(); @@ -137,21 +137,16 @@ pub trait Multisig: self.discard_action(action_id); } - /// Discard all the actions in the given group + /// Discard all the actions with the given IDs #[endpoint(discardBatch)] - fn discard_batch(&self, group_id: GroupId) { + fn discard_batch(&self, action_ids: MultiValueEncoded) { let (_, caller_role) = self.get_caller_id_and_role(); require!( caller_role.can_discard_action(), "only board members and proposers can discard actions" ); - let mut action_ids = ManagedVec::::new(); - for action_id in self.action_groups(group_id).iter() { - action_ids.push(action_id); - } - - for action_id in &action_ids { + for action_id in action_ids { self.discard_action(action_id); } } diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index 2f8e39b8..90e4d2f6 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -1,6 +1,6 @@ use crate::{ action::{Action, ActionFullInfo, GasLimit}, - multisig_state::{ActionId, GroupId}, + multisig_state::ActionId, user_role::UserRole, }; @@ -115,21 +115,16 @@ pub trait MultisigPerformModule: self.perform_action(action_id) } - /// Perform all the actions in the given group + /// Perform all the actions with the given IDs #[endpoint(performBatch)] - fn perform_batch(&self, group_id: GroupId) { + fn perform_batch(&self, action_ids: MultiValueEncoded) { let (_, caller_role) = self.get_caller_id_and_role(); require!( caller_role.can_perform_action(), "only board members and proposers can perform actions" ); - let mut action_ids = ManagedVec::::new(); - for action_id in self.action_groups(group_id).iter() { - action_ids.push(action_id); - } - - for action_id in &action_ids { + for action_id in action_ids { require!( self.quorum_reached(action_id), "quorum has not been reached" diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index f59208fc..28fc2338 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -1,4 +1,4 @@ -use crate::multisig_state::{ActionId, GroupId}; +use crate::multisig_state::ActionId; multiversx_sc::imports!(); @@ -64,18 +64,13 @@ pub trait MultisigSignModule: self.unsign_action(action_id, caller_id); } - /// Unsign all actions in the given group + /// Unsign all actions with the given IDs #[endpoint(unsignBatch)] - fn unsign_batch(&self, group_id: GroupId) { + fn unsign_batch(&self, action_ids: MultiValueEncoded) { let (caller_id, caller_role) = self.get_caller_id_and_role(); require!(caller_role.can_sign(), "only board members can un-sign"); - let mut action_ids = ManagedVec::::new(); - for action_id in self.action_groups(group_id).iter() { - action_ids.push(action_id); - } - - for action_id in &action_ids { + for action_id in action_ids { self.unsign_action(action_id, caller_id); } } From d66e498630ae066307ed663d6ffe842e971585b3 Mon Sep 17 00:00:00 2001 From: Dorin Marian Iancu Date: Tue, 6 Feb 2024 15:39:10 +0200 Subject: [PATCH 5/6] fix test --- contracts/multisig/scenarios/changeQuorum.scen.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contracts/multisig/scenarios/changeQuorum.scen.json b/contracts/multisig/scenarios/changeQuorum.scen.json index 2b407b62..390c32fc 100644 --- a/contracts/multisig/scenarios/changeQuorum.scen.json +++ b/contracts/multisig/scenarios/changeQuorum.scen.json @@ -64,8 +64,9 @@ "out": [ [ "u32:1", + "u32:0", "u8:4|u32:3", - "u32:2|address:alice|address:bob|u32:0" + "u32:2|address:alice|address:bob" ] ], "status": "" @@ -127,4 +128,4 @@ } } ] -} +} \ No newline at end of file From 2ec699725230f804f957a2fb5c3c1403b2eb42c1 Mon Sep 17 00:00:00 2001 From: Dorin Marian Iancu Date: Tue, 6 Feb 2024 15:39:46 +0200 Subject: [PATCH 6/6] newline --- contracts/multisig/scenarios/changeQuorum.scen.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/multisig/scenarios/changeQuorum.scen.json b/contracts/multisig/scenarios/changeQuorum.scen.json index 390c32fc..3f8baf2d 100644 --- a/contracts/multisig/scenarios/changeQuorum.scen.json +++ b/contracts/multisig/scenarios/changeQuorum.scen.json @@ -128,4 +128,4 @@ } } ] -} \ No newline at end of file +}