NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Do not emit an event when setSignerWeight is a no-op (#5775)

Browse Source 
Signed-off-by: Hadrien Croubois <hadrien.croubois@gmail.com>
This commit is contained in:
Hadrien Croubois
2025-07-03 21:19:29 +02:00
parent e1277f7ad2
commit 86d3dd9d9f
2 changed files with 25 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
contracts/utils/cryptography/signers/MultiSignerERC7913Weighted.sol
View File

@ -105,19 +105,23 @@ abstract contract MultiSignerERC7913Weighted is MultiSignerERC7913 {
uint256 extraWeightRemoved = 0; uint256 extraWeightRemoved = 0;
for (uint256 i = 0; i < signers.length; ++i) { for (uint256 i = 0; i < signers.length; ++i) {
bytes memory signer = signers[i]; bytes memory signer = signers[i];
uint64 weight = weights[i];
require(isSigner(signer), MultiSignerERC7913NonexistentSigner(signer)); require(isSigner(signer), MultiSignerERC7913NonexistentSigner(signer));
uint64 weight = weights[i];
require(weight > 0, MultiSignerERC7913WeightedInvalidWeight(signer, weight)); require(weight > 0, MultiSignerERC7913WeightedInvalidWeight(signer, weight));
unchecked { unchecked {
// Overflow impossible: weight values are bounded by uint64 and economic constraints uint64 oldExtraWeight = _extraWeights[signer];
extraWeightRemoved += _extraWeights[signer]; uint64 newExtraWeight = weight - 1;
extraWeightAdded += _extraWeights[signer] = weight - 1;
}
if (oldExtraWeight != newExtraWeight) {
// Overflow impossible: weight values are bounded by uint64 and economic constraints
extraWeightRemoved += oldExtraWeight;
extraWeightAdded += _extraWeights[signer] = newExtraWeight;
emit ERC7913SignerWeightChanged(signer, weight); emit ERC7913SignerWeightChanged(signer, weight);
} }
}
}
unchecked { unchecked {
// Safe from underflow: `extraWeightRemoved` is bounded by `_totalExtraWeight` by construction // Safe from underflow: `extraWeightRemoved` is bounded by `_totalExtraWeight` by construction
// and weight values are bounded by uint64 and economic constraints // and weight values are bounded by uint64 and economic constraints

24
test/account/AccountMultiSignerWeighted.test.js
View File

@ -158,6 +158,10 @@ describe('AccountMultiSignerWeighted', function () {
await expect(this.mock.signerWeight(signer3)).to.eventually.equal(3); // unchanged 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 () { it('cannot set weight to non-existent signer', async function () {
// Reverts when setting weight for non-existent signer // Reverts when setting weight for non-existent signer
await expect(this.mock.$_setSignerWeights([signer4], [1])) await expect(this.mock.$_setSignerWeights([signer4], [1]))
@ -186,28 +190,28 @@ describe('AccountMultiSignerWeighted', function () {
}); });
it('validates threshold is reachable when updating weights', async 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) // First, lower the weights so the sum is exactly 9 (just enough for threshold=9)
await expect(this.mock.$_setSignerWeights([signer1, signer2, signer3], [1, 2, 3])) await expect(this.mock.$_setSignerWeights([signer1, signer2, signer3], [2, 3, 4]))
.to.emit(this.mock, 'ERC7913SignerWeightChanged') .to.emit(this.mock, 'ERC7913SignerWeightChanged')
.withArgs(signer1, 1) .withArgs(signer1, 2)
.to.emit(this.mock, 'ERC7913SignerWeightChanged') .to.emit(this.mock, 'ERC7913SignerWeightChanged')
.withArgs(signer2, 2) .withArgs(signer2, 3)
.to.emit(this.mock, 'ERC7913SignerWeightChanged') .to.emit(this.mock, 'ERC7913SignerWeightChanged')
.withArgs(signer3, 3); .withArgs(signer3, 4);
// Increase threshold to 6 // Increase threshold to 9
await expect(this.mock.$_setThreshold(6)).to.emit(this.mock, 'ERC7913ThresholdSet').withArgs(6); 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 // 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, this.mock,
'MultiSignerERC7913UnreachableThreshold', 'MultiSignerERC7913UnreachableThreshold',
); );
// Try to increase threshold to be larger than the total weight // 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') .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 () { it('reports default weight of 1 for signers without explicit weight', async function () {