diff --git a/.changeset/public-crabs-heal.md b/.changeset/public-crabs-heal.md new file mode 100644 index 000000000..463dc9887 --- /dev/null +++ b/.changeset/public-crabs-heal.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`MultiSignerERC7913Weighted`: Extension of `MultiSignerERC7913` that supports assigning different weights to each signer, enabling more flexible governance schemes. diff --git a/contracts/mocks/account/AccountMock.sol b/contracts/mocks/account/AccountMock.sol index b1fa4d2e8..0ba0d2920 100644 --- a/contracts/mocks/account/AccountMock.sol +++ b/contracts/mocks/account/AccountMock.sol @@ -19,6 +19,7 @@ import {SignerRSA} from "../../utils/cryptography/signers/SignerRSA.sol"; import {SignerERC7702} from "../../utils/cryptography/signers/SignerERC7702.sol"; import {SignerERC7913} from "../../utils/cryptography/signers/SignerERC7913.sol"; import {MultiSignerERC7913} from "../../utils/cryptography/signers/MultiSignerERC7913.sol"; +import {MultiSignerERC7913Weighted} from "../../utils/cryptography/signers/MultiSignerERC7913Weighted.sol"; abstract contract AccountMock is Account, ERC7739, ERC7821, ERC721Holder, ERC1155Holder { /// Validates a user operation with a boolean signature. @@ -139,6 +140,21 @@ abstract contract AccountERC7579HookedMock is AccountERC7579Hooked { } } +abstract contract AccountERC7913Mock is Account, SignerERC7913, ERC7739, ERC7821, ERC721Holder, ERC1155Holder { + constructor(bytes memory _signer) { + _setSigner(_signer); + } + + /// @inheritdoc ERC7821 + function _erc7821AuthorizedExecutor( + address caller, + bytes32 mode, + bytes calldata executionData + ) internal view virtual override returns (bool) { + return caller == address(entryPoint()) || super._erc7821AuthorizedExecutor(caller, mode, executionData); + } +} + abstract contract AccountMultiSignerMock is Account, MultiSignerERC7913, ERC7739, ERC7821, ERC721Holder, ERC1155Holder { constructor(bytes[] memory signers, uint64 threshold) { _addSigners(signers); @@ -155,9 +171,18 @@ abstract contract AccountMultiSignerMock is Account, MultiSignerERC7913, ERC7739 } } -abstract contract AccountERC7913Mock is Account, SignerERC7913, ERC7739, ERC7821, ERC721Holder, ERC1155Holder { - constructor(bytes memory _signer) { - _setSigner(_signer); +abstract contract AccountMultiSignerWeightedMock is + Account, + MultiSignerERC7913Weighted, + ERC7739, + ERC7821, + ERC721Holder, + ERC1155Holder +{ + constructor(bytes[] memory signers, uint64[] memory weights, uint64 threshold) { + _addSigners(signers); + _setSignerWeights(signers, weights); + _setThreshold(threshold); } /// @inheritdoc ERC7821 diff --git a/contracts/utils/cryptography/README.adoc b/contracts/utils/cryptography/README.adoc index 79b104373..37a982448 100644 --- a/contracts/utils/cryptography/README.adoc +++ b/contracts/utils/cryptography/README.adoc @@ -17,7 +17,7 @@ A collection of contracts and libraries that implement various signature validat * {ERC7739}: An abstract contract to validate signatures following the rehashing scheme from {ERC7739Utils}. * {SignerECDSA}, {SignerP256}, {SignerRSA}: Implementations of an {AbstractSigner} with specific signature validation algorithms. * {SignerERC7702}: Implementation of {AbstractSigner} that validates signatures using the contract's own address as the signer, useful for delegated accounts following EIP-7702. - * {SignerERC7913}, {MultiSignerERC7913}: Implementations of {AbstractSigner} that validate signatures based on ERC-7913. Including a simple multisignature scheme. + * {SignerERC7913}, {MultiSignerERC7913}, {MultiSignerERC7913Weighted}: Implementations of {AbstractSigner} that validate signatures based on ERC-7913. Including a simple and weighted multisignature scheme. * {ERC7913P256Verifier}, {ERC7913RSAVerifier}: Ready to use ERC-7913 signature verifiers for P256 and RSA keys. == Utils @@ -58,6 +58,8 @@ A collection of contracts and libraries that implement various signature validat {{MultiSignerERC7913}} +{{MultiSignerERC7913Weighted}} + == Verifiers {{ERC7913P256Verifier}} diff --git a/contracts/utils/cryptography/signers/MultiSignerERC7913.sol b/contracts/utils/cryptography/signers/MultiSignerERC7913.sol index 8ee983d95..e8473fca9 100644 --- a/contracts/utils/cryptography/signers/MultiSignerERC7913.sol +++ b/contracts/utils/cryptography/signers/MultiSignerERC7913.sol @@ -18,8 +18,6 @@ import {EnumerableSet} from "../../structs/EnumerableSet.sol"; * * ```solidity * contract MyMultiSignerAccount is Account, MultiSignerERC7913, Initializable { - * constructor() EIP712("MyMultiSignerAccount", "1") {} - * * function initialize(bytes[] memory signers, uint64 threshold) public initializer { * _addSigners(signers); * _setThreshold(threshold); @@ -84,6 +82,11 @@ abstract contract MultiSignerERC7913 is AbstractSigner { return _signers.values(start, end); } + /// @dev Returns the number of authorized signers + function getSignerCount() public view virtual returns (uint256) { + return _signers.length(); + } + /// @dev Returns whether the `signer` is an authorized signer. function isSigner(bytes memory signer) public view virtual returns (bool) { return _signers.contains(signer); diff --git a/contracts/utils/cryptography/signers/MultiSignerERC7913Weighted.sol b/contracts/utils/cryptography/signers/MultiSignerERC7913Weighted.sol new file mode 100644 index 000000000..4fa3fa683 --- /dev/null +++ b/contracts/utils/cryptography/signers/MultiSignerERC7913Weighted.sol @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.27; + +import {SafeCast} from "../../math/SafeCast.sol"; +import {MultiSignerERC7913} from "./MultiSignerERC7913.sol"; + +/** + * @dev Extension of {MultiSignerERC7913} that supports weighted signatures. + * + * This contract allows assigning different weights to each signer, enabling more + * flexible governance schemes. For example, some signers could have higher weight + * than others, allowing for weighted voting or prioritized authorization. + * + * Example of usage: + * + * ```solidity + * contract MyWeightedMultiSignerAccount is Account, MultiSignerERC7913Weighted, Initializable { + * function initialize(bytes[] memory signers, uint64[] memory weights, uint64 threshold) public initializer { + * _addSigners(signers); + * _setSignerWeights(signers, weights); + * _setThreshold(threshold); + * } + * + * function addSigners(bytes[] memory signers) public onlyEntryPointOrSelf { + * _addSigners(signers); + * } + * + * function removeSigners(bytes[] memory signers) public onlyEntryPointOrSelf { + * _removeSigners(signers); + * } + * + * function setThreshold(uint64 threshold) public onlyEntryPointOrSelf { + * _setThreshold(threshold); + * } + * + * function setSignerWeights(bytes[] memory signers, uint64[] memory weights) public onlyEntryPointOrSelf { + * _setSignerWeights(signers, weights); + * } + * } + * ``` + * + * 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 at + * least two signers (e.g., one with weight 1 and one with weight 3). See {signerWeight}. + */ +abstract contract MultiSignerERC7913Weighted is MultiSignerERC7913 { + using SafeCast for *; + + // Sum of all the extra weights of all signers. Storage packed with `MultiSignerERC7913._threshold` + uint64 private _totalExtraWeight; + + // Mapping from signer to extraWeight (in addition to all authorized signers having weight 1) + 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 ERC7913SignerWeightChanged(bytes indexed signer, uint64 weight); + + /// @dev Thrown when a signer's weight is invalid. + error MultiSignerERC7913WeightedInvalidWeight(bytes signer, uint64 weight); + + /// @dev Thrown when the arrays lengths don't match. See {_setSignerWeights}. + error MultiSignerERC7913WeightedMismatchedLength(); + + /// @dev Gets the weight of a signer. Returns 0 if the signer is not authorized. + function signerWeight(bytes memory signer) public view virtual returns (uint64) { + unchecked { + // Safe cast, _setSignerWeights guarantees 1+_extraWeights is a uint64 + return uint64(isSigner(signer).toUint() * (1 + _extraWeights[signer])); + } + } + + /// @dev Gets the total weight of all signers. + function totalWeight() public view virtual returns (uint64) { + return (getSignerCount() + _totalExtraWeight).toUint64(); + } + + /** + * @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 {MultiSignerERC7913WeightedMismatchedLength} on mismatch. + * * Each signer must exist in the set of authorized signers. Otherwise reverts with {MultiSignerERC7913NonexistentSigner} + * * Each weight must be greater than 0. Otherwise reverts with {MultiSignerERC7913WeightedInvalidWeight} + * * See {_validateReachableThreshold} for the threshold validation. + * + * Emits {ERC7913SignerWeightChanged} for each signer. + */ + function _setSignerWeights(bytes[] memory signers, uint64[] memory weights) internal virtual { + require(signers.length == weights.length, MultiSignerERC7913WeightedMismatchedLength()); + + uint256 extraWeightAdded = 0; + uint256 extraWeightRemoved = 0; + for (uint256 i = 0; i < signers.length; ++i) { + bytes memory signer = signers[i]; + uint64 weight = weights[i]; + + require(isSigner(signer), MultiSignerERC7913NonexistentSigner(signer)); + require(weight > 0, MultiSignerERC7913WeightedInvalidWeight(signer, weight)); + + unchecked { + // Overflow impossible: weight values are bounded by uint64 and economic constraints + extraWeightRemoved += _extraWeights[signer]; + extraWeightAdded += _extraWeights[signer] = weight - 1; + } + + emit ERC7913SignerWeightChanged(signer, weight); + } + unchecked { + // Safe from underflow: `extraWeightRemoved` is bounded by `_totalExtraWeight` by construction + // and weight values are bounded by uint64 and economic constraints + _totalExtraWeight = (uint256(_totalExtraWeight) + extraWeightAdded - extraWeightRemoved).toUint64(); + } + _validateReachableThreshold(); + } + + /** + * @dev See {MultiSignerERC7913-_removeSigners}. + * + * Just like {_addSigners}, this function does not emit {ERC7913SignerWeightChanged} events. The + * {ERC7913SignerRemoved} event emitted by {MultiSignerERC7913-_removeSigners} is enough to track weights here. + */ + function _removeSigners(bytes[] memory signers) 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 < signers.length; ++i) { + bytes memory signer = signers[i]; + + extraWeightRemoved += _extraWeights[signer]; + delete _extraWeights[signer]; + } + _totalExtraWeight -= extraWeightRemoved; + } + super._removeSigners(signers); + } + + /** + * @dev Sets the threshold for the multisignature operation. Internal version without access control. + * + * Requirements: + * + * * The {totalWeight} must be `>=` the {threshold}. Otherwise reverts with {MultiSignerERC7913UnreachableThreshold} + * + * 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() internal view virtual override { + uint64 weight = totalWeight(); + uint64 currentThreshold = threshold(); + require(weight >= currentThreshold, MultiSignerERC7913UnreachableThreshold(weight, currentThreshold)); + } + + /** + * @dev Validates that the total weight of signers meets the threshold requirement. + * + * NOTE: This function intentionally does not call `super._validateThreshold` 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 _validateThreshold(bytes[] memory signers) internal view virtual override returns (bool) { + unchecked { + uint64 weight = 0; + for (uint256 i = 0; i < signers.length; ++i) { + // Overflow impossible: weight values are bounded by uint64 and economic constraints + weight += signerWeight(signers[i]); + } + return weight >= threshold(); + } + } +} diff --git a/test/account/AccountMultiSignerWeighted.test.js b/test/account/AccountMultiSignerWeighted.test.js new file mode 100644 index 000000000..15f24c2d4 --- /dev/null +++ b/test/account/AccountMultiSignerWeighted.test.js @@ -0,0 +1,292 @@ +const { ethers, entrypoint } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +const { getDomain } = require('../helpers/eip712'); +const { ERC4337Helper } = require('../helpers/erc4337'); +const { NonNativeSigner, P256SigningKey, RSASHA256SigningKey, MultiERC7913SigningKey } = require('../helpers/signers'); +const { PackedUserOperation } = require('../helpers/eip712-types'); + +const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior'); +const { shouldBehaveLikeERC1271 } = require('../utils/cryptography/ERC1271.behavior'); +const { shouldBehaveLikeERC7821 } = require('./extensions/ERC7821.behavior'); + +// Prepare signers in advance (RSA are long to initialize) +const signerECDSA1 = ethers.Wallet.createRandom(); +const signerECDSA2 = ethers.Wallet.createRandom(); +const signerECDSA3 = ethers.Wallet.createRandom(); +const signerECDSA4 = ethers.Wallet.createRandom(); +const signerP256 = new NonNativeSigner(P256SigningKey.random()); +const signerRSA = new NonNativeSigner(RSASHA256SigningKey.random()); + +// Minimal fixture common to the different signer verifiers +async function fixture() { + // EOAs and environment + const [beneficiary, other] = await ethers.getSigners(); + const target = await ethers.deployContract('CallReceiverMock'); + + // 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(entrypoint.v08); + const domain = { name: 'AccountMultiSignerWeighted', version: '1', chainId: entrypointDomain.chainId }; // Missing verifyingContract + + const makeMock = (signers, weights, threshold) => + helper + .newAccount('$AccountMultiSignerWeightedMock', ['AccountMultiSignerWeighted', '1', signers, weights, threshold]) + .then(mock => { + domain.verifyingContract = mock.address; + return mock; + }); + + // Sign user operations using NonNativeSigner with MultiERC7913SigningKey + const signUserOp = function (userOp) { + return this.signer + .signTypedData(entrypointDomain, { PackedUserOperation }, userOp.packed) + .then(signature => Object.assign(userOp, { signature })); + }; + + const invalidSig = function () { + return this.signer.signMessage('invalid'); + }; + + return { + helper, + verifierP256, + verifierRSA, + domain, + target, + beneficiary, + other, + makeMock, + signUserOp, + invalidSig, + }; +} + +describe('AccountMultiSignerWeighted', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('Weighted signers with equal weights (1, 1, 1) and threshold=2', function () { + beforeEach(async function () { + this.signer = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA1, signerECDSA3])); // 2 accounts, weight 1+1=2 + this.mock = await this.makeMock([signerECDSA1.address, signerECDSA2.address, signerECDSA3.address], [1, 1, 1], 2); + }); + + shouldBehaveLikeAccountCore(); + shouldBehaveLikeAccountHolder(); + shouldBehaveLikeERC1271({ erc7739: true }); + shouldBehaveLikeERC7821(); + }); + + describe('Weighted signers with varying weights (1, 2, 3) and threshold=3', function () { + beforeEach(async function () { + this.signer = new NonNativeSigner(new MultiERC7913SigningKey([signerECDSA1, signerECDSA2])); // 2 accounts, weight 1+2=3 + this.mock = await this.makeMock([signerECDSA1.address, signerECDSA2.address, signerECDSA3.address], [1, 2, 3], 3); + }); + + shouldBehaveLikeAccountCore(); + shouldBehaveLikeAccountHolder(); + shouldBehaveLikeERC1271({ erc7739: true }); + shouldBehaveLikeERC7821(); + }); + + describe('Mixed weighted signers with threshold=4', function () { + beforeEach(async function () { + // Create signers array with all three types + signerP256.bytes = ethers.concat([ + this.verifierP256.target, + signerP256.signingKey.publicKey.qx, + signerP256.signingKey.publicKey.qy, + ]); + + signerRSA.bytes = ethers.concat([ + this.verifierRSA.target, + ethers.AbiCoder.defaultAbiCoder().encode( + ['bytes', 'bytes'], + [signerRSA.signingKey.publicKey.e, signerRSA.signingKey.publicKey.n], + ), + ]); + + this.signer = new NonNativeSigner(new MultiERC7913SigningKey([signerP256, signerRSA])); // 2 accounts, weight 2+3=5 + this.mock = await this.makeMock( + [signerECDSA1.address, signerP256.bytes, signerRSA.bytes], + [1, 2, 3], + 4, // Requires at least signer2 + signer3, or all three signers + ); + }); + + shouldBehaveLikeAccountCore(); + shouldBehaveLikeAccountHolder(); + shouldBehaveLikeERC1271({ erc7739: true }); + shouldBehaveLikeERC7821(); + }); + + describe('Weight management', function () { + const signer1 = signerECDSA1.address; + const signer2 = signerECDSA2.address; + const signer3 = signerECDSA3.address; + const signer4 = signerECDSA4.address; + + beforeEach(async function () { + this.mock = await this.makeMock([signer1, signer2, signer3], [1, 2, 3], 4); + await this.mock.deploy(); + }); + + it('can get signer weights', async function () { + await expect(this.mock.signerWeight(signer1)).to.eventually.equal(1); + await expect(this.mock.signerWeight(signer2)).to.eventually.equal(2); + await expect(this.mock.signerWeight(signer3)).to.eventually.equal(3); + }); + + it('can update signer weights', async function () { + // Successfully updates weights and emits event + await expect(this.mock.$_setSignerWeights([signer1, signer2], [5, 6])) + .to.emit(this.mock, 'ERC7913SignerWeightChanged') + .withArgs(signer1, 5) + .to.emit(this.mock, 'ERC7913SignerWeightChanged') + .withArgs(signer2, 6); + + await expect(this.mock.signerWeight(signer1)).to.eventually.equal(5); + await expect(this.mock.signerWeight(signer2)).to.eventually.equal(6); + await expect(this.mock.signerWeight(signer3)).to.eventually.equal(3); // unchanged + }); + + it('cannot set weight to non-existent signer', async function () { + // Reverts when setting weight for non-existent signer + await expect(this.mock.$_setSignerWeights([signer4], [1])) + .to.be.revertedWithCustomError(this.mock, 'MultiSignerERC7913NonexistentSigner') + .withArgs(signer4.toLowerCase()); + }); + + it('cannot set weight to 0', async function () { + // Reverts when setting weight to 0 + await expect(this.mock.$_setSignerWeights([signer1], [0])) + .to.be.revertedWithCustomError(this.mock, 'MultiSignerERC7913WeightedInvalidWeight') + .withArgs(signer1.toLowerCase(), 0); + }); + + it('requires signers and weights arrays to have same length', async function () { + // Reverts when arrays have different lengths + await expect(this.mock.$_setSignerWeights([signer1, signer2], [1])).to.be.revertedWithCustomError( + this.mock, + 'MultiSignerERC7913WeightedMismatchedLength', + ); + + await expect(this.mock.$_setSignerWeights([signer1], [1, 2])).to.be.revertedWithCustomError( + this.mock, + 'MultiSignerERC7913WeightedMismatchedLength', + ); + }); + + it('validates threshold is reachable when updating weights', async function () { + // First, lower the weights so the sum is exactly 6 (just enough for threshold=6) + await expect(this.mock.$_setSignerWeights([signer1, signer2, signer3], [1, 2, 3])) + .to.emit(this.mock, 'ERC7913SignerWeightChanged') + .withArgs(signer1, 1) + .to.emit(this.mock, 'ERC7913SignerWeightChanged') + .withArgs(signer2, 2) + .to.emit(this.mock, 'ERC7913SignerWeightChanged') + .withArgs(signer3, 3); + + // Increase threshold to 6 + await expect(this.mock.$_setThreshold(6)).to.emit(this.mock, 'ERC7913ThresholdSet').withArgs(6); + + // Now try to lower weights so their sum is less than the threshold + await expect(this.mock.$_setSignerWeights([signer1, signer2, signer3], [1, 1, 1])).to.be.revertedWithCustomError( + this.mock, + 'MultiSignerERC7913UnreachableThreshold', + ); + + // Try to increase threshold to be larger than the total weight + await expect(this.mock.$_setThreshold(7)) + .to.be.revertedWithCustomError(this.mock, 'MultiSignerERC7913UnreachableThreshold') + .withArgs(6, 7); + }); + + it('reports default weight of 1 for signers without explicit weight', async function () { + // Add a new signer without setting weight + await this.mock.$_addSigners([signer4]); + + // Should have default weight of 1 + await expect(this.mock.signerWeight(signer4)).to.eventually.equal(1); + }); + + it('reports weight of 0 for invalid signers', async function () { + // not authorized + await expect(this.mock.signerWeight(signer4)).to.eventually.equal(0); + }); + + it('can get total weight of all signers', async function () { + await expect(this.mock.totalWeight()).to.eventually.equal(6); // 1+2+3=6 + }); + + it('totalWeight returns correct value when all signers have default weight of 1', async function () { + // Deploy a new mock with all signers having default weight (1) + const signers = [signerECDSA1.address, signerECDSA2.address, signerECDSA3.address]; + const defaultWeights = [1, 1, 1]; // All weights are 1 (default) + const newMock = await this.makeMock(signers, defaultWeights, 2); + await newMock.deploy(); + + // totalWeight should return max(3, 3) = 3 when all weights are default + await expect(newMock.totalWeight()).to.eventually.equal(3); + + // Clear custom weights to ensure we're using default weights + await newMock.$_setSignerWeights(signers, [1, 1, 1]); + + // totalWeight should still be max(3, 3) = 3 + await expect(newMock.totalWeight()).to.eventually.equal(3); + }); + + it('_setSignerWeights correctly handles default weights when updating', async function () { + await expect(this.mock.totalWeight()).to.eventually.equal(6); // 1+2+3=6 + + // Set weight for signer1 from 1 (default) to 5 + await this.mock.$_setSignerWeights([signer1], [5]); + await expect(this.mock.totalWeight()).to.eventually.equal(10); // 5+2+3=10 + + // Reset signer1 to default weight (1) + await this.mock.$_setSignerWeights([signer1], [1]); + await expect(this.mock.totalWeight()).to.eventually.equal(6); // 1+2+3=6 + }); + + it('updates total weight when adding and removing signers', async function () { + await expect(this.mock.totalWeight()).to.eventually.equal(6); // 1+2+3=6 + + // Add a new signer - should increase total weight by default weight (1) + await this.mock.$_addSigners([signer4]); + await expect(this.mock.totalWeight()).to.eventually.equal(7); // 1+2+3+1=7 + + // Set weight to 5 - should increase total weight by 4 + await this.mock.$_setSignerWeights([signer4], [5]); + await expect(this.mock.totalWeight()).to.eventually.equal(11); // 1+2+3+5=11 + + // Remove signer - should decrease total weight by current weight (5) + await this.mock.$_removeSigners([signer4]); + await expect(this.mock.totalWeight()).to.eventually.equal(6); // 1+2+3=6 + }); + + it('removing signers should not make threshold unreachable', async function () { + // current threshold = 4, totalWeight = 1+2+3 = 6 + + // After removing signer3, the threshold is unreachable because totalWeight = 1+2 = 3 but threshold = 4 + // [reverts] + await expect(this.mock.$_removeSigners([signer3])) + .to.be.revertedWithCustomError(this.mock, 'MultiSignerERC7913UnreachableThreshold') + .withArgs(3, 4); + + // After removing signer1, the threshold is still reachable because totalWeight = 2+3 = 5 and threshold = 4 + // [does not revert] + await expect(this.mock.$_removeSigners([signer1])) + .to.emit(this.mock, 'ERC7913SignerRemoved') + .withArgs(signer1) + .to.not.emit(this.mock, 'ERC7913SignerWeightChanged'); + }); + }); +});