diff --git a/contracts/utils/cryptography/signers/MultiSignerERC7913Weighted.sol b/contracts/utils/cryptography/signers/MultiSignerERC7913Weighted.sol index a73828a16..846364ee7 100644 --- a/contracts/utils/cryptography/signers/MultiSignerERC7913Weighted.sol +++ b/contracts/utils/cryptography/signers/MultiSignerERC7913Weighted.sol @@ -104,18 +104,22 @@ abstract contract MultiSignerERC7913Weighted is MultiSignerERC7913 { 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)); + + uint64 weight = weights[i]; 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; - } + uint64 oldExtraWeight = _extraWeights[signer]; + uint64 newExtraWeight = weight - 1; - emit ERC7913SignerWeightChanged(signer, weight); + if (oldExtraWeight != newExtraWeight) { + // Overflow impossible: weight values are bounded by uint64 and economic constraints + extraWeightRemoved += oldExtraWeight; + extraWeightAdded += _extraWeights[signer] = newExtraWeight; + emit ERC7913SignerWeightChanged(signer, weight); + } + } } unchecked { // Safe from underflow: `extraWeightRemoved` is bounded by `_totalExtraWeight` by construction diff --git a/test/account/AccountMultiSignerWeighted.test.js b/test/account/AccountMultiSignerWeighted.test.js index d3522360d..0d78f3eca 100644 --- a/test/account/AccountMultiSignerWeighted.test.js +++ b/test/account/AccountMultiSignerWeighted.test.js @@ -158,6 +158,10 @@ describe('AccountMultiSignerWeighted', function () { await expect(this.mock.signerWeight(signer3)).to.eventually.equal(3); // unchanged }); + it("no-op doesn't emit an event", async function () { + await expect(this.mock.$_setSignerWeights([signer1], [1])).to.not.emit(this.mock, 'ERC7913SignerWeightChanged'); + }); + 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])) @@ -186,28 +190,28 @@ describe('AccountMultiSignerWeighted', function () { }); 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])) + // First, lower the weights so the sum is exactly 9 (just enough for threshold=9) + await expect(this.mock.$_setSignerWeights([signer1, signer2, signer3], [2, 3, 4])) .to.emit(this.mock, 'ERC7913SignerWeightChanged') - .withArgs(signer1, 1) + .withArgs(signer1, 2) .to.emit(this.mock, 'ERC7913SignerWeightChanged') - .withArgs(signer2, 2) + .withArgs(signer2, 3) .to.emit(this.mock, 'ERC7913SignerWeightChanged') - .withArgs(signer3, 3); + .withArgs(signer3, 4); - // Increase threshold to 6 - await expect(this.mock.$_setThreshold(6)).to.emit(this.mock, 'ERC7913ThresholdSet').withArgs(6); + // Increase threshold to 9 + await expect(this.mock.$_setThreshold(9)).to.emit(this.mock, 'ERC7913ThresholdSet').withArgs(9); // 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( + await expect(this.mock.$_setSignerWeights([signer1, signer2, signer3], [2, 2, 2])).to.be.revertedWithCustomError( this.mock, 'MultiSignerERC7913UnreachableThreshold', ); // Try to increase threshold to be larger than the total weight - await expect(this.mock.$_setThreshold(7)) + await expect(this.mock.$_setThreshold(10)) .to.be.revertedWithCustomError(this.mock, 'MultiSignerERC7913UnreachableThreshold') - .withArgs(6, 7); + .withArgs(9, 10); }); it('reports default weight of 1 for signers without explicit weight', async function () {