From 7464e218ede641671168fa570dabb470707f46e4 Mon Sep 17 00:00:00 2001 From: Lyova Potyomkin Date: Mon, 25 Nov 2024 19:51:08 +0200 Subject: [PATCH] fix: eip1271 on sessions module (#198) * chore: remove a bunch of stuff * fix: tests and scripts * chore: cleanup * fix: remove eslint * chore: update lockfile * fix: eip1271 on sessions module * fix: spellcheck * fix: remove isValidSignature --- cspell-config/cspell-sol.txt | 2 ++ hardhat.config.ts | 5 ++++- src/helpers/Base64.sol | 2 +- src/helpers/EIP712.sol | 2 +- src/helpers/TokenCallbackHandler.sol | 2 +- src/helpers/VerifierCaller.sol | 2 +- src/test/VerifierCaller.sol | 2 +- src/validators/SessionKeyValidator.sol | 30 +++++++++++++++----------- test/utils.ts | 4 ++-- 9 files changed, 30 insertions(+), 21 deletions(-) diff --git a/cspell-config/cspell-sol.txt b/cspell-config/cspell-sol.txt index 9dd0c866..3967600e 100644 --- a/cspell-config/cspell-sol.txt +++ b/cspell-config/cspell-sol.txt @@ -32,3 +32,5 @@ RPID Raphson solady xbatch +tload +tstore diff --git a/hardhat.config.ts b/hardhat.config.ts index 1faa15b6..17a15383 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -52,7 +52,10 @@ const config: HardhatUserConfig = { }, }, solidity: { - version: "0.8.24", + version: "0.8.28", + settings: { + evmVersion: "cancun", + } }, }; diff --git a/src/helpers/Base64.sol b/src/helpers/Base64.sol index e258fc8b..b4f452fe 100644 --- a/src/helpers/Base64.sol +++ b/src/helpers/Base64.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v5.0.0) (utils/Base64.sol) -pragma solidity 0.8.24; +pragma solidity ^0.8.24; /** * @dev Provides a set of functions to operate with Base64 strings. diff --git a/src/helpers/EIP712.sol b/src/helpers/EIP712.sol index 140777f8..00505f43 100644 --- a/src/helpers/EIP712.sol +++ b/src/helpers/EIP712.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v5.0.0) (utils/cryptography/EIP712.sol) -pragma solidity 0.8.24; +pragma solidity ^0.8.24; // OpenZeppelin Contracts (last updated v5.0.0) (utils/cryptography/MessageHashUtils.sol) diff --git a/src/helpers/TokenCallbackHandler.sol b/src/helpers/TokenCallbackHandler.sol index 6703ef8a..4693f8d7 100644 --- a/src/helpers/TokenCallbackHandler.sol +++ b/src/helpers/TokenCallbackHandler.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-3.0 -pragma solidity 0.8.24; +pragma solidity ^0.8.24; import "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; import "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; diff --git a/src/helpers/VerifierCaller.sol b/src/helpers/VerifierCaller.sol index c975f8f9..41ce61fc 100644 --- a/src/helpers/VerifierCaller.sol +++ b/src/helpers/VerifierCaller.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-3.0 -pragma solidity 0.8.24; +pragma solidity ^0.8.24; abstract contract VerifierCaller { /** diff --git a/src/test/VerifierCaller.sol b/src/test/VerifierCaller.sol index 811e2eaa..63a43460 100644 --- a/src/test/VerifierCaller.sol +++ b/src/test/VerifierCaller.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-3.0 -pragma solidity 0.8.24; +pragma solidity ^0.8.24; abstract contract VerifierCaller { /** diff --git a/src/validators/SessionKeyValidator.sol b/src/validators/SessionKeyValidator.sol index ccdb4f3b..7609b5bf 100644 --- a/src/validators/SessionKeyValidator.sol +++ b/src/validators/SessionKeyValidator.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.23; +pragma solidity ^0.8.24; import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; @@ -42,8 +42,14 @@ contract SessionKeyValidator is IValidationHook, IModuleValidator, IModule { } function handleValidation(bytes32 signedHash, bytes memory signature) external view returns (bool) { - // this only validates that the session key is linked to the account, not the transaction against the session spec - return isValidSignature(signedHash, signature) == EIP1271_SUCCESS_RETURN_VALUE; + // This only succeeds if the validationHook has previously succeeded for this hash. + uint256 slot = uint256(signedHash); + uint256 hookResult; + assembly { + hookResult := tload(slot) + } + require(hookResult == 1, "Can't call this function without calling validationHook"); + return true; } function addValidationKey(bytes memory sessionData) external returns (bool) { @@ -114,9 +120,9 @@ contract SessionKeyValidator is IValidationHook, IModuleValidator, IModule { } /* - * If there are any spend limits configured + * Check if the validator is registered for the smart account * @param smartAccount The smart account to check - * @return true if spend limits are configured initialized, false otherwise + * @return true if validator is registered for the account, false otherwise */ function isInitialized(address smartAccount) external view returns (bool) { return _isInitialized(smartAccount); @@ -127,14 +133,6 @@ contract SessionKeyValidator is IValidationHook, IModuleValidator, IModule { // && IValidatorManager(smartAccount).isModuleValidator(address(this)); } - /* - * Currently doing 1271 validation, but might update the interface to match the zksync account validation - */ - function isValidSignature(bytes32 hash, bytes memory signature) public view returns (bytes4 magic) { - magic = EIP1271_SUCCESS_RETURN_VALUE; - // TODO: Does this method have to work standalone? If not, validationHook is sufficient for validation. - } - function validationHook(bytes32 signedHash, Transaction calldata transaction, bytes calldata hookData) external { (bytes memory signature, address validator, ) = abi.decode(transaction.signature, (bytes, address, bytes[])); if (validator != address(this)) { @@ -146,6 +144,12 @@ contract SessionKeyValidator is IValidationHook, IModuleValidator, IModule { require(recoveredAddress == spec.signer, "Invalid signer"); bytes32 sessionHash = keccak256(abi.encode(spec)); sessions[sessionHash].validate(transaction, spec); + + // Set the validation result to 1 for this hash, so that isValidSignature succeeds + uint256 slot = uint256(signedHash); + assembly { + tstore(slot, 1) + } } /** diff --git a/test/utils.ts b/test/utils.ts index 853f38a3..d4fa7ca4 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -274,8 +274,8 @@ export const LOCAL_RICH_WALLETS = [ privateKey: masterWallet.privateKey, } : { - address: "0xBC989fDe9e54cAd2aB4392Af6dF60f04873A033A", - privateKey: "0x3d3cbc973389cb26f657686445bcc75662b415b656078503592ac8c1abb8810e", + address: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266", + privateKey: "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", }, { address: "0x36615Cf349d7F6344891B1e7CA7C72883F5dc049",