From cc94ea49301460aa6bd39edf29b2ab953d1d80b5 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 27 Jun 2025 13:29:40 +0200 Subject: [PATCH] MultiSignerERC7913: prevent setting the threshold to zero (#5772) --- .../utils/cryptography/signers/MultiSignerERC7913.sol | 4 ++++ test/account/AccountMultiSigner.test.js | 9 +++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/contracts/utils/cryptography/signers/MultiSignerERC7913.sol b/contracts/utils/cryptography/signers/MultiSignerERC7913.sol index cd9e97303..e318eb52c 100644 --- a/contracts/utils/cryptography/signers/MultiSignerERC7913.sol +++ b/contracts/utils/cryptography/signers/MultiSignerERC7913.sol @@ -66,6 +66,9 @@ abstract contract MultiSignerERC7913 is AbstractSigner { /// @dev The `signer` is less than 20 bytes long. error MultiSignerERC7913InvalidSigner(bytes signer); + /// @dev The `threshold` is zero. + error MultiSignerERC7913ZeroThreshold(); + /// @dev The `threshold` is unreachable given the number of `signers`. error MultiSignerERC7913UnreachableThreshold(uint64 signers, uint64 threshold); @@ -146,6 +149,7 @@ abstract contract MultiSignerERC7913 is AbstractSigner { * * See {_validateReachableThreshold} for the threshold validation. */ function _setThreshold(uint64 newThreshold) internal virtual { + require(newThreshold > 0, MultiSignerERC7913ZeroThreshold()); _threshold = newThreshold; _validateReachableThreshold(); emit ERC7913ThresholdSet(newThreshold); diff --git a/test/account/AccountMultiSigner.test.js b/test/account/AccountMultiSigner.test.js index db563a2f0..78011a390 100644 --- a/test/account/AccountMultiSigner.test.js +++ b/test/account/AccountMultiSigner.test.js @@ -176,9 +176,14 @@ describe('AccountMultiSigner', function () { await expect(this.mock.$_setThreshold(2)).to.emit(this.mock, 'ERC7913ThresholdSet'); // Unreachable threshold reverts - await expect(this.mock.$_setThreshold(3)).to.revertedWithCustomError( + await expect(this.mock.$_setThreshold(3)) + .to.revertedWithCustomError(this.mock, 'MultiSignerERC7913UnreachableThreshold') + .withArgs(2, 3); + + // Zero threshold reverts + await expect(this.mock.$_setThreshold(0)).to.revertedWithCustomError( this.mock, - 'MultiSignerERC7913UnreachableThreshold', + 'MultiSignerERC7913ZeroThreshold', ); });