diff --git a/packages/smart-vaults/README.md b/packages/smart-vaults/README.md index ff937af..ee77202 100644 --- a/packages/smart-vaults/README.md +++ b/packages/smart-vaults/README.md @@ -13,7 +13,7 @@ Currently, these smart accounts are compatible with entry point [v0.7](https://g * **Token Support**: Accepts **ERC721** and **ERC1155**(single and batch) tokens. * **Fallback Manager**: Allows users to extend their smart accounts to handle future callback-based interactions, such as those involving **ERC721** tokens, ensuring future-proofing. * **Module Manager**: Provides users the ability to add trusted modules that can interact on behalf of the smart account. -* **Contract Deployment**: Enables the deployment of new contracts using `create` from within the smart account during a UserOP. +* **Contract Deployment**: Enables the deployment of new contracts using `create` from within the smart account during a UserOp. * **[Merkelized User Operations](#merkelized-user-operations)**: Supports signing once for multiple user operations across different networks and accounts using Merkle trees. * **[Light User Operation](#light-user-operation)**: When multiple signatures are required, allows the last signer to set gas according to current market conditions. diff --git a/packages/smart-vaults/src/signers/MultiSigner.sol b/packages/smart-vaults/src/signers/MultiSigner.sol index dff850e..f863baf 100644 --- a/packages/smart-vaults/src/signers/MultiSigner.sol +++ b/packages/smart-vaults/src/signers/MultiSigner.sol @@ -14,7 +14,7 @@ struct MultiSigner { /// @dev number of signers uint8 signerCount; /// @dev signers of type `Signer`; - mapping(uint8 => Signer) signers; + Signer[256] signers; } using MultiSignerLib for MultiSigner global; @@ -52,6 +52,18 @@ library MultiSignerLib { /// @notice Thrown when number of signers is more than 256. error InvalidNumberOfSigners(); + /** + * @notice Thrown when trying to remove an empty signer. + * @param index Index of the empty signer. + */ + error SignerNotPresent(uint8 index); + + /** + * @notice Thrown when trying to replace an existing signer. + * @param index Index of the existing signer. + */ + error SignerAlreadyPresent(uint8 index); + /* -------------------------------------------------------------------------- */ /* FUNCTIONS */ /* -------------------------------------------------------------------------- */ @@ -74,54 +86,106 @@ library MultiSignerLib { /** * @notice Adds signer at index in storage. * + * @dev Throws error when a signer is already present at index. * @dev Throws error when signer is not EOA or Passkey. * * @param $_ Multi signer storage reference. * @param signer_ Signer to be set. - * @param index_ Index to set signer at. Assumed to be currently unoccupied. + * @param index_ Index to set signer at. */ function addSigner(MultiSigner storage $_, Signer calldata signer_, uint8 index_) internal { - if (signer_.isPasskey()) { - /// if passkey store signer as is. - $_.signers[index_] = signer_; - } else if (signer_.isEOA()) { - /// if EOA only store slot1 since slot2 is zero. - $_.signers[index_].slot1 = signer_.slot1; - } else { - revert InvalidSigner(signer_); - } + if (!$_.signers[index_].isEmptyMem()) revert SignerAlreadyPresent(index_); + + _addSigner($_, signer_, index_); + + $_.signerCount = $_.signerCount + 1; } /** * @notice Removes signer at the given `index`. * + * @dev Reverts if 'threshold' is equal to signer count. + * @dev Reverts if signer is empty at `index`. + * * @param $_ Multi signer storage reference. * @param index_ The index of the signer to be removed. + * @return signer Signer being removed. */ - function removeSigner(MultiSigner storage $_, uint8 index_) internal { + function removeSigner(MultiSigner storage $_, uint8 index_) internal returns (Signer memory signer) { + uint8 signerCount = $_.signerCount; + + if (signerCount == $_.threshold) revert InvalidThreshold(); + + signer = $_.signers[index_]; + + if (signer.isEmptyMem()) revert SignerNotPresent(index_); + delete $_.signers[index_]; + $_.signerCount = signerCount - 1; } /** * @notice Updates threshold of the signer set. * + * @dev Reverts if 'threshold' is greater than signer count. + * @dev Reverts if 'threshold' is 0. + * * @param $_ Multi signer storage reference. * @param threshold_ The new signer set threshold. */ function updateThreshold(MultiSigner storage $_, uint8 threshold_) internal { + if (threshold_ == 0) revert InvalidThreshold(); + + if ($_.signerCount < threshold_) revert InvalidThreshold(); + $_.threshold = threshold_; } /** - * @notice Updates signer count of the signer set. + * @notice Initialize the signers. * - * @param $_ Multi signer storage reference. - * @param signerCount_ The new signer count. + * @dev Intended to be called when initializing the signer set. + * @dev Reverts if signer is neither an EOA or a passkey. + * @dev Reverts if 'threshold' is less than number of signers. + * @dev Reverts if 'threshold' is 0. + * @dev Reverts if number of signers is more than 256. + * + * @param signers_ The initial set of signers. + * @param threshold_ The number of signers needed for approval. */ - function updateSignerCount(MultiSigner storage $_, uint8 signerCount_) internal { - $_.signerCount = signerCount_; + function initializeSigners(MultiSigner storage $_, Signer[] calldata signers_, uint8 threshold_) internal { + if (signers_.length > type(uint8).max) revert InvalidNumberOfSigners(); + + uint8 numSigners = uint8(signers_.length); + + if (numSigners < threshold_ || threshold_ < 1) revert InvalidThreshold(); + + for (uint8 i; i < numSigners; i++) { + _addSigner($_, signers_[i], i); + } + + $_.signerCount = numSigners; + $_.threshold = threshold_; } + /// @dev reverts if signer is neither an EOA or a Passkey. + /// @dev replaces signer at `index`. Should be used with proper checks. + function _addSigner(MultiSigner storage $_, Signer calldata signer_, uint8 index_) private { + if (signer_.isPasskey()) { + /// if passkey store signer as is. + $_.signers[index_] = signer_; + } else if (signer_.isEOA()) { + /// if EOA only store slot1 since slot2 is zero. + $_.signers[index_].slot1 = signer_.slot1; + } else { + revert InvalidSigner(signer_); + } + } + + /* -------------------------------------------------------------------------- */ + /* VALIDATE SIGNERS */ + /* -------------------------------------------------------------------------- */ + /** * @notice Validates the list of `signers` and `threshold`. * @@ -132,7 +196,7 @@ library MultiSignerLib { * @param threshold_ minimum number of signers required for approval. */ function validateSigners(Signer[] calldata signers_, uint8 threshold_) internal pure { - if (signers_.length > type(uint8).max || signers_.length == 0) revert InvalidNumberOfSigners(); + if (signers_.length > type(uint8).max) revert InvalidNumberOfSigners(); uint8 numberOfSigners = uint8(signers_.length); @@ -152,6 +216,10 @@ library MultiSignerLib { if (!signer_.isValid()) revert InvalidSigner(signer_); } + /* -------------------------------------------------------------------------- */ + /* VALIDATE SIGNATURES */ + /* -------------------------------------------------------------------------- */ + /** * @notice validates if `hash_` was signed by the signer set present in `$_` * diff --git a/packages/smart-vaults/src/utils/MultiSignerAuth.sol b/packages/smart-vaults/src/utils/MultiSignerAuth.sol index 98bc4fb..d70c473 100644 --- a/packages/smart-vaults/src/utils/MultiSignerAuth.sol +++ b/packages/smart-vaults/src/utils/MultiSignerAuth.sol @@ -35,30 +35,15 @@ abstract contract MultiSignerAuth { } /* -------------------------------------------------------------------------- */ - /* ERRORS */ + /* EVENTS */ /* -------------------------------------------------------------------------- */ - /// @notice Thrown when threshold is greater than number of signers or when zero. - error InvalidThreshold(); - - /// @notice Thrown when number of signers is more than 256. - error InvalidNumberOfSigners(); - /** - * @notice Thrown when trying to remove an empty signer. - * @param index Index of the empty signer. + * @notice Emitted when MultiSigner is initialized. + * @param signers Initial set of signers. + * @param threshold Initial threshold for the signer set. */ - error SignerNotPresent(uint8 index); - - /** - * @notice Thrown when trying to replace an existing signer. - * @param index Index of the existing signer. - */ - error SignerAlreadyPresent(uint8 index); - - /* -------------------------------------------------------------------------- */ - /* EVENTS */ - /* -------------------------------------------------------------------------- */ + event InitializedSigners(Signer[] signers, uint8 threshold); /** * @notice Emitted when a new signer is registered. @@ -93,19 +78,19 @@ abstract contract MultiSignerAuth { /* PUBLIC VIEW FUNCTIONS */ /* -------------------------------------------------------------------------- */ - /// @notice Returns the owner bytes at the given `index`. - function getSigner(uint8 index_) public view virtual returns (Signer memory) { - return _getMultiSignerAuthStorage().signers.getSigner(index_); + /// @notice Returns the Signer at the given `index`. + function getSigner(uint8 index_) public view returns (Signer memory) { + return _getMultiSignerStorage().getSigner(index_); } /// @notice Returns the current number of signers - function getSignerCount() public view virtual returns (uint8) { - return _getMultiSignerAuthStorage().signers.getSignerCount(); + function getSignerCount() public view returns (uint8) { + return _getMultiSignerStorage().getSignerCount(); } /// @notice Returns the threshold - function getThreshold() public view virtual returns (uint8) { - return _getMultiSignerAuthStorage().signers.getThreshold(); + function getThreshold() public view returns (uint8) { + return _getMultiSignerStorage().getThreshold(); } /* -------------------------------------------------------------------------- */ @@ -115,18 +100,14 @@ abstract contract MultiSignerAuth { /** * @notice Adds a `signer` at the `index`. * + * @dev Throws error when a signer is already present at index. * @dev Reverts if Signer is neither EOA or Passkey. * * @param signer_ The owner raw bytes to register. * @param index_ The index to register the signer. */ function addSigner(Signer calldata signer_, uint8 index_) external onlyAuthorized { - MultiSignerAuthStorage storage $ = _getMultiSignerAuthStorage(); - - if (!$.signers.getSigner(index_).isEmptyMem()) revert SignerAlreadyPresent(index_); - - $.signers.addSigner(signer_, index_); - $.signers.updateSignerCount($.signers.getSignerCount() + 1); + _getMultiSignerStorage().addSigner(signer_, index_); emit AddSigner(index_, signer_); } @@ -140,18 +121,7 @@ abstract contract MultiSignerAuth { * @param index_ The index of the signer to be removed. */ function removeSigner(uint8 index_) external onlyAuthorized { - MultiSignerAuthStorage storage $ = _getMultiSignerAuthStorage(); - - uint8 signerCount = $.signers.getSignerCount(); - - if (signerCount == $.signers.getThreshold()) revert InvalidThreshold(); - - Signer memory signer = $.signers.getSigner(index_); - - if (signer.isEmptyMem()) revert SignerNotPresent(index_); - - $.signers.removeSigner(index_); - $.signers.updateSignerCount(signerCount - 1); + Signer memory signer = _getMultiSignerStorage().removeSigner(index_); emit RemoveSigner(index_, signer); } @@ -165,12 +135,7 @@ abstract contract MultiSignerAuth { * @param threshold_ The new signer set threshold. */ function updateThreshold(uint8 threshold_) external onlyAuthorized { - if (threshold_ == 0) revert InvalidThreshold(); - - MultiSignerAuthStorage storage $ = _getMultiSignerAuthStorage(); - if ($.signers.getSignerCount() < threshold_) revert InvalidThreshold(); - - $.signers.updateThreshold(threshold_); + _getMultiSignerStorage().updateThreshold(threshold_); emit UpdateThreshold(threshold_); } @@ -182,10 +147,12 @@ abstract contract MultiSignerAuth { function _authorize() internal virtual; /// @notice Helper function to get storage reference to the `MultiSignerStorage` struct. - function _getMultiSignerAuthStorage() internal pure returns (MultiSignerAuthStorage storage $) { + function _getMultiSignerStorage() internal view returns (MultiSigner storage) { + MultiSignerAuthStorage storage $; assembly ("memory-safe") { $.slot := _MUTLI_SIGNER_AUTH_STORAGE_LOCATION } + return $.signers; } /** @@ -200,24 +167,9 @@ abstract contract MultiSignerAuth { * @param signers_ The initial set of signers. * @param threshold_ The number of signers needed for approval. */ - function _initializeSigners(Signer[] calldata signers_, uint8 threshold_) internal virtual { - if (signers_.length > type(uint8).max || signers_.length == 0) revert InvalidNumberOfSigners(); - - uint8 numSigners = uint8(signers_.length); - - if (numSigners < threshold_ || threshold_ < 1) revert InvalidThreshold(); + function _initializeMultiSignerAuth(Signer[] calldata signers_, uint8 threshold_) internal { + _getMultiSignerStorage().initializeSigners(signers_, threshold_); - MultiSignerAuthStorage storage $ = _getMultiSignerAuthStorage(); - - for (uint8 i; i < numSigners; i++) { - $.signers.addSigner(signers_[i], i); - - emit AddSigner(i, signers_[i]); - } - - $.signers.updateSignerCount(numSigners); - $.signers.updateThreshold(threshold_); - - emit UpdateThreshold(threshold_); + emit InitializedSigners(signers_, threshold_); } } diff --git a/packages/smart-vaults/src/vault/SmartVault.sol b/packages/smart-vaults/src/vault/SmartVault.sol index 2baa1d0..e27d8a2 100644 --- a/packages/smart-vaults/src/vault/SmartVault.sol +++ b/packages/smart-vaults/src/vault/SmartVault.sol @@ -168,7 +168,7 @@ contract SmartVault is IAccount, Ownable, UUPSUpgradeable, MultiSignerAuth, ERC1 if (msg.sender != FACTORY) revert OnlyFactory(); _initializeOwner(owner_); - _initializeSigners(signers_, threshold_); + _initializeMultiSignerAuth(signers_, threshold_); } /** @@ -313,9 +313,7 @@ contract SmartVault is IAccount, Ownable, UUPSUpgradeable, MultiSignerAuth, ERC1 /// @dev validates if the given hash (ERC1271) was signed by the signers. function _isValidSignature(bytes32 hash_, bytes calldata signature_) internal view override returns (bool) { - return _getMultiSignerAuthStorage().signers.isValidSignature( - hash_, abi.decode(signature_, (ERC1271Signature)).signatures - ); + return _getMultiSignerStorage().isValidSignature(hash_, abi.decode(signature_, (ERC1271Signature)).signatures); } function _validateSingleUserOp( @@ -361,9 +359,7 @@ contract SmartVault is IAccount, Ownable, UUPSUpgradeable, MultiSignerAuth, ERC1 view returns (uint256 validationData) { - MultiSignerAuthStorage storage $ = _getMultiSignerAuthStorage(); - - return $.signers.isValidSignature(lightHash_, hash_, signatures) + return _getMultiSignerStorage().isValidSignature(lightHash_, hash_, signatures) ? UserOperationLib.VALID_SIGNATURE : UserOperationLib.INVALID_SIGNATURE; } diff --git a/packages/smart-vaults/test/MultiSigner.t.sol b/packages/smart-vaults/test/MultiSigner.t.sol index 20326b2..2ef724b 100644 --- a/packages/smart-vaults/test/MultiSigner.t.sol +++ b/packages/smart-vaults/test/MultiSigner.t.sol @@ -47,9 +47,7 @@ contract MultiSignerTest is BaseTest { function test_initialize_signers() public { vm.expectEmit(); - emit MultiSignerAuth.AddSigner(0, createSigner(ALICE.addr)); - emit MultiSignerAuth.AddSigner(1, createSigner(BOB.addr)); - emit MultiSignerAuth.AddSigner(2, createSigner(MIKE.x, MIKE.y)); + emit MultiSignerAuth.InitializedSigners(signers, 1); initializeSigners(); assertEq(multiSigner.getSigner(0), createSigner(ALICE.addr)); @@ -70,7 +68,7 @@ contract MultiSignerTest is BaseTest { } function test_initialize_signers_RevertWhen_zeroSigners() public { - vm.expectRevert(InvalidNumberOfSigners.selector); + vm.expectRevert(InvalidThreshold.selector); multiSigner.initialize(ALICE.addr, new Signer[](0), 1); } diff --git a/packages/smart-vaults/test/SmartVault.t.sol b/packages/smart-vaults/test/SmartVault.t.sol index 8ae7224..695ba6b 100644 --- a/packages/smart-vaults/test/SmartVault.t.sol +++ b/packages/smart-vaults/test/SmartVault.t.sol @@ -148,7 +148,7 @@ contract SmartVaultTest is BaseTest { } function test_initialize_RevertsWhen_signersIsZero() public { - vm.expectRevert(abi.encodeWithSelector(InvalidNumberOfSigners.selector)); + vm.expectRevert(abi.encodeWithSelector(InvalidThreshold.selector)); vm.prank(address(smartVaultFactory)); vault.initialize(root, new Signer[](0), 1); } diff --git a/packages/smart-vaults/test/mocks/MultiSignerMock.sol b/packages/smart-vaults/test/mocks/MultiSignerMock.sol index e3c344c..3559265 100644 --- a/packages/smart-vaults/test/mocks/MultiSignerMock.sol +++ b/packages/smart-vaults/test/mocks/MultiSignerMock.sol @@ -32,7 +32,7 @@ contract MultiSignerMock is MultiSignerAuth, Ownable { function initialize(address _root, Signer[] calldata _signers, uint8 _threshold) external { if (msg.sender != deployer) revert(); _initializeOwner(_root); - _initializeSigners(_signers, _threshold); + _initializeMultiSignerAuth(_signers, _threshold); } function _authorize() internal view override(MultiSignerAuth) onlyOwner { }