fix: revert changes on single signer policy

KintoXYZ · Jan 29, 2024 · 729f3a3 · 729f3a3
1 parent a54f039
commit 729f3a3
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 46 deletions.
diff --git a/src/wallet/KintoWallet.sol b/src/wallet/KintoWallet.sol
@@ -282,8 +282,8 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto
         // if app key is not set or signature is not valid, verify signer policy
         if (
             (
-                signerPolicy == SINGLE_SIGNER
-                    && _verifySingleSignature(hashData, userOp.signature) == SIG_VALIDATION_SUCCESS
+                signerPolicy == SINGLE_SIGNER && owners.length == 1
+                    && _verifySingleSignature(owners[0], hashData, userOp.signature) == SIG_VALIDATION_SUCCESS
             )
                 || (
                     signerPolicy != SINGLE_SIGNER
@@ -335,19 +335,6 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto
         return appRegistry.isSponsored(sponsor, target) || appRegistry.childToParentContract(target) == sponsor;
     }
 
-    // @notice ensures at least 1 signer has signed the hash
-    // @dev useful when owners.length > 1
-    function _verifySingleSignature(bytes32 hashData, bytes memory signature) private view returns (uint256) {
-        // ensure at least one signer has signed
-        address recovered = hashData.recover(signature);
-        for (uint256 i = 0; i < owners.length; i++) {
-            if (owners[i] == recovered) {
-                return _packValidationData(false, 0, 0);
-            }
-        }
-        return SIG_VALIDATION_FAILED;
-    }
-
     // @notice ensures signer has signed the hash
     function _verifySingleSignature(address signer, bytes32 hashData, bytes memory signature)
         private
@@ -386,6 +373,8 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto
         return _packValidationData(requiredSigners != 0, 0, 0);
     }
 
+    // @dev SINGLE_SIGNER policy expects the wallet to have only one owner though this is not enforced.
+    // Any "extra" owners won't be considered when validating the signature.
     function _resetSigners(address[] calldata newSigners, uint8 _policy) internal {
         require(newSigners.length > 0 && newSigners.length <= MAX_SIGNERS, "KW-rs: invalid array");
         require(newSigners[0] != address(0) && kintoID.isKYC(newSigners[0]), "KW-rs: KYC Required");
@@ -402,7 +391,8 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto
         }
         owners = newSigners;
         emit SignersChanged(newSigners, owners);
-        // Change policy if needed.
+
+        // change policy if needed.
         if (_policy != signerPolicy) {
             setSignerPolicy(_policy);
         } else {

diff --git a/test/wallet/validateSignature/ValidateSignature.t.sol b/test/wallet/validateSignature/ValidateSignature.t.sol
@@ -780,34 +780,4 @@ contract ValidateSignatureTest is SharedSetup {
             )
         );
     }
-
-    // special case 2: requiredSigners == 1, owners.length == 3 and the owner 2 is the signer.
-    function testValidateSignature_SpecialCase2() public {
-        // reset signers & change policy
-        address[] memory owners = new address[](3);
-        owners[0] = _owner;
-        owners[1] = _user;
-        owners[2] = _user2;
-        resetSigners(owners, _kintoWallet.SINGLE_SIGNER());
-
-        // create user op with owners 1 as signer
-        privateKeys = new uint256[](1);
-        privateKeys[0] = _userPk;
-
-        UserOperation memory userOp = _createUserOperation(
-            address(_kintoWallet),
-            address(counter),
-            _kintoWallet.getNonce(),
-            privateKeys,
-            abi.encodeWithSignature("increment()"),
-            address(_paymaster)
-        );
-
-        assertEq(
-            SIG_VALIDATION_SUCCESS,
-            KintoWalletHarness(payable(address(_kintoWallet))).exposed_validateSignature(
-                userOp, _entryPoint.getUserOpHash(userOp)
-            )
-        );
-    }
 }