From 8b85c79b52454606a11b5857018af797ee8f1c3d Mon Sep 17 00:00:00 2001 From: Mikhail <16622558+mmv08@users.noreply.github.com> Date: Fri, 28 Jun 2024 11:49:15 +0200 Subject: [PATCH] `Safe4337Mock`: Remove signature check function inlining and call the function via call instead (#450) In https://github.com/safe-global/safe-modules/pull/446, I inlined the signature check function because we didn't want it to revert in unsuccessful cases. I inlined it because the `try/catch` statement didn't work for internal function calls. Little did I know that I could do `this.f()` and that would use the `CALL` opcode instead of `JUMP`, making the `try/catch` statement possible More info: https://www.perplexity.ai/search/if-i-call-1VYIfl5eQEWvJ302U7wVOA#0 --- modules/4337/contracts/test/SafeMock.sol | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/modules/4337/contracts/test/SafeMock.sol b/modules/4337/contracts/test/SafeMock.sol index 2e8ec9942..c09005bc4 100644 --- a/modules/4337/contracts/test/SafeMock.sol +++ b/modules/4337/contracts/test/SafeMock.sol @@ -217,14 +217,12 @@ contract Safe4337Mock is SafeMock, IAccount { function _validateSignatures(PackedUserOperation calldata userOp) internal view returns (uint256 validationData) { (bytes memory operationData, uint48 validAfter, uint48 validUntil, bytes calldata signatures) = _getSafeOp(userOp); - bytes32 dataHash = keccak256(operationData); - uint8 v; - bytes32 r; - bytes32 s; - (v, r, s) = _signatureSplit(signatures); - bool validSignature = owner == ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s); - - validationData = _packValidationData(!validSignature, validUntil, validAfter); + try this.checkSignatures(keccak256(operationData), operationData, signatures) { + // The timestamps are validated by the entry point, therefore we will not check them again + validationData = _packValidationData(false, validUntil, validAfter); + } catch { + validationData = _packValidationData(true, validUntil, validAfter); + } } /**