From a6a3b4cf6488c26af0b49d118f018d3f11c24fac Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Wed, 10 Dec 2025 12:01:36 +0530 Subject: [PATCH 01/12] added smart contracts, scripts and tests for multisig --- .../script/DeployMultisig.s.sol | 54 ++ .../src/MultiSignatureWallet.sol | 482 ++++++++++++ .../src/MultisigBeacon.sol | 17 + .../test/MultiSignatureWallet.t.sol | 700 ++++++++++++++++++ 4 files changed, 1253 insertions(+) create mode 100644 solidity/automation_registry/script/DeployMultisig.s.sol create mode 100644 solidity/automation_registry/src/MultiSignatureWallet.sol create mode 100644 solidity/automation_registry/src/MultisigBeacon.sol create mode 100644 solidity/automation_registry/test/MultiSignatureWallet.t.sol diff --git a/solidity/automation_registry/script/DeployMultisig.s.sol b/solidity/automation_registry/script/DeployMultisig.s.sol new file mode 100644 index 0000000000..bdc7c4c23b --- /dev/null +++ b/solidity/automation_registry/script/DeployMultisig.s.sol @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import {Script, console} from "forge-std/Script.sol"; +import {MultiSignatureWallet} from "../src/MultiSignatureWallet.sol"; +import {MultisigBeacon} from "../src/MultisigBeacon.sol"; +import {BeaconProxy} from "../lib/openzeppelin-contracts/contracts/proxy/beacon/BeaconProxy.sol"; + +contract DeployMultisig is Script { + address[] owners; + uint256 numConfirmations; + + function setUp() public { + owners = vm.envAddress("OWNERS", ","); + numConfirmations = vm.envUint("NUM_CONFIRMATIONS"); + } + + function run() public { + vm.startBroadcast(); + + // --------------------------------- + // Deploy multisig implementation + // --------------------------------- + MultiSignatureWallet multisigImpl = new MultiSignatureWallet(); + console.log("Multisig implementation deployed at: ", address(multisigImpl)); + + // ------------------------------------------- + // Deploy beacon pointing to implementation + // ------------------------------------------- + MultisigBeacon beacon = new MultisigBeacon(address(multisigImpl)); + console.log("Beacon deployed at: ", address(beacon)); + + // ---------------------- + // Deploy multisig proxy + // ---------------------- + console.log("Number of confirmations: ", numConfirmations); + console.log("Adding following owners: "); + for (uint i = 0; i < owners.length; i++) { + console.logAddress(owners[i]); + } + + bytes memory initData = abi.encodeCall(MultiSignatureWallet.initialize, (owners, numConfirmations)); + BeaconProxy multisigProxy = new BeaconProxy(address(beacon), initData); + console.log("Multisig Proxy deployed at: ", address(multisigProxy)); + + // ------------------------------------------ + // Transfer beacon's ownership to multisig + // ------------------------------------------ + beacon.transferOwnership(address(multisigProxy)); + console.log("Beacon ownership transferred to multisig proxy at: ", address(multisigProxy)); + + vm.stopBroadcast(); + } +} \ No newline at end of file diff --git a/solidity/automation_registry/src/MultiSignatureWallet.sol b/solidity/automation_registry/src/MultiSignatureWallet.sol new file mode 100644 index 0000000000..e0a105a0f2 --- /dev/null +++ b/solidity/automation_registry/src/MultiSignatureWallet.sol @@ -0,0 +1,482 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import {EnumerableSet} from "../lib/openzeppelin-contracts/contracts/utils/structs/EnumerableSet.sol"; +import {Initializable} from "../lib/openzeppelin-contracts/contracts/proxy/utils/Initializable.sol"; + +/** + * @title MultiSignatureWallet + * @dev A multisignature wallet contract that requires multiple owners to confirm transactions. + */ +contract MultiSignatureWallet is Initializable { + using EnumerableSet for EnumerableSet.AddressSet; + + /** + * @dev Emitted when a deposit is made to the contract. + * @param sender The address that sent the funds. + * @param amount The amount of funds deposited. + * @param balance The new balance of the contract after the deposit. + */ + event Deposit(address indexed sender, uint256 amount, uint256 balance); + + /** + * @dev Emitted when a new transaction is submitted. + * @param owner The address of the owner who submitted the transaction. + * @param txIndex The index of the transaction in the transactions array. + * @param to The contract address the transaction is directed to. + * @param value The amount of ether to be sent with the transaction. + * @param data The data payload of the transaction. + */ + event SubmitTransaction( + address indexed owner, + uint256 indexed txIndex, + address indexed to, + uint256 value, + bytes data + ); + + /** + * @dev Emitted when a transaction is confirmed by an owner. + * @param owner The address of the owner who confirmed the transaction. + * @param txIndex The index of the transaction in the transactions array. + */ + event ConfirmTransaction(address indexed owner, uint256 indexed txIndex); + + /** + * @dev Emitted when a confirmation is revoked by an owner. + * @param owner The address of the owner who revoked the confirmation. + * @param txIndex The index of the transaction in the transactions array. + */ + event RevokeConfirmation(address indexed owner, uint256 indexed txIndex); + + /** + * @dev Emitted when a transaction is executed. + * @param owner The address of the owner who executed the transaction. + * @param txIndex The index of the transaction in the transactions array. + * @param txData The data returned by the transaction call. + */ + event ExecuteTransaction(address indexed owner, uint256 indexed txIndex, bytes txData); + + /** + * @dev Emitted when a transaction to deploy a contract is executed. + * @param owner The address of the owner who executed the transaction. + * @param txIndex The index of the transaction in the transactions array. + * @param deployed The address of the deployed contract. + */ + event ExecuteTransactionDeployment(address indexed owner, uint256 indexed txIndex, address indexed deployed); + + /** + * @dev Emitted when new owners are added to the contract. + * @param owners An array of addresses representing the newly added owners. + */ + event OwnersAdded(address[] owners); + + /** + * @dev Emitted when owners are removed from the contract. + * @param owners An array of addresses representing the removed owners. + */ + event OwnersRemoved(address[] owners); + + /** + * @dev Emitted when the number of confirmations required is updated. + * @param newNumConfirmation The new number of confirmations required for a transaction. + */ + event NumConfirmationUpdated(uint256 newNumConfirmation); + + + // Custom error definitions + + /** + * @dev Error for when the function caller is not an owner. + */ + error NotAnOwner(); + + /** + * @dev Error for when a transaction ID is invalid (e.g., out of bounds). + */ + error InvalidTxnId(); + + /** + * @dev Error for when a transaction has already been executed. + */ + error TxnAlreadyExecuted(); + + /** + * @dev Error for when a transaction has already been confirmed by the caller. + */ + error TxnAlreadyConfirmed(); + + /** + * @dev Error for when the owners array is empty upon contract creation. + */ + error OwnersRequired(); + + /** + * @dev Error for when the number of required confirmations is invalid (0 or more than the number of owners). + */ + error InvalidNumberOfConfirmations(); + + /** + * @dev Error for when an invalid owner address is provided (e.g., zero address). + */ + error InvalidOwner(); + + /** + * @dev Error for when a duplicate owner address is provided. + */ + error OwnerNotUnique(); + + /** + * @dev Error for when a transaction does not have enough confirmations to be executed. + */ + error NotEnoughConfirmation(); + + /** + * @dev Error to revert with when a transaction execution fails. + */ + error ExecutionFailed(); + + /** + * @dev Error to revert with if empty contract creation code is passed. + */ + error EmptyCreationCode(); + + /** + * @dev Error to revert with when contract creation fails. + */ + error ContractCreationFailed(); + + /** + * @dev Error for when a transaction has not been confirmed by the caller. + */ + error TransactionNotConfirmed(); + + /** + * @dev Error for when a transaction has already expired. + */ + error TransactionAlreadyExpired(); + + /** + * @dev Error for when a function is called by an account other than the multisig wallet itself. + */ + error OnlyMultisigAccountCanCall(); + + EnumerableSet.AddressSet private owners; + uint256 public numConfirmationsRequired; + + // Structure to hold transaction details + struct Transaction { + address to; // Transaction target address + bool executed; // Flag indicating if the transaction has been executed + uint64 timeout; // Expiry timestamp of the transaction + uint24 numConfirmations; // Number of confirmations received for the transaction + uint256 value; // Amount of ether sent with the transaction + bytes data; // Data payload of the transaction + } + + // Mapping to track confirmations for each transaction by each owner + mapping(uint256 transactionIndex => mapping(address owner => bool permissionToExecute)) public isConfirmed; + + // Array to store all transactions + Transaction[] private transactions; + + // Function to ensure the caller is an owner + function onlyOwner(address owner) private view { + if (!owners.contains(owner)) + revert NotAnOwner(); + } + + // Function to ensure the caller is the multisig contract itself + function onlyMultiSig() private view { + if (msg.sender != address(this)) { + revert OnlyMultisigAccountCanCall(); + } + } + + // Function to check if a transaction exists + function txExists(uint256 _txIndex) private view { + if (_txIndex >= transactions.length) + revert InvalidTxnId(); + } + + // Function to check if a transaction has not been executed + function notExecuted(uint256 _txIndex) private view { + if (transactions[_txIndex].executed) + revert TxnAlreadyExecuted(); + } + + // Function to check if a transaction has not been expired or not + function txNotExpired(uint256 _txIndex) private view { + if (transactions[_txIndex].timeout < block.timestamp) + revert TransactionAlreadyExpired(); + } + + // Function to check if a transaction has not been confirmed by the caller + function notConfirmed(uint256 _txIndex) private view { + if (isConfirmed[_txIndex][msg.sender]) revert TxnAlreadyConfirmed(); + } + + /** + * @dev Disables the initialization for the implementation contract. + */ + constructor() { + _disableInitializers(); + } + + /** + * @dev Initializes the contract with initial owners and required confirmations. + * @param _owners Array of initial owner addresses. + * @param _numConfirmationsRequired Number of confirmations required for transactions. + */ + function initialize(address[] memory _owners, uint256 _numConfirmationsRequired) public initializer { + if (_owners.length == 0) revert OwnersRequired(); + if ( + _numConfirmationsRequired == 0 || + _numConfirmationsRequired > _owners.length + ) revert InvalidNumberOfConfirmations(); + + for (uint256 i = 0; i < _owners.length; i++) { + address owner = _owners[i]; + if (owner == address(0)) revert InvalidOwner(); + require(owners.add(owner), OwnerNotUnique()); + } + + numConfirmationsRequired = _numConfirmationsRequired; + } + + /** + * @dev Fallback function to receive ether and emit a deposit event. + */ + receive() external payable { + emit Deposit(msg.sender, msg.value, address(this).balance); + } + + /** + * @dev Function to submit a new transaction to the wallet. + * @param _to Address of the contract the transaction is directed to. + * @param _value Amount of ether to be sent with the transaction. + * @param _timeoutDuration Duration after which the transaction will get expire. + * @param _data Data payload of the transaction. + */ + function submitTransaction( + address _to, + uint256 _value, + uint64 _timeoutDuration, + bytes memory _data + ) external payable { + onlyOwner(msg.sender); + uint256 txIndex = transactions.length; + + transactions.push( + Transaction({ + to: _to, + executed: false, + timeout: uint64(block.timestamp) + _timeoutDuration, + //We assume the act of submission is an implicit confirmation + numConfirmations: 1, + value: _value, + data: _data + }) + ); + + isConfirmed[txIndex][msg.sender] = true; + + emit SubmitTransaction(msg.sender, txIndex, _to, _value, _data); + } + + /** + * @dev Function to confirm an existing transaction. + * @param _txIndex Index of the transaction to confirm. + */ + function confirmTransaction(uint256 _txIndex) public { + onlyOwner(msg.sender); + txExists(_txIndex); + notExecuted(_txIndex); + notConfirmed(_txIndex); + txNotExpired(_txIndex); + Transaction storage transaction = transactions[_txIndex]; + transaction.numConfirmations += 1; + isConfirmed[_txIndex][msg.sender] = true; + + emit ConfirmTransaction(msg.sender, _txIndex); + } + + /** + * @dev Function to execute a confirmed transaction. + * @param _txIndex Index of the transaction to execute. + */ + function executeTransaction(uint256 _txIndex) public { + onlyOwner(msg.sender); + txExists(_txIndex); + notExecuted(_txIndex); + txNotExpired(_txIndex); + Transaction storage transaction = transactions[_txIndex]; + if (transaction.numConfirmations < numConfirmationsRequired) + revert NotEnoughConfirmation(); + transaction.executed = true; + if (transaction.to == address(0)) { + address deployed = deploy(transaction.data, transaction.value); + + emit ExecuteTransactionDeployment(msg.sender, _txIndex, deployed); + } else { + (bool success, bytes memory data) = transaction.to.call{value: transaction.value}(transaction.data); + if (!success) { revert ExecutionFailed(); } + + emit ExecuteTransaction(msg.sender, _txIndex, data); + } + } + + /** + * @dev Function to revoke a previously given confirmation for a transaction. + * @param _txIndex Index of the transaction to revoke confirmation. + */ + function revokeConfirmation(uint256 _txIndex) external { + onlyOwner(msg.sender); + txExists(_txIndex); + notExecuted(_txIndex); + txNotExpired(_txIndex); + if (!isConfirmed[_txIndex][msg.sender]) { + revert TransactionNotConfirmed(); + } + + Transaction storage transaction = transactions[_txIndex]; + + transaction.numConfirmations -= 1; + isConfirmed[_txIndex][msg.sender] = false; + + emit RevokeConfirmation(msg.sender, _txIndex); + } + + /** + * @dev Function to add new owners to the wallet. + * @param _owners Array of new owner addresses to be added. + */ + function addOwners(address[] memory _owners) external { + onlyMultiSig(); + if (_owners.length == 0) revert OwnersRequired(); + + address[] memory ownersToUpdate = new address[](_owners.length); + uint256 c = 0; + + for (uint256 i = 0; i < _owners.length; i++) { + address owner = _owners[i]; + if (owner == address(0)) revert InvalidOwner(); + if (owners.add(owner)) { + ownersToUpdate[c++] = owner; + } + } + if (c > 0) + emit OwnersAdded(ownersToUpdate); + } + + /** + * @dev Function to remove existing owners from the wallet. + * @param _owners Array of existing owner addresses to be removed. + */ + function removeOwners(address[] memory _owners) external { + onlyMultiSig(); + if (_owners.length == 0) revert OwnersRequired(); + address[] memory ownersToUpdate = new address[](_owners.length); + uint256 c = 0; + + for (uint256 i = 0; i < _owners.length; i++) { + address owner = _owners[i]; + if (owners.remove(owner)) { + ownersToUpdate[c++] = owner; + } + } + + if (owners.length() < numConfirmationsRequired) { + revert InvalidNumberOfConfirmations(); + } + + if (c > 0) + emit OwnersRemoved(ownersToUpdate); + } + + /** + * @dev Function to update the number of required confirmations for transactions. + * @param _numConfirmationsRequired New number of confirmations required for transactions. + */ + function updateNumConfirmations(uint256 _numConfirmationsRequired) external { + onlyMultiSig(); + if ( + _numConfirmationsRequired == 0 || + _numConfirmationsRequired > owners.length() + ) revert InvalidNumberOfConfirmations(); + numConfirmationsRequired = _numConfirmationsRequired; + emit NumConfirmationUpdated(_numConfirmationsRequired); + } + + /** + * @dev Function to retrieve the list of current owners of the wallet. + * @return Array of addresses representing the current owners. + */ + function getOwners() public view returns (address[] memory) { + return owners.values(); + } + + /** + * @dev Function to retrieve the count of transactions submitted to the wallet. + * @return Total number of transactions in the wallet. + */ + function getTransactionCount() public view returns (uint256) { + return transactions.length; + } + + /** + * @dev Function to retrieve details of a specific transaction. + * @param _txIndex Index of the transaction to retrieve details for. + * @return to Transaction target address. + * @return value Amount of ether sent with the transaction. + * @return executed Boolean indicating if the transaction has been executed. + * @return numConfirmations Number of confirmations received for the transaction. + * @return timeout Expiry timestamp of the transaction. + * @return data Data payload of the transaction. + */ + function getTransaction( + uint256 _txIndex + ) + public + view + returns ( + address to, + uint256 value, + bool executed, + uint24 numConfirmations, + uint64 timeout, + bytes memory data + ) + { + Transaction storage transaction = transactions[_txIndex]; + + return ( + transaction.to, + transaction.value, + transaction.executed, + transaction.numConfirmations, + transaction.timeout, + transaction.data + ); + } + + /** + * @notice Deploys a contract using raw CREATE opcode + * @param _creationCode The creation bytecode of the contract to deploy + * @param _value Amount of ETH to sent along with contract creation. + * @return deployed The address of the deployed contract + */ + function deploy(bytes memory _creationCode, uint256 _value) private returns (address deployed) { + if (_creationCode.length == 0) { revert EmptyCreationCode(); } + + assembly { + // CREATE(value, offset, size) + deployed := create( + _value, // forward ETH if any + add(_creationCode, 0x20), // skip the length slot + mload(_creationCode) // size of creation code + ) + } + if (deployed == address(0)) { revert ContractCreationFailed(); } + } +} diff --git a/solidity/automation_registry/src/MultisigBeacon.sol b/solidity/automation_registry/src/MultisigBeacon.sol new file mode 100644 index 0000000000..b8c9426ed6 --- /dev/null +++ b/solidity/automation_registry/src/MultisigBeacon.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import {UpgradeableBeacon} from "../lib/openzeppelin-contracts/contracts/proxy/beacon/UpgradeableBeacon.sol"; + +/** + * @title MultisigBeacon + * @dev A beacon that stores the implementation address for multisig proxies. + * Admin can upgrade the implementation to a new version. + */ +contract MultisigBeacon is UpgradeableBeacon { + /** + * @dev Constructor to initialize the addresses for implementation and initial owner. + * @param _implementation Address of the initial multisig implementation contract. + */ + constructor(address _implementation) UpgradeableBeacon(_implementation, msg.sender) {} +} diff --git a/solidity/automation_registry/test/MultiSignatureWallet.t.sol b/solidity/automation_registry/test/MultiSignatureWallet.t.sol new file mode 100644 index 0000000000..594e818753 --- /dev/null +++ b/solidity/automation_registry/test/MultiSignatureWallet.t.sol @@ -0,0 +1,700 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.27; + +import {console} from "forge-std/Script.sol"; +import {Test} from "forge-std/Test.sol"; +import {BlockMeta} from "../src/BlockMeta.sol"; +import {ERC1967Proxy} from "../lib/openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {BeaconProxy} from "../lib/openzeppelin-contracts/contracts/proxy/beacon/BeaconProxy.sol"; +import "../lib/openzeppelin-contracts-upgradeable/contracts/access/Ownable2StepUpgradeable.sol"; +import {MultiSignatureWallet} from "../src/MultiSignatureWallet.sol"; +import "../src/MultisigBeacon.sol"; + +contract Multisig is Test { + BlockMeta blockMeta; + MultisigBeacon beacon; + address multiSigImplV1; + MultiSignatureWallet multiSig; + + address[] owners; + address[] newOwners; + address alice = address(0xA11CE); + + /// @dev Sets up initial state for testing. + /// @dev Deploys all required contracts. + function setUp() public { + address owner1 = address(1001); + address owner2 = address(1002); + address owner3 = address(1003); + address owner4 = address(1004); + address owner5 = address(1005); + owners.push(owner1); + owners.push(owner2); + owners.push(owner3); + owners.push(owner4); + owners.push(owner5); + + vm.startPrank(alice); + // Deploy Beacon contract + multiSigImplV1 = address(new MultiSignatureWallet()); + beacon = new MultisigBeacon(multiSigImplV1); + + // Deploy BeaconProxy for MultiSig + bytes memory multiSigInitData = abi.encodeCall(MultiSignatureWallet.initialize, (owners, 4)); + BeaconProxy multisigProxy = new BeaconProxy(address(beacon), multiSigInitData); + multiSig = MultiSignatureWallet(payable(multisigProxy)); + + // Transfer Beacon's ownership to multisig + beacon.transferOwnership(address(multisigProxy)); + vm.stopPrank(); + + + vm.startPrank(address(multisigProxy)); + // Deploy BlockMeta proxy contract + BlockMeta blockMetaImpl = new BlockMeta(); + bytes memory blockMetaInitData = abi.encodeCall(BlockMeta.initialize, ()); + ERC1967Proxy blockMetaProxy = new ERC1967Proxy(address(blockMetaImpl), blockMetaInitData); + blockMeta = BlockMeta(address(blockMetaProxy)); + vm.stopPrank(); + } + + /// @dev Test to ensure ownership and implementation address is initialized correctly. + function testOwnerAndImplementation() public view { + assertEq(beacon.owner(), address(multiSig)); + assertEq(blockMeta.owner(), address(multiSig)); + assertEq(beacon.implementation(), multiSigImplV1); + } + + /// @dev Test to ensure owners and number of confirmations required are initialized correctly. + function testInitialize() public view { + assertEq(multiSig.getOwners(), owners); + assertEq(multiSig.numConfirmationsRequired(), 4); + } + + /// @dev Test to ensure 'initialize' reverts if array of owners is empty. + function testInitializeRevertsIfOwnersArrayEmpty() public { + address[] memory emptyOwners; + vm.expectRevert(MultiSignatureWallet.OwnersRequired.selector); + + bytes memory initData = abi.encodeCall(MultiSignatureWallet.initialize, (emptyOwners, 1)); + new BeaconProxy(address(beacon), initData); + } + + /// @dev Test to ensure 'initialize' reverts if number of confirmations required is zero. + function testInitializeRevertsIfNumConfirmationsZero() public { + vm.expectRevert(MultiSignatureWallet.InvalidNumberOfConfirmations.selector); + + bytes memory initData = abi.encodeCall(MultiSignatureWallet.initialize, (owners, 0)); + new BeaconProxy(address(beacon), initData); + } + + /// @dev Test to ensure 'initialize' reverts if number of confirmations required is more than the number of owners. + function testInitializeRevertsIfNumConfirmationsMoreThanOwners() public { + vm.expectRevert(MultiSignatureWallet.InvalidNumberOfConfirmations.selector); + + bytes memory initData = abi.encodeCall(MultiSignatureWallet.initialize, (owners, 6)); + new BeaconProxy(address(beacon), initData); + } + + /// @dev Test to ensure 'initialize' reverts if any of the owner is address(0). + function testInitializeRevertsIfOwnerAddressZero() public { + address[] memory invalidOwners = new address[](3); + invalidOwners[0] = address(1001); + invalidOwners[1] = address(0); // Invalid owner + invalidOwners[2] = address(1002); + + vm.expectRevert(MultiSignatureWallet.InvalidOwner.selector); + + bytes memory initData = abi.encodeCall(MultiSignatureWallet.initialize, (invalidOwners, 1)); + new BeaconProxy(address(beacon), initData); + } + + /// @dev Test to ensure 'initialize' reverts if a duplicate owner is passed. + function testInitializeRevertsIfDuplicateOwner() public { + address[] memory duplicateOwners = new address[](3); + duplicateOwners[0] = address(1001); + duplicateOwners[1] = address(1001); // Duplicate owner + duplicateOwners[2] = address(1002); + + vm.expectRevert(MultiSignatureWallet.OwnerNotUnique.selector); + + bytes memory initData = abi.encodeCall(MultiSignatureWallet.initialize, (duplicateOwners, 1)); + new BeaconProxy(address(beacon), initData); + } + + /// @dev Helper function that returns calldata to set automation controller in BlockMeta. + function dataToSetAutomationController(address _controller) private pure returns (bytes memory) { + return (abi.encodeCall(BlockMeta.setAutomationController, (_controller))); + } + + /// @dev Helper function to submit a transaction to perform an action in the BlockMeta contract. + function submitTransaction(bytes memory _data) private { + vm.prank(address(1001)); + multiSig.submitTransaction( + address(blockMeta), + 0, + 10000, + _data + ); + } + + /// @dev Test to ensure 'submitTransaction' submits a transaction. + function testSubmitTransactionSetAutomationController() public { + BlockMeta impl = new BlockMeta(); + bytes memory data = dataToSetAutomationController(address(impl)); + submitTransaction(data); + + (address to, uint256 value, bool executed, uint24 numConfirmations, uint64 timeout, bytes memory storedData) = multiSig.getTransaction(0); + assertEq(to, address(blockMeta)); + assertEq(value, 0); + assertEq(executed, false); + assertEq(numConfirmations, 1); + assertEq(timeout, block.timestamp + 10000); + assertEq(storedData, data); + } + + /// @dev Test to ensure 'submitTransaction' reverts if caller is not an owner. + function testSubmitTransactionSetAutomationControllerRevertsIfNotOwner() public { + BlockMeta impl = new BlockMeta(); + bytes memory data = dataToSetAutomationController(address(impl)); + + vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); + + vm.prank(alice); // Not an owner + multiSig.submitTransaction( + address(blockMeta), + 0, + 100000, + data + ); + } + + /// @dev Helper function to confirm a transaction. + function confirmTransaction(address _owner, uint256 _txnId) private { + vm.prank(_owner); + multiSig.confirmTransaction(_txnId); + } + + /// @dev Helper function to grant sufficient confirmations. + function grantSufficientConfirmations(uint256 _txnId) private { + confirmTransaction(address(1002), _txnId); + confirmTransaction(address(1003), _txnId); + confirmTransaction(address(1004), _txnId); + } + + /// @dev Test to ensure 'confirmTransaction' confirms a transaction. + function testConfirmTransactionSetAutomationController() public { + testSubmitTransactionSetAutomationController(); + + grantSufficientConfirmations(0); + + ( , , , uint256 numConfirmations, , ) = multiSig.getTransaction(0); + assertEq(numConfirmations, 4); + } + + /// @dev Test to ensure 'confirmTransaction' reverts if caller is not an owner. + function testConfirmTransactionRevertsIfNotOwner() public { + testSubmitTransactionSetAutomationController(); + + vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); + confirmTransaction(alice, 0); // Not an owner + } + + /// @dev Test to ensure 'confirmTransaction' reverts if transaction does not exist. + function testConfirmTransactionRevertsIfTxDoesNotExist() public { + testSubmitTransactionSetAutomationController(); + + vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); + confirmTransaction(address(1002), 1); + } + + /// @dev Test to ensure 'confirmTransaction' reverts if the transaction is already executed. + function testConfirmTransactionRevertsIfTxAlreadyExecuted() public { + testSubmitTransactionSetAutomationController(); + + uint256 txId = 0; + grantSufficientConfirmations(txId); + + vm.prank(address(1002)); + multiSig.executeTransaction(txId); + + vm.expectRevert(MultiSignatureWallet.TxnAlreadyExecuted.selector); + + confirmTransaction(address(1005), txId); + } + + /// @dev Test to ensure 'confirmTransaction' reverts if transaction is already confirmed. + function testConfirmTransactionRevertsIfTxAlreadyConfirmed() public { + testSubmitTransactionSetAutomationController(); + + vm.expectRevert(MultiSignatureWallet.TxnAlreadyConfirmed.selector); + confirmTransaction(address(1001), 0); + } + + // @dev Test to ensure 'confirmTransaction' reverts if transaction has expired. + function testConfirmTransactionRevertsIfTxExpired() public { + vm.warp(500); + testSubmitTransactionSetAutomationController(); + + vm.warp(10501); + vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + + confirmTransaction(address(1005), 0); + } + + /// @dev Helper function to revoke confirmation. + function revokeConfirmation(address _owner, uint256 _txIndex) private { + vm.prank(_owner); + multiSig.revokeConfirmation(_txIndex); + } + + /// @dev Test to ensure 'revokeConfirmation' revokes the confirmation of an owner. + function testRevokeConfirmation() public { + testSubmitTransactionSetAutomationController(); + + uint256 txId = 0; + confirmTransaction(address(1002), txId); + revokeConfirmation(address(1001), txId); + + ( , , , uint256 confirmations , , ) = multiSig.getTransaction(txId); + assertEq(confirmations, 1); + } + + /// @dev Test to ensure 'revokeConfirmation' reverts if caller is not an owner. + function testRevokeConfirmationRevertsIfNotOwner() public { + testSubmitTransactionSetAutomationController(); + + vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); + revokeConfirmation(alice, 1); + } + + /// @dev Test to ensure 'revokeConfirmation' reverts if transaction does not exist. + function testRevokeConfirmationRevertsIfTxDoesNotExist() public { + testSubmitTransactionSetAutomationController(); + + vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); + revokeConfirmation(address(1001), 1); + } + + /// @dev Test to ensure 'revokeConfirmation' reverts if the transaction is already executed. + function testRevokeConfirmationRevertsIfTxAlreadyExecuted() public { + testSubmitTransactionSetAutomationController(); + + uint256 txId = 0; + grantSufficientConfirmations(txId); + + vm.prank(address(1002)); + multiSig.executeTransaction(txId); + + vm.expectRevert(MultiSignatureWallet.TxnAlreadyExecuted.selector); + revokeConfirmation(address(1001), txId); + } + + /// @dev Test to ensure 'revokeConfirmation' reverts if the transaction has expired. + function testRevokeConfirmationRevertsIfTxExpired() public { + vm.warp(500); + testSubmitTransactionSetAutomationController(); + + vm.warp(10501); + vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + + revokeConfirmation(address(1001), 0); + } + + /// @dev Test to ensure 'revokeConfirmation' reverts if the transaction was not confirmed. + function testRevokeConfirmationRevertsIfTxNotConfirmed() public { + testSubmitTransactionSetAutomationController(); + + vm.expectRevert(MultiSignatureWallet.TransactionNotConfirmed.selector); + revokeConfirmation(address(1002), 0); + } + + /// @dev Test to ensure 'executeTransaction' executes a transaction. + function testExecuteTransaction() public { + testSubmitTransactionSetAutomationController(); + + uint256 txId = 0; + grantSufficientConfirmations(txId); + + vm.prank(address(1001)); + multiSig.executeTransaction(txId); + + ( , , bool executed, , , ) = multiSig.getTransaction(txId); + assertTrue(executed); + } + + /// @dev Test to ensure 'executeTransaction' reverts if caller is not an owner. + function testExecuteTransactionRevertsIfCallerNotOwner() public { + testSubmitTransactionSetAutomationController(); + + vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); + + vm.prank(alice); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'executeTransaction' reverts if transaction does not exist. + function testExecuteTransactionRevertsIfTxDoesNotExist() public { + vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(1); + } + + /// @dev Test to ensure 'executeTransaction' reverts if transaction is already executed. + function testExecuteTransactionRevertsIfTxAlreadyExecuted() public { + testExecuteTransaction(); + + vm.expectRevert(MultiSignatureWallet.TxnAlreadyExecuted.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'executeTransaction' reverts if transaction has expired. + function testExecuteTransactionRevertsIfTxExpired() public { + vm.warp(500); + testSubmitTransactionSetAutomationController(); + + vm.warp(10501); + vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'executeTransaction' reverts if the transaction has insufficient number of confirmations. + function testExecuteTransactionRevertsIfInsufficientConfirmations() public { + testSubmitTransactionSetAutomationController(); + + uint256 txId = 0; + confirmTransaction(address(1002), txId); + confirmTransaction(address(1003), txId); + + vm.expectRevert(MultiSignatureWallet.NotEnoughConfirmation.selector); + + vm.prank(address(1001)); + multiSig.executeTransaction(txId); + } + + /// @dev Helper function that returns calldata to transfer ownership. + function dataToTransferOwnership() private view returns (bytes memory) { + return abi.encodeCall(Ownable2StepUpgradeable.transferOwnership, (alice)); + } + + /// @dev Test to ensure ownership transfer works correctly. + function testChangeOwnership() public { + submitTransaction(dataToTransferOwnership()); + grantSufficientConfirmations(0); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + + vm.prank(alice); + blockMeta.acceptOwnership(); + + assertEq(blockMeta.owner(), alice); + } + + /// @dev Helper function to return calldata to add an owner in multisig. + function dataToAddOwnerInMultiSig() private returns (bytes memory) { + newOwners.push(address(5001)); + return abi.encodeCall(MultiSignatureWallet.addOwners, (newOwners)); + } + + /// @dev Helper function to submit a transaction to perform an action in the MultiSignatureWallet. + function submitTransactionToMultiSig(bytes memory _data) private { + vm.prank(address(1001)); + multiSig.submitTransaction( + address(multiSig), + 0, + 10000, + _data + ); + } + + /// @dev Test to ensure 'addOwners' adds an array of owners in multisig. + function testAddOwners() public { + submitTransactionToMultiSig(dataToAddOwnerInMultiSig()); + grantSufficientConfirmations(0); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + + address[] memory updatedOwners = multiSig.getOwners(); + assertEq(updatedOwners[5], newOwners[0]); + assertEq(multiSig.getOwners().length, 6); + } + + /// @dev Test to ensure 'addOwners' reverts if array of owners is empty. + function testAddOwnersRevertsIfOwnersArrayEmpty() public { + address[] memory emptyOwners; + bytes memory data = abi.encodeCall(MultiSignatureWallet.addOwners, (emptyOwners)); + submitTransactionToMultiSig(data); + + grantSufficientConfirmations(0); + + vm.expectRevert(MultiSignatureWallet.ExecutionFailed.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'addOwners' reverts if any of the owners is address(0). + function testAddOwnersRevertsIfOwnerAddressZero() public { + newOwners.push(address(0)); + submitTransactionToMultiSig(dataToAddOwnerInMultiSig()); + + grantSufficientConfirmations(0); + + vm.expectRevert(MultiSignatureWallet.ExecutionFailed.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'addOwners' reverts if caller is not an owner. + function testAddOwnersRevertsIfCallerNotOwner() public { + submitTransactionToMultiSig(dataToAddOwnerInMultiSig()); + grantSufficientConfirmations(0); + + vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); + + vm.prank(alice); // Not an owner + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'addOwners' reverts if transaction has expired. + function testAddOwnersRevertsIfTimestampExpired() public { + vm.warp(500); + submitTransactionToMultiSig(dataToAddOwnerInMultiSig()); + + grantSufficientConfirmations(0); + + vm.warp(10501); + vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'addOwners' reverts if transaction has insufficient number of confirmations. + function testAddOwnersRevertsIfInsufficientConfirmations() public { + submitTransactionToMultiSig(dataToAddOwnerInMultiSig()); + + uint256 txId = 0; + confirmTransaction(address(1004), txId); + confirmTransaction(address(1005), txId); + + vm.expectRevert(MultiSignatureWallet.NotEnoughConfirmation.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(txId); + } + + /// @dev Helper function to return calldata to remove an array of owners from multisig. + function dataToRemoveOwnerFromMultiSig() private returns (bytes memory) { + newOwners.push(address(1001)); + return abi.encodeCall(MultiSignatureWallet.removeOwners, (newOwners)); + } + + /// @dev Test to ensure 'removeOwners' removes an array of owners from multisig. + function testRemoveOwners() public { + testAddOwners(); + + submitTransactionToMultiSig(dataToRemoveOwnerFromMultiSig()); + grantSufficientConfirmations(1); + + vm.prank(address(1002)); + multiSig.executeTransaction(1); + + assertEq(multiSig.getOwners().length, 4); + } + + /// @dev Test to ensure 'removeOwners' reverts if array of owners is empty. + function testRemoveOwnersRevertsIfOwnersArrayEmpty() public { + address[] memory emptyOwners; + bytes memory data = abi.encodeCall(MultiSignatureWallet.removeOwners, (emptyOwners)); + submitTransactionToMultiSig(data); + + grantSufficientConfirmations(0); + + vm.expectRevert(MultiSignatureWallet.ExecutionFailed.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'removeOwners' reverts if number of owners goes below the number of confirmations required. + function testRemoveOwnersRevertsIfNumOfOwnersGoesBelowNumConfirmations() public { + newOwners.push(address(1003)); + newOwners.push(address(1004)); + newOwners.push(address(1005)); + + bytes memory data = abi.encodeCall(MultiSignatureWallet.removeOwners, (newOwners)); + submitTransactionToMultiSig(data); + + grantSufficientConfirmations(0); + + vm.expectRevert(MultiSignatureWallet.ExecutionFailed.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'removeOwners' reverts if caller is not an owner. + function testRemoveOwnersRevertsIfnotOwner() public { + testAddOwners(); + + submitTransactionToMultiSig(dataToRemoveOwnerFromMultiSig()); + + grantSufficientConfirmations(1); + + vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); + + vm.prank(alice); // Not an owner + multiSig.executeTransaction(1); + } + + /// @dev Test to ensure 'removeOwners' reverts if transaction has expired. + function testRemoveOwnersRevertsIfTimestampExpired() public { + testAddOwners(); + + vm.warp(500); + submitTransactionToMultiSig(dataToRemoveOwnerFromMultiSig()); + + grantSufficientConfirmations(1); + + vm.warp(10501); + vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(1); + } + + /// @dev Test to ensure 'removeOwners' reverts if transaction has insufficient number of confirmations. + function testRemoveOwnersRevertsIfInsufficientConfirmations() public { + testAddOwners(); + + submitTransactionToMultiSig(dataToRemoveOwnerFromMultiSig()); + + uint256 txId = 1; + confirmTransaction(address(1004), txId); + confirmTransaction(address(1005), txId); + + vm.expectRevert(MultiSignatureWallet.NotEnoughConfirmation.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(txId); + } + + /// @dev Helper function to return calldata to update the number of confirmations required in the multisig. + function dataToUpdateNumConfimationsMultiSig(uint256 _num) private pure returns (bytes memory) { + return abi.encodeCall(MultiSignatureWallet.updateNumConfirmations, (_num)); + } + + /// @dev Test to ensure 'updateNumConfirmations' updates the number of confirmations required. + function testUpdateNumConfimations() public { + submitTransactionToMultiSig(dataToUpdateNumConfimationsMultiSig(3)); + grantSufficientConfirmations(0); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + + assertEq(multiSig.numConfirmationsRequired(), 3); + } + + /// @dev Test to ensure 'updateNumConfirmations' reverts if the number of confirmations required is zero. + function testUpdateNumConfimationsRevertsIfNumConfirmationsZero() public { + submitTransactionToMultiSig(dataToUpdateNumConfimationsMultiSig(0)); + grantSufficientConfirmations(0); + + vm.expectRevert(MultiSignatureWallet.ExecutionFailed.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'updateNumConfirmations' reverts if the number of confirmations required is more than the number of owners. + function testUpdateNumConfimationsRevertsIfNumConfirmationsMoreThanOwners() public { + submitTransactionToMultiSig(dataToUpdateNumConfimationsMultiSig(6)); + grantSufficientConfirmations(0); + + vm.expectRevert(MultiSignatureWallet.ExecutionFailed.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'updateNumConfirmations' reverts if the caller is not an owner. + function testUpdateNumConfimationsRevertsIfNotOwner() public { + submitTransactionToMultiSig(dataToUpdateNumConfimationsMultiSig(3)); + grantSufficientConfirmations(0); + + vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); + + vm.prank(alice); // Not an owner + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'updateNumConfirmations' reverts if the transaction has expired. + function testUpdateNumConfimationsRevertsIftimestampExpired() public { + vm.warp(500); + submitTransactionToMultiSig(dataToUpdateNumConfimationsMultiSig(3)); + + grantSufficientConfirmations(0); + + vm.warp(10501); + vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'updateNumConfirmations' reverts if the transaction has insufficient number of confirmations. + function testUpdateNumConfimationsRevertsIfInsufficientConfirmations() public { + submitTransactionToMultiSig(dataToUpdateNumConfimationsMultiSig(3)); + + uint256 txId = 0; + confirmTransaction(address(1002), txId); + confirmTransaction(address(1003), txId); + + vm.expectRevert(MultiSignatureWallet.NotEnoughConfirmation.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(txId); + } + + + /// @dev Test to ensure 'upgradeTo' upgrades the implementation address of the beacon. + function testUpgradeBeacon() public { + MultiSignatureWallet implV2 = new MultiSignatureWallet(); + bytes memory data = abi.encodeWithSelector(UpgradeableBeacon.upgradeTo.selector, address(implV2)); + + vm.prank(address(1001)); + multiSig.submitTransaction( + address(beacon), + 0, + 100000, + data + ); + + grantSufficientConfirmations(0); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + + assertEq(beacon.implementation(), address(implV2)); + assertNotEq(beacon.implementation(), multiSigImplV1); + } + + /// @dev Test to ensure 'upgradeTo' reverts if caller is not the owner. + function testUpgradeBeaconRevertIfNotOwner() public { + MultiSignatureWallet implV2 = new MultiSignatureWallet(); + + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, alice)); + + vm.prank(alice); + beacon.upgradeTo(address(implV2)); + } +} From 5e1994030e65b1db8162b70b0efa96dfcdffaf38 Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Fri, 12 Dec 2025 12:58:11 +0530 Subject: [PATCH 02/12] -fixed deployContract to allow deployment using multisig as msg.sender -added test cases for deployContract --- .../src/MultiSignatureWallet.sol | 25 ++--- .../test/MultiSignatureWallet.t.sol | 94 ++++++++++++++++++- 2 files changed, 102 insertions(+), 17 deletions(-) diff --git a/solidity/automation_registry/src/MultiSignatureWallet.sol b/solidity/automation_registry/src/MultiSignatureWallet.sol index e0a105a0f2..302ccae25a 100644 --- a/solidity/automation_registry/src/MultiSignatureWallet.sol +++ b/solidity/automation_registry/src/MultiSignatureWallet.sol @@ -59,11 +59,9 @@ contract MultiSignatureWallet is Initializable { /** * @dev Emitted when a transaction to deploy a contract is executed. - * @param owner The address of the owner who executed the transaction. - * @param txIndex The index of the transaction in the transactions array. - * @param deployed The address of the deployed contract. + * @param deployedContract The address of the deployed contract. */ - event ExecuteTransactionDeployment(address indexed owner, uint256 indexed txIndex, address indexed deployed); + event ContractDeployed(address indexed deployedContract); /** * @dev Emitted when new owners are added to the contract. @@ -305,7 +303,7 @@ contract MultiSignatureWallet is Initializable { * @dev Function to execute a confirmed transaction. * @param _txIndex Index of the transaction to execute. */ - function executeTransaction(uint256 _txIndex) public { + function executeTransaction(uint256 _txIndex) public returns (bytes memory) { onlyOwner(msg.sender); txExists(_txIndex); notExecuted(_txIndex); @@ -314,16 +312,11 @@ contract MultiSignatureWallet is Initializable { if (transaction.numConfirmations < numConfirmationsRequired) revert NotEnoughConfirmation(); transaction.executed = true; - if (transaction.to == address(0)) { - address deployed = deploy(transaction.data, transaction.value); - - emit ExecuteTransactionDeployment(msg.sender, _txIndex, deployed); - } else { - (bool success, bytes memory data) = transaction.to.call{value: transaction.value}(transaction.data); - if (!success) { revert ExecutionFailed(); } + (bool success, bytes memory data) = transaction.to.call{value: transaction.value}(transaction.data); + if (!success) { revert ExecutionFailed(); } - emit ExecuteTransaction(msg.sender, _txIndex, data); - } + emit ExecuteTransaction(msg.sender, _txIndex, data); + return data; } /** @@ -466,7 +459,8 @@ contract MultiSignatureWallet is Initializable { * @param _value Amount of ETH to sent along with contract creation. * @return deployed The address of the deployed contract */ - function deploy(bytes memory _creationCode, uint256 _value) private returns (address deployed) { + function deployContract(bytes memory _creationCode, uint256 _value) external returns (address deployed) { + onlyMultiSig(); if (_creationCode.length == 0) { revert EmptyCreationCode(); } assembly { @@ -478,5 +472,6 @@ contract MultiSignatureWallet is Initializable { ) } if (deployed == address(0)) { revert ContractCreationFailed(); } + emit ContractDeployed(deployed); } } diff --git a/solidity/automation_registry/test/MultiSignatureWallet.t.sol b/solidity/automation_registry/test/MultiSignatureWallet.t.sol index 594e818753..315b59d3d6 100644 --- a/solidity/automation_registry/test/MultiSignatureWallet.t.sol +++ b/solidity/automation_registry/test/MultiSignatureWallet.t.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.27; -import {console} from "forge-std/Script.sol"; import {Test} from "forge-std/Test.sol"; import {BlockMeta} from "../src/BlockMeta.sol"; import {ERC1967Proxy} from "../lib/openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol"; @@ -10,7 +9,7 @@ import "../lib/openzeppelin-contracts-upgradeable/contracts/access/Ownable2StepU import {MultiSignatureWallet} from "../src/MultiSignatureWallet.sol"; import "../src/MultisigBeacon.sol"; -contract Multisig is Test { +contract MultiSignatureWalletTest is Test { BlockMeta blockMeta; MultisigBeacon beacon; address multiSigImplV1; @@ -697,4 +696,95 @@ contract Multisig is Test { vm.prank(alice); beacon.upgradeTo(address(implV2)); } + + /// @dev Helper function to submit a transaction for contract deployment and grant sufficient confirmations. + function submitToDeploy(bytes memory _creationCode, uint256 _value, uint256 _tx) private { + bytes memory data = abi.encodeCall(MultiSignatureWallet.deployContract, (_creationCode, _value)); + submitTransactionToMultiSig(data); + grantSufficientConfirmations(_tx); + } + + /// @dev Helper function that returns creation code to deploy ERC1967 proxy contract. + function proxyCreationCode(address _impl) private pure returns (bytes memory) { + bytes memory initData = abi.encodeCall(BlockMeta.initialize, ()); + + return abi.encodePacked( + type(ERC1967Proxy).creationCode, + abi.encode(_impl, initData) + ); + } + + /// @dev Test to ensure 'deployContract' deploys contract and assigns MultiSig as contract owner. + function testDeployContract() public { + // Deploy implementation + submitToDeploy(type(BlockMeta).creationCode, 0, 0); + + vm.prank(address(1002)); + bytes memory dataImpl = multiSig.executeTransaction(0); + address impl = abi.decode(dataImpl, (address)); + + + // Deploy proxy + bytes memory creationCode = proxyCreationCode(impl); + submitToDeploy(creationCode, 0, 1); + + vm.prank(address(1002)); + bytes memory dataProxy = multiSig.executeTransaction(1); + address proxy = abi.decode(dataProxy, (address)); + assertEq(BlockMeta(proxy).owner(), address(multiSig)); + } + + /// @dev Test to ensure 'deployContract' reverts if caller is not MultiSig itself. + function testDeployContractRevertsIfCallerNotMultiSig() public { + bytes memory creationCode = type(BlockMeta).creationCode; + + vm.expectRevert(MultiSignatureWallet.OnlyMultisigAccountCanCall.selector); + + vm.prank(alice); + multiSig.deployContract(creationCode, 0); + } + + /// @dev Test to ensure 'deployContract' reverts if contract creation code is empty. + function testDeployContractRevertsIfCreationCodeEmpty() public { + // Deploy implementation + submitToDeploy("", 0, 0); // Empty creation code + + vm.expectRevert(MultiSignatureWallet.ExecutionFailed.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } + + /// @dev Test to ensure 'deployContract' reverts if initialize function is non-payable. + function testDeployContractRevertsIfInitializerNonPayable() public { + vm.deal(address(multiSig), 4 ether); + + // Deploy implementation + submitToDeploy(type(BlockMeta).creationCode, 0, 0); + + vm.prank(address(1002)); + bytes memory dataImpl = multiSig.executeTransaction(0); + address impl = abi.decode(dataImpl, (address)); + + + // Deploy proxy + bytes memory creationCode = proxyCreationCode(impl); + submitToDeploy(creationCode, 1 ether, 1); + + vm.expectRevert(MultiSignatureWallet.ExecutionFailed.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(1); + } + + /// @dev Test to ensure 'deployContract' reverts if creation code is invalid. + function testDeployContractRevertsIfInvalidCreationCode() public { + // Deploy implementation + submitToDeploy(hex"f1", 0, 0); // Invalid creation code + + vm.expectRevert(MultiSignatureWallet.ExecutionFailed.selector); + + vm.prank(address(1002)); + multiSig.executeTransaction(0); + } } From 0aa778e09acef9e983194dc09f6c52e98558b66a Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Mon, 15 Dec 2025 15:42:16 +0530 Subject: [PATCH 03/12] added test cases for receive --- .../test/MultiSignatureWallet.t.sol | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/solidity/automation_registry/test/MultiSignatureWallet.t.sol b/solidity/automation_registry/test/MultiSignatureWallet.t.sol index 315b59d3d6..3240874b3a 100644 --- a/solidity/automation_registry/test/MultiSignatureWallet.t.sol +++ b/solidity/automation_registry/test/MultiSignatureWallet.t.sol @@ -22,6 +22,8 @@ contract MultiSignatureWalletTest is Test { /// @dev Sets up initial state for testing. /// @dev Deploys all required contracts. function setUp() public { + vm.deal(alice, 10 ether); + address owner1 = address(1001); address owner2 = address(1002); address owner3 = address(1003); @@ -787,4 +789,24 @@ contract MultiSignatureWalletTest is Test { vm.prank(address(1002)); multiSig.executeTransaction(0); } + + /// @dev Test to ensure 'receive' works correctly. + function testReceive() public { + assertEq(address(multiSig).balance, 0); + + // Send ETH to multisig + vm.prank(alice); + (bool success, ) = address(multiSig).call{value: 1 ether}(""); + assertTrue(success); + + assertEq(address(multiSig).balance, 1 ether); + } + + /// @dev Test to ensure 'receive' emits event 'Deposit'. + function testReceiveEmitsEvent() public { + vm.expectEmit(true, false, false, true); + emit MultiSignatureWallet.Deposit(alice, 1 ether, 1 ether); + + testReceive(); + } } From b5ec7e1f9573baec1d55c063dbb279a640fa329d Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Tue, 16 Dec 2025 12:55:34 +0530 Subject: [PATCH 04/12] -resolved PR comments --- .../script/DeployMultisig.s.sol | 11 +- solidity/automation_registry/src/Counter.sol | 32 ++++++ .../src/MultisigBeacon.sol | 3 +- .../test/MultiSignatureWallet.t.sol | 104 +++++++++--------- 4 files changed, 87 insertions(+), 63 deletions(-) create mode 100644 solidity/automation_registry/src/Counter.sol diff --git a/solidity/automation_registry/script/DeployMultisig.s.sol b/solidity/automation_registry/script/DeployMultisig.s.sol index bdc7c4c23b..771c5ca5fb 100644 --- a/solidity/automation_registry/script/DeployMultisig.s.sol +++ b/solidity/automation_registry/script/DeployMultisig.s.sol @@ -9,10 +9,12 @@ import {BeaconProxy} from "../lib/openzeppelin-contracts/contracts/proxy/beacon/ contract DeployMultisig is Script { address[] owners; uint256 numConfirmations; + address beaconOwner; function setUp() public { owners = vm.envAddress("OWNERS", ","); numConfirmations = vm.envUint("NUM_CONFIRMATIONS"); + beaconOwner = vm.envAddress("BEACON_OWNER"); } function run() public { @@ -27,8 +29,9 @@ contract DeployMultisig is Script { // ------------------------------------------- // Deploy beacon pointing to implementation // ------------------------------------------- - MultisigBeacon beacon = new MultisigBeacon(address(multisigImpl)); + MultisigBeacon beacon = new MultisigBeacon(address(multisigImpl), beaconOwner); console.log("Beacon deployed at: ", address(beacon)); + console.log("Beacon owner: ", beacon.owner()); // ---------------------- // Deploy multisig proxy @@ -43,12 +46,6 @@ contract DeployMultisig is Script { BeaconProxy multisigProxy = new BeaconProxy(address(beacon), initData); console.log("Multisig Proxy deployed at: ", address(multisigProxy)); - // ------------------------------------------ - // Transfer beacon's ownership to multisig - // ------------------------------------------ - beacon.transferOwnership(address(multisigProxy)); - console.log("Beacon ownership transferred to multisig proxy at: ", address(multisigProxy)); - vm.stopBroadcast(); } } \ No newline at end of file diff --git a/solidity/automation_registry/src/Counter.sol b/solidity/automation_registry/src/Counter.sol new file mode 100644 index 0000000000..e25bff2a59 --- /dev/null +++ b/solidity/automation_registry/src/Counter.sol @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import {OwnableUpgradeable} from "../lib/openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; +import {UUPSUpgradeable} from "../lib/openzeppelin-contracts/contracts/proxy/utils/UUPSUpgradeable.sol"; + +contract Counter is OwnableUpgradeable, UUPSUpgradeable { + uint256 public counter; + + /// @dev Disables the initialization for the implementation contract. + constructor() { + _disableInitializers(); + } + + /// @notice Initializes the owner of the contract. + function initialize() public initializer { + __Ownable_init(msg.sender); + } + + /// @notice Increments the counter by 1. + function increment() external onlyOwner { + counter = counter + 1; + } + + // ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: UPGRADEABILITY FUNCTIONS ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: + + /// @notice Helper function that reverts when 'msg.sender' is not authorized to upgrade the contract. + /// @dev called by 'upgradeTo' and 'upgradeToAndCall' in UUPSUpgradeable + /// @dev must be called by 'owner' + /// @param newImplementation address of the new implementation + function _authorizeUpgrade(address newImplementation) internal virtual override onlyOwner{ } +} diff --git a/solidity/automation_registry/src/MultisigBeacon.sol b/solidity/automation_registry/src/MultisigBeacon.sol index b8c9426ed6..d657a718ed 100644 --- a/solidity/automation_registry/src/MultisigBeacon.sol +++ b/solidity/automation_registry/src/MultisigBeacon.sol @@ -12,6 +12,7 @@ contract MultisigBeacon is UpgradeableBeacon { /** * @dev Constructor to initialize the addresses for implementation and initial owner. * @param _implementation Address of the initial multisig implementation contract. + * @param _owner Address of the Beacon owner. */ - constructor(address _implementation) UpgradeableBeacon(_implementation, msg.sender) {} + constructor(address _implementation, address _owner) UpgradeableBeacon(_implementation, _owner) {} } diff --git a/solidity/automation_registry/test/MultiSignatureWallet.t.sol b/solidity/automation_registry/test/MultiSignatureWallet.t.sol index 3240874b3a..2f70a1af42 100644 --- a/solidity/automation_registry/test/MultiSignatureWallet.t.sol +++ b/solidity/automation_registry/test/MultiSignatureWallet.t.sol @@ -1,16 +1,16 @@ -// SPDX-License-Identifier: UNLICENSED +// SPDX-License-Identifier: MIT pragma solidity 0.8.27; import {Test} from "forge-std/Test.sol"; -import {BlockMeta} from "../src/BlockMeta.sol"; +import {Counter} from "../src/Counter.sol"; import {ERC1967Proxy} from "../lib/openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import {BeaconProxy} from "../lib/openzeppelin-contracts/contracts/proxy/beacon/BeaconProxy.sol"; -import "../lib/openzeppelin-contracts-upgradeable/contracts/access/Ownable2StepUpgradeable.sol"; +import {OwnableUpgradeable} from "../lib/openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; import {MultiSignatureWallet} from "../src/MultiSignatureWallet.sol"; import "../src/MultisigBeacon.sol"; contract MultiSignatureWalletTest is Test { - BlockMeta blockMeta; + Counter counter; MultisigBeacon beacon; address multiSigImplV1; MultiSignatureWallet multiSig; @@ -38,31 +38,29 @@ contract MultiSignatureWalletTest is Test { vm.startPrank(alice); // Deploy Beacon contract multiSigImplV1 = address(new MultiSignatureWallet()); - beacon = new MultisigBeacon(multiSigImplV1); + beacon = new MultisigBeacon(multiSigImplV1, 0xE64Bd5C4810e6C7666C544a05c980C9Fe617283f); // Pre-determined address of multisigProxy // Deploy BeaconProxy for MultiSig bytes memory multiSigInitData = abi.encodeCall(MultiSignatureWallet.initialize, (owners, 4)); BeaconProxy multisigProxy = new BeaconProxy(address(beacon), multiSigInitData); multiSig = MultiSignatureWallet(payable(multisigProxy)); - // Transfer Beacon's ownership to multisig - beacon.transferOwnership(address(multisigProxy)); vm.stopPrank(); vm.startPrank(address(multisigProxy)); - // Deploy BlockMeta proxy contract - BlockMeta blockMetaImpl = new BlockMeta(); - bytes memory blockMetaInitData = abi.encodeCall(BlockMeta.initialize, ()); - ERC1967Proxy blockMetaProxy = new ERC1967Proxy(address(blockMetaImpl), blockMetaInitData); - blockMeta = BlockMeta(address(blockMetaProxy)); + // Deploy Counter proxy contract + Counter counterImpl = new Counter(); + bytes memory counterInitData = abi.encodeCall(Counter.initialize, ()); + ERC1967Proxy counterProxy = new ERC1967Proxy(address(counterImpl), counterInitData); + counter = Counter(address(counterProxy)); vm.stopPrank(); } /// @dev Test to ensure ownership and implementation address is initialized correctly. function testOwnerAndImplementation() public view { assertEq(beacon.owner(), address(multiSig)); - assertEq(blockMeta.owner(), address(multiSig)); + assertEq(counter.owner(), address(multiSig)); assertEq(beacon.implementation(), multiSigImplV1); } @@ -123,16 +121,16 @@ contract MultiSignatureWalletTest is Test { new BeaconProxy(address(beacon), initData); } - /// @dev Helper function that returns calldata to set automation controller in BlockMeta. - function dataToSetAutomationController(address _controller) private pure returns (bytes memory) { - return (abi.encodeCall(BlockMeta.setAutomationController, (_controller))); + /// @dev Helper function that returns calldata for 'increment' in Counter. + function dataForIncrement() private pure returns (bytes memory) { + return abi.encodeCall(Counter.increment, ()); } - /// @dev Helper function to submit a transaction to perform an action in the BlockMeta contract. + /// @dev Helper function to submit a transaction to perform an action in the Counter contract. function submitTransaction(bytes memory _data) private { vm.prank(address(1001)); multiSig.submitTransaction( - address(blockMeta), + address(counter), 0, 10000, _data @@ -140,13 +138,12 @@ contract MultiSignatureWalletTest is Test { } /// @dev Test to ensure 'submitTransaction' submits a transaction. - function testSubmitTransactionSetAutomationController() public { - BlockMeta impl = new BlockMeta(); - bytes memory data = dataToSetAutomationController(address(impl)); + function testSubmitTransactionIncrement() public { + bytes memory data = dataForIncrement(); submitTransaction(data); (address to, uint256 value, bool executed, uint24 numConfirmations, uint64 timeout, bytes memory storedData) = multiSig.getTransaction(0); - assertEq(to, address(blockMeta)); + assertEq(to, address(counter)); assertEq(value, 0); assertEq(executed, false); assertEq(numConfirmations, 1); @@ -155,15 +152,14 @@ contract MultiSignatureWalletTest is Test { } /// @dev Test to ensure 'submitTransaction' reverts if caller is not an owner. - function testSubmitTransactionSetAutomationControllerRevertsIfNotOwner() public { - BlockMeta impl = new BlockMeta(); - bytes memory data = dataToSetAutomationController(address(impl)); + function testSubmitTransactionIncrementRevertsIfNotOwner() public { + bytes memory data = dataForIncrement(); vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); vm.prank(alice); // Not an owner multiSig.submitTransaction( - address(blockMeta), + address(counter), 0, 100000, data @@ -184,8 +180,8 @@ contract MultiSignatureWalletTest is Test { } /// @dev Test to ensure 'confirmTransaction' confirms a transaction. - function testConfirmTransactionSetAutomationController() public { - testSubmitTransactionSetAutomationController(); + function testConfirmTransactionIncrement() public { + testSubmitTransactionIncrement(); grantSufficientConfirmations(0); @@ -195,7 +191,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'confirmTransaction' reverts if caller is not an owner. function testConfirmTransactionRevertsIfNotOwner() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); confirmTransaction(alice, 0); // Not an owner @@ -203,7 +199,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'confirmTransaction' reverts if transaction does not exist. function testConfirmTransactionRevertsIfTxDoesNotExist() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); confirmTransaction(address(1002), 1); @@ -211,7 +207,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'confirmTransaction' reverts if the transaction is already executed. function testConfirmTransactionRevertsIfTxAlreadyExecuted() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); uint256 txId = 0; grantSufficientConfirmations(txId); @@ -226,7 +222,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'confirmTransaction' reverts if transaction is already confirmed. function testConfirmTransactionRevertsIfTxAlreadyConfirmed() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.expectRevert(MultiSignatureWallet.TxnAlreadyConfirmed.selector); confirmTransaction(address(1001), 0); @@ -235,7 +231,7 @@ contract MultiSignatureWalletTest is Test { // @dev Test to ensure 'confirmTransaction' reverts if transaction has expired. function testConfirmTransactionRevertsIfTxExpired() public { vm.warp(500); - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.warp(10501); vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); @@ -251,7 +247,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'revokeConfirmation' revokes the confirmation of an owner. function testRevokeConfirmation() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); uint256 txId = 0; confirmTransaction(address(1002), txId); @@ -263,7 +259,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'revokeConfirmation' reverts if caller is not an owner. function testRevokeConfirmationRevertsIfNotOwner() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); revokeConfirmation(alice, 1); @@ -271,7 +267,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'revokeConfirmation' reverts if transaction does not exist. function testRevokeConfirmationRevertsIfTxDoesNotExist() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); revokeConfirmation(address(1001), 1); @@ -279,7 +275,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'revokeConfirmation' reverts if the transaction is already executed. function testRevokeConfirmationRevertsIfTxAlreadyExecuted() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); uint256 txId = 0; grantSufficientConfirmations(txId); @@ -294,7 +290,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'revokeConfirmation' reverts if the transaction has expired. function testRevokeConfirmationRevertsIfTxExpired() public { vm.warp(500); - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.warp(10501); vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); @@ -304,7 +300,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'revokeConfirmation' reverts if the transaction was not confirmed. function testRevokeConfirmationRevertsIfTxNotConfirmed() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.expectRevert(MultiSignatureWallet.TransactionNotConfirmed.selector); revokeConfirmation(address(1002), 0); @@ -312,7 +308,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'executeTransaction' executes a transaction. function testExecuteTransaction() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); uint256 txId = 0; grantSufficientConfirmations(txId); @@ -322,11 +318,12 @@ contract MultiSignatureWalletTest is Test { ( , , bool executed, , , ) = multiSig.getTransaction(txId); assertTrue(executed); + assertEq(counter.counter(), 1); } /// @dev Test to ensure 'executeTransaction' reverts if caller is not an owner. function testExecuteTransactionRevertsIfCallerNotOwner() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.expectRevert(MultiSignatureWallet.NotAnOwner.selector); @@ -355,7 +352,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'executeTransaction' reverts if transaction has expired. function testExecuteTransactionRevertsIfTxExpired() public { vm.warp(500); - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); vm.warp(10501); vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); @@ -366,7 +363,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'executeTransaction' reverts if the transaction has insufficient number of confirmations. function testExecuteTransactionRevertsIfInsufficientConfirmations() public { - testSubmitTransactionSetAutomationController(); + testSubmitTransactionIncrement(); uint256 txId = 0; confirmTransaction(address(1002), txId); @@ -380,7 +377,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Helper function that returns calldata to transfer ownership. function dataToTransferOwnership() private view returns (bytes memory) { - return abi.encodeCall(Ownable2StepUpgradeable.transferOwnership, (alice)); + return abi.encodeCall(OwnableUpgradeable.transferOwnership, (alice)); } /// @dev Test to ensure ownership transfer works correctly. @@ -391,10 +388,7 @@ contract MultiSignatureWalletTest is Test { vm.prank(address(1002)); multiSig.executeTransaction(0); - vm.prank(alice); - blockMeta.acceptOwnership(); - - assertEq(blockMeta.owner(), alice); + assertEq(counter.owner(), alice); } /// @dev Helper function to return calldata to add an owner in multisig. @@ -700,15 +694,15 @@ contract MultiSignatureWalletTest is Test { } /// @dev Helper function to submit a transaction for contract deployment and grant sufficient confirmations. - function submitToDeploy(bytes memory _creationCode, uint256 _value, uint256 _tx) private { + function submitToDeploy(bytes memory _creationCode, uint256 _value, uint256 _txIndex) private { bytes memory data = abi.encodeCall(MultiSignatureWallet.deployContract, (_creationCode, _value)); submitTransactionToMultiSig(data); - grantSufficientConfirmations(_tx); + grantSufficientConfirmations(_txIndex); } /// @dev Helper function that returns creation code to deploy ERC1967 proxy contract. function proxyCreationCode(address _impl) private pure returns (bytes memory) { - bytes memory initData = abi.encodeCall(BlockMeta.initialize, ()); + bytes memory initData = abi.encodeCall(Counter.initialize, ()); return abi.encodePacked( type(ERC1967Proxy).creationCode, @@ -719,7 +713,7 @@ contract MultiSignatureWalletTest is Test { /// @dev Test to ensure 'deployContract' deploys contract and assigns MultiSig as contract owner. function testDeployContract() public { // Deploy implementation - submitToDeploy(type(BlockMeta).creationCode, 0, 0); + submitToDeploy(type(Counter).creationCode, 0, 0); vm.prank(address(1002)); bytes memory dataImpl = multiSig.executeTransaction(0); @@ -733,12 +727,12 @@ contract MultiSignatureWalletTest is Test { vm.prank(address(1002)); bytes memory dataProxy = multiSig.executeTransaction(1); address proxy = abi.decode(dataProxy, (address)); - assertEq(BlockMeta(proxy).owner(), address(multiSig)); + assertEq(Counter(proxy).owner(), address(multiSig)); } /// @dev Test to ensure 'deployContract' reverts if caller is not MultiSig itself. function testDeployContractRevertsIfCallerNotMultiSig() public { - bytes memory creationCode = type(BlockMeta).creationCode; + bytes memory creationCode = type(Counter).creationCode; vm.expectRevert(MultiSignatureWallet.OnlyMultisigAccountCanCall.selector); @@ -762,7 +756,7 @@ contract MultiSignatureWalletTest is Test { vm.deal(address(multiSig), 4 ether); // Deploy implementation - submitToDeploy(type(BlockMeta).creationCode, 0, 0); + submitToDeploy(type(Counter).creationCode, 0, 0); vm.prank(address(1002)); bytes memory dataImpl = multiSig.executeTransaction(0); From ebe93a858ee1698967c3e9404d42adfd54909846 Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Tue, 16 Dec 2025 15:09:20 +0530 Subject: [PATCH 05/12] moved Counter to tests --- solidity/automation_registry/{src => test}/Counter.sol | 0 solidity/automation_registry/test/MultiSignatureWallet.t.sol | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename solidity/automation_registry/{src => test}/Counter.sol (100%) diff --git a/solidity/automation_registry/src/Counter.sol b/solidity/automation_registry/test/Counter.sol similarity index 100% rename from solidity/automation_registry/src/Counter.sol rename to solidity/automation_registry/test/Counter.sol diff --git a/solidity/automation_registry/test/MultiSignatureWallet.t.sol b/solidity/automation_registry/test/MultiSignatureWallet.t.sol index 2f70a1af42..a2558922ac 100644 --- a/solidity/automation_registry/test/MultiSignatureWallet.t.sol +++ b/solidity/automation_registry/test/MultiSignatureWallet.t.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.27; import {Test} from "forge-std/Test.sol"; -import {Counter} from "../src/Counter.sol"; +import {Counter} from "./Counter.sol"; import {ERC1967Proxy} from "../lib/openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import {BeaconProxy} from "../lib/openzeppelin-contracts/contracts/proxy/beacon/BeaconProxy.sol"; import {OwnableUpgradeable} from "../lib/openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; From 20a55b8db60738769a9d870e5f84017b7d4af043 Mon Sep 17 00:00:00 2001 From: Aregnaz Harutyunyan <> Date: Tue, 16 Dec 2025 19:07:06 +0400 Subject: [PATCH 06/12] Renamed solidity/automation_registry -> solidity/supra_contracts --- .../script/DeployMultisig.s.sol | 0 .../src/MultiSignatureWallet.sol | 0 .../src/MultisigBeacon.sol | 0 .../{automation_registry => supra_contracts}/test/Counter.sol | 0 .../test/MultiSignatureWallet.t.sol | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename solidity/{automation_registry => supra_contracts}/script/DeployMultisig.s.sol (100%) rename solidity/{automation_registry => supra_contracts}/src/MultiSignatureWallet.sol (100%) rename solidity/{automation_registry => supra_contracts}/src/MultisigBeacon.sol (100%) rename solidity/{automation_registry => supra_contracts}/test/Counter.sol (100%) rename solidity/{automation_registry => supra_contracts}/test/MultiSignatureWallet.t.sol (100%) diff --git a/solidity/automation_registry/script/DeployMultisig.s.sol b/solidity/supra_contracts/script/DeployMultisig.s.sol similarity index 100% rename from solidity/automation_registry/script/DeployMultisig.s.sol rename to solidity/supra_contracts/script/DeployMultisig.s.sol diff --git a/solidity/automation_registry/src/MultiSignatureWallet.sol b/solidity/supra_contracts/src/MultiSignatureWallet.sol similarity index 100% rename from solidity/automation_registry/src/MultiSignatureWallet.sol rename to solidity/supra_contracts/src/MultiSignatureWallet.sol diff --git a/solidity/automation_registry/src/MultisigBeacon.sol b/solidity/supra_contracts/src/MultisigBeacon.sol similarity index 100% rename from solidity/automation_registry/src/MultisigBeacon.sol rename to solidity/supra_contracts/src/MultisigBeacon.sol diff --git a/solidity/automation_registry/test/Counter.sol b/solidity/supra_contracts/test/Counter.sol similarity index 100% rename from solidity/automation_registry/test/Counter.sol rename to solidity/supra_contracts/test/Counter.sol diff --git a/solidity/automation_registry/test/MultiSignatureWallet.t.sol b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol similarity index 100% rename from solidity/automation_registry/test/MultiSignatureWallet.t.sol rename to solidity/supra_contracts/test/MultiSignatureWallet.t.sol From dacec734b454e789bc20c20de95f2dd07e0ca75a Mon Sep 17 00:00:00 2001 From: Aregnaz Harutyunyan <> Date: Tue, 16 Dec 2025 19:27:29 +0400 Subject: [PATCH 07/12] Added missing files for build and tests --- .gitmodules | 9 +++ solidity/supra_contracts/README.md | 73 +++++++++++++++++++ solidity/supra_contracts/foundry.lock | 20 +++++ solidity/supra_contracts/foundry.toml | 8 ++ solidity/supra_contracts/lib/forge-std | 1 + .../lib/openzeppelin-contracts | 1 + .../lib/openzeppelin-contracts-upgradeable | 1 + 7 files changed, 113 insertions(+) create mode 100644 solidity/supra_contracts/README.md create mode 100644 solidity/supra_contracts/foundry.lock create mode 100644 solidity/supra_contracts/foundry.toml create mode 160000 solidity/supra_contracts/lib/forge-std create mode 160000 solidity/supra_contracts/lib/openzeppelin-contracts create mode 160000 solidity/supra_contracts/lib/openzeppelin-contracts-upgradeable diff --git a/.gitmodules b/.gitmodules index e69de29bb2..ed45310f57 100644 --- a/.gitmodules +++ b/.gitmodules @@ -0,0 +1,9 @@ +[submodule "solidity/supra_contracts/lib/openzeppelin-contracts"] + path = solidity/supra_contracts/lib/openzeppelin-contracts + url = https://github.com/OpenZeppelin/openzeppelin-contracts +[submodule "solidity/supra_contracts/lib/openzeppelin-contracts-upgradeable"] + path = solidity/supra_contracts/lib/openzeppelin-contracts-upgradeable + url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable +[submodule "solidity/supra_contracts/lib/forge-std"] + path = solidity/supra_contracts/lib/forge-std + url = https://github.com/foundry-rs/forge-std diff --git a/solidity/supra_contracts/README.md b/solidity/supra_contracts/README.md new file mode 100644 index 0000000000..53ae762878 --- /dev/null +++ b/solidity/supra_contracts/README.md @@ -0,0 +1,73 @@ +## Supra EVM Automation Registry + +**This repository includes Supra EVM Automation Registry contract and related contracts.** + +Foundry consists of: + +- **Forge**: Ethereum testing framework (like Truffle, Hardhat and DappTools). +- **Cast**: Swiss army knife for interacting with EVM smart contracts, sending transactions and getting chain data. +- **Anvil**: Local Ethereum node, akin to Ganache, Hardhat Network. +- **Chisel**: Fast, utilitarian, and verbose solidity REPL. + +## Documentation + +https://book.getfoundry.sh/ + +## Usage + +### Install dependencies + +``` +forge install OpenZeppelin/openzeppelin-contracts +forge install OpenZeppelin/openzeppelin-contracts-upgradeable +``` + +### Build + +```shell +$ forge build +``` + +### Test + +```shell +$ forge test +``` + +### Format + +```shell +$ forge fmt +``` + +### Gas Snapshots + +```shell +$ forge snapshot +``` + +### Anvil + +```shell +$ anvil +``` + +### Deploy + +```shell +$ forge script script/Counter.s.sol:CounterScript --rpc-url --private-key +``` + +### Cast + +```shell +$ cast +``` + +### Help + +```shell +$ forge --help +$ anvil --help +$ cast --help +``` diff --git a/solidity/supra_contracts/foundry.lock b/solidity/supra_contracts/foundry.lock new file mode 100644 index 0000000000..977ce84399 --- /dev/null +++ b/solidity/supra_contracts/foundry.lock @@ -0,0 +1,20 @@ +{ + "lib/forge-std": { + "tag": { + "name": "v1.12.0", + "rev": "7117c90c8cf6c68e5acce4f09a6b24715cea4de6" + } + }, + "lib/openzeppelin-contracts": { + "tag": { + "name": "v5.5.0", + "rev": "fcbae5394ae8ad52d8e580a3477db99814b9d565" + } + }, + "lib/openzeppelin-contracts-upgradeable": { + "tag": { + "name": "v5.5.0", + "rev": "aa677e9d28ed78fc427ec47ba2baef2030c58e7c" + } + } +} \ No newline at end of file diff --git a/solidity/supra_contracts/foundry.toml b/solidity/supra_contracts/foundry.toml new file mode 100644 index 0000000000..eb22be94ce --- /dev/null +++ b/solidity/supra_contracts/foundry.toml @@ -0,0 +1,8 @@ +[profile.default] +src = "src" +out = "out" +libs = ["lib"] +via_ir = true +optimizer = true + +# See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options diff --git a/solidity/supra_contracts/lib/forge-std b/solidity/supra_contracts/lib/forge-std new file mode 160000 index 0000000000..27ba11c86a --- /dev/null +++ b/solidity/supra_contracts/lib/forge-std @@ -0,0 +1 @@ +Subproject commit 27ba11c86ac93d8d4a50437ae26621468fe63c20 diff --git a/solidity/supra_contracts/lib/openzeppelin-contracts b/solidity/supra_contracts/lib/openzeppelin-contracts new file mode 160000 index 0000000000..353f564d1d --- /dev/null +++ b/solidity/supra_contracts/lib/openzeppelin-contracts @@ -0,0 +1 @@ +Subproject commit 353f564d1db53c1d30cfa8a631771c205e41107b diff --git a/solidity/supra_contracts/lib/openzeppelin-contracts-upgradeable b/solidity/supra_contracts/lib/openzeppelin-contracts-upgradeable new file mode 160000 index 0000000000..c1f5d81e2f --- /dev/null +++ b/solidity/supra_contracts/lib/openzeppelin-contracts-upgradeable @@ -0,0 +1 @@ +Subproject commit c1f5d81e2f53599bc9e4653bbc7c126032c96bd1 From 2ccf370de9eded48373cbbb3726e8be20e57f8c2 Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Wed, 17 Dec 2025 11:19:24 +0530 Subject: [PATCH 08/12] updated .gitignore and import statment in test file --- .gitignore | 7 ++++++- solidity/supra_contracts/test/MultiSignatureWallet.t.sol | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 10e3bcae4d..a6604dc95b 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,11 @@ target .vscode .idea pkg/ +cache +out +broadcast +.env +all-chain.json bins/revme/temp_folder bins/revme/tests @@ -25,4 +30,4 @@ rustc-ice-* /index.html # Fixtures -/test-fixtures +/test-fixtures \ No newline at end of file diff --git a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol index a2558922ac..80f668e55c 100644 --- a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol +++ b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol @@ -7,7 +7,7 @@ import {ERC1967Proxy} from "../lib/openzeppelin-contracts/contracts/proxy/ERC196 import {BeaconProxy} from "../lib/openzeppelin-contracts/contracts/proxy/beacon/BeaconProxy.sol"; import {OwnableUpgradeable} from "../lib/openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; import {MultiSignatureWallet} from "../src/MultiSignatureWallet.sol"; -import "../src/MultisigBeacon.sol"; +import {MultisigBeacon, UpgradeableBeacon} from "../src/MultisigBeacon.sol"; contract MultiSignatureWalletTest is Test { Counter counter; From fa6c0fb9d540aef1971716ecb743d5b3e10d87fd Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Mon, 22 Dec 2025 14:33:39 +0530 Subject: [PATCH 09/12] updated Counter and tests associated with it --- solidity/supra_contracts/test/Counter.sol | 13 +++++++++---- .../supra_contracts/test/MultiSignatureWallet.t.sol | 6 +++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/solidity/supra_contracts/test/Counter.sol b/solidity/supra_contracts/test/Counter.sol index e25bff2a59..a74d98c018 100644 --- a/solidity/supra_contracts/test/Counter.sol +++ b/solidity/supra_contracts/test/Counter.sol @@ -6,20 +6,25 @@ import {UUPSUpgradeable} from "../lib/openzeppelin-contracts/contracts/proxy/uti contract Counter is OwnableUpgradeable, UUPSUpgradeable { uint256 public counter; + address public privilegedAddress; /// @dev Disables the initialization for the implementation contract. constructor() { _disableInitializers(); } - /// @notice Initializes the owner of the contract. - function initialize() public initializer { + /// @notice Initializes the owner and privileged address of the contract. + /// @param _privileged Privileged address. + function initialize(address _privileged) public initializer { + privilegedAddress = _privileged; __Ownable_init(msg.sender); } /// @notice Increments the counter by 1. - function increment() external onlyOwner { - counter = counter + 1; + function increment() external { + if (msg.sender == privilegedAddress) { + counter = counter + 1; + } } // ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: UPGRADEABILITY FUNCTIONS ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: diff --git a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol index 80f668e55c..77f8720c83 100644 --- a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol +++ b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol @@ -51,7 +51,7 @@ contract MultiSignatureWalletTest is Test { vm.startPrank(address(multisigProxy)); // Deploy Counter proxy contract Counter counterImpl = new Counter(); - bytes memory counterInitData = abi.encodeCall(Counter.initialize, ()); + bytes memory counterInitData = abi.encodeCall(Counter.initialize, (address(multiSig))); ERC1967Proxy counterProxy = new ERC1967Proxy(address(counterImpl), counterInitData); counter = Counter(address(counterProxy)); vm.stopPrank(); @@ -701,8 +701,8 @@ contract MultiSignatureWalletTest is Test { } /// @dev Helper function that returns creation code to deploy ERC1967 proxy contract. - function proxyCreationCode(address _impl) private pure returns (bytes memory) { - bytes memory initData = abi.encodeCall(Counter.initialize, ()); + function proxyCreationCode(address _impl) private view returns (bytes memory) { + bytes memory initData = abi.encodeCall(Counter.initialize, (address(multiSig))); return abi.encodePacked( type(ERC1967Proxy).creationCode, From 38d1d173a7fa21a75eddca515ceddea7fe250efa Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Tue, 23 Dec 2025 16:11:06 +0530 Subject: [PATCH 10/12] added transaction deletion --- .../src/MultiSignatureWallet.sol | 89 ++++++++++++------- .../test/MultiSignatureWallet.t.sol | 35 ++++++-- 2 files changed, 86 insertions(+), 38 deletions(-) diff --git a/solidity/supra_contracts/src/MultiSignatureWallet.sol b/solidity/supra_contracts/src/MultiSignatureWallet.sol index 302ccae25a..b7ac070ce3 100644 --- a/solidity/supra_contracts/src/MultiSignatureWallet.sol +++ b/solidity/supra_contracts/src/MultiSignatureWallet.sol @@ -119,6 +119,11 @@ contract MultiSignatureWallet is Initializable { */ error InvalidOwner(); + /** + * @dev Error for when address(0) is passed as recipient while submitting a transaction. + */ + error InvalidRecipient(); + /** * @dev Error for when a duplicate owner address is provided. */ @@ -172,11 +177,17 @@ contract MultiSignatureWallet is Initializable { bytes data; // Data payload of the transaction } - // Mapping to track confirmations for each transaction by each owner - mapping(uint256 transactionIndex => mapping(address owner => bool permissionToExecute)) public isConfirmed; + // Mapping to track confirmations for each transaction. + mapping(uint256 => EnumerableSet.AddressSet) private confirmations; - // Array to store all transactions - Transaction[] private transactions; + // Mapping from transaction index to Transaction + mapping(uint256 => Transaction) private transactions; + + // Auto-incrementing transaction index + uint256 private txIndex; + + // Number of active transactions + uint256 public txCount; // Function to ensure the caller is an owner function onlyOwner(address owner) private view { @@ -193,7 +204,7 @@ contract MultiSignatureWallet is Initializable { // Function to check if a transaction exists function txExists(uint256 _txIndex) private view { - if (_txIndex >= transactions.length) + if (transactions[_txIndex].to == address(0)) revert InvalidTxnId(); } @@ -211,7 +222,7 @@ contract MultiSignatureWallet is Initializable { // Function to check if a transaction has not been confirmed by the caller function notConfirmed(uint256 _txIndex) private view { - if (isConfirmed[_txIndex][msg.sender]) revert TxnAlreadyConfirmed(); + if (confirmations[_txIndex].contains(msg.sender)) revert TxnAlreadyConfirmed(); } /** @@ -263,23 +274,25 @@ contract MultiSignatureWallet is Initializable { bytes memory _data ) external payable { onlyOwner(msg.sender); - uint256 txIndex = transactions.length; - - transactions.push( - Transaction({ - to: _to, - executed: false, - timeout: uint64(block.timestamp) + _timeoutDuration, - //We assume the act of submission is an implicit confirmation - numConfirmations: 1, - value: _value, - data: _data - }) - ); + if (_to == address(0)) revert InvalidRecipient(); + + uint256 currentTxIndex = txIndex; + + transactions[currentTxIndex] = Transaction({ + to: _to, + executed: false, + timeout: uint64(block.timestamp) + _timeoutDuration, + //We assume the act of submission is an implicit confirmation + numConfirmations: 1, + value: _value, + data: _data + }); - isConfirmed[txIndex][msg.sender] = true; + confirmations[currentTxIndex].add(msg.sender); + txIndex++; + txCount++; - emit SubmitTransaction(msg.sender, txIndex, _to, _value, _data); + emit SubmitTransaction(msg.sender, currentTxIndex, _to, _value, _data); } /** @@ -294,7 +307,7 @@ contract MultiSignatureWallet is Initializable { txNotExpired(_txIndex); Transaction storage transaction = transactions[_txIndex]; transaction.numConfirmations += 1; - isConfirmed[_txIndex][msg.sender] = true; + confirmations[_txIndex].add(msg.sender); emit ConfirmTransaction(msg.sender, _txIndex); } @@ -308,10 +321,21 @@ contract MultiSignatureWallet is Initializable { txExists(_txIndex); notExecuted(_txIndex); txNotExpired(_txIndex); - Transaction storage transaction = transactions[_txIndex]; + Transaction memory transaction = transactions[_txIndex]; if (transaction.numConfirmations < numConfirmationsRequired) revert NotEnoughConfirmation(); - transaction.executed = true; + + // Remove the transaction from storage + delete transactions[_txIndex]; + + // Clear all confirmations + confirmations[_txIndex].clear(); + + // Remove confirmations mapping + delete confirmations[_txIndex]; + + txCount--; + (bool success, bytes memory data) = transaction.to.call{value: transaction.value}(transaction.data); if (!success) { revert ExecutionFailed(); } @@ -328,14 +352,12 @@ contract MultiSignatureWallet is Initializable { txExists(_txIndex); notExecuted(_txIndex); txNotExpired(_txIndex); - if (!isConfirmed[_txIndex][msg.sender]) { - revert TransactionNotConfirmed(); - } + if (!confirmations[_txIndex].contains(msg.sender)) revert TransactionNotConfirmed(); Transaction storage transaction = transactions[_txIndex]; transaction.numConfirmations -= 1; - isConfirmed[_txIndex][msg.sender] = false; + confirmations[_txIndex].remove(msg.sender); emit RevokeConfirmation(msg.sender, _txIndex); } @@ -410,11 +432,13 @@ contract MultiSignatureWallet is Initializable { } /** - * @dev Function to retrieve the count of transactions submitted to the wallet. - * @return Total number of transactions in the wallet. + * @dev Checks if a transaction is confirmed by an owner. + * @param _txIndex Index of the transaction to check for. + * @param _owner Address of the owner. */ - function getTransactionCount() public view returns (uint256) { - return transactions.length; + function isConfirmed(uint256 _txIndex, address _owner) external view returns (bool) { + txExists(_txIndex); + return confirmations[_txIndex].contains(_owner); } /** @@ -441,6 +465,7 @@ contract MultiSignatureWallet is Initializable { bytes memory data ) { + txExists(_txIndex); Transaction storage transaction = transactions[_txIndex]; return ( diff --git a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol index 77f8720c83..fe8f10eb48 100644 --- a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol +++ b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol @@ -64,10 +64,11 @@ contract MultiSignatureWalletTest is Test { assertEq(beacon.implementation(), multiSigImplV1); } - /// @dev Test to ensure owners and number of confirmations required are initialized correctly. + /// @dev Test to ensure contract is initialized correctly. function testInitialize() public view { assertEq(multiSig.getOwners(), owners); assertEq(multiSig.numConfirmationsRequired(), 4); + assertEq(multiSig.txCount(), 0); } /// @dev Test to ensure 'initialize' reverts if array of owners is empty. @@ -149,6 +150,7 @@ contract MultiSignatureWalletTest is Test { assertEq(numConfirmations, 1); assertEq(timeout, block.timestamp + 10000); assertEq(storedData, data); + assertEq(multiSig.txCount(), 1); } /// @dev Test to ensure 'submitTransaction' reverts if caller is not an owner. @@ -166,6 +168,21 @@ contract MultiSignatureWalletTest is Test { ); } + /// @dev Test to ensure 'submitTransaction' reverts if address(0) is passed as recipient. + function testSubmitTransactionIncrementRevertsIfAddressZero() public { + bytes memory data = dataForIncrement(); + + vm.expectRevert(MultiSignatureWallet.InvalidRecipient.selector); + + vm.prank(address(1001)); + multiSig.submitTransaction( + address(0), + 0, + 100000, + data + ); + } + /// @dev Helper function to confirm a transaction. function confirmTransaction(address _owner, uint256 _txnId) private { vm.prank(_owner); @@ -215,7 +232,7 @@ contract MultiSignatureWalletTest is Test { vm.prank(address(1002)); multiSig.executeTransaction(txId); - vm.expectRevert(MultiSignatureWallet.TxnAlreadyExecuted.selector); + vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); confirmTransaction(address(1005), txId); } @@ -255,6 +272,7 @@ contract MultiSignatureWalletTest is Test { ( , , , uint256 confirmations , , ) = multiSig.getTransaction(txId); assertEq(confirmations, 1); + assertFalse(multiSig.isConfirmed(txId, address(1001))); } /// @dev Test to ensure 'revokeConfirmation' reverts if caller is not an owner. @@ -283,7 +301,7 @@ contract MultiSignatureWalletTest is Test { vm.prank(address(1002)); multiSig.executeTransaction(txId); - vm.expectRevert(MultiSignatureWallet.TxnAlreadyExecuted.selector); + vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); revokeConfirmation(address(1001), txId); } @@ -316,8 +334,7 @@ contract MultiSignatureWalletTest is Test { vm.prank(address(1001)); multiSig.executeTransaction(txId); - ( , , bool executed, , , ) = multiSig.getTransaction(txId); - assertTrue(executed); + assertEq(multiSig.txCount(), 0); assertEq(counter.counter(), 1); } @@ -343,7 +360,7 @@ contract MultiSignatureWalletTest is Test { function testExecuteTransactionRevertsIfTxAlreadyExecuted() public { testExecuteTransaction(); - vm.expectRevert(MultiSignatureWallet.TxnAlreadyExecuted.selector); + vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); vm.prank(address(1002)); multiSig.executeTransaction(0); @@ -803,4 +820,10 @@ contract MultiSignatureWalletTest is Test { testReceive(); } + + /// @dev Test to ensure 'getTransaction' reverts if transaction does not exist. + function testGetTransactionRevertsIfTxDoesNotExist() public { + vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); + multiSig.getTransaction(0); + } } From 42cf8825c8f8bc78346e2e0f9c1388f4ba5a6b15 Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Tue, 30 Dec 2025 19:11:56 +0530 Subject: [PATCH 11/12] updated to remove expired txs --- .../src/MultiSignatureWallet.sol | 85 ++++++++++--------- .../test/MultiSignatureWallet.t.sol | 51 +++++++---- 2 files changed, 81 insertions(+), 55 deletions(-) diff --git a/solidity/supra_contracts/src/MultiSignatureWallet.sol b/solidity/supra_contracts/src/MultiSignatureWallet.sol index b7ac070ce3..70d6db7236 100644 --- a/solidity/supra_contracts/src/MultiSignatureWallet.sol +++ b/solidity/supra_contracts/src/MultiSignatureWallet.sol @@ -22,7 +22,7 @@ contract MultiSignatureWallet is Initializable { /** * @dev Emitted when a new transaction is submitted. * @param owner The address of the owner who submitted the transaction. - * @param txIndex The index of the transaction in the transactions array. + * @param txIndex The index of the transaction. * @param to The contract address the transaction is directed to. * @param value The amount of ether to be sent with the transaction. * @param data The data payload of the transaction. @@ -35,24 +35,30 @@ contract MultiSignatureWallet is Initializable { bytes data ); + /** + * @dev Emitted when a transaction is expired. + * @param txIndex The index of the expired transaction. + */ + event TransactionExpired(uint256 indexed txIndex); + /** * @dev Emitted when a transaction is confirmed by an owner. * @param owner The address of the owner who confirmed the transaction. - * @param txIndex The index of the transaction in the transactions array. + * @param txIndex The index of the transaction. */ event ConfirmTransaction(address indexed owner, uint256 indexed txIndex); /** * @dev Emitted when a confirmation is revoked by an owner. * @param owner The address of the owner who revoked the confirmation. - * @param txIndex The index of the transaction in the transactions array. + * @param txIndex The index of the transaction. */ event RevokeConfirmation(address indexed owner, uint256 indexed txIndex); /** * @dev Emitted when a transaction is executed. * @param owner The address of the owner who executed the transaction. - * @param txIndex The index of the transaction in the transactions array. + * @param txIndex The index of the transaction. * @param txData The data returned by the transaction call. */ event ExecuteTransaction(address indexed owner, uint256 indexed txIndex, bytes txData); @@ -154,11 +160,6 @@ contract MultiSignatureWallet is Initializable { */ error TransactionNotConfirmed(); - /** - * @dev Error for when a transaction has already expired. - */ - error TransactionAlreadyExpired(); - /** * @dev Error for when a function is called by an account other than the multisig wallet itself. */ @@ -170,7 +171,6 @@ contract MultiSignatureWallet is Initializable { // Structure to hold transaction details struct Transaction { address to; // Transaction target address - bool executed; // Flag indicating if the transaction has been executed uint64 timeout; // Expiry timestamp of the transaction uint24 numConfirmations; // Number of confirmations received for the transaction uint256 value; // Amount of ether sent with the transaction @@ -208,16 +208,29 @@ contract MultiSignatureWallet is Initializable { revert InvalidTxnId(); } - // Function to check if a transaction has not been executed - function notExecuted(uint256 _txIndex) private view { - if (transactions[_txIndex].executed) - revert TxnAlreadyExecuted(); + /// @dev Helper function to remove a transaction and emit an event if it is expired. + /// @param _txIndex Index of the transaction. + /// @return bool True if the transaction was expired and removed. + function cleanupIfExpired(uint256 _txIndex) private returns (bool) { + if (transactions[_txIndex].timeout < block.timestamp) { + removeTransaction(_txIndex); + emit TransactionExpired(_txIndex); + + return true; + } + return false; } - // Function to check if a transaction has not been expired or not - function txNotExpired(uint256 _txIndex) private view { - if (transactions[_txIndex].timeout < block.timestamp) - revert TransactionAlreadyExpired(); + /// @dev Helper function to remove a transaction from the storage. + /// @param _txIndex Index of the transaction to remove. + function removeTransaction(uint256 _txIndex) private { + // Remove the transaction from storage + delete transactions[_txIndex]; + + // Remove confirmations mapping + delete confirmations[_txIndex]; + + txCount--; } // Function to check if a transaction has not been confirmed by the caller @@ -280,7 +293,6 @@ contract MultiSignatureWallet is Initializable { transactions[currentTxIndex] = Transaction({ to: _to, - executed: false, timeout: uint64(block.timestamp) + _timeoutDuration, //We assume the act of submission is an implicit confirmation numConfirmations: 1, @@ -297,14 +309,17 @@ contract MultiSignatureWallet is Initializable { /** * @dev Function to confirm an existing transaction. + * @dev If the transaction is expired, it is deleted and TransactionExpired is emitted. * @param _txIndex Index of the transaction to confirm. */ function confirmTransaction(uint256 _txIndex) public { onlyOwner(msg.sender); txExists(_txIndex); - notExecuted(_txIndex); notConfirmed(_txIndex); - txNotExpired(_txIndex); + if (cleanupIfExpired(_txIndex)) { + // Transaction expired, action is no longer applicable + return; + } Transaction storage transaction = transactions[_txIndex]; transaction.numConfirmations += 1; confirmations[_txIndex].add(msg.sender); @@ -314,27 +329,21 @@ contract MultiSignatureWallet is Initializable { /** * @dev Function to execute a confirmed transaction. + * @dev If the transaction is expired, it is deleted and TransactionExpired is emitted. * @param _txIndex Index of the transaction to execute. */ function executeTransaction(uint256 _txIndex) public returns (bytes memory) { onlyOwner(msg.sender); txExists(_txIndex); - notExecuted(_txIndex); - txNotExpired(_txIndex); + if (cleanupIfExpired(_txIndex)) { + // Transaction expired, action is no longer applicable + return bytes(""); + } Transaction memory transaction = transactions[_txIndex]; if (transaction.numConfirmations < numConfirmationsRequired) revert NotEnoughConfirmation(); - // Remove the transaction from storage - delete transactions[_txIndex]; - - // Clear all confirmations - confirmations[_txIndex].clear(); - - // Remove confirmations mapping - delete confirmations[_txIndex]; - - txCount--; + removeTransaction(_txIndex); (bool success, bytes memory data) = transaction.to.call{value: transaction.value}(transaction.data); if (!success) { revert ExecutionFailed(); } @@ -345,13 +354,16 @@ contract MultiSignatureWallet is Initializable { /** * @dev Function to revoke a previously given confirmation for a transaction. + * @dev If the transaction is expired, it is deleted and TransactionExpired is emitted. * @param _txIndex Index of the transaction to revoke confirmation. */ function revokeConfirmation(uint256 _txIndex) external { onlyOwner(msg.sender); txExists(_txIndex); - notExecuted(_txIndex); - txNotExpired(_txIndex); + if (cleanupIfExpired(_txIndex)) { + // Transaction expired, action is no longer applicable + return; + } if (!confirmations[_txIndex].contains(msg.sender)) revert TransactionNotConfirmed(); Transaction storage transaction = transactions[_txIndex]; @@ -446,7 +458,6 @@ contract MultiSignatureWallet is Initializable { * @param _txIndex Index of the transaction to retrieve details for. * @return to Transaction target address. * @return value Amount of ether sent with the transaction. - * @return executed Boolean indicating if the transaction has been executed. * @return numConfirmations Number of confirmations received for the transaction. * @return timeout Expiry timestamp of the transaction. * @return data Data payload of the transaction. @@ -459,7 +470,6 @@ contract MultiSignatureWallet is Initializable { returns ( address to, uint256 value, - bool executed, uint24 numConfirmations, uint64 timeout, bytes memory data @@ -471,7 +481,6 @@ contract MultiSignatureWallet is Initializable { return ( transaction.to, transaction.value, - transaction.executed, transaction.numConfirmations, transaction.timeout, transaction.data diff --git a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol index fe8f10eb48..76a3bed034 100644 --- a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol +++ b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol @@ -143,10 +143,9 @@ contract MultiSignatureWalletTest is Test { bytes memory data = dataForIncrement(); submitTransaction(data); - (address to, uint256 value, bool executed, uint24 numConfirmations, uint64 timeout, bytes memory storedData) = multiSig.getTransaction(0); + (address to, uint256 value, uint24 numConfirmations, uint64 timeout, bytes memory storedData) = multiSig.getTransaction(0); assertEq(to, address(counter)); assertEq(value, 0); - assertEq(executed, false); assertEq(numConfirmations, 1); assertEq(timeout, block.timestamp + 10000); assertEq(storedData, data); @@ -202,7 +201,7 @@ contract MultiSignatureWalletTest is Test { grantSufficientConfirmations(0); - ( , , , uint256 numConfirmations, , ) = multiSig.getTransaction(0); + ( , , uint256 numConfirmations, , ) = multiSig.getTransaction(0); assertEq(numConfirmations, 4); } @@ -245,15 +244,18 @@ contract MultiSignatureWalletTest is Test { confirmTransaction(address(1001), 0); } - // @dev Test to ensure 'confirmTransaction' reverts if transaction has expired. + /// @dev Test to ensure 'confirmTransaction' removes the tx and emits 'TransactionExpired' if transaction has expired. function testConfirmTransactionRevertsIfTxExpired() public { vm.warp(500); testSubmitTransactionIncrement(); + assertEq(multiSig.txCount(), 1); vm.warp(10501); - vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + vm.expectEmit(true, false, false, false); + emit MultiSignatureWallet.TransactionExpired(0); confirmTransaction(address(1005), 0); + assertEq(multiSig.txCount(), 0); } /// @dev Helper function to revoke confirmation. @@ -270,7 +272,7 @@ contract MultiSignatureWalletTest is Test { confirmTransaction(address(1002), txId); revokeConfirmation(address(1001), txId); - ( , , , uint256 confirmations , , ) = multiSig.getTransaction(txId); + ( , , uint256 confirmations , , ) = multiSig.getTransaction(txId); assertEq(confirmations, 1); assertFalse(multiSig.isConfirmed(txId, address(1001))); } @@ -305,15 +307,18 @@ contract MultiSignatureWalletTest is Test { revokeConfirmation(address(1001), txId); } - /// @dev Test to ensure 'revokeConfirmation' reverts if the transaction has expired. + /// @dev Test to ensure 'revokeConfirmation' removes the tx and emits 'TransactionExpired' if the transaction has expired. function testRevokeConfirmationRevertsIfTxExpired() public { vm.warp(500); testSubmitTransactionIncrement(); + assertEq(multiSig.txCount(), 1); vm.warp(10501); - vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + vm.expectEmit(true, false, false, false); + emit MultiSignatureWallet.TransactionExpired(0); revokeConfirmation(address(1001), 0); + assertEq(multiSig.txCount(), 0); } /// @dev Test to ensure 'revokeConfirmation' reverts if the transaction was not confirmed. @@ -366,16 +371,19 @@ contract MultiSignatureWalletTest is Test { multiSig.executeTransaction(0); } - /// @dev Test to ensure 'executeTransaction' reverts if transaction has expired. + /// @dev Test to ensure 'executeTransaction' removes the tx and emits 'TransactionExpired' if transaction has expired. function testExecuteTransactionRevertsIfTxExpired() public { vm.warp(500); testSubmitTransactionIncrement(); + assertEq(multiSig.txCount(), 1); vm.warp(10501); - vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + vm.expectEmit(true, false, false, false); + emit MultiSignatureWallet.TransactionExpired(0); vm.prank(address(1002)); multiSig.executeTransaction(0); + assertEq(multiSig.txCount(), 0); } /// @dev Test to ensure 'executeTransaction' reverts if the transaction has insufficient number of confirmations. @@ -476,18 +484,21 @@ contract MultiSignatureWalletTest is Test { multiSig.executeTransaction(0); } - /// @dev Test to ensure 'addOwners' reverts if transaction has expired. + /// @dev Test to ensure 'addOwners' removes the tx and emits 'TransactionExpired' if transaction has expired. function testAddOwnersRevertsIfTimestampExpired() public { vm.warp(500); submitTransactionToMultiSig(dataToAddOwnerInMultiSig()); + assertEq(multiSig.txCount(), 1); grantSufficientConfirmations(0); vm.warp(10501); - vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + vm.expectEmit(true, false, false, false); + emit MultiSignatureWallet.TransactionExpired(0); vm.prank(address(1002)); multiSig.executeTransaction(0); + assertEq(multiSig.txCount(), 0); } /// @dev Test to ensure 'addOwners' reverts if transaction has insufficient number of confirmations. @@ -568,20 +579,23 @@ contract MultiSignatureWalletTest is Test { multiSig.executeTransaction(1); } - /// @dev Test to ensure 'removeOwners' reverts if transaction has expired. + /// @dev Test to ensure 'removeOwners' removes the tx and emits 'TransactionExpired' if transaction has expired. function testRemoveOwnersRevertsIfTimestampExpired() public { testAddOwners(); vm.warp(500); submitTransactionToMultiSig(dataToRemoveOwnerFromMultiSig()); + assertEq(multiSig.txCount(), 1); grantSufficientConfirmations(1); vm.warp(10501); - vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + vm.expectEmit(true, false, false, false); + emit MultiSignatureWallet.TransactionExpired(1); vm.prank(address(1002)); multiSig.executeTransaction(1); + assertEq(multiSig.txCount(), 0); } /// @dev Test to ensure 'removeOwners' reverts if transaction has insufficient number of confirmations. @@ -649,18 +663,21 @@ contract MultiSignatureWalletTest is Test { multiSig.executeTransaction(0); } - /// @dev Test to ensure 'updateNumConfirmations' reverts if the transaction has expired. + /// @dev Test to ensure 'updateNumConfirmations' removes the tx and emits 'TransactionExpired' if the transaction has expired. function testUpdateNumConfimationsRevertsIftimestampExpired() public { vm.warp(500); submitTransactionToMultiSig(dataToUpdateNumConfimationsMultiSig(3)); + assertEq(multiSig.txCount(), 1); grantSufficientConfirmations(0); vm.warp(10501); - vm.expectRevert(MultiSignatureWallet.TransactionAlreadyExpired.selector); + vm.expectEmit(true, false, false, false); + emit MultiSignatureWallet.TransactionExpired(0); vm.prank(address(1002)); - multiSig.executeTransaction(0); + multiSig.executeTransaction(0); + assertEq(multiSig.txCount(), 0); } /// @dev Test to ensure 'updateNumConfirmations' reverts if the transaction has insufficient number of confirmations. From b68bdc591a0a848a9ceac424e92c41e8d68b6e63 Mon Sep 17 00:00:00 2001 From: Udit Yadav Date: Wed, 31 Dec 2025 11:08:50 +0530 Subject: [PATCH 12/12] renamed test cases --- .../test/MultiSignatureWallet.t.sol | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol index 76a3bed034..7cdbe18c45 100644 --- a/solidity/supra_contracts/test/MultiSignatureWallet.t.sol +++ b/solidity/supra_contracts/test/MultiSignatureWallet.t.sol @@ -245,10 +245,9 @@ contract MultiSignatureWalletTest is Test { } /// @dev Test to ensure 'confirmTransaction' removes the tx and emits 'TransactionExpired' if transaction has expired. - function testConfirmTransactionRevertsIfTxExpired() public { + function testConfirmTransactionRemovesTxIfExpired() public { vm.warp(500); testSubmitTransactionIncrement(); - assertEq(multiSig.txCount(), 1); vm.warp(10501); vm.expectEmit(true, false, false, false); @@ -308,10 +307,9 @@ contract MultiSignatureWalletTest is Test { } /// @dev Test to ensure 'revokeConfirmation' removes the tx and emits 'TransactionExpired' if the transaction has expired. - function testRevokeConfirmationRevertsIfTxExpired() public { + function testRevokeConfirmationRemovesTxIfExpired() public { vm.warp(500); testSubmitTransactionIncrement(); - assertEq(multiSig.txCount(), 1); vm.warp(10501); vm.expectEmit(true, false, false, false); @@ -372,10 +370,9 @@ contract MultiSignatureWalletTest is Test { } /// @dev Test to ensure 'executeTransaction' removes the tx and emits 'TransactionExpired' if transaction has expired. - function testExecuteTransactionRevertsIfTxExpired() public { + function testExecuteTransactionRemovesTxIfExpired() public { vm.warp(500); testSubmitTransactionIncrement(); - assertEq(multiSig.txCount(), 1); vm.warp(10501); vm.expectEmit(true, false, false, false); @@ -485,7 +482,7 @@ contract MultiSignatureWalletTest is Test { } /// @dev Test to ensure 'addOwners' removes the tx and emits 'TransactionExpired' if transaction has expired. - function testAddOwnersRevertsIfTimestampExpired() public { + function testAddOwnersRemovesTxIfExpired() public { vm.warp(500); submitTransactionToMultiSig(dataToAddOwnerInMultiSig()); assertEq(multiSig.txCount(), 1); @@ -580,7 +577,7 @@ contract MultiSignatureWalletTest is Test { } /// @dev Test to ensure 'removeOwners' removes the tx and emits 'TransactionExpired' if transaction has expired. - function testRemoveOwnersRevertsIfTimestampExpired() public { + function testRemoveOwnersRemovesTxIfExpired() public { testAddOwners(); vm.warp(500); @@ -664,7 +661,7 @@ contract MultiSignatureWalletTest is Test { } /// @dev Test to ensure 'updateNumConfirmations' removes the tx and emits 'TransactionExpired' if the transaction has expired. - function testUpdateNumConfimationsRevertsIftimestampExpired() public { + function testUpdateNumConfimationsRemovesTxIfExpired() public { vm.warp(500); submitTransactionToMultiSig(dataToUpdateNumConfimationsMultiSig(3)); assertEq(multiSig.txCount(), 1); @@ -843,4 +840,17 @@ contract MultiSignatureWalletTest is Test { vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); multiSig.getTransaction(0); } + + /// @dev Test to ensure expired transaction is removed and accessing it results in a revert. + function testGetTransactionRevertsIfTxExpiredAndCleanedUp() public { + vm.warp(500); + testSubmitTransactionIncrement(); + + vm.warp(10501); + confirmTransaction(address(1005), 0); + assertEq(multiSig.txCount(), 0); + + vm.expectRevert(MultiSignatureWallet.InvalidTxnId.selector); + multiSig.getTransaction(0); + } }