Browse Source

MultiSignerERC7913: prevent setting the threshold to zero (#5772)

Hadrien Croubois 3 months ago
parent
commit
cc94ea4930

+ 4 - 0
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);

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