From 22ff5e5d68f319a9366ab90fe39d100083b27b62 Mon Sep 17 00:00:00 2001 From: adam Date: Fri, 23 Aug 2024 15:22:47 -0700 Subject: [PATCH] feat: add flag for user op validation and pack efficiently --- src/account/AccountFactory.sol | 2 +- src/account/AccountStorage.sol | 4 +- src/account/ModularAccountView.sol | 1 + src/account/ModuleManagerInternals.sol | 1 + src/account/ReferenceModularAccount.sol | 7 ++ src/helpers/ValidationConfigLib.sol | 99 +++++++++++------ src/interfaces/IModularAccount.sol | 19 +++- src/interfaces/IModularAccountView.sol | 2 + test/account/AccountReturnData.t.sol | 4 +- test/account/DirectCallsFromModule.t.sol | 6 +- test/account/ModularAccountView.t.sol | 5 +- test/account/MultiValidation.t.sol | 2 +- test/account/PerHookData.t.sol | 8 +- test/account/ReferenceModularAccount.t.sol | 104 +++++++++++++++++- test/account/ReplaceModule.t.sol | 4 +- test/account/SelfCallAuthorization.t.sol | 4 +- test/account/ValidationIntersection.t.sol | 9 +- test/libraries/ValidationConfigLib.t.sol | 44 ++++---- test/mocks/SingleSignerFactoryFixture.sol | 2 +- test/module/AllowlistModule.t.sol | 5 +- test/module/ERC20TokenLimitModule.t.sol | 2 +- test/module/NativeTokenLimitModule.t.sol | 2 +- .../module/SingleSignerValidationModule.t.sol | 2 +- test/utils/CustomValidationTestBase.sol | 6 +- 24 files changed, 255 insertions(+), 89 deletions(-) diff --git a/src/account/AccountFactory.sol b/src/account/AccountFactory.sol index 7f2b8442..7515df9d 100644 --- a/src/account/AccountFactory.sol +++ b/src/account/AccountFactory.sol @@ -59,7 +59,7 @@ contract AccountFactory is Ownable { new ERC1967Proxy{salt: combinedSalt}(address(ACCOUNT_IMPL), ""); // point proxy to actual implementation and init plugins ReferenceModularAccount(payable(addr)).initializeWithValidation( - ValidationConfigLib.pack(SINGLE_SIGNER_VALIDATION_MODULE, entityId, true, true), + ValidationConfigLib.pack(SINGLE_SIGNER_VALIDATION_MODULE, entityId, true, true, true), new bytes4[](0), pluginInstallData, new bytes[](0) diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol index b3069bdb..7681e294 100644 --- a/src/account/AccountStorage.sol +++ b/src/account/AccountStorage.sol @@ -27,8 +27,10 @@ struct ExecutionData { struct ValidationData { // Whether or not this validation can be used as a global validation function. bool isGlobal; - // Whether or not this validation is a signature validator. + // Whether or not this validation is allowed to validate ERC-1271 signatures. bool isSignatureValidation; + // Whether or not this validation is allowed to validate ERC-4337 user operations. + bool isUserOpValidation; // The pre validation hooks for this validation function. ModuleEntity[] preValidationHooks; // Permission hooks for this validation function. diff --git a/src/account/ModularAccountView.sol b/src/account/ModularAccountView.sol index 2d5ec56e..c432f02f 100644 --- a/src/account/ModularAccountView.sol +++ b/src/account/ModularAccountView.sol @@ -49,6 +49,7 @@ abstract contract ModularAccountView is IModularAccountView { ValidationData storage validationData = getAccountStorage().validationData[validationFunction]; data.isGlobal = validationData.isGlobal; data.isSignatureValidation = validationData.isSignatureValidation; + data.isUserOpValidation = validationData.isUserOpValidation; data.preValidationHooks = validationData.preValidationHooks; uint256 permissionHooksLen = validationData.permissionHooks.length(); diff --git a/src/account/ModuleManagerInternals.sol b/src/account/ModuleManagerInternals.sol index 04130e41..3731264d 100644 --- a/src/account/ModuleManagerInternals.sol +++ b/src/account/ModuleManagerInternals.sol @@ -261,6 +261,7 @@ abstract contract ModuleManagerInternals is IModularAccount { _validationData.isGlobal = validationConfig.isGlobal(); _validationData.isSignatureValidation = validationConfig.isSignatureValidation(); + _validationData.isUserOpValidation = validationConfig.isUserOpValidation(); _onInstall(validationConfig.module(), installData, type(IValidationModule).interfaceId); emit ValidationInstalled(validationConfig.module(), validationConfig.entityId()); diff --git a/src/account/ReferenceModularAccount.sol b/src/account/ReferenceModularAccount.sol index fe136861..d142e8ef 100644 --- a/src/account/ReferenceModularAccount.sol +++ b/src/account/ReferenceModularAccount.sol @@ -76,6 +76,7 @@ contract ReferenceModularAccount is error RuntimeValidationFunctionReverted(address module, uint32 entityId, bytes revertReason); error SelfCallRecursionDepthExceeded(); error SignatureValidationInvalid(address module, uint32 entityId); + error UserOpValidationInvalid(address module, uint32 entityId); error UnexpectedAggregator(address module, uint32 entityId, address aggregator); error UnrecognizedFunction(bytes4 selector); error ValidationFunctionMissing(bytes4 selector); @@ -593,8 +594,14 @@ contract ReferenceModularAccount is PackedUserOperation memory userOp, bytes32 userOpHash ) internal virtual returns (uint256) { + AccountStorage storage _storage = getAccountStorage(); + (address module, uint32 entityId) = userOpValidationFunction.unpack(); + if (!_storage.validationData[userOpValidationFunction].isUserOpValidation) { + revert UserOpValidationInvalid(module, entityId); + } + return IValidationModule(module).validateUserOp(entityId, userOp, userOpHash); } diff --git a/src/helpers/ValidationConfigLib.sol b/src/helpers/ValidationConfigLib.sol index cad994c3..be3ff705 100644 --- a/src/helpers/ValidationConfigLib.sol +++ b/src/helpers/ValidationConfigLib.sol @@ -7,42 +7,55 @@ import {ModuleEntity, ValidationConfig} from "../interfaces/IModularAccount.sol" // Layout: // 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address // 0x________________________________________BBBBBBBB________________ // Entity ID -// 0x________________________________________________CC______________ // isGlobal -// 0x__________________________________________________DD____________ // isSignatureValidation -// 0x____________________________________________________000000000000 // unused +// 0x________________________________________________CC______________ // validation flags +// 0x__________________________________________________00000000000000 // unused + +// Validation flags layout: +// 0b00000___ // unused +// 0b_____A__ // isGlobal +// 0b______B_ // isSignatureValidation +// 0b_______C // isUserOpValidation library ValidationConfigLib { - function pack(ModuleEntity _validationFunction, bool _isGlobal, bool _isSignatureValidation) - internal - pure - returns (ValidationConfig) - { + // is user op validation flag stored in last bit of the 25th byte + bytes32 internal constant _VALIDATION_FLAG_IS_USER_OP = bytes32(uint256(1) << 56); + // is signature validation flag stored in second to last bit of the 25th byte + bytes32 internal constant _VALIDATION_FLAG_IS_SIGNATURE = bytes32(uint256(1) << 57); + // is global flag stored in the third to last bit of the 25th byte + bytes32 internal constant _VALIDATION_FLAG_IS_GLOBAL = bytes32(uint256(1) << 58); + + function pack( + ModuleEntity _validationFunction, + bool _isGlobal, + bool _isSignatureValidation, + bool _isUserOpValidation + ) internal pure returns (ValidationConfig) { return ValidationConfig.wrap( - bytes26( - bytes26(ModuleEntity.unwrap(_validationFunction)) - // isGlobal flag stored in the 25th byte - | bytes26(bytes32(_isGlobal ? uint256(1) << 56 : 0)) - // isSignatureValidation flag stored in the 26th byte - | bytes26(bytes32(_isSignatureValidation ? uint256(1) << 48 : 0)) + bytes25( + bytes25(ModuleEntity.unwrap(_validationFunction)) + | bytes25(bytes32(_isGlobal ? _VALIDATION_FLAG_IS_GLOBAL : bytes32(0))) + | bytes25(bytes32(_isSignatureValidation ? _VALIDATION_FLAG_IS_SIGNATURE : bytes32(0))) + | bytes25(bytes32(_isUserOpValidation ? _VALIDATION_FLAG_IS_USER_OP : bytes32(0))) ) ); } - function pack(address _module, uint32 _entityId, bool _isGlobal, bool _isSignatureValidation) - internal - pure - returns (ValidationConfig) - { + function pack( + address _module, + uint32 _entityId, + bool _isGlobal, + bool _isSignatureValidation, + bool _isUserOpValidation + ) internal pure returns (ValidationConfig) { return ValidationConfig.wrap( - bytes26( + bytes25( // module address stored in the first 20 bytes - bytes26(bytes20(_module)) + bytes25(bytes20(_module)) // entityId stored in the 21st - 24th byte - | bytes26(bytes24(uint192(_entityId))) - // isGlobal flag stored in the 25th byte - | bytes26(bytes32(_isGlobal ? uint256(1) << 56 : 0)) - // isSignatureValidation flag stored in the 26th byte - | bytes26(bytes32(_isSignatureValidation ? uint256(1) << 48 : 0)) + | bytes25(bytes24(uint192(_entityId))) + | bytes25(bytes32(_isGlobal ? _VALIDATION_FLAG_IS_GLOBAL : bytes32(0))) + | bytes25(bytes32(_isSignatureValidation ? _VALIDATION_FLAG_IS_SIGNATURE : bytes32(0))) + | bytes25(bytes32(_isUserOpValidation ? _VALIDATION_FLAG_IS_USER_OP : bytes32(0))) ) ); } @@ -50,24 +63,22 @@ library ValidationConfigLib { function unpackUnderlying(ValidationConfig config) internal pure - returns (address _module, uint32 _entityId, bool _isGlobal, bool _isSignatureValidation) + returns (address _module, uint32 _entityId, uint8 flags) { - bytes26 configBytes = ValidationConfig.unwrap(config); + bytes25 configBytes = ValidationConfig.unwrap(config); _module = address(bytes20(configBytes)); _entityId = uint32(bytes4(configBytes << 160)); - _isGlobal = uint8(configBytes[24]) == 1; - _isSignatureValidation = uint8(configBytes[25]) == 1; + flags = uint8(configBytes[24]); } function unpack(ValidationConfig config) internal pure - returns (ModuleEntity _validationFunction, bool _isGlobal, bool _isSignatureValidation) + returns (ModuleEntity _validationFunction, uint8 flags) { - bytes26 configBytes = ValidationConfig.unwrap(config); + bytes25 configBytes = ValidationConfig.unwrap(config); _validationFunction = ModuleEntity.wrap(bytes24(configBytes)); - _isGlobal = uint8(configBytes[24]) == 1; - _isSignatureValidation = uint8(configBytes[25]) == 1; + flags = uint8(configBytes[24]); } function module(ValidationConfig config) internal pure returns (address) { @@ -83,10 +94,26 @@ library ValidationConfigLib { } function isGlobal(ValidationConfig config) internal pure returns (bool) { - return uint8(ValidationConfig.unwrap(config)[24]) == 1; + return ValidationConfig.unwrap(config) & _VALIDATION_FLAG_IS_GLOBAL != 0; + } + + function isGlobal(uint8 flags) internal pure returns (bool) { + return flags & 0x04 != 0; } function isSignatureValidation(ValidationConfig config) internal pure returns (bool) { - return uint8(ValidationConfig.unwrap(config)[25]) == 1; + return ValidationConfig.unwrap(config) & _VALIDATION_FLAG_IS_SIGNATURE != 0; + } + + function isSignatureValidation(uint8 flags) internal pure returns (bool) { + return flags & 0x02 != 0; + } + + function isUserOpValidation(ValidationConfig config) internal pure returns (bool) { + return ValidationConfig.unwrap(config) & _VALIDATION_FLAG_IS_USER_OP != 0; + } + + function isUserOpValidation(uint8 flags) internal pure returns (bool) { + return flags & 0x01 != 0; } } diff --git a/src/interfaces/IModularAccount.sol b/src/interfaces/IModularAccount.sol index 6ce78d44..680d1b68 100644 --- a/src/interfaces/IModularAccount.sol +++ b/src/interfaces/IModularAccount.sol @@ -4,8 +4,25 @@ pragma solidity ^0.8.25; import {ExecutionManifest} from "./IExecutionModule.sol"; type ModuleEntity is bytes24; +// ModuleEntity is a packed representation of a module function +// Layout: +// 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address +// 0x________________________________________BBBBBBBB________________ // Entity ID +// 0x________________________________________________0000000000000000 // unused -type ValidationConfig is bytes26; +type ValidationConfig is bytes25; +// ValidationConfig is a packed representation of a validation function and flags for its configuration. +// Layout: +// 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address +// 0x________________________________________BBBBBBBB________________ // Entity ID +// 0x________________________________________________CC______________ // validation flags +// 0x__________________________________________________00000000000000 // unused +// +// Validation flags layout: +// 0b00000___ // unused +// 0b_____A__ // isGlobal +// 0b______B_ // isSignatureValidation +// 0b_______C // isUserOpValidation type HookConfig is bytes26; diff --git a/src/interfaces/IModularAccountView.sol b/src/interfaces/IModularAccountView.sol index e5d11877..8b27cd97 100644 --- a/src/interfaces/IModularAccountView.sol +++ b/src/interfaces/IModularAccountView.sol @@ -24,6 +24,8 @@ struct ValidationDataView { bool isGlobal; // Whether or not this validation is a signature validator. bool isSignatureValidation; + // Whether or not this validation is a user operation validator. + bool isUserOpValidation; // The pre validation hooks for this validation function. ModuleEntity[] preValidationHooks; // Permission hooks for this validation function. diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index 08611074..21192131 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -43,7 +43,9 @@ contract AccountReturnDataTest is AccountTestBase { bytes4[] memory selectors = new bytes4[](1); selectors[0] = IModularAccount.execute.selector; account1.installValidation( - ValidationConfigLib.pack(address(resultConsumerModule), DIRECT_CALL_VALIDATION_ENTITYID, false, false), + ValidationConfigLib.pack( + address(resultConsumerModule), DIRECT_CALL_VALIDATION_ENTITYID, false, false, true + ), // todo: does this need UO validation permission? selectors, "", new bytes[](0) diff --git a/test/account/DirectCallsFromModule.t.sol b/test/account/DirectCallsFromModule.t.sol index c85a9c31..668ef0c2 100644 --- a/test/account/DirectCallsFromModule.t.sol +++ b/test/account/DirectCallsFromModule.t.sol @@ -128,7 +128,7 @@ contract DirectCallsFromModuleTest is AccountTestBase { vm.prank(address(entryPoint)); account1.installValidation( - ValidationConfigLib.pack(extraOwner, DIRECT_CALL_VALIDATION_ENTITYID, false, false), + ValidationConfigLib.pack(extraOwner, DIRECT_CALL_VALIDATION_ENTITYID, false, false, false), selectors, "", new bytes[](0) @@ -154,7 +154,7 @@ contract DirectCallsFromModuleTest is AccountTestBase { vm.prank(address(entryPoint)); - ValidationConfig validationConfig = ValidationConfigLib.pack(_moduleEntity, false, false); + ValidationConfig validationConfig = ValidationConfigLib.pack(_moduleEntity, false, false, false); account1.installValidation(validationConfig, selectors, "", hooks); } @@ -168,7 +168,7 @@ contract DirectCallsFromModuleTest is AccountTestBase { vm.prank(address(entryPoint)); - ValidationConfig validationConfig = ValidationConfigLib.pack(_moduleEntity, true, false); + ValidationConfig validationConfig = ValidationConfigLib.pack(_moduleEntity, true, false, false); account1.installValidation(validationConfig, new bytes4[](0), "", hooks); } diff --git a/test/account/ModularAccountView.t.sol b/test/account/ModularAccountView.t.sol index 40cfd369..9806116b 100644 --- a/test/account/ModularAccountView.t.sol +++ b/test/account/ModularAccountView.t.sol @@ -102,6 +102,7 @@ contract ModularAccountViewTest is CustomValidationTestBase { assertTrue(data.isGlobal); assertTrue(data.isSignatureValidation); + assertTrue(data.isUserOpValidation); assertEq(data.preValidationHooks.length, 2); assertEq( ModuleEntity.unwrap(data.preValidationHooks[0]), @@ -131,7 +132,7 @@ contract ModularAccountViewTest is CustomValidationTestBase { internal virtual override - returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes[] memory) + returns (ModuleEntity, bool, bool, bool, bytes4[] memory, bytes memory, bytes[] memory) { bytes[] memory hooks = new bytes[](2); hooks[0] = abi.encodePacked( @@ -148,6 +149,6 @@ contract ModularAccountViewTest is CustomValidationTestBase { bytes4[] memory selectors = new bytes4[](1); selectors[0] = comprehensiveModule.foo.selector; - return (comprehensiveModuleValidation, true, true, selectors, bytes(""), hooks); + return (comprehensiveModuleValidation, true, true, true, selectors, bytes(""), hooks); } } diff --git a/test/account/MultiValidation.t.sol b/test/account/MultiValidation.t.sol index e1081061..73a17395 100644 --- a/test/account/MultiValidation.t.sol +++ b/test/account/MultiValidation.t.sol @@ -36,7 +36,7 @@ contract MultiValidationTest is AccountTestBase { function test_overlappingValidationInstall() public { vm.prank(address(entryPoint)); account1.installValidation( - ValidationConfigLib.pack(address(validator2), TEST_DEFAULT_VALIDATION_ENTITY_ID, true, true), + ValidationConfigLib.pack(address(validator2), TEST_DEFAULT_VALIDATION_ENTITY_ID, true, true, true), new bytes4[](0), abi.encode(TEST_DEFAULT_VALIDATION_ENTITY_ID, owner2), new bytes[](0) diff --git a/test/account/PerHookData.t.sol b/test/account/PerHookData.t.sol index 7e391f8b..fa4e58fb 100644 --- a/test/account/PerHookData.t.sol +++ b/test/account/PerHookData.t.sol @@ -493,7 +493,7 @@ contract PerHookDataTest is CustomValidationTestBase { ); vm.prank(address(entryPoint)); account1.installValidation( - ValidationConfigLib.pack(_signerValidation, true, false), new bytes4[](0), "", hooks + ValidationConfigLib.pack(_signerValidation, true, false, true), new bytes4[](0), "", hooks ); } @@ -523,7 +523,7 @@ contract PerHookDataTest is CustomValidationTestBase { internal virtual override - returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes[] memory) + returns (ModuleEntity, bool, bool, bool, bytes4[] memory, bytes memory, bytes[] memory) { bytes[] memory hooks = new bytes[](1); hooks[0] = abi.encodePacked( @@ -532,6 +532,8 @@ contract PerHookDataTest is CustomValidationTestBase { ); // patched to also work during SMA tests by differentiating the validation _signerValidation = ModuleEntityLib.pack(address(singleSignerValidationModule), _VALIDATION_ENTITY_ID); - return (_signerValidation, true, true, new bytes4[](0), abi.encode(_VALIDATION_ENTITY_ID, owner1), hooks); + return ( + _signerValidation, true, true, true, new bytes4[](0), abi.encode(_VALIDATION_ENTITY_ID, owner1), hooks + ); } } diff --git a/test/account/ReferenceModularAccount.t.sol b/test/account/ReferenceModularAccount.t.sol index 8da004ae..6dca79af 100644 --- a/test/account/ReferenceModularAccount.t.sol +++ b/test/account/ReferenceModularAccount.t.sol @@ -3,27 +3,24 @@ pragma solidity ^0.8.19; import {console} from "forge-std/Test.sol"; +import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; - import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {ModuleManagerInternals} from "../../src/account/ModuleManagerInternals.sol"; - import {ReferenceModularAccount} from "../../src/account/ReferenceModularAccount.sol"; import {SemiModularAccount} from "../../src/account/SemiModularAccount.sol"; - +import {ModuleEntityLib} from "../../src/helpers/ModuleEntityLib.sol"; +import {ValidationConfigLib} from "../../src/helpers/ValidationConfigLib.sol"; import {ExecutionManifest} from "../../src/interfaces/IExecutionModule.sol"; - import {Call} from "../../src/interfaces/IModularAccount.sol"; import {ExecutionDataView} from "../../src/interfaces/IModularAccountView.sol"; - import {TokenReceiverModule} from "../../src/modules/TokenReceiverModule.sol"; import {SingleSignerValidationModule} from "../../src/modules/validation/SingleSignerValidationModule.sol"; import {Counter} from "../mocks/Counter.sol"; - import {MockModule} from "../mocks/MockModule.sol"; import {ComprehensiveModule} from "../mocks/modules/ComprehensiveModule.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; @@ -415,6 +412,101 @@ contract ReferenceModularAccountTest is AccountTestBase { assertEq(validationResult, bytes4(0x1626ba7e)); } + // Only need a test case for the negative case, as the positive case is covered by the isValidSignature test + function test_signatureValidationFlag_enforce() public { + // Install a new copy of SingleSignerValidationModule with the signature validation flag set to false + uint32 newEntityId = 2; + vm.prank(address(entryPoint)); + account1.installValidation( + ValidationConfigLib.pack(address(singleSignerValidationModule), newEntityId, false, false, true), + new bytes4[](0), + abi.encode(newEntityId, owner1), + new bytes[](0) + ); + + bytes32 message = keccak256("hello world"); + + bytes32 replaySafeHash = vm.envOr("SMA_TEST", false) + ? SemiModularAccount(payable(account1)).replaySafeHash(message) + : singleSignerValidationModule.replaySafeHash(address(account1), message); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, replaySafeHash); + + bytes memory signature = _encode1271Signature( + ModuleEntityLib.pack(address(singleSignerValidationModule), newEntityId), abi.encodePacked(r, s, v) + ); + + vm.expectRevert( + abi.encodeWithSelector( + ReferenceModularAccount.SignatureValidationInvalid.selector, + singleSignerValidationModule, + newEntityId + ) + ); + IERC1271(address(account1)).isValidSignature(message, signature); + } + + function test_userOpValidationFlag_enforce() public { + // Install a new copy of SingleSignerValidationModule with the userOp validation flag set to false + uint32 newEntityId = 2; + vm.prank(address(entryPoint)); + account1.installValidation( + ValidationConfigLib.pack(address(singleSignerValidationModule), newEntityId, true, false, false), + new bytes4[](0), + abi.encode(newEntityId, owner1), + new bytes[](0) + ); + + PackedUserOperation memory userOp = PackedUserOperation({ + sender: address(account1), + nonce: 0, + initCode: "", + callData: abi.encodeCall(ReferenceModularAccount.execute, (ethRecipient, 1 wei, "")), + accountGasLimits: _encodeGas(VERIFICATION_GAS_LIMIT, CALL_GAS_LIMIT), + preVerificationGas: 0, + gasFees: _encodeGas(1, 1), + paymasterAndData: "", + signature: "" + }); + + // Generate signature + bytes32 userOpHash = entryPoint.getUserOpHash(userOp); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); + userOp.signature = _encodeSignature( + ModuleEntityLib.pack(address(singleSignerValidationModule), newEntityId), + GLOBAL_VALIDATION, + abi.encodePacked(r, s, v) + ); + + PackedUserOperation[] memory userOps = new PackedUserOperation[](1); + userOps[0] = userOp; + + vm.expectRevert( + abi.encodeWithSelector( + IEntryPoint.FailedOpWithRevert.selector, + 0, + "AA23 reverted", + abi.encodeWithSelector( + ReferenceModularAccount.UserOpValidationInvalid.selector, + singleSignerValidationModule, + newEntityId + ) + ) + ); + entryPoint.handleOps(userOps, beneficiary); + + //show working rt validation + vm.startPrank(address(owner1)); + account1.executeWithAuthorization( + abi.encodeCall(ReferenceModularAccount.execute, (ethRecipient, 1 wei, "")), + _encodeSignature( + ModuleEntityLib.pack(address(singleSignerValidationModule), newEntityId), GLOBAL_VALIDATION, "" + ) + ); + + assertEq(ethRecipient.balance, 2 wei); + } + // Internal Functions function _printStorageReadsAndWrites(address addr) internal { diff --git a/test/account/ReplaceModule.t.sol b/test/account/ReplaceModule.t.sol index 79c6f469..050a9e09 100644 --- a/test/account/ReplaceModule.t.sol +++ b/test/account/ReplaceModule.t.sol @@ -125,7 +125,7 @@ contract UpgradeModuleTest is AccountTestBase { vm.prank(address(entryPoint)); account1.installValidation( - ValidationConfigLib.pack(currModuleEntity, true, false), + ValidationConfigLib.pack(currModuleEntity, true, false, true), new bytes4[](0), abi.encode(validationEntityId1, owner1), hooksForVal1 @@ -174,7 +174,7 @@ contract UpgradeModuleTest is AccountTestBase { data: abi.encodeCall( IModularAccount.installValidation, ( - ValidationConfigLib.pack(newModuleEntity, true, false), + ValidationConfigLib.pack(newModuleEntity, true, false, true), new bytes4[](0), abi.encode(validationEntityId2, owner1), hooksForVal2 diff --git a/test/account/SelfCallAuthorization.t.sol b/test/account/SelfCallAuthorization.t.sol index c0b8a2b9..3c3eed5e 100644 --- a/test/account/SelfCallAuthorization.t.sol +++ b/test/account/SelfCallAuthorization.t.sol @@ -33,7 +33,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { vm.startPrank(address(entryPoint)); account1.installExecution(address(comprehensiveModule), comprehensiveModule.executionManifest(), ""); account1.installValidation( - ValidationConfigLib.pack(comprehensiveModuleValidation, false, false), + ValidationConfigLib.pack(comprehensiveModuleValidation, false, false, true), validationSelectors, "", new bytes[](0) @@ -304,7 +304,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { abi.encodeCall( ReferenceModularAccount.installValidation, ( - ValidationConfigLib.pack(comprehensiveModuleValidation, false, false), + ValidationConfigLib.pack(comprehensiveModuleValidation, false, false, true), selectors, "", new bytes[](0) diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index 626fbcc4..097a12aa 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -54,7 +54,10 @@ contract ValidationIntersectionTest is AccountTestBase { vm.startPrank(address(entryPoint)); // Install noHookValidation account1.installValidation( - ValidationConfigLib.pack(noHookValidation, true, true), validationSelectors, bytes(""), new bytes[](0) + ValidationConfigLib.pack(noHookValidation, true, true, true), + validationSelectors, + bytes(""), + new bytes[](0) ); // Install oneHookValidation @@ -66,7 +69,7 @@ contract ValidationIntersectionTest is AccountTestBase { ) ); account1.installValidation( - ValidationConfigLib.pack(oneHookValidation, true, true), validationSelectors, bytes(""), hooks + ValidationConfigLib.pack(oneHookValidation, true, true, true), validationSelectors, bytes(""), hooks ); // Install twoHookValidation @@ -83,7 +86,7 @@ contract ValidationIntersectionTest is AccountTestBase { ) ); account1.installValidation( - ValidationConfigLib.pack(twoHookValidation, true, true), validationSelectors, bytes(""), hooks + ValidationConfigLib.pack(twoHookValidation, true, true, true), validationSelectors, bytes(""), hooks ); vm.stopPrank(); } diff --git a/test/libraries/ValidationConfigLib.t.sol b/test/libraries/ValidationConfigLib.t.sol index da46f2cf..44f94231 100644 --- a/test/libraries/ValidationConfigLib.t.sol +++ b/test/libraries/ValidationConfigLib.t.sol @@ -8,7 +8,7 @@ import {ValidationConfig, ValidationConfigLib} from "../../src/helpers/Validatio contract ValidationConfigLibTest is Test { using ModuleEntityLib for ModuleEntity; - using ValidationConfigLib for ValidationConfig; + using ValidationConfigLib for *; // Tests the packing and unpacking of a validation config with a randomized state @@ -16,33 +16,35 @@ contract ValidationConfigLibTest is Test { address module, uint32 entityId, bool isGlobal, - bool isSignatureValidation + bool isSignatureValidation, + bool isUserOpValidation ) public { ValidationConfig validationConfig = - ValidationConfigLib.pack(module, entityId, isGlobal, isSignatureValidation); + ValidationConfigLib.pack(module, entityId, isGlobal, isSignatureValidation, isUserOpValidation); // Test unpacking underlying - (address module2, uint32 entityId2, bool isGlobal2, bool isSignatureValidation2) = - validationConfig.unpackUnderlying(); + (address module2, uint32 entityId2, uint8 flags2) = validationConfig.unpackUnderlying(); assertEq(module, module2, "module mismatch"); assertEq(entityId, entityId2, "entityId mismatch"); - assertEq(isGlobal, isGlobal2, "isGlobal mismatch"); - assertEq(isSignatureValidation, isSignatureValidation2, "isSignatureValidation mismatch"); + assertEq(isGlobal, flags2.isGlobal(), "isGlobal mismatch"); + assertEq(isSignatureValidation, flags2.isSignatureValidation(), "isSignatureValidation mismatch"); + assertEq(isUserOpValidation, flags2.isUserOpValidation(), "isUserOpValidation mismatch"); // Test unpacking to ModuleEntity ModuleEntity expectedModuleEntity = ModuleEntityLib.pack(module, entityId); - (ModuleEntity validationFunction, bool isGlobal3, bool isSignatureValidation3) = validationConfig.unpack(); + (ModuleEntity validationFunction, uint8 flags3) = validationConfig.unpack(); assertEq( ModuleEntity.unwrap(validationFunction), ModuleEntity.unwrap(expectedModuleEntity), "validationFunction mismatch" ); - assertEq(isGlobal, isGlobal3, "isGlobal mismatch"); - assertEq(isSignatureValidation, isSignatureValidation3, "isSignatureValidation mismatch"); + assertEq(isGlobal, flags3.isGlobal(), "isGlobal mismatch"); + assertEq(isSignatureValidation, flags3.isSignatureValidation(), "isSignatureValidation mismatch"); + assertEq(isUserOpValidation, flags3.isUserOpValidation(), "isUserOpValidation mismatch"); // Test individual view functions @@ -55,39 +57,42 @@ contract ValidationConfigLibTest is Test { ); assertEq(validationConfig.isGlobal(), isGlobal, "isGlobal mismatch"); assertEq(validationConfig.isSignatureValidation(), isSignatureValidation, "isSignatureValidation mismatch"); + assertEq(validationConfig.isUserOpValidation(), isUserOpValidation, "isUserOpValidation mismatch"); } function testFuzz_validationConfig_packingModuleEntity( ModuleEntity validationFunction, bool isGlobal, - bool isSignatureValidation + bool isSignatureValidation, + bool isUserOpValidation ) public { ValidationConfig validationConfig = - ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation); + ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation, isUserOpValidation); // Test unpacking underlying (address expectedModule, uint32 expectedEntityId) = validationFunction.unpack(); - (address module, uint32 entityId, bool isGlobal2, bool isSignatureValidation2) = - validationConfig.unpackUnderlying(); + (address module, uint32 entityId, uint8 flags2) = validationConfig.unpackUnderlying(); assertEq(expectedModule, module, "module mismatch"); assertEq(expectedEntityId, entityId, "entityId mismatch"); - assertEq(isGlobal, isGlobal2, "isGlobal mismatch"); - assertEq(isSignatureValidation, isSignatureValidation2, "isSignatureValidation mismatch"); + assertEq(isGlobal, flags2.isGlobal(), "isGlobal mismatch"); + assertEq(isSignatureValidation, flags2.isSignatureValidation(), "isSignatureValidation mismatch"); + assertEq(isUserOpValidation, flags2.isUserOpValidation(), "isUserOpValidation mismatch"); // Test unpacking to ModuleEntity - (ModuleEntity validationFunction2, bool isGlobal3, bool isSignatureValidation3) = validationConfig.unpack(); + (ModuleEntity validationFunction2, uint8 flags3) = validationConfig.unpack(); assertEq( ModuleEntity.unwrap(validationFunction), ModuleEntity.unwrap(validationFunction2), "validationFunction mismatch" ); - assertEq(isGlobal, isGlobal3, "isGlobal mismatch"); - assertEq(isSignatureValidation, isSignatureValidation3, "isSignatureValidation mismatch"); + assertEq(isGlobal, flags3.isGlobal(), "isGlobal mismatch"); + assertEq(isSignatureValidation, flags3.isSignatureValidation(), "isSignatureValidation mismatch"); + assertEq(isUserOpValidation, flags3.isUserOpValidation(), "isUserOpValidation mismatch"); // Test individual view functions @@ -100,5 +105,6 @@ contract ValidationConfigLibTest is Test { ); assertEq(validationConfig.isGlobal(), isGlobal, "isGlobal mismatch"); assertEq(validationConfig.isSignatureValidation(), isSignatureValidation, "isSignatureValidation mismatch"); + assertEq(validationConfig.isUserOpValidation(), isUserOpValidation, "isUserOpValidation mismatch"); } } diff --git a/test/mocks/SingleSignerFactoryFixture.sol b/test/mocks/SingleSignerFactoryFixture.sol index e5a68bfc..89dff51e 100644 --- a/test/mocks/SingleSignerFactoryFixture.sol +++ b/test/mocks/SingleSignerFactoryFixture.sol @@ -65,7 +65,7 @@ contract SingleSignerFactoryFixture is OptimizedTest { // point proxy to actual implementation and init modules ReferenceModularAccount(payable(addr)).initializeWithValidation( ValidationConfigLib.pack( - address(singleSignerValidationModule), TEST_DEFAULT_VALIDATION_ENTITY_ID, true, true + address(singleSignerValidationModule), TEST_DEFAULT_VALIDATION_ENTITY_ID, true, true, true ), new bytes4[](0), moduleInstallData, diff --git a/test/module/AllowlistModule.t.sol b/test/module/AllowlistModule.t.sol index 54e15718..c27787f4 100644 --- a/test/module/AllowlistModule.t.sol +++ b/test/module/AllowlistModule.t.sol @@ -332,7 +332,7 @@ contract AllowlistModuleTest is CustomValidationTestBase { internal virtual override - returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes[] memory) + returns (ModuleEntity, bool, bool, bool, bytes4[] memory, bytes memory, bytes[] memory) { bytes[] memory hooks = new bytes[](1); hooks[0] = abi.encodePacked( @@ -341,7 +341,8 @@ contract AllowlistModuleTest is CustomValidationTestBase { ); // patched to also work during SMA tests by differentiating the validation _signerValidation = ModuleEntityLib.pack(address(singleSignerValidationModule), type(uint32).max - 1); - return (_signerValidation, true, true, new bytes4[](0), abi.encode(type(uint32).max - 1, owner1), hooks); + return + (_signerValidation, true, true, true, new bytes4[](0), abi.encode(type(uint32).max - 1, owner1), hooks); } // Unfortunately, this is a feature that solidity has only implemented in via-ir, so we need to do it manually diff --git a/test/module/ERC20TokenLimitModule.t.sol b/test/module/ERC20TokenLimitModule.t.sol index fccd0d0f..2be4fd0e 100644 --- a/test/module/ERC20TokenLimitModule.t.sol +++ b/test/module/ERC20TokenLimitModule.t.sol @@ -53,7 +53,7 @@ contract ERC20TokenLimitModuleTest is AccountTestBase { vm.prank(address(acct)); acct.installValidation( - ValidationConfigLib.pack(address(validationModule), 0, true, true), new bytes4[](0), "", hooks + ValidationConfigLib.pack(address(validationModule), 0, true, true, true), new bytes4[](0), "", hooks ); validationFunction = ModuleEntityLib.pack(address(validationModule), 0); diff --git a/test/module/NativeTokenLimitModule.t.sol b/test/module/NativeTokenLimitModule.t.sol index 7d80deaf..5815749c 100644 --- a/test/module/NativeTokenLimitModule.t.sol +++ b/test/module/NativeTokenLimitModule.t.sol @@ -52,7 +52,7 @@ contract NativeTokenLimitModuleTest is AccountTestBase { vm.prank(address(acct)); acct.installValidation( - ValidationConfigLib.pack(address(validationModule), 0, true, true), + ValidationConfigLib.pack(address(validationModule), 0, true, true, true), new bytes4[](0), new bytes(0), hooks diff --git a/test/module/SingleSignerValidationModule.t.sol b/test/module/SingleSignerValidationModule.t.sol index a104b017..9b310854 100644 --- a/test/module/SingleSignerValidationModule.t.sol +++ b/test/module/SingleSignerValidationModule.t.sol @@ -91,7 +91,7 @@ contract SingleSignerValidationModuleTest is AccountTestBase { vm.expectEmit(true, true, true, true); emit ValidationInstalled(address(singleSignerValidationModule), newEntityId); account.installValidation( - ValidationConfigLib.pack(address(singleSignerValidationModule), newEntityId, true, false), + ValidationConfigLib.pack(address(singleSignerValidationModule), newEntityId, true, false, false), new bytes4[](0), abi.encode(newEntityId, owner2), new bytes[](0) diff --git a/test/utils/CustomValidationTestBase.sol b/test/utils/CustomValidationTestBase.sol index 7de02601..80470020 100644 --- a/test/utils/CustomValidationTestBase.sol +++ b/test/utils/CustomValidationTestBase.sol @@ -19,6 +19,7 @@ abstract contract CustomValidationTestBase is AccountTestBase { ModuleEntity validationFunction, bool isGlobal, bool isSignatureValidation, + bool isUserOpValidation, bytes4[] memory selectors, bytes memory installData, bytes[] memory hooks @@ -32,14 +33,14 @@ abstract contract CustomValidationTestBase is AccountTestBase { vm.prank(address(entryPoint)); // The initializer doesn't work on the SMA account1.installValidation( - ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation), + ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation, isUserOpValidation), selectors, installData, hooks ); } else { account1.initializeWithValidation( - ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation), + ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation, isUserOpValidation), selectors, installData, hooks @@ -56,6 +57,7 @@ abstract contract CustomValidationTestBase is AccountTestBase { ModuleEntity validationFunction, bool shared, bool isSignatureValidation, + bool isUserOpValidation, bytes4[] memory selectors, bytes memory installData, bytes[] memory hooks