diff --git a/src/modules/validation/ISingleSignerValidation.sol b/src/modules/validation/ISingleSignerValidation.sol index 2653b752..3c62f20e 100644 --- a/src/modules/validation/ISingleSignerValidation.sol +++ b/src/modules/validation/ISingleSignerValidation.sol @@ -19,10 +19,4 @@ interface ISingleSignerValidation is IValidation { /// @param entityId The entityId for the account and the signer. /// @param newSigner The address of the new signer. function transferSigner(uint32 entityId, address newSigner) external; - - /// @notice Get the signer of the `account`'s validation. - /// @param entityId The entityId for the account and the signer. - /// @param account The account to get the signer of. - /// @return The address of the signer. - function signerOf(uint32 entityId, address account) external view returns (address); } diff --git a/src/modules/validation/SingleSignerValidation.sol b/src/modules/validation/SingleSignerValidation.sol index 16ac6225..b9432009 100644 --- a/src/modules/validation/SingleSignerValidation.sol +++ b/src/modules/validation/SingleSignerValidation.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.25; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; -import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol"; @@ -25,7 +24,6 @@ import {ISingleSignerValidation} from "./ISingleSignerValidation.sol"; /// - This validation supports composition that other validation can relay on entities in this validation /// to validate partially or fully. contract SingleSignerValidation is ISingleSignerValidation, BaseModule { - using ECDSA for bytes32; using MessageHashUtils for bytes32; string internal constant _NAME = "SingleSigner Validation"; @@ -39,7 +37,7 @@ contract SingleSignerValidation is ISingleSignerValidation, BaseModule { bytes4 internal constant _1271_MAGIC_VALUE = 0x1626ba7e; bytes4 internal constant _1271_INVALID = 0xffffffff; - mapping(uint32 entityId => mapping(address account => address)) public signer; + mapping(uint32 entityId => mapping(address account => address)) public signers; /// @inheritdoc ISingleSignerValidation function transferSigner(uint32 entityId, address newSigner) external { @@ -59,11 +57,6 @@ contract SingleSignerValidation is ISingleSignerValidation, BaseModule { _transferSigner(abi.decode(data, (uint32)), address(0)); } - /// @inheritdoc ISingleSignerValidation - function signerOf(uint32 entityId, address account) external view returns (address) { - return signer[entityId][account]; - } - /// @inheritdoc IValidation function validateUserOp(uint32 entityId, PackedUserOperation calldata userOp, bytes32 userOpHash) external @@ -72,11 +65,14 @@ contract SingleSignerValidation is ISingleSignerValidation, BaseModule { returns (uint256) { // Validate the user op signature against the owner. - (address sigSigner,,) = (userOpHash.toEthSignedMessageHash()).tryRecover(userOp.signature); - if (sigSigner == address(0) || sigSigner != signer[entityId][userOp.sender]) { - return _SIG_VALIDATION_FAILED; + if ( + SignatureChecker.isValidSignatureNow( + signers[entityId][userOp.sender], userOpHash.toEthSignedMessageHash(), userOp.signature + ) + ) { + return _SIG_VALIDATION_PASSED; } - return _SIG_VALIDATION_PASSED; + return _SIG_VALIDATION_FAILED; } /// @inheritdoc IValidation @@ -89,7 +85,7 @@ contract SingleSignerValidation is ISingleSignerValidation, BaseModule { bytes calldata ) external view override { // Validate that the sender is the owner of the account or self. - if (sender != signer[entityId][account]) { + if (sender != signers[entityId][account]) { revert NotAuthorized(); } return; @@ -108,7 +104,7 @@ contract SingleSignerValidation is ISingleSignerValidation, BaseModule { override returns (bytes4) { - if (SignatureChecker.isValidSignatureNow(signer[entityId][account], digest, signature)) { + if (SignatureChecker.isValidSignatureNow(signers[entityId][account], digest, signature)) { return _1271_MAGIC_VALUE; } return _1271_INVALID; @@ -132,8 +128,8 @@ contract SingleSignerValidation is ISingleSignerValidation, BaseModule { // ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ function _transferSigner(uint32 entityId, address newSigner) internal { - address previousSigner = signer[entityId][msg.sender]; - signer[entityId][msg.sender] = newSigner; + address previousSigner = signers[entityId][msg.sender]; + signers[entityId][msg.sender] = newSigner; emit SignerTransferred(msg.sender, entityId, previousSigner, newSigner); } } diff --git a/test/account/UpgradeableModularAccount.t.sol b/test/account/UpgradeableModularAccount.t.sol index a492e7ab..6e305197 100644 --- a/test/account/UpgradeableModularAccount.t.sol +++ b/test/account/UpgradeableModularAccount.t.sol @@ -354,7 +354,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { } function test_transferOwnership() public { - assertEq(singleSignerValidation.signerOf(TEST_DEFAULT_VALIDATION_ENTITY_ID, address(account1)), owner1); + assertEq(singleSignerValidation.signers(TEST_DEFAULT_VALIDATION_ENTITY_ID, address(account1)), owner1); vm.prank(address(entryPoint)); account1.execute( @@ -363,7 +363,7 @@ contract UpgradeableModularAccountTest is AccountTestBase { abi.encodeCall(SingleSignerValidation.transferSigner, (TEST_DEFAULT_VALIDATION_ENTITY_ID, owner2)) ); - assertEq(singleSignerValidation.signerOf(TEST_DEFAULT_VALIDATION_ENTITY_ID, address(account1)), owner2); + assertEq(singleSignerValidation.signers(TEST_DEFAULT_VALIDATION_ENTITY_ID, address(account1)), owner2); } function test_isValidSignature() public { diff --git a/test/module/SingleSignerValidation.t.sol b/test/module/SingleSignerValidation.t.sol index 89e5965c..beff705a 100644 --- a/test/module/SingleSignerValidation.t.sol +++ b/test/module/SingleSignerValidation.t.sol @@ -121,7 +121,7 @@ contract SingleSignerValidationTest is AccountTestBase { // transfer ownership to signer singleSignerValidation.transferSigner(TEST_DEFAULT_VALIDATION_ENTITY_ID, signer); - assertEq(signer, singleSignerValidation.signerOf(TEST_DEFAULT_VALIDATION_ENTITY_ID, accountAddr)); + assertEq(signer, singleSignerValidation.signers(TEST_DEFAULT_VALIDATION_ENTITY_ID, accountAddr)); // sig check should pass assertEq(