Add MultiSignerERC7913Weighted (#5718)

.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.

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

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}}

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);

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();
+        }
+    }
+}

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');
+    });
+  });
+});