From cb38a5d8ffc3e587c9669f93b3c85fc051f94c2d Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Tue, 5 Mar 2024 09:55:49 +0200 Subject: [PATCH 01/44] audit fixes part 1 --- contracts/multisig/src/action.rs | 30 ++++++-- contracts/multisig/src/multisig.rs | 57 ++++++++++++-- contracts/multisig/src/multisig_perform.rs | 75 ++++++++++++++----- contracts/multisig/src/multisig_propose.rs | 24 ++++-- contracts/multisig/src/multisig_sign.rs | 28 ++++++- contracts/multisig/src/multisig_state.rs | 18 ++++- .../multisig/wasm-multisig-full/src/lib.rs | 5 +- .../multisig/wasm-multisig-view/src/lib.rs | 5 +- 8 files changed, 197 insertions(+), 45 deletions(-) diff --git a/contracts/multisig/src/action.rs b/contracts/multisig/src/action.rs index 3bb37af8..88b9846d 100644 --- a/contracts/multisig/src/action.rs +++ b/contracts/multisig/src/action.rs @@ -19,6 +19,15 @@ pub struct CallActionData { pub arguments: ManagedVec>, } +#[derive(NestedEncode, NestedDecode, TypeAbi, Clone)] +pub struct EsdtTransferExecuteData { + pub to: ManagedAddress, + pub tokens: PaymentsVec, + pub opt_gas_limit: Option, + pub endpoint_name: ManagedBuffer, + pub arguments: ManagedVec>, +} + #[derive(NestedEncode, NestedDecode, TopEncode, TopDecode, TypeAbi, Clone)] pub enum Action { Nothing, @@ -27,13 +36,7 @@ pub enum Action { RemoveUser(ManagedAddress), ChangeQuorum(usize), SendTransferExecuteEgld(CallActionData), - SendTransferExecuteEsdt { - to: ManagedAddress, - tokens: PaymentsVec, - opt_gas_limit: Option, - endpoint_name: ManagedBuffer, - arguments: ManagedVec>, - }, + SendTransferExecuteEsdt(EsdtTransferExecuteData), SendAsyncCall(CallActionData), SCDeployFromSource { amount: BigUint, @@ -65,6 +68,19 @@ impl Action { pub fn is_async_call(&self) -> bool { matches!(*self, Action::SendAsyncCall(_)) } + + pub fn is_sc_upgrade(&self) -> bool { + matches!( + self, + Action::SCUpgradeFromSource { + sc_address: _, + amount: _, + source: _, + code_metadata: _, + arguments: _ + } + ) + } } /// Not used internally, just to retrieve results via endpoint. diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index ea22036f..0d3f04e5 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, ActionStatus}; use user_role::UserRole; multiversx_sc::imports!(); @@ -45,9 +45,7 @@ pub trait Multisig: } #[upgrade] - fn upgrade(&self, quorum: usize, board: MultiValueEncoded) { - self.init(quorum, board); - } + fn upgrade(&self) {} /// Allows the contract to receive funds even if it is marked as unpayable in the protocol. #[payable("*")] @@ -61,11 +59,51 @@ pub trait Multisig: /// - (number of signers followed by) list of signer addresses. #[label("multisig-external-view")] #[view(getPendingActionFullInfo)] - fn get_pending_action_full_info(&self) -> MultiValueEncoded> { + fn get_pending_action_full_info( + &self, + offset: OptionalValue, + ) -> MultiValueEncoded> { + let mut result = MultiValueEncoded::new(); + let action_last_index = self.get_action_last_index(); + let action_mapper = self.action_mapper(); + let mut index_of_first_action = 1; + if let OptionalValue::Some(unwrapped_offset) = offset { + require!( + unwrapped_offset <= action_last_index, + "offset needs to be smaller than the total number of actions" + ); + index_of_first_action += action_last_index - unwrapped_offset; + } + for action_id in index_of_first_action..=action_last_index { + let action_data = action_mapper.get(action_id); + if action_data.is_pending() { + result.push(ActionFullInfo { + action_id, + action_data, + signers: self.get_action_signers(action_id), + group_id: self.group_for_action(action_id).get(), + }); + } + } + + result + } + + #[label("multisig-external-view")] + #[view(getPendingActionFullInfoInRange)] + fn get_pending_action_full_info_in_range( + &self, + action_start_id: usize, + action_stop_id: usize, + ) -> MultiValueEncoded> { let mut result = MultiValueEncoded::new(); let action_last_index = self.get_action_last_index(); let action_mapper = self.action_mapper(); - for action_id in 1..=action_last_index { + require!( + action_start_id < action_stop_id && action_stop_id <= action_last_index, + "action ids provided need to be in the range of the available actions" + ); + for action_id in action_start_id..=action_stop_id { let action_data = action_mapper.get(action_id); if action_data.is_pending() { result.push(ActionFullInfo { @@ -156,7 +194,12 @@ pub trait Multisig: self.get_action_valid_signer_count(action_id) == 0, "cannot discard action with valid signatures" ); - + self.abort_batch_of_action(action_id); self.clear_action(action_id); } + fn abort_batch_of_action(&self, action_id: ActionId) { + let batch_id = self.group_for_action(action_id).get(); + self.action_group_status(batch_id) + .set(ActionStatus::Aborted); + } } diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index dc953796..f188df1e 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, ActionStatus, GroupId}, user_role::UserRole, }; @@ -8,6 +8,7 @@ multiversx_sc::imports!(); /// Gas required to finish transaction after transfer-execute. const PERFORM_ACTION_FINISH_GAS: u64 = 300_000; +const MAX_BOARD_MEMBERS: usize = 30; fn usize_add_isize(value: &mut usize, delta: isize) { *value = (*value as isize + delta) as usize; @@ -84,7 +85,7 @@ pub trait MultisigPerformModule: /// Returns `true` (`1`) if `getActionValidSignerCount >= getQuorum`. #[view(quorumReached)] fn quorum_reached(&self, action_id: ActionId) -> bool { - let quorum = self.quorum().get(); + let quorum = self.quorum_for_action(action_id).get(); let valid_signers_count = self.get_action_valid_signer_count(action_id); valid_signers_count >= quorum } @@ -118,6 +119,21 @@ pub trait MultisigPerformModule: self.perform_action(action_id) } + fn try_perform_action(&self, action_id: ActionId) -> OptionalValue { + let (_, caller_role) = self.get_caller_id_and_role(); + require!( + caller_role.can_perform_action(), + "only board members and proposers can perform actions" + ); + if self.quorum_reached(action_id) { + let group_id = self.group_for_action(action_id).get(); + require!(group_id == 0, "May not execute this action by itself"); + + return self.perform_action(action_id); + } + OptionalValue::None + } + /// Perform all the actions in the given batch #[endpoint(performBatch)] fn perform_batch(&self, group_id: GroupId) { @@ -127,6 +143,12 @@ pub trait MultisigPerformModule: "only board members and proposers can perform actions" ); + let group_status = self.action_group_status(group_id).get(); + require!( + group_status == ActionStatus::Available, + "cannot perform actions of an aborted batch" + ); + let mapper = self.action_groups(group_id); require!(!mapper.is_empty(), "Invalid group ID"); @@ -148,11 +170,17 @@ pub trait MultisigPerformModule: fn perform_action(&self, action_id: ActionId) -> OptionalValue { let action = self.action_mapper().get(action_id); + let group_id = self.group_for_action(action_id).get(); + let group_status = self.action_group_status(group_id).get(); + require!( + group_status == ActionStatus::Available, + "cannot perform actions of an aborted batch" + ); self.start_perform_action_event(&ActionFullInfo { action_id, action_data: action.clone(), signers: self.get_action_signers(action_id), - group_id: self.group_for_action(action_id).get(), + group_id, }); // clean up storage @@ -164,6 +192,10 @@ pub trait MultisigPerformModule: Action::Nothing => OptionalValue::None, Action::AddBoardMember(board_member_address) => { self.change_user_role(action_id, board_member_address, UserRole::BoardMember); + require!( + self.num_board_members().get() <= MAX_BOARD_MEMBERS, + "board size cannot exceed limit" + ); OptionalValue::None } Action::AddProposer(proposer_address) => { @@ -200,6 +232,7 @@ pub trait MultisigPerformModule: OptionalValue::None } Action::SendTransferExecuteEgld(call_data) => { + require!(call_data.egld_amount != 0, "EGLD amount cannot be zero"); let gas = call_data .opt_gas_limit .unwrap_or_else(|| self.gas_for_transfer_exec()); @@ -224,28 +257,29 @@ pub trait MultisigPerformModule: OptionalValue::None } - Action::SendTransferExecuteEsdt { - to, - tokens, - opt_gas_limit, - endpoint_name, - arguments, - } => { - let gas = opt_gas_limit.unwrap_or_else(|| self.blockchain().get_gas_left()); + Action::SendTransferExecuteEsdt(call_data) => { + let gas = call_data + .opt_gas_limit + .unwrap_or_else(|| self.blockchain().get_gas_left()); + require!(gas > PERFORM_ACTION_FINISH_GAS, "insufficient gas for call"); + require!( + call_data.tokens.len() != 0, + "number of tokens cannot be zero" + ); self.perform_transfer_execute_esdt_event( action_id, - &to, - &tokens, + &call_data.to, + &call_data.tokens, gas, - &endpoint_name, - arguments.as_multi(), + &call_data.endpoint_name, + call_data.arguments.as_multi(), ); let result = self.send_raw().multi_esdt_transfer_execute( - &to, - &tokens, + &call_data.to, + &call_data.tokens, gas, - &endpoint_name, - &arguments.into(), + &call_data.endpoint_name, + &call_data.arguments.into(), ); if let Result::Err(e) = result { sc_panic!(e); @@ -257,6 +291,9 @@ pub trait MultisigPerformModule: let gas = call_data .opt_gas_limit .unwrap_or_else(|| self.blockchain().get_gas_left()); + if gas <= PERFORM_ACTION_FINISH_GAS { + sc_panic!("insufficient gas for call"); + } self.perform_async_call_event( action_id, &call_data.to, diff --git a/contracts/multisig/src/multisig_propose.rs b/contracts/multisig/src/multisig_propose.rs index 2950e584..dd50d8a8 100644 --- a/contracts/multisig/src/multisig_propose.rs +++ b/contracts/multisig/src/multisig_propose.rs @@ -1,8 +1,8 @@ use multiversx_sc_modules::transfer_role_proxy::PaymentsVec; use crate::{ - action::{Action, CallActionData, GasLimit}, - multisig_state::{ActionId, GroupId}, + action::{Action, CallActionData, EsdtTransferExecuteData, GasLimit}, + multisig_state::{ActionId, ActionStatus, GroupId}, }; multiversx_sc::imports!(); @@ -18,6 +18,7 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { ); let action_id = self.action_mapper().push(&action); + self.quorum_for_action(action_id).set(self.quorum().get()); if caller_role.can_sign() { // also sign // since the action is newly created, the caller can be the only signer @@ -90,13 +91,15 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { ) -> ActionId { require!(!tokens.is_empty(), "No tokens to transfer"); - self.propose_action(Action::SendTransferExecuteEsdt { + let call_data = EsdtTransferExecuteData { to, tokens, opt_gas_limit, endpoint_name: function_call.function_name, arguments: function_call.arg_buffer.into_vec_of_buffers(), - }) + }; + + self.propose_action(Action::SendTransferExecuteEsdt(call_data)) } /// Propose a transaction in which the contract will perform an async call call. @@ -178,9 +181,14 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { let mut action_mapper = self.action_mapper(); let mut action_groups_mapper = self.action_groups(group_id); + self.action_group_status(group_id) + .set(ActionStatus::Available); + + require!(!action_groups_mapper.is_empty(), "group cannot be empty"); + for action in actions { require!( - !action.is_nothing() && !action.is_async_call(), + !action.is_nothing() && !action.is_async_call() && !action.is_sc_upgrade(), "Invalid action" ); @@ -190,6 +198,12 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { own_shard == other_sc_shard, "All transfer exec must be to the same shard" ); + } else if let Action::SendTransferExecuteEsdt(call_data) = &action { + let other_sc_shard = self.blockchain().get_shard_of_address(&call_data.to); + require!( + own_shard == other_sc_shard, + "All transfer exec must be to the same shard" + ); } let action_id = action_mapper.push(&action); diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index 54c848f8..0e44f692 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, ActionStatus, GroupId}; multiversx_sc::imports!(); @@ -16,6 +16,13 @@ pub trait MultisigSignModule: !self.action_mapper().item_is_empty_unchecked(action_id), "action does not exist" ); + let group_id = self.group_for_action(action_id).get(); + + let group_status = self.action_group_status(group_id).get(); + require!( + group_status == ActionStatus::Available, + "cannot sign actions of an aborted batch" + ); let (caller_id, caller_role) = self.get_caller_id_and_role(); require!(caller_role.can_sign(), "only board members can sign"); @@ -29,6 +36,11 @@ pub trait MultisigSignModule: let (caller_id, caller_role) = self.get_caller_id_and_role(); require!(caller_role.can_sign(), "only board members can sign"); + let group_status = self.action_group_status(group_id).get(); + require!( + group_status == ActionStatus::Available, + "cannot sign actions of an aborted batch" + ); let mapper = self.action_groups(group_id); require!(!mapper.is_empty(), "Invalid group ID"); @@ -45,7 +57,7 @@ pub trait MultisigSignModule: #[endpoint(signAndPerform)] fn sign_and_perform(&self, action_id: ActionId) -> OptionalValue { self.sign(action_id); - self.perform_action_endpoint(action_id) + self.try_perform_action(action_id) } #[endpoint(signBatchAndPerform)] @@ -63,6 +75,13 @@ pub trait MultisigSignModule: 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"); + let group_id = self.group_for_action(action_id).get(); + + let group_status = self.action_group_status(group_id).get(); + require!( + group_status == ActionStatus::Available, + "cannot sign actions of an aborted batch" + ); self.unsign_action(action_id, caller_id); } @@ -73,6 +92,11 @@ 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 group_status = self.action_group_status(group_id).get(); + require!( + group_status == ActionStatus::Available, + "cannot sign actions of an aborted batch" + ); let mapper = self.action_groups(group_id); require!(!mapper.is_empty(), "Invalid group ID"); diff --git a/contracts/multisig/src/multisig_state.rs b/contracts/multisig/src/multisig_state.rs index 61d37c0a..293a4baf 100644 --- a/contracts/multisig/src/multisig_state.rs +++ b/contracts/multisig/src/multisig_state.rs @@ -1,11 +1,20 @@ use crate::{action::Action, user_role::UserRole}; multiversx_sc::imports!(); +multiversx_sc::derive_imports!(); pub type ActionId = usize; pub type GroupId = usize; pub type UserId = usize; +#[derive( + TopEncode, TopDecode, NestedEncode, NestedDecode, TypeAbi, PartialEq, Eq, Clone, Copy, Debug, +)] +pub enum ActionStatus { + Available, + Aborted, +} + /// Contains all events that can be emitted by the contract. #[multiversx_sc::module] pub trait MultisigStateModule { @@ -14,9 +23,12 @@ pub trait MultisigStateModule { #[storage_mapper("quorum")] fn quorum(&self) -> SingleValueMapper; - #[storage_mapper("user")] + #[storage_mapper("sc_user")] fn user_mapper(&self) -> UserMapper; + #[storage_mapper("quorum_for_action")] + fn quorum_for_action(&self, action_id: ActionId) -> SingleValueMapper; + #[storage_mapper("user_role")] fn user_id_to_role(&self, user_id: UserId) -> SingleValueMapper; @@ -66,6 +78,10 @@ pub trait MultisigStateModule { #[storage_mapper("action_groups")] fn action_groups(&self, group_id: GroupId) -> UnorderedSetMapper; + #[view(getActionGroup)] + #[storage_mapper("action_group_status")] + fn action_group_status(&self, group_id: GroupId) -> SingleValueMapper; + #[storage_mapper("group_for_action")] fn group_for_action(&self, action_id: ActionId) -> SingleValueMapper; diff --git a/contracts/multisig/wasm-multisig-full/src/lib.rs b/contracts/multisig/wasm-multisig-full/src/lib.rs index 82b2b780..4177dc89 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: 38 +// Endpoints: 39 // Async Callback: 1 -// Total number of exported functions: 40 +// Total number of exported functions: 41 #![no_std] #![allow(internal_features)] @@ -51,6 +51,7 @@ multiversx_sc_wasm_adapter::endpoints! { performBatch => perform_batch dnsRegister => dns_register getPendingActionFullInfo => get_pending_action_full_info + getPendingActionFullInfoInRange => get_pending_action_full_info_in_range userRole => user_role getAllBoardMembers => get_all_board_members getAllProposers => get_all_proposers diff --git a/contracts/multisig/wasm-multisig-view/src/lib.rs b/contracts/multisig/wasm-multisig-view/src/lib.rs index c01d319c..bc0f1a26 100644 --- a/contracts/multisig/wasm-multisig-view/src/lib.rs +++ b/contracts/multisig/wasm-multisig-view/src/lib.rs @@ -5,9 +5,9 @@ //////////////////////////////////////////////////// // Init: 1 -// Endpoints: 8 +// Endpoints: 9 // Async Callback (empty): 1 -// Total number of exported functions: 10 +// Total number of exported functions: 11 #![no_std] #![allow(internal_features)] @@ -22,6 +22,7 @@ multiversx_sc_wasm_adapter::external_view_endpoints! { multisig ( getPendingActionFullInfo => get_pending_action_full_info + getPendingActionFullInfoInRange => get_pending_action_full_info_in_range userRole => user_role getAllBoardMembers => get_all_board_members getAllProposers => get_all_proposers From a85f0d3614a93db074a4afba394c0de97a785126 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Tue, 5 Mar 2024 10:05:52 +0200 Subject: [PATCH 02/44] clippy --- contracts/multisig/src/multisig_perform.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index f188df1e..4f032fd7 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -263,7 +263,7 @@ pub trait MultisigPerformModule: .unwrap_or_else(|| self.blockchain().get_gas_left()); require!(gas > PERFORM_ACTION_FINISH_GAS, "insufficient gas for call"); require!( - call_data.tokens.len() != 0, + call_data.tokens.is_empty(), "number of tokens cannot be zero" ); self.perform_transfer_execute_esdt_event( From aa516c047ebd541d35e149a64e725f79ef428a44 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Tue, 5 Mar 2024 10:23:27 +0200 Subject: [PATCH 03/44] solve A.4. --- contracts/multisig/src/multisig_propose.rs | 28 +++++++++++++++------- contracts/multisig/src/multisig_state.rs | 4 ++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/contracts/multisig/src/multisig_propose.rs b/contracts/multisig/src/multisig_propose.rs index dd50d8a8..e2affe37 100644 --- a/contracts/multisig/src/multisig_propose.rs +++ b/contracts/multisig/src/multisig_propose.rs @@ -166,8 +166,21 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { } #[endpoint(proposeBatch)] - fn propose_batch(&self, group_id: GroupId, actions: MultiValueEncoded>) { - require!(group_id != 0, "May not use group ID 0"); + fn propose_batch( + &self, + actions: MultiValueEncoded>, + group_id: OptionalValue, + ) { + let mut batch_id = self.num_groups().get() + 1; + let mut is_new_batch = false; + if let OptionalValue::Some(unwrapped_group_id) = group_id { + require!(batch_id != 0, "May not use group ID 0"); + batch_id = unwrapped_group_id; + } else { + is_new_batch = true; + self.action_group_status(batch_id) + .set(ActionStatus::Available); + } require!(!actions.is_empty(), "No actions"); let (caller_id, caller_role) = self.get_caller_id_and_role(); @@ -180,12 +193,11 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { let own_shard = self.blockchain().get_shard_of_address(&own_sc_address); let mut action_mapper = self.action_mapper(); - let mut action_groups_mapper = self.action_groups(group_id); - self.action_group_status(group_id) - .set(ActionStatus::Available); - - require!(!action_groups_mapper.is_empty(), "group cannot be empty"); + let mut action_groups_mapper = self.action_groups(batch_id); + if !is_new_batch { + require!(!action_groups_mapper.is_empty(), "group cannot be empty"); + } for action in actions { require!( !action.is_nothing() && !action.is_async_call() && !action.is_sc_upgrade(), @@ -212,7 +224,7 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { } let _ = action_groups_mapper.insert(action_id); - self.group_for_action(action_id).set(group_id); + self.group_for_action(action_id).set(batch_id); } } } diff --git a/contracts/multisig/src/multisig_state.rs b/contracts/multisig/src/multisig_state.rs index 293a4baf..32ce5815 100644 --- a/contracts/multisig/src/multisig_state.rs +++ b/contracts/multisig/src/multisig_state.rs @@ -45,6 +45,10 @@ pub trait MultisigStateModule { #[storage_mapper("num_board_members")] fn num_board_members(&self) -> SingleValueMapper; + #[view(getNumGroups)] + #[storage_mapper("num_groups")] + fn num_groups(&self) -> SingleValueMapper; + /// Denormalized proposer count. /// It is kept in sync with the user list by the contract. #[view(getNumProposers)] From 62ef538f633c6de0af5441b1e0e889bb45f11d14 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Tue, 5 Mar 2024 12:50:17 +0200 Subject: [PATCH 04/44] undo group is as option in propose batch --- contracts/multisig/src/multisig_propose.rs | 28 +++++++--------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/contracts/multisig/src/multisig_propose.rs b/contracts/multisig/src/multisig_propose.rs index e2affe37..dd50d8a8 100644 --- a/contracts/multisig/src/multisig_propose.rs +++ b/contracts/multisig/src/multisig_propose.rs @@ -166,21 +166,8 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { } #[endpoint(proposeBatch)] - fn propose_batch( - &self, - actions: MultiValueEncoded>, - group_id: OptionalValue, - ) { - let mut batch_id = self.num_groups().get() + 1; - let mut is_new_batch = false; - if let OptionalValue::Some(unwrapped_group_id) = group_id { - require!(batch_id != 0, "May not use group ID 0"); - batch_id = unwrapped_group_id; - } else { - is_new_batch = true; - self.action_group_status(batch_id) - .set(ActionStatus::Available); - } + fn propose_batch(&self, group_id: GroupId, actions: MultiValueEncoded>) { + require!(group_id != 0, "May not use group ID 0"); require!(!actions.is_empty(), "No actions"); let (caller_id, caller_role) = self.get_caller_id_and_role(); @@ -193,11 +180,12 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { let own_shard = self.blockchain().get_shard_of_address(&own_sc_address); let mut action_mapper = self.action_mapper(); - let mut action_groups_mapper = self.action_groups(batch_id); + let mut action_groups_mapper = self.action_groups(group_id); + self.action_group_status(group_id) + .set(ActionStatus::Available); + + require!(!action_groups_mapper.is_empty(), "group cannot be empty"); - if !is_new_batch { - require!(!action_groups_mapper.is_empty(), "group cannot be empty"); - } for action in actions { require!( !action.is_nothing() && !action.is_async_call() && !action.is_sc_upgrade(), @@ -224,7 +212,7 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { } let _ = action_groups_mapper.insert(action_id); - self.group_for_action(action_id).set(batch_id); + self.group_for_action(action_id).set(group_id); } } } From 21864719ec0a3af73a81ecf9d5194aa2ad6fa6d2 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Tue, 5 Mar 2024 13:13:22 +0200 Subject: [PATCH 05/44] A.1 & A.4 solved --- contracts/multisig/src/multisig_propose.rs | 11 ++++++++--- contracts/multisig/src/multisig_state.rs | 4 ++++ contracts/multisig/wasm-multisig-full/src/lib.rs | 5 +++-- contracts/multisig/wasm/src/lib.rs | 5 +++-- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/contracts/multisig/src/multisig_propose.rs b/contracts/multisig/src/multisig_propose.rs index dd50d8a8..6c765c42 100644 --- a/contracts/multisig/src/multisig_propose.rs +++ b/contracts/multisig/src/multisig_propose.rs @@ -166,8 +166,8 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { } #[endpoint(proposeBatch)] - fn propose_batch(&self, group_id: GroupId, actions: MultiValueEncoded>) { - require!(group_id != 0, "May not use group ID 0"); + fn propose_batch(&self, actions: MultiValueEncoded>) -> GroupId { + let group_id = self.last_action_group_id().get() + 1; require!(!actions.is_empty(), "No actions"); let (caller_id, caller_role) = self.get_caller_id_and_role(); @@ -184,7 +184,10 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { self.action_group_status(group_id) .set(ActionStatus::Available); - require!(!action_groups_mapper.is_empty(), "group cannot be empty"); + require!( + action_groups_mapper.is_empty(), + "cannot add actions to an already existing batch" + ); for action in actions { require!( @@ -214,5 +217,7 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { let _ = action_groups_mapper.insert(action_id); self.group_for_action(action_id).set(group_id); } + self.last_action_group_id().set(group_id); + group_id } } diff --git a/contracts/multisig/src/multisig_state.rs b/contracts/multisig/src/multisig_state.rs index 32ce5815..c800af13 100644 --- a/contracts/multisig/src/multisig_state.rs +++ b/contracts/multisig/src/multisig_state.rs @@ -82,6 +82,10 @@ pub trait MultisigStateModule { #[storage_mapper("action_groups")] fn action_groups(&self, group_id: GroupId) -> UnorderedSetMapper; + #[view(getLastGroupActionId)] + #[storage_mapper("last_action_group_id")] + fn last_action_group_id(&self) -> SingleValueMapper; + #[view(getActionGroup)] #[storage_mapper("action_group_status")] fn action_group_status(&self, group_id: GroupId) -> SingleValueMapper; diff --git a/contracts/multisig/wasm-multisig-full/src/lib.rs b/contracts/multisig/wasm-multisig-full/src/lib.rs index 4177dc89..869d1bf3 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: 39 +// Endpoints: 40 // Async Callback: 1 -// Total number of exported functions: 41 +// Total number of exported functions: 42 #![no_std] #![allow(internal_features)] @@ -26,6 +26,7 @@ multiversx_sc_wasm_adapter::endpoints! { discardBatch => discard_batch getQuorum => quorum getNumBoardMembers => num_board_members + getNumGroups => num_groups getNumProposers => num_proposers getActionGroup => action_groups getActionLastIndex => get_action_last_index diff --git a/contracts/multisig/wasm/src/lib.rs b/contracts/multisig/wasm/src/lib.rs index c5573a47..dc15d679 100644 --- a/contracts/multisig/wasm/src/lib.rs +++ b/contracts/multisig/wasm/src/lib.rs @@ -5,9 +5,9 @@ //////////////////////////////////////////////////// // Init: 1 -// Endpoints: 30 +// Endpoints: 31 // Async Callback: 1 -// Total number of exported functions: 32 +// Total number of exported functions: 33 #![no_std] #![allow(internal_features)] @@ -26,6 +26,7 @@ multiversx_sc_wasm_adapter::endpoints! { discardBatch => discard_batch getQuorum => quorum getNumBoardMembers => num_board_members + getNumGroups => num_groups getNumProposers => num_proposers getActionGroup => action_groups getActionLastIndex => get_action_last_index From bce3f75f60e463bcbb236f6bbcbccafcc9c16f86 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Tue, 5 Mar 2024 14:57:04 +0200 Subject: [PATCH 06/44] cleanup part 1 --- contracts/multisig/src/multisig_perform.rs | 26 +++++----- contracts/multisig/src/multisig_propose.rs | 52 +++++++++++-------- contracts/multisig/src/multisig_sign.rs | 4 +- .../multisig/wasm-multisig-full/src/lib.rs | 5 +- contracts/multisig/wasm/src/lib.rs | 5 +- 5 files changed, 52 insertions(+), 40 deletions(-) diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index 4f032fd7..2f1bb744 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -19,11 +19,12 @@ fn usize_add_isize(value: &mut usize, delta: isize) { pub trait MultisigPerformModule: crate::multisig_state::MultisigStateModule + crate::multisig_events::MultisigEventsModule { - fn gas_for_transfer_exec(&self) -> GasLimit { + fn ensure_and_get_gas_for_transfer_exec(&self) -> GasLimit { let gas_left = self.blockchain().get_gas_left(); - if gas_left <= PERFORM_ACTION_FINISH_GAS { - sc_panic!("insufficient gas for call"); - } + require!( + gas_left > PERFORM_ACTION_FINISH_GAS, + "insufficient gas for call" + ); gas_left - PERFORM_ACTION_FINISH_GAS } @@ -191,25 +192,25 @@ pub trait MultisigPerformModule: match action { Action::Nothing => OptionalValue::None, Action::AddBoardMember(board_member_address) => { - self.change_user_role(action_id, board_member_address, UserRole::BoardMember); require!( self.num_board_members().get() <= MAX_BOARD_MEMBERS, "board size cannot exceed limit" ); + self.change_user_role(action_id, board_member_address, UserRole::BoardMember); + OptionalValue::None } Action::AddProposer(proposer_address) => { - self.change_user_role(action_id, proposer_address, UserRole::Proposer); - // validation required for the scenario when a board member becomes a proposer require!( self.quorum().get() <= self.num_board_members().get(), "quorum cannot exceed board size" ); + self.change_user_role(action_id, proposer_address, UserRole::Proposer); + OptionalValue::None } Action::RemoveUser(user_address) => { - self.change_user_role(action_id, user_address, UserRole::None); let num_board_members = self.num_board_members().get(); let num_proposers = self.num_proposers().get(); require!( @@ -220,6 +221,7 @@ pub trait MultisigPerformModule: self.quorum().get() <= num_board_members, "quorum cannot exceed board size" ); + self.change_user_role(action_id, user_address, UserRole::None); OptionalValue::None } Action::ChangeQuorum(new_quorum) => { @@ -235,7 +237,7 @@ pub trait MultisigPerformModule: require!(call_data.egld_amount != 0, "EGLD amount cannot be zero"); let gas = call_data .opt_gas_limit - .unwrap_or_else(|| self.gas_for_transfer_exec()); + .unwrap_or_else(|| self.ensure_and_get_gas_for_transfer_exec()); self.perform_transfer_execute_egld_event( action_id, &call_data.to, @@ -261,7 +263,7 @@ pub trait MultisigPerformModule: let gas = call_data .opt_gas_limit .unwrap_or_else(|| self.blockchain().get_gas_left()); - require!(gas > PERFORM_ACTION_FINISH_GAS, "insufficient gas for call"); + let _ = self.ensure_and_get_gas_for_transfer_exec(); require!( call_data.tokens.is_empty(), "number of tokens cannot be zero" @@ -291,9 +293,7 @@ pub trait MultisigPerformModule: let gas = call_data .opt_gas_limit .unwrap_or_else(|| self.blockchain().get_gas_left()); - if gas <= PERFORM_ACTION_FINISH_GAS { - sc_panic!("insufficient gas for call"); - } + let _ = self.ensure_and_get_gas_for_transfer_exec(); self.perform_async_call_event( action_id, &call_data.to, diff --git a/contracts/multisig/src/multisig_propose.rs b/contracts/multisig/src/multisig_propose.rs index 6c765c42..c85e9267 100644 --- a/contracts/multisig/src/multisig_propose.rs +++ b/contracts/multisig/src/multisig_propose.rs @@ -176,9 +176,6 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { "only board members and proposers can propose" ); - let own_sc_address = self.blockchain().get_sc_address(); - let own_shard = self.blockchain().get_shard_of_address(&own_sc_address); - let mut action_mapper = self.action_mapper(); let mut action_groups_mapper = self.action_groups(group_id); self.action_group_status(group_id) @@ -190,24 +187,8 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { ); for action in actions { - require!( - !action.is_nothing() && !action.is_async_call() && !action.is_sc_upgrade(), - "Invalid action" - ); - - if let Action::SendTransferExecuteEgld(call_data) = &action { - let other_sc_shard = self.blockchain().get_shard_of_address(&call_data.to); - require!( - own_shard == other_sc_shard, - "All transfer exec must be to the same shard" - ); - } else if let Action::SendTransferExecuteEsdt(call_data) = &action { - let other_sc_shard = self.blockchain().get_shard_of_address(&call_data.to); - require!( - own_shard == other_sc_shard, - "All transfer exec must be to the same shard" - ); - } + self.require_valid_action_type(&action); + self.ensure_valid_transfer_action(&action); let action_id = action_mapper.push(&action); if caller_role.can_sign() { @@ -220,4 +201,33 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { self.last_action_group_id().set(group_id); group_id } + + fn require_valid_action_type(&self, action: &Action) { + require!( + !action.is_nothing() && !action.is_async_call() && !action.is_sc_upgrade(), + "Invalid action" + ); + } + + fn ensure_valid_transfer_action(&self, action: &Action) { + let own_sc_address = self.blockchain().get_sc_address(); + let own_shard = self.blockchain().get_shard_of_address(&own_sc_address); + + if let Action::SendTransferExecuteEgld(call_data) = &action { + let other_sc_shard = self.blockchain().get_shard_of_address(&call_data.to); + require!( + own_shard == other_sc_shard, + "All transfer exec must be to the same shard" + ); + return; + } + + if let Action::SendTransferExecuteEsdt(call_data) = &action { + let other_sc_shard = self.blockchain().get_shard_of_address(&call_data.to); + require!( + own_shard == other_sc_shard, + "All transfer exec must be to the same shard" + ); + } + } } diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index 0e44f692..7173d2a6 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -80,7 +80,7 @@ pub trait MultisigSignModule: let group_status = self.action_group_status(group_id).get(); require!( group_status == ActionStatus::Available, - "cannot sign actions of an aborted batch" + "cannot unsign actions of an aborted batch" ); self.unsign_action(action_id, caller_id); @@ -95,7 +95,7 @@ pub trait MultisigSignModule: let group_status = self.action_group_status(group_id).get(); require!( group_status == ActionStatus::Available, - "cannot sign actions of an aborted batch" + "cannot unsign actions of an aborted batch" ); let mapper = self.action_groups(group_id); require!(!mapper.is_empty(), "Invalid group ID"); diff --git a/contracts/multisig/wasm-multisig-full/src/lib.rs b/contracts/multisig/wasm-multisig-full/src/lib.rs index 869d1bf3..8100614c 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: 40 +// Endpoints: 41 // Async Callback: 1 -// Total number of exported functions: 42 +// Total number of exported functions: 43 #![no_std] #![allow(internal_features)] @@ -29,6 +29,7 @@ multiversx_sc_wasm_adapter::endpoints! { getNumGroups => num_groups getNumProposers => num_proposers getActionGroup => action_groups + getLastGroupActionId => last_action_group_id getActionLastIndex => get_action_last_index proposeAddBoardMember => propose_add_board_member proposeAddProposer => propose_add_proposer diff --git a/contracts/multisig/wasm/src/lib.rs b/contracts/multisig/wasm/src/lib.rs index dc15d679..fc705050 100644 --- a/contracts/multisig/wasm/src/lib.rs +++ b/contracts/multisig/wasm/src/lib.rs @@ -5,9 +5,9 @@ //////////////////////////////////////////////////// // Init: 1 -// Endpoints: 31 +// Endpoints: 32 // Async Callback: 1 -// Total number of exported functions: 33 +// Total number of exported functions: 34 #![no_std] #![allow(internal_features)] @@ -29,6 +29,7 @@ multiversx_sc_wasm_adapter::endpoints! { getNumGroups => num_groups getNumProposers => num_proposers getActionGroup => action_groups + getLastGroupActionId => last_action_group_id getActionLastIndex => get_action_last_index proposeAddBoardMember => propose_add_board_member proposeAddProposer => propose_add_proposer From 68292424c97a52724c862d6e6e94d85bd752cbe1 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Tue, 5 Mar 2024 14:57:24 +0200 Subject: [PATCH 07/44] fix change quorum test --- .../multisig/tests/multisig_blackbox_test.rs | 47 ++++++++++++------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index d4a8373b..1334b3aa 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -64,7 +64,10 @@ impl MultisigTestState { PROPOSER_ADDRESS_EXPR, Account::new().nonce(1).balance(PROPOSER_BALANCE_EXPR), ) - .put_account(BOARD_MEMBER_ADDRESS_EXPR, Account::new().nonce(1)) + .put_account( + BOARD_MEMBER_ADDRESS_EXPR, + Account::new().nonce(1).balance(PROPOSER_BALANCE_EXPR), + ) .put_account(ADDER_OWNER_ADDRESS_EXPR, Account::new().nonce(1)) .new_address(ADDER_OWNER_ADDRESS_EXPR, 1, ADDER_ADDRESS_EXPR), ); @@ -380,20 +383,6 @@ fn test_change_quorum() { .call(state.multisig_contract.unsign(action_id)), ); - state.world.sc_call( - ScCallStep::new() - .from(BOARD_MEMBER_ADDRESS_EXPR) - .call(state.multisig_contract.discard_action_endpoint(action_id)), - ); - - // try sign discarded action - state.world.sc_call( - ScCallStep::new() - .from(BOARD_MEMBER_ADDRESS_EXPR) - .call(state.multisig_contract.sign(action_id)) - .expect(TxExpect::user_error("str:action does not exist")), - ); - // add another board member const NEW_BOARD_MEMBER_ADDRESS_EXPR: &str = "address:new-board-member"; let new_board_member_address = AddressValue::from(NEW_BOARD_MEMBER_ADDRESS_EXPR).to_address(); @@ -410,6 +399,30 @@ fn test_change_quorum() { let action_id = state.propose_change_quorum(new_quorum); state.sign(action_id); state.perform(action_id); + + state.world.sc_call( + ScCallStep::new() + .from(BOARD_MEMBER_ADDRESS_EXPR) + .call(state.multisig_contract.discard_action_endpoint(action_id)), + ); + + // try sign discarded action + state.world.sc_call( + ScCallStep::new() + .from(BOARD_MEMBER_ADDRESS_EXPR) + .call(state.multisig_contract.unsign(action_id)) + .expect(TxExpect::user_error( + "str:cannot unsign actions of an aborted batch", + )), + ); + + // try sign discarded action + state.world.sc_call( + ScCallStep::new() + .from(BOARD_MEMBER_ADDRESS_EXPR) + .call(state.multisig_contract.sign(action_id)) + .expect(TxExpect::user_error("str:action does not exist")), + ); } #[test] @@ -481,7 +494,7 @@ fn test_transfer_execute_sc_all() { let action_id = state.propose_transfer_execute( state.adder_address.clone(), - 0u64, + 5u64, Option::::None, adder_call, ); @@ -550,7 +563,7 @@ fn test_deploy_and_upgrade_from_source() { let action_id = state.propose_transfer_execute( new_adder_address, - 0u64, + 5u64, Option::::None, adder_call, ); From 8f8b6d8533097d8b8e7fe148a32f65093d7bc331 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Tue, 5 Mar 2024 15:09:20 +0200 Subject: [PATCH 08/44] try remove all board members bugfix --- contracts/multisig/src/multisig_perform.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index 2f1bb744..d4025770 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -193,7 +193,7 @@ pub trait MultisigPerformModule: Action::Nothing => OptionalValue::None, Action::AddBoardMember(board_member_address) => { require!( - self.num_board_members().get() <= MAX_BOARD_MEMBERS, + self.num_board_members().get() < MAX_BOARD_MEMBERS, "board size cannot exceed limit" ); self.change_user_role(action_id, board_member_address, UserRole::BoardMember); @@ -203,7 +203,7 @@ pub trait MultisigPerformModule: Action::AddProposer(proposer_address) => { // validation required for the scenario when a board member becomes a proposer require!( - self.quorum().get() <= self.num_board_members().get(), + self.quorum().get() < self.num_board_members().get(), "quorum cannot exceed board size" ); self.change_user_role(action_id, proposer_address, UserRole::Proposer); @@ -218,7 +218,7 @@ pub trait MultisigPerformModule: "cannot remove all board members and proposers" ); require!( - self.quorum().get() <= num_board_members, + self.quorum().get() < num_board_members, "quorum cannot exceed board size" ); self.change_user_role(action_id, user_address, UserRole::None); From 6d0da455936e183bdd2766ec7890b4f302f5dc64 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Tue, 5 Mar 2024 16:13:45 +0200 Subject: [PATCH 09/44] sign_batch_and_perform optimized fix --- contracts/multisig/src/multisig_sign.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index 7173d2a6..e0fc7f4c 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -64,8 +64,16 @@ pub trait MultisigSignModule: fn sign_batch_and_perform(&self, group_id: GroupId) { self.sign_batch(group_id); + 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() { - let _ = self.perform_action_endpoint(action_id); + if self.quorum_reached(action_id) { + let _ = self.perform_action(action_id); + } } } From 3b8e4ecdddfb1c16c55f215008629d137bcc114a Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Wed, 6 Mar 2024 08:12:02 +0200 Subject: [PATCH 10/44] getPendingActionFullInfo with pagination --- contracts/multisig/src/multisig.rs | 49 +++++++++--------------------- 1 file changed, 14 insertions(+), 35 deletions(-) diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index 0d3f04e5..c543116f 100644 --- a/contracts/multisig/src/multisig.rs +++ b/contracts/multisig/src/multisig.rs @@ -61,49 +61,29 @@ pub trait Multisig: #[view(getPendingActionFullInfo)] fn get_pending_action_full_info( &self, - offset: OptionalValue, + count: OptionalValue, + offset_id: OptionalValue, ) -> MultiValueEncoded> { let mut result = MultiValueEncoded::new(); let action_last_index = self.get_action_last_index(); let action_mapper = self.action_mapper(); let mut index_of_first_action = 1; - if let OptionalValue::Some(unwrapped_offset) = offset { + let mut index_of_last_action = action_last_index; + if let OptionalValue::Some(unwrapped_offset_id) = offset_id { require!( - unwrapped_offset <= action_last_index, - "offset needs to be smaller than the total number of actions" + unwrapped_offset_id <= action_last_index, + "offset_id needs to be within the range of the available action ids" ); - index_of_first_action += action_last_index - unwrapped_offset; + index_of_first_action = unwrapped_offset_id; } - for action_id in index_of_first_action..=action_last_index { - let action_data = action_mapper.get(action_id); - if action_data.is_pending() { - result.push(ActionFullInfo { - action_id, - action_data, - signers: self.get_action_signers(action_id), - group_id: self.group_for_action(action_id).get(), - }); - } + if let OptionalValue::Some(unwrapped_count) = count { + require!( + unwrapped_count <= action_last_index, + "cannot exceed the total number of actions" + ); + index_of_last_action = index_of_first_action + unwrapped_count; } - - result - } - - #[label("multisig-external-view")] - #[view(getPendingActionFullInfoInRange)] - fn get_pending_action_full_info_in_range( - &self, - action_start_id: usize, - action_stop_id: usize, - ) -> MultiValueEncoded> { - let mut result = MultiValueEncoded::new(); - let action_last_index = self.get_action_last_index(); - let action_mapper = self.action_mapper(); - require!( - action_start_id < action_stop_id && action_stop_id <= action_last_index, - "action ids provided need to be in the range of the available actions" - ); - for action_id in action_start_id..=action_stop_id { + for action_id in index_of_first_action..=index_of_last_action { let action_data = action_mapper.get(action_id); if action_data.is_pending() { result.push(ActionFullInfo { @@ -114,7 +94,6 @@ pub trait Multisig: }); } } - result } From 14ac6f07827e74fad2707ed970adefb3267ab612 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Wed, 6 Mar 2024 08:24:47 +0200 Subject: [PATCH 11/44] make sure single actions don't get checked for group status --- contracts/multisig/src/multisig_perform.rs | 12 ++++++---- contracts/multisig/src/multisig_sign.rs | 28 +++++++++++----------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index d4025770..33b1121f 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -172,11 +172,13 @@ pub trait MultisigPerformModule: let action = self.action_mapper().get(action_id); let group_id = self.group_for_action(action_id).get(); - let group_status = self.action_group_status(group_id).get(); - require!( - group_status == ActionStatus::Available, - "cannot perform actions of an aborted batch" - ); + if group_id != 0 { + let group_status = self.action_group_status(group_id).get(); + require!( + group_status == ActionStatus::Available, + "cannot perform actions of an aborted batch" + ); + } self.start_perform_action_event(&ActionFullInfo { action_id, action_data: action.clone(), diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index e0fc7f4c..eb95e1c3 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -17,13 +17,13 @@ pub trait MultisigSignModule: "action does not exist" ); let group_id = self.group_for_action(action_id).get(); - - let group_status = self.action_group_status(group_id).get(); - require!( - group_status == ActionStatus::Available, - "cannot sign actions of an aborted batch" - ); - + if group_id != 0 { + let group_status = self.action_group_status(group_id).get(); + require!( + group_status == ActionStatus::Available, + "cannot sign actions of an aborted batch" + ); + } let (caller_id, caller_role) = self.get_caller_id_and_role(); require!(caller_role.can_sign(), "only board members can sign"); @@ -84,13 +84,13 @@ 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 group_id = self.group_for_action(action_id).get(); - - let group_status = self.action_group_status(group_id).get(); - require!( - group_status == ActionStatus::Available, - "cannot unsign actions of an aborted batch" - ); - + if group_id != 0 { + let group_status = self.action_group_status(group_id).get(); + require!( + group_status == ActionStatus::Available, + "cannot unsign actions of an aborted batch" + ); + } self.unsign_action(action_id, caller_id); } From 9ec41d2fb61cc2ce2c13ef91900535b662276a9c Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Wed, 6 Mar 2024 08:41:45 +0200 Subject: [PATCH 12/44] allow_multiple_var_args for get_pending_action_full_info --- contracts/multisig/src/multisig.rs | 1 + contracts/multisig/wasm-multisig-full/src/lib.rs | 5 ++--- contracts/multisig/wasm-multisig-view/src/lib.rs | 5 ++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index c543116f..53593121 100644 --- a/contracts/multisig/src/multisig.rs +++ b/contracts/multisig/src/multisig.rs @@ -58,6 +58,7 @@ pub trait Multisig: /// - the serialized action data /// - (number of signers followed by) list of signer addresses. #[label("multisig-external-view")] + #[allow_multiple_var_args] #[view(getPendingActionFullInfo)] fn get_pending_action_full_info( &self, diff --git a/contracts/multisig/wasm-multisig-full/src/lib.rs b/contracts/multisig/wasm-multisig-full/src/lib.rs index 8100614c..25f4e774 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: 41 +// Endpoints: 40 // Async Callback: 1 -// Total number of exported functions: 43 +// Total number of exported functions: 42 #![no_std] #![allow(internal_features)] @@ -53,7 +53,6 @@ multiversx_sc_wasm_adapter::endpoints! { performBatch => perform_batch dnsRegister => dns_register getPendingActionFullInfo => get_pending_action_full_info - getPendingActionFullInfoInRange => get_pending_action_full_info_in_range userRole => user_role getAllBoardMembers => get_all_board_members getAllProposers => get_all_proposers diff --git a/contracts/multisig/wasm-multisig-view/src/lib.rs b/contracts/multisig/wasm-multisig-view/src/lib.rs index bc0f1a26..c01d319c 100644 --- a/contracts/multisig/wasm-multisig-view/src/lib.rs +++ b/contracts/multisig/wasm-multisig-view/src/lib.rs @@ -5,9 +5,9 @@ //////////////////////////////////////////////////// // Init: 1 -// Endpoints: 9 +// Endpoints: 8 // Async Callback (empty): 1 -// Total number of exported functions: 11 +// Total number of exported functions: 10 #![no_std] #![allow(internal_features)] @@ -22,7 +22,6 @@ multiversx_sc_wasm_adapter::external_view_endpoints! { multisig ( getPendingActionFullInfo => get_pending_action_full_info - getPendingActionFullInfoInRange => get_pending_action_full_info_in_range userRole => user_role getAllBoardMembers => get_all_board_members getAllProposers => get_all_proposers From 46ee929db2e0abb8b84adbc2dbec7511898c5bfc Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Wed, 6 Mar 2024 09:08:39 +0200 Subject: [PATCH 13/44] fix checks before change user role --- contracts/multisig/src/multisig_perform.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index 33b1121f..f42591a9 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -203,16 +203,17 @@ pub trait MultisigPerformModule: OptionalValue::None } Action::AddProposer(proposer_address) => { + self.change_user_role(action_id, proposer_address, UserRole::Proposer); + // validation required for the scenario when a board member becomes a proposer require!( - self.quorum().get() < self.num_board_members().get(), + self.quorum().get() <= self.num_board_members().get(), "quorum cannot exceed board size" ); - self.change_user_role(action_id, proposer_address, UserRole::Proposer); - OptionalValue::None } Action::RemoveUser(user_address) => { + self.change_user_role(action_id, user_address, UserRole::None); let num_board_members = self.num_board_members().get(); let num_proposers = self.num_proposers().get(); require!( @@ -220,10 +221,9 @@ pub trait MultisigPerformModule: "cannot remove all board members and proposers" ); require!( - self.quorum().get() < num_board_members, + self.quorum().get() <= num_board_members, "quorum cannot exceed board size" ); - self.change_user_role(action_id, user_address, UserRole::None); OptionalValue::None } Action::ChangeQuorum(new_quorum) => { From 4211a6bcd62c95c0b914fb4246c3582518ffdd1d Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Wed, 6 Mar 2024 09:13:58 +0200 Subject: [PATCH 14/44] fix change quorum test --- contracts/multisig/tests/multisig_blackbox_test.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index 1334b3aa..171794b1 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -411,9 +411,7 @@ fn test_change_quorum() { ScCallStep::new() .from(BOARD_MEMBER_ADDRESS_EXPR) .call(state.multisig_contract.unsign(action_id)) - .expect(TxExpect::user_error( - "str:cannot unsign actions of an aborted batch", - )), + .expect(TxExpect::user_error("str:action does not exist")), ); // try sign discarded action From 8ea54b0148d0052c78b7648d7258840ff2b17321 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Thu, 7 Mar 2024 13:00:57 +0200 Subject: [PATCH 15/44] fix broken blackbox tests --- .../multisig/tests/multisig_blackbox_test.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index 171794b1..05290762 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -490,9 +490,16 @@ fn test_transfer_execute_sc_all() { let adder_call = state.adder_contract.add(5u64); + state.world.sc_call( + ScCallStep::new() + .from(PROPOSER_ADDRESS_EXPR) + .egld_value("100") + .call(state.multisig_contract.deposit()), + ); + let action_id = state.propose_transfer_execute( state.adder_address.clone(), - 5u64, + 100u64, Option::::None, adder_call, ); @@ -559,9 +566,16 @@ fn test_deploy_and_upgrade_from_source() { let adder_call = state.adder_contract.add(5u64); + state.world.sc_call( + ScCallStep::new() + .from(PROPOSER_ADDRESS_EXPR) + .egld_value("100") + .call(state.multisig_contract.deposit()), + ); + let action_id = state.propose_transfer_execute( new_adder_address, - 5u64, + 100u64, Option::::None, adder_call, ); From 3169b995e0e2b8d6f2e9e2fa25bba8c2702a2344 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Thu, 7 Mar 2024 14:43:31 +0200 Subject: [PATCH 16/44] test_perform_action_signed_by_removed_board_user --- .../multisig/tests/multisig_blackbox_test.rs | 143 +++++++++++++++++- 1 file changed, 137 insertions(+), 6 deletions(-) diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index 05290762..6c8c0547 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -9,8 +9,11 @@ use multiversx_sc::{ test_util::top_encode_to_vec_u8_or_panic, }, storage::mappers::SingleValue, - types::{Address, CodeMetadata, ContractCallNoPayment, FunctionCall}, + types::{ + Address, CodeMetadata, ContractCallNoPayment, EsdtTokenPayment, FunctionCall, ManagedVec, + }, }; +use multiversx_sc_modules::transfer_role_proxy::PaymentsVec; use multiversx_sc_scenario::{ api::StaticApi, scenario_model::{ @@ -30,6 +33,7 @@ const MULTISIG_PATH_EXPR: &str = "file:output/multisig.wasm"; const OWNER_ADDRESS_EXPR: &str = "address:owner"; const PROPOSER_ADDRESS_EXPR: &str = "address:proposer"; const PROPOSER_BALANCE_EXPR: &str = "100,000,000"; +const NFT_TOKEN_ID: &str = "str:NFT-123456"; const QUORUM_SIZE: usize = 1; type MultisigContract = ContractInfo>; @@ -62,12 +66,12 @@ impl MultisigTestState { .new_address(OWNER_ADDRESS_EXPR, 1, MULTISIG_ADDRESS_EXPR) .put_account( PROPOSER_ADDRESS_EXPR, - Account::new().nonce(1).balance(PROPOSER_BALANCE_EXPR), - ) - .put_account( - BOARD_MEMBER_ADDRESS_EXPR, - Account::new().nonce(1).balance(PROPOSER_BALANCE_EXPR), + Account::new() + .nonce(1) + .balance(PROPOSER_BALANCE_EXPR) + .esdt_nft_balance(NFT_TOKEN_ID, 1u64, "1", Option::<&[u8]>::None), ) + .put_account(BOARD_MEMBER_ADDRESS_EXPR, Account::new().nonce(1)) .put_account(ADDER_OWNER_ADDRESS_EXPR, Account::new().nonce(1)) .new_address(ADDER_OWNER_ADDRESS_EXPR, 1, ADDER_ADDRESS_EXPR), ); @@ -151,6 +155,23 @@ impl MultisigTestState { .call(self.multisig_contract.propose_change_quorum(new_quorum)), ) } + fn propose_transfer_execute_esdt( + &mut self, + to: Address, + tokens: PaymentsVec, + opt_gas_limit: Option, + contract_call: ContractCallNoPayment, + ) -> usize { + self.world + .sc_call_get_result(ScCallStep::new().from(PROPOSER_ADDRESS_EXPR).call( + self.multisig_contract.propose_transfer_execute_esdt( + to, + tokens, + opt_gas_limit, + contract_call.into_function_call(), + ), + )) + } fn propose_transfer_execute( &mut self, @@ -345,6 +366,54 @@ fn test_remove_proposer() { ); } +#[test] +fn test_perform_action_signed_by_removed_board_user() { + let mut state = MultisigTestState::new(); + state.deploy_multisig_contract(); + + const NEW_BOARD_MEMBER_ADDRESS_EXPR: &str = "address:new-board-member"; + let new_board_member_address = AddressValue::from(NEW_BOARD_MEMBER_ADDRESS_EXPR).to_address(); + + state.world.set_state_step( + SetStateStep::new().put_account(NEW_BOARD_MEMBER_ADDRESS_EXPR, Account::new().nonce(1)), + ); + + let action_id = state.propose_add_board_member(new_board_member_address.clone()); + state.sign(action_id); + state.perform(action_id); + + let adder_call = state.adder_contract.add(5u64); + + state.world.sc_call( + ScCallStep::new() + .from(PROPOSER_ADDRESS_EXPR) + .egld_value("100") + .call(state.multisig_contract.deposit()), + ); + + let special_action_id = state.propose_transfer_execute( + state.adder_address.clone(), + 100u64, + Option::::None, + adder_call, + ); + + state.world.sc_call( + ScCallStep::new() + .from(NEW_BOARD_MEMBER_ADDRESS_EXPR) + .call(state.multisig_contract.sign(special_action_id)), + ); + + let action_id = state.propose_remove_user(new_board_member_address.clone()); + state.sign(action_id); + state.perform(action_id); + + state.perform_and_expect_err(special_action_id, "quorum has not been reached"); + + state.sign(special_action_id); + state.perform(special_action_id); +} + #[test] fn test_try_remove_all_board_members() { let mut state = MultisigTestState::new(); @@ -488,6 +557,41 @@ fn test_transfer_execute_sc_all() { let mut state = MultisigTestState::new(); state.deploy_multisig_contract().deploy_adder_contract(); + // try make a transfer propose with 0 EGLD + + let adder_call = state.adder_contract.add(5u64); + + let action_id = state.propose_transfer_execute( + state.adder_address.clone(), + 0u64, + Option::::None, + adder_call, + ); + state.sign(action_id); + state.perform_and_expect_err(action_id, "EGLD amount cannot be zero"); + + // try make a transfer propose with 0 EGLD + + let adder_call = state.adder_contract.add(5u64); + + let mut esdt_payment: PaymentsVec = ManagedVec::new(); + esdt_payment.push(EsdtTokenPayment { + token_identifier: NFT_TOKEN_ID.into(), + token_nonce: 1u64.into(), + amount: 1u64.into(), + }); + + let action_id = state.propose_transfer_execute_esdt( + state.adder_address.clone(), + esdt_payment, + Option::::None, + adder_call, + ); + state.sign(action_id); + state.perform_and_expect_err(action_id, "number of tokens cannot be zero"); + + // perform an EGLD transfer purpose after a deposit to the contract + let adder_call = state.adder_contract.add(5u64); state.world.sc_call( @@ -511,6 +615,33 @@ fn test_transfer_execute_sc_all() { .call(state.adder_contract.sum()) .expect_value(SingleValue::from(BigUint::from(10u64))), ); + + // perform an ESDT transfer purpose after a deposit to the contract + + let adder_call = state.adder_contract.add(5u64); + + let mut esdt_payment: PaymentsVec = ManagedVec::new(); + esdt_payment.push(EsdtTokenPayment { + token_identifier: NFT_TOKEN_ID.into(), + token_nonce: 1u64.into(), + amount: 1u64.into(), + }); + + state.world.sc_call( + ScCallStep::new() + .from(PROPOSER_ADDRESS_EXPR) + .esdt_transfer(NFT_TOKEN_ID, 1u64, "1") + .call(state.multisig_contract.deposit()), + ); + + let action_id = state.propose_transfer_execute_esdt( + state.adder_address.clone(), + esdt_payment, + Option::::None, + adder_call, + ); + state.sign(action_id); + state.perform_and_expect_err(action_id, "number of tokens cannot be zero"); } #[test] From 7a1e34d7917b93d1db0fec861e7c57cce935098a Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Thu, 7 Mar 2024 15:26:15 +0200 Subject: [PATCH 17/44] whitebox fix + clippy --- .../multisig/tests/multisig_blackbox_test.rs | 4 ++-- .../multisig/tests/multisig_whitebox_test.rs | 20 +++++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index 6c8c0547..1336f3b5 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -577,7 +577,7 @@ fn test_transfer_execute_sc_all() { let mut esdt_payment: PaymentsVec = ManagedVec::new(); esdt_payment.push(EsdtTokenPayment { token_identifier: NFT_TOKEN_ID.into(), - token_nonce: 1u64.into(), + token_nonce: 1u64, amount: 1u64.into(), }); @@ -623,7 +623,7 @@ fn test_transfer_execute_sc_all() { let mut esdt_payment: PaymentsVec = ManagedVec::new(); esdt_payment.push(EsdtTokenPayment { token_identifier: NFT_TOKEN_ID.into(), - token_nonce: 1u64.into(), + token_nonce: 1u64, amount: 1u64.into(), }); diff --git a/contracts/multisig/tests/multisig_whitebox_test.rs b/contracts/multisig/tests/multisig_whitebox_test.rs index 68333061..3ec64850 100644 --- a/contracts/multisig/tests/multisig_whitebox_test.rs +++ b/contracts/multisig/tests/multisig_whitebox_test.rs @@ -616,11 +616,19 @@ fn test_transfer_execute_sc_all() { }, ); + world.whitebox_call( + &multisig_whitebox, + ScCallStep::new() + .from(PROPOSER_ADDRESS_EXPR) + .egld_value("100"), + |sc| sc.deposit(), + ); + let action_id = call_propose( &mut world, ActionRaw::SendTransferExecute(CallActionDataRaw { to: address_expr_to_address(ADDER_ADDRESS_EXPR), - egld_amount: 0u64.into(), + egld_amount: 100u64.into(), endpoint_name: BoxedBytes::from(&b"add"[..]), arguments: vec![BoxedBytes::from(&[5u8][..])], }), @@ -771,11 +779,19 @@ fn test_deploy_and_upgrade_from_source() { assert_eq!(address_expr_to_address(NEW_ADDER_ADDRESS_EXPR), addr); + world.whitebox_call( + &multisig_whitebox, + ScCallStep::new() + .from(PROPOSER_ADDRESS_EXPR) + .egld_value("100"), + |sc| sc.deposit(), + ); + let action_id = call_propose( &mut world, ActionRaw::SendTransferExecute(CallActionDataRaw { to: address_expr_to_address(NEW_ADDER_ADDRESS_EXPR), - egld_amount: 0u64.into(), + egld_amount: 100u64.into(), endpoint_name: BoxedBytes::from(&b"add"[..]), arguments: vec![BoxedBytes::from(&[5u8][..])], }), From 00c3526148ff7e47f4a0e205a2758be954022259 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Thu, 7 Mar 2024 15:56:58 +0200 Subject: [PATCH 18/44] unsign_for_outdated_board_members --- contracts/multisig/src/multisig_sign.rs | 13 +++++++ .../multisig/tests/multisig_blackbox_test.rs | 36 +++++++++++++++++++ .../multisig/wasm-multisig-full/src/lib.rs | 5 +-- contracts/multisig/wasm/src/lib.rs | 5 +-- 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index eb95e1c3..986a37ef 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -133,4 +133,17 @@ pub trait MultisigSignModule: self.action_signer_ids(action_id).contains(&user_id) } } + + #[endpoint(unsignForOutdatedBoardMembers)] + fn unsign_for_outdated_board_members(&self, action_id: ActionId) { + let mut outdated_board_members: ManagedVec = ManagedVec::new(); + for signer_id in self.action_signer_ids(action_id).iter() { + if !self.user_id_to_role(signer_id).get().can_sign() { + outdated_board_members.push(signer_id); + } + } + for member in outdated_board_members.iter() { + self.action_signer_ids(action_id).swap_remove(&member); + } + } } diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index 1336f3b5..ef259cdf 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -371,6 +371,8 @@ fn test_perform_action_signed_by_removed_board_user() { let mut state = MultisigTestState::new(); state.deploy_multisig_contract(); + // a new board member is added + const NEW_BOARD_MEMBER_ADDRESS_EXPR: &str = "address:new-board-member"; let new_board_member_address = AddressValue::from(NEW_BOARD_MEMBER_ADDRESS_EXPR).to_address(); @@ -398,19 +400,53 @@ fn test_perform_action_signed_by_removed_board_user() { adder_call, ); + // the new board member signs the special action + state.world.sc_call( ScCallStep::new() .from(NEW_BOARD_MEMBER_ADDRESS_EXPR) .call(state.multisig_contract.sign(special_action_id)), ); + state.world.sc_call( + ScCallStep::new() + .from(PROPOSER_ADDRESS_EXPR) + .call(state.multisig_contract.quorum_reached(special_action_id)) + .expect_value(true), + ); + + // just before performing the new board member gets removed + let action_id = state.propose_remove_user(new_board_member_address.clone()); state.sign(action_id); state.perform(action_id); + state.world.sc_call( + ScCallStep::new() + .from(PROPOSER_ADDRESS_EXPR) + .call(state.multisig_contract.quorum_reached(special_action_id)) + .expect_value(false), + ); + state.perform_and_expect_err(special_action_id, "quorum has not been reached"); + // removing the signature of the removed board member + + state.world.sc_call( + ScCallStep::new().from(PROPOSER_ADDRESS_EXPR).call( + state + .multisig_contract + .unsign_for_outdated_board_members(special_action_id), + ), + ); state.sign(special_action_id); + + state.world.sc_call( + ScCallStep::new() + .from(PROPOSER_ADDRESS_EXPR) + .call(state.multisig_contract.quorum_reached(special_action_id)) + .expect_value(true), + ); state.perform(special_action_id); } diff --git a/contracts/multisig/wasm-multisig-full/src/lib.rs b/contracts/multisig/wasm-multisig-full/src/lib.rs index 25f4e774..a9271830 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: 40 +// Endpoints: 41 // Async Callback: 1 -// Total number of exported functions: 42 +// Total number of exported functions: 43 #![no_std] #![allow(internal_features)] @@ -48,6 +48,7 @@ multiversx_sc_wasm_adapter::endpoints! { unsign => unsign unsignBatch => unsign_batch signed => signed + unsignForOutdatedBoardMembers => unsign_for_outdated_board_members quorumReached => quorum_reached performAction => perform_action_endpoint performBatch => perform_batch diff --git a/contracts/multisig/wasm/src/lib.rs b/contracts/multisig/wasm/src/lib.rs index fc705050..b4bf0020 100644 --- a/contracts/multisig/wasm/src/lib.rs +++ b/contracts/multisig/wasm/src/lib.rs @@ -5,9 +5,9 @@ //////////////////////////////////////////////////// // Init: 1 -// Endpoints: 32 +// Endpoints: 33 // Async Callback: 1 -// Total number of exported functions: 34 +// Total number of exported functions: 35 #![no_std] #![allow(internal_features)] @@ -48,6 +48,7 @@ multiversx_sc_wasm_adapter::endpoints! { unsign => unsign unsignBatch => unsign_batch signed => signed + unsignForOutdatedBoardMembers => unsign_for_outdated_board_members quorumReached => quorum_reached performAction => perform_action_endpoint performBatch => perform_batch From b5d0922b3bcec78b8c30884cb6eb8002c443dc62 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 08:31:08 +0200 Subject: [PATCH 19/44] fix test_perform_action_signed_by_removed_board_user --- contracts/multisig/tests/multisig_blackbox_test.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index ef259cdf..4d8b1341 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -370,6 +370,7 @@ fn test_remove_proposer() { fn test_perform_action_signed_by_removed_board_user() { let mut state = MultisigTestState::new(); state.deploy_multisig_contract(); + state.deploy_adder_contract(); // a new board member is added From 688db3c6a5dabe5ef7f2294e14a3f9e39c0d5fb0 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 09:16:33 +0200 Subject: [PATCH 20/44] fix mandos tests part 1 --- .../multisig/scenarios/changeBoard.scen.json | 20 -------- .../scenarios/deployAdder_then_call.scen.json | 49 ++++++++++++------ .../scenarios/deployFactorial.scen.json | 50 +++++++++++-------- .../scenarios/deployOtherMultisig.scen.json | 6 +-- .../scenarios/steps/deploy.steps.json | 14 +++--- .../scenarios/steps/init_accounts.steps.json | 2 +- .../multisig/scenarios/upgrade.scen.json | 6 +-- .../scenarios/upgrade_from_source.scen.json | 10 ++-- 8 files changed, 82 insertions(+), 75 deletions(-) diff --git a/contracts/multisig/scenarios/changeBoard.scen.json b/contracts/multisig/scenarios/changeBoard.scen.json index 8746a247..85a10eef 100644 --- a/contracts/multisig/scenarios/changeBoard.scen.json +++ b/contracts/multisig/scenarios/changeBoard.scen.json @@ -7,26 +7,6 @@ { "step": "externalSteps", "path": "steps/deploy.steps.json" - }, - { - "step": "externalSteps", - "path": "steps/add_prop.steps.json" - }, - { - "step": "externalSteps", - "path": "steps/add_bm.steps.json" - }, - { - "step": "externalSteps", - "path": "steps/rem_alice.steps.json" - }, - { - "step": "externalSteps", - "path": "steps/rem_unknown.steps.json" - }, - { - "step": "externalSteps", - "path": "steps/sign_unsign_bad_action.steps.json" } ] } diff --git a/contracts/multisig/scenarios/deployAdder_then_call.scen.json b/contracts/multisig/scenarios/deployAdder_then_call.scen.json index f9d2fb2d..6bd68ce6 100644 --- a/contracts/multisig/scenarios/deployAdder_then_call.scen.json +++ b/contracts/multisig/scenarios/deployAdder_then_call.scen.json @@ -49,17 +49,20 @@ "nonce": "0", "balance": "0", "storage": { - "str:user_count": "5", - "str:user_address_to_id|address:alice": "1", - "str:user_address_to_id|address:bob": "2", - "str:user_address_to_id|address:charlie": "3", - "str:user_address_to_id|address:paul": "4", - "str:user_address_to_id|address:dan": "5", - "str:user_id_to_address|u32:1": "address:alice", - "str:user_id_to_address|u32:2": "address:bob", - "str:user_id_to_address|u32:3": "address:charlie", - "str:user_id_to_address|u32:4": "address:paul", - "str:user_id_to_address|u32:5": "address:dan", + "str:sc_user_count": "5", + "str:sc_user_address_to_id|address:alice": "1", + "str:sc_user_address_to_id|address:bob": "2", + "str:sc_user_address_to_id|address:charlie": "3", + "str:sc_user_address_to_id|address:paul": "4", + "str:sc_user_address_to_id|address:dan": "5", + "str:sc_user_id_to_address|u32:1": "address:alice", + "str:sc_user_id_to_address|u32:2": "address:bob", + "str:sc_user_id_to_address|u32:3": "address:charlie", + "str:sc_user_id_to_address|u32:4": "address:paul", + "str:sc_user_id_to_address|u32:5": "address:dan", + "str:quorum_for_action|u32:1": "2", + "str:quorum_for_action|u32:2": "2", + "str:quorum_for_action|u32:3": "2", "str:user_role|u32:1": "2", "str:user_role|u32:2": "2", "str:user_role|u32:3": "2", @@ -183,7 +186,7 @@ "function": "proposeTransferExecute", "arguments": [ "sc:adder", - "0", + "100", "0x", "str:add", "1234" @@ -243,6 +246,24 @@ "refund": "*" } }, + { + "step": "scCall", + "tx": { + "from": "address:paul", + "to": "sc:multisig", + "egldValue": "100", + "function": "deposit", + "arguments": [], + "gasLimit": "70,000,000", + "gasPrice": "0" + }, + "expect": { + "out": [], + "status": "0", + "gas": "*", + "refund": "*" + } + }, { "step": "scCall", "id": "sc-deploy-perform-2", @@ -258,7 +279,7 @@ }, "expect": { "out": [], - "status": "", + "status": "0", "logs": "*", "gas": "*", "refund": "*" @@ -304,7 +325,7 @@ "accounts": { "sc:adder": { "nonce": "0", - "balance": "0", + "balance": "100", "storage": { "str:sum": "2468" }, diff --git a/contracts/multisig/scenarios/deployFactorial.scen.json b/contracts/multisig/scenarios/deployFactorial.scen.json index 4ff05b08..cef13491 100644 --- a/contracts/multisig/scenarios/deployFactorial.scen.json +++ b/contracts/multisig/scenarios/deployFactorial.scen.json @@ -48,17 +48,20 @@ "nonce": "0", "balance": "0", "storage": { - "str:user_count": "5", - "str:user_address_to_id|address:alice": "1", - "str:user_address_to_id|address:bob": "2", - "str:user_address_to_id|address:charlie": "3", - "str:user_address_to_id|address:paul": "4", - "str:user_address_to_id|address:dan": "5", - "str:user_id_to_address|u32:1": "address:alice", - "str:user_id_to_address|u32:2": "address:bob", - "str:user_id_to_address|u32:3": "address:charlie", - "str:user_id_to_address|u32:4": "address:paul", - "str:user_id_to_address|u32:5": "address:dan", + "str:sc_user_count": "5", + "str:sc_user_address_to_id|address:alice": "1", + "str:sc_user_address_to_id|address:bob": "2", + "str:sc_user_address_to_id|address:charlie": "3", + "str:sc_user_address_to_id|address:paul": "4", + "str:sc_user_address_to_id|address:dan": "5", + "str:sc_user_id_to_address|u32:1": "address:alice", + "str:sc_user_id_to_address|u32:2": "address:bob", + "str:sc_user_id_to_address|u32:3": "address:charlie", + "str:sc_user_id_to_address|u32:4": "address:paul", + "str:sc_user_id_to_address|u32:5": "address:dan", + "str:quorum_for_action|u32:1": "2", + "str:quorum_for_action|u32:2": "2", + "str:quorum_for_action|u32:3": "2", "str:user_role|u32:1": "2", "str:user_role|u32:2": "2", "str:user_role|u32:3": "2", @@ -163,17 +166,20 @@ "nonce": "1", "balance": "0", "storage": { - "str:user_count": "5", - "str:user_address_to_id|address:alice": "1", - "str:user_address_to_id|address:bob": "2", - "str:user_address_to_id|address:charlie": "3", - "str:user_address_to_id|address:paul": "4", - "str:user_address_to_id|address:dan": "5", - "str:user_id_to_address|u32:1": "address:alice", - "str:user_id_to_address|u32:2": "address:bob", - "str:user_id_to_address|u32:3": "address:charlie", - "str:user_id_to_address|u32:4": "address:paul", - "str:user_id_to_address|u32:5": "address:dan", + "str:sc_user_count": "5", + "str:sc_user_address_to_id|address:alice": "1", + "str:sc_user_address_to_id|address:bob": "2", + "str:sc_user_address_to_id|address:charlie": "3", + "str:sc_user_address_to_id|address:paul": "4", + "str:sc_user_address_to_id|address:dan": "5", + "str:sc_user_id_to_address|u32:1": "address:alice", + "str:sc_user_id_to_address|u32:2": "address:bob", + "str:sc_user_id_to_address|u32:3": "address:charlie", + "str:sc_user_id_to_address|u32:4": "address:paul", + "str:sc_user_id_to_address|u32:5": "address:dan", + "str:quorum_for_action|u32:1": "2", + "str:quorum_for_action|u32:2": "2", + "str:quorum_for_action|u32:3": "2", "str:user_role|u32:1": "2", "str:user_role|u32:2": "2", "str:user_role|u32:3": "2", diff --git a/contracts/multisig/scenarios/deployOtherMultisig.scen.json b/contracts/multisig/scenarios/deployOtherMultisig.scen.json index a180dd0c..d6f6c016 100644 --- a/contracts/multisig/scenarios/deployOtherMultisig.scen.json +++ b/contracts/multisig/scenarios/deployOtherMultisig.scen.json @@ -125,9 +125,9 @@ "nonce": "0", "balance": "0", "storage": { - "str:user_count": "1", - "str:user_address_to_id|address:paul": "1", - "str:user_id_to_address|u32:1": "address:paul", + "str:sc_user_count": "1", + "str:sc_user_address_to_id|address:paul": "1", + "str:sc_user_id_to_address|u32:1": "address:paul", "str:user_role|u32:1": "2", "str:num_board_members": "1", "str:quorum": "1" diff --git a/contracts/multisig/scenarios/steps/deploy.steps.json b/contracts/multisig/scenarios/steps/deploy.steps.json index 3f3796ce..86b631ff 100644 --- a/contracts/multisig/scenarios/steps/deploy.steps.json +++ b/contracts/multisig/scenarios/steps/deploy.steps.json @@ -94,13 +94,13 @@ "nonce": "0", "balance": "0", "storage": { - "str:user_count": "3", - "str:user_address_to_id|address:alice": "1", - "str:user_address_to_id|address:bob": "2", - "str:user_address_to_id|address:charlie": "3", - "str:user_id_to_address|u32:1": "address:alice", - "str:user_id_to_address|u32:2": "address:bob", - "str:user_id_to_address|u32:3": "address:charlie", + "str:sc_user_count": "3", + "str:sc_user_address_to_id|address:alice": "1", + "str:sc_user_address_to_id|address:bob": "2", + "str:sc_user_address_to_id|address:charlie": "3", + "str:sc_user_id_to_address|u32:1": "address:alice", + "str:sc_user_id_to_address|u32:2": "address:bob", + "str:sc_user_id_to_address|u32:3": "address:charlie", "str:user_role|u32:1": "2", "str:user_role|u32:2": "2", "str:user_role|u32:3": "2", diff --git a/contracts/multisig/scenarios/steps/init_accounts.steps.json b/contracts/multisig/scenarios/steps/init_accounts.steps.json index 2e16b4d4..c849cbc2 100644 --- a/contracts/multisig/scenarios/steps/init_accounts.steps.json +++ b/contracts/multisig/scenarios/steps/init_accounts.steps.json @@ -27,7 +27,7 @@ }, "address:paul": { "nonce": "0", - "balance": "0" + "balance": "1000" }, "sc:adder-code": { "code": "file:../../test-contracts/adder.wasm" diff --git a/contracts/multisig/scenarios/upgrade.scen.json b/contracts/multisig/scenarios/upgrade.scen.json index 89efaae4..b65e5035 100644 --- a/contracts/multisig/scenarios/upgrade.scen.json +++ b/contracts/multisig/scenarios/upgrade.scen.json @@ -102,9 +102,9 @@ "nonce": "0", "balance": "0", "storage": { - "str:user_count": "1", - "str:user_address_to_id|address:paul": "1", - "str:user_id_to_address|u32:1": "address:paul", + "str:sc_user_count": "1", + "str:sc_user_address_to_id|address:paul": "1", + "str:sc_user_id_to_address|u32:1": "address:paul", "str:user_role|u32:1": "2", "str:num_board_members": "1", "str:quorum": "1", diff --git a/contracts/multisig/scenarios/upgrade_from_source.scen.json b/contracts/multisig/scenarios/upgrade_from_source.scen.json index f8d1d7a9..d8d3be49 100644 --- a/contracts/multisig/scenarios/upgrade_from_source.scen.json +++ b/contracts/multisig/scenarios/upgrade_from_source.scen.json @@ -103,11 +103,11 @@ "nonce": "0", "balance": "0", "storage": { - "str:user_count": "2", - "str:user_address_to_id|address:paul": "1", - "str:user_address_to_id|address:alice": "2", - "str:user_id_to_address|u32:1": "address:paul", - "str:user_id_to_address|u32:2": "address:alice", + "str:sc_user_count": "2", + "str:sc_user_address_to_id|address:paul": "1", + "str:sc_user_address_to_id|address:alice": "2", + "str:sc_user_id_to_address|u32:1": "address:paul", + "str:sc_user_id_to_address|u32:2": "address:alice", "str:user_role|u32:1": "2", "str:user_role|u32:2": "2", "str:num_board_members": "2", From caaa6d79e1ca48e7190ec1556b204f74bb129c76 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 09:25:02 +0200 Subject: [PATCH 21/44] fix mandos tests part 2 --- .../multisig/scenarios/upgrade_from_source.scen.json | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/contracts/multisig/scenarios/upgrade_from_source.scen.json b/contracts/multisig/scenarios/upgrade_from_source.scen.json index d8d3be49..38eb20bf 100644 --- a/contracts/multisig/scenarios/upgrade_from_source.scen.json +++ b/contracts/multisig/scenarios/upgrade_from_source.scen.json @@ -16,9 +16,7 @@ "sc:multisig-child", "0", "sc:multisig", - "0x0000", - "1", - "address:alice" + "0x0000" ], "gasLimit": "20,000,000", "gasPrice": "0" @@ -103,14 +101,11 @@ "nonce": "0", "balance": "0", "storage": { - "str:sc_user_count": "2", + "str:sc_user_count": "1", "str:sc_user_address_to_id|address:paul": "1", - "str:sc_user_address_to_id|address:alice": "2", "str:sc_user_id_to_address|u32:1": "address:paul", - "str:sc_user_id_to_address|u32:2": "address:alice", "str:user_role|u32:1": "2", - "str:user_role|u32:2": "2", - "str:num_board_members": "2", + "str:num_board_members": "1", "str:quorum": "1", "str:sum": "1234" }, From 8abdc919a92ec23ce28c4bd609c04855780bbe02 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 09:35:42 +0200 Subject: [PATCH 22/44] fix mandos tests part 3 --- .../scenarios/call_other_shard-2.scen.json | 20 ++++++++++++++++++- .../scenarios/steps/init_accounts.steps.json | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/contracts/multisig/scenarios/call_other_shard-2.scen.json b/contracts/multisig/scenarios/call_other_shard-2.scen.json index b265b2b3..75a5a35e 100644 --- a/contracts/multisig/scenarios/call_other_shard-2.scen.json +++ b/contracts/multisig/scenarios/call_other_shard-2.scen.json @@ -26,7 +26,7 @@ "function": "proposeTransferExecute", "arguments": [ "sc:other-shard-2", - "0", + "100", "0x", "str:method-from-other-shard", "str:arg1", @@ -45,6 +45,24 @@ "refund": "*" } }, + { + "step": "scCall", + "tx": { + "from": "address:alice", + "to": "sc:multisig", + "egldValue": "100", + "function": "deposit", + "arguments": [], + "gasLimit": "70,000,000", + "gasPrice": "0" + }, + "expect": { + "out": [], + "status": "0", + "gas": "*", + "refund": "*" + } + }, { "step": "scCall", "id": "perform-send-to-other-shard-2", diff --git a/contracts/multisig/scenarios/steps/init_accounts.steps.json b/contracts/multisig/scenarios/steps/init_accounts.steps.json index c849cbc2..30233adb 100644 --- a/contracts/multisig/scenarios/steps/init_accounts.steps.json +++ b/contracts/multisig/scenarios/steps/init_accounts.steps.json @@ -11,7 +11,7 @@ }, "address:alice": { "nonce": "0", - "balance": "0" + "balance": "1000" }, "address:bob": { "nonce": "0", From 697d60f0213c2c046b8e384c8427d8ca63fc43aa Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 09:42:59 +0200 Subject: [PATCH 23/44] fixes after Dorin's review --- contracts/multisig/src/multisig.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index 53593121..d36a005c 100644 --- a/contracts/multisig/src/multisig.rs +++ b/contracts/multisig/src/multisig.rs @@ -62,27 +62,27 @@ pub trait Multisig: #[view(getPendingActionFullInfo)] fn get_pending_action_full_info( &self, - count: OptionalValue, - offset_id: OptionalValue, + opt_count: OptionalValue, + opt_offset_id: OptionalValue, ) -> MultiValueEncoded> { let mut result = MultiValueEncoded::new(); let action_last_index = self.get_action_last_index(); let action_mapper = self.action_mapper(); let mut index_of_first_action = 1; let mut index_of_last_action = action_last_index; - if let OptionalValue::Some(unwrapped_offset_id) = offset_id { + if let OptionalValue::Some(offset_id) = opt_offset_id { require!( - unwrapped_offset_id <= action_last_index, + offset_id <= action_last_index, "offset_id needs to be within the range of the available action ids" ); - index_of_first_action = unwrapped_offset_id; + index_of_first_action = offset_id; } - if let OptionalValue::Some(unwrapped_count) = count { + if let OptionalValue::Some(count) = opt_count { require!( - unwrapped_count <= action_last_index, + count <= action_last_index, "cannot exceed the total number of actions" ); - index_of_last_action = index_of_first_action + unwrapped_count; + index_of_last_action = index_of_first_action + count; } for action_id in index_of_first_action..=index_of_last_action { let action_data = action_mapper.get(action_id); From 0823c74f5f681abc14f4fd497953e19847563395 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 10:08:49 +0200 Subject: [PATCH 24/44] update adder and factorial wasm files --- contracts/multisig/test-contracts/adder.wasm | Bin 695 -> 832 bytes .../multisig/test-contracts/factorial.wasm | Bin 577 -> 679 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/contracts/multisig/test-contracts/adder.wasm b/contracts/multisig/test-contracts/adder.wasm index b6bc9b4e13b3123daeafd40369383a54197cd160..41e7f8bbcee8fac7e13085a8da6ac58861660729 100755 GIT binary patch literal 832 zcmaJ<%Wl&^6uoyQFEWA*ZB-NmlF6#9x?_oOBvK)v3aPMmCh^1xjqPgefU>fAmv^^F zS@1)AQ<#TcgoNDGC?^7N*f}TG#?GMva@5>98Odgi)Ei z=uZ~Wqa;6)`LiTjM2QT26|G{KWUppqK3wNA18UN)P3LlYdYnEH zYkP)<0r%V)M~h^U%`fwthPvV?KMbKoSr@U{25gw8iD#&n6oIz(X@P!Oc31)a&p)L- zYPVt;r)o{0q0Mu+-qN4zWEH~>720yQGZo9_P)tu@uNwrR$VDKNkRl6$xfEx?L}ZeZ z?l02aM=X5cU&^wC0dQSP@POU6unz}hQ=XUL7taT9&-I%1fIQ%@ob>1_^qAhHQT6$2 z0|GnH<6iBG{Eb0xdUzT0)&bf{3oDdp{#}K8gul0~-AFy=|F9{Pf2hYz4nH?5ANf~P z`DgRK1N_2Ny@&6nGJP~BgGy#;{>_ejw37Qs|LZkCCm#@xXx~rPAN5XM0s+Pj3E1np g^-`rt7IS*GvE-{`AsY_{9&Z(w5VEdT%j delta 405 zcmXX=Jx>Bb5S`h*FK+c1BOw7bInP40&`w*oF|jdpmUjzx0Yf+lqDGsgvL{p~7XAxc zOMjMgKrvtMy?Hag-jCY>^E5^Pz_TC&$N+67tlebD&_X6{H7`3CD3=eXoa{%_l|}gy z^@l}&bU*2uw{cO<0agdtzIH0A)_WY>m2;iv;7JuCI=-x~fXhXCbzwn&F9NkKem7XX z7aURVy$t+N%ERfu8V8Ie~9k*&LB&d)OYiHt2ozU2hYzNc@jLWmU z{y`S}BYskti4##-y7%1go@0+jhAI*QK#$rZWGr4HBW z+unE>7eC-CM0HxyJ=TNng?{p}r`F?qWU`Cvo&u^ukE=nH#r7aqt@4nj!C&raFwZ`Z|p^2`=djpQ%4 zIE4JM>BNRUFM%bL1h6GvmmlAI3AJf&Q{8abf zOz-^lTZ}w8AIOiU|6X^SvNlE9mWRsJJ($nWO4HvTs%NqTc~*F--hF7>rX6(Cd~NdW zee*>>{x61?CFhAwjL;L~fp}l-5xu~7haLC}oEF7^F9-eYrrvbx=LHe)C?pEp8;m%C zhcjl8{yE0td_{LrSedG69~lBy+T&vUp!2?IH+qL@oa^20M(111qQkyybzyLkWLcqm zote7e5Lvc1dY`RyXE-IlDV!2nDXwrs5(;AOVES~7y}f2gX?ulh<};U&OyM?oHxud6 z3FgwZiy?y)aw5J`U|mRZ;-G6MNnycdvT!wWf#bub{b6^#X6uAQ#ITnh^ tctS#q6cchRI9&Y3?ck<~`OmhgOI07Lm1$M;p!E4OJNQ*a@N_zz{sBK5j6wha From 9e98b0169e4a8052885f0e1277a790e9c5631448 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 10:34:12 +0200 Subject: [PATCH 25/44] update code metadata - contract upgradable --- contracts/multisig/scenarios/deployAdder_err.scen.json | 2 +- contracts/multisig/scenarios/deployAdder_then_call.scen.json | 4 ++-- contracts/multisig/scenarios/deployFactorial.scen.json | 4 ++-- contracts/multisig/scenarios/deployOtherMultisig.scen.json | 2 +- contracts/multisig/scenarios/upgrade.scen.json | 2 +- contracts/multisig/scenarios/upgrade_from_source.scen.json | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/multisig/scenarios/deployAdder_err.scen.json b/contracts/multisig/scenarios/deployAdder_err.scen.json index b0403cac..80b788bd 100644 --- a/contracts/multisig/scenarios/deployAdder_err.scen.json +++ b/contracts/multisig/scenarios/deployAdder_err.scen.json @@ -27,7 +27,7 @@ "arguments": [ "0", "sc:adder-code", - "0x0000" + "0x0502" ], "gasLimit": "200,000,000", "gasPrice": "0" diff --git a/contracts/multisig/scenarios/deployAdder_then_call.scen.json b/contracts/multisig/scenarios/deployAdder_then_call.scen.json index 6bd68ce6..d3c58a73 100644 --- a/contracts/multisig/scenarios/deployAdder_then_call.scen.json +++ b/contracts/multisig/scenarios/deployAdder_then_call.scen.json @@ -26,7 +26,7 @@ "arguments": [ "0", "sc:adder-code", - "0x0000", + "0x0502", "1234" ], "gasLimit": "200,000,000", @@ -76,7 +76,7 @@ "1-discriminant": "0x08", "2-amount": "u32:0", "3-code_source": "sc:adder-code", - "4-code_metadata": "0x0000", + "4-code_metadata": "0x0502", "5-arguments": [ "u32:1", "u32:2|1234" diff --git a/contracts/multisig/scenarios/deployFactorial.scen.json b/contracts/multisig/scenarios/deployFactorial.scen.json index cef13491..c25bdd5d 100644 --- a/contracts/multisig/scenarios/deployFactorial.scen.json +++ b/contracts/multisig/scenarios/deployFactorial.scen.json @@ -26,7 +26,7 @@ "arguments": [ "0", "sc:factorial-code", - "0x0000" + "0x0502" ], "gasLimit": "200,000,000", "gasPrice": "0" @@ -75,7 +75,7 @@ "1-discriminant": "0x08", "2-amount": "u32:0", "3-code_source": "sc:factorial-code", - "4-code_metadata": "0x0000", + "4-code_metadata": "0x0502", "5-arguments": "u32:0" } }, diff --git a/contracts/multisig/scenarios/deployOtherMultisig.scen.json b/contracts/multisig/scenarios/deployOtherMultisig.scen.json index d6f6c016..30770ffa 100644 --- a/contracts/multisig/scenarios/deployOtherMultisig.scen.json +++ b/contracts/multisig/scenarios/deployOtherMultisig.scen.json @@ -26,7 +26,7 @@ "arguments": [ "0", "sc:multisig", - "0x0000", + "0x0502", "1", "address:paul" ], diff --git a/contracts/multisig/scenarios/upgrade.scen.json b/contracts/multisig/scenarios/upgrade.scen.json index b65e5035..daaa0a55 100644 --- a/contracts/multisig/scenarios/upgrade.scen.json +++ b/contracts/multisig/scenarios/upgrade.scen.json @@ -16,7 +16,7 @@ "sc:multisig-child", "0", "sc:adder-code", - "0x0000", + "0x0502", "1234" ], "gasLimit": "15,000,000", diff --git a/contracts/multisig/scenarios/upgrade_from_source.scen.json b/contracts/multisig/scenarios/upgrade_from_source.scen.json index 38eb20bf..894cb366 100644 --- a/contracts/multisig/scenarios/upgrade_from_source.scen.json +++ b/contracts/multisig/scenarios/upgrade_from_source.scen.json @@ -16,7 +16,7 @@ "sc:multisig-child", "0", "sc:multisig", - "0x0000" + "0x0502" ], "gasLimit": "20,000,000", "gasPrice": "0" From ff740ad16e21be97296ee43e8d1b4303908d80ff Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 11:29:14 +0200 Subject: [PATCH 26/44] rebuild test contrats wasm - fix upgrade --- contracts/multisig/test-contracts/adder.wasm | Bin 832 -> 842 bytes .../multisig/test-contracts/factorial.wasm | Bin 679 -> 689 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/contracts/multisig/test-contracts/adder.wasm b/contracts/multisig/test-contracts/adder.wasm index 41e7f8bbcee8fac7e13085a8da6ac58861660729..8e6480c1cda1fcf41f11e0027b5047d3fca8df32 100755 GIT binary patch delta 169 zcmX@Wc8YC6^Tb_N6PwK^eu-dWV4v*AsKpe(F}aFSi;KOqAiXFtC6z&N@)E{UPX$KC zdV!vXh6V;DCIv=;W)MqCfkA;upasn2)&MF{V1O!N6ljGgV1x=V2(&?UOip5QlIsI; mxRn?LIzS9Z27z7>EieI0_D?> delta 19 acmdnUx}0@`5hIf`+hh|)_06e_K8yf1umt7+ From 591070ea32ce27f464d15d306e91624fad063878 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 12:43:54 +0200 Subject: [PATCH 27/44] set the check for tokens in batch, not in perform --- .../scenarios/call_other_shard-2.scen.json | 20 +--- .../scenarios/deployAdder_then_call.scen.json | 22 +--- .../scenarios/steps/init_accounts.steps.json | 4 +- contracts/multisig/src/multisig_perform.rs | 6 +- contracts/multisig/src/multisig_propose.rs | 5 + .../multisig/tests/multisig_blackbox_test.rs | 100 +----------------- .../multisig/tests/multisig_whitebox_test.rs | 20 +--- 7 files changed, 16 insertions(+), 161 deletions(-) diff --git a/contracts/multisig/scenarios/call_other_shard-2.scen.json b/contracts/multisig/scenarios/call_other_shard-2.scen.json index 75a5a35e..b265b2b3 100644 --- a/contracts/multisig/scenarios/call_other_shard-2.scen.json +++ b/contracts/multisig/scenarios/call_other_shard-2.scen.json @@ -26,7 +26,7 @@ "function": "proposeTransferExecute", "arguments": [ "sc:other-shard-2", - "100", + "0", "0x", "str:method-from-other-shard", "str:arg1", @@ -45,24 +45,6 @@ "refund": "*" } }, - { - "step": "scCall", - "tx": { - "from": "address:alice", - "to": "sc:multisig", - "egldValue": "100", - "function": "deposit", - "arguments": [], - "gasLimit": "70,000,000", - "gasPrice": "0" - }, - "expect": { - "out": [], - "status": "0", - "gas": "*", - "refund": "*" - } - }, { "step": "scCall", "id": "perform-send-to-other-shard-2", diff --git a/contracts/multisig/scenarios/deployAdder_then_call.scen.json b/contracts/multisig/scenarios/deployAdder_then_call.scen.json index d3c58a73..7d2acc4c 100644 --- a/contracts/multisig/scenarios/deployAdder_then_call.scen.json +++ b/contracts/multisig/scenarios/deployAdder_then_call.scen.json @@ -186,7 +186,7 @@ "function": "proposeTransferExecute", "arguments": [ "sc:adder", - "100", + "0", "0x", "str:add", "1234" @@ -246,24 +246,6 @@ "refund": "*" } }, - { - "step": "scCall", - "tx": { - "from": "address:paul", - "to": "sc:multisig", - "egldValue": "100", - "function": "deposit", - "arguments": [], - "gasLimit": "70,000,000", - "gasPrice": "0" - }, - "expect": { - "out": [], - "status": "0", - "gas": "*", - "refund": "*" - } - }, { "step": "scCall", "id": "sc-deploy-perform-2", @@ -325,7 +307,7 @@ "accounts": { "sc:adder": { "nonce": "0", - "balance": "100", + "balance": "0", "storage": { "str:sum": "2468" }, diff --git a/contracts/multisig/scenarios/steps/init_accounts.steps.json b/contracts/multisig/scenarios/steps/init_accounts.steps.json index 30233adb..2e16b4d4 100644 --- a/contracts/multisig/scenarios/steps/init_accounts.steps.json +++ b/contracts/multisig/scenarios/steps/init_accounts.steps.json @@ -11,7 +11,7 @@ }, "address:alice": { "nonce": "0", - "balance": "1000" + "balance": "0" }, "address:bob": { "nonce": "0", @@ -27,7 +27,7 @@ }, "address:paul": { "nonce": "0", - "balance": "1000" + "balance": "0" }, "sc:adder-code": { "code": "file:../../test-contracts/adder.wasm" diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index f42591a9..0dab1071 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -236,7 +236,6 @@ pub trait MultisigPerformModule: OptionalValue::None } Action::SendTransferExecuteEgld(call_data) => { - require!(call_data.egld_amount != 0, "EGLD amount cannot be zero"); let gas = call_data .opt_gas_limit .unwrap_or_else(|| self.ensure_and_get_gas_for_transfer_exec()); @@ -266,10 +265,7 @@ pub trait MultisigPerformModule: .opt_gas_limit .unwrap_or_else(|| self.blockchain().get_gas_left()); let _ = self.ensure_and_get_gas_for_transfer_exec(); - require!( - call_data.tokens.is_empty(), - "number of tokens cannot be zero" - ); + self.perform_transfer_execute_esdt_event( action_id, &call_data.to, diff --git a/contracts/multisig/src/multisig_propose.rs b/contracts/multisig/src/multisig_propose.rs index c85e9267..c8bc13b1 100644 --- a/contracts/multisig/src/multisig_propose.rs +++ b/contracts/multisig/src/multisig_propose.rs @@ -215,6 +215,10 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { if let Action::SendTransferExecuteEgld(call_data) = &action { let other_sc_shard = self.blockchain().get_shard_of_address(&call_data.to); + require!( + call_data.egld_amount > 0 || !call_data.endpoint_name.is_empty(), + "proposed action has no effect" + ); require!( own_shard == other_sc_shard, "All transfer exec must be to the same shard" @@ -223,6 +227,7 @@ pub trait MultisigProposeModule: crate::multisig_state::MultisigStateModule { } if let Action::SendTransferExecuteEsdt(call_data) = &action { + require!(!call_data.tokens.is_empty(), "No tokens to transfer"); let other_sc_shard = self.blockchain().get_shard_of_address(&call_data.to); require!( own_shard == other_sc_shard, diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index 4d8b1341..789625b9 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -9,11 +9,8 @@ use multiversx_sc::{ test_util::top_encode_to_vec_u8_or_panic, }, storage::mappers::SingleValue, - types::{ - Address, CodeMetadata, ContractCallNoPayment, EsdtTokenPayment, FunctionCall, ManagedVec, - }, + types::{Address, CodeMetadata, ContractCallNoPayment, FunctionCall}, }; -use multiversx_sc_modules::transfer_role_proxy::PaymentsVec; use multiversx_sc_scenario::{ api::StaticApi, scenario_model::{ @@ -155,23 +152,6 @@ impl MultisigTestState { .call(self.multisig_contract.propose_change_quorum(new_quorum)), ) } - fn propose_transfer_execute_esdt( - &mut self, - to: Address, - tokens: PaymentsVec, - opt_gas_limit: Option, - contract_call: ContractCallNoPayment, - ) -> usize { - self.world - .sc_call_get_result(ScCallStep::new().from(PROPOSER_ADDRESS_EXPR).call( - self.multisig_contract.propose_transfer_execute_esdt( - to, - tokens, - opt_gas_limit, - contract_call.into_function_call(), - ), - )) - } fn propose_transfer_execute( &mut self, @@ -387,16 +367,9 @@ fn test_perform_action_signed_by_removed_board_user() { let adder_call = state.adder_contract.add(5u64); - state.world.sc_call( - ScCallStep::new() - .from(PROPOSER_ADDRESS_EXPR) - .egld_value("100") - .call(state.multisig_contract.deposit()), - ); - let special_action_id = state.propose_transfer_execute( state.adder_address.clone(), - 100u64, + 0u64, Option::::None, adder_call, ); @@ -594,53 +567,13 @@ fn test_transfer_execute_sc_all() { let mut state = MultisigTestState::new(); state.deploy_multisig_contract().deploy_adder_contract(); - // try make a transfer propose with 0 EGLD - - let adder_call = state.adder_contract.add(5u64); - - let action_id = state.propose_transfer_execute( - state.adder_address.clone(), - 0u64, - Option::::None, - adder_call, - ); - state.sign(action_id); - state.perform_and_expect_err(action_id, "EGLD amount cannot be zero"); - - // try make a transfer propose with 0 EGLD - - let adder_call = state.adder_contract.add(5u64); - - let mut esdt_payment: PaymentsVec = ManagedVec::new(); - esdt_payment.push(EsdtTokenPayment { - token_identifier: NFT_TOKEN_ID.into(), - token_nonce: 1u64, - amount: 1u64.into(), - }); - - let action_id = state.propose_transfer_execute_esdt( - state.adder_address.clone(), - esdt_payment, - Option::::None, - adder_call, - ); - state.sign(action_id); - state.perform_and_expect_err(action_id, "number of tokens cannot be zero"); - // perform an EGLD transfer purpose after a deposit to the contract let adder_call = state.adder_contract.add(5u64); - state.world.sc_call( - ScCallStep::new() - .from(PROPOSER_ADDRESS_EXPR) - .egld_value("100") - .call(state.multisig_contract.deposit()), - ); - let action_id = state.propose_transfer_execute( state.adder_address.clone(), - 100u64, + 0u64, Option::::None, adder_call, ); @@ -652,33 +585,6 @@ fn test_transfer_execute_sc_all() { .call(state.adder_contract.sum()) .expect_value(SingleValue::from(BigUint::from(10u64))), ); - - // perform an ESDT transfer purpose after a deposit to the contract - - let adder_call = state.adder_contract.add(5u64); - - let mut esdt_payment: PaymentsVec = ManagedVec::new(); - esdt_payment.push(EsdtTokenPayment { - token_identifier: NFT_TOKEN_ID.into(), - token_nonce: 1u64, - amount: 1u64.into(), - }); - - state.world.sc_call( - ScCallStep::new() - .from(PROPOSER_ADDRESS_EXPR) - .esdt_transfer(NFT_TOKEN_ID, 1u64, "1") - .call(state.multisig_contract.deposit()), - ); - - let action_id = state.propose_transfer_execute_esdt( - state.adder_address.clone(), - esdt_payment, - Option::::None, - adder_call, - ); - state.sign(action_id); - state.perform_and_expect_err(action_id, "number of tokens cannot be zero"); } #[test] diff --git a/contracts/multisig/tests/multisig_whitebox_test.rs b/contracts/multisig/tests/multisig_whitebox_test.rs index 3ec64850..68333061 100644 --- a/contracts/multisig/tests/multisig_whitebox_test.rs +++ b/contracts/multisig/tests/multisig_whitebox_test.rs @@ -616,19 +616,11 @@ fn test_transfer_execute_sc_all() { }, ); - world.whitebox_call( - &multisig_whitebox, - ScCallStep::new() - .from(PROPOSER_ADDRESS_EXPR) - .egld_value("100"), - |sc| sc.deposit(), - ); - let action_id = call_propose( &mut world, ActionRaw::SendTransferExecute(CallActionDataRaw { to: address_expr_to_address(ADDER_ADDRESS_EXPR), - egld_amount: 100u64.into(), + egld_amount: 0u64.into(), endpoint_name: BoxedBytes::from(&b"add"[..]), arguments: vec![BoxedBytes::from(&[5u8][..])], }), @@ -779,19 +771,11 @@ fn test_deploy_and_upgrade_from_source() { assert_eq!(address_expr_to_address(NEW_ADDER_ADDRESS_EXPR), addr); - world.whitebox_call( - &multisig_whitebox, - ScCallStep::new() - .from(PROPOSER_ADDRESS_EXPR) - .egld_value("100"), - |sc| sc.deposit(), - ); - let action_id = call_propose( &mut world, ActionRaw::SendTransferExecute(CallActionDataRaw { to: address_expr_to_address(NEW_ADDER_ADDRESS_EXPR), - egld_amount: 100u64.into(), + egld_amount: 0u64.into(), endpoint_name: BoxedBytes::from(&b"add"[..]), arguments: vec![BoxedBytes::from(&[5u8][..])], }), From fae5820f1e2d0a262170745351108b14b46e0429 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 13:05:49 +0200 Subject: [PATCH 28/44] rebuild adder wasm - attempt 3 --- contracts/multisig/test-contracts/adder.wasm | Bin 842 -> 836 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/contracts/multisig/test-contracts/adder.wasm b/contracts/multisig/test-contracts/adder.wasm index 8e6480c1cda1fcf41f11e0027b5047d3fca8df32..5172ef6b2eab511eaaf96ba93d1685401e0ff340 100755 GIT binary patch delta 161 zcmX@bc7$z0^TfU86Pv9jeu-dWXPE5ASi^XH@@d8hPYp)KdIbi7o`!}71|>#;W)Ll< zzz7v!5NLr2FmNjX6+%Usfby+SaRnxUHZXH?4wDmO|K#;dG6H=d0f9~s$!)>NTtE3C blcYFUP>D%_320jvNJ@bbsJDNzAhRU^T{bO4 delta 163 zcmX@Yc8YC6^Tb_N6PwK^eu-dWV4v*ASi^XB@@d8hPX$KCdV!vXh6V;DCIv=;W)MqC zfkA;upasn2)&MF{V1O!N6ljGgV1x=V2(&?UOwM6)V(go|o=JwkA0#Bu2_m`e7?~&c eG093PF$#c%fwnLKjq3tQDlh`A>6^^WYzY91?<}nV From 789b5010cdc7ea9502db6a92716117bb63ce1475 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 13:44:23 +0200 Subject: [PATCH 29/44] change vm-tools version --- .github/workflows/actions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index a69a895b..18598b9e 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -16,6 +16,6 @@ jobs: uses: multiversx/mx-sc-actions/.github/workflows/contracts.yml@v2.3.5 with: rust-toolchain: nightly-2023-12-11 - vmtools-version: v1.4.60 + vmtools-version: v1.5.24 secrets: token: ${{ secrets.GITHUB_TOKEN }} From 0fd235fe4c8491ab086c4fd0903788d169f02583 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 8 Mar 2024 14:03:51 +0200 Subject: [PATCH 30/44] update gas test - fail because vmtools version change --- contracts/crowdfunding-esdt/scenarios/_generated_fund.scen.json | 2 +- .../crowdfunding-esdt/scenarios/_generated_sc_err.scen.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/crowdfunding-esdt/scenarios/_generated_fund.scen.json b/contracts/crowdfunding-esdt/scenarios/_generated_fund.scen.json index 0bce2f06..048a7c67 100644 --- a/contracts/crowdfunding-esdt/scenarios/_generated_fund.scen.json +++ b/contracts/crowdfunding-esdt/scenarios/_generated_fund.scen.json @@ -109,7 +109,7 @@ ], "function": "fund", "arguments": [], - "gasLimit": "18446744073709551615" + "gasLimit": "5,000,000" }, "expect": { "out": [], diff --git a/contracts/crowdfunding-esdt/scenarios/_generated_sc_err.scen.json b/contracts/crowdfunding-esdt/scenarios/_generated_sc_err.scen.json index 165e03ba..ddeec24f 100644 --- a/contracts/crowdfunding-esdt/scenarios/_generated_sc_err.scen.json +++ b/contracts/crowdfunding-esdt/scenarios/_generated_sc_err.scen.json @@ -126,7 +126,7 @@ "egldValue": "1000", "function": "fund", "arguments": [], - "gasLimit": "18446744073709551615" + "gasLimit": "5,000,000" }, "expect": { "out": [], From 986af61a57ced01e7b03785899901405af578ec3 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Mon, 11 Mar 2024 13:32:41 +0200 Subject: [PATCH 31/44] fix the fixes part 1 --- .../scenarios/deployAdder_then_call.scen.json | 22 +++++----- .../scenarios/deployFactorial.scen.json | 44 +++++++++---------- .../scenarios/deployOtherMultisig.scen.json | 6 +-- .../scenarios/steps/deploy.steps.json | 14 +++--- .../multisig/scenarios/upgrade.scen.json | 6 +-- .../scenarios/upgrade_from_source.scen.json | 6 +-- contracts/multisig/src/multisig_perform.rs | 1 + contracts/multisig/src/multisig_state.rs | 2 +- 8 files changed, 51 insertions(+), 50 deletions(-) diff --git a/contracts/multisig/scenarios/deployAdder_then_call.scen.json b/contracts/multisig/scenarios/deployAdder_then_call.scen.json index 7d2acc4c..97af19c7 100644 --- a/contracts/multisig/scenarios/deployAdder_then_call.scen.json +++ b/contracts/multisig/scenarios/deployAdder_then_call.scen.json @@ -49,17 +49,17 @@ "nonce": "0", "balance": "0", "storage": { - "str:sc_user_count": "5", - "str:sc_user_address_to_id|address:alice": "1", - "str:sc_user_address_to_id|address:bob": "2", - "str:sc_user_address_to_id|address:charlie": "3", - "str:sc_user_address_to_id|address:paul": "4", - "str:sc_user_address_to_id|address:dan": "5", - "str:sc_user_id_to_address|u32:1": "address:alice", - "str:sc_user_id_to_address|u32:2": "address:bob", - "str:sc_user_id_to_address|u32:3": "address:charlie", - "str:sc_user_id_to_address|u32:4": "address:paul", - "str:sc_user_id_to_address|u32:5": "address:dan", + "str:user_ids_count": "5", + "str:user_ids_address_to_id|address:alice": "1", + "str:user_ids_address_to_id|address:bob": "2", + "str:user_ids_address_to_id|address:charlie": "3", + "str:user_ids_address_to_id|address:paul": "4", + "str:user_ids_address_to_id|address:dan": "5", + "str:user_ids_id_to_address|u32:1": "address:alice", + "str:user_ids_id_to_address|u32:2": "address:bob", + "str:user_ids_id_to_address|u32:3": "address:charlie", + "str:user_ids_id_to_address|u32:4": "address:paul", + "str:user_ids_id_to_address|u32:5": "address:dan", "str:quorum_for_action|u32:1": "2", "str:quorum_for_action|u32:2": "2", "str:quorum_for_action|u32:3": "2", diff --git a/contracts/multisig/scenarios/deployFactorial.scen.json b/contracts/multisig/scenarios/deployFactorial.scen.json index c25bdd5d..acef2e84 100644 --- a/contracts/multisig/scenarios/deployFactorial.scen.json +++ b/contracts/multisig/scenarios/deployFactorial.scen.json @@ -48,17 +48,17 @@ "nonce": "0", "balance": "0", "storage": { - "str:sc_user_count": "5", - "str:sc_user_address_to_id|address:alice": "1", - "str:sc_user_address_to_id|address:bob": "2", - "str:sc_user_address_to_id|address:charlie": "3", - "str:sc_user_address_to_id|address:paul": "4", - "str:sc_user_address_to_id|address:dan": "5", - "str:sc_user_id_to_address|u32:1": "address:alice", - "str:sc_user_id_to_address|u32:2": "address:bob", - "str:sc_user_id_to_address|u32:3": "address:charlie", - "str:sc_user_id_to_address|u32:4": "address:paul", - "str:sc_user_id_to_address|u32:5": "address:dan", + "str:user_ids_count": "5", + "str:user_ids_address_to_id|address:alice": "1", + "str:user_ids_address_to_id|address:bob": "2", + "str:user_ids_address_to_id|address:charlie": "3", + "str:user_ids_address_to_id|address:paul": "4", + "str:user_ids_address_to_id|address:dan": "5", + "str:user_ids_id_to_address|u32:1": "address:alice", + "str:user_ids_id_to_address|u32:2": "address:bob", + "str:user_ids_id_to_address|u32:3": "address:charlie", + "str:user_ids_id_to_address|u32:4": "address:paul", + "str:user_ids_id_to_address|u32:5": "address:dan", "str:quorum_for_action|u32:1": "2", "str:quorum_for_action|u32:2": "2", "str:quorum_for_action|u32:3": "2", @@ -166,17 +166,17 @@ "nonce": "1", "balance": "0", "storage": { - "str:sc_user_count": "5", - "str:sc_user_address_to_id|address:alice": "1", - "str:sc_user_address_to_id|address:bob": "2", - "str:sc_user_address_to_id|address:charlie": "3", - "str:sc_user_address_to_id|address:paul": "4", - "str:sc_user_address_to_id|address:dan": "5", - "str:sc_user_id_to_address|u32:1": "address:alice", - "str:sc_user_id_to_address|u32:2": "address:bob", - "str:sc_user_id_to_address|u32:3": "address:charlie", - "str:sc_user_id_to_address|u32:4": "address:paul", - "str:sc_user_id_to_address|u32:5": "address:dan", + "str:user_ids_count": "5", + "str:user_ids_address_to_id|address:alice": "1", + "str:user_ids_address_to_id|address:bob": "2", + "str:user_ids_address_to_id|address:charlie": "3", + "str:user_ids_address_to_id|address:paul": "4", + "str:user_ids_address_to_id|address:dan": "5", + "str:user_ids_id_to_address|u32:1": "address:alice", + "str:user_ids_id_to_address|u32:2": "address:bob", + "str:user_ids_id_to_address|u32:3": "address:charlie", + "str:user_ids_id_to_address|u32:4": "address:paul", + "str:user_ids_id_to_address|u32:5": "address:dan", "str:quorum_for_action|u32:1": "2", "str:quorum_for_action|u32:2": "2", "str:quorum_for_action|u32:3": "2", diff --git a/contracts/multisig/scenarios/deployOtherMultisig.scen.json b/contracts/multisig/scenarios/deployOtherMultisig.scen.json index 30770ffa..56407cf9 100644 --- a/contracts/multisig/scenarios/deployOtherMultisig.scen.json +++ b/contracts/multisig/scenarios/deployOtherMultisig.scen.json @@ -125,9 +125,9 @@ "nonce": "0", "balance": "0", "storage": { - "str:sc_user_count": "1", - "str:sc_user_address_to_id|address:paul": "1", - "str:sc_user_id_to_address|u32:1": "address:paul", + "str:user_ids_count": "1", + "str:user_ids_address_to_id|address:paul": "1", + "str:user_ids_id_to_address|u32:1": "address:paul", "str:user_role|u32:1": "2", "str:num_board_members": "1", "str:quorum": "1" diff --git a/contracts/multisig/scenarios/steps/deploy.steps.json b/contracts/multisig/scenarios/steps/deploy.steps.json index 86b631ff..a2533e64 100644 --- a/contracts/multisig/scenarios/steps/deploy.steps.json +++ b/contracts/multisig/scenarios/steps/deploy.steps.json @@ -94,13 +94,13 @@ "nonce": "0", "balance": "0", "storage": { - "str:sc_user_count": "3", - "str:sc_user_address_to_id|address:alice": "1", - "str:sc_user_address_to_id|address:bob": "2", - "str:sc_user_address_to_id|address:charlie": "3", - "str:sc_user_id_to_address|u32:1": "address:alice", - "str:sc_user_id_to_address|u32:2": "address:bob", - "str:sc_user_id_to_address|u32:3": "address:charlie", + "str:user_ids_count": "3", + "str:user_ids_address_to_id|address:alice": "1", + "str:user_ids_address_to_id|address:bob": "2", + "str:user_ids_address_to_id|address:charlie": "3", + "str:user_ids_id_to_address|u32:1": "address:alice", + "str:user_ids_id_to_address|u32:2": "address:bob", + "str:user_ids_id_to_address|u32:3": "address:charlie", "str:user_role|u32:1": "2", "str:user_role|u32:2": "2", "str:user_role|u32:3": "2", diff --git a/contracts/multisig/scenarios/upgrade.scen.json b/contracts/multisig/scenarios/upgrade.scen.json index daaa0a55..ab40888a 100644 --- a/contracts/multisig/scenarios/upgrade.scen.json +++ b/contracts/multisig/scenarios/upgrade.scen.json @@ -102,9 +102,9 @@ "nonce": "0", "balance": "0", "storage": { - "str:sc_user_count": "1", - "str:sc_user_address_to_id|address:paul": "1", - "str:sc_user_id_to_address|u32:1": "address:paul", + "str:user_ids_count": "1", + "str:user_ids_address_to_id|address:paul": "1", + "str:user_ids_id_to_address|u32:1": "address:paul", "str:user_role|u32:1": "2", "str:num_board_members": "1", "str:quorum": "1", diff --git a/contracts/multisig/scenarios/upgrade_from_source.scen.json b/contracts/multisig/scenarios/upgrade_from_source.scen.json index 894cb366..fa7205ca 100644 --- a/contracts/multisig/scenarios/upgrade_from_source.scen.json +++ b/contracts/multisig/scenarios/upgrade_from_source.scen.json @@ -101,9 +101,9 @@ "nonce": "0", "balance": "0", "storage": { - "str:sc_user_count": "1", - "str:sc_user_address_to_id|address:paul": "1", - "str:sc_user_id_to_address|u32:1": "address:paul", + "str:user_ids_count": "1", + "str:user_ids_address_to_id|address:paul": "1", + "str:user_ids_id_to_address|u32:1": "address:paul", "str:user_role|u32:1": "2", "str:num_board_members": "1", "str:quorum": "1", diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index 0dab1071..dea32b71 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -304,6 +304,7 @@ pub trait MultisigPerformModule: .contract_call::<()>(call_data.to, call_data.endpoint_name) .with_egld_transfer(call_data.egld_amount) .with_raw_arguments(call_data.arguments.into()) + .with_gas_limit(gas) .async_call() .with_callback(self.callbacks().perform_async_call_callback()) .call_and_exit() diff --git a/contracts/multisig/src/multisig_state.rs b/contracts/multisig/src/multisig_state.rs index c800af13..89282412 100644 --- a/contracts/multisig/src/multisig_state.rs +++ b/contracts/multisig/src/multisig_state.rs @@ -23,7 +23,7 @@ pub trait MultisigStateModule { #[storage_mapper("quorum")] fn quorum(&self) -> SingleValueMapper; - #[storage_mapper("sc_user")] + #[storage_mapper("user_ids")] fn user_mapper(&self) -> UserMapper; #[storage_mapper("quorum_for_action")] From 968076149499bfa381e4505f94786fca70a9b3a3 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Mon, 11 Mar 2024 13:41:29 +0200 Subject: [PATCH 32/44] fix the fixes part 2 --- contracts/multisig/src/multisig.rs | 10 ++++------ contracts/multisig/src/multisig_perform.rs | 2 +- contracts/multisig/src/multisig_state.rs | 7 ++++++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index d36a005c..cd6b98c3 100644 --- a/contracts/multisig/src/multisig.rs +++ b/contracts/multisig/src/multisig.rs @@ -62,24 +62,22 @@ pub trait Multisig: #[view(getPendingActionFullInfo)] fn get_pending_action_full_info( &self, - opt_count: OptionalValue, - opt_offset_id: OptionalValue, + opt_range: OptionalValue<(usize, usize)>, ) -> MultiValueEncoded> { let mut result = MultiValueEncoded::new(); let action_last_index = self.get_action_last_index(); let action_mapper = self.action_mapper(); let mut index_of_first_action = 1; let mut index_of_last_action = action_last_index; - if let OptionalValue::Some(offset_id) = opt_offset_id { + if let OptionalValue::Some((count, offset_id)) = opt_range { require!( offset_id <= action_last_index, "offset_id needs to be within the range of the available action ids" ); index_of_first_action = offset_id; - } - if let OptionalValue::Some(count) = opt_count { + require!( - count <= action_last_index, + index_of_first_action + count <= action_last_index, "cannot exceed the total number of actions" ); index_of_last_action = index_of_first_action + count; diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index dea32b71..e63e69bb 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -8,7 +8,7 @@ multiversx_sc::imports!(); /// Gas required to finish transaction after transfer-execute. const PERFORM_ACTION_FINISH_GAS: u64 = 300_000; -const MAX_BOARD_MEMBERS: usize = 30; +pub const MAX_BOARD_MEMBERS: usize = 30; fn usize_add_isize(value: &mut usize, delta: isize) { *value = (*value as isize + delta) as usize; diff --git a/contracts/multisig/src/multisig_state.rs b/contracts/multisig/src/multisig_state.rs index 89282412..3374b9fc 100644 --- a/contracts/multisig/src/multisig_state.rs +++ b/contracts/multisig/src/multisig_state.rs @@ -1,5 +1,5 @@ +use crate::multisig_perform::MAX_BOARD_MEMBERS; use crate::{action::Action, user_role::UserRole}; - multiversx_sc::imports!(); multiversx_sc::derive_imports!(); @@ -57,6 +57,11 @@ pub trait MultisigStateModule { fn add_multiple_board_members(&self, new_board_members: ManagedVec) -> usize { let mut duplicates = false; + require!( + self.num_board_members().get() + new_board_members.len() <= MAX_BOARD_MEMBERS, + "board size cannot exceed limit" + ); + self.user_mapper().get_or_create_users( new_board_members.into_iter(), |user_id, new_user| { From abebc6cf61758f942a658df236e1e1cb96b5d607 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Mon, 11 Mar 2024 13:46:25 +0200 Subject: [PATCH 33/44] fix the fixes part 3 --- contracts/multisig/src/multisig.rs | 6 ++++-- contracts/multisig/src/multisig_sign.rs | 13 ------------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index cd6b98c3..88ff81db 100644 --- a/contracts/multisig/src/multisig.rs +++ b/contracts/multisig/src/multisig.rs @@ -177,7 +177,9 @@ pub trait Multisig: } fn abort_batch_of_action(&self, action_id: ActionId) { let batch_id = self.group_for_action(action_id).get(); - self.action_group_status(batch_id) - .set(ActionStatus::Aborted); + if batch_id != 0 { + self.action_group_status(batch_id) + .set(ActionStatus::Aborted); + } } } diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index 986a37ef..7ac88ed6 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -83,14 +83,6 @@ pub trait MultisigSignModule: 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"); - let group_id = self.group_for_action(action_id).get(); - if group_id != 0 { - let group_status = self.action_group_status(group_id).get(); - require!( - group_status == ActionStatus::Available, - "cannot unsign actions of an aborted batch" - ); - } self.unsign_action(action_id, caller_id); } @@ -100,11 +92,6 @@ 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 group_status = self.action_group_status(group_id).get(); - require!( - group_status == ActionStatus::Available, - "cannot unsign actions of an aborted batch" - ); let mapper = self.action_groups(group_id); require!(!mapper.is_empty(), "Invalid group ID"); From ed904a139d29fc8173cc0a72a89062f4dd9e596f Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Mon, 11 Mar 2024 13:58:22 +0200 Subject: [PATCH 34/44] fix the fises part 5 --- .../scenarios/deployAdder_then_call.scen.json | 25 +++++++++++++++++++ contracts/multisig/src/multisig_perform.rs | 3 +-- contracts/multisig/src/multisig_sign.rs | 10 +++++--- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/contracts/multisig/scenarios/deployAdder_then_call.scen.json b/contracts/multisig/scenarios/deployAdder_then_call.scen.json index 97af19c7..33b3312b 100644 --- a/contracts/multisig/scenarios/deployAdder_then_call.scen.json +++ b/contracts/multisig/scenarios/deployAdder_then_call.scen.json @@ -204,6 +204,31 @@ "refund": "*" } }, + { + "step": "scCall", + "id": "proposeTransferExecute-fail", + "tx": { + "from": "address:paul", + "to": "sc:multisig", + "function": "proposeTransferExecute", + "arguments": [ + "sc:adder", + "0", + "0x", + "str:add", + "1234" + ], + "gasPrice": "0" + }, + "expect": { + "out": [], + "status": "5", + "message": "str:not enough gas", + "logs": "*", + "gas": "*", + "refund": "*" + } + }, { "step": "scCall", "id": "sc-deploy-sign-alice", diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index e63e69bb..7351617b 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -290,8 +290,7 @@ pub trait MultisigPerformModule: Action::SendAsyncCall(call_data) => { let gas = call_data .opt_gas_limit - .unwrap_or_else(|| self.blockchain().get_gas_left()); - let _ = self.ensure_and_get_gas_for_transfer_exec(); + .unwrap_or_else(|| self.ensure_and_get_gas_for_transfer_exec()); self.perform_async_call_event( action_id, &call_data.to, diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index 7ac88ed6..648449ab 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -71,9 +71,13 @@ pub trait MultisigSignModule: ); for action_id in self.action_groups(group_id).iter() { - if self.quorum_reached(action_id) { - let _ = self.perform_action(action_id); - } + require!( + self.quorum_reached(action_id), + "quorum has not been reached" + ); + } + for action_id in self.action_groups(group_id).iter() { + let _ = self.perform_action(action_id); } } From 83eb7558e114a09a25978f1383c6d34607a5b466 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Mon, 11 Mar 2024 15:40:00 +0200 Subject: [PATCH 35/44] sed default gas - not enough gas --- contracts/multisig/scenarios/deployAdder_then_call.scen.json | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/multisig/scenarios/deployAdder_then_call.scen.json b/contracts/multisig/scenarios/deployAdder_then_call.scen.json index 33b3312b..429060cd 100644 --- a/contracts/multisig/scenarios/deployAdder_then_call.scen.json +++ b/contracts/multisig/scenarios/deployAdder_then_call.scen.json @@ -218,6 +218,7 @@ "str:add", "1234" ], + "gasLimit": "5,000,000", "gasPrice": "0" }, "expect": { From 8c684c1a8fba146a108ee6013bf3135ac515cc2a Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Mon, 11 Mar 2024 16:31:36 +0200 Subject: [PATCH 36/44] transfer with no gas specified separate test --- .../scenarios/deployAdder_then_call.scen.json | 26 --- .../transfer_no_gas_specified_test.scen.json | 206 ++++++++++++++++++ .../tests/multisig_scenario_go_test.rs | 5 + .../tests/multisig_scenario_rs_test.rs | 6 + 4 files changed, 217 insertions(+), 26 deletions(-) create mode 100644 contracts/multisig/scenarios/transfer_no_gas_specified_test.scen.json diff --git a/contracts/multisig/scenarios/deployAdder_then_call.scen.json b/contracts/multisig/scenarios/deployAdder_then_call.scen.json index 429060cd..97af19c7 100644 --- a/contracts/multisig/scenarios/deployAdder_then_call.scen.json +++ b/contracts/multisig/scenarios/deployAdder_then_call.scen.json @@ -204,32 +204,6 @@ "refund": "*" } }, - { - "step": "scCall", - "id": "proposeTransferExecute-fail", - "tx": { - "from": "address:paul", - "to": "sc:multisig", - "function": "proposeTransferExecute", - "arguments": [ - "sc:adder", - "0", - "0x", - "str:add", - "1234" - ], - "gasLimit": "5,000,000", - "gasPrice": "0" - }, - "expect": { - "out": [], - "status": "5", - "message": "str:not enough gas", - "logs": "*", - "gas": "*", - "refund": "*" - } - }, { "step": "scCall", "id": "sc-deploy-sign-alice", diff --git a/contracts/multisig/scenarios/transfer_no_gas_specified_test.scen.json b/contracts/multisig/scenarios/transfer_no_gas_specified_test.scen.json new file mode 100644 index 00000000..3145e5c7 --- /dev/null +++ b/contracts/multisig/scenarios/transfer_no_gas_specified_test.scen.json @@ -0,0 +1,206 @@ +{ + "steps": [ + { + "step": "externalSteps", + "path": "steps/init_accounts.steps.json" + }, + { + "step": "externalSteps", + "path": "steps/deploy.steps.json" + }, + { + "step": "externalSteps", + "path": "steps/add_prop.steps.json" + }, + { + "step": "externalSteps", + "path": "steps/add_bm.steps.json" + }, + { + "step": "scCall", + "id": "proposeSCDeploy", + "tx": { + "from": "address:paul", + "to": "sc:multisig", + "function": "proposeSCDeployFromSource", + "arguments": [ + "0", + "sc:adder-code", + "0x0502", + "1234" + ], + "gasLimit": "200,000,000", + "gasPrice": "0" + }, + "expect": { + "out": [ + "3" + ], + "status": "", + "logs": "*", + "gas": "*", + "refund": "*" + } + }, + { + "step": "checkState", + "accounts": { + "sc:multisig": { + "nonce": "0", + "balance": "0", + "storage": { + "str:user_ids_count": "5", + "str:user_ids_address_to_id|address:alice": "1", + "str:user_ids_address_to_id|address:bob": "2", + "str:user_ids_address_to_id|address:charlie": "3", + "str:user_ids_address_to_id|address:paul": "4", + "str:user_ids_address_to_id|address:dan": "5", + "str:user_ids_id_to_address|u32:1": "address:alice", + "str:user_ids_id_to_address|u32:2": "address:bob", + "str:user_ids_id_to_address|u32:3": "address:charlie", + "str:user_ids_id_to_address|u32:4": "address:paul", + "str:user_ids_id_to_address|u32:5": "address:dan", + "str:quorum_for_action|u32:1": "2", + "str:quorum_for_action|u32:2": "2", + "str:quorum_for_action|u32:3": "2", + "str:user_role|u32:1": "2", + "str:user_role|u32:2": "2", + "str:user_role|u32:3": "2", + "str:user_role|u32:4": "1", + "str:user_role|u32:5": "2", + "str:num_board_members": "4", + "str:num_proposers": "1", + "str:quorum": "2", + "str:action_data.len": "3", + "str:action_data.item|u32:3": { + "1-discriminant": "0x08", + "2-amount": "u32:0", + "3-code_source": "sc:adder-code", + "4-code_metadata": "0x0502", + "5-arguments": [ + "u32:1", + "u32:2|1234" + ] + } + }, + "code": "file:../output/multisig.wasm" + }, + "+": "" + } + }, + { + "step": "scCall", + "id": "sc-deploy-sign-alice", + "tx": { + "from": "address:alice", + "to": "sc:multisig", + "function": "sign", + "arguments": [ + "3" + ], + "gasLimit": "50,000,000", + "gasPrice": "0" + }, + "expect": { + "out": [], + "status": "", + "logs": "*", + "gas": "*", + "refund": "*" + } + }, + { + "step": "scCall", + "id": "sc-deploy-sign-bob", + "tx": { + "from": "address:bob", + "to": "sc:multisig", + "function": "sign", + "arguments": [ + "3" + ], + "gasLimit": "50,000,000", + "gasPrice": "0" + }, + "expect": { + "out": [], + "status": "", + "logs": "*", + "gas": "*", + "refund": "*" + } + }, + { + "step": "setState", + "newAddresses": [ + { + "creatorAddress": "sc:multisig", + "creatorNonce": "0", + "newAddress": "sc:adder" + } + ] + }, + { + "step": "scCall", + "id": "sc-deploy-perform-1", + "tx": { + "from": "address:paul", + "to": "sc:multisig", + "function": "performAction", + "arguments": [ + "3" + ], + "gasLimit": "40,000,000", + "gasPrice": "0" + }, + "expect": { + "out": [ + "sc:adder" + ], + "status": "", + "logs": "*", + "gas": "*", + "refund": "*" + } + }, + { + "step": "checkState", + "accounts": { + "sc:adder": { + "nonce": "0", + "balance": "0", + "storage": { + "str:sum": "1234" + }, + "code": "file:../test-contracts/adder.wasm" + }, + "+": "" + } + }, + { + "step": "scCall", + "id": "proposeTransferExecute-fail", + "tx": { + "from": "address:paul", + "to": "sc:multisig", + "function": "proposeTransferExecute", + "arguments": [ + "sc:adder", + "0", + "0x", + "str:add", + "1234" + ], + "gasPrice": "0" + }, + "expect": { + "out": [], + "status": "5", + "message": "str:not enough gas", + "logs": "*", + "gas": "*", + "refund": "*" + } + } + ] +} diff --git a/contracts/multisig/tests/multisig_scenario_go_test.rs b/contracts/multisig/tests/multisig_scenario_go_test.rs index a5e40d03..27b67039 100644 --- a/contracts/multisig/tests/multisig_scenario_go_test.rs +++ b/contracts/multisig/tests/multisig_scenario_go_test.rs @@ -84,6 +84,11 @@ fn send_esdt_go() { world().run("scenarios/sendEsdt.scen.json"); } +#[test] +fn transfer_no_gas_specified_test_go() { + world().run("scenarios/transfer_no_gas_specified_test.scen.json"); +} + #[test] fn upgrade_go() { world().run("scenarios/upgrade.scen.json"); diff --git a/contracts/multisig/tests/multisig_scenario_rs_test.rs b/contracts/multisig/tests/multisig_scenario_rs_test.rs index 983f2908..02d6e6bf 100644 --- a/contracts/multisig/tests/multisig_scenario_rs_test.rs +++ b/contracts/multisig/tests/multisig_scenario_rs_test.rs @@ -104,6 +104,12 @@ fn remove_everyone_rs() { world().run("scenarios/remove_everyone.scen.json"); } +#[test] +#[ignore] +fn transfer_no_gas_specified_test_go() { + world().run("scenarios/transfer_no_gas_specified_test.scen.json"); +} + #[test] fn send_esdt_rs() { world().run("scenarios/sendEsdt.scen.json"); From 6d949b3ce2bb7b9454578b62eb6640dc1361c47e Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 15 Mar 2024 11:36:20 +0200 Subject: [PATCH 37/44] provide gas for SendTransferExecuteEsdt --- contracts/multisig/src/multisig_perform.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/multisig/src/multisig_perform.rs b/contracts/multisig/src/multisig_perform.rs index 7351617b..ae9834e2 100644 --- a/contracts/multisig/src/multisig_perform.rs +++ b/contracts/multisig/src/multisig_perform.rs @@ -263,8 +263,7 @@ pub trait MultisigPerformModule: Action::SendTransferExecuteEsdt(call_data) => { let gas = call_data .opt_gas_limit - .unwrap_or_else(|| self.blockchain().get_gas_left()); - let _ = self.ensure_and_get_gas_for_transfer_exec(); + .unwrap_or_else(|| self.ensure_and_get_gas_for_transfer_exec()); self.perform_transfer_execute_esdt_event( action_id, From 157037cba2b64fc60cc9cbaf2ea0320b9aac1a68 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 15 Mar 2024 11:40:54 +0200 Subject: [PATCH 38/44] solve A.7. --- contracts/multisig/src/multisig_sign.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index 648449ab..fa4b385b 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -70,14 +70,18 @@ pub trait MultisigSignModule: "only board members and proposers can perform actions" ); + let mut quorums_reached = true; + for action_id in self.action_groups(group_id).iter() { - require!( - self.quorum_reached(action_id), - "quorum has not been reached" - ); + if !self.quorum_reached(action_id) { + quorums_reached = false; + } } - for action_id in self.action_groups(group_id).iter() { - let _ = self.perform_action(action_id); + + if quorums_reached { + for action_id in self.action_groups(group_id).iter() { + let _ = self.perform_action(action_id); + } } } From 9c64008fdee2665c9e05ce5757b2ebab6f9b95c2 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 15 Mar 2024 11:44:26 +0200 Subject: [PATCH 39/44] solve ambiguous storage naming --- contracts/multisig/src/multisig_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/multisig/src/multisig_state.rs b/contracts/multisig/src/multisig_state.rs index 3374b9fc..034b4d40 100644 --- a/contracts/multisig/src/multisig_state.rs +++ b/contracts/multisig/src/multisig_state.rs @@ -20,7 +20,7 @@ pub enum ActionStatus { pub trait MultisigStateModule { /// Minimum number of signatures needed to perform any action. #[view(getQuorum)] - #[storage_mapper("quorum")] + #[storage_mapper("quorum_ids")] fn quorum(&self) -> SingleValueMapper; #[storage_mapper("user_ids")] From 6753e2cb8c5b90f6d13e0d37e1e4fc3ab8fb6dd9 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 15 Mar 2024 11:47:46 +0200 Subject: [PATCH 40/44] fix renaming in tests --- contracts/multisig/scenarios/deployAdder_then_call.scen.json | 2 +- contracts/multisig/scenarios/deployFactorial.scen.json | 4 ++-- contracts/multisig/scenarios/deployOtherMultisig.scen.json | 2 +- contracts/multisig/scenarios/steps/deploy.steps.json | 2 +- .../scenarios/transfer_no_gas_specified_test.scen.json | 2 +- contracts/multisig/scenarios/upgrade.scen.json | 2 +- contracts/multisig/scenarios/upgrade_from_source.scen.json | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/multisig/scenarios/deployAdder_then_call.scen.json b/contracts/multisig/scenarios/deployAdder_then_call.scen.json index 97af19c7..2431cb0e 100644 --- a/contracts/multisig/scenarios/deployAdder_then_call.scen.json +++ b/contracts/multisig/scenarios/deployAdder_then_call.scen.json @@ -70,7 +70,7 @@ "str:user_role|u32:5": "2", "str:num_board_members": "4", "str:num_proposers": "1", - "str:quorum": "2", + "str:quorum_ids": "2", "str:action_data.len": "3", "str:action_data.item|u32:3": { "1-discriminant": "0x08", diff --git a/contracts/multisig/scenarios/deployFactorial.scen.json b/contracts/multisig/scenarios/deployFactorial.scen.json index acef2e84..c2070134 100644 --- a/contracts/multisig/scenarios/deployFactorial.scen.json +++ b/contracts/multisig/scenarios/deployFactorial.scen.json @@ -69,7 +69,7 @@ "str:user_role|u32:5": "2", "str:num_board_members": "4", "str:num_proposers": "1", - "str:quorum": "2", + "str:quorum_ids": "2", "str:action_data.len": "3", "str:action_data.item|u32:3": { "1-discriminant": "0x08", @@ -187,7 +187,7 @@ "str:user_role|u32:5": "2", "str:num_board_members": "4", "str:num_proposers": "1", - "str:quorum": "2", + "str:quorum_ids": "2", "str:action_data.len": "3" }, "code": "file:../output/multisig.wasm" diff --git a/contracts/multisig/scenarios/deployOtherMultisig.scen.json b/contracts/multisig/scenarios/deployOtherMultisig.scen.json index 56407cf9..7f7ea891 100644 --- a/contracts/multisig/scenarios/deployOtherMultisig.scen.json +++ b/contracts/multisig/scenarios/deployOtherMultisig.scen.json @@ -130,7 +130,7 @@ "str:user_ids_id_to_address|u32:1": "address:paul", "str:user_role|u32:1": "2", "str:num_board_members": "1", - "str:quorum": "1" + "str:quorum_ids": "1" }, "code": "file:../output/multisig.wasm" }, diff --git a/contracts/multisig/scenarios/steps/deploy.steps.json b/contracts/multisig/scenarios/steps/deploy.steps.json index a2533e64..aa3198b7 100644 --- a/contracts/multisig/scenarios/steps/deploy.steps.json +++ b/contracts/multisig/scenarios/steps/deploy.steps.json @@ -105,7 +105,7 @@ "str:user_role|u32:2": "2", "str:user_role|u32:3": "2", "str:num_board_members": "3", - "str:quorum": "2" + "str:quorum_ids": "2" }, "code": "file:../../output/multisig.wasm" }, diff --git a/contracts/multisig/scenarios/transfer_no_gas_specified_test.scen.json b/contracts/multisig/scenarios/transfer_no_gas_specified_test.scen.json index 3145e5c7..36590098 100644 --- a/contracts/multisig/scenarios/transfer_no_gas_specified_test.scen.json +++ b/contracts/multisig/scenarios/transfer_no_gas_specified_test.scen.json @@ -70,7 +70,7 @@ "str:user_role|u32:5": "2", "str:num_board_members": "4", "str:num_proposers": "1", - "str:quorum": "2", + "str:quorum_ids": "2", "str:action_data.len": "3", "str:action_data.item|u32:3": { "1-discriminant": "0x08", diff --git a/contracts/multisig/scenarios/upgrade.scen.json b/contracts/multisig/scenarios/upgrade.scen.json index ab40888a..652be978 100644 --- a/contracts/multisig/scenarios/upgrade.scen.json +++ b/contracts/multisig/scenarios/upgrade.scen.json @@ -107,7 +107,7 @@ "str:user_ids_id_to_address|u32:1": "address:paul", "str:user_role|u32:1": "2", "str:num_board_members": "1", - "str:quorum": "1", + "str:quorum_ids": "1", "str:sum": "1234" }, "code": "file:../test-contracts/adder.wasm" diff --git a/contracts/multisig/scenarios/upgrade_from_source.scen.json b/contracts/multisig/scenarios/upgrade_from_source.scen.json index fa7205ca..a689ea4f 100644 --- a/contracts/multisig/scenarios/upgrade_from_source.scen.json +++ b/contracts/multisig/scenarios/upgrade_from_source.scen.json @@ -106,7 +106,7 @@ "str:user_ids_id_to_address|u32:1": "address:paul", "str:user_role|u32:1": "2", "str:num_board_members": "1", - "str:quorum": "1", + "str:quorum_ids": "1", "str:sum": "1234" }, "code": "file:../output/multisig.wasm" From c49dd966bc3de7a3c2866f3737380329b2664aca Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 15 Mar 2024 11:50:37 +0200 Subject: [PATCH 41/44] rename variable --- contracts/multisig/src/multisig.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/multisig/src/multisig.rs b/contracts/multisig/src/multisig.rs index 88ff81db..a72d2d2c 100644 --- a/contracts/multisig/src/multisig.rs +++ b/contracts/multisig/src/multisig.rs @@ -69,12 +69,12 @@ pub trait Multisig: let action_mapper = self.action_mapper(); let mut index_of_first_action = 1; let mut index_of_last_action = action_last_index; - if let OptionalValue::Some((count, offset_id)) = opt_range { + if let OptionalValue::Some((count, first_action_id)) = opt_range { require!( - offset_id <= action_last_index, - "offset_id needs to be within the range of the available action ids" + first_action_id <= action_last_index, + "first_action_id needs to be within the range of the available action ids" ); - index_of_first_action = offset_id; + index_of_first_action = first_action_id; require!( index_of_first_action + count <= action_last_index, From 319c2c865711ba5616e6d38730762e16e63d8b3f Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Fri, 15 Mar 2024 12:11:32 +0200 Subject: [PATCH 42/44] unsign_for_outdated_board_members arg as MultiValueEncoded --- contracts/multisig/src/multisig_sign.rs | 20 +++++++++++++------ .../multisig/tests/multisig_blackbox_test.rs | 7 ++++--- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index fa4b385b..5ace2842 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -130,14 +130,22 @@ pub trait MultisigSignModule: } #[endpoint(unsignForOutdatedBoardMembers)] - fn unsign_for_outdated_board_members(&self, action_id: ActionId) { - let mut outdated_board_members: ManagedVec = ManagedVec::new(); - for signer_id in self.action_signer_ids(action_id).iter() { - if !self.user_id_to_role(signer_id).get().can_sign() { - outdated_board_members.push(signer_id); + fn unsign_for_outdated_board_members( + &self, + action_id: ActionId, + outdated_board_members: MultiValueEncoded, + ) { + let mut board_members_to_remove: ManagedVec = ManagedVec::new(); + if outdated_board_members.is_empty() { + for signer_id in self.action_signer_ids(action_id).iter() { + if !self.user_id_to_role(signer_id).get().can_sign() { + board_members_to_remove.push(signer_id); + } } + } else { + board_members_to_remove = outdated_board_members.to_vec(); } - for member in outdated_board_members.iter() { + for member in board_members_to_remove.iter() { self.action_signer_ids(action_id).swap_remove(&member); } } diff --git a/contracts/multisig/tests/multisig_blackbox_test.rs b/contracts/multisig/tests/multisig_blackbox_test.rs index 789625b9..71c9744e 100644 --- a/contracts/multisig/tests/multisig_blackbox_test.rs +++ b/contracts/multisig/tests/multisig_blackbox_test.rs @@ -408,9 +408,10 @@ fn test_perform_action_signed_by_removed_board_user() { state.world.sc_call( ScCallStep::new().from(PROPOSER_ADDRESS_EXPR).call( - state - .multisig_contract - .unsign_for_outdated_board_members(special_action_id), + state.multisig_contract.unsign_for_outdated_board_members( + special_action_id, + MultiValueVec::::new(), + ), ), ); state.sign(special_action_id); From 6444205c6e2d08d62b5893c027a3b1acb8472728 Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Sat, 16 Mar 2024 03:56:58 +0200 Subject: [PATCH 43/44] check for no role before remove outdated BM --- contracts/multisig/src/multisig_sign.rs | 8 ++++++-- contracts/multisig/src/user_role.rs | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index 5ace2842..f77c6566 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -138,12 +138,16 @@ pub trait MultisigSignModule: let mut board_members_to_remove: ManagedVec = ManagedVec::new(); if outdated_board_members.is_empty() { for signer_id in self.action_signer_ids(action_id).iter() { - if !self.user_id_to_role(signer_id).get().can_sign() { + if self.user_id_to_role(signer_id).get().has_no_role() { board_members_to_remove.push(signer_id); } } } else { - board_members_to_remove = outdated_board_members.to_vec(); + for signer_id in outdated_board_members.into_iter() { + if self.user_id_to_role(signer_id).get().has_no_role() { + board_members_to_remove.push(signer_id); + } + } } for member in board_members_to_remove.iter() { self.action_signer_ids(action_id).swap_remove(&member); diff --git a/contracts/multisig/src/user_role.rs b/contracts/multisig/src/user_role.rs index fa401130..3423a174 100644 --- a/contracts/multisig/src/user_role.rs +++ b/contracts/multisig/src/user_role.rs @@ -23,4 +23,8 @@ impl UserRole { pub fn can_sign(&self) -> bool { matches!(*self, UserRole::BoardMember) } + + pub fn has_no_role(&self) -> bool { + matches!(*self, UserRole::None) + } } From 6ee6ce22ec88f8f0dbdb06bf50ed2b8ae66b4a8f Mon Sep 17 00:00:00 2001 From: Alin Cruceat Date: Mon, 18 Mar 2024 10:34:45 +0200 Subject: [PATCH 44/44] change condition for outdated users --- contracts/multisig/src/multisig_sign.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/multisig/src/multisig_sign.rs b/contracts/multisig/src/multisig_sign.rs index f77c6566..79ff099d 100644 --- a/contracts/multisig/src/multisig_sign.rs +++ b/contracts/multisig/src/multisig_sign.rs @@ -138,13 +138,13 @@ pub trait MultisigSignModule: let mut board_members_to_remove: ManagedVec = ManagedVec::new(); if outdated_board_members.is_empty() { for signer_id in self.action_signer_ids(action_id).iter() { - if self.user_id_to_role(signer_id).get().has_no_role() { + if !self.user_id_to_role(signer_id).get().can_sign() { board_members_to_remove.push(signer_id); } } } else { for signer_id in outdated_board_members.into_iter() { - if self.user_id_to_role(signer_id).get().has_no_role() { + if !self.user_id_to_role(signer_id).get().can_sign() { board_members_to_remove.push(signer_id); } }