diff --git a/contracts/account/modules/ERC7579Multisig.sol b/contracts/account/modules/ERC7579Multisig.sol index dcf847c6..7efc8dfd 100644 --- a/contracts/account/modules/ERC7579Multisig.sol +++ b/contracts/account/modules/ERC7579Multisig.sol @@ -31,7 +31,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { event ERC7913SignerRemoved(address indexed account, bytes signer); /// @dev Emitted when the threshold is updated. - event ERC7913ThresholdSet(address indexed account, uint256 threshold); + event ERC7913ThresholdSet(address indexed account, uint64 threshold); /// @dev The `signer` already exists. error ERC7579MultisigAlreadyExists(bytes signer); @@ -42,11 +42,14 @@ abstract contract ERC7579Multisig is ERC7579Validator { /// @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(uint256 signers, uint256 threshold); + error ERC7579MultisigUnreachableThreshold(uint64 signers, uint64 threshold); mapping(address account => EnumerableSet.BytesSet) private _signersSetByAccount; - mapping(address account => uint256) private _thresholdByAccount; + mapping(address account => uint64) private _thresholdByAccount; /** * @dev Sets up the module's initial configuration when installed by an account. @@ -54,7 +57,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { * include `signers` and `threshold`. * * The initData should be encoded as: - * `abi.encode(bytes[] signers, uint256 threshold)` + * `abi.encode(bytes[] signers, uint64 threshold)` * * If no signers or threshold are provided, the multisignature functionality will be * disabled until they are added later. @@ -63,9 +66,9 @@ abstract contract ERC7579Multisig is ERC7579Validator { * 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 && _signers(msg.sender).length() == 0) { + if (initData.length > 32 && getSignerCount(msg.sender) == 0) { // More than just delay parameter - (bytes[] memory signers_, uint256 threshold_) = abi.decode(initData, (bytes[], uint256)); + (bytes[] memory signers_, uint64 threshold_) = abi.decode(initData, (bytes[], uint64)); _addSigners(msg.sender, signers_); _setThreshold(msg.sender, threshold_); } @@ -86,30 +89,33 @@ abstract contract ERC7579Multisig is ERC7579Validator { } /** - * @dev Returns the set of authorized signers for the specified account. + * @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: This operation copies the entire signers set to memory, which - * can be expensive or may result in unbounded computation. + * 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 signers(address account) public view virtual returns (bytes[] memory) { - return _signers(account).values(); + function getSigners(address account, uint64 start, uint64 end) public view virtual returns (bytes[] memory) { + return _signersSetByAccount[account].values(start, end); } - /// @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 _signers(account).contains(signer); + /// @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 the set of authorized signers for the specified account. - function _signers(address account) internal view virtual returns (EnumerableSet.BytesSet storage) { - return _signersSetByAccount[account]; + /// @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 (uint256) { + function threshold(address account) public view virtual returns (uint64) { return _thresholdByAccount[account]; } @@ -147,7 +153,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { * * * The threshold must be reachable with the current number of signers. */ - function setThreshold(uint256 newThreshold) public virtual { + function setThreshold(uint64 newThreshold) public virtual { _setThreshold(msg.sender, newThreshold); } @@ -181,11 +187,10 @@ abstract contract ERC7579Multisig is ERC7579Validator { * * Each of `newSigners` must not be authorized. Reverts with {ERC7579MultisigAlreadyExists} if it already exists. */ function _addSigners(address account, bytes[] memory newSigners) internal virtual { - uint256 newSignersLength = newSigners.length; - for (uint256 i = 0; i < newSignersLength; i++) { + for (uint256 i = 0; i < newSigners.length; ++i) { bytes memory signer = newSigners[i]; require(signer.length >= 20, ERC7579MultisigInvalidSigner(signer)); - require(_signers(account).add(signer), ERC7579MultisigAlreadyExists(signer)); + require(_signersSetByAccount[account].add(signer), ERC7579MultisigAlreadyExists(signer)); emit ERC7913SignerAdded(account, signer); } } @@ -199,10 +204,9 @@ abstract contract ERC7579Multisig is ERC7579Validator { * * The threshold must remain reachable after removal. See {_validateReachableThreshold} for details. */ function _removeSigners(address account, bytes[] memory oldSigners) internal virtual { - uint256 oldSignersLength = oldSigners.length; - for (uint256 i = 0; i < oldSignersLength; i++) { + for (uint256 i = 0; i < oldSigners.length; ++i) { bytes memory signer = oldSigners[i]; - require(_signers(account).remove(signer), ERC7579MultisigNonexistentSigner(signer)); + require(_signersSetByAccount[account].remove(signer), ERC7579MultisigNonexistentSigner(signer)); emit ERC7913SignerRemoved(account, signer); } _validateReachableThreshold(account); @@ -213,9 +217,11 @@ abstract contract ERC7579Multisig is ERC7579Validator { * * 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, uint256 newThreshold) internal virtual { + function _setThreshold(address account, uint64 newThreshold) internal virtual { + require(newThreshold > 0, ERC7579MultisigZeroThreshold()); _thresholdByAccount[account] = newThreshold; _validateReachableThreshold(account); emit ERC7913ThresholdSet(account, newThreshold); @@ -229,9 +235,15 @@ abstract contract ERC7579Multisig is ERC7579Validator { * * The number of signers must be >= the threshold. Reverts with {ERC7579MultisigUnreachableThreshold} if not. */ function _validateReachableThreshold(address account) internal view virtual { - uint256 totalSigners = _signers(account).length(); - uint256 currentThreshold = threshold(account); - require(totalSigners >= currentThreshold, ERC7579MultisigUnreachableThreshold(totalSigners, currentThreshold)); + uint256 totalSigners = getSignerCount(account); + uint64 currentThreshold = threshold(account); + require( + totalSigners >= currentThreshold, + ERC7579MultisigUnreachableThreshold( + uint64(totalSigners), // Safe cast. Economically impossible to overflow. + currentThreshold + ) + ); } /** @@ -252,8 +264,7 @@ abstract contract ERC7579Multisig is ERC7579Validator { bytes[] memory signingSigners, bytes[] memory signatures ) internal view virtual returns (bool valid) { - uint256 signersLength = signingSigners.length; - for (uint256 i = 0; i < signersLength; i++) { + for (uint256 i = 0; i < signingSigners.length; ++i) { if (!isSigner(account, signingSigners[i])) { return false; } diff --git a/contracts/account/modules/ERC7579MultisigWeighted.sol b/contracts/account/modules/ERC7579MultisigWeighted.sol index 596df9f0..679b3ab7 100644 --- a/contracts/account/modules/ERC7579MultisigWeighted.sol +++ b/contracts/account/modules/ERC7579MultisigWeighted.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.27; -import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; +import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {ERC7579Multisig} from "./ERC7579Multisig.sol"; @@ -25,18 +25,24 @@ import {ERC7579Multisig} from "./ERC7579Multisig.sol"; */ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { using EnumerableSet for EnumerableSet.BytesSet; + using SafeCast for *; - // Mapping from account => signer => weight - mapping(address account => mapping(bytes signer => uint256)) private _weights; + // Sum of all the extra weights of all signers. Each signer has a base weight of 1. + mapping(address account => uint64 totalExtraWeight) private _totalExtraWeight; - // Invariant: sum(weights(account)) >= threshold(account) - mapping(address account => uint256 totalWeight) private _totalWeight; + // 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. - event ERC7579MultisigWeightChanged(address indexed account, bytes indexed signer, uint256 weight); + /** + * @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, uint256 weight); + error ERC7579MultisigInvalidWeight(bytes signer, uint64 weight); /// @dev Thrown when the arrays lengths don't match. error ERC7579MultisigMismatchedLength(); @@ -47,7 +53,7 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { * signer weights. * * The initData should be encoded as: - * `abi.encode(bytes[] signers, uint256 threshold, uint256[] weights)` + * `abi.encode(bytes[] signers, uint64 threshold, uint64[] weights)` * * If weights are not provided but signers are, all signers default to weight 1. * @@ -55,10 +61,10 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { * 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 = _signers(msg.sender).length() > 0; + bool installed = getSignerCount(msg.sender) > 0; super.onInstall(initData); if (initData.length > 96 && !installed) { - (bytes[] memory signers, , uint256[] memory weights) = abi.decode(initData, (bytes[], uint256, uint256[])); + (bytes[] memory signers, , uint64[] memory weights) = abi.decode(initData, (bytes[], uint64, uint64[])); _setSignerWeights(msg.sender, signers, weights); } } @@ -72,43 +78,38 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { function onUninstall(bytes calldata data) public virtual override { address account = msg.sender; - bytes[] memory allSigners = signers(account); + bytes[] memory allSigners = getSigners(account, 0, type(uint64).max); uint256 allSignersLength = allSigners.length; - for (uint256 i = 0; i < allSignersLength; i++) { - delete _weights[account][allSigners[i]]; + for (uint256 i = 0; i < allSignersLength; ++i) { + delete _extraWeights[account][allSigners[i]]; } - delete _totalWeight[account]; + 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 (uint256) { - return isSigner(account, signer) ? _signerWeight(account, signer) : 0; + 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 (uint256) { - return _totalWeight[account]; // Doesn't need Math.max because it's incremented by the default 1 in `_addSigners` + 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, uint256[] memory weights) public virtual { + function setSignerWeights(bytes[] memory signers, uint64[] memory weights) public virtual { _setSignerWeights(msg.sender, signers, weights); } - /** - * @dev Gets the weight of the current signer. Returns 1 if not explicitly set. - * This internal function doesn't check if the signer is authorized. - */ - function _signerWeight(address account, bytes memory signer) internal view virtual returns (uint256) { - return Math.max(_weights[account][signer], 1); - } - /** * @dev Sets weights for multiple signers at once. Internal version without access control. * @@ -121,40 +122,76 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { * * Emits {ERC7579MultisigWeightChanged} for each signer. */ - function _setSignerWeights(address account, bytes[] memory signers, uint256[] memory newWeights) internal virtual { - uint256 signersLength = signers.length; - require(signersLength == newWeights.length, ERC7579MultisigMismatchedLength()); - uint256 oldWeight = _weightSigners(account, signers); + function _setSignerWeights(address account, bytes[] memory signers, uint64[] memory weights) internal virtual { + require(signers.length == weights.length, ERC7579MultisigMismatchedLength()); - for (uint256 i = 0; i < signersLength; i++) { + uint256 extraWeightAdded = 0; + uint256 extraWeightRemoved = 0; + for (uint256 i = 0; i < signers.length; ++i) { bytes memory signer = signers[i]; - uint256 newWeight = newWeights[i]; require(isSigner(account, signer), ERC7579MultisigNonexistentSigner(signer)); - require(newWeight > 0, ERC7579MultisigInvalidWeight(signer, newWeight)); - } - _unsafeSetSignerWeights(account, signers, newWeights); - _totalWeight[account] = totalWeight(account) - oldWeight + _weightSigners(account, signers); + 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); - _totalWeight[account] += newSigners.length; // Default weight of 1 per signer. + + // This will revert if the new signers cause an overflow + _validateReachableThreshold(account); } - /// @dev Override to handle weight tracking during removal. See {ERC7579Multisig-_removeSigners}. + /** + * @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 { - uint256 removedWeight = _weightSigners(account, oldSigners); + // 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 { - // Can't overflow. Invariant: sum(weights) >= threshold - _totalWeight[account] -= removedWeight; + 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; } - _unsafeSetSignerWeights(account, oldSigners, new uint256[](oldSigners.length)); super._removeSigners(account, oldSigners); } @@ -167,8 +204,8 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { * depending on the linearization order. */ function _validateReachableThreshold(address account) internal view virtual override { - uint256 weight = totalWeight(account); - uint256 currentThreshold = threshold(account); + uint64 weight = totalWeight(account); + uint64 currentThreshold = threshold(account); require(weight >= currentThreshold, ERC7579MultisigUnreachableThreshold(weight, currentThreshold)); } @@ -183,35 +220,13 @@ abstract contract ERC7579MultisigWeighted is ERC7579Multisig { address account, bytes[] memory validatingSigners ) internal view virtual override returns (bool) { - uint256 totalSigningWeight = _weightSigners(account, validatingSigners); - return totalSigningWeight >= threshold(account); - } - - /// @dev Calculates the total weight of a set of signers. - function _weightSigners(address account, bytes[] memory signers) internal view virtual returns (uint256) { - uint256 weight = 0; - uint256 signersLength = signers.length; - for (uint256 i = 0; i < signersLength; i++) { - weight += signerWeight(account, signers[i]); - } - return weight; - } - - /** - * @dev Sets the `newWeights` for multiple `signers` without updating the {totalWeight} or - * validating the threshold of `account`. - * - * Requirements: - * - * * The `newWeights` array must be at least as large as the `signers` array. Panics otherwise. - * - * Emits {ERC7579MultisigWeightChanged} for each signer. - */ - function _unsafeSetSignerWeights(address account, bytes[] memory signers, uint256[] memory newWeights) private { - uint256 signersLength = signers.length; - for (uint256 i = 0; i < signersLength; i++) { - _weights[account][signers[i]] = newWeights[i]; - emit ERC7579MultisigWeightChanged(account, signers[i], newWeights[i]); + 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/test/account/modules/ERC7579Multisig.test.js b/test/account/modules/ERC7579Multisig.test.js index 01921154..75640bac 100644 --- a/test/account/modules/ERC7579Multisig.test.js +++ b/test/account/modules/ERC7579Multisig.test.js @@ -12,6 +12,7 @@ const { encodeSingle, } = require('@openzeppelin/contracts/test/helpers/erc7579'); const { NonNativeSigner, MultiERC7913SigningKey } = require('@openzeppelin/contracts/test/helpers/signers'); +const { MAX_UINT64 } = require('@openzeppelin/contracts/test/helpers/constants'); const { shouldBehaveLikeERC7579Module } = require('./ERC7579Module.behavior'); @@ -91,7 +92,7 @@ describe('ERC7579Multisig', function () { await expect(tx).to.emit(this.mock, 'ERC7913ThresholdSet').withArgs(this.mockAccount.address, this.threshold); // Verify signers and threshold - await expect(this.mock.signers(this.mockAccount.address)).to.eventually.deep.equal(this.signers); + 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 @@ -100,7 +101,7 @@ describe('ERC7579Multisig', function () { ); // Should still have the original signers and threshold - await expect(this.mock.signers(this.mockAccount.address)).to.eventually.deep.equal(this.signers); + 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); }); @@ -109,7 +110,7 @@ describe('ERC7579Multisig', function () { await this.mockAccountFromEntrypoint.uninstallModule(this.moduleType, this.mock.target, '0x'); // Verify signers and threshold are cleared - await expect(this.mock.signers(this.mockAccount.address)).to.eventually.deep.equal([]); + 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); }); @@ -128,7 +129,7 @@ describe('ERC7579Multisig', function () { const newSigners = [signerECDSA3.address]; // Get signers before adding - const signersBefore = await this.mock.signers(this.mockAccount.address); + const signersBefore = await this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64); // Add new signers const tx = await this.mockFromAccount.addSigners(newSigners); @@ -139,7 +140,7 @@ describe('ERC7579Multisig', function () { } // Get signers after adding - const signersAfter = await this.mock.signers(this.mockAccount.address); + 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); @@ -158,7 +159,7 @@ describe('ERC7579Multisig', function () { const removedSigners = [signerECDSA1.address].map(address => address.toLowerCase()); // Get signers before removing - const signersBefore = await this.mock.signers(this.mockAccount.address); + const signersBefore = await this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64); // Remove signers const tx = await this.mockFromAccount.removeSigners(removedSigners); @@ -169,7 +170,7 @@ describe('ERC7579Multisig', function () { } // Get signers after removing - const signersAfter = await this.mock.signers(this.mockAccount.address); + 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); diff --git a/test/account/modules/ERC7579MultisigConfirmation.test.js b/test/account/modules/ERC7579MultisigConfirmation.test.js index 1696b935..493bf072 100644 --- a/test/account/modules/ERC7579MultisigConfirmation.test.js +++ b/test/account/modules/ERC7579MultisigConfirmation.test.js @@ -11,6 +11,7 @@ const { MultisigConfirmation } = require('../../helpers/eip712-types'); const { shouldBehaveLikeERC7579Module } = require('./ERC7579Module.behavior'); // Prepare signers in advance +const initialSigner = ethers.Wallet.createRandom(); const signerToConfirm = ethers.Wallet.createRandom(); async function fixture() { @@ -24,9 +25,6 @@ async function fixture() { const helper = new ERC4337Helper(); await helper.wait(); - // Prepare module installation data - const installData = ethers.AbiCoder.defaultAbiCoder().encode(['bytes[]', 'uint256'], [[], 0]); // Empty - // ERC-7579 account const mockAccount = await helper.newAccount('$AccountERC7579'); const mockFromAccount = await impersonate(mockAccount.address).then(asAccount => mock.connect(asAccount)); @@ -34,14 +32,34 @@ async function fixture() { mockAccount.connect(asEntrypoint), ); + // Get the EIP-712 domain for the mock module + const domain = await getDomain(mock); + + // Prepare module installation data + const abiCoder = ethers.AbiCoder.defaultAbiCoder(); + const signers = abiCoder.encode( + ['uint256', 'bytes', 'bytes'], + [ + (await time.latest()) + time.duration.days(1), + initialSigner.address, + await initialSigner.signTypedData( + domain, + { MultisigConfirmation }, + { + account: mockAccount.address, + module: mock.target, + deadline: (await time.latest()) + time.duration.days(1), + }, + ), + ], + ); + const installData = abiCoder.encode(['bytes[]', 'uint64'], [[signers], 1]); + const moduleType = MODULE_TYPE_EXECUTOR; await mockAccount.deploy(); await mockAccountFromEntrypoint.installModule(moduleType, mock.target, installData); - // Get the EIP-712 domain for the mock module - const domain = await getDomain(mock); - return { moduleType, mock, diff --git a/test/account/modules/ERC7579MultisigWeighted.test.js b/test/account/modules/ERC7579MultisigWeighted.test.js index f3a927e9..d226c4ab 100644 --- a/test/account/modules/ERC7579MultisigWeighted.test.js +++ b/test/account/modules/ERC7579MultisigWeighted.test.js @@ -12,6 +12,7 @@ const { encodeSingle, } = require('@openzeppelin/contracts/test/helpers/erc7579'); const { NonNativeSigner, MultiERC7913SigningKey } = require('@openzeppelin/contracts/test/helpers/signers'); +const { MAX_UINT64 } = require('@openzeppelin/contracts/test/helpers/constants'); const { shouldBehaveLikeERC7579Module } = require('./ERC7579Module.behavior'); @@ -121,7 +122,7 @@ describe('ERC7579MultisigWeighted', function () { ); // Should still have the original signers, weights, and threshold - await expect(this.mock.signers(this.mockAccount.address)).to.eventually.deep.equal(this.signers); + 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); }); @@ -131,7 +132,7 @@ describe('ERC7579MultisigWeighted', function () { await this.mockAccountFromEntrypoint.uninstallModule(this.moduleType, this.mock.target, '0x'); // Verify signers and threshold are cleared - await expect(this.mock.signers(this.mockAccount.address)).to.eventually.deep.equal([]); + 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) @@ -147,7 +148,7 @@ describe('ERC7579MultisigWeighted', function () { const newSigners = [signerECDSA4.address]; // Get signers before adding - const signersBefore = await this.mock.signers(this.mockAccount.address); + const signersBefore = await this.mock.getSigners(this.mockAccount.address, 0, MAX_UINT64); // Add new signer const tx = await this.mockFromAccount.addSigners(newSigners); @@ -158,7 +159,7 @@ describe('ERC7579MultisigWeighted', function () { } // Get signers after adding - const signersAfter = await this.mock.signers(this.mockAccount.address); + 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);