From 10eb64b40c231d496e7bc19960424bc68fd990f9 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 27 Aug 2025 10:11:30 -1000 Subject: [PATCH 1/8] Add ERC7579 validators --- contracts/account/README.adoc | 8 + .../account/modules/ERC7579Signature.sol | 86 ++++++++++ .../account/modules/ERC7579Validator.sol | 101 ++++++++++++ .../account/modules/ERC7579Module.behavior.js | 70 ++++++++ test/account/modules/ERC7579Signature.test.js | 151 ++++++++++++++++++ test/account/modules/ERC7579Validator.test.js | 57 +++++++ 6 files changed, 473 insertions(+) create mode 100644 contracts/account/modules/ERC7579Signature.sol create mode 100644 contracts/account/modules/ERC7579Validator.sol create mode 100644 test/account/modules/ERC7579Module.behavior.js create mode 100644 test/account/modules/ERC7579Signature.test.js create mode 100644 test/account/modules/ERC7579Validator.test.js diff --git a/contracts/account/README.adoc b/contracts/account/README.adoc index dc3c9a010a7..1351fd52c82 100644 --- a/contracts/account/README.adoc +++ b/contracts/account/README.adoc @@ -7,6 +7,8 @@ This directory includes contracts to build accounts for ERC-4337. These include: * {Account}: An ERC-4337 smart account implementation that includes the core logic to process user operations. * {AccountERC7579}: An extension of `Account` that implements support for ERC-7579 modules. * {AccountERC7579Hooked}: An extension of `AccountERC7579` with support for a single hook module (type 4). + * {ERC7579Validator}: Abstract validator module for ERC-7579 accounts that provides base implementation for signature validation. + * {ERC7579Signature}: Implementation of {ERC7579Validator} using ERC-7913 signature verification for address-less cryptographic keys and account signatures. * {ERC7821}: Minimal batch executor implementation contracts. Useful to enable easy batch execution for smart contracts. * {ERC4337Utils}: Utility functions for working with ERC-4337 user operations. * {ERC7579Utils}: Utility functions for working with ERC-7579 modules and account modularity. @@ -23,6 +25,12 @@ This directory includes contracts to build accounts for ERC-4337. These include: {{ERC7821}} +=== Validators + +{{ERC7579Validator}} + +{{ERC7579Signature}} + == Utilities {{ERC4337Utils}} diff --git a/contracts/account/modules/ERC7579Signature.sol b/contracts/account/modules/ERC7579Signature.sol new file mode 100644 index 00000000000..45e1e60850b --- /dev/null +++ b/contracts/account/modules/ERC7579Signature.sol @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.27; + +import {IERC7579Module} from "../../interfaces/draft-IERC7579.sol"; +import {SignatureChecker} from "../../utils/cryptography/SignatureChecker.sol"; +import {ERC7579Validator} from "./ERC7579Validator.sol"; + +/** + * @dev Implementation of {ERC7579Validator} module using ERC-7913 signature verification. + * + * This validator allows ERC-7579 accounts to integrate with address-less cryptographic keys + * and account signatures through the ERC-7913 signature verification system. Each account + * can store its own ERC-7913 formatted signer (a concatenation of a verifier address and a + * key: `verifier || key`). + * + * This enables accounts to use signature schemes without requiring each key to have its own + * Ethereum address.A smart account with this module installed can keep an emergency key as a + * backup. + */ +contract ERC7579Signature is ERC7579Validator { + mapping(address account => bytes signer) private _signers; + + /// @dev Emitted when the signer is set. + event ERC7579SignatureSignerSet(address indexed account, bytes signer); + + /// @dev Thrown when the signer length is less than 20 bytes. + error ERC7579SignatureInvalidSignerLength(); + + /// @dev Return the ERC-7913 signer (i.e. `verifier || key`). + function signer(address account) public view virtual returns (bytes memory) { + return _signers[account]; + } + + /** + * @dev See {IERC7579Module-onInstall}. + * + * NOTE: An account can only call onInstall once. If called directly by the account, + * the signer will be set to the provided data. Future installations will behave as a no-op. + */ + function onInstall(bytes calldata data) public virtual { + if (signer(msg.sender).length == 0) { + setSigner(data); + } + } + + /** + * @dev See {IERC7579Module-onUninstall}. + * + * WARNING: The signer's key will be removed if the account calls this function, potentially + * making the account unusable. As an account operator, make sure to uninstall to a predefined path + * in your account that properly handles side effects of uninstallation. See {AccountERC7579-uninstallModule}. + */ + function onUninstall(bytes calldata) public virtual { + _setSigner(msg.sender, ""); + } + + /// @dev Sets the ERC-7913 signer (i.e. `verifier || key`) for the calling account. + function setSigner(bytes memory signer_) public virtual { + require(signer_.length >= 20, ERC7579SignatureInvalidSignerLength()); + _setSigner(msg.sender, signer_); + } + + /// @dev Internal version of {setSigner} that takes an `account` as argument without validating `signer_`. + function _setSigner(address account, bytes memory signer_) internal virtual { + _signers[account] = signer_; + emit ERC7579SignatureSignerSet(account, signer_); + } + + /** + * @dev See {ERC7579Validator-_rawERC7579Validation}. + * + * Validates a `signature` using ERC-7913 verification. + * + * This base implementation ignores the `sender` parameter and validates using + * the account's stored signer. Derived contracts can override this to implement + * custom validation logic based on the sender. + */ + function _rawERC7579Validation( + address account, + bytes32 hash, + bytes calldata signature + ) internal view virtual override returns (bool) { + return SignatureChecker.isValidSignatureNow(signer(account), hash, signature); + } +} diff --git a/contracts/account/modules/ERC7579Validator.sol b/contracts/account/modules/ERC7579Validator.sol new file mode 100644 index 00000000000..c456ef84897 --- /dev/null +++ b/contracts/account/modules/ERC7579Validator.sol @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.27; + +import {IERC7579Module, IERC7579Validator, MODULE_TYPE_VALIDATOR} from "../../interfaces/draft-IERC7579.sol"; +import {PackedUserOperation} from "../../interfaces/draft-IERC4337.sol"; +import {ERC4337Utils} from "../../account/utils/draft-ERC4337Utils.sol"; +import {IERC1271} from "../../interfaces/IERC1271.sol"; + +/** + * @dev Abstract validator module for ERC-7579 accounts. + * + * This contract provides the base implementation for signature validation in ERC-7579 accounts. + * Developers must implement the onInstall, onUninstall, and {_rawERC7579Validation} + * functions in derived contracts to define the specific signature validation logic. + * + * Example usage: + * + * ```solidity + * contract MyValidatorModule is ERC7579Validator { + * function onInstall(bytes calldata data) public { + * // Install logic here + * } + * + * function onUninstall(bytes calldata data) public { + * // Uninstall logic here + * } + * + * function _rawERC7579Validation( + * address account, + * bytes32 hash, + * bytes calldata signature + * ) internal view override returns (bool) { + * // Signature validation logic here + * } + * } + * ``` + * + * Developers can restrict other operations by using the internal {_rawERC7579Validation}. + * Example usage: + * + * ```solidity + * function execute( + * address account, + * Mode mode, + * bytes calldata executionCalldata, + * bytes32 salt, + * bytes calldata signature + * ) public virtual { + * require(_rawERC7579Validation(account, hash, signature)); + * // ... rest of execute logic + * } + * ``` + */ +abstract contract ERC7579Validator is IERC7579Module, IERC7579Validator { + /// @inheritdoc IERC7579Module + function isModuleType(uint256 moduleTypeId) public pure virtual returns (bool) { + return moduleTypeId == MODULE_TYPE_VALIDATOR; + } + + /// @inheritdoc IERC7579Validator + function validateUserOp( + PackedUserOperation calldata userOp, + bytes32 userOpHash + ) public view virtual returns (uint256) { + return + _rawERC7579Validation(msg.sender, userOpHash, userOp.signature) + ? ERC4337Utils.SIG_VALIDATION_SUCCESS + : ERC4337Utils.SIG_VALIDATION_FAILED; + } + + /** + * @dev See {IERC7579Validator-isValidSignatureWithSender}. + * + * Ignores the `sender` parameter and validates using {_rawERC7579Validation}. + * Consider overriding this function to implement custom validation logic + * based on the original sender. + */ + function isValidSignatureWithSender( + address /* sender */, + bytes32 hash, + bytes calldata signature + ) public view virtual returns (bytes4) { + return + _rawERC7579Validation(msg.sender, hash, signature) + ? IERC1271.isValidSignature.selector + : bytes4(0xffffffff); + } + + /** + * @dev Validation algorithm. + * + * WARNING: Validation is a critical security function. Implementations must carefully + * handle cryptographic verification to prevent unauthorized access. + */ + function _rawERC7579Validation( + address account, + bytes32 hash, + bytes calldata signature + ) internal view virtual returns (bool); +} diff --git a/test/account/modules/ERC7579Module.behavior.js b/test/account/modules/ERC7579Module.behavior.js new file mode 100644 index 00000000000..f309662d502 --- /dev/null +++ b/test/account/modules/ERC7579Module.behavior.js @@ -0,0 +1,70 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { SIG_VALIDATION_SUCCESS, SIG_VALIDATION_FAILURE } = require('../../helpers/erc4337'); + +function shouldBehaveLikeERC7579Module() { + describe('behaves like ERC7579Module', function () { + it('identifies its module type correctly', async function () { + await expect(this.mock.isModuleType(this.moduleType)).to.eventually.be.true; + await expect(this.mock.isModuleType(999)).to.eventually.be.false; // Using random unassigned module type + }); + + it('handles installation, uninstallation, and re-installation', async function () { + await expect(this.mockFromAccount.onInstall(this.installData || '0x')).to.not.be.reverted; + await expect(this.mockFromAccount.onUninstall(this.uninstallData || '0x')).to.not.be.reverted; + await expect(this.mockFromAccount.onInstall(this.installData || '0x')).to.not.be.reverted; + }); + }); +} + +function shouldBehaveLikeERC7579Validator() { + describe('behaves like ERC7579Validator', function () { + const MAGIC_VALUE = '0x1626ba7e'; + const INVALID_VALUE = '0xffffffff'; + + beforeEach(async function () { + await this.mockFromAccount.onInstall(this.installData); + }); + + describe('validateUserOp', function () { + it('returns SIG_VALIDATION_SUCCESS when signature is valid', async function () { + const userOp = await this.mockAccount.createUserOp(this.userOp).then(op => this.signUserOp(op)); + await expect(this.mockFromAccount.validateUserOp(userOp.packed, userOp.hash())).to.eventually.equal( + SIG_VALIDATION_SUCCESS, + ); + }); + + it('returns SIG_VALIDATION_FAILURE when signature is invalid', async function () { + const userOp = await this.mockAccount.createUserOp(this.userOp); + userOp.signature = this.invalidSignature || '0x00'; + await expect(this.mockFromAccount.validateUserOp(userOp.packed, userOp.hash())).to.eventually.equal( + SIG_VALIDATION_FAILURE, + ); + }); + }); + + describe('isValidSignatureWithSender', function () { + it('returns magic value for valid signature', async function () { + const message = 'Hello, world!'; + const hash = ethers.hashMessage(message); + const signature = await this.signer.signMessage(message); + await expect(this.mockFromAccount.isValidSignatureWithSender(this.other, hash, signature)).to.eventually.equal( + MAGIC_VALUE, + ); + }); + + it('returns failure value for invalid signature', async function () { + const hash = ethers.hashMessage('Hello, world!'); + const signature = this.invalidSignature || '0x00'; + await expect(this.mock.isValidSignatureWithSender(this.other, hash, signature)).to.eventually.equal( + INVALID_VALUE, + ); + }); + }); + }); +} + +module.exports = { + shouldBehaveLikeERC7579Module, + shouldBehaveLikeERC7579Validator, +}; diff --git a/test/account/modules/ERC7579Signature.test.js b/test/account/modules/ERC7579Signature.test.js new file mode 100644 index 00000000000..cf09a48bf48 --- /dev/null +++ b/test/account/modules/ERC7579Signature.test.js @@ -0,0 +1,151 @@ +const { ethers, predeploy } = require('hardhat'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { expect } = require('chai'); + +const { impersonate } = require('../../helpers/account'); +const { getDomain, PackedUserOperation } = require('../../helpers/eip712'); +const { ERC4337Helper } = require('../../helpers/erc4337'); +const { MODULE_TYPE_VALIDATOR } = require('../../helpers/erc7579'); +const { NonNativeSigner, P256SigningKey, RSASHA256SigningKey } = require('../../helpers/signers'); + +const { shouldBehaveLikeERC7579Module, shouldBehaveLikeERC7579Validator } = require('./ERC7579Module.behavior'); + +// Prepare signers in advance (RSA are long to initialize) +const signerECDSA = ethers.Wallet.createRandom(); +const signerP256 = new NonNativeSigner(P256SigningKey.random()); +const signerRSA = new NonNativeSigner(RSASHA256SigningKey.random()); + +async function fixture() { + const [other] = await ethers.getSigners(); + + // Deploy ERC-7579 signature validator + const mock = await ethers.deployContract('$ERC7579Signature'); + + // ERC-7913 verifiers + const verifierP256 = await ethers.deployContract('ERC7913P256Verifier'); + const verifierRSA = await ethers.deployContract('ERC7913RSAVerifier'); + + // ERC-4337 env + const helper = new ERC4337Helper(); + await helper.wait(); + const entrypointDomain = await getDomain(predeploy.entrypoint.v08); + + // ERC-7579 account + const mockAccount = await helper.newAccount('$AccountERC7579'); + const mockFromAccount = await impersonate(mockAccount.address).then(asAccount => mock.connect(asAccount)); + + return { + moduleType: MODULE_TYPE_VALIDATOR, + mock, + verifierP256, + verifierRSA, + mockFromAccount, + entrypointDomain, + mockAccount, + other, + }; +} + +function prepareSigner(prototype) { + this.signUserOp = userOp => + prototype.signTypedData + .call(this.signer, this.entrypointDomain, { PackedUserOperation }, userOp.packed) + .then(signature => Object.assign(userOp, { signature })); +} + +describe('ERC7579Signature', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + it('reverts with ERC7579SignatureInvalidSignerLength when signer length is less than 20 bytes', async function () { + const shortSigner = '0x0123456789'; // Less than 20 bytes + await expect(this.mockFromAccount.onInstall(shortSigner)).to.be.revertedWithCustomError( + this.mock, + 'ERC7579SignatureInvalidSignerLength', + ); + }); + + it('behaves as a noop when the validator is already installed for an account', async function () { + // First installation should succeed + const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]); + await expect(this.mockFromAccount.onInstall(signerData)).to.not.be.reverted; + + // Second installation should behave as a no-op + await this.mockFromAccount.onInstall(ethers.solidityPacked(['address'], [ethers.Wallet.createRandom().address])); // Not revert + await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal(signerData); // No change in signers + }); + + it('emits event on ERC7579SignatureSignerSet on both installation and uninstallation', async function () { + const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]); + + // First install + await expect(this.mockFromAccount.onInstall(signerData)) + .to.emit(this.mock, 'ERC7579SignatureSignerSet') + .withArgs(this.mockAccount.address, signerData); + + // Then uninstall + await expect(this.mockFromAccount.onUninstall('0x')) + .to.emit(this.mock, 'ERC7579SignatureSignerSet') + .withArgs(this.mockAccount.address, '0x'); + }); + + it('returns the correct signer bytes when set', async function () { + // Starts empty + await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal('0x'); + + const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]); + await this.mockFromAccount.onInstall(signerData); + + await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal(signerData); + }); + + it('sets signer correctly with setSigner and emits event', async function () { + const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]); + await expect(this.mockFromAccount.setSigner(signerData)) + .to.emit(this.mockFromAccount, 'ERC7579SignatureSignerSet') + .withArgs(this.mockAccount.address, signerData); + await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal(signerData); + }); + + it('reverts when calling setSigner with invalid signer length', async function () { + await expect(this.mock.setSigner('0x0123456789')).to.be.revertedWithCustomError( + this.mock, + 'ERC7579SignatureInvalidSignerLength', + ); + }); + + // ECDSA tested in ./ERC7579Validator.test.js + + describe('P256 key', function () { + beforeEach(async function () { + this.signer = signerP256; + prepareSigner.call(this, new NonNativeSigner(this.signer.signingKey)); + this.installData = ethers.concat([ + this.verifierP256.target, + this.signer.signingKey.publicKey.qx, + this.signer.signingKey.publicKey.qy, + ]); + }); + + shouldBehaveLikeERC7579Module(); + shouldBehaveLikeERC7579Validator(); + }); + + describe('RSA key', function () { + beforeEach(async function () { + this.signer = signerRSA; + prepareSigner.call(this, new NonNativeSigner(this.signer.signingKey)); + this.installData = ethers.concat([ + this.verifierRSA.target, + ethers.AbiCoder.defaultAbiCoder().encode( + ['bytes', 'bytes'], + [this.signer.signingKey.publicKey.e, this.signer.signingKey.publicKey.n], + ), + ]); + }); + + shouldBehaveLikeERC7579Module(); + shouldBehaveLikeERC7579Validator(); + }); +}); diff --git a/test/account/modules/ERC7579Validator.test.js b/test/account/modules/ERC7579Validator.test.js new file mode 100644 index 00000000000..83a6539da3d --- /dev/null +++ b/test/account/modules/ERC7579Validator.test.js @@ -0,0 +1,57 @@ +const { ethers, predeploy } = require('hardhat'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +const { impersonate } = require('../../helpers/account'); +const { getDomain, PackedUserOperation } = require('../../helpers/eip712'); +const { ERC4337Helper } = require('../../helpers/erc4337'); +const { MODULE_TYPE_VALIDATOR } = require('../../helpers/erc7579'); + +const { shouldBehaveLikeERC7579Module, shouldBehaveLikeERC7579Validator } = require('./ERC7579Module.behavior'); + +async function fixture() { + const [other] = await ethers.getSigners(); + + // Deploy ERC-7579 validator module + const mock = await ethers.deployContract('$ERC7579Signature'); + + // ERC-4337 env + const helper = new ERC4337Helper(); + await helper.wait(); + const entrypointDomain = await getDomain(predeploy.entrypoint.v08); + + // Prepare signer + const signer = ethers.Wallet.createRandom(); + const signUserOp = userOp => + signer + .signTypedData(entrypointDomain, { PackedUserOperation }, userOp.packed) + .then(signature => Object.assign(userOp, { signature })); + + // Prepare module installation data + const installData = ethers.solidityPacked(['address'], [signer.address]); + + // ERC-7579 account + const mockAccount = await helper.newAccount('$AccountERC7579'); + const mockFromAccount = await impersonate(mockAccount.address).then(asAccount => mock.connect(asAccount)); + + return { + moduleType: MODULE_TYPE_VALIDATOR, + mock, + mockFromAccount, + mockAccount, + other, + signer, + signUserOp, + installData, + }; +} + +describe('ERC7579Validator', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('ECDSA key', function () { + shouldBehaveLikeERC7579Module(); + shouldBehaveLikeERC7579Validator(); + }); +}); From 3531217a65189d1301498566066489d69a85b3bf Mon Sep 17 00:00:00 2001 From: ernestognw Date: Wed, 27 Aug 2025 10:14:47 -1000 Subject: [PATCH 2/8] Add changesets --- .changeset/solid-squids-cough.md | 5 +++++ .changeset/yummy-ideas-stay.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/solid-squids-cough.md create mode 100644 .changeset/yummy-ideas-stay.md diff --git a/.changeset/solid-squids-cough.md b/.changeset/solid-squids-cough.md new file mode 100644 index 00000000000..ff6e9b1d8f0 --- /dev/null +++ b/.changeset/solid-squids-cough.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC7579Signature`: Add implementation of `ERC7579Validator` that enables ERC-7579 accounts to integrate with address-less cryptographic keys and account signatures through ERC-7913 signature verification. diff --git a/.changeset/yummy-ideas-stay.md b/.changeset/yummy-ideas-stay.md new file mode 100644 index 00000000000..1972dcd0c46 --- /dev/null +++ b/.changeset/yummy-ideas-stay.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC7579Validator`: Add abstract validator module for ERC-7579 accounts that provides base implementation for signature validation. From 6227e75925b01101953b9f5cd7aaec8015d8128b Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 14 Oct 2025 11:32:05 -0600 Subject: [PATCH 3/8] Remove ERC7579Signature and add ERC7579Multisig and ERC7579MultisigWeighted --- .changeset/solid-squids-cough.md | 5 - contracts/account/README.adoc | 3 - contracts/account/modules/ERC7579Multisig.sol | 285 ++++++++++++++ .../modules/ERC7579MultisigWeighted.sol | 232 +++++++++++ .../account/modules/ERC7579Signature.sol | 86 ----- .../account/modules/ERC7579ValidatorMock.sol | 47 +++ test/account/modules/ERC7579Multisig.test.js | 285 ++++++++++++++ .../modules/ERC7579MultisigWeighted.test.js | 364 ++++++++++++++++++ test/account/modules/ERC7579Signature.test.js | 151 -------- test/account/modules/ERC7579Validator.test.js | 2 +- 10 files changed, 1214 insertions(+), 246 deletions(-) delete mode 100644 .changeset/solid-squids-cough.md create mode 100644 contracts/account/modules/ERC7579Multisig.sol create mode 100644 contracts/account/modules/ERC7579MultisigWeighted.sol delete mode 100644 contracts/account/modules/ERC7579Signature.sol create mode 100644 contracts/mocks/account/modules/ERC7579ValidatorMock.sol create mode 100644 test/account/modules/ERC7579Multisig.test.js create mode 100644 test/account/modules/ERC7579MultisigWeighted.test.js delete mode 100644 test/account/modules/ERC7579Signature.test.js diff --git a/.changeset/solid-squids-cough.md b/.changeset/solid-squids-cough.md deleted file mode 100644 index ff6e9b1d8f0..00000000000 --- a/.changeset/solid-squids-cough.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': minor ---- - -`ERC7579Signature`: Add implementation of `ERC7579Validator` that enables ERC-7579 accounts to integrate with address-less cryptographic keys and account signatures through ERC-7913 signature verification. diff --git a/contracts/account/README.adoc b/contracts/account/README.adoc index 1351fd52c82..a70d56ff356 100644 --- a/contracts/account/README.adoc +++ b/contracts/account/README.adoc @@ -8,7 +8,6 @@ This directory includes contracts to build accounts for ERC-4337. These include: * {AccountERC7579}: An extension of `Account` that implements support for ERC-7579 modules. * {AccountERC7579Hooked}: An extension of `AccountERC7579` with support for a single hook module (type 4). * {ERC7579Validator}: Abstract validator module for ERC-7579 accounts that provides base implementation for signature validation. - * {ERC7579Signature}: Implementation of {ERC7579Validator} using ERC-7913 signature verification for address-less cryptographic keys and account signatures. * {ERC7821}: Minimal batch executor implementation contracts. Useful to enable easy batch execution for smart contracts. * {ERC4337Utils}: Utility functions for working with ERC-4337 user operations. * {ERC7579Utils}: Utility functions for working with ERC-7579 modules and account modularity. @@ -29,8 +28,6 @@ This directory includes contracts to build accounts for ERC-4337. These include: {{ERC7579Validator}} -{{ERC7579Signature}} - == Utilities {{ERC4337Utils}} diff --git a/contracts/account/modules/ERC7579Multisig.sol b/contracts/account/modules/ERC7579Multisig.sol new file mode 100644 index 00000000000..f71a2b9d4b9 --- /dev/null +++ b/contracts/account/modules/ERC7579Multisig.sol @@ -0,0 +1,285 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.27; + +import {Mode} from "../../account/utils/draft-ERC7579Utils.sol"; +import {SignatureChecker} from "../../utils/cryptography/SignatureChecker.sol"; +import {EnumerableSet} from "../../utils/structs/EnumerableSet.sol"; +import {ERC7579Validator} from "./ERC7579Validator.sol"; + +/** + * @dev Implementation of an {ERC7579Validator} that uses ERC-7913 signers for multisignature + * validation. + * + * This module provides a base implementation for multisignature validation that can be + * attached to any function through the {_rawERC7579Validation} internal function. The signers + * are represented using the ERC-7913 format, which concatenates a verifier address and + * a key: `verifier || key`. + * + * A smart account with this module installed can require multiple signers to approve + * operations before they are executed, such as requiring 3-of-5 guardians to approve + * a social recovery operation. + */ +abstract contract ERC7579Multisig is ERC7579Validator { + using EnumerableSet for EnumerableSet.BytesSet; + using SignatureChecker for bytes32; + using SignatureChecker for bytes; + + /// @dev Emitted when signers are added. + event ERC7913SignerAdded(address indexed account, bytes signer); + + /// @dev Emitted when signers are removed. + event ERC7913SignerRemoved(address indexed account, bytes signer); + + /// @dev Emitted when the threshold is updated. + event ERC7913ThresholdSet(address indexed account, uint64 threshold); + + /// @dev The `signer` already exists. + error ERC7579MultisigAlreadyExists(bytes signer); + + /// @dev The `signer` does not exist. + error ERC7579MultisigNonexistentSigner(bytes signer); + + /// @dev The `signer` is less than 20 bytes long. + error ERC7579MultisigInvalidSigner(bytes signer); + + /// @dev The `threshold` is zero. + error ERC7579MultisigZeroThreshold(); + + /// @dev The `threshold` is unreachable given the number of `signers`. + error ERC7579MultisigUnreachableThreshold(uint64 signers, uint64 threshold); + + mapping(address account => EnumerableSet.BytesSet) private _signersSetByAccount; + mapping(address account => uint64) private _thresholdByAccount; + + /** + * @dev Sets up the module's initial configuration when installed by an account. + * See {ERC7579DelayedExecutor-onInstall}. Besides the delay setup, the `initdata` can + * include `signers` and `threshold`. + * + * The initData should be encoded as: + * `abi.encode(bytes[] signers, uint64 threshold)` + * + * If no signers or threshold are provided, the multisignature functionality will be + * disabled until they are added later. + * + * NOTE: An account can only call onInstall once. If called directly by the account, + * the signer will be set to the provided data. Future installations will behave as a no-op. + */ + function onInstall(bytes calldata initData) public virtual { + if (initData.length > 32 && getSignerCount(msg.sender) == 0) { + // More than just delay parameter + (bytes[] memory signers_, uint64 threshold_) = abi.decode(initData, (bytes[], uint64)); + _addSigners(msg.sender, signers_); + _setThreshold(msg.sender, threshold_); + } + } + + /** + * @dev Cleans up module's configuration when uninstalled from an account. + * Clears all signers and resets the threshold. + * + * See {ERC7579DelayedExecutor-onUninstall}. + * + * WARNING: This function has unbounded gas costs and may become uncallable if the set grows too large. + * See {EnumerableSet-clear}. + */ + function onUninstall(bytes calldata /* data */) public virtual { + _signersSetByAccount[msg.sender].clear(); + delete _thresholdByAccount[msg.sender]; + } + + /** + * @dev Returns a slice of the set of authorized signers for the specified account. + * + * Using `start = 0` and `end = type(uint64).max` will return the entire set of signers. + * + * WARNING: Depending on the `start` and `end`, this operation can copy a large amount of data to memory, which + * can be expensive. This is designed for view accessors queried without gas fees. Using it in state-changing + * functions may become uncallable if the slice grows too large. + */ + function getSigners(address account, uint64 start, uint64 end) public view virtual returns (bytes[] memory) { + return _signersSetByAccount[account].values(start, end); + } + + /// @dev Returns the number of authorized signers for the specified account. + function getSignerCount(address account) public view virtual returns (uint256) { + return _signersSetByAccount[account].length(); + } + + /// @dev Returns whether the `signer` is an authorized signer for the specified account. + function isSigner(address account, bytes memory signer) public view virtual returns (bool) { + return _signersSetByAccount[account].contains(signer); + } + + /** + * @dev Returns the minimum number of signers required to approve a multisignature operation + * for the specified account. + */ + function threshold(address account) public view virtual returns (uint64) { + return _thresholdByAccount[account]; + } + + /** + * @dev Adds new signers to the authorized set for the calling account. + * Can only be called by the account itself. + * + * Requirements: + * + * * Each of `newSigners` must be at least 20 bytes long. + * * Each of `newSigners` must not be already authorized. + */ + function addSigners(bytes[] memory newSigners) public virtual { + _addSigners(msg.sender, newSigners); + } + + /** + * @dev Removes signers from the authorized set for the calling account. + * Can only be called by the account itself. + * + * Requirements: + * + * * Each of `oldSigners` must be authorized. + * * After removal, the threshold must still be reachable. + */ + function removeSigners(bytes[] memory oldSigners) public virtual { + _removeSigners(msg.sender, oldSigners); + } + + /** + * @dev Sets the threshold for the calling account. + * Can only be called by the account itself. + * + * Requirements: + * + * * The threshold must be reachable with the current number of signers. + */ + function setThreshold(uint64 newThreshold) public virtual { + _setThreshold(msg.sender, newThreshold); + } + + /** + * @dev Returns whether the number of valid signatures meets or exceeds the + * threshold set for the target account. + * + * The signature should be encoded as: + * `abi.encode(bytes[] signingSigners, bytes[] signatures)` + * + * Where `signingSigners` are the authorized signers and signatures are their corresponding + * signatures of the operation `hash`. + */ + function _rawERC7579Validation( + address account, + bytes32 hash, + bytes calldata signature + ) internal view virtual override returns (bool) { + (bytes[] memory signingSigners, bytes[] memory signatures) = abi.decode(signature, (bytes[], bytes[])); + return + _validateThreshold(account, signingSigners) && + _validateSignatures(account, hash, signingSigners, signatures); + } + + /** + * @dev Adds the `newSigners` to those allowed to sign on behalf of the account. + * + * Requirements: + * + * * Each of `newSigners` must be at least 20 bytes long. Reverts with {ERC7579MultisigInvalidSigner} if not. + * * Each of `newSigners` must not be authorized. Reverts with {ERC7579MultisigAlreadyExists} if it already exists. + */ + function _addSigners(address account, bytes[] memory newSigners) internal virtual { + for (uint256 i = 0; i < newSigners.length; ++i) { + bytes memory signer = newSigners[i]; + require(signer.length >= 20, ERC7579MultisigInvalidSigner(signer)); + require(_signersSetByAccount[account].add(signer), ERC7579MultisigAlreadyExists(signer)); + emit ERC7913SignerAdded(account, signer); + } + } + + /** + * @dev Removes the `oldSigners` from the authorized signers for the account. + * + * Requirements: + * + * * Each of `oldSigners` must be authorized. Reverts with {ERC7579MultisigNonexistentSigner} if not. + * * The threshold must remain reachable after removal. See {_validateReachableThreshold} for details. + */ + function _removeSigners(address account, bytes[] memory oldSigners) internal virtual { + for (uint256 i = 0; i < oldSigners.length; ++i) { + bytes memory signer = oldSigners[i]; + require(_signersSetByAccount[account].remove(signer), ERC7579MultisigNonexistentSigner(signer)); + emit ERC7913SignerRemoved(account, signer); + } + _validateReachableThreshold(account); + } + + /** + * @dev Sets the signatures `threshold` required to approve a multisignature operation. + * + * Requirements: + * + * * The threshold must be greater than 0. Reverts with {ERC7579MultisigZeroThreshold} if not. + * * The threshold must be reachable with the current number of signers. See {_validateReachableThreshold} for details. + */ + function _setThreshold(address account, uint64 newThreshold) internal virtual { + require(newThreshold > 0, ERC7579MultisigZeroThreshold()); + _thresholdByAccount[account] = newThreshold; + _validateReachableThreshold(account); + emit ERC7913ThresholdSet(account, newThreshold); + } + + /** + * @dev Validates the current threshold is reachable with the number of {signers}. + * + * Requirements: + * + * * The number of signers must be >= the threshold. Reverts with {ERC7579MultisigUnreachableThreshold} if not. + */ + function _validateReachableThreshold(address account) internal view virtual { + uint256 totalSigners = getSignerCount(account); + uint64 currentThreshold = threshold(account); + require( + totalSigners >= currentThreshold, + ERC7579MultisigUnreachableThreshold( + uint64(totalSigners), // Safe cast. Economically impossible to overflow. + currentThreshold + ) + ); + } + + /** + * @dev Validates the signatures using the signers and their corresponding signatures. + * Returns whether the signers are authorized and the signatures are valid for the given hash. + * + * The signers must be ordered by their `keccak256` hash to prevent duplications and to optimize + * the verification process. The function will return `false` if any signer is not authorized or + * if the signatures are invalid for the given hash. + * + * Requirements: + * + * * The `signatures` array must be at least the `signers` array's length. + */ + function _validateSignatures( + address account, + bytes32 hash, + bytes[] memory signingSigners, + bytes[] memory signatures + ) internal view virtual returns (bool valid) { + for (uint256 i = 0; i < signingSigners.length; ++i) { + if (!isSigner(account, signingSigners[i])) { + return false; + } + } + return hash.areValidSignaturesNow(signingSigners, signatures); + } + + /** + * @dev Validates that the number of signers meets the {threshold} requirement. + * Assumes the signers were already validated. See {_validateSignatures} for more details. + */ + function _validateThreshold( + address account, + bytes[] memory validatingSigners + ) internal view virtual returns (bool) { + return validatingSigners.length >= threshold(account); + } +} diff --git a/contracts/account/modules/ERC7579MultisigWeighted.sol b/contracts/account/modules/ERC7579MultisigWeighted.sol new file mode 100644 index 00000000000..7c6983a521d --- /dev/null +++ b/contracts/account/modules/ERC7579MultisigWeighted.sol @@ -0,0 +1,232 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.27; + +import {SafeCast} from "../../utils/math/SafeCast.sol"; +import {EnumerableSet} from "../../utils/structs/EnumerableSet.sol"; +import {ERC7579Multisig} from "./ERC7579Multisig.sol"; + +/** + * @dev Extension of {ERC7579Multisig} that supports weighted signatures. + * + * This module extends the multisignature module to allow assigning different weights + * to each signer, enabling more flexible governance schemes. For example, some guardians + * could have higher weight than others, allowing for weighted voting or prioritized authorization. + * + * Example use case: + * + * A smart account with this module installed can schedule social recovery operations + * after obtaining approval from guardians with sufficient total weight (e.g., requiring + * a total weight of 10, with 3 guardians weighted as 5, 3, and 2), and then execute them + * after the time delay has passed. + * + * IMPORTANT: When setting a threshold value, ensure it matches the scale used for signer weights. + * For example, if signers have weights like 1, 2, or 3, then a threshold of 4 would require + * signatures with a total weight of at least 4 (e.g., one with weight 1 and one with weight 3). + */ +abstract contract ERC7579MultisigWeighted is ERC7579Multisig { + using EnumerableSet for EnumerableSet.BytesSet; + using SafeCast for *; + + // Sum of all the extra weights of all signers. Each signer has a base weight of 1. + mapping(address account => uint64 totalExtraWeight) private _totalExtraWeight; + + // Mapping from account => signer => extraWeight (in addition to all authorized signers having weight 1) + mapping(address account => mapping(bytes signer => uint64)) private _extraWeights; + + /** + * @dev Emitted when a signer's weight is changed. + * + * NOTE: Not emitted in {_addSigners} or {_removeSigners}. Indexers must rely on {ERC7913SignerAdded} + * and {ERC7913SignerRemoved} to index a default weight of 1. See {signerWeight}. + */ + event ERC7579MultisigWeightChanged(address indexed account, bytes indexed signer, uint64 weight); + + /// @dev Thrown when a signer's weight is invalid. + error ERC7579MultisigInvalidWeight(bytes signer, uint64 weight); + + /// @dev Thrown when the arrays lengths don't match. + error ERC7579MultisigMismatchedLength(); + + /** + * @dev Sets up the module's initial configuration when installed by an account. + * Besides the standard delay and signer configuration, this can also include + * signer weights. + * + * The initData should be encoded as: + * `abi.encode(bytes[] signers, uint64 threshold, uint64[] weights)` + * + * If weights are not provided but signers are, all signers default to weight 1. + * + * NOTE: An account can only call onInstall once. If called directly by the account, + * the signer will be set to the provided data. Future installations will behave as a no-op. + */ + function onInstall(bytes calldata initData) public virtual override { + bool installed = getSignerCount(msg.sender) > 0; + super.onInstall(initData); + if (initData.length > 96 && !installed) { + (bytes[] memory signers, , uint64[] memory weights) = abi.decode(initData, (bytes[], uint64, uint64[])); + _setSignerWeights(msg.sender, signers, weights); + } + } + + /** + * @dev Cleans up module's configuration when uninstalled from an account. + * Clears all signers, weights, and total weights. + * + * See {ERC7579Multisig-onUninstall}. + */ + function onUninstall(bytes calldata data) public virtual override { + address account = msg.sender; + + bytes[] memory allSigners = getSigners(account, 0, type(uint64).max); + uint256 allSignersLength = allSigners.length; + for (uint256 i = 0; i < allSignersLength; ++i) { + delete _extraWeights[account][allSigners[i]]; + } + delete _totalExtraWeight[account]; + + // Call parent implementation which will clear signers and threshold + super.onUninstall(data); + } + + /// @dev Gets the weight of a signer for a specific account. Returns 0 if the signer is not authorized. + function signerWeight(address account, bytes memory signer) public view virtual returns (uint64) { + unchecked { + // Safe cast, _setSignerWeights guarantees 1+_extraWeights is a uint64 + return uint64(isSigner(account, signer).toUint() * (1 + _extraWeights[account][signer])); + } + } + + /// @dev Gets the total weight of all signers for a specific account. + function totalWeight(address account) public view virtual returns (uint64) { + return (getSignerCount(account) + _totalExtraWeight[account]).toUint64(); + } + + /** + * @dev Sets weights for signers for the calling account. + * Can only be called by the account itself. + */ + function setSignerWeights(bytes[] memory signers, uint64[] memory weights) public virtual { + _setSignerWeights(msg.sender, signers, weights); + } + + /** + * @dev Sets weights for multiple signers at once. Internal version without access control. + * + * Requirements: + * + * * `signers` and `weights` arrays must have the same length. Reverts with {ERC7579MultisigMismatchedLength} on mismatch. + * * Each signer must exist in the set of authorized signers. Reverts with {ERC7579MultisigNonexistentSigner} if not. + * * Each weight must be greater than 0. Reverts with {ERC7579MultisigInvalidWeight} if not. + * * See {_validateReachableThreshold} for the threshold validation. + * + * Emits {ERC7579MultisigWeightChanged} for each signer. + */ + function _setSignerWeights(address account, bytes[] memory signers, uint64[] memory weights) internal virtual { + require(signers.length == weights.length, ERC7579MultisigMismatchedLength()); + + uint256 extraWeightAdded = 0; + uint256 extraWeightRemoved = 0; + for (uint256 i = 0; i < signers.length; ++i) { + bytes memory signer = signers[i]; + require(isSigner(account, signer), ERC7579MultisigNonexistentSigner(signer)); + + uint64 weight = weights[i]; + require(weight > 0, ERC7579MultisigInvalidWeight(signer, weight)); + + unchecked { + uint64 oldExtraWeight = _extraWeights[account][signer]; + uint64 newExtraWeight = weight - 1; + + if (oldExtraWeight != newExtraWeight) { + // Overflow impossible: weight values are bounded by uint64 and economic constraints + extraWeightRemoved += oldExtraWeight; + extraWeightAdded += _extraWeights[account][signer] = newExtraWeight; + emit ERC7579MultisigWeightChanged(account, signer, weight); + } + } + } + unchecked { + // Safe from underflow: `extraWeightRemoved` is bounded by `_totalExtraWeight` by construction + // and weight values are bounded by uint64 and economic constraints + _totalExtraWeight[account] = (uint256(_totalExtraWeight[account]) + extraWeightAdded - extraWeightRemoved) + .toUint64(); + } + _validateReachableThreshold(account); + } + + /** + * @dev Override to add weight tracking. See {ERC7579Multisig-_addSigners}. + * Each new signer has a default weight of 1. + * + * In cases where {totalWeight} is almost `type(uint64).max` (due to a large `_totalExtraWeight`), adding new + * signers could cause the {totalWeight} computation to overflow. Adding a {totalWeight} call after the new + * signers are added ensures no such overflow happens. + */ + function _addSigners(address account, bytes[] memory newSigners) internal virtual override { + super._addSigners(account, newSigners); + + // This will revert if the new signers cause an overflow + _validateReachableThreshold(account); + } + + /** + * @dev Override to handle weight tracking during removal. See {ERC7579Multisig-_removeSigners}. + * + * Just like {_addSigners}, this function does not emit {ERC7579MultisigWeightChanged} events. The + * {ERC7913SignerRemoved} event emitted by {ERC7579Multisig-_removeSigners} is enough to track weights here. + */ + function _removeSigners(address account, bytes[] memory oldSigners) internal virtual override { + // Clean up weights for removed signers + // + // The `extraWeightRemoved` is bounded by `_totalExtraWeight`. The `super._removeSigners` function will revert + // if the signers array contains any duplicates, ensuring each signer's weight is only counted once. Since + // `_totalExtraWeight` is stored as a `uint64`, the final subtraction operation is also safe. + unchecked { + uint64 extraWeightRemoved = 0; + for (uint256 i = 0; i < oldSigners.length; ++i) { + bytes memory signer = oldSigners[i]; + + extraWeightRemoved += _extraWeights[account][signer]; + delete _extraWeights[account][signer]; + } + _totalExtraWeight[account] -= extraWeightRemoved; + } + super._removeSigners(account, oldSigners); + } + + /** + * @dev Override to validate threshold against total weight instead of signer count. + * + * NOTE: This function intentionally does not call `super._validateReachableThreshold` because the base implementation + * assumes each signer has a weight of 1, which is a subset of this weighted implementation. Consider that multiple + * implementations of this function may exist in the contract, so important side effects may be missed + * depending on the linearization order. + */ + function _validateReachableThreshold(address account) internal view virtual override { + uint64 weight = totalWeight(account); + uint64 currentThreshold = threshold(account); + require(weight >= currentThreshold, ERC7579MultisigUnreachableThreshold(weight, currentThreshold)); + } + + /** + * @dev Validates that the total weight of signers meets the {threshold} requirement. + * Overrides the base implementation to use weights instead of count. + * + * NOTE: This function intentionally does not call `super._validateThreshold` because the base implementation + * assumes each signer has a weight of 1, which is incompatible with this weighted implementation. + */ + function _validateThreshold( + address account, + bytes[] memory validatingSigners + ) internal view virtual override returns (bool) { + unchecked { + uint64 weight = 0; + for (uint256 i = 0; i < validatingSigners.length; ++i) { + // Overflow impossible: weight values are bounded by uint64 and economic constraints + weight += signerWeight(account, validatingSigners[i]); + } + return weight >= threshold(account); + } + } +} diff --git a/contracts/account/modules/ERC7579Signature.sol b/contracts/account/modules/ERC7579Signature.sol deleted file mode 100644 index 45e1e60850b..00000000000 --- a/contracts/account/modules/ERC7579Signature.sol +++ /dev/null @@ -1,86 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.27; - -import {IERC7579Module} from "../../interfaces/draft-IERC7579.sol"; -import {SignatureChecker} from "../../utils/cryptography/SignatureChecker.sol"; -import {ERC7579Validator} from "./ERC7579Validator.sol"; - -/** - * @dev Implementation of {ERC7579Validator} module using ERC-7913 signature verification. - * - * This validator allows ERC-7579 accounts to integrate with address-less cryptographic keys - * and account signatures through the ERC-7913 signature verification system. Each account - * can store its own ERC-7913 formatted signer (a concatenation of a verifier address and a - * key: `verifier || key`). - * - * This enables accounts to use signature schemes without requiring each key to have its own - * Ethereum address.A smart account with this module installed can keep an emergency key as a - * backup. - */ -contract ERC7579Signature is ERC7579Validator { - mapping(address account => bytes signer) private _signers; - - /// @dev Emitted when the signer is set. - event ERC7579SignatureSignerSet(address indexed account, bytes signer); - - /// @dev Thrown when the signer length is less than 20 bytes. - error ERC7579SignatureInvalidSignerLength(); - - /// @dev Return the ERC-7913 signer (i.e. `verifier || key`). - function signer(address account) public view virtual returns (bytes memory) { - return _signers[account]; - } - - /** - * @dev See {IERC7579Module-onInstall}. - * - * NOTE: An account can only call onInstall once. If called directly by the account, - * the signer will be set to the provided data. Future installations will behave as a no-op. - */ - function onInstall(bytes calldata data) public virtual { - if (signer(msg.sender).length == 0) { - setSigner(data); - } - } - - /** - * @dev See {IERC7579Module-onUninstall}. - * - * WARNING: The signer's key will be removed if the account calls this function, potentially - * making the account unusable. As an account operator, make sure to uninstall to a predefined path - * in your account that properly handles side effects of uninstallation. See {AccountERC7579-uninstallModule}. - */ - function onUninstall(bytes calldata) public virtual { - _setSigner(msg.sender, ""); - } - - /// @dev Sets the ERC-7913 signer (i.e. `verifier || key`) for the calling account. - function setSigner(bytes memory signer_) public virtual { - require(signer_.length >= 20, ERC7579SignatureInvalidSignerLength()); - _setSigner(msg.sender, signer_); - } - - /// @dev Internal version of {setSigner} that takes an `account` as argument without validating `signer_`. - function _setSigner(address account, bytes memory signer_) internal virtual { - _signers[account] = signer_; - emit ERC7579SignatureSignerSet(account, signer_); - } - - /** - * @dev See {ERC7579Validator-_rawERC7579Validation}. - * - * Validates a `signature` using ERC-7913 verification. - * - * This base implementation ignores the `sender` parameter and validates using - * the account's stored signer. Derived contracts can override this to implement - * custom validation logic based on the sender. - */ - function _rawERC7579Validation( - address account, - bytes32 hash, - bytes calldata signature - ) internal view virtual override returns (bool) { - return SignatureChecker.isValidSignatureNow(signer(account), hash, signature); - } -} diff --git a/contracts/mocks/account/modules/ERC7579ValidatorMock.sol b/contracts/mocks/account/modules/ERC7579ValidatorMock.sol new file mode 100644 index 00000000000..b812814dfc0 --- /dev/null +++ b/contracts/mocks/account/modules/ERC7579ValidatorMock.sol @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.27; + +import {IERC7579Module} from "../../../interfaces/draft-IERC7579.sol"; +import {SignatureChecker} from "../../../utils/cryptography/SignatureChecker.sol"; +import {ERC7579Validator} from "../../../account/modules/ERC7579Validator.sol"; + +contract ERC7579Signature is ERC7579Validator { + mapping(address account => bytes signer) private _signers; + + event ERC7579SignatureSignerSet(address indexed account, bytes signer); + + error ERC7579SignatureInvalidSignerLength(); + + function signer(address account) public view virtual returns (bytes memory) { + return _signers[account]; + } + + function onInstall(bytes calldata data) public virtual { + if (signer(msg.sender).length == 0) { + setSigner(data); + } + } + + function onUninstall(bytes calldata) public virtual { + _setSigner(msg.sender, ""); + } + + function setSigner(bytes memory signer_) public virtual { + require(signer_.length >= 20, ERC7579SignatureInvalidSignerLength()); + _setSigner(msg.sender, signer_); + } + + function _setSigner(address account, bytes memory signer_) internal virtual { + _signers[account] = signer_; + emit ERC7579SignatureSignerSet(account, signer_); + } + + function _rawERC7579Validation( + address account, + bytes32 hash, + bytes calldata signature + ) internal view virtual override returns (bool) { + return SignatureChecker.isValidSignatureNow(signer(account), hash, signature); + } +} diff --git a/test/account/modules/ERC7579Multisig.test.js b/test/account/modules/ERC7579Multisig.test.js new file mode 100644 index 00000000000..57dc10b2798 --- /dev/null +++ b/test/account/modules/ERC7579Multisig.test.js @@ -0,0 +1,285 @@ +const { ethers, predeploy } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +const { impersonate } = require('../../helpers/account'); +const { ERC4337Helper } = require('../../helpers/erc4337'); +const { + MODULE_TYPE_VALIDATOR, + CALL_TYPE_CALL, + EXEC_TYPE_DEFAULT, + encodeMode, + encodeSingle, +} = require('../../helpers/erc7579'); +const { NonNativeSigner, MultiERC7913SigningKey } = require('../../helpers/signers'); +const { MAX_UINT64 } = require('../../helpers/constants'); + +const { shouldBehaveLikeERC7579Module } = require('./ERC7579Module.behavior'); + +// Prepare signers in advance +const signerECDSA1 = ethers.Wallet.createRandom(); +const signerECDSA2 = ethers.Wallet.createRandom(); +const signerECDSA3 = ethers.Wallet.createRandom(); +const signerECDSA4 = ethers.Wallet.createRandom(); // Unauthorized signer + +async function fixture() { + // Deploy ERC-7579 multisig module + const mock = await ethers.deployContract('$ERC7579Multisig'); + const target = await ethers.deployContract('CallReceiverMock'); + + // ERC-4337 env + const helper = new ERC4337Helper(); + await helper.wait(); + + // Prepare signers + const signers = [signerECDSA1.address, signerECDSA2.address]; + const threshold = 1; + const multiSigner = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA1, signerECDSA2])); + + // Prepare module installation data + const installData = ethers.AbiCoder.defaultAbiCoder().encode(['bytes[]', 'uint256'], [signers, threshold]); + + // ERC-7579 account + const mockAccount = await helper.newAccount('$AccountERC7579'); + const mockFromAccount = await impersonate(mockAccount.address).then(asAccount => mock.connect(asAccount)); + const mockAccountFromEntrypoint = await impersonate(predeploy.entrypoint.v08.target).then(asEntrypoint => + mockAccount.connect(asEntrypoint), + ); + + const moduleType = MODULE_TYPE_VALIDATOR; + + await mockAccount.deploy(); + + const args = [42, '0x1234']; + const data = target.interface.encodeFunctionData('mockFunctionWithArgs', args); + const calldata = encodeSingle(target, 0, data); + const mode = encodeMode({ callType: CALL_TYPE_CALL, execType: EXEC_TYPE_DEFAULT }); + + return { + moduleType, + mock, + mockAccount, + mockFromAccount, + mockAccountFromEntrypoint, + target, + installData, + args, + data, + calldata, + mode, + signers, + threshold, + multiSigner, + }; +} + +describe('ERC7579Multisig', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + shouldBehaveLikeERC7579Module(); + + it('sets initial signers and threshold on installation', async function () { + const tx = await this.mockAccountFromEntrypoint.installModule(this.moduleType, this.mock.target, this.installData); + + for (const signer of this.signers) { + await expect(tx) + .to.emit(this.mock, 'ERC7913SignerAdded') + .withArgs(this.mockAccount.address, signer.toLowerCase()); + } + + await expect(tx).to.emit(this.mock, 'ERC7913ThresholdSet').withArgs(this.mockAccount.address, this.threshold); + + // Verify signers and threshold + await expect(this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64)).to.eventually.deep.equal(this.signers); + await expect(this.mock.threshold(this.mockAccount.address)).to.eventually.equal(this.threshold); + + // onInstall is allowed again but is a noop + await this.mockFromAccount.onInstall( + ethers.AbiCoder.defaultAbiCoder().encode(['bytes[]', 'uint256'], [[signerECDSA3.address], 2]), + ); + + // Should still have the original signers and threshold + await expect(this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64)).to.eventually.deep.equal(this.signers); + await expect(this.mock.threshold(this.mockAccount.address)).to.eventually.equal(this.threshold); + }); + + it('cleans up signers and threshold on uninstallation', async function () { + await this.mockAccountFromEntrypoint.installModule(this.moduleType, this.mock.target, this.installData); + await this.mockAccountFromEntrypoint.uninstallModule(this.moduleType, this.mock.target, '0x'); + + // Verify signers and threshold are cleared + await expect(this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64)).to.eventually.deep.equal([]); + await expect(this.mock.threshold(this.mockAccount.address)).to.eventually.equal(0); + }); + + describe('signer management', function () { + beforeEach(async function () { + await this.mockAccountFromEntrypoint.installModule(this.moduleType, this.mock.target, this.installData); + }); + + it('reverts adding an invalid signer', async function () { + await expect(this.mockFromAccount.addSigners(['0x1234'])) + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigInvalidSigner') + .withArgs('0x1234'); + }); + + it('can add signers', async function () { + const newSigners = [signerECDSA3.address]; + + // Get signers before adding + const signersBefore = await this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64); + + // Add new signers + const tx = await this.mockFromAccount.addSigners(newSigners); + for (const signer of newSigners) { + await expect(tx) + .to.emit(this.mock, 'ERC7913SignerAdded') + .withArgs(this.mockAccount.address, signer.toLowerCase()); + } + + // Get signers after adding + const signersAfter = await this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64); + + // Check that new signers were added + expect(signersAfter.length).to.equal(signersBefore.length + 1); + expect(signersAfter.map(ethers.getAddress)).to.include(ethers.getAddress(signerECDSA3.address)); + + // Verify isSigner function + await expect(this.mock.isSigner(this.mockAccount.address, signerECDSA3.address)).to.eventually.be.true; + + // Reverts if the signer already exists + await expect(this.mockFromAccount.addSigners(newSigners)) + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigAlreadyExists') + .withArgs(signerECDSA3.address.toLowerCase()); + }); + + it('can remove signers', async function () { + const removedSigners = [signerECDSA1.address].map(address => address.toLowerCase()); + + // Get signers before removing + const signersBefore = await this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64); + + // Remove signers + const tx = await this.mockFromAccount.removeSigners(removedSigners); + for (const signer of removedSigners) { + await expect(tx) + .to.emit(this.mock, 'ERC7913SignerRemoved') + .withArgs(this.mockAccount.address, signer.toLowerCase()); + } + + // Get signers after removing + const signersAfter = await this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64); + + // Check that signers were removed + expect(signersAfter.length).to.equal(signersBefore.length - 1); + expect(signersAfter).to.not.include(signerECDSA1.address); + + // Verify isSigner function + await expect(this.mock.isSigner(this.mockAccount.address, signerECDSA1.address)).to.eventually.be.false; + + // Reverts if the signer doesn't exist + await expect(this.mockFromAccount.removeSigners(removedSigners)) + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigNonexistentSigner') + .withArgs(signerECDSA1.address.toLowerCase()); + + // Reverts if threshold becomes unreachable after removal + await this.mockFromAccount.setThreshold(1); + await expect(this.mockFromAccount.removeSigners([signerECDSA2.address])) + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigUnreachableThreshold') + .withArgs(0, 1); + }); + + it('can set threshold', async function () { + // Set threshold to 2 + await expect(this.mockFromAccount.setThreshold(2)) + .to.emit(this.mock, 'ERC7913ThresholdSet') + .withArgs(this.mockAccount.address, 2); + + // Verify threshold + await expect(this.mock.threshold(this.mockAccount.address)).to.eventually.equal(2); + + // Reverts if threshold is unreachable + await expect(this.mockFromAccount.setThreshold(3)) + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigUnreachableThreshold') + .withArgs(2, 3); + }); + }); + + describe('signature validation', function () { + beforeEach(async function () { + await this.mockAccountFromEntrypoint.installModule(this.moduleType, this.mock.target, this.installData); + }); + + it('validates multiple signatures meeting threshold', async function () { + // Set threshold to 2 + await this.mockFromAccount.setThreshold(2); + + // Create hash and sign it + const testMessage = 'test'; + const messageHash = ethers.hashMessage(testMessage); + const multiSignature = await this.multiSigner.signMessage(testMessage); + // Should succeed with valid signatures meeting threshold + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, multiSignature)).to + .eventually.be.true; + }); + + it('rejects signatures not meeting threshold', async function () { + // First set threshold to 2 + await this.mockFromAccount.setThreshold(2); + + // Create MultiERC7913SigningKey with one authorized signer + const multiSigner = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA1])); + + // Create hash and sign it + const testMessage = 'test'; + const messageHash = ethers.hashMessage(testMessage); + const multiSignature = await multiSigner.signMessage(testMessage); + + // Should fail because threshold is 2 but only 1 signature provided + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, multiSignature)).to + .eventually.be.false; + }); + + it('validates valid signatures meeting threshold', async function () { + // Create MultiERC7913SigningKey with one authorized signer + const multiSigner = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA1])); + + // Create hash and sign it + const testMessage = 'test'; + const messageHash = ethers.hashMessage(testMessage); + const multiSignature = await multiSigner.signMessage(testMessage); + + // Should succeed with valid signature meeting threshold + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, multiSignature)).to + .eventually.be.true; + }); + + it('rejects signatures from unauthorized signers', async function () { + // Create MultiERC7913SigningKey with unauthorized signer + const multiSigner = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA4])); + + // Create hash and sign it + const testMessage = 'test'; + const messageHash = ethers.hashMessage(testMessage); + const multiSignature = await multiSigner.signMessage(testMessage); + + // Should fail because signer is not authorized + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, multiSignature)).to + .eventually.be.false; + }); + + it('rejects invalid signatures from authorized signers', async function () { + // Create hash and sign it with a different message + const testMessage = 'test'; + const differentMessage = 'different test'; + const messageHash = ethers.hashMessage(testMessage); + const multiSignature = await this.multiSigner.signMessage(differentMessage); + + // Should fail because signature is for a different hash + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, multiSignature)).to + .eventually.be.false; + }); + }); +}); diff --git a/test/account/modules/ERC7579MultisigWeighted.test.js b/test/account/modules/ERC7579MultisigWeighted.test.js new file mode 100644 index 00000000000..a5b8dd3492c --- /dev/null +++ b/test/account/modules/ERC7579MultisigWeighted.test.js @@ -0,0 +1,364 @@ +const { ethers, predeploy } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +const { impersonate } = require('../../helpers/account'); +const { ERC4337Helper } = require('../../helpers/erc4337'); +const { + MODULE_TYPE_VALIDATOR, + CALL_TYPE_CALL, + EXEC_TYPE_DEFAULT, + encodeMode, + encodeSingle, +} = require('../../helpers/erc7579'); +const { NonNativeSigner, MultiERC7913SigningKey } = require('../../helpers/signers'); +const { MAX_UINT64 } = require('../../helpers/constants'); + +const { shouldBehaveLikeERC7579Module } = require('./ERC7579Module.behavior'); + +// Prepare signers in advance +const signerECDSA1 = ethers.Wallet.createRandom(); +const signerECDSA2 = ethers.Wallet.createRandom(); +const signerECDSA3 = ethers.Wallet.createRandom(); +const signerECDSA4 = ethers.Wallet.createRandom(); // Unauthorized signer + +async function fixture() { + // Deploy ERC-7579 multisig weighted module + const mock = await ethers.deployContract('$ERC7579MultisigWeighted'); + const target = await ethers.deployContract('CallReceiverMock'); + + // ERC-4337 env + const helper = new ERC4337Helper(); + await helper.wait(); + + // Prepare signers with weights + const signers = [signerECDSA1.address, signerECDSA2.address, signerECDSA3.address]; + const weights = [1, 2, 3]; // Different weights for each signer + const threshold = 3; // Set to 3 to match the default weights during initialization (3 signers × 1 weight = 3) + + // Create multi-signer instance + const multiSigner = new NonNativeSigner( + new MultiERC7913SigningKey([signerECDSA1, signerECDSA2, signerECDSA3], weights), + ); + + // Prepare module installation data + const installData = ethers.AbiCoder.defaultAbiCoder().encode( + ['bytes[]', 'uint256', 'uint256[]'], + [signers, threshold, weights], + ); + + // ERC-7579 account + const mockAccount = await helper.newAccount('$AccountERC7579'); + const mockFromAccount = await impersonate(mockAccount.address).then(asAccount => mock.connect(asAccount)); + const mockAccountFromEntrypoint = await impersonate(predeploy.entrypoint.v08.target).then(asEntrypoint => + mockAccount.connect(asEntrypoint), + ); + + const moduleType = MODULE_TYPE_VALIDATOR; + + await mockAccount.deploy(); + + const args = [42, '0x1234']; + const data = target.interface.encodeFunctionData('mockFunctionWithArgs', args); + const calldata = encodeSingle(target, 0, data); + const mode = encodeMode({ callType: CALL_TYPE_CALL, execType: EXEC_TYPE_DEFAULT }); + + return { + moduleType, + mock, + mockAccount, + mockFromAccount, + mockAccountFromEntrypoint, + target, + installData, + args, + data, + calldata, + mode, + signers, + weights, + threshold, + multiSigner, + }; +} + +describe('ERC7579MultisigWeighted', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + shouldBehaveLikeERC7579Module(); + + it('sets initial signers, weights, and threshold on installation', async function () { + const tx = await this.mockAccountFromEntrypoint.installModule(this.moduleType, this.mock.target, this.installData); + + for (const signer of this.signers) { + await expect(tx) + .to.emit(this.mock, 'ERC7913SignerAdded') + .withArgs(this.mockAccount.address, signer.toLowerCase()); + } + await expect(tx).to.emit(this.mock, 'ERC7913ThresholdSet').withArgs(this.mockAccount.address, this.threshold); + + // Verify signers and weights were set correctly + for (let i = 0; i < this.signers.length; i++) { + await expect(this.mock.signerWeight(this.mockAccount.address, this.signers[i])).to.eventually.equal( + this.weights[i], + ); + } + + // Verify threshold was set correctly + await expect(this.mock.threshold(this.mockAccount.address)).to.eventually.equal(this.threshold); + + // onInstall is allowed again but is a noop + const newSigners = [signerECDSA4.address]; + const newWeights = [5]; + const newThreshold = 10; + + await this.mockFromAccount.onInstall( + ethers.AbiCoder.defaultAbiCoder().encode( + ['bytes[]', 'uint256', 'uint256[]'], + [newSigners, newThreshold, newWeights], + ), + ); + + // Should still have the original signers, weights, and threshold + await expect(this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64)).to.eventually.deep.equal(this.signers); + + await expect(this.mock.threshold(this.mockAccount.address)).to.eventually.equal(this.threshold); + }); + + it('cleans up signers, weights, and threshold on uninstallation', async function () { + await this.mockAccountFromEntrypoint.installModule(this.moduleType, this.mock.target, this.installData); + await this.mockAccountFromEntrypoint.uninstallModule(this.moduleType, this.mock.target, '0x'); + + // Verify signers and threshold are cleared + await expect(this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64)).to.eventually.deep.equal([]); + await expect(this.mock.threshold(this.mockAccount.address)).to.eventually.equal(0); + + // Verify weights are cleared (by checking a previously existing signer) + await expect(this.mock.signerWeight(this.mockAccount.address, this.signers[0])).to.eventually.equal(0); + }); + + describe('signer and weight management', function () { + beforeEach(async function () { + await this.mockAccountFromEntrypoint.installModule(this.moduleType, this.mock.target, this.installData); + }); + + it('can add signers with default weight', async function () { + const newSigners = [signerECDSA4.address]; + + // Get signers before adding + const signersBefore = await this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64); + + // Add new signer + const tx = await this.mockFromAccount.addSigners(newSigners); + for (const signer of newSigners) { + await expect(tx) + .to.emit(this.mock, 'ERC7913SignerAdded') + .withArgs(this.mockAccount.address, signer.toLowerCase()); + } + + // Get signers after adding + const signersAfter = await this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64); + + // Check that new signer was added + expect(signersAfter.length).to.equal(signersBefore.length + 1); + expect(signersAfter.map(ethers.getAddress)).to.include(ethers.getAddress(signerECDSA4.address)); + + // Check that default weight is 1 + await expect(this.mock.signerWeight(this.mockAccount.address, signerECDSA4.address)).to.eventually.equal(1); + + // Check that total weight was updated + const totalWeight = await this.mock.totalWeight(this.mockAccount.address); + expect(totalWeight).to.equal(1 + 2 + 3 + 1); // Sum of all weights including new signer + }); + + it('can set signer weights', async function () { + // Set new weights for existing signers + const updateSigners = [this.signers[0], this.signers[1]]; + const newWeights = [5, 5]; + + await expect(this.mockFromAccount.setSignerWeights(updateSigners, newWeights)) + .to.emit(this.mock, 'ERC7579MultisigWeightChanged') + .withArgs(this.mockAccount.address, updateSigners[0].toLowerCase(), newWeights[0]) + .to.emit(this.mock, 'ERC7579MultisigWeightChanged') + .withArgs(this.mockAccount.address, updateSigners[1].toLowerCase(), newWeights[1]); + + // Verify new weights + await expect(this.mock.signerWeight(this.mockAccount.address, updateSigners[0])).to.eventually.equal( + newWeights[0], + ); + await expect(this.mock.signerWeight(this.mockAccount.address, updateSigners[1])).to.eventually.equal( + newWeights[1], + ); + + // Third signer weight should remain unchanged + await expect(this.mock.signerWeight(this.mockAccount.address, this.signers[2])).to.eventually.equal( + this.weights[2], + ); + + // Check total weight + await expect(this.mock.totalWeight(this.mockAccount.address)).to.eventually.equal(5 + 5 + 3); // Sum of all weights after update + }); + + it('cannot set weight to non-existent signer', async function () { + const randomSigner = ethers.Wallet.createRandom().address; + + // Reverts when setting weight for non-existent signer + await expect(this.mockFromAccount.setSignerWeights([randomSigner], [1])) + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigNonexistentSigner') + .withArgs(randomSigner.toLowerCase()); + }); + + it('cannot set weight to 0', async function () { + // Reverts when setting weight to 0 + await expect(this.mockFromAccount.setSignerWeights([this.signers[0]], [0])) + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigInvalidWeight') + .withArgs(this.signers[0].toLowerCase(), 0); + }); + + it('requires signers and weights arrays to have same length', async function () { + // Reverts when arrays have different lengths + await expect( + this.mockFromAccount.setSignerWeights([this.signers[0], this.signers[1]], [1]), + ).to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigMismatchedLength'); + }); + + it('can remove signers and updates total weight', async function () { + const removedSigner = this.signers[0].toLowerCase(); // weight = 1 + const weightBefore = await this.mock.totalWeight(this.mockAccount.address); + + // Remove signer + await expect(this.mockFromAccount.removeSigners([removedSigner])) + .to.emit(this.mock, 'ERC7913SignerRemoved') + .withArgs(this.mockAccount.address, removedSigner); + + // Check weight was updated + const weightAfter = await this.mock.totalWeight(this.mockAccount.address); + expect(weightAfter).to.equal(weightBefore - 1n); // Should be decreased by removed signer's weight + + // Cannot remove non-existent signer + await expect(this.mockFromAccount.removeSigners([removedSigner])) + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigNonexistentSigner') + .withArgs(removedSigner); + }); + + it('validates threshold is reachable when updating weights', async function () { + // Increase threshold to match total weight + const totalWeight = await this.mock.totalWeight(this.mockAccount.address); + + // Ensure totalWeight is what we expect (should be 6) + expect(totalWeight).to.equal(6); // 1+2+3 after weights are properly set + + // Set threshold to total weight + await this.mockFromAccount.setThreshold(totalWeight); + + // Now try to lower a weight, making total weight less than threshold + await expect(this.mockFromAccount.setSignerWeights([this.signers[2]], [1])) // Change weight from 3 to 1 + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigUnreachableThreshold') + .withArgs(totalWeight - 2n, totalWeight); // Total weight would be 2 less than threshold + }); + + it('prevents removing signers if threshold becomes unreachable', async function () { + // First check initial total weight + const initialTotalWeight = await this.mock.totalWeight(this.mockAccount.address); + expect(initialTotalWeight).to.equal(6); // 1+2+3 + + // Set threshold to current total weight + await this.mockFromAccount.setThreshold(initialTotalWeight); + + // Cannot remove a signer with weight > 0 as threshold would become unreachable + await expect(this.mockFromAccount.removeSigners([this.signers[0]])) // Weight 1 + .to.be.revertedWithCustomError(this.mock, 'ERC7579MultisigUnreachableThreshold') + .withArgs(initialTotalWeight - 1n, initialTotalWeight); + }); + }); + + describe('signature validation with weights', function () { + beforeEach(async function () { + await this.mockAccountFromEntrypoint.installModule(this.moduleType, this.mock.target, this.installData); + }); + + it('validates signatures meeting threshold through combined weights', async function () { + // Threshold is 3, signerECDSA1(weight=1) + signerECDSA2(weight=2) = 3, which equals threshold + // Or just signerECDSA3(weight=3) alone is enough + const testMessage = 'test'; + const messageHash = ethers.hashMessage(testMessage); + + // Try with exactly the threshold weight (1+2=3 = threshold 3) + const exactSigner = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA1, signerECDSA2])); + const exactSignature = await exactSigner.signMessage(testMessage); + + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, exactSignature)).to + .eventually.be.true; + + // Also works with all signers (1+2+3=6 > threshold 3) + const sufficientSignature = await this.multiSigner.signMessage(testMessage); + + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, sufficientSignature)).to + .eventually.be.true; + + // Also try with just signerECDSA3 (weight 3) = 3, exactly meeting threshold + const minimumSigner = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA3])); + const minimumSignature = await minimumSigner.signMessage(testMessage); + + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, minimumSignature)).to + .eventually.be.true; + }); + + it('rejects signatures that collectively miss threshold', async function () { + // Increase threshold to 4 (more than the default total of 3) + await this.mockFromAccount.setThreshold(4); + + // Single signer with weight 1 is insufficient + const testMessage = 'test'; + const messageHash = ethers.hashMessage(testMessage); + const insufficientSigner = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA1])); + const insufficientSignature = await insufficientSigner.signMessage(testMessage); + + // Should fail because total weight (1) < threshold (4) + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, insufficientSignature)).to + .eventually.be.false; + }); + + it('considers weight changes when validating signatures', async function () { + // Increase threshold to 4 + await this.mockFromAccount.setThreshold(4); + + const testMessage = 'test'; + const messageHash = ethers.hashMessage(testMessage); + + // Create signer with just signerECDSA1 + signerECDSA2 (weight 1+2=3 < threshold 4) + const insufficientSigner = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA1, signerECDSA2])); + const insufficientSignature = await insufficientSigner.signMessage(testMessage); + + // First verify this combination is insufficient + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, insufficientSignature)).to + .eventually.be.false; + + // Now increase the weight of signerECDSA2 to make it sufficient + await this.mockFromAccount.setSignerWeights([this.signers[1]], [3]); // Now weight is 1+3=4 >= threshold 4 + + // Same signature should now pass + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, insufficientSignature)).to + .eventually.be.true; + }); + + it('rejects invalid signatures regardless of weight', async function () { + // Even with a high weight, invalid signatures should be rejected + await this.mockFromAccount.setSignerWeights([this.signers[0]], [10]); // Very high weight + + const testMessage = 'test'; + const differentMessage = 'different test'; + const messageHash = ethers.hashMessage(testMessage); + + // Sign the wrong message + const invalidSigner = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA1])); + const invalidSignature = await invalidSigner.signMessage(differentMessage); + + // Should fail because signature is invalid for the hash + await expect(this.mock.$_rawERC7579Validation(this.mockAccount.address, messageHash, invalidSignature)).to + .eventually.be.false; + }); + }); +}); diff --git a/test/account/modules/ERC7579Signature.test.js b/test/account/modules/ERC7579Signature.test.js deleted file mode 100644 index cf09a48bf48..00000000000 --- a/test/account/modules/ERC7579Signature.test.js +++ /dev/null @@ -1,151 +0,0 @@ -const { ethers, predeploy } = require('hardhat'); -const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); -const { expect } = require('chai'); - -const { impersonate } = require('../../helpers/account'); -const { getDomain, PackedUserOperation } = require('../../helpers/eip712'); -const { ERC4337Helper } = require('../../helpers/erc4337'); -const { MODULE_TYPE_VALIDATOR } = require('../../helpers/erc7579'); -const { NonNativeSigner, P256SigningKey, RSASHA256SigningKey } = require('../../helpers/signers'); - -const { shouldBehaveLikeERC7579Module, shouldBehaveLikeERC7579Validator } = require('./ERC7579Module.behavior'); - -// Prepare signers in advance (RSA are long to initialize) -const signerECDSA = ethers.Wallet.createRandom(); -const signerP256 = new NonNativeSigner(P256SigningKey.random()); -const signerRSA = new NonNativeSigner(RSASHA256SigningKey.random()); - -async function fixture() { - const [other] = await ethers.getSigners(); - - // Deploy ERC-7579 signature validator - const mock = await ethers.deployContract('$ERC7579Signature'); - - // ERC-7913 verifiers - const verifierP256 = await ethers.deployContract('ERC7913P256Verifier'); - const verifierRSA = await ethers.deployContract('ERC7913RSAVerifier'); - - // ERC-4337 env - const helper = new ERC4337Helper(); - await helper.wait(); - const entrypointDomain = await getDomain(predeploy.entrypoint.v08); - - // ERC-7579 account - const mockAccount = await helper.newAccount('$AccountERC7579'); - const mockFromAccount = await impersonate(mockAccount.address).then(asAccount => mock.connect(asAccount)); - - return { - moduleType: MODULE_TYPE_VALIDATOR, - mock, - verifierP256, - verifierRSA, - mockFromAccount, - entrypointDomain, - mockAccount, - other, - }; -} - -function prepareSigner(prototype) { - this.signUserOp = userOp => - prototype.signTypedData - .call(this.signer, this.entrypointDomain, { PackedUserOperation }, userOp.packed) - .then(signature => Object.assign(userOp, { signature })); -} - -describe('ERC7579Signature', function () { - beforeEach(async function () { - Object.assign(this, await loadFixture(fixture)); - }); - - it('reverts with ERC7579SignatureInvalidSignerLength when signer length is less than 20 bytes', async function () { - const shortSigner = '0x0123456789'; // Less than 20 bytes - await expect(this.mockFromAccount.onInstall(shortSigner)).to.be.revertedWithCustomError( - this.mock, - 'ERC7579SignatureInvalidSignerLength', - ); - }); - - it('behaves as a noop when the validator is already installed for an account', async function () { - // First installation should succeed - const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]); - await expect(this.mockFromAccount.onInstall(signerData)).to.not.be.reverted; - - // Second installation should behave as a no-op - await this.mockFromAccount.onInstall(ethers.solidityPacked(['address'], [ethers.Wallet.createRandom().address])); // Not revert - await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal(signerData); // No change in signers - }); - - it('emits event on ERC7579SignatureSignerSet on both installation and uninstallation', async function () { - const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]); - - // First install - await expect(this.mockFromAccount.onInstall(signerData)) - .to.emit(this.mock, 'ERC7579SignatureSignerSet') - .withArgs(this.mockAccount.address, signerData); - - // Then uninstall - await expect(this.mockFromAccount.onUninstall('0x')) - .to.emit(this.mock, 'ERC7579SignatureSignerSet') - .withArgs(this.mockAccount.address, '0x'); - }); - - it('returns the correct signer bytes when set', async function () { - // Starts empty - await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal('0x'); - - const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]); - await this.mockFromAccount.onInstall(signerData); - - await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal(signerData); - }); - - it('sets signer correctly with setSigner and emits event', async function () { - const signerData = ethers.solidityPacked(['address'], [signerECDSA.address]); - await expect(this.mockFromAccount.setSigner(signerData)) - .to.emit(this.mockFromAccount, 'ERC7579SignatureSignerSet') - .withArgs(this.mockAccount.address, signerData); - await expect(this.mock.signer(this.mockAccount.address)).to.eventually.equal(signerData); - }); - - it('reverts when calling setSigner with invalid signer length', async function () { - await expect(this.mock.setSigner('0x0123456789')).to.be.revertedWithCustomError( - this.mock, - 'ERC7579SignatureInvalidSignerLength', - ); - }); - - // ECDSA tested in ./ERC7579Validator.test.js - - describe('P256 key', function () { - beforeEach(async function () { - this.signer = signerP256; - prepareSigner.call(this, new NonNativeSigner(this.signer.signingKey)); - this.installData = ethers.concat([ - this.verifierP256.target, - this.signer.signingKey.publicKey.qx, - this.signer.signingKey.publicKey.qy, - ]); - }); - - shouldBehaveLikeERC7579Module(); - shouldBehaveLikeERC7579Validator(); - }); - - describe('RSA key', function () { - beforeEach(async function () { - this.signer = signerRSA; - prepareSigner.call(this, new NonNativeSigner(this.signer.signingKey)); - this.installData = ethers.concat([ - this.verifierRSA.target, - ethers.AbiCoder.defaultAbiCoder().encode( - ['bytes', 'bytes'], - [this.signer.signingKey.publicKey.e, this.signer.signingKey.publicKey.n], - ), - ]); - }); - - shouldBehaveLikeERC7579Module(); - shouldBehaveLikeERC7579Validator(); - }); -}); diff --git a/test/account/modules/ERC7579Validator.test.js b/test/account/modules/ERC7579Validator.test.js index 83a6539da3d..b4ed91b73dd 100644 --- a/test/account/modules/ERC7579Validator.test.js +++ b/test/account/modules/ERC7579Validator.test.js @@ -12,7 +12,7 @@ async function fixture() { const [other] = await ethers.getSigners(); // Deploy ERC-7579 validator module - const mock = await ethers.deployContract('$ERC7579Signature'); + const mock = await ethers.deployContract('$ERC7579ValidatorMock'); // ERC-4337 env const helper = new ERC4337Helper(); From 709dac823b56ab71aeb1d19cf7046dcae267aa44 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 14 Oct 2025 13:50:58 -0600 Subject: [PATCH 4/8] Add missing changesets and docs items --- .changeset/brown-flies-poke.md | 5 +++++ .changeset/shy-grapes-ask.md | 5 +++++ contracts/account/README.adoc | 6 ++++++ 3 files changed, 16 insertions(+) create mode 100644 .changeset/brown-flies-poke.md create mode 100644 .changeset/shy-grapes-ask.md diff --git a/.changeset/brown-flies-poke.md b/.changeset/brown-flies-poke.md new file mode 100644 index 00000000000..b846a5f3820 --- /dev/null +++ b/.changeset/brown-flies-poke.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC7579Multisig`: Add an abstract multisig module for ERC-7579 accounts using ERC-7913 signer keys. diff --git a/.changeset/shy-grapes-ask.md b/.changeset/shy-grapes-ask.md new file mode 100644 index 00000000000..bb1ca78eb97 --- /dev/null +++ b/.changeset/shy-grapes-ask.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC7579MultisigWeighted`: Add an abstract weighted multisig module that allows different weights to be assigned to signers. diff --git a/contracts/account/README.adoc b/contracts/account/README.adoc index a70d56ff356..7f77f4188df 100644 --- a/contracts/account/README.adoc +++ b/contracts/account/README.adoc @@ -8,6 +8,8 @@ This directory includes contracts to build accounts for ERC-4337. These include: * {AccountERC7579}: An extension of `Account` that implements support for ERC-7579 modules. * {AccountERC7579Hooked}: An extension of `AccountERC7579` with support for a single hook module (type 4). * {ERC7579Validator}: Abstract validator module for ERC-7579 accounts that provides base implementation for signature validation. + * {ERC7579Multisig}: An extension of {ERC7579Validator} that enables validation using ERC-7913 signer keys. + * {ERC7579MultisigWeighted}: An extension of {ERC7579Multisig} that allows different weights to be assigned to signers. * {ERC7821}: Minimal batch executor implementation contracts. Useful to enable easy batch execution for smart contracts. * {ERC4337Utils}: Utility functions for working with ERC-4337 user operations. * {ERC7579Utils}: Utility functions for working with ERC-7579 modules and account modularity. @@ -28,6 +30,10 @@ This directory includes contracts to build accounts for ERC-4337. These include: {{ERC7579Validator}} +{{ERC7579Multisig}} + +{{ERC7579MultisigWeighted}} + == Utilities {{ERC4337Utils}} From d8ddaad46f6b26adc11e14248b3930104172cc06 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 14 Oct 2025 20:50:29 -0600 Subject: [PATCH 5/8] Update ERC7579ValidatorMock --- .../mocks/account/modules/ERC7579Mock.sol | 34 +++++-------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/contracts/mocks/account/modules/ERC7579Mock.sol b/contracts/mocks/account/modules/ERC7579Mock.sol index 41083186cb2..93103092117 100644 --- a/contracts/mocks/account/modules/ERC7579Mock.sol +++ b/contracts/mocks/account/modules/ERC7579Mock.sol @@ -5,15 +5,14 @@ pragma solidity ^0.8.24; import { MODULE_TYPE_HOOK, MODULE_TYPE_FALLBACK, - MODULE_TYPE_VALIDATOR, IERC7579Hook, - IERC7579Module, - IERC7579Validator + IERC7579Module } from "../../../interfaces/draft-IERC7579.sol"; import {SignatureChecker} from "../../../utils/cryptography/SignatureChecker.sol"; import {PackedUserOperation} from "../../../interfaces/draft-IERC4337.sol"; import {IERC1271} from "../../../interfaces/IERC1271.sol"; import {ERC4337Utils} from "../../../account/utils/draft-ERC4337Utils.sol"; +import {ERC7579Validator} from "../../../account/modules/ERC7579Validator.sol"; abstract contract ERC7579ModuleMock is IERC7579Module { uint256 private _moduleTypeId; @@ -86,37 +85,22 @@ abstract contract ERC7579FallbackHandlerMock is ERC7579ModuleMock(MODULE_TYPE_FA } } -abstract contract ERC7579ValidatorMock is ERC7579ModuleMock(MODULE_TYPE_VALIDATOR), IERC7579Validator { +abstract contract ERC7579ValidatorMock is ERC7579Validator { mapping(address sender => address signer) private _associatedSigners; - function onInstall(bytes calldata data) public virtual override(IERC7579Module, ERC7579ModuleMock) { + function onInstall(bytes calldata data) public virtual override { _associatedSigners[msg.sender] = address(bytes20(data[0:20])); - super.onInstall(data); } - function onUninstall(bytes calldata data) public virtual override(IERC7579Module, ERC7579ModuleMock) { + function onUninstall(bytes calldata /* data */) public virtual override { delete _associatedSigners[msg.sender]; - super.onUninstall(data); } - function validateUserOp( - PackedUserOperation calldata userOp, - bytes32 userOpHash - ) public view virtual returns (uint256) { - return - SignatureChecker.isValidSignatureNow(_associatedSigners[msg.sender], userOpHash, userOp.signature) - ? ERC4337Utils.SIG_VALIDATION_SUCCESS - : ERC4337Utils.SIG_VALIDATION_FAILED; - } - - function isValidSignatureWithSender( - address /*sender*/, + function _rawERC7579Validation( + address account, bytes32 hash, bytes calldata signature - ) public view virtual returns (bytes4) { - return - SignatureChecker.isValidSignatureNow(_associatedSigners[msg.sender], hash, signature) - ? IERC1271.isValidSignature.selector - : bytes4(0xffffffff); + ) internal view virtual override returns (bool) { + return SignatureChecker.isValidSignatureNow(_associatedSigners[account], hash, signature); } } From 8c5fd32635ca418c0dd548eed0d2a4f647dbe783 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Tue, 14 Oct 2025 20:52:25 -0600 Subject: [PATCH 6/8] Remove unnecessary mock --- .../account/modules/ERC7579ValidatorMock.sol | 47 ------------------- 1 file changed, 47 deletions(-) delete mode 100644 contracts/mocks/account/modules/ERC7579ValidatorMock.sol diff --git a/contracts/mocks/account/modules/ERC7579ValidatorMock.sol b/contracts/mocks/account/modules/ERC7579ValidatorMock.sol deleted file mode 100644 index b812814dfc0..00000000000 --- a/contracts/mocks/account/modules/ERC7579ValidatorMock.sol +++ /dev/null @@ -1,47 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.27; - -import {IERC7579Module} from "../../../interfaces/draft-IERC7579.sol"; -import {SignatureChecker} from "../../../utils/cryptography/SignatureChecker.sol"; -import {ERC7579Validator} from "../../../account/modules/ERC7579Validator.sol"; - -contract ERC7579Signature is ERC7579Validator { - mapping(address account => bytes signer) private _signers; - - event ERC7579SignatureSignerSet(address indexed account, bytes signer); - - error ERC7579SignatureInvalidSignerLength(); - - function signer(address account) public view virtual returns (bytes memory) { - return _signers[account]; - } - - function onInstall(bytes calldata data) public virtual { - if (signer(msg.sender).length == 0) { - setSigner(data); - } - } - - function onUninstall(bytes calldata) public virtual { - _setSigner(msg.sender, ""); - } - - function setSigner(bytes memory signer_) public virtual { - require(signer_.length >= 20, ERC7579SignatureInvalidSignerLength()); - _setSigner(msg.sender, signer_); - } - - function _setSigner(address account, bytes memory signer_) internal virtual { - _signers[account] = signer_; - emit ERC7579SignatureSignerSet(account, signer_); - } - - function _rawERC7579Validation( - address account, - bytes32 hash, - bytes calldata signature - ) internal view virtual override returns (bool) { - return SignatureChecker.isValidSignatureNow(signer(account), hash, signature); - } -} From b71f18017214b614f2765b520f355092f1871c9a Mon Sep 17 00:00:00 2001 From: ernestognw Date: Thu, 16 Oct 2025 16:12:54 -0600 Subject: [PATCH 7/8] Decode in calldata --- .changeset/fine-teams-serve.md | 5 + contracts/account/modules/ERC7579Multisig.sol | 130 ++++++++++++++++-- .../modules/ERC7579MultisigWeighted.sol | 30 +++- .../utils/cryptography/SignatureChecker.sol | 56 ++++++++ .../cryptography/SignatureChecker.test.js | 107 +++++++++++++- 5 files changed, 310 insertions(+), 18 deletions(-) create mode 100644 .changeset/fine-teams-serve.md diff --git a/.changeset/fine-teams-serve.md b/.changeset/fine-teams-serve.md new file mode 100644 index 00000000000..241d5fac807 --- /dev/null +++ b/.changeset/fine-teams-serve.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`SignatureChecker`: Add `isValidSignatureNowCalldata(bytes,bytes32,bytes)` and `areValidSignaturesNowCalldata(hash,bytes[],bytes[])` that take arguments in calldata rather than memory. diff --git a/contracts/account/modules/ERC7579Multisig.sol b/contracts/account/modules/ERC7579Multisig.sol index f71a2b9d4b9..deac3856e5f 100644 --- a/contracts/account/modules/ERC7579Multisig.sol +++ b/contracts/account/modules/ERC7579Multisig.sol @@ -30,6 +30,12 @@ abstract contract ERC7579Multisig is ERC7579Validator { /// @dev Emitted when signers are removed. event ERC7913SignerRemoved(address indexed account, bytes signer); + /// @dev The `initData` is invalid. + error ERC7579MultisigInvalidInitData(); + + /// @dev The signature format for the signature provided to `_rawERC7579Validation` is invalid. + error ERC7579MultisigInvalidValidation(); + /// @dev Emitted when the threshold is updated. event ERC7913ThresholdSet(address indexed account, uint64 threshold); @@ -56,8 +62,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { * See {ERC7579DelayedExecutor-onInstall}. Besides the delay setup, the `initdata` can * include `signers` and `threshold`. * - * The initData should be encoded as: - * `abi.encode(bytes[] signers, uint64 threshold)` + * See {_decodeMultisigInitData} for the encoding details. * * If no signers or threshold are provided, the multisignature functionality will be * disabled until they are added later. @@ -68,7 +73,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { function onInstall(bytes calldata initData) public virtual { if (initData.length > 32 && getSignerCount(msg.sender) == 0) { // More than just delay parameter - (bytes[] memory signers_, uint64 threshold_) = abi.decode(initData, (bytes[], uint64)); + (bytes[] calldata signers_, uint64 threshold_) = _decodeMultisigInitData(initData); _addSigners(msg.sender, signers_); _setThreshold(msg.sender, threshold_); } @@ -161,8 +166,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { * @dev Returns whether the number of valid signatures meets or exceeds the * threshold set for the target account. * - * The signature should be encoded as: - * `abi.encode(bytes[] signingSigners, bytes[] signatures)` + * See {_decodeValidationSignature} for the encoding details. * * Where `signingSigners` are the authorized signers and signatures are their corresponding * signatures of the operation `hash`. @@ -172,12 +176,118 @@ abstract contract ERC7579Multisig is ERC7579Validator { bytes32 hash, bytes calldata signature ) internal view virtual override returns (bool) { - (bytes[] memory signingSigners, bytes[] memory signatures) = abi.decode(signature, (bytes[], bytes[])); + (bytes[] calldata signingSigners, bytes[] calldata signatures) = _decodeValidationSignature(signature); return _validateThreshold(account, signingSigners) && _validateSignatures(account, hash, signingSigners, signatures); } + /** + * @dev Decodes multisig initialization data from calldata without memory allocation. + * + * The initData should be encoded as: + * `abi.encode(bytes[] signers, uint64 threshold)` + * + * Requirements: + * + * * The `initData` must be at least 96 bytes long to contain the minimum ABI-encoded structure. + * * The signers array offset must be properly aligned and within bounds. + * * The array length field must be accessible within the provided data. + */ + function _decodeMultisigInitData( + bytes calldata initData + ) internal pure virtual returns (bytes[] calldata signers, uint64 threshold_) { + // Minimum length: offset(32) + threshold(32) + array_length(32) = 96 bytes + require(initData.length > 0x5f, ERC7579MultisigInvalidInitData()); + + // Get offset to the signers array (first 32 bytes) + uint256 signersOffset = uint256(bytes32(initData[:0x20])); + + // Get the threshold (second 32 bytes, but only use the last 8 bytes as uint64) + threshold_ = uint64(uint256(bytes32(initData[0x20:0x40]))); + + // The signers data starts after the length field + uint256 signersDataOffset = signersOffset + 0x20; + + // Validate offset is within bounds and has space for array length + require(signersOffset > 0x3f && signersDataOffset <= initData.length, ERC7579MultisigInvalidInitData()); + uint256 signersLength = uint256(bytes32(initData[signersOffset:signersDataOffset])); + + // Set up the calldata slice for the signers array + assembly ("memory-safe") { + signers.offset := add(initData.offset, signersDataOffset) + signers.length := signersLength + } + + return (signers, threshold_); + } + + /** + * @dev Decodes validation signature data from calldata without memory allocation. + * + * The signature should be encoded as: + * `abi.encode(bytes[] signingSigners, bytes[] signatures)` + * + * Where `signingSigners` are the authorized signers and `signatures` are their corresponding + * signatures of the operation hash. This function is equivalent to + * `abi.decode(signature, (bytes[], bytes[]))` but operates directly on calldata to avoid + * memory allocation, improving gas efficiency during signature validation. + * + * Requirements: + * + * * The `signature` must be at least 128 bytes long to contain the minimum ABI-encoded structure. + * * Both array offsets must be properly aligned and within bounds. + * * Both array length fields must be accessible within the provided data. + * * The arrays should have the same length for proper signature validation. + */ + function _decodeValidationSignature( + bytes calldata signature + ) internal pure virtual returns (bytes[] calldata signingSigners, bytes[] calldata signatures) { + // Minimum length: offset1(32) + offset2(32) + array1_length(32) + array2_length(32) = 128 bytes + require(signature.length > 0x7f, ERC7579MultisigInvalidValidation()); + + // Get offset to the first array (signingSigners) + uint256 signersOffset = uint256(bytes32(signature[:0x20])); + + // Get offset to the second array (signatures) + uint256 signaturesOffset = uint256(bytes32(signature[0x20:0x40])); + + // Validate offsets are within bounds and properly aligned + require( + signersOffset > 0x3f && + signersOffset + 0x1f < signature.length && + signaturesOffset > 0x3f && + signaturesOffset + 0x1f < signature.length, + ERC7579MultisigInvalidValidation() + ); + + // Get the signers array length + uint256 signersLength = uint256(bytes32(signature[signersOffset:signersOffset + 0x20])); + + // Get the signatures array length + uint256 signaturesLength = uint256(bytes32(signature[signaturesOffset:signaturesOffset + 0x20])); + + // The array data starts after the length field + uint256 signersDataOffset = signersOffset + 0x20; + uint256 signaturesDataOffset = signaturesOffset + 0x20; + + // Validate data offsets are within bounds + require( + signersDataOffset <= signature.length && signaturesDataOffset <= signature.length, + ERC7579MultisigInvalidValidation() + ); + + // Set up the calldata slices for both arrays + assembly ("memory-safe") { + signingSigners.offset := add(signature.offset, signersDataOffset) + signingSigners.length := signersLength + signatures.offset := add(signature.offset, signaturesDataOffset) + signatures.length := signaturesLength + } + + return (signingSigners, signatures); + } + /** * @dev Adds the `newSigners` to those allowed to sign on behalf of the account. * @@ -261,15 +371,15 @@ abstract contract ERC7579Multisig is ERC7579Validator { function _validateSignatures( address account, bytes32 hash, - bytes[] memory signingSigners, - bytes[] memory signatures + bytes[] calldata signingSigners, + bytes[] calldata signatures ) internal view virtual returns (bool valid) { for (uint256 i = 0; i < signingSigners.length; ++i) { if (!isSigner(account, signingSigners[i])) { return false; } } - return hash.areValidSignaturesNow(signingSigners, signatures); + return hash.areValidSignaturesNowCalldata(signingSigners, signatures); } /** @@ -278,7 +388,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { */ function _validateThreshold( address account, - bytes[] memory validatingSigners + bytes[] calldata validatingSigners ) internal view virtual returns (bool) { return validatingSigners.length >= threshold(account); } diff --git a/contracts/account/modules/ERC7579MultisigWeighted.sol b/contracts/account/modules/ERC7579MultisigWeighted.sol index 7c6983a521d..e125553dbb4 100644 --- a/contracts/account/modules/ERC7579MultisigWeighted.sol +++ b/contracts/account/modules/ERC7579MultisigWeighted.sol @@ -64,7 +64,7 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { bool installed = getSignerCount(msg.sender) > 0; super.onInstall(initData); if (initData.length > 96 && !installed) { - (bytes[] memory signers, , uint64[] memory weights) = abi.decode(initData, (bytes[], uint64, uint64[])); + (bytes[] calldata signers, , uint64[] calldata weights) = _decodeMultisigWeightedInitData(initData); _setSignerWeights(msg.sender, signers, weights); } } @@ -110,6 +110,32 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { _setSignerWeights(msg.sender, signers, weights); } + function _decodeMultisigWeightedInitData( + bytes calldata initData + ) internal pure virtual returns (bytes[] calldata signers, uint64 threshold_, uint64[] calldata weights) { + (signers, threshold_) = _decodeMultisigInitData(initData); + + // Get offset to the weights array (third 32 bytes) + uint256 weightsOffset = uint256(bytes32(initData[0x40:0x60])); + + // Get the weights array length + uint256 weightsLength = uint256(bytes32(initData[weightsOffset:weightsOffset + 0x20])); + + // The weights data starts after the length field + uint256 weightsDataOffset = weightsOffset + 0x20; + + // Validate offset is within bounds and has space for array length + require(weightsOffset > 0x3f && weightsDataOffset <= initData.length, ERC7579MultisigInvalidInitData()); + + // Set up the calldata slice for the weights array + assembly ("memory-safe") { + weights.offset := add(initData.offset, weightsDataOffset) + weights.length := weightsLength + } + + return (signers, threshold_, weights); + } + /** * @dev Sets weights for multiple signers at once. Internal version without access control. * @@ -218,7 +244,7 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { */ function _validateThreshold( address account, - bytes[] memory validatingSigners + bytes[] calldata validatingSigners ) internal view virtual override returns (bool) { unchecked { uint64 weight = 0; diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index ebf6f92cf7e..9b39058d189 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -121,6 +121,28 @@ library SignatureChecker { } } + /** + * @dev Variant of {isValidSignatureNow-bytes-bytes32-bytes-} that takes a signature in calldata + */ + function isValidSignatureNowCalldata( + bytes calldata signer, + bytes32 hash, + bytes calldata signature + ) internal view returns (bool) { + if (signer.length < 20) { + return false; + } else if (signer.length == 20) { + return isValidSignatureNowCalldata(address(bytes20(signer)), hash, signature); + } else { + (bool success, bytes memory result) = address(bytes20(signer)).staticcall( + abi.encodeCall(IERC7913SignatureVerifier.verify, (signer[20:], hash, signature)) + ); + return (success && + result.length >= 32 && + abi.decode(result, (bytes32)) == bytes32(IERC7913SignatureVerifier.verify.selector)); + } + } + /** * @dev Verifies multiple ERC-7913 `signatures` for a given `hash` using a set of `signers`. * Returns `false` if the number of signers and signatures is not the same. @@ -161,4 +183,38 @@ library SignatureChecker { return true; } + + /** + * @dev Variant of {areValidSignaturesNow} that takes signers and signatures in calldata + */ + function areValidSignaturesNowCalldata( + bytes32 hash, + bytes[] calldata signers, + bytes[] calldata signatures + ) internal view returns (bool) { + if (signers.length != signatures.length) return false; + + bytes32 lastId = bytes32(0); + + for (uint256 i = 0; i < signers.length; ++i) { + bytes calldata signer = signers[i]; + + // If one of the signatures is invalid, reject the batch + if (!isValidSignatureNowCalldata(signer, hash, signatures[i])) return false; + + bytes32 id = keccak256(signer); + // If the current signer ID is greater than all previous IDs, then this is a new signer. + if (lastId < id) { + lastId = id; + } else { + // If this signer id is not greater than all the previous ones, verify that it is not a duplicate of a previous one + // This loop is never executed if the signers are ordered by id. + for (uint256 j = 0; j < i; ++j) { + if (id == keccak256(signers[j])) return false; + } + } + } + + return true; + } } diff --git a/test/utils/cryptography/SignatureChecker.test.js b/test/utils/cryptography/SignatureChecker.test.js index 82561d53877..23e1fb212bd 100644 --- a/test/utils/cryptography/SignatureChecker.test.js +++ b/test/utils/cryptography/SignatureChecker.test.js @@ -36,24 +36,39 @@ describe('SignatureChecker (ERC1271)', function () { await expect( this.mock.$isValidSignatureNow(ethers.Typed.address(this.signer.address), TEST_MESSAGE_HASH, this.signature), ).to.eventually.be.true; - await expect(this.mock.$isValidSignatureNowCalldata(this.signer.address, TEST_MESSAGE_HASH, this.signature)).to - .eventually.be.true; + await expect( + this.mock.$isValidSignatureNowCalldata( + ethers.Typed.address(this.signer.address), + TEST_MESSAGE_HASH, + this.signature, + ), + ).to.eventually.be.true; }); it('with invalid signer', async function () { await expect( this.mock.$isValidSignatureNow(ethers.Typed.address(this.other.address), TEST_MESSAGE_HASH, this.signature), ).to.eventually.be.false; - await expect(this.mock.$isValidSignatureNowCalldata(this.other.address, TEST_MESSAGE_HASH, this.signature)).to - .eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata( + ethers.Typed.address(this.other.address), + TEST_MESSAGE_HASH, + this.signature, + ), + ).to.eventually.be.false; }); it('with invalid signature', async function () { await expect( this.mock.$isValidSignatureNow(ethers.Typed.address(this.signer.address), WRONG_MESSAGE_HASH, this.signature), ).to.eventually.be.false; - await expect(this.mock.$isValidSignatureNowCalldata(this.signer.address, WRONG_MESSAGE_HASH, this.signature)).to - .eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata( + ethers.Typed.address(this.signer.address), + WRONG_MESSAGE_HASH, + this.signature, + ), + ).to.eventually.be.false; }); }); @@ -117,6 +132,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature)).to .eventually.be.true; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature), + ).to.eventually.be.true; }); it('with invalid signer', async function () { @@ -124,6 +142,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature), + ).to.eventually.be.false; }); it('with invalid signature', async function () { @@ -131,6 +152,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(eoaSigner), WRONG_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(eoaSigner), WRONG_MESSAGE_HASH, signature), + ).to.eventually.be.false; }); }); @@ -140,6 +164,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature)) .to.eventually.be.true; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature), + ).to.eventually.be.true; }); it('with invalid signer', async function () { @@ -147,6 +174,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature)) .to.eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature), + ).to.eventually.be.false; }); it('with invalid signature', async function () { @@ -154,6 +184,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(walletSigner), WRONG_MESSAGE_HASH, signature)) .to.eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(walletSigner), WRONG_MESSAGE_HASH, signature), + ).to.eventually.be.false; }); }); @@ -168,6 +201,8 @@ describe('SignatureChecker (ERC1271)', function () { await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.true; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.true; }); it('with invalid verifier', async function () { @@ -180,6 +215,8 @@ describe('SignatureChecker (ERC1271)', function () { await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); it('with invalid key', async function () { @@ -188,6 +225,8 @@ describe('SignatureChecker (ERC1271)', function () { await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); it('with invalid signature', async function () { @@ -200,6 +239,8 @@ describe('SignatureChecker (ERC1271)', function () { await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); it('with signer too short', async function () { @@ -207,6 +248,8 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await aliceP256.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); }); }); @@ -220,6 +263,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$areValidSignaturesNow(TEST_MESSAGE_HASH, [signer], [signature])).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata(TEST_MESSAGE_HASH, [ethers.Typed.bytes(signer)], [signature]), + ).to.eventually.be.true; }); it('should validate multiple signatures with different signer types', async function () { @@ -249,6 +295,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.true; }); it('should validate multiple EOA signatures', async function () { @@ -270,6 +323,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.true; }); it('should validate multiple ERC-1271 wallet signatures', async function () { @@ -291,6 +351,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.true; }); it('should validate multiple ERC-7913 signatures (ordered by ID)', async function () { @@ -320,6 +387,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.true; }); it('should validate multiple ERC-7913 signatures (unordered)', async function () { @@ -370,6 +444,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.false; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.false; }); it('should return false if there are duplicate signers', async function () { @@ -391,6 +472,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.false; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.false; }); it('should return false if signatures array length does not match signers array length', async function () { @@ -412,6 +500,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature).slice(1), ), ).to.eventually.be.false; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature).slice(1), + ), + ).to.eventually.be.false; }); it('should pass with empty arrays', async function () { From 78f304024e122d1e3055221dc6a5d26f23f34c63 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Thu, 16 Oct 2025 16:15:32 -0600 Subject: [PATCH 8/8] Nit --- contracts/account/modules/ERC7579Multisig.sol | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/contracts/account/modules/ERC7579Multisig.sol b/contracts/account/modules/ERC7579Multisig.sol index deac3856e5f..177fe9b7fdf 100644 --- a/contracts/account/modules/ERC7579Multisig.sol +++ b/contracts/account/modules/ERC7579Multisig.sol @@ -59,7 +59,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { /** * @dev Sets up the module's initial configuration when installed by an account. - * See {ERC7579DelayedExecutor-onInstall}. Besides the delay setup, the `initdata` can + * See {ERC7579Validator-onInstall}. Besides the delay setup, the `initdata` can * include `signers` and `threshold`. * * See {_decodeMultisigInitData} for the encoding details. @@ -72,7 +72,6 @@ abstract contract ERC7579Multisig is ERC7579Validator { */ function onInstall(bytes calldata initData) public virtual { if (initData.length > 32 && getSignerCount(msg.sender) == 0) { - // More than just delay parameter (bytes[] calldata signers_, uint64 threshold_) = _decodeMultisigInitData(initData); _addSigners(msg.sender, signers_); _setThreshold(msg.sender, threshold_); @@ -83,7 +82,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { * @dev Cleans up module's configuration when uninstalled from an account. * Clears all signers and resets the threshold. * - * See {ERC7579DelayedExecutor-onUninstall}. + * See {ERC7579Validator-onUninstall}. * * WARNING: This function has unbounded gas costs and may become uncallable if the set grows too large. * See {EnumerableSet-clear}.