From 93b2f9381f11f066abe3dd5d43feff401cd9be96 Mon Sep 17 00:00:00 2001 From: "Emmanuel Joseph (JET)" Date: Tue, 1 Oct 2024 02:01:03 +0100 Subject: [PATCH 1/2] feat: signer and transaction --- .github/PULL_REQUEST_TEMPLATE.md | 4 +- README.md | 10 + src/MultiSigEnterpriseVault.sol | 12 +- src/components/MultiSigTransaction.sol | 234 ++++++++++++++++++ src/components/User.sol | 49 +++- src/components/roles/SignerRole.sol | 17 +- src/interfaces/IMultiSigEnterpriseVault.sol | 10 +- src/interfaces/IMultiSigTransaction.sol | 111 +++++++++ src/interfaces/IUser.sol | 12 + src/libraries/AddressUtils.sol | 36 +++ src/libraries/ArraysUtils.sol | 88 +++++++ src/utilities/VaultStructs.sol | 33 ++- test/components/BaseMultiSigTest.t.sol | 27 +- test/components/MultiSigTransactionTest.t.sol | 130 ++++++++++ test/mocks/MockERC20Token.sol | 14 ++ 15 files changed, 741 insertions(+), 46 deletions(-) create mode 100644 src/components/MultiSigTransaction.sol create mode 100644 src/interfaces/IMultiSigTransaction.sol create mode 100644 src/libraries/ArraysUtils.sol create mode 100644 test/components/MultiSigTransactionTest.t.sol create mode 100644 test/mocks/MockERC20Token.sol diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 398266f..9ce0a40 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,4 +1,4 @@ - + - ### Before Merging diff --git a/README.md b/README.md index 8995542..14607a6 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,16 @@ forge test forge snapshot ``` +## Flatten Contracts for Verification + +To flatten the contract for verification (e.g., on [Remix IDE](https://remix.ethereum.org/)), run the flatten command: + +```bash +forge flatten ./src/MultiSigEnterpriseVault.sol > ./.private/MultiSigEnterpriseVault.sol +``` + +This command outputs a single Solidity file containing all dependencies. + ## License ```md diff --git a/src/MultiSigEnterpriseVault.sol b/src/MultiSigEnterpriseVault.sol index 6c53c82..717275e 100644 --- a/src/MultiSigEnterpriseVault.sol +++ b/src/MultiSigEnterpriseVault.sol @@ -1,18 +1,15 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.27; -import {User} from './components/User.sol'; -import {AddressUtils} from './libraries/AddressUtils.sol'; import {IMultiSigEnterpriseVault} from './interfaces/IMultiSigEnterpriseVault.sol'; +import {MultiSigTransaction} from './components/MultiSigTransaction.sol'; +import {AddressUtils} from './libraries/AddressUtils.sol'; /** * @title MultiSig Enterprise Vault Contract * @author Emmanuel Joseph (JET) */ -contract MultiSigEnterpriseVault is User, IMultiSigEnterpriseVault { - /// @notice The current threshold required for signatory approval. - uint256 public signatoryThreshold; - +contract MultiSigEnterpriseVault is MultiSigTransaction, IMultiSigEnterpriseVault { /** * @dev Initializes the MultiSigEnterpriseVault with the owner, initial signatory threshold, and owner override limit. * @param owner The address of the contract owner. @@ -23,9 +20,8 @@ contract MultiSigEnterpriseVault is User, IMultiSigEnterpriseVault { address owner, uint256 initialThreshold, uint256 initialOwnerOverrideLimit - ) User(owner, initialOwnerOverrideLimit) { + ) MultiSigTransaction(owner, initialThreshold, initialOwnerOverrideLimit) { AddressUtils.requireValidUserAddress(owner); - signatoryThreshold = initialThreshold; } /** diff --git a/src/components/MultiSigTransaction.sol b/src/components/MultiSigTransaction.sol new file mode 100644 index 0000000..db58efe --- /dev/null +++ b/src/components/MultiSigTransaction.sol @@ -0,0 +1,234 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.27; + +import '../libraries/Counters.sol'; +import '../libraries/AddressUtils.sol'; +import '../utilities/VaultConstants.sol'; +import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; +import {SafeERC20} from '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol'; +import {IMultiSigTransaction} from '../interfaces/IMultiSigTransaction.sol'; +import {Transaction} from '../utilities/VaultStructs.sol'; +import {ArraysUtils} from '../libraries/ArraysUtils.sol'; +import {User} from './User.sol'; + +/** + * @title MultiSigTransaction + * @dev Manages transaction initiation, approval, and execution for ETH and ERC20 transfers. + */ +abstract contract MultiSigTransaction is User, IMultiSigTransaction { + using Counters for Counters.Counter; + + /// @notice The minimum number of approvals to execute a transaction + uint256 public signatoryThreshold; + + /// @notice Using Counter library for total transactions + Counters.Counter private _transactionCount; + + /// @notice Mapping to store all transactions by their ID + mapping(uint256 => Transaction) private _transactions; + + /// @notice Mapping to track approvals for each transaction by signers + mapping(uint256 => mapping(address => bool)) public approvals; + + /** + * @dev Initializes the `MultiSigTransaction` and `User` contracts. + * @param owner The address of the contract owner. + * @param initialThreshold The initial threshold for signatory approval. + * @param initialOwnerOverrideLimit The initial timelock limit for owner override. + */ + constructor( + address owner, + uint256 initialThreshold, + uint256 initialOwnerOverrideLimit + ) User(owner, initialOwnerOverrideLimit) { + signatoryThreshold = initialThreshold; + } + + /** + * @dev Modifier to ensure a valid vault transaction. + * Reverts with `InvalidTransaction` if the given transaction ID is invalid. + * @param transactionId The ID of the transaction to validate. + */ + modifier validTransaction( + uint256 transactionId + ) { + if (transactionId == 0 || transactionId < _transactionCount.current()) { + revert InvalidTransaction(transactionId); + } + _; + } + + /** + * Receives ETH sent to the contract and emits `FundsReceived` event + */ + receive() external payable { + emit FundsReceived(_msgSender(), address(0), msg.value); + } + + /** + * @inheritdoc IMultiSigTransaction + */ + function depositToken(address token, uint256 amount) external payable { + AddressUtils.requireValidTokenAddress(token); + require(amount >= msg.value, 'MultiSigTransaction: Invalid deposit amount'); + + IERC20(token).transferFrom(_msgSender(), address(this), msg.value); + emit FundsReceived(_msgSender(), token, msg.value); + } + + /** + * @notice Returns the balance of ETH held in the contract. + * @return The ETH balance of the contract. + */ + function getBalance() public view returns (uint256) { + return address(this).balance; + } + + /** + * @notice Returns the balance of a specific ERC20 token held in the contract. + * @param token The ERC20 token address. + * @return The token balance of the contract. + */ + function getTokenBalance(address token) public view returns (uint256) { + AddressUtils.requireValidTokenAddress(token); + return IERC20(token).balanceOf(address(this)); + } + + /** + * @notice Returns the total number of transactions. + * @return uint256 The total number of transactions. + * @dev Only callable by the owner. + */ + function totalTransactions() public view returns (uint256) { + return _transactionCount.current(); + } + + /** + * @notice Returns the transaction details of a given ID. + * @param transactionId The ID of the requested transaction. + * @return Transaction The transaction object associated with the provided ID. + */ + function getTransaction( + uint256 transactionId + ) public view validTransaction(transactionId) onlyUser returns (Transaction memory) { + return _transactions[transactionId]; + } + + /** + * @notice Returns the total number of approvals a transaction has. + * @param transactionId The ID of the requested transaction. + * @return uint256 The transaction total number of approvers. + */ + function getTransactionApprovals( + uint256 transactionId + ) public view validTransaction(transactionId) onlyUser returns (uint256) { + return _transactions[transactionId].approvals.current(); + } + + /** + * @notice Returns the signatures associated with a transaction. + * @param transactionId The ID of the requested transaction. + * @return address[] The transaction signatures array. + */ + function getTransactionSignatures( + uint256 transactionId + ) public view validTransaction(transactionId) onlyUser returns (address[] memory) { + return _transactions[transactionId].signatures; + } + + /** + * @notice Initiates a new transaction by an Owner or Signer. + * @param target The target address for the transaction. + * @param value The amount of ETH or tokens to send. + * @param token The ERC20 token address (0x0 for ETH). + * @param data The transaction data (0x0 for empty data). + */ + function initiateTransaction( + address payable target, + address token, + uint256 value, + bytes memory data + ) public validSigner { + AddressUtils.requireValidTransactionTarget(target); + if (token == address(0)) { + if (value > getBalance()) revert InsufficientTokenBalance(getBalance(), value); + } else { + if (value > getTokenBalance(token)) revert InsufficientTokenBalance(getTokenBalance(token), value); + } + + _transactionCount.increment(); + uint256 transactionId = _transactionCount.current(); + Transaction storage txn = _transactions[transactionId]; + + txn.initiator = _msgSender(); + txn.target = target; + txn.token = token; + txn.value = value; + txn.data = data; + + emit TransactionInitiated(transactionId, _msgSender(), target, token, value); + } + + /** + * @notice Enables valid signers to approve a transaction. + * @param transactionId The ID of the transaction to approve. + */ + function approveTransaction( + uint256 transactionId + ) public validTransaction(transactionId) validSigner { + Transaction storage txn = _transactions[transactionId]; + if (txn.isExecuted) revert TransactionAlreadyExecuted(transactionId); + if (approvals[transactionId][_msgSender()]) revert TransactionNotApproved(transactionId); + + txn.approvals.increment(); + txn.signatures.push(_msgSender()); + approvals[transactionId][_msgSender()] = true; + emit TransactionApproved(transactionId, _msgSender(), block.timestamp); + } + + /** + * @notice Enables valid signers to revokes a transaction approval. + * @param transactionId The ID of the transaction to revoke approval for. + */ + function revokeApproval( + uint256 transactionId + ) public validTransaction(transactionId) validSigner { + Transaction storage txn = _transactions[transactionId]; + if (txn.isExecuted) revert TransactionAlreadyExecuted(transactionId); + if (!approvals[transactionId][_msgSender()]) revert TransactionNotApproved(transactionId); + + txn.approvals.decrement(); + approvals[transactionId][_msgSender()] = false; + uint256 signerSignatureIndex = ArraysUtils.arrayElementIndexLookup(_msgSender(), txn.signatures); + ArraysUtils.removeElementFromArray(signerSignatureIndex, txn.signatures); + + emit TransactionRevoked(transactionId, _msgSender(), block.timestamp); + } + + /** + * @notice Executes a transaction if the threshold is met. + * @param transactionId The ID of the transaction to execute. + */ + function executeTransaction( + uint256 transactionId + ) public validTransaction(transactionId) validExecutor { + Transaction storage txn = _transactions[transactionId]; + if (txn.isExecuted) revert TransactionAlreadyExecuted(transactionId); + if (txn.approvals.current() < signatoryThreshold) revert InsufficientApprovals(signatoryThreshold, txn.approvals.current()); + if (hasRole(OWNER_ROLE, _msgSender())) { + if (!approvals[transactionId][_msgSender()]) revert TransactionNotApproved(transactionId); + } + + txn.isExecuted = true; + if (txn.token == address(0)) { + // Send ETH + Address.sendValue(txn.target, txn.value); + } else { + // Send ERC20 tokens + IERC20 token = IERC20(txn.token); + SafeERC20.safeTransfer(token, txn.target, txn.value); + } + + emit TransactionExecuted(transactionId, _msgSender(), block.timestamp); + } +} diff --git a/src/components/User.sol b/src/components/User.sol index 9f7e30d..b864cd2 100644 --- a/src/components/User.sol +++ b/src/components/User.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.27; import '../libraries/Counters.sol'; +import '../utilities/VaultConstants.sol'; import {IUser} from '../interfaces/IUser.sol'; import {OwnerRole} from './roles/OwnerRole.sol'; import {SignerRole} from './roles/SignerRole.sol'; @@ -39,7 +40,7 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser { /** * @dev Modifier to restrict access to functions to only vault users. - * Reverts with `AccessControlUnauthorizedSigner` if the caller is not the signer. + * Reverts with `InvalidUserProfile` if the caller is not a user. */ modifier onlyUser() { if (!_isUser(_msgSender())) { @@ -48,6 +49,42 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser { _; } + /** + * @dev Modifier to ensure a valid vault user. + * Reverts with `InvalidUserProfile` if the given user address is invalid. + * @param user The user address to validate. + */ + modifier validUser( + address user + ) { + if (!_isUser(user)) { + revert InvalidUserProfile(user); + } + _; + } + + /** + * @dev Modifier to restrict access to functions to valid signers. + * Reverts with `UnauthorizedTransactionSigner` if the caller is not an authorized transaction signer. + */ + modifier validSigner() { + if (!isSigner(_msgSender()) && !hasRole(OWNER_ROLE, _msgSender())) { + revert UnauthorizedTransactionSigner(_msgSender()); + } + _; + } + + /** + * @dev Modifier to restrict access to functions to valid executors. + * Reverts with `UnauthorizedTransactionExecutor` if the caller is not an authorized transaction executor. + */ + modifier validExecutor() { + if (!hasRole(EXECUTOR_ROLE, _msgSender()) && !hasRole(OWNER_ROLE, _msgSender())) { + revert UnauthorizedTransactionExecutor(_msgSender()); + } + _; + } + /** * @notice Returns the total number of users. * @return uint256 The total number of users. @@ -68,10 +105,7 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser { */ function getUserProfile( address user - ) public view onlyOwner returns (UserProfile memory) { - if (!_isUser(user)) { - revert InvalidUserProfile(user); - } + ) public view validUser(user) onlyOwner returns (UserProfile memory) { return _users[user]; } @@ -190,10 +224,7 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser { */ function _removeUser( address user - ) private { - UserProfile storage profile = _users[user]; - profile.user.requireValidUserAddress(); - + ) private validUser(user) { delete _users[user]; _userCount.decrement(); } diff --git a/src/components/roles/SignerRole.sol b/src/components/roles/SignerRole.sol index 8118265..eb8e02c 100644 --- a/src/components/roles/SignerRole.sol +++ b/src/components/roles/SignerRole.sol @@ -5,6 +5,7 @@ import {AccessControl} from '@openzeppelin/contracts/access/AccessControl.sol'; import {SIGNER_ROLE, OWNER_ROLE} from '../../utilities/VaultConstants.sol'; import {ISignerRole} from '../../interfaces/roles/ISignerRole.sol'; import {AddressUtils} from '../../libraries/AddressUtils.sol'; +import {ArraysUtils} from '../../libraries/ArraysUtils.sol'; import '../../libraries/Counters.sol'; /** @@ -24,7 +25,7 @@ abstract contract SignerRole is AccessControl, ISignerRole { /** * @dev Modifier to restrict access to functions to only the signer. - * Reverts with `AccessControlUnauthorizedSigner` if the caller is not the signer. + * Reverts with `AccessControlUnauthorizedSigner` if the caller is not a signer. */ modifier onlySigner() { if (!isSigner(_msgSender())) { @@ -91,16 +92,10 @@ abstract contract SignerRole is AccessControl, ISignerRole { revokeRole(SIGNER_ROLE, signer); // Remove signer from the _signers array - uint256 _totalSigners = totalSigners(); - for (uint256 i = 0; i < _totalSigners; i++) { - if (_signers[i] == signer) { - _signers[i] = _signers[_totalSigners - 1]; // Move the last element into the place of the removed signer - _signers.pop(); // Remove the last element - _signerCount.decrement(); - break; - } - } - + uint256 signerIndex = ArraysUtils.arrayElementIndexLookup(signer, _signers); + ArraysUtils.removeElementFromArray(signerIndex, _signers); + _signerCount.decrement(); + emit SignerRemoved(signer); } } diff --git a/src/interfaces/IMultiSigEnterpriseVault.sol b/src/interfaces/IMultiSigEnterpriseVault.sol index 432cead..bcfd76b 100644 --- a/src/interfaces/IMultiSigEnterpriseVault.sol +++ b/src/interfaces/IMultiSigEnterpriseVault.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.27; /** * @title IMultiSigEnterpriseVault Interface * @author Emmanuel Joseph (JET) - * @dev Interface defining events and external functions for the MultiSig Enterprise Vault contract. + * @dev Interface defining errors, events and external functions for the MultiSig Enterprise Vault contract. */ interface IMultiSigEnterpriseVault { /** @@ -13,14 +13,8 @@ interface IMultiSigEnterpriseVault { error SignersApprovalRequired(); /** - * @notice Emitted when the signatory threshold is updated. + * @dev Event emitted when the signatory threshold is updated. * @param newThreshold The new threshold value for signatory approval. */ event ThresholdUpdated(uint256 newThreshold); - - /** - * @notice Returns the current signatory threshold for the vault. - * @return The current signatory threshold. - */ - function signatoryThreshold() external view returns (uint256); } diff --git a/src/interfaces/IMultiSigTransaction.sol b/src/interfaces/IMultiSigTransaction.sol new file mode 100644 index 0000000..0b9a50d --- /dev/null +++ b/src/interfaces/IMultiSigTransaction.sol @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.27; + +/** + * @title IMultiSigTransaction Interface + * @dev Interface defining custom errors, events, and external functions for transaction management. + */ +interface IMultiSigTransaction { + /** + * @dev Error thrown when the provided transaction ID is not valid. + * @param transactionId The ID of the invalid transaction. + */ + error InvalidTransaction(uint256 transactionId); + + /** + * @dev Error thrown when there are insufficient tokens to complete a transaction. + * @param availableBalance The available vault token balance. + * @param requestedValue The requested transaction value. + */ + error InsufficientTokenBalance(uint256 availableBalance, uint256 requestedValue); + + /** + * @dev Error thrown when a transaction has already been executed. + * @param transactionId The ID of the transaction that was already executed. + */ + error TransactionAlreadyExecuted(uint256 transactionId); + + /** + * @dev Error thrown when the required number of approvals has not been met. + * @param transactionId The ID of the not approved transaction. + */ + error TransactionNotApproved(uint256 transactionId); + + /** + * @dev Error thrown when there are insufficient approvals to execute a transaction. + * @param approvalsRequired The required number of approvals. + * @param currentApprovals The current number of approvals. + */ + error InsufficientApprovals(uint256 approvalsRequired, uint256 currentApprovals); + + /** + * @dev Event emitted when tokens (ETH or ERC20) are received. + * @param from The address that sent the token + * @param token The token contract address (0x0 for ETH). + * @param amount The received token amount. + */ + event FundsReceived( + address indexed from, + address token, + uint256 amount + ); + + /** + * @dev Event emitted when a transaction is initiated. + * @param transactionId The ID of the initiated transaction. + * @param initiator The address of the initiator. + * @param target The address to receive the value. + * @param token The token contract address (0x0 for ETH). + * @param value The value to be transferred. + */ + event TransactionInitiated( + uint256 indexed transactionId, + address indexed initiator, + address indexed target, + address token, + uint256 value + ); + + /** + * @dev Event emitted when a transaction is approved. + * @param transactionId The ID of the approved transaction. + * @param approver The address of the account who approved the transaction. + * @param timestamp The timestamp (in seconds) when the transaction was approved. + */ + event TransactionApproved(uint256 indexed transactionId, address indexed approver, uint256 timestamp); + + /** + * @dev Event emitted when a transaction approval is revoked. + * @param transactionId The ID of the transaction. + * @param revoker The address of the account who revoked approval. + * @param timestamp The timestamp (in seconds) when the transaction was revoked. + */ + event TransactionRevoked(uint256 indexed transactionId, address indexed revoker, uint256 timestamp); + + /** + * @dev Event emitted when a transaction is executed. + * @param transactionId The ID of the executed transaction. + * @param executor The address of the signer who executed the transaction. + * @param timestamp The timestamp (in seconds) when the transaction was executed. + */ + event TransactionExecuted(uint256 indexed transactionId, address indexed executor, uint256 timestamp); + + /** + * @notice Receives ERC20 tokens and emits `FundsReceived` event + * @param token The ERC20 token address. + * @param amount The amount of tokens sent. + */ + function depositToken(address token, uint256 amount) external payable; + + /** + * @dev Returns the current signatory threshold for the vault. + * @return The current signatory threshold. + */ + function signatoryThreshold() external view returns (uint256); + + /** + * @dev Returns the vault's total transactions. + * @return The total transaction value. + */ + function totalTransactions() external view returns (uint256); +} diff --git a/src/interfaces/IUser.sol b/src/interfaces/IUser.sol index 5366eba..70cc1c7 100644 --- a/src/interfaces/IUser.sol +++ b/src/interfaces/IUser.sol @@ -12,4 +12,16 @@ interface IUser { * @param user The address of the user with an invalid profile. */ error InvalidUserProfile(address user); + + /** + * @dev Error thrown when an unauthorized account attempts to sign a transaction. + * @param account The address of the unauthorized account. + */ + error UnauthorizedTransactionSigner(address account); + + /** + * @dev Error thrown when an unauthorized account attempts to execute a transaction. + * @param account The address of the unauthorized account. + */ + error UnauthorizedTransactionExecutor(address account); } diff --git a/src/libraries/AddressUtils.sol b/src/libraries/AddressUtils.sol index 0fb1283..9543d9c 100644 --- a/src/libraries/AddressUtils.sol +++ b/src/libraries/AddressUtils.sol @@ -16,6 +16,18 @@ library AddressUtils { */ error InvalidUserAddress(address account); + /** + * @dev Error thrown when an invalid token address is provided. + * @param token The invalid token address. + */ + error InvalidTokenAddress(address token); + + /** + * @dev Error thrown when an invalid transaction target is provided. + * @param target The invalid target address. + */ + error InvalidTransactionTarget(address target); + /** * @notice Ensure a user address is valid (i.e., not a zero address). * @param account The user address to validate. @@ -27,4 +39,28 @@ library AddressUtils { revert InvalidUserAddress(account); } } + + /** + * @notice Ensure a token address is valid. + * @param token The token address to validate. + */ + function requireValidTokenAddress( + address token + ) internal pure { + if (token == address(0)) { + revert InvalidTokenAddress(token); + } + } + + /** + * @notice Ensure a transaction target is valid. + * @param target The transaction target to validate. + */ + function requireValidTransactionTarget( + address payable target + ) internal pure { + if (target == address(0)) { + revert InvalidTransactionTarget(target); + } + } } diff --git a/src/libraries/ArraysUtils.sol b/src/libraries/ArraysUtils.sol new file mode 100644 index 0000000..1988517 --- /dev/null +++ b/src/libraries/ArraysUtils.sol @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.27; + +import '@openzeppelin/contracts/utils/Arrays.sol'; + +/** + * @title ArraysUtils Library + * @dev Extends OpenZeppelin's Arrays library to add custom utility functions. + */ +library ArraysUtils { + /** + * @dev Error thrown when the provided index is not found in an array. + * @param index The invalid array index + */ + error ArrayIndexOutOfBound(uint256 index); + + /** + * @dev Finds the index of a 'uint256' array element. + * @param element The array element to find + * @param array The array to lookup + * @return index The index of the found element + */ + function arrayElementIndexLookup( + uint256 element, + uint256[] memory array + ) internal pure returns (uint256 index) { + index = 0; + while (array[index] != element) { + index++; + } + } + + /** + * @dev Finds the index of a 'address' array element. + * @param element The array element to find + * @param array The array to lookup + * @return index The index of the found element + */ + function arrayElementIndexLookup( + address element, + address[] memory array + ) internal pure returns (uint256 index) { + index = 0; + while (array[index] != element) { + index++; + } + } + + /** + * @dev Removes an element by index from a 'uint256' array. + * @param index The array index of the element + * @param array The array to lookup for removal + * + * Requirements: + * - Ensures the `index` is available in the given array + */ + function removeElementFromArray( + uint256 index, + uint256[] storage array + ) internal { + if (index > array.length) revert ArrayIndexOutOfBound(index); + for (uint256 i = index; i < array.length - 1; i++) { + array[i] = array[i + 1]; + } + + array.pop(); + } + + /** + * @dev Removes an element by index from a 'address' array. + * @param index The array index of the element + * @param array The array to lookup for removal + * + * Requirements: + * - Ensures the `index` is available in the given array + */ + function removeElementFromArray( + uint256 index, + address[] storage array + ) internal { + if (index > array.length) revert ArrayIndexOutOfBound(index); + for (uint256 i = index; i < array.length - 1; i++) { + array[i] = array[i + 1]; + } + + array.pop(); + } +} diff --git a/src/utilities/VaultStructs.sol b/src/utilities/VaultStructs.sol index 417307e..1e5451d 100644 --- a/src/utilities/VaultStructs.sol +++ b/src/utilities/VaultStructs.sol @@ -1,12 +1,15 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.27; +import '../libraries/Counters.sol'; import {RoleType} from './VaultEnums.sol'; +using Counters for Counters.Counter; + /** * @title UserProfile Struct * @author Emmanuel Joseph (JET) - * @notice This struct stores the user profile details such as the user address, role, and the timestamp of when they joined. + * @notice This struct stores the vault's user profile details. * * @dev * - `user`: The address of the user. @@ -18,3 +21,31 @@ struct UserProfile { RoleType role; uint256 joinedAt; } + +/** + * @title Transaction Struct + * @author Emmanuel Joseph (JET) + * @notice This struct stores the vault's transaction details. + * + * @dev + * - `initiator`: The initiator of the transaction (Owner or Signer). + * - `target`: The address to receive the transaction tokens. + * - `token`: The token contract address (0x0 for ETH). + * - `value`: The ETH or token value to send. + * - `approvals`: The total number of approvals + * - `signatures`: The addresses of those who approved/signed the transaction + * - `isExecuted`: Checks whether the transaction has been executed. + * - `isOverride`: Checks whether the transaction was override by the `executor`. + * - `data`: The transaction data (0x0 for empty data) + */ +struct Transaction { + address initiator; + address payable target; + address token; + uint256 value; + Counters.Counter approvals; + address[] signatures; + bool isExecuted; + bool isOverride; + bytes data; +} diff --git a/test/components/BaseMultiSigTest.t.sol b/test/components/BaseMultiSigTest.t.sol index 8ce67b6..17525b1 100644 --- a/test/components/BaseMultiSigTest.t.sol +++ b/test/components/BaseMultiSigTest.t.sol @@ -4,16 +4,29 @@ pragma solidity ^0.8.27; import '../MultiSigEnterpriseVault.t.sol'; contract BaseMultiSigTest is MultiSigEnterpriseVaultTest { - function testOwnerAddress() public view { - assertEq(vault.owner(), vaultOwner); - } + address internal firstSigner; + address internal secondSigner; + address internal vaultExecutor; + + function setUp() public virtual override { + super.setUp(); + + firstSigner = makeAddr('firstSigner'); + secondSigner = makeAddr('secondSigner'); + vaultExecutor = makeAddr('vaultExecutor'); + + vm.prank(vaultOwner); + vault.addExecutor(vaultExecutor); + + vm.prank(vaultOwner); + vault.addSigner(firstSigner); - function testTotalUsers() public { vm.prank(vaultOwner); - assertEq(vault.totalUsers(), 1); + vault.addSigner(secondSigner); } - function testInitialThreshold() public view { - assertEq(vault.signatoryThreshold(), initialThreshold); + function testTotalUsers() public { + vm.prank(vaultOwner); + assertEq(vault.totalUsers(), 4); } } diff --git a/test/components/MultiSigTransactionTest.t.sol b/test/components/MultiSigTransactionTest.t.sol new file mode 100644 index 0000000..97101f5 --- /dev/null +++ b/test/components/MultiSigTransactionTest.t.sol @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.27; + +import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; +import {Address} from '@openzeppelin/contracts/utils/Address.sol'; +import {Transaction} from '../../src/utilities/VaultStructs.sol'; +import {MockERC20Token} from '../mocks/MockERC20Token.sol'; +import './BaseMultiSigTest.t.sol'; + +contract MultiSigTransactionTest is BaseMultiSigTest { + IERC20 internal mockToken; + address internal mockAddress; + + function setUp() public override { + super.setUp(); + + mockToken = IERC20(address(new MockERC20Token())); + mockAddress = address(mockToken); + + deal(mockAddress, vaultAddress, 1000 ether); + Address.sendValue(payable(vaultAddress), 10 ether); + } + + function testContactBalance() public view { + assertEq(vault.getBalance(), 10 ether); + assertEq(vault.getTokenBalance(mockAddress), 1000 ether); + } + + function testInitiateETHTransaction() public { + vm.prank(firstSigner); + bytes memory txData = '0x'; + uint256 txValue = 1 ether; + address txReceiver = address(0x1234); + vault.initiateTransaction(payable(txReceiver), address(0), txValue, txData); + assertEq(vault.totalTransactions(), 1); + + vm.prank(vaultOwner); + assertEq(vault.getTransactionApprovals(1), 0); + + vm.prank(secondSigner); + Transaction memory txn = vault.getTransaction(1); + assertEq(txn.initiator, firstSigner); + assertEq(txn.target, txReceiver); + assertEq(txn.token, address(0)); + assertEq(txn.value, txValue); + assertFalse(txn.isExecuted); + assertEq(txn.data, txData); + } + + function testApproveAndExecuteETHTransaction() public { + uint256 transactionValue = 1 ether; + vm.prank(vaultOwner); + vault.initiateTransaction(payable(address(0x1234)), address(0), transactionValue, '0x'); + + // Approvals + vm.prank(firstSigner); + vault.approveTransaction(1); + vm.prank(secondSigner); + vault.approveTransaction(1); + vm.prank(vaultOwner); + vault.approveTransaction(1); + + // Execute + vm.prank(vaultOwner); + vault.executeTransaction(1); + + // Assert transaction executed + assertEq(vault.getBalance(), 9 ether); + } + + function testInitiateERC20Transaction() public { + vm.prank(secondSigner); + uint256 tokenAmount = 100 ether; + vault.initiateTransaction(payable(address(0x1234)), mockAddress, tokenAmount, '0x'); + assertEq(vault.totalTransactions(), 1); + + vm.prank(firstSigner); + Transaction memory txn = vault.getTransaction(1); + assertEq(txn.initiator, secondSigner); + assertEq(txn.token, mockAddress); + assertEq(txn.value, tokenAmount); + } + + function testApproveAndExecuteERC20Transaction() public { + vm.prank(vaultOwner); + uint256 tokenAmount = 100 ether; + vault.initiateTransaction(payable(address(0x1234)), mockAddress, tokenAmount, '0x'); + + vm.prank(firstSigner); + vault.approveTransaction(1); + vm.prank(secondSigner); + vault.approveTransaction(1); + vm.prank(vaultOwner); + vault.approveTransaction(1); + + vm.prank(vaultExecutor); + vault.executeTransaction(1); + + assertEq(mockToken.balanceOf(payable(address(0x1234))), tokenAmount); + assertEq(vault.getTokenBalance(mockAddress), 900 ether); + } + + function testRevokeApproval() public { + vm.prank(firstSigner); + vault.initiateTransaction(payable(address(0x1234)), address(0), 1 ether, '0x'); + + vm.prank(firstSigner); + vault.approveTransaction(1); + vm.prank(firstSigner); + assertEq(vault.getTransactionApprovals(1), 1); + + vm.prank(firstSigner); + vault.revokeApproval(1); + + vm.prank(firstSigner); + assertEq(vault.getTransactionApprovals(1), 0); + } + + function testInsufficientApprovals() public { + vm.prank(firstSigner); + vault.initiateTransaction(payable(address(0x1234)), address(0), 1 ether, '0x'); + + vm.prank(firstSigner); + vault.approveTransaction(1); + + vm.prank(vaultExecutor); + vm.expectRevert(); + vault.executeTransaction(1); + } +} diff --git a/test/mocks/MockERC20Token.sol b/test/mocks/MockERC20Token.sol new file mode 100644 index 0000000..5981822 --- /dev/null +++ b/test/mocks/MockERC20Token.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.27; + +import '@openzeppelin/contracts/token/ERC20/ERC20.sol'; + +/** + * @title MockERC20Token + * @dev A mock ERC20 token for testing purposes. + */ +contract MockERC20Token is ERC20 { + constructor() ERC20('Mock Token', 'MOCK') { + _mint(msg.sender, 1000 ether); + } +} From 2997dabd1d7f8852b1aaeac7851d25887cc4491b Mon Sep 17 00:00:00 2001 From: "Emmanuel Joseph (JET)" Date: Tue, 1 Oct 2024 02:19:20 +0100 Subject: [PATCH 2/2] fix: forge format --- src/components/MultiSigTransaction.sol | 8 ++++++-- src/components/roles/SignerRole.sol | 2 +- src/interfaces/IMultiSigTransaction.sol | 12 ++---------- src/libraries/ArraysUtils.sol | 20 ++++---------------- 4 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/components/MultiSigTransaction.sol b/src/components/MultiSigTransaction.sol index db58efe..8226148 100644 --- a/src/components/MultiSigTransaction.sol +++ b/src/components/MultiSigTransaction.sol @@ -89,7 +89,9 @@ abstract contract MultiSigTransaction is User, IMultiSigTransaction { * @param token The ERC20 token address. * @return The token balance of the contract. */ - function getTokenBalance(address token) public view returns (uint256) { + function getTokenBalance( + address token + ) public view returns (uint256) { AddressUtils.requireValidTokenAddress(token); return IERC20(token).balanceOf(address(this)); } @@ -214,7 +216,9 @@ abstract contract MultiSigTransaction is User, IMultiSigTransaction { ) public validTransaction(transactionId) validExecutor { Transaction storage txn = _transactions[transactionId]; if (txn.isExecuted) revert TransactionAlreadyExecuted(transactionId); - if (txn.approvals.current() < signatoryThreshold) revert InsufficientApprovals(signatoryThreshold, txn.approvals.current()); + if (txn.approvals.current() < signatoryThreshold) { + revert InsufficientApprovals(signatoryThreshold, txn.approvals.current()); + } if (hasRole(OWNER_ROLE, _msgSender())) { if (!approvals[transactionId][_msgSender()]) revert TransactionNotApproved(transactionId); } diff --git a/src/components/roles/SignerRole.sol b/src/components/roles/SignerRole.sol index eb8e02c..0131dfe 100644 --- a/src/components/roles/SignerRole.sol +++ b/src/components/roles/SignerRole.sol @@ -95,7 +95,7 @@ abstract contract SignerRole is AccessControl, ISignerRole { uint256 signerIndex = ArraysUtils.arrayElementIndexLookup(signer, _signers); ArraysUtils.removeElementFromArray(signerIndex, _signers); _signerCount.decrement(); - + emit SignerRemoved(signer); } } diff --git a/src/interfaces/IMultiSigTransaction.sol b/src/interfaces/IMultiSigTransaction.sol index 0b9a50d..bf6d8ac 100644 --- a/src/interfaces/IMultiSigTransaction.sol +++ b/src/interfaces/IMultiSigTransaction.sol @@ -44,11 +44,7 @@ interface IMultiSigTransaction { * @param token The token contract address (0x0 for ETH). * @param amount The received token amount. */ - event FundsReceived( - address indexed from, - address token, - uint256 amount - ); + event FundsReceived(address indexed from, address token, uint256 amount); /** * @dev Event emitted when a transaction is initiated. @@ -59,11 +55,7 @@ interface IMultiSigTransaction { * @param value The value to be transferred. */ event TransactionInitiated( - uint256 indexed transactionId, - address indexed initiator, - address indexed target, - address token, - uint256 value + uint256 indexed transactionId, address indexed initiator, address indexed target, address token, uint256 value ); /** diff --git a/src/libraries/ArraysUtils.sol b/src/libraries/ArraysUtils.sol index 1988517..90d85ac 100644 --- a/src/libraries/ArraysUtils.sol +++ b/src/libraries/ArraysUtils.sol @@ -20,10 +20,7 @@ library ArraysUtils { * @param array The array to lookup * @return index The index of the found element */ - function arrayElementIndexLookup( - uint256 element, - uint256[] memory array - ) internal pure returns (uint256 index) { + function arrayElementIndexLookup(uint256 element, uint256[] memory array) internal pure returns (uint256 index) { index = 0; while (array[index] != element) { index++; @@ -36,10 +33,7 @@ library ArraysUtils { * @param array The array to lookup * @return index The index of the found element */ - function arrayElementIndexLookup( - address element, - address[] memory array - ) internal pure returns (uint256 index) { + function arrayElementIndexLookup(address element, address[] memory array) internal pure returns (uint256 index) { index = 0; while (array[index] != element) { index++; @@ -54,10 +48,7 @@ library ArraysUtils { * Requirements: * - Ensures the `index` is available in the given array */ - function removeElementFromArray( - uint256 index, - uint256[] storage array - ) internal { + function removeElementFromArray(uint256 index, uint256[] storage array) internal { if (index > array.length) revert ArrayIndexOutOfBound(index); for (uint256 i = index; i < array.length - 1; i++) { array[i] = array[i + 1]; @@ -74,10 +65,7 @@ library ArraysUtils { * Requirements: * - Ensures the `index` is available in the given array */ - function removeElementFromArray( - uint256 index, - address[] storage array - ) internal { + function removeElementFromArray(uint256 index, address[] storage array) internal { if (index > array.length) revert ArrayIndexOutOfBound(index); for (uint256 i = index; i < array.length - 1; i++) { array[i] = array[i + 1];