diff --git a/CHANGELOG.md b/CHANGELOG.md index f0c630e04..2c420ba1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Multisig component (#1193) - `is_valid_p256_signature` utility function to `openzeppelin_account::utils::signature` (#1189) - `Secp256r1KeyPair` type and helpers to `openzeppelin_testing::signing` (#1189) - `all_tokens_of_owner` function to `ERC721EnumerableComponent` fetching all owner's tokens in a single call (#1196) @@ -27,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `DebugSecp256k1Point` replaced by a generic `DebugSecp256Point` - Apply underscore pattern to the internal functions of `ERC2981Component` to prevent collisions with new external functions (#1173) +- Move `Hash` and `PartialEq` impls of `Call` struct from `openzeppelin_governance::timelock::utils` to `openzeppelin_governance::utils` (#1193) ## 0.18.0 (2024-10-17) diff --git a/packages/account/src/account.cairo b/packages/account/src/account.cairo index 6582a6245..53957dbbe 100644 --- a/packages/account/src/account.cairo +++ b/packages/account/src/account.cairo @@ -16,10 +16,8 @@ pub mod AccountComponent { use openzeppelin_introspection::src5::SRC5Component::SRC5Impl; use openzeppelin_introspection::src5::SRC5Component; use starknet::account::Call; - use starknet::get_caller_address; - use starknet::get_contract_address; - use starknet::get_tx_info; use starknet::storage::{StoragePointerReadAccess, StoragePointerWriteAccess}; + use starknet::{get_caller_address, get_contract_address, get_tx_info}; #[storage] pub struct Storage { diff --git a/packages/governance/Scarb.toml b/packages/governance/Scarb.toml index 5ab2fc5d2..d32d87a15 100644 --- a/packages/governance/Scarb.toml +++ b/packages/governance/Scarb.toml @@ -49,6 +49,7 @@ build-external-contracts = [ "openzeppelin_test_common::mocks::timelock::TimelockControllerMock", "openzeppelin_test_common::mocks::timelock::MockContract", "openzeppelin_test_common::mocks::timelock::TimelockAttackerMock", + "openzeppelin_test_common::mocks::multisig::MultisigTargetMock", "openzeppelin_test_common::mocks::votes::ERC721VotesMock", "openzeppelin_test_common::mocks::votes::ERC20VotesMock" ] diff --git a/packages/governance/src/lib.cairo b/packages/governance/src/lib.cairo index 0b56faddc..041e1b05e 100644 --- a/packages/governance/src/lib.cairo +++ b/packages/governance/src/lib.cairo @@ -1,5 +1,8 @@ +pub mod multisig; + #[cfg(test)] mod tests; pub mod timelock; +pub mod utils; pub mod votes; diff --git a/packages/governance/src/multisig.cairo b/packages/governance/src/multisig.cairo new file mode 100644 index 000000000..8faab868a --- /dev/null +++ b/packages/governance/src/multisig.cairo @@ -0,0 +1,6 @@ +pub mod interface; +pub mod multisig; +pub mod storage_utils; + +pub use interface::{TransactionID, TransactionState}; +pub use multisig::MultisigComponent; diff --git a/packages/governance/src/multisig/interface.cairo b/packages/governance/src/multisig/interface.cairo new file mode 100644 index 000000000..ccc872a50 --- /dev/null +++ b/packages/governance/src/multisig/interface.cairo @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo v0.18.0 (governance/multisig/interface.cairo) + +use starknet::ContractAddress; +use starknet::account::Call; + +pub type TransactionID = felt252; + +/// Represents the possible states of a Multisig transaction. +#[derive(Copy, Drop, Serde, PartialEq, Debug)] +pub enum TransactionState { + NotFound, + Pending, + Confirmed, + Executed +} + +/// Interface of a contract providing the Multisig functionality. +#[starknet::interface] +pub trait IMultisig { + fn get_quorum(self: @TState) -> u32; + fn is_signer(self: @TState, signer: ContractAddress) -> bool; + fn get_signers(self: @TState) -> Span; + fn is_confirmed(self: @TState, id: TransactionID) -> bool; + fn is_confirmed_by(self: @TState, id: TransactionID, signer: ContractAddress) -> bool; + fn is_executed(self: @TState, id: TransactionID) -> bool; + fn get_submitted_block(self: @TState, id: TransactionID) -> u64; + fn get_transaction_state(self: @TState, id: TransactionID) -> TransactionState; + fn get_transaction_confirmations(self: @TState, id: TransactionID) -> u32; + fn hash_transaction( + self: @TState, + to: ContractAddress, + selector: felt252, + calldata: Span, + salt: felt252 + ) -> TransactionID; + fn hash_transaction_batch(self: @TState, calls: Span, salt: felt252) -> TransactionID; + + fn add_signers(ref self: TState, new_quorum: u32, signers_to_add: Span); + fn remove_signers(ref self: TState, new_quorum: u32, signers_to_remove: Span); + fn replace_signer( + ref self: TState, signer_to_remove: ContractAddress, signer_to_add: ContractAddress + ); + fn change_quorum(ref self: TState, new_quorum: u32); + fn submit_transaction( + ref self: TState, + to: ContractAddress, + selector: felt252, + calldata: Span, + salt: felt252, + ) -> TransactionID; + fn submit_transaction_batch( + ref self: TState, calls: Span, salt: felt252 + ) -> TransactionID; + fn confirm_transaction(ref self: TState, id: TransactionID); + fn revoke_confirmation(ref self: TState, id: TransactionID); + fn execute_transaction( + ref self: TState, + to: ContractAddress, + selector: felt252, + calldata: Span, + salt: felt252 + ); + fn execute_transaction_batch(ref self: TState, calls: Span, salt: felt252); +} diff --git a/packages/governance/src/multisig/multisig.cairo b/packages/governance/src/multisig/multisig.cairo new file mode 100644 index 000000000..4056a6189 --- /dev/null +++ b/packages/governance/src/multisig/multisig.cairo @@ -0,0 +1,645 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo v0.18.0 (governance/multisig/multisig.cairo) + +/// # Multisig Component +/// +/// Component that implements a multi-signature mechanism to enhance the security and governance +/// of smart contract transactions. It requires multiple registered signers to collectively approve +/// and execute transactions, ensuring that no single signer can perform critical actions +/// unilaterally. +/// +/// By default, this component is self-administered, meaning modifications to signers or quorum must +/// be performed by the contract itself through the multisig approval process. Only registered +/// signers can submit, confirm, revoke, or execute transactions. A common use case is to secure +/// important operations by requiring multiple approvals, such as in fund management or protocol +/// governance. +#[starknet::component] +pub mod MultisigComponent { + use core::hash::{HashStateTrait, HashStateExTrait}; + use core::num::traits::Zero; + use core::panic_with_felt252; + use core::pedersen::PedersenTrait; + use crate::multisig::interface::{IMultisig, TransactionID, TransactionState}; + use crate::multisig::storage_utils::{SignersInfo, SignersInfoStorePacking}; + use crate::multisig::storage_utils::{TxInfo, TxInfoStorePacking}; + use crate::utils::call_impls::{HashCallImpl, HashCallsImpl, CallPartialEq}; + use starknet::account::Call; + use starknet::storage::{Map, StorageMapReadAccess, StorageMapWriteAccess}; + use starknet::storage::{StoragePointerReadAccess, StoragePointerWriteAccess}; + use starknet::syscalls::call_contract_syscall; + use starknet::{ContractAddress, SyscallResultTrait}; + + #[storage] + pub struct Storage { + pub Multisig_signers_info: SignersInfo, + pub Multisig_is_signer: Map, + pub Multisig_signers_by_index: Map, + pub Multisig_signers_indices: Map, + pub Multisig_tx_info: Map, + pub Multisig_tx_confirmed_by: Map<(TransactionID, ContractAddress), bool> + } + + #[event] + #[derive(Drop, starknet::Event)] + pub enum Event { + SignerAdded: SignerAdded, + SignerRemoved: SignerRemoved, + QuorumUpdated: QuorumUpdated, + TransactionSubmitted: TransactionSubmitted, + TransactionConfirmed: TransactionConfirmed, + TransactionExecuted: TransactionExecuted, + ConfirmationRevoked: ConfirmationRevoked, + CallSalt: CallSalt + } + + /// Emitted when a new `signer` is added. + #[derive(Drop, starknet::Event)] + pub struct SignerAdded { + #[key] + pub signer: ContractAddress + } + + /// Emitted when a `signer` is removed. + #[derive(Drop, starknet::Event)] + pub struct SignerRemoved { + #[key] + pub signer: ContractAddress + } + + /// Emitted when the `quorum` value is updated. + #[derive(Drop, starknet::Event)] + pub struct QuorumUpdated { + pub old_quorum: u32, + pub new_quorum: u32 + } + + /// Emitted when a new transaction is submitted by a `signer`. + #[derive(Drop, starknet::Event)] + pub struct TransactionSubmitted { + #[key] + pub id: TransactionID, + #[key] + pub signer: ContractAddress + } + + /// Emitted when a transaction is confirmed by a `signer`. + #[derive(Drop, starknet::Event)] + pub struct TransactionConfirmed { + #[key] + pub id: TransactionID, + #[key] + pub signer: ContractAddress + } + + /// Emitted when a `signer` revokes his confirmation. + #[derive(Drop, starknet::Event)] + pub struct ConfirmationRevoked { + #[key] + pub id: TransactionID, + #[key] + pub signer: ContractAddress + } + + /// Emitted when a transaction is executed. + #[derive(Drop, starknet::Event)] + pub struct TransactionExecuted { + #[key] + pub id: TransactionID + } + + /// Emitted when a new transaction is submitted with non-zero salt. + #[derive(Drop, PartialEq, starknet::Event)] + pub struct CallSalt { + #[key] + pub id: felt252, + pub salt: felt252 + } + + pub mod Errors { + pub const UNAUTHORIZED: felt252 = 'Multisig: unauthorized'; + pub const NOT_A_SIGNER: felt252 = 'Multisig: not a signer'; + pub const ALREADY_A_SIGNER: felt252 = 'Multisig: already a signer'; + pub const ALREADY_CONFIRMED: felt252 = 'Multisig: already confirmed'; + pub const HAS_NOT_CONFIRMED: felt252 = 'Multisig: has not confirmed'; + pub const TX_ALREADY_EXISTS: felt252 = 'Multisig: tx already exists'; + pub const TX_NOT_FOUND: felt252 = 'Multisig: tx not found'; + pub const TX_NOT_CONFIRMED: felt252 = 'Multisig: tx not confirmed'; + pub const TX_ALREADY_EXECUTED: felt252 = 'Multisig: tx already executed'; + pub const ZERO_ADDRESS_SIGNER: felt252 = 'Multisig: zero address signer'; + pub const ZERO_QUORUM: felt252 = 'Multisig: quorum cannot be 0'; + pub const QUORUM_TOO_HIGH: felt252 = 'Multisig: quorum > signers'; + } + + #[embeddable_as(MultisigImpl)] + impl Multisig< + TContractState, +HasComponent, +Drop + > of IMultisig> { + /// Returns the current quorum value. The quorum is the minimum number + /// of confirmations required to approve a transaction. + fn get_quorum(self: @ComponentState) -> u32 { + self.Multisig_signers_info.read().quorum + } + + /// Returns whether the given `signer` is a registered signer. + /// Only registered signers can submit, confirm, or execute transactions. + fn is_signer(self: @ComponentState, signer: ContractAddress) -> bool { + self.Multisig_is_signer.read(signer) + } + + /// Returns the list of all current signers. + fn get_signers(self: @ComponentState) -> Span { + let mut result = array![]; + let signers_count = self.Multisig_signers_info.read().signers_count; + for i in 0..signers_count { + result.append(self.Multisig_signers_by_index.read(i)); + }; + result.span() + } + + /// Returns whether the transaction with the given `id` has been confirmed. + /// A confirmed transaction has received the required number of confirmations (quorum). + fn is_confirmed(self: @ComponentState, id: TransactionID) -> bool { + match self.resolve_tx_state(id) { + TransactionState::NotFound => false, + TransactionState::Pending => false, + TransactionState::Confirmed => true, + TransactionState::Executed => true + } + } + + /// Returns whether the transaction with the given `id` has been confirmed by the specified + /// `signer`. + /// + /// NOTE: Even if a `signer` is removed after confirming a transaction, this function will + /// still return true. However, their confirmation does not count toward the quorum when it + /// is checked. Therefore, this function should not be relied upon to verify legitimate + /// confirmations toward meeting the quorum. + fn is_confirmed_by( + self: @ComponentState, id: TransactionID, signer: ContractAddress + ) -> bool { + self.Multisig_tx_confirmed_by.read((id, signer)) + } + + /// Returns whether the transaction with the given `id` has been executed. + fn is_executed(self: @ComponentState, id: TransactionID) -> bool { + self.Multisig_tx_info.read(id).is_executed + } + + /// Returns the current state of the transaction with the given `id`. + /// + /// The possible states are: + /// - `NotFound`: The transaction does not exist. + /// - `Pending`: The transaction exists but hasn't reached the required confirmations. + /// - `Confirmed`: The transaction has reached the required confirmations but hasn't been + /// executed. + /// - `Executed`: The transaction has been executed. + fn get_transaction_state( + self: @ComponentState, id: TransactionID + ) -> TransactionState { + self.resolve_tx_state(id) + } + + /// Returns the number of confirmations from registered signers for the transaction with the + /// specified `id`. + fn get_transaction_confirmations( + self: @ComponentState, id: TransactionID + ) -> u32 { + let mut result = 0; + let all_signers = self.get_signers(); + for signer in all_signers { + if self.is_confirmed_by(id, *signer) { + result += 1; + } + }; + result + } + + /// Returns the block number when the transaction with the given `id` was submitted. + fn get_submitted_block(self: @ComponentState, id: TransactionID) -> u64 { + self.Multisig_tx_info.read(id).submitted_block + } + + /// Adds new signers and updates the quorum. + /// + /// Requirements: + /// + /// - The caller must be the contract itself. + /// - `new_quorum` must be less than or equal to the total number of signers after addition. + /// + /// Emits a `SignerAdded` event for each signer added. + fn add_signers( + ref self: ComponentState, + new_quorum: u32, + signers_to_add: Span + ) { + self.assert_only_self(); + self._add_signers(new_quorum, signers_to_add); + } + + /// Removes signers and updates the quorum. + /// + /// Requirements: + /// + /// - The caller must be the contract itself. + /// - `new_quorum` must be less than or equal to the total number of signers after removal. + /// + /// Emits a `SignerRemoved` event for each signer removed. + fn remove_signers( + ref self: ComponentState, + new_quorum: u32, + signers_to_remove: Span + ) { + self.assert_only_self(); + self._remove_signers(new_quorum, signers_to_remove); + } + + /// Replaces an existing signer with a new signer. + /// + /// Requirements: + /// + /// - The caller must be the contract itself. + /// - `signer_to_remove` must be an existing signer. + /// - `signer_to_add` must not be an existing signer. + /// + /// Emits a `SignerRemoved` event for the removed signer. + /// Emits a `SignerAdded` event for the new signer. + fn replace_signer( + ref self: ComponentState, + signer_to_remove: ContractAddress, + signer_to_add: ContractAddress + ) { + self.assert_only_self(); + self._replace_signer(signer_to_remove, signer_to_add); + } + + /// Updates the quorum value to `new_quorum` if it differs from the current quorum. + /// + /// Requirements: + /// + /// - The caller must be the contract itself. + /// - `new_quorum` must be non-zero. + /// - `new_quorum` must be less than or equal to the total number of signers. + /// + /// Emits a `QuorumUpdated` event if the quorum changes. + fn change_quorum(ref self: ComponentState, new_quorum: u32) { + self.assert_only_self(); + self._change_quorum(new_quorum); + } + + /// Submits a new transaction for confirmation. + /// + /// Requirements: + /// + /// - The caller must be a registered signer. + /// + /// Emits a `TransactionSubmitted` event. + /// If `salt` is not zero, emits a `CallSalt` event. + fn submit_transaction( + ref self: ComponentState, + to: ContractAddress, + selector: felt252, + calldata: Span, + salt: felt252 + ) -> TransactionID { + let call = Call { to, selector, calldata }; + self.submit_transaction_batch(array![call].span(), salt) + } + + /// Submits a new batch transaction for confirmation. + /// + /// Requirements: + /// + /// - The caller must be a registered signer. + /// + /// Emits a `TransactionSubmitted` event. + /// If `salt` is not zero, emits a `CallSalt` event. + fn submit_transaction_batch( + ref self: ComponentState, calls: Span, salt: felt252 + ) -> TransactionID { + let caller = starknet::get_caller_address(); + self.assert_one_of_signers(caller); + let id = self.hash_transaction_batch(calls, salt); + assert(self.get_submitted_block(id).is_zero(), Errors::TX_ALREADY_EXISTS); + + let tx_info = TxInfo { + is_executed: false, submitted_block: starknet::get_block_number() + }; + self.Multisig_tx_info.write(id, tx_info); + if salt.is_non_zero() { + self.emit(CallSalt { id, salt }); + } + self.emit(TransactionSubmitted { id, signer: caller }); + + id + } + + /// Confirms a transaction with the given `id`. + /// + /// Requirements: + /// + /// - The caller must be a registered signer. + /// - The transaction must exist and not be executed. + /// - The caller must not have already confirmed the transaction. + /// + /// Emits a `TransactionConfirmed` event. + fn confirm_transaction(ref self: ComponentState, id: TransactionID) { + let caller = starknet::get_caller_address(); + self.assert_one_of_signers(caller); + self.assert_tx_exists(id); + assert(!self.is_executed(id), Errors::TX_ALREADY_EXECUTED); + assert(!self.is_confirmed_by(id, caller), Errors::ALREADY_CONFIRMED); + + self.Multisig_tx_confirmed_by.write((id, caller), true); + self.emit(TransactionConfirmed { id, signer: caller }); + } + + /// Revokes a previous confirmation for a transaction with the given `id`. + /// + /// Requirements: + /// + /// - The transaction must exist and not be executed. + /// - The caller must have previously confirmed the transaction. + /// + /// Emits a `ConfirmationRevoked` event. + fn revoke_confirmation(ref self: ComponentState, id: TransactionID) { + let caller = starknet::get_caller_address(); + self.assert_tx_exists(id); + assert(!self.is_executed(id), Errors::TX_ALREADY_EXECUTED); + assert(self.is_confirmed_by(id, caller), Errors::HAS_NOT_CONFIRMED); + + self.Multisig_tx_confirmed_by.write((id, caller), false); + self.emit(ConfirmationRevoked { id, signer: caller }); + } + + /// Executes a confirmed transaction. + /// + /// Requirements: + /// + /// - The caller must be a registered signer. + /// - The transaction must be confirmed and not yet executed. + /// + /// Emits a `TransactionExecuted` event. + fn execute_transaction( + ref self: ComponentState, + to: ContractAddress, + selector: felt252, + calldata: Span, + salt: felt252 + ) { + let call = Call { to, selector, calldata }; + self.execute_transaction_batch(array![call].span(), salt) + } + + /// Executes a confirmed batch transaction. + /// + /// Requirements: + /// + /// - The caller must be a registered signer. + /// - The transaction must be confirmed and not yet executed. + /// + /// Emits a `TransactionExecuted` event. + fn execute_transaction_batch( + ref self: ComponentState, calls: Span, salt: felt252 + ) { + let id = self.hash_transaction_batch(calls, salt); + match self.resolve_tx_state(id) { + TransactionState::NotFound => panic_with_felt252(Errors::TX_NOT_FOUND), + TransactionState::Pending => panic_with_felt252(Errors::TX_NOT_CONFIRMED), + TransactionState::Executed => panic_with_felt252(Errors::TX_ALREADY_EXECUTED), + TransactionState::Confirmed => { + let caller = starknet::get_caller_address(); + self.assert_one_of_signers(caller); + let mut tx_info = self.Multisig_tx_info.read(id); + tx_info.is_executed = true; + self.Multisig_tx_info.write(id, tx_info); + for call in calls { + let Call { to, selector, calldata } = *call; + call_contract_syscall(to, selector, calldata).unwrap_syscall(); + }; + self.emit(TransactionExecuted { id }); + } + }; + } + + /// Returns the computed identifier of a transaction containing a single call. + fn hash_transaction( + self: @ComponentState, + to: ContractAddress, + selector: felt252, + calldata: Span, + salt: felt252 + ) -> TransactionID { + let call = Call { to, selector, calldata }; + self.hash_transaction_batch(array![call].span(), salt) + } + + /// Returns the computed identifier of a transaction containing a batch of calls. + fn hash_transaction_batch( + self: @ComponentState, calls: Span, salt: felt252 + ) -> TransactionID { + PedersenTrait::new(0).update_with(calls).update_with(salt).finalize() + } + } + + #[generate_trait] + pub impl InternalImpl< + TContractState, +HasComponent, +Drop + > of InternalTrait { + /// Initializes the Multisig component with the initial `quorum` and `signers`. + /// This function must be called during contract initialization to set up the initial state. + /// + /// Requirements: + /// + /// - `quorum` must be non-zero and less than or equal to the number of `signers`. + /// + /// Emits a `SignerAdded` event for each signer added. + /// Emits a `QuorumUpdated` event if the quorum changes. + fn initializer( + ref self: ComponentState, quorum: u32, signers: Span + ) { + self._add_signers(quorum, signers); + } + + /// Resolves and returns the current state of the transaction with the given `id`. + /// + /// The possible states are: + /// - `NotFound`: The transaction does not exist. + /// - `Pending`: The transaction exists but hasn't reached the required confirmations. + /// - `Confirmed`: The transaction has reached the required confirmations but hasn't been + /// executed. + /// - `Executed`: The transaction has been executed. + fn resolve_tx_state( + self: @ComponentState, id: TransactionID + ) -> TransactionState { + let TxInfo { is_executed, submitted_block } = self.Multisig_tx_info.read(id); + if submitted_block.is_zero() { + TransactionState::NotFound + } else if is_executed { + TransactionState::Executed + } else { + let is_confirmed = self.get_transaction_confirmations(id) >= self.get_quorum(); + if is_confirmed { + TransactionState::Confirmed + } else { + TransactionState::Pending + } + } + } + + /// Asserts that the `caller` is one of the registered signers. + /// + /// Requirements: + /// + /// - The `caller` must be a registered signer. + fn assert_one_of_signers(self: @ComponentState, caller: ContractAddress) { + assert(self.is_signer(caller), Errors::NOT_A_SIGNER); + } + + /// Asserts that a transaction with the given `id` exists. + /// + /// Requirements: + /// + /// - The transaction with `id` must have been submitted. + fn assert_tx_exists(self: @ComponentState, id: TransactionID) { + assert(self.get_submitted_block(id).is_non_zero(), Errors::TX_NOT_FOUND); + } + + /// Asserts that the caller is the contract itself. + /// + /// Requirements: + /// + /// - The caller must be the contract's own address. + fn assert_only_self(self: @ComponentState) { + let caller = starknet::get_caller_address(); + let self = starknet::get_contract_address(); + assert(caller == self, Errors::UNAUTHORIZED); + } + + /// Adds new signers and updates the quorum. + /// + /// Requirements: + /// + /// - Each signer address must be non-zero. + /// - `new_quorum` must be non-zero and less than or equal to the total number of signers + /// after addition. + /// + /// Emits a `SignerAdded` event for each new signer added. + /// Emits a `QuorumUpdated` event if the quorum changes. + fn _add_signers( + ref self: ComponentState, + new_quorum: u32, + signers_to_add: Span + ) { + if !signers_to_add.is_empty() { + let SignersInfo { quorum, mut signers_count } = self.Multisig_signers_info.read(); + for signer in signers_to_add { + let signer_to_add = *signer; + assert(signer_to_add.is_non_zero(), Errors::ZERO_ADDRESS_SIGNER); + if self.is_signer(signer_to_add) { + continue; + } + let index = signers_count; + self.Multisig_is_signer.write(signer_to_add, true); + self.Multisig_signers_by_index.write(index, signer_to_add); + self.Multisig_signers_indices.write(signer_to_add, index); + self.emit(SignerAdded { signer: signer_to_add }); + + signers_count += 1; + }; + self.Multisig_signers_info.write(SignersInfo { quorum, signers_count }); + } + self._change_quorum(new_quorum); + } + + /// Removes existing signers and updates the quorum. + /// + /// Requirements: + /// + /// - `new_quorum` must be non-zero and less than or equal to the total number of signers + /// after removal. + /// + /// Emits a `SignerRemoved` event for each signer removed. + /// Emits a `QuorumUpdated` event if the quorum changes. + fn _remove_signers( + ref self: ComponentState, + new_quorum: u32, + signers_to_remove: Span + ) { + if !signers_to_remove.is_empty() { + let SignersInfo { quorum, mut signers_count } = self.Multisig_signers_info.read(); + for signer in signers_to_remove { + let signer_to_remove = *signer; + if !self.is_signer(signer_to_remove) { + continue; + } + let last_index = signers_count - 1; + let index = self.Multisig_signers_indices.read(signer_to_remove); + if index != last_index { + // Swap signer to remove with the last signer + let last_signer = self.Multisig_signers_by_index.read(last_index); + self.Multisig_signers_indices.write(last_signer, index); + self.Multisig_signers_by_index.write(index, last_signer); + } + // Remove the last signer + self.Multisig_is_signer.write(signer_to_remove, false); + self.Multisig_signers_by_index.write(last_index, Zero::zero()); + self.Multisig_signers_indices.write(signer_to_remove, 0); + self.emit(SignerRemoved { signer: signer_to_remove }); + + signers_count -= 1; + }; + self.Multisig_signers_info.write(SignersInfo { quorum, signers_count }); + } + self._change_quorum(new_quorum); + } + + /// Replaces an existing signer with a new signer. + /// + /// Requirements: + /// + /// - `signer_to_remove` must be an existing signer. + /// - `signer_to_add` must not be an existing signer. + /// - `signer_to_add` must be a non-zero address. + /// + /// Emits a `SignerRemoved` event for the removed signer. + /// Emits a `SignerAdded` event for the new signer. + fn _replace_signer( + ref self: ComponentState, + signer_to_remove: ContractAddress, + signer_to_add: ContractAddress + ) { + assert(signer_to_add.is_non_zero(), Errors::ZERO_ADDRESS_SIGNER); + assert(!self.is_signer(signer_to_add), Errors::ALREADY_A_SIGNER); + assert(self.is_signer(signer_to_remove), Errors::NOT_A_SIGNER); + + self.Multisig_is_signer.write(signer_to_remove, false); + self.Multisig_is_signer.write(signer_to_add, true); + let index = self.Multisig_signers_indices.read(signer_to_remove); + self.Multisig_signers_by_index.write(index, signer_to_add); + self.Multisig_signers_indices.write(signer_to_add, index); + self.Multisig_signers_indices.write(signer_to_remove, 0); + self.emit(SignerRemoved { signer: signer_to_remove }); + self.emit(SignerAdded { signer: signer_to_add }); + } + + /// Updates the quorum value to `new_quorum` if it differs from the current quorum. + /// + /// Requirements: + /// + /// - `new_quorum` must be non-zero. + /// - `new_quorum` must be less than or equal to the total number of signers. + /// + /// Emits a `QuorumUpdated` event if the quorum changes. + fn _change_quorum(ref self: ComponentState, new_quorum: u32) { + let SignersInfo { quorum: old_quorum, signers_count } = self + .Multisig_signers_info + .read(); + if new_quorum != old_quorum { + assert(new_quorum.is_non_zero(), Errors::ZERO_QUORUM); + assert(new_quorum <= signers_count, Errors::QUORUM_TOO_HIGH); + self.Multisig_signers_info.write(SignersInfo { quorum: new_quorum, signers_count }); + self.emit(QuorumUpdated { old_quorum, new_quorum }); + } + } + } +} diff --git a/packages/governance/src/multisig/storage_utils.cairo b/packages/governance/src/multisig/storage_utils.cairo new file mode 100644 index 000000000..09ca71f35 --- /dev/null +++ b/packages/governance/src/multisig/storage_utils.cairo @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo v0.18.0 (governance/multisig/storage_utils.cairo) + +use core::integer::u128_safe_divmod; +use starknet::storage_access::StorePacking; + +const _2_POW_32: NonZero = 0xffffffff; + +/// Helper struct for `MultisigComponent` that optimizes how transaction-related information +/// is stored, including the transaction's execution status and the block it was submitted in. +#[derive(Drop)] +pub struct TxInfo { + pub is_executed: bool, + pub submitted_block: u64 +} + +/// Packs a `TxInfo` entity into a `u128` value. +/// +/// The packing is done as follows: +/// - The boolean `is_executed` is stored as a single bit at the highest bit position (index 127). +/// - The `submitted_block` value occupies 64 bits in the range [63..126]. +pub impl TxInfoStorePacking of StorePacking { + fn pack(value: TxInfo) -> u128 { + let TxInfo { is_executed, submitted_block } = value; + let is_executed_value = if is_executed { + 1 + } else { + 0 + }; + submitted_block.into() * 2 + is_executed_value + } + + fn unpack(value: u128) -> TxInfo { + let (submitted_block, is_executed_value) = u128_safe_divmod(value, 2); + let is_executed = is_executed_value == 1; + TxInfo { is_executed, submitted_block: submitted_block.try_into().unwrap() } + } +} + +/// Helper struct for `MultisigComponent` that optimizes how the quorum +/// value and the total number of signers are stored. +#[derive(Drop)] +pub struct SignersInfo { + pub quorum: u32, + pub signers_count: u32 +} + +/// Packs a `SignersInfo` entity into a `u128` value. +/// +/// The packing is done as follows: +/// - `quorum` value occupies 32 bits in bit range [64..95]. +/// - `signers_count` value occupies the highest 32 bits in bit range [96..127]. +pub impl SignersInfoStorePacking of StorePacking { + fn pack(value: SignersInfo) -> u128 { + let SignersInfo { quorum, signers_count } = value; + quorum.into() * _2_POW_32.into() + signers_count.into() + } + + fn unpack(value: u128) -> SignersInfo { + let (quorum, signers_count) = u128_safe_divmod(value, _2_POW_32); + SignersInfo { + quorum: quorum.try_into().unwrap(), signers_count: signers_count.try_into().unwrap() + } + } +} diff --git a/packages/governance/src/tests.cairo b/packages/governance/src/tests.cairo index c71f711c4..6a51994ef 100644 --- a/packages/governance/src/tests.cairo +++ b/packages/governance/src/tests.cairo @@ -1,3 +1,4 @@ +mod test_multisig; mod test_timelock; mod test_utils; mod test_votes; diff --git a/packages/governance/src/tests/test_multisig.cairo b/packages/governance/src/tests/test_multisig.cairo new file mode 100644 index 000000000..cfdee1788 --- /dev/null +++ b/packages/governance/src/tests/test_multisig.cairo @@ -0,0 +1,1588 @@ +use core::num::traits::Zero; +use crate::multisig::MultisigComponent::{ConfirmationRevoked, TransactionExecuted}; +use crate::multisig::MultisigComponent::{MultisigImpl, InternalImpl, Event}; +use crate::multisig::MultisigComponent::{SignerAdded, SignerRemoved, QuorumUpdated, CallSalt}; +use crate::multisig::MultisigComponent::{TransactionSubmitted, TransactionConfirmed}; +use crate::multisig::{MultisigComponent, TransactionID, TransactionState}; +use openzeppelin_test_common::mocks::multisig::IMultisigTargetMockDispatcherTrait; +use openzeppelin_test_common::mocks::multisig::{MultisigWalletMock, IMultisigTargetMockDispatcher}; +use openzeppelin_testing as utils; +use openzeppelin_testing::constants::{ZERO, OTHER, ALICE, BOB, CHARLIE, SALT, BLOCK_NUMBER}; +use openzeppelin_testing::events::EventSpyExt; +use snforge_std::{EventSpy, spy_events, test_address}; +use snforge_std::{start_cheat_caller_address, start_cheat_block_number_global}; +use starknet::account::Call; +use starknet::{ContractAddress, contract_address_const}; + +// +// Setup +// + +type ComponentState = MultisigComponent::ComponentState; + +fn COMPONENT_STATE() -> ComponentState { + MultisigComponent::component_state_for_testing() +} + +fn DEFAULT_DATA() -> (u32, Span) { + let signers = array![ALICE(), BOB(), CHARLIE()]; + let quorum = signers.len() - 1; + (quorum, signers.span()) +} + +fn MOCK_ADDRESS() -> ContractAddress { + contract_address_const::<'MOCK_ADDRESS'>() +} + +fn setup_component(quorum: u32, signers: Span) -> ComponentState { + start_cheat_block_number_global(BLOCK_NUMBER); + let mut state = COMPONENT_STATE(); + state.initializer(quorum, signers); + state +} + +fn deploy_mock() -> IMultisigTargetMockDispatcher { + let contract_address = MOCK_ADDRESS(); + utils::declare_and_deploy_at("MultisigTargetMock", contract_address, array![]); + IMultisigTargetMockDispatcher { contract_address } +} + +// +// Submit tx +// + +#[test] +fn test_submit_tx() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let contract_address = test_address(); + + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt = 0; + let expected_id = state.hash_transaction(to, selector, calldata, salt); + assert_tx_state(expected_id, TransactionState::NotFound); + + let signer = ALICE(); + start_cheat_caller_address(contract_address, signer); + + let id = state.submit_transaction(to, selector, calldata, salt); + assert_eq!(id, expected_id); + assert_eq!(state.get_submitted_block(id), BLOCK_NUMBER); + assert_tx_state(id, TransactionState::Pending); + spy.assert_only_event_tx_submitted(contract_address, id, signer); +} + +#[test] +fn test_submit_tx_with_salt() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let contract_address = test_address(); + + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt = SALT; + let expected_id = state.hash_transaction(to, selector, calldata, salt); + assert_tx_state(expected_id, TransactionState::NotFound); + + let signer = ALICE(); + start_cheat_caller_address(contract_address, signer); + + let id = state.submit_transaction(to, selector, calldata, salt); + assert_eq!(id, expected_id); + assert_eq!(state.get_submitted_block(id), BLOCK_NUMBER); + assert_tx_state(id, TransactionState::Pending); + spy.assert_event_call_salt(contract_address, id, salt); + spy.assert_event_tx_submitted(contract_address, id, signer); +} + +#[test] +fn test_submit_same_tx_again_different_salt() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let contract_address = test_address(); + + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt_1 = 0; + let expected_id_1 = state.hash_transaction(to, selector, calldata, salt_1); + let salt_2 = SALT; + let expected_id_2 = state.hash_transaction(to, selector, calldata, salt_2); + assert!(expected_id_1 != expected_id_2); + + let signer = ALICE(); + start_cheat_caller_address(contract_address, signer); + + let id_1 = state.submit_transaction(to, selector, calldata, salt_1); + assert_eq!(id_1, expected_id_1); + assert_tx_state(id_1, TransactionState::Pending); + spy.assert_only_event_tx_submitted(contract_address, id_1, signer); + + let id_2 = state.submit_transaction(to, selector, calldata, salt_2); + assert_eq!(id_2, expected_id_2); + assert_tx_state(id_2, TransactionState::Pending); + spy.assert_event_call_salt(contract_address, id_2, salt_2); + spy.assert_only_event_tx_submitted(contract_address, id_2, signer); +} + +#[test] +fn test_submit_tx_batch() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let contract_address = test_address(); + + let calls = array![ + build_call(MockCall::AddNumber(42)), + build_call(MockCall::AddNumber(18)), + build_call(MockCall::AddNumber(40)) + ] + .span(); + let salt = 0; + let expected_id = state.hash_transaction_batch(calls, salt); + assert_tx_state(expected_id, TransactionState::NotFound); + + let signer = ALICE(); + start_cheat_caller_address(contract_address, signer); + + let id = state.submit_transaction_batch(calls, salt); + assert_eq!(id, expected_id); + assert_eq!(state.get_submitted_block(id), BLOCK_NUMBER); + assert_tx_state(id, TransactionState::Pending); + spy.assert_only_event_tx_submitted(contract_address, id, signer); +} + +#[test] +fn test_submit_tx_batch_with_salt() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let contract_address = test_address(); + + let calls = array![ + build_call(MockCall::AddNumber(42)), + build_call(MockCall::AddNumber(18)), + build_call(MockCall::AddNumber(40)) + ] + .span(); + let salt = SALT; + let expected_id = state.hash_transaction_batch(calls, salt); + assert_tx_state(expected_id, TransactionState::NotFound); + + let signer = ALICE(); + start_cheat_caller_address(contract_address, signer); + + let id = state.submit_transaction_batch(calls, salt); + assert_eq!(id, expected_id); + assert_eq!(state.get_submitted_block(id), BLOCK_NUMBER); + assert_tx_state(id, TransactionState::Pending); + spy.assert_event_call_salt(contract_address, id, salt); + spy.assert_event_tx_submitted(contract_address, id, signer); +} + +#[test] +fn test_submit_same_tx_batch_different_salt() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let contract_address = test_address(); + + let calls = array![ + build_call(MockCall::AddNumber(42)), + build_call(MockCall::AddNumber(18)), + build_call(MockCall::AddNumber(40)) + ] + .span(); + let salt_1 = 0; + let expected_id_1 = state.hash_transaction_batch(calls, salt_1); + let salt_2 = SALT; + let expected_id_2 = state.hash_transaction_batch(calls, salt_2); + assert!(expected_id_1 != expected_id_2); + + let signer = ALICE(); + start_cheat_caller_address(contract_address, signer); + + let id_1 = state.submit_transaction_batch(calls, salt_1); + assert_eq!(id_1, expected_id_1); + assert_tx_state(id_1, TransactionState::Pending); + spy.assert_only_event_tx_submitted(contract_address, id_1, signer); + + let id_2 = state.submit_transaction_batch(calls, salt_2); + assert_eq!(id_2, expected_id_2); + assert_tx_state(id_2, TransactionState::Pending); + spy.assert_event_call_salt(contract_address, id_2, salt_2); + spy.assert_event_tx_submitted(contract_address, id_2, signer); +} + +#[test] +#[should_panic(expected: 'Multisig: not a signer')] +fn test_cannot_submit_tx_unauthorized() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let signer = OTHER(); + start_cheat_caller_address(test_address(), signer); + state.submit_transaction(to, selector, calldata, 0); +} + +#[test] +#[should_panic(expected: 'Multisig: not a signer')] +fn test_cannot_submit_tx_batch_unauthorized() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + + let calls = array![ + build_call(MockCall::AddNumber(42)), + build_call(MockCall::AddNumber(18)), + build_call(MockCall::AddNumber(40)) + ] + .span(); + let signer = OTHER(); + start_cheat_caller_address(test_address(), signer); + state.submit_transaction_batch(calls, 0); +} + +#[test] +#[should_panic(expected: 'Multisig: tx already exists')] +fn test_cannot_submit_tx_twice() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let signer = ALICE(); + start_cheat_caller_address(test_address(), signer); + state.submit_transaction(to, selector, calldata, 0); + state.submit_transaction(to, selector, calldata, 0); +} + +#[test] +#[should_panic(expected: 'Multisig: tx already exists')] +fn test_cannot_submit_tx_batch_twice() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + + let calls = array![ + build_call(MockCall::AddNumber(42)), + build_call(MockCall::AddNumber(18)), + build_call(MockCall::AddNumber(40)) + ] + .span(); + let signer = ALICE(); + start_cheat_caller_address(test_address(), signer); + state.submit_transaction_batch(calls, 0); + state.submit_transaction_batch(calls, 0); +} + +// +// Confirm tx +// + +#[test] +fn test_confirm_tx() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let contract_address = test_address(); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + + // Submit by Alice + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, 0); + + // Confirm by Bob + spy.drop_all_events(); + start_cheat_caller_address(contract_address, BOB()); + assert_eq!(state.is_confirmed_by(id, BOB()), false); + state.confirm_transaction(id); + assert_eq!(state.is_confirmed_by(id, BOB()), true); + assert_tx_state(id, TransactionState::Pending); + assert_eq!(state.get_transaction_confirmations(id), 1); + spy.assert_only_event_tx_confirmed(contract_address, id, BOB()); + + // Confirm by Charlie + start_cheat_caller_address(contract_address, CHARLIE()); + assert_eq!(state.is_confirmed_by(id, CHARLIE()), false); + state.confirm_transaction(id); + assert_eq!(state.is_confirmed_by(id, CHARLIE()), true); + assert_tx_state(id, TransactionState::Confirmed); + assert_eq!(state.get_transaction_confirmations(id), 2); + spy.assert_only_event_tx_confirmed(contract_address, id, CHARLIE()); +} + +#[test] +fn test_confirmed_status_changed_when_quorum_increased() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + + // Submit by Alice + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, 0); + + // Confirm by Bob + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + + // Confirm by Charlie + start_cheat_caller_address(contract_address, CHARLIE()); + state.confirm_transaction(id); + + assert_tx_state(id, TransactionState::Confirmed); + state._change_quorum(quorum + 1); + assert_tx_state(id, TransactionState::Pending); +} + +#[test] +fn test_pending_status_changed_when_quorum_reduced() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + + // Submit by Alice + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, 0); + + // Confirm by Bob + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + + assert_tx_state(id, TransactionState::Pending); + state._change_quorum(quorum - 1); + assert_tx_state(id, TransactionState::Confirmed); +} + +#[test] +fn test_confirm_tx_batch() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let contract_address = test_address(); + + let calls = array![ + build_call(MockCall::AddNumber(42)), + build_call(MockCall::AddNumber(18)), + build_call(MockCall::AddNumber(40)) + ] + .span(); + + // Submit by Alice + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction_batch(calls, 0); + assert_tx_state(id, TransactionState::Pending); + assert_eq!(state.get_transaction_confirmations(id), 0); + spy.drop_all_events(); + + // Confirm by Bob + start_cheat_caller_address(contract_address, BOB()); + assert_eq!(state.is_confirmed_by(id, BOB()), false); + state.confirm_transaction(id); + assert_eq!(state.is_confirmed_by(id, BOB()), true); + assert_tx_state(id, TransactionState::Pending); + assert_eq!(state.get_transaction_confirmations(id), 1); + spy.assert_only_event_tx_confirmed(contract_address, id, BOB()); + + // Confirm by Charlie + start_cheat_caller_address(contract_address, CHARLIE()); + assert_eq!(state.is_confirmed_by(id, CHARLIE()), false); + state.confirm_transaction(id); + assert_eq!(state.is_confirmed_by(id, CHARLIE()), true); + assert_tx_state(id, TransactionState::Confirmed); + assert_eq!(state.get_transaction_confirmations(id), 2); + spy.assert_only_event_tx_confirmed(contract_address, id, CHARLIE()); +} + +#[test] +#[should_panic(expected: 'Multisig: tx not found')] +fn test_cannot_confirm_nonexistent_tx() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let id = state.hash_transaction(to, selector, calldata, 0); + + start_cheat_caller_address(contract_address, ALICE()); + state.confirm_transaction(id); +} + +#[test] +#[should_panic(expected: 'Multisig: not a signer')] +fn test_cannot_confirm_tx_unauthorized() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let id = state.hash_transaction(to, selector, calldata, 0); + start_cheat_caller_address(contract_address, ALICE()); + state.submit_transaction(to, selector, calldata, 0); + + start_cheat_caller_address(contract_address, OTHER()); + state.confirm_transaction(id); +} + +#[test] +#[should_panic(expected: 'Multisig: already confirmed')] +fn test_cannot_confirm_tx_twice() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + + // Submit by Alice + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let id = state.hash_transaction(to, selector, calldata, 0); + start_cheat_caller_address(contract_address, ALICE()); + state.submit_transaction(to, selector, calldata, 0); + + // Confirm by Bob + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + assert_eq!(state.is_confirmed_by(id, BOB()), true); + assert_eq!(state.get_transaction_confirmations(id), 1); + + // Try to confirm again by Bob + state.confirm_transaction(id); +} + +// +// Revoke confirmation +// + +#[test] +fn test_revoke_confirmation() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let contract_address = test_address(); + + // Submit by Alice + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, 0); + + // Confirm by Bob + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + + // Confirm by Charlie + start_cheat_caller_address(contract_address, CHARLIE()); + state.confirm_transaction(id); + + // Revoke confirmation by Charlie + spy.drop_all_events(); + assert_tx_state(id, TransactionState::Confirmed); + assert_eq!(state.is_confirmed_by(id, CHARLIE()), true); + assert_eq!(state.get_transaction_confirmations(id), 2); + state.revoke_confirmation(id); + assert_tx_state(id, TransactionState::Pending); + assert_eq!(state.is_confirmed_by(id, CHARLIE()), false); + assert_eq!(state.get_transaction_confirmations(id), 1); + spy.assert_only_event_confirmation_revoked(contract_address, id, CHARLIE()); +} + +#[test] +fn test_tx_not_confirmed_after_signer_removal() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + + // Submit & confirm by Alice + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, 0); + state.confirm_transaction(id); + + // Confirm by Bob + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + + // Check state before removal + assert_tx_state(id, TransactionState::Confirmed); + assert_eq!(state.is_confirmed_by(id, BOB()), true); + assert_eq!(state.get_transaction_confirmations(id), 2); + + // Remove Bob from signers + start_cheat_caller_address(contract_address, contract_address); + state.remove_signers(quorum, array![BOB()].span()); + + // Check state after removal + assert_tx_state(id, TransactionState::Pending); + assert_eq!(state.is_confirmed_by(id, BOB()), true); + assert_eq!(state.get_transaction_confirmations(id), 1); +} + +#[test] +fn test_can_revoke_confirmation_after_being_removed() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + + // Submit & confirm by Alice + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, 0); + state.confirm_transaction(id); + + // Confirm by Bob + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + + // Remove Bob from signers + start_cheat_caller_address(contract_address, contract_address); + state.remove_signers(quorum, array![BOB()].span()); + + // Check state before revocation + assert_tx_state(id, TransactionState::Pending); + assert_eq!(state.is_confirmed_by(id, BOB()), true); + assert_eq!(state.get_transaction_confirmations(id), 1); + + // Revoke confirmation by Bob + start_cheat_caller_address(contract_address, BOB()); + state.revoke_confirmation(id); + + // Check state after revocation + assert_tx_state(id, TransactionState::Pending); + assert_eq!(state.is_confirmed_by(id, BOB()), false); + assert_eq!(state.get_transaction_confirmations(id), 1); +} + +#[test] +#[should_panic(expected: 'Multisig: has not confirmed')] +fn test_cannot_revoke_confirmation_has_not_confirmed() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + + // Submit by Alice + start_cheat_caller_address(test_address(), ALICE()); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let id = state.submit_transaction(to, selector, calldata, 0); + + // Revoke confirmation by Bob + start_cheat_caller_address(test_address(), BOB()); + state.revoke_confirmation(id); +} + +#[test] +#[should_panic(expected: 'Multisig: tx not found')] +fn test_cannot_revoke_confirmation_nonexistent_tx() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let id = state.hash_transaction(to, selector, calldata, 0); + state.revoke_confirmation(id); +} + +// +// Execute tx +// + +#[test] +fn test_execute_tx() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let mut spy = spy_events(); + let mock = deploy_mock(); + let contract_address = test_address(); + + // Submit + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt = 0; + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, salt); + + // Confirm + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + start_cheat_caller_address(contract_address, CHARLIE()); + state.confirm_transaction(id); + + // Check state before + assert_eq!(mock.get_current_sum(), 0); + assert_tx_state(id, TransactionState::Confirmed); + + // Execute + spy.drop_all_events(); + start_cheat_caller_address(contract_address, ALICE()); + state.execute_transaction(to, selector, calldata, salt); + + // Check state after + assert_eq!(mock.get_current_sum(), 42); + assert_tx_state(id, TransactionState::Executed); + spy.assert_only_event_tx_executed(contract_address, id); +} + +#[test] +fn test_execute_tx_batch() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + let mut spy = spy_events(); + let mock = deploy_mock(); + let calls = array![ + build_call(MockCall::AddNumber(42)), + build_call(MockCall::AddNumber(18)), + build_call(MockCall::AddNumber(40)) + ] + .span(); + let salt = 0; + + // Submit + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction_batch(calls, salt); + + // Confirm + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + start_cheat_caller_address(contract_address, CHARLIE()); + state.confirm_transaction(id); + + // Check state before + assert_eq!(mock.get_current_sum(), 0); + assert_tx_state(id, TransactionState::Confirmed); + + // Execute + spy.drop_all_events(); + start_cheat_caller_address(contract_address, ALICE()); + state.execute_transaction_batch(calls, salt); + + // Check state after + assert_eq!(mock.get_current_sum(), 100); + assert_tx_state(id, TransactionState::Executed); + spy.assert_only_event_tx_executed(contract_address, id); +} + +#[test] +#[should_panic(expected: 'Multisig: tx not found')] +fn test_cannot_execute_not_submitted_tx() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt = 0; + + // Try to execute + start_cheat_caller_address(contract_address, ALICE()); + state.execute_transaction(to, selector, calldata, salt); +} + +#[test] +#[should_panic(expected: 'Multisig: not a signer')] +fn test_cannot_execute_unauthorized() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt = 0; + + // Submit + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, salt); + + // Confirm + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + start_cheat_caller_address(contract_address, CHARLIE()); + state.confirm_transaction(id); + + // Try to execute + start_cheat_caller_address(contract_address, OTHER()); + state.execute_transaction(to, selector, calldata, salt); +} + +#[test] +#[should_panic(expected: 'Multisig: not a signer')] +fn test_cannot_execute_batch_unauthorized() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + let calls = array![ + build_call(MockCall::AddNumber(42)), + build_call(MockCall::AddNumber(18)), + build_call(MockCall::AddNumber(40)) + ] + .span(); + let salt = 0; + + // Submit + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction_batch(calls, salt); + + // Confirm + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + start_cheat_caller_address(contract_address, CHARLIE()); + state.confirm_transaction(id); + + // Try to execute + start_cheat_caller_address(contract_address, OTHER()); + state.execute_transaction_batch(calls, salt); +} + +#[test] +#[should_panic(expected: 'Multisig: tx not confirmed')] +fn test_cannot_execute_not_confirmed() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt = 0; + + // Submit + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, salt); + + // Confirm once + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + + // Execute + state.execute_transaction(to, selector, calldata, salt); +} + +#[test] +#[should_panic(expected: 'Multisig: tx not confirmed')] +fn test_cannot_execute_batch_not_confirmed() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt = 0; + + // Submit + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, salt); + + // Confirm once + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + + // Execute + state.execute_transaction(to, selector, calldata, salt); +} + +#[test] +#[should_panic(expected: 'Multisig: tx already executed')] +fn test_cannot_execute_twice() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt = 0; + deploy_mock(); + + // Submit + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, salt); + + // Confirm + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + start_cheat_caller_address(contract_address, CHARLIE()); + state.confirm_transaction(id); + + // Execute 1st time + state.execute_transaction(to, selector, calldata, salt); + + // Try to execute 2nd time + state.execute_transaction(to, selector, calldata, salt); +} + +#[test] +#[should_panic(expected: 'Multisig: tx already executed')] +fn test_cannot_execute_batch_twice() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + deploy_mock(); + + // Submit + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + let salt = 0; + start_cheat_caller_address(contract_address, ALICE()); + let id = state.submit_transaction(to, selector, calldata, salt); + + // Confirm + start_cheat_caller_address(contract_address, BOB()); + state.confirm_transaction(id); + start_cheat_caller_address(contract_address, CHARLIE()); + state.confirm_transaction(id); + + // Execute 1st time + state.execute_transaction(to, selector, calldata, salt); + + // Try to execute 2nd time + state.execute_transaction(to, selector, calldata, salt); +} + +// +// hash_transaction +// + +#[test] +fn test_tx_hash_depends_on_salt() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(42)); + start_cheat_caller_address(test_address(), ALICE()); + + let mut salt = 0; + while salt != 10 { + let id_from_hash = state.hash_transaction(to, selector, calldata, salt); + let id = state.submit_transaction(to, selector, calldata, salt); + assert_eq!(id_from_hash, id); + salt += 1; + }; +} + +#[test] +fn test_tx_batch_hash_depends_on_salt() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let calls = array![ + build_call(MockCall::AddNumber(42)), + build_call(MockCall::AddNumber(18)), + build_call(MockCall::AddNumber(40)) + ] + .span(); + start_cheat_caller_address(test_address(), ALICE()); + + let mut salt = 0; + while salt != 10 { + let id_from_hash = state.hash_transaction_batch(calls, salt); + let id = state.submit_transaction_batch(calls, salt); + assert_eq!(id_from_hash, id); + salt += 1; + }; +} + +#[test] +fn test_tx_hash_depends_on_calldata() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + start_cheat_caller_address(test_address(), ALICE()); + + let mut num = 0; + while num != 10 { + let Call { to, selector, calldata } = build_call(MockCall::AddNumber(num)); + let id_from_hash = state.hash_transaction(to, selector, calldata, SALT); + let id = state.submit_transaction(to, selector, calldata, SALT); + assert_eq!(id_from_hash, id); + num += 1; + }; +} + +#[test] +fn test_tx_hash_depends_on_selector() { + let (quorum, signers) = DEFAULT_DATA(); + let state = setup_component(quorum, signers); + + let to = MOCK_ADDRESS(); + let empty_calldata = array![].span(); + let id_1 = state.hash_transaction(to, selector!("selector_1"), empty_calldata, SALT); + let id_2 = state.hash_transaction(to, selector!("selector_2"), empty_calldata, SALT); + let id_3 = state.hash_transaction(to, selector!("selector_3"), empty_calldata, SALT); + assert!(id_1 != id_2); + assert!(id_2 != id_3); + assert!(id_1 != id_3); +} + +#[test] +fn test_tx_hash_depends_on_to_address() { + let (quorum, signers) = DEFAULT_DATA(); + let state = setup_component(quorum, signers); + + let Call { to: _, selector, calldata } = build_call(MockCall::AddNumber(42)); + let id_1 = state.hash_transaction(ALICE(), selector, calldata, SALT); + let id_2 = state.hash_transaction(BOB(), selector, calldata, SALT); + let id_3 = state.hash_transaction(CHARLIE(), selector, calldata, SALT); + assert!(id_1 != id_2); + assert!(id_2 != id_3); + assert!(id_1 != id_3); +} + +// +// add_signers +// + +#[test] +fn test_add_single_signer() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice].span()); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Add Bob as signer + state.add_signers(quorum, array![bob].span()); + assert_signers_list(array![alice, bob].span()); + spy.assert_only_event_signer_added(contract_address, bob); + + // Add Charlie as signer + state.add_signers(quorum, array![charlie].span()); + assert_signers_list(array![alice, bob, charlie].span()); + spy.assert_only_event_signer_added(contract_address, charlie); +} + +#[test] +fn test_add_multiple_signers() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice].span()); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Add Bob and Charlie as signers + state.add_signers(quorum, array![bob, charlie].span()); + assert_signers_list(array![alice, bob, charlie].span()); + spy.assert_event_signer_added(contract_address, bob); + spy.assert_only_event_signer_added(contract_address, charlie); +} + +#[test] +fn test_add_remove_add() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Add Bob and Charlie as signers + state.add_signers(quorum, array![bob, charlie].span()); + assert_signers_list(array![alice, bob, charlie].span()); + + // Remove Alice + state.remove_signers(quorum, array![alice].span()); + assert_signers_list(array![charlie, bob].span()); + + // Remove Charlie + state.remove_signers(quorum, array![charlie].span()); + assert_signers_list(array![bob].span()); + + // Add Alice + state.add_signers(quorum, array![alice].span()); + assert_signers_list(array![bob, alice].span()); + + // Add Charlie + state.add_signers(quorum, array![charlie].span()); + assert_signers_list(array![bob, alice, charlie].span()); +} + +#[test] +fn test_signers_ignored_if_added_again() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Add Bob as signer + state.add_signers(quorum, array![bob].span()); + assert_signers_list(array![alice, bob].span()); + + // Add Alice and Bob again and Charlie as a new signer + let mut spy = spy_events(); + state.add_signers(quorum, array![alice, bob, charlie].span()); + assert_signers_list(array![alice, bob, charlie].span()); + spy.assert_only_event_signer_added(contract_address, charlie); +} + +#[test] +fn test_add_signers_does_nothing_if_signers_empty() { + let (quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(quorum, signers); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Call `add_signers` with an empty list + let mut spy = spy_events(); + let empty_list = array![].span(); + state.add_signers(quorum, empty_list); + spy.assert_no_events_left_from(contract_address); +} + +#[test] +#[should_panic(expected: 'Multisig: unauthorized')] +fn test_cannot_add_when_not_multisig_itself() { + let quorum = 1; + let (alice, bob) = (ALICE(), BOB()); + let mut state = setup_component(quorum, array![alice].span()); + + // Try to add signer + start_cheat_caller_address(test_address(), OTHER()); + state.add_signers(quorum, array![bob].span()); +} + +#[test] +#[should_panic(expected: 'Multisig: zero address signer')] +fn test_cannot_add_zero_address_as_signer() { + let quorum = 1; + let mut state = setup_component(quorum, array![ALICE()].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to add zero address as signer + state.add_signers(quorum, array![ZERO()].span()); +} + +#[test] +#[should_panic(expected: 'Multisig: quorum cannot be 0')] +fn test_cannot_add_with_zero_quorum() { + let quorum = 1; + let (alice, bob) = (ALICE(), BOB()); + let mut state = setup_component(quorum, array![alice].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to add with 0 quorum value + state.add_signers(0, array![bob].span()); +} + +#[test] +#[should_panic(expected: 'Multisig: quorum > signers')] +fn test_cannot_add_with_quorum_too_high() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to add with quorum value > signers count + state.add_signers(4, array![bob, charlie].span()); +} + +// +// remove_signers +// + +#[test] +fn test_remove_single_signer() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice, bob, charlie].span()); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Remove Alice from signers + assert_signers_list(array![alice, bob, charlie].span()); + state.remove_signers(quorum, array![alice].span()); + assert_signers_list(array![charlie, bob].span()); + spy.assert_only_event_signer_removed(contract_address, alice); + assert_eq!(state.is_signer(alice), false); + + // Remove Charlie from signers + state.remove_signers(quorum, array![charlie].span()); + assert_signers_list(array![bob].span()); + spy.assert_only_event_signer_removed(contract_address, charlie); + assert_eq!(state.is_signer(charlie), false); +} + +#[test] +fn test_remove_multiple_signers() { + let quorum = 1; + let (alice, bob, charlie, other) = (ALICE(), BOB(), CHARLIE(), OTHER()); + let mut state = setup_component(quorum, array![alice, bob, charlie, other].span()); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Remove Alice and Other from signers + assert_signers_list(array![alice, bob, charlie, other].span()); + state.remove_signers(quorum, array![alice, other].span()); + assert_signers_list(array![charlie, bob].span()); + spy.assert_event_signer_removed(contract_address, alice); + spy.assert_only_event_signer_removed(contract_address, other); + assert_eq!(state.is_signer(alice), false); + assert_eq!(state.is_signer(other), false); +} + +#[test] +fn test_remove_add_remove() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice, bob, charlie].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Remove Alice from signers + state.remove_signers(quorum, array![alice].span()); + assert_signers_list(array![charlie, bob].span()); + assert_eq!(state.is_signer(alice), false); + + // Add Alice to signers + state.add_signers(quorum, array![alice].span()); + assert_signers_list(array![charlie, bob, alice].span()); + assert_eq!(state.is_signer(alice), true); + + // Remove Alice from signers + state.remove_signers(quorum, array![alice].span()); + assert_signers_list(array![charlie, bob].span()); + assert_eq!(state.is_signer(alice), false); +} + +#[test] +fn test_not_signers_ignored_when_removing() { + let quorum = 1; + let (alice, bob, charlie, other) = (ALICE(), BOB(), CHARLIE(), OTHER()); + let mut state = setup_component(quorum, array![alice, bob, charlie].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Remove Alice and Other from signers + let mut spy = spy_events(); + state.remove_signers(quorum, array![alice, other].span()); + assert_signers_list(array![charlie, bob].span()); + assert_eq!(state.is_signer(alice), false); + assert_eq!(state.is_signer(other), false); + spy.assert_only_event_signer_removed(contract_address, alice); +} + +#[test] +fn test_remove_signers_does_nothing_if_signers_empty() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice, bob, charlie].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Call `remove_signers` with an empty list + let mut spy = spy_events(); + let empty_list = array![].span(); + state.remove_signers(quorum, empty_list); + assert_signers_list(array![alice, bob, charlie].span()); + spy.assert_no_events_left_from(contract_address); +} + +#[test] +#[should_panic(expected: 'Multisig: unauthorized')] +fn test_cannot_remove_when_not_multisig_itself() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice, bob, charlie].span()); + + // Try to call 'remove_signers' from Other + start_cheat_caller_address(test_address(), OTHER()); + state.remove_signers(quorum, array![alice].span()); +} + +#[test] +#[should_panic(expected: 'Multisig: quorum cannot be 0')] +fn test_cannot_remove_with_zero_quorum() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice, bob, charlie].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to remove signers with 0 quorum value + state.remove_signers(0, array![bob, charlie].span()); +} + +#[test] +#[should_panic(expected: 'Multisig: quorum > signers')] +fn test_cannot_remove_with_quorum_too_high() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice, bob, charlie].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to remove signers with quorum value too high + state.remove_signers(3, array![bob].span()); +} + +// +// replace_signer +// + +#[test] +fn test_replace_signer() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice, bob].span()); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Check state before + assert_signers_list(array![alice, bob].span()); + assert_eq!(state.is_signer(alice), true); + assert_eq!(state.is_signer(charlie), false); + + // Replace Alice with Charlie + state.replace_signer(alice, charlie); + spy.assert_event_signer_removed(contract_address, alice); + spy.assert_only_event_signer_added(contract_address, charlie); + + // Check state after + assert_signers_list(array![charlie, bob].span()); + assert_eq!(state.is_signer(alice), false); + assert_eq!(state.is_signer(charlie), true); +} + +#[test] +#[should_panic(expected: 'Multisig: not a signer')] +fn test_cannot_replace_not_signer() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice, bob].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to replace not a signer + state.replace_signer(OTHER(), charlie); +} + +#[test] +#[should_panic(expected: 'Multisig: already a signer')] +fn test_cannot_replace_with_existing_signer() { + let quorum = 1; + let (alice, bob) = (ALICE(), BOB()); + let mut state = setup_component(quorum, array![alice, bob].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to replace with existing signer + state.replace_signer(alice, bob); +} + +#[test] +#[should_panic(expected: 'Multisig: zero address signer')] +fn test_cannot_replace_with_zero_address() { + let quorum = 1; + let (alice, bob, charlie) = (ALICE(), BOB(), CHARLIE()); + let mut state = setup_component(quorum, array![alice, bob, charlie].span()); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to replace with zero address + state.replace_signer(alice, ZERO()); +} + +// +// change_quorum +// + +#[test] +fn test_change_quorum_higher_value() { + let (initial_quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(initial_quorum, signers); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Increase quorum value + let new_quorum = initial_quorum + 1; + state.change_quorum(new_quorum); + assert_eq!(state.get_quorum(), new_quorum); + spy.assert_only_event_quorum_updated(contract_address, initial_quorum, new_quorum); +} + +#[test] +fn test_change_quorum_lower_value() { + let (initial_quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(initial_quorum, signers); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Reduce quorum value + let new_quorum = initial_quorum - 1; + state.change_quorum(new_quorum); + assert_eq!(state.get_quorum(), new_quorum); + spy.assert_only_event_quorum_updated(contract_address, initial_quorum, new_quorum); +} + +#[test] +fn test_change_quorum_to_same_value() { + let (initial_quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(initial_quorum, signers); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Call 'change_quorum' with the current quorum value + state.change_quorum(initial_quorum); + assert_eq!(state.get_quorum(), initial_quorum); + spy.assert_no_events_left_from(contract_address); +} + +#[test] +fn test_change_quorum_to_min_value() { + let (initial_quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(initial_quorum, signers); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Set quorum to min allowed value (1) + let new_quorum = 1; + state.change_quorum(new_quorum); + assert_eq!(state.get_quorum(), new_quorum); + spy.assert_only_event_quorum_updated(contract_address, initial_quorum, new_quorum); +} + +#[test] +fn test_change_quorum_to_max_value() { + let (initial_quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(initial_quorum, signers); + let contract_address = test_address(); + let mut spy = spy_events(); + start_cheat_caller_address(contract_address, contract_address); + + // Set quorum to max allowed value (signers count) + let new_quorum = signers.len(); + state.change_quorum(new_quorum); + assert_eq!(state.get_quorum(), new_quorum); + spy.assert_only_event_quorum_updated(contract_address, initial_quorum, new_quorum); +} + +#[test] +#[should_panic(expected: 'Multisig: quorum cannot be 0')] +fn test_cannot_change_quorum_to_zero() { + let (initial_quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(initial_quorum, signers); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to set quorum to 0 + state.change_quorum(0); +} + +#[test] +#[should_panic(expected: 'Multisig: quorum > signers')] +fn test_cannot_set_quorum_too_high() { + let (initial_quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(initial_quorum, signers); + let contract_address = test_address(); + start_cheat_caller_address(contract_address, contract_address); + + // Try to set quorum to a value too high (signers count + 1) + let new_quorum = signers.len() + 1; + state.change_quorum(new_quorum); +} + +#[test] +#[should_panic(expected: 'Multisig: unauthorized')] +fn test_cannot_change_quorum_when_not_multisig_itself() { + let (initial_quorum, signers) = DEFAULT_DATA(); + let mut state = setup_component(initial_quorum, signers); + start_cheat_caller_address(test_address(), OTHER()); + + // Try to set quorum to 0 + state.change_quorum(0); +} + +// +// Helpers +// + +#[derive(Copy, Drop)] +enum MockCall { + AddNumber: felt252, + FailingFn, + BadSelector +} + +fn build_call(call: MockCall) -> Call { + let (selector, calldata) = match call { + MockCall::AddNumber(number) => (selector!("add_number"), array![number]), + MockCall::FailingFn => (selector!("failing_function"), array![]), + MockCall::BadSelector => (selector!("bad_selector"), array![]), + }; + Call { to: MOCK_ADDRESS(), selector, calldata: calldata.span() } +} + +fn assert_tx_state(id: TransactionID, expected_state: TransactionState) { + let state = COMPONENT_STATE(); + let tx_state = state.get_transaction_state(id); + let block = state.get_submitted_block(id); + let is_confirmed = state.is_confirmed(id); + let is_executed = state.is_executed(id); + let tx_confirmations = state.get_transaction_confirmations(id); + + assert_eq!(tx_state, expected_state); + match expected_state { + TransactionState::NotFound => { + assert!(block.is_zero()); + assert!(!is_confirmed); + assert!(!is_executed); + assert!(tx_confirmations.is_zero()); + }, + TransactionState::Pending => { + assert!(block.is_non_zero()); + assert!(!is_confirmed); + assert!(!is_executed); + assert!(tx_confirmations < state.get_quorum()); + }, + TransactionState::Confirmed => { + assert!(block.is_non_zero()); + assert!(is_confirmed); + assert!(!is_executed); + assert!(tx_confirmations >= state.get_quorum()); + }, + TransactionState::Executed => { + assert!(block.is_non_zero()); + assert!(is_confirmed); + assert!(is_executed); + assert!(tx_confirmations >= state.get_quorum()); + } + }; +} + +fn assert_signers_list(expected_signers: Span) { + let state = COMPONENT_STATE(); + let actual_signers = state.get_signers(); + assert_eq!(actual_signers, expected_signers); + for signer in expected_signers { + assert!(state.is_signer(*signer)); + }; +} + +// +// Events +// + +#[generate_trait] +impl MultisigSpyHelpersImpl of MultisigSpyHelpers { + // + // SignerAdded + // + + fn assert_event_signer_added( + ref self: EventSpy, contract: ContractAddress, signer: ContractAddress + ) { + let expected = Event::SignerAdded(SignerAdded { signer }); + self.assert_emitted_single(contract, expected); + } + + fn assert_only_event_signer_added( + ref self: EventSpy, contract: ContractAddress, signer: ContractAddress + ) { + self.assert_event_signer_added(contract, signer); + self.assert_no_events_left_from(contract); + } + + // + // SignerRemoved + // + + fn assert_event_signer_removed( + ref self: EventSpy, contract: ContractAddress, signer: ContractAddress + ) { + let expected = Event::SignerRemoved(SignerRemoved { signer }); + self.assert_emitted_single(contract, expected); + } + + fn assert_only_event_signer_removed( + ref self: EventSpy, contract: ContractAddress, signer: ContractAddress + ) { + self.assert_event_signer_removed(contract, signer); + self.assert_no_events_left_from(contract); + } + + // + // QuorumUpdated + // + + fn assert_event_quorum_updated( + ref self: EventSpy, contract: ContractAddress, old_quorum: u32, new_quorum: u32 + ) { + let expected = Event::QuorumUpdated(QuorumUpdated { old_quorum, new_quorum }); + self.assert_emitted_single(contract, expected); + } + + fn assert_only_event_quorum_updated( + ref self: EventSpy, contract: ContractAddress, old_quorum: u32, new_quorum: u32 + ) { + self.assert_event_quorum_updated(contract, old_quorum, new_quorum); + self.assert_no_events_left_from(contract); + } + + // + // TransactionSubmitted + // + + fn assert_event_tx_submitted( + ref self: EventSpy, contract: ContractAddress, id: TransactionID, signer: ContractAddress + ) { + let expected = Event::TransactionSubmitted(TransactionSubmitted { id, signer }); + self.assert_emitted_single(contract, expected); + } + + fn assert_only_event_tx_submitted( + ref self: EventSpy, contract: ContractAddress, id: TransactionID, signer: ContractAddress + ) { + self.assert_event_tx_submitted(contract, id, signer); + self.assert_no_events_left_from(contract); + } + + // + // TransactionConfirmed + // + + fn assert_event_tx_confirmed( + ref self: EventSpy, contract: ContractAddress, id: TransactionID, signer: ContractAddress + ) { + let expected = Event::TransactionConfirmed(TransactionConfirmed { id, signer }); + self.assert_emitted_single(contract, expected); + } + + fn assert_only_event_tx_confirmed( + ref self: EventSpy, contract: ContractAddress, id: TransactionID, signer: ContractAddress + ) { + self.assert_event_tx_confirmed(contract, id, signer); + self.assert_no_events_left_from(contract); + } + + // + // ConfirmationRevoked + // + + fn assert_event_confirmation_revoked( + ref self: EventSpy, contract: ContractAddress, id: TransactionID, signer: ContractAddress + ) { + let expected = Event::ConfirmationRevoked(ConfirmationRevoked { id, signer }); + self.assert_emitted_single(contract, expected); + } + + fn assert_only_event_confirmation_revoked( + ref self: EventSpy, contract: ContractAddress, id: TransactionID, signer: ContractAddress + ) { + self.assert_event_confirmation_revoked(contract, id, signer); + self.assert_no_events_left_from(contract); + } + + // + // TransactionExecuted + // + + fn assert_event_tx_executed(ref self: EventSpy, contract: ContractAddress, id: TransactionID) { + let expected = Event::TransactionExecuted(TransactionExecuted { id }); + self.assert_emitted_single(contract, expected); + } + + fn assert_only_event_tx_executed( + ref self: EventSpy, contract: ContractAddress, id: TransactionID + ) { + self.assert_event_tx_executed(contract, id); + self.assert_no_events_left_from(contract); + } + + // + // CallSalt + // + + fn assert_event_call_salt( + ref self: EventSpy, contract: ContractAddress, id: TransactionID, salt: felt252 + ) { + let expected = Event::CallSalt(CallSalt { id, salt }); + self.assert_emitted_single(contract, expected); + } +} diff --git a/packages/governance/src/tests/test_utils.cairo b/packages/governance/src/tests/test_utils.cairo index b19e27785..d1bf2b7c1 100644 --- a/packages/governance/src/tests/test_utils.cairo +++ b/packages/governance/src/tests/test_utils.cairo @@ -1,4 +1,4 @@ -use crate::timelock::utils::call_impls::CallPartialEq; +use crate::utils::call_impls::CallPartialEq; use starknet::account::Call; use starknet::contract_address_const; diff --git a/packages/governance/src/timelock.cairo b/packages/governance/src/timelock.cairo index 0abcf6e3b..5d7929f3a 100644 --- a/packages/governance/src/timelock.cairo +++ b/packages/governance/src/timelock.cairo @@ -1,6 +1,5 @@ pub mod interface; pub mod timelock_controller; -pub mod utils; pub use timelock_controller::OperationState; pub use timelock_controller::TimelockControllerComponent::{ diff --git a/packages/governance/src/timelock/timelock_controller.cairo b/packages/governance/src/timelock/timelock_controller.cairo index bd8fbcb38..474a50475 100644 --- a/packages/governance/src/timelock/timelock_controller.cairo +++ b/packages/governance/src/timelock/timelock_controller.cairo @@ -18,7 +18,7 @@ pub mod TimelockControllerComponent { use core::num::traits::Zero; use core::pedersen::PedersenTrait; use crate::timelock::interface::{ITimelock, TimelockABI}; - use crate::timelock::utils::call_impls::{HashCallImpl, HashCallsImpl, CallPartialEq}; + use crate::utils::call_impls::{HashCallImpl, HashCallsImpl, CallPartialEq}; use openzeppelin_access::accesscontrol::AccessControlComponent::InternalTrait as AccessControlInternalTrait; use openzeppelin_access::accesscontrol::AccessControlComponent::{ AccessControlImpl, AccessControlCamelImpl @@ -156,11 +156,11 @@ pub mod TimelockControllerComponent { self: @ComponentState, id: felt252 ) -> OperationState { let timestamp = Self::get_timestamp(self, id); - if (timestamp == 0) { + if timestamp == 0 { return OperationState::Unset; - } else if (timestamp == DONE_TIMESTAMP) { + } else if timestamp == DONE_TIMESTAMP { return OperationState::Done; - } else if (timestamp > starknet::get_block_timestamp()) { + } else if timestamp > starknet::get_block_timestamp() { return OperationState::Waiting; } else { return OperationState::Ready; diff --git a/packages/governance/src/timelock/utils.cairo b/packages/governance/src/utils.cairo similarity index 100% rename from packages/governance/src/timelock/utils.cairo rename to packages/governance/src/utils.cairo diff --git a/packages/governance/src/timelock/utils/call_impls.cairo b/packages/governance/src/utils/call_impls.cairo similarity index 91% rename from packages/governance/src/timelock/utils/call_impls.cairo rename to packages/governance/src/utils/call_impls.cairo index 6e1b1663d..c76d77fca 100644 --- a/packages/governance/src/timelock/utils/call_impls.cairo +++ b/packages/governance/src/utils/call_impls.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.18.0 (governance/timelock/utils/call_impls.cairo) +// OpenZeppelin Contracts for Cairo v0.18.0 (governance/utils/call_impls.cairo) use core::hash::{HashStateTrait, HashStateExTrait, Hash}; use starknet::account::Call; @@ -36,6 +36,6 @@ pub impl CallPartialEq of PartialEq { } #[inline(always)] fn ne(lhs: @Call, rhs: @Call) -> bool { - !(lhs == rhs) + lhs != rhs } } diff --git a/packages/test_common/src/mocks.cairo b/packages/test_common/src/mocks.cairo index 027f99625..2e6901947 100644 --- a/packages/test_common/src/mocks.cairo +++ b/packages/test_common/src/mocks.cairo @@ -5,6 +5,7 @@ pub mod erc1155; pub mod erc20; pub mod erc2981; pub mod erc721; +pub mod multisig; pub mod non_implementing; pub mod nonces; pub mod security; diff --git a/packages/test_common/src/mocks/multisig.cairo b/packages/test_common/src/mocks/multisig.cairo new file mode 100644 index 000000000..c7141a04b --- /dev/null +++ b/packages/test_common/src/mocks/multisig.cairo @@ -0,0 +1,61 @@ +#[starknet::contract] +pub mod MultisigWalletMock { + use openzeppelin_governance::multisig::MultisigComponent; + use starknet::ContractAddress; + + component!(path: MultisigComponent, storage: multisig, event: MultisigEvent); + + #[abi(embed_v0)] + impl MultisigImpl = MultisigComponent::MultisigImpl; + impl InternalImpl = MultisigComponent::InternalImpl; + + #[storage] + pub struct Storage { + #[substorage(v0)] + pub multisig: MultisigComponent::Storage, + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + MultisigEvent: MultisigComponent::Event + } + + #[constructor] + fn constructor(ref self: ContractState, quorum: u32, signers: Span) { + self.multisig.initializer(quorum, signers); + } +} + +#[starknet::interface] +pub trait IMultisigTargetMock { + fn add_number(ref self: TState, number: felt252); + fn get_current_sum(self: @TState) -> felt252; + fn failing_function(self: @TState); +} + +#[starknet::contract] +pub mod MultisigTargetMock { + use starknet::storage::{StoragePointerReadAccess, StoragePointerWriteAccess}; + + #[storage] + pub struct Storage { + pub total_sum: felt252, + } + + #[abi(embed_v0)] + impl MockContractImpl of super::IMultisigTargetMock { + fn add_number(ref self: ContractState, number: felt252) { + self.total_sum.write(self.total_sum.read() + number); + } + + fn get_current_sum(self: @ContractState) -> felt252 { + self.total_sum.read() + } + + fn failing_function(self: @ContractState) { + core::panic_with_felt252('Expected failure'); + } + } +} diff --git a/packages/testing/src/constants.cairo b/packages/testing/src/constants.cairo index 9a4a8891c..d3cd64fdc 100644 --- a/packages/testing/src/constants.cairo +++ b/packages/testing/src/constants.cairo @@ -9,6 +9,7 @@ pub const VALUE: u256 = 300; pub const FELT_VALUE: felt252 = 'FELT_VALUE'; pub const ROLE: felt252 = 'ROLE'; pub const TIMESTAMP: u64 = 1704067200; // 2024-01-01 00:00:00 UTC +pub const BLOCK_NUMBER: u64 = 1234567; pub const OTHER_ROLE: felt252 = 'OTHER_ROLE'; pub const CHAIN_ID: felt252 = 'CHAIN_ID'; pub const TOKEN_ID: u256 = 21; @@ -101,6 +102,18 @@ pub fn DELEGATEE() -> ContractAddress { contract_address_const::<'DELEGATEE'>() } +pub fn ALICE() -> ContractAddress { + contract_address_const::<'ALICE'>() +} + +pub fn BOB() -> ContractAddress { + contract_address_const::<'BOB'>() +} + +pub fn CHARLIE() -> ContractAddress { + contract_address_const::<'CHARLIE'>() +} + pub fn DATA(success: bool) -> Span { let value = if success { SUCCESS