diff --git a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap index 91d40b53..1cabdd6e 100644 --- a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -79509 \ No newline at end of file +79479 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap index e29e14ff..d5f32952 100644 --- a/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -113067 \ No newline at end of file +113037 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_BatchTransfers.snap b/.forge-snapshots/ModularAccount_UserOp_BatchTransfers.snap index f8918f32..531657a5 100644 --- a/.forge-snapshots/ModularAccount_UserOp_BatchTransfers.snap +++ b/.forge-snapshots/ModularAccount_UserOp_BatchTransfers.snap @@ -1 +1 @@ -198425 \ No newline at end of file +198437 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_Erc20Transfer.snap b/.forge-snapshots/ModularAccount_UserOp_Erc20Transfer.snap index 62aded4e..1c39110f 100644 --- a/.forge-snapshots/ModularAccount_UserOp_Erc20Transfer.snap +++ b/.forge-snapshots/ModularAccount_UserOp_Erc20Transfer.snap @@ -1 +1 @@ -185394 \ No newline at end of file +185406 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap b/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap index dd6810d9..4e34790c 100644 --- a/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -532140 \ No newline at end of file +532152 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_NativeTransfer.snap b/.forge-snapshots/ModularAccount_UserOp_NativeTransfer.snap index 436151f5..d60bdac3 100644 --- a/.forge-snapshots/ModularAccount_UserOp_NativeTransfer.snap +++ b/.forge-snapshots/ModularAccount_UserOp_NativeTransfer.snap @@ -1 +1 @@ -161675 \ No newline at end of file +161687 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap index a4f94866..96000a80 100644 --- a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -195886 \ No newline at end of file +195938 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap index 7b1c42ae..13ac764b 100644 --- a/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -227010 \ No newline at end of file +227062 \ No newline at end of file diff --git a/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap b/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap index 5899013f..e6aadc95 100644 --- a/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap +++ b/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap @@ -1 +1 @@ -261242 \ No newline at end of file +261264 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap index 535f549e..ab3bb827 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -79925 \ No newline at end of file +79895 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap index d61eab07..2a935d57 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -113483 \ No newline at end of file +113453 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap b/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap index 8b587a17..b12e3ecd 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap @@ -1 +1 @@ -193634 \ No newline at end of file +193639 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap b/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap index e216c182..ba360fee 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap @@ -1 +1 @@ -180730 \ No newline at end of file +180735 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap index a637222b..25d7e4dd 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -529720 \ No newline at end of file +529725 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap b/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap index f764a420..b10db97a 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap @@ -1 +1 @@ -157005 \ No newline at end of file +157010 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap index 7bf25d20..a58597c7 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -196127 \ No newline at end of file +196232 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap index dd92cd6e..f57c5ed5 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -227263 \ No newline at end of file +227368 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap index 145c4684..4bdce39f 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap @@ -1 +1 @@ -257938 \ No newline at end of file +257961 \ No newline at end of file diff --git a/package.json b/package.json index 29f8a28c..089c9a36 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "lint:gas": "solhint --max-warnings 0 -c ./config/solhint-gas.json './gas/**/*.sol'", "lint:script": "solhint --max-warnings 0 -c ./config/solhint-script.json './script/**/*.sol'", "prep": "pnpm fmt && forge b --deny-warnings && pnpm lint && pnpm test && pnpm gas", + "sizes": "FOUNDRY_PROFILE=optimized-build forge b --sizes | grep '^|' | grep -v -e '| 17 |' -e 'Lib'", "test": "forge test" } } diff --git a/src/account/ModularAccountBase.sol b/src/account/ModularAccountBase.sol index 87a2309f..0fd5a04d 100644 --- a/src/account/ModularAccountBase.sol +++ b/src/account/ModularAccountBase.sol @@ -91,21 +91,16 @@ abstract contract ModularAccountBase is event DeferredActionNonceInvalidated(uint256 nonce); - error PostExecHookReverted(address module, uint32 entityId, bytes revertReason); - error PreExecHookReverted(address module, uint32 entityId, bytes revertReason); - error PreRuntimeValidationHookFailed(address module, uint32 entityId, bytes revertReason); + error CreateFailed(); + error DeferredActionNonceInvalid(); + error DeferredActionSignatureInvalid(); error RequireUserOperationContext(); - 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 SignatureValidationInvalid(ModuleEntity validationFunction); + error UserOpValidationInvalid(ModuleEntity validationFunction); + error UnexpectedAggregator(ModuleEntity validationFunction, address aggregator); error UnrecognizedFunction(bytes4 selector); error ValidationFunctionMissing(bytes4 selector); - error DeferredActionNonceInvalid(); - error DeferredActionSignatureInvalid(); - // This error is trigged in performCreate and performCreate2 - error CreateFailed(); // Wraps execution of a native function with runtime validation and hooks // Used for upgradeTo, upgradeToAndCall, execute, executeBatch, installExecution, uninstallExecution, @@ -577,14 +572,14 @@ abstract contract ModularAccountBase is (currentSignatureSlice, signature) = signature.advanceSegmentIfAtIndex(uint8(preUserOpValidationHooks.length - i - 1)); - uint256 currentValidationRes = ExecutionLib.invokeUserOpCallBuffer( - userOpCallBuffer, preUserOpValidationHooks[i].moduleEntity(), currentSignatureSlice - ); + ModuleEntity uoValidationHook = preUserOpValidationHooks[i].moduleEntity(); + + uint256 currentValidationRes = + ExecutionLib.invokeUserOpCallBuffer(userOpCallBuffer, uoValidationHook, currentSignatureSlice); if (uint160(currentValidationRes) > 1) { // If the aggregator is not 0 or 1, it is an unexpected value - (address module, uint32 entityId) = preUserOpValidationHooks[i].moduleEntity().unpack(); - revert UnexpectedAggregator(module, entityId, address(uint160(currentValidationRes))); + revert UnexpectedAggregator(uoValidationHook, address(uint160(currentValidationRes))); } validationRes = _coalescePreValidation(validationRes, currentValidationRes); } @@ -730,8 +725,7 @@ abstract contract ModularAccountBase is AccountStorage storage _storage = getAccountStorage(); if (!_storage.validationStorage[userOpValidationFunction].isUserOpValidation) { - (address module, uint32 entityId) = userOpValidationFunction.unpack(); - revert UserOpValidationInvalid(module, entityId); + revert UserOpValidationInvalid(userOpValidationFunction); } ExecutionLib.convertToValidationBuffer(callBuffer); @@ -789,8 +783,7 @@ abstract contract ModularAccountBase is AccountStorage storage _storage = getAccountStorage(); if (!_storage.validationStorage[sigValidation].isSignatureValidation) { - (address module, uint32 entityId) = sigValidation.unpack(); - revert SignatureValidationInvalid(module, entityId); + revert SignatureValidationInvalid(sigValidation); } if (ExecutionLib.invokeSignatureValidation(buffer, sigValidation, signatureSegment) == _1271_MAGIC_VALUE) { diff --git a/src/libraries/ExecutionLib.sol b/src/libraries/ExecutionLib.sol index 3b869856..f5b2b665 100644 --- a/src/libraries/ExecutionLib.sol +++ b/src/libraries/ExecutionLib.sol @@ -27,11 +27,14 @@ using HookConfigLib for HookConfig; // Functions are more readable in original order // solhint-disable ordering library ExecutionLib { - // Duplicate definition to make it easier to revert in the library. - error PostExecHookReverted(address module, uint32 entityId, bytes revertReason); - error PreExecHookReverted(address module, uint32 entityId, bytes revertReason); - error PreRuntimeValidationHookFailed(address module, uint32 entityId, bytes revertReason); - error RuntimeValidationFunctionReverted(address module, uint32 entityId, bytes revertReason); + error PostExecHookReverted(ModuleEntity moduleFunction, bytes revertReason); + error PreExecHookReverted(ModuleEntity moduleFunction, bytes revertReason); + error PreRuntimeValidationHookReverted(ModuleEntity moduleFunction, bytes revertReason); + error PreSignatureValidationHookReverted(ModuleEntity moduleFunction, bytes revertReason); + error PreUserOpValidationHookReverted(ModuleEntity moduleFunction, bytes revertReason); + error RuntimeValidationFunctionReverted(ModuleEntity moduleFunction, bytes revertReason); + error SignatureValidationFunctionReverted(ModuleEntity moduleFunction, bytes revertReason); + error UserOpValidationFunctionReverted(ModuleEntity moduleFunction, bytes revertReason); // Perform the following call, without capturing any return data. // If the call reverts, the revert message will be directly bubbled up. @@ -153,10 +156,14 @@ library ExecutionLib { ModuleEntity moduleEntity, bytes calldata signatureSegment ) internal returns (uint256 validationData) { + bool success; + address moduleAddress; + uint32 entityId; + assembly ("memory-safe") { // Load the module address and entity Id - let entityId := and(shr(64, moduleEntity), 0xffffffff) - let moduleAddress := shr(96, moduleEntity) + entityId := and(shr(64, moduleEntity), 0xffffffff) + moduleAddress := shr(96, moduleEntity) // Update the buffer with the entity Id mstore(add(buffer, 0x24), entityId) @@ -196,35 +203,51 @@ library ExecutionLib { actualCallLength := add(actualCallLength, and(add(signatureSegment.length, 0x1f), not(0x1f))) // Perform the call, reverting on failure or insufficient return data. - switch and( - gt(returndatasize(), 0x1f), - call( - // If gas is the leftmost item before the call, it *should* be placed immediately before the - // call opcode and be allowed in validation. - gas(), - moduleAddress, - /*value*/ - 0, - /*argOffset*/ - add(buffer, 0x20), // jump over 32 bytes for length - /*argSize*/ - actualCallLength, - /*retOffset*/ - 0, - /*retSize*/ - 0x20 + + success := + and( + gt(returndatasize(), 0x1f), + call( + // If gas is the leftmost item before the call, it *should* be placed immediately before the + // call opcode and be allowed in validation. + gas(), + moduleAddress, + /*value*/ + 0, + /*argOffset*/ + add(buffer, 0x20), // jump over 32 bytes for length + /*argSize*/ + actualCallLength, + /*retOffset*/ + 0, + /*retSize*/ + 0x20 + ) ) - ) - case 0 { - // Bubble up the revert if the call reverts. - let m := mload(0x40) - returndatacopy(m, 0, returndatasize()) - revert(m, returndatasize()) - } - default { - // Otherwise, we return the first word of the return data as the validation data + } + + if (success) { + assembly ("memory-safe") { + // If the call was successful, we return the first word of the return data as the validation data. validationData := mload(0) } + } else { + // Revert with the appropriate error type for the selector used. + + uint32 selectorUsed; + uint32 errorSelector; + + assembly ("memory-safe") { + selectorUsed := and(mload(add(buffer, 0x4)), 0xffffffff) + } + + if (selectorUsed == uint32(IValidationHookModule.preUserOpValidationHook.selector)) { + errorSelector = uint32(PreUserOpValidationHookReverted.selector); + } else { + errorSelector = uint32(UserOpValidationFunctionReverted.selector); + } + + _revertModuleFunction(errorSelector, moduleAddress, entityId); } } @@ -331,7 +354,7 @@ library ExecutionLib { } if (!success) { - revert PreRuntimeValidationHookFailed(moduleAddress, entityId, collectReturnData()); + _revertModuleFunction(uint32(PreRuntimeValidationHookReverted.selector), moduleAddress, entityId); } } @@ -432,7 +455,7 @@ library ExecutionLib { } if (!success) { - revert RuntimeValidationFunctionReverted(moduleAddress, entityId, collectReturnData()); + _revertModuleFunction(uint32(RuntimeValidationFunctionReverted.selector), moduleAddress, entityId); } } @@ -615,7 +638,7 @@ library ExecutionLib { } if (!success) { - revert PreExecHookReverted(moduleAddress, entityId, collectReturnData()); + _revertModuleFunction(uint32(PreExecHookReverted.selector), moduleAddress, entityId); } } @@ -808,7 +831,7 @@ library ExecutionLib { } if (!success) { - revert PostExecHookReverted(moduleAddress, entityId, collectReturnData()); + _revertModuleFunction(uint32(PostExecHookReverted.selector), moduleAddress, entityId); } } } @@ -844,10 +867,14 @@ library ExecutionLib { HookConfig hookEntity, bytes calldata signatureSegment ) internal view { + bool success; + address moduleAddress; + uint32 entityId; + assembly ("memory-safe") { // Load the module address and entity id - let entityId := and(shr(64, hookEntity), 0xffffffff) - let moduleAddress := shr(96, hookEntity) + entityId := and(shr(64, hookEntity), 0xffffffff) + moduleAddress := shr(96, hookEntity) // Update the buffer with the entity Id mstore(add(buffer, 0x44), entityId) @@ -873,7 +900,7 @@ library ExecutionLib { let actualCallLength := add(0xa4, and(add(signatureSegment.length, 0x1f), not(0x1f))) // Perform the call - let success := + success := staticcall( gas(), moduleAddress, @@ -886,13 +913,10 @@ library ExecutionLib { /*retSize*/ 0x20 ) + } - if iszero(success) { - // Bubble up the revert if the call reverts. - let m := mload(0x40) - returndatacopy(m, 0, returndatasize()) - revert(m, returndatasize()) - } + if (!success) { + _revertModuleFunction(uint32(PreSignatureValidationHookReverted.selector), moduleAddress, entityId); } } @@ -901,10 +925,14 @@ library ExecutionLib { ModuleEntity validationFunction, bytes calldata signatureSegment ) internal view returns (bytes4 result) { + bool success; + address moduleAddress; + uint32 entityId; + assembly ("memory-safe") { // Load the module address and entity id - let entityId := and(shr(64, validationFunction), 0xffffffff) - let moduleAddress := shr(96, validationFunction) + entityId := and(shr(64, validationFunction), 0xffffffff) + moduleAddress := shr(96, validationFunction) // Store the account in the `account` field. mstore(add(buffer, 0x24), address()) @@ -937,34 +965,34 @@ library ExecutionLib { let actualCallLength := add(0xc4, and(add(signatureSegment.length, 0x1f), not(0x1f))) // Perform the call - switch and( - gt(returndatasize(), 0x1f), - staticcall( - gas(), - moduleAddress, - /*argOffset*/ - add(buffer, 0x20), // jump over 32 bytes for length, and another 32 bytes for the account - /*argSize*/ - actualCallLength, - /*retOffset*/ - 0, - /*retSize*/ - 0x20 + success := + and( + gt(returndatasize(), 0x1f), + staticcall( + gas(), + moduleAddress, + /*argOffset*/ + add(buffer, 0x20), // jump over 32 bytes for length, and another 32 bytes for the account + /*argSize*/ + actualCallLength, + /*retOffset*/ + 0, + /*retSize*/ + 0x20 + ) ) - ) - case 0 { - // Bubble up the revert if the call reverts. - let m := mload(0x40) - returndatacopy(m, 0, returndatasize()) - revert(m, returndatasize()) - } - default { + } + + if (success) { + assembly ("memory-safe") { // Otherwise, we return the first word of the return data as the signature validation result result := mload(0) // If any of the lower 28 bytes are nonzero, it would be an abi decoding failure. if shl(32, result) { revert(0, 0) } } + } else { + _revertModuleFunction(uint32(SignatureValidationFunctionReverted.selector), moduleAddress, entityId); } } @@ -1029,6 +1057,37 @@ library ExecutionLib { return workingMemPtr; } + function _revertModuleFunction(uint32 errorSelector, address moduleAddress, uint32 entityId) private pure { + // All of the module function reverts have the same parameter layout: + // - module address + // - entity Id + // - revert data + + assembly ("memory-safe") { + let m := mload(0x40) + // Write in order of entityId -> address -> selector, to avoid masking or shifts. + mstore(add(m, 0x18), entityId) + mstore(add(m, 0x14), moduleAddress) + mstore(m, errorSelector) + mstore(add(m, 0x40), 0x40) // fixed offset for the revert data + mstore(add(m, 0x60), returndatasize()) + + if returndatasize() { + // Store a zero in the last word of the revert data, to do strict ABI-encoding for the error. + let roundedDownDataLength := and(returndatasize(), not(0x1f)) + mstore(add(m, add(0x80, roundedDownDataLength)), 0) + returndatacopy(add(m, 0x80), 0, returndatasize()) + } + + let roundedUpDataLength := and(add(returndatasize(), 0x1f), not(0x1f)) + + // 4 bytes for the selector, and 0x60 for the 3 words of fixed-size data. + let totalRevertDataLength := add(0x64, roundedUpDataLength) + + revert(add(m, 0x1c), totalRevertDataLength) + } + } + function _prepareRuntimeCallBufferPreValidationHooks(RTCallBuffer buffer) private pure { uint32 selector = uint32(IValidationHookModule.preRuntimeValidationHook.selector); assembly ("memory-safe") { diff --git a/test/account/GlobalValidationTest.t.sol b/test/account/GlobalValidationTest.t.sol index f160c9c8..d1ec6bef 100644 --- a/test/account/GlobalValidationTest.t.sol +++ b/test/account/GlobalValidationTest.t.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.26; import {ModuleEntityLib} from "@erc6900/reference-implementation/libraries/ModuleEntityLib.sol"; +import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; @@ -76,4 +77,42 @@ contract GlobalValidationTest is AccountTestBase { assertEq(ethRecipient.balance, 2 wei); } + + function test_globalValidation_failsOnSelectorApplicability() public withSMATest { + PackedUserOperation memory userOp = PackedUserOperation({ + sender: address(account2), + nonce: 0, + initCode: abi.encodePacked( + address(factory), abi.encodeCall(factory.createAccount, (owner2, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID)) + ), + callData: abi.encodeCall(ModularAccountBase.execute, (ethRecipient, 1 wei, "")), + accountGasLimits: _encodeGas(VERIFICATION_GAS_LIMIT, CALL_GAS_LIMIT), + preVerificationGas: 0, + gasFees: _encodeGas(1, 1), + paymasterAndData: "", + signature: "" + }); + + bytes32 userOpHash = entryPoint.getUserOpHash(userOp); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner2Key, userOpHash.toEthSignedMessageHash()); + // Use the wrong checking mode - SELECTOR_ASSOCIATED_VALIDATION + userOp.signature = _encodeSignature( + _signerValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v) + ); + + PackedUserOperation[] memory userOps = new PackedUserOperation[](1); + userOps[0] = userOp; + + vm.expectRevert( + abi.encodeWithSelector( + IEntryPoint.FailedOpWithRevert.selector, + 0, + "AA23 reverted", + abi.encodeWithSelector( + ModularAccountBase.ValidationFunctionMissing.selector, ModularAccountBase.execute.selector + ) + ) + ); + entryPoint.handleOps(userOps, beneficiary); + } } diff --git a/test/account/ModularAccount.t.sol b/test/account/ModularAccount.t.sol index 2ae8692f..f989fd2d 100644 --- a/test/account/ModularAccount.t.sol +++ b/test/account/ModularAccount.t.sol @@ -11,6 +11,7 @@ import {ValidationConfigLib} from "@erc6900/reference-implementation/libraries/V 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 {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; @@ -22,6 +23,8 @@ import {SemiModularAccountBytecode} from "../../src/account/SemiModularAccountBy import {SingleSignerValidationModule} from "../../src/modules/validation/SingleSignerValidationModule.sol"; import {Counter} from "../mocks/Counter.sol"; +import {MockInterface} from "../mocks/MockInterface.sol"; +import {MockRevertingConstructor} from "../mocks/MockRevertingConstructor.sol"; import {ComprehensiveModule} from "../mocks/modules/ComprehensiveModule.sol"; import {MockExecutionInstallationModule} from "../mocks/modules/MockExecutionInstallationModule.sol"; import {MockModule} from "../mocks/modules/MockModule.sol"; @@ -495,7 +498,8 @@ contract ModularAccountTest is AccountTestBase { vm.expectRevert( abi.encodeWithSelector( - ModularAccountBase.SignatureValidationInvalid.selector, singleSignerValidationModule, newEntityId + ModularAccountBase.SignatureValidationInvalid.selector, + ModuleEntityLib.pack(address(singleSignerValidationModule), newEntityId) ) ); IERC1271(address(account1)).isValidSignature(message, signature); @@ -542,7 +546,8 @@ contract ModularAccountTest is AccountTestBase { 0, "AA23 reverted", abi.encodeWithSelector( - ModularAccountBase.UserOpValidationInvalid.selector, singleSignerValidationModule, newEntityId + ModularAccountBase.UserOpValidationInvalid.selector, + ModuleEntityLib.pack(address(singleSignerValidationModule), newEntityId) ) ) ); @@ -570,6 +575,11 @@ contract ModularAccountTest is AccountTestBase { assertEq(returnedAddr, expectedAddr); assertEq(address(ModularAccount(payable(expectedAddr)).entryPoint()), address(entryPoint)); + + // Assert a reverting constructor causes a `CreateFailed` error + vm.prank(address(entryPoint)); + vm.expectRevert(ModularAccountBase.CreateFailed.selector); + account1.performCreate(0, abi.encodePacked(type(MockRevertingConstructor).creationCode)); } function test_performCreate2() public withSMATest { @@ -604,6 +614,74 @@ contract ModularAccountTest is AccountTestBase { account1.executeUserOp(userOp, bytes32(0)); } + function test_revertOnNoInstalledModuleExecFunction() public withSMATest { + bytes4 nonInstalledSelector = MockInterface.doSomething.selector; + + vm.prank(beneficiary); + vm.expectRevert( + abi.encodeWithSelector(ModularAccountBase.UnrecognizedFunction.selector, nonInstalledSelector) + ); + MockInterface(address(account1)).doSomething(); + } + + function test_modularAccountBase_supportsInterface() public withSMATest { + // Assert that the invalid interface ID is not supported + assertFalse(account1.supportsInterface(0xffffffff)); + + // Assert that the ERC165 interface ID is supported + assertTrue(account1.supportsInterface(type(IERC165).interfaceId)); + + // Assert that the interfaceId for a new interface starts out unsupported, but becomes supported after + // install. + assertFalse(account1.supportsInterface(type(MockInterface).interfaceId)); + + // Install the interface id. + ExecutionManifest memory manifest; + manifest.interfaceIds = new bytes4[](1); + manifest.interfaceIds[0] = type(MockInterface).interfaceId; + + vm.prank(address(entryPoint)); + account1.installExecution(makeAddr("mockModule"), manifest, ""); + + assertTrue(account1.supportsInterface(type(MockInterface).interfaceId)); + } + + function test_validationRevertsOnShortCalldata() public withSMATest { + PackedUserOperation memory userOp = PackedUserOperation({ + sender: address(account1), + nonce: 0, + initCode: "", + callData: abi.encodePacked(bytes3(ModularAccountBase.execute.selector)), // Short calldata + 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(_signerValidation, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v)); + + PackedUserOperation[] memory userOps = new PackedUserOperation[](1); + userOps[0] = userOp; + + vm.expectRevert( + abi.encodeWithSelector( + IEntryPoint.FailedOpWithRevert.selector, + 0, + "AA23 reverted", + abi.encodeWithSelector( + ModularAccountBase.UnrecognizedFunction.selector, + bytes4(bytes3(ModularAccountBase.execute.selector)) + ) + ) + ); + entryPoint.handleOps(userOps, beneficiary); + } + // Internal Functions function _printStorageReadsAndWrites(address addr) internal { diff --git a/test/account/MultiValidation.t.sol b/test/account/MultiValidation.t.sol index b76dc69e..e548aa92 100644 --- a/test/account/MultiValidation.t.sol +++ b/test/account/MultiValidation.t.sol @@ -10,6 +10,7 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; +import {ExecutionLib} from "../../src/libraries/ExecutionLib.sol"; import {SingleSignerValidationModule} from "../../src/modules/validation/SingleSignerValidationModule.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; @@ -59,9 +60,8 @@ contract MultiValidationTest is AccountTestBase { vm.prank(owner1); vm.expectRevert( abi.encodeWithSelector( - ModularAccountBase.RuntimeValidationFunctionReverted.selector, - address(validator2), - TEST_DEFAULT_VALIDATION_ENTITY_ID, + ExecutionLib.RuntimeValidationFunctionReverted.selector, + ModuleEntityLib.pack(address(validator2), TEST_DEFAULT_VALIDATION_ENTITY_ID), abi.encodeWithSignature("NotAuthorized()") ) ); diff --git a/test/account/PerHookData.t.sol b/test/account/PerHookData.t.sol index aa78efdb..1fda147f 100644 --- a/test/account/PerHookData.t.sol +++ b/test/account/PerHookData.t.sol @@ -11,6 +11,7 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; +import {ExecutionLib} from "../../src/libraries/ExecutionLib.sol"; import {Counter} from "../mocks/Counter.sol"; import {MockAccessControlHookModule} from "../mocks/modules/MockAccessControlHookModule.sol"; @@ -84,7 +85,11 @@ contract PerHookDataTest is CustomValidationTestBase { IEntryPoint.FailedOpWithRevert.selector, 0, "AA23 reverted", - abi.encodeWithSignature("Error(string)", "Proof doesn't match target") + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(_accessControlHookModule), _PRE_HOOK_ENTITY_ID_1), + abi.encodeWithSignature("Error(string)", "Proof doesn't match target") + ) ) ); entryPoint.handleOps(userOps, beneficiary); @@ -104,7 +109,11 @@ contract PerHookDataTest is CustomValidationTestBase { IEntryPoint.FailedOpWithRevert.selector, 0, "AA23 reverted", - abi.encodeWithSignature("Error(string)", "Proof doesn't match target") + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(_accessControlHookModule), _PRE_HOOK_ENTITY_ID_1), + abi.encodeWithSignature("Error(string)", "Proof doesn't match target") + ) ) ); entryPoint.handleOps(userOps, beneficiary); @@ -223,7 +232,11 @@ contract PerHookDataTest is CustomValidationTestBase { IEntryPoint.FailedOpWithRevert.selector, 0, "AA23 reverted", - abi.encodeWithSignature("Error(string)", "Target not allowed") + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(_accessControlHookModule), _PRE_HOOK_ENTITY_ID_1), + abi.encodeWithSignature("Error(string)", "Target not allowed") + ) ) ); entryPoint.handleOps(userOps, beneficiary); @@ -281,9 +294,8 @@ contract PerHookDataTest is CustomValidationTestBase { vm.prank(owner1); vm.expectRevert( abi.encodeWithSelector( - ModularAccountBase.PreRuntimeValidationHookFailed.selector, - _accessControlHookModule, - _PRE_HOOK_ENTITY_ID_1, + ExecutionLib.PreRuntimeValidationHookReverted.selector, + ModuleEntityLib.pack(address(_accessControlHookModule), _PRE_HOOK_ENTITY_ID_1), abi.encodeWithSignature("Error(string)", "Proof doesn't match target") ) ); @@ -299,9 +311,8 @@ contract PerHookDataTest is CustomValidationTestBase { vm.prank(owner1); vm.expectRevert( abi.encodeWithSelector( - ModularAccountBase.PreRuntimeValidationHookFailed.selector, - _accessControlHookModule, - _PRE_HOOK_ENTITY_ID_1, + ExecutionLib.PreRuntimeValidationHookReverted.selector, + ModuleEntityLib.pack(address(_accessControlHookModule), _PRE_HOOK_ENTITY_ID_1), abi.encodeWithSignature("Error(string)", "Proof doesn't match target") ) ); @@ -374,9 +385,8 @@ contract PerHookDataTest is CustomValidationTestBase { vm.prank(owner1); vm.expectRevert( abi.encodeWithSelector( - ModularAccountBase.PreRuntimeValidationHookFailed.selector, - _accessControlHookModule, - _PRE_HOOK_ENTITY_ID_1, + ExecutionLib.PreRuntimeValidationHookReverted.selector, + ModuleEntityLib.pack(address(_accessControlHookModule), _PRE_HOOK_ENTITY_ID_1), abi.encodeWithSignature("Error(string)", "Target not allowed") ) ); @@ -449,7 +459,13 @@ contract PerHookDataTest is CustomValidationTestBase { validationData: abi.encodePacked(address(0x1234123412341234123412341234123412341234)) }); - vm.expectRevert("Preimage not provided"); + vm.expectRevert( + abi.encodeWithSelector( + ExecutionLib.PreSignatureValidationHookReverted.selector, + ModuleEntityLib.pack(address(_accessControlHookModule), _PRE_HOOK_ENTITY_ID_1), + abi.encodeWithSignature("Error(string)", "Preimage not provided") + ) + ); account1.isValidSignature( messageHash, _encode1271Signature( @@ -468,7 +484,13 @@ contract PerHookDataTest is CustomValidationTestBase { (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, messageHash); - vm.expectRevert("Preimage not provided"); + vm.expectRevert( + abi.encodeWithSelector( + ExecutionLib.PreSignatureValidationHookReverted.selector, + ModuleEntityLib.pack(address(_accessControlHookModule), _PRE_HOOK_ENTITY_ID_1), + abi.encodeWithSignature("Error(string)", "Preimage not provided") + ) + ); account1.isValidSignature( messageHash, _encode1271Signature(_signerValidation, abi.encodePacked(r, s, v), messageHash) ); diff --git a/test/account/UOCallBuffer.t.sol b/test/account/UOCallBuffer.t.sol index 6ac7deb7..f5dee341 100644 --- a/test/account/UOCallBuffer.t.sol +++ b/test/account/UOCallBuffer.t.sol @@ -4,12 +4,12 @@ pragma solidity ^0.8.26; import {ExecutionManifest} from "@erc6900/reference-implementation/interfaces/IExecutionModule.sol"; import {IValidationHookModule} from "@erc6900/reference-implementation/interfaces/IValidationHookModule.sol"; import {IValidationModule} from "@erc6900/reference-implementation/interfaces/IValidationModule.sol"; - import {HookConfigLib} from "@erc6900/reference-implementation/libraries/HookConfigLib.sol"; import {ModuleEntity, ModuleEntityLib} from "@erc6900/reference-implementation/libraries/ModuleEntityLib.sol"; import {ValidationConfigLib} from "@erc6900/reference-implementation/libraries/ValidationConfigLib.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; +import {ExecutionLib} from "../../src/libraries/ExecutionLib.sol"; import {MockModule} from "../mocks/modules/MockModule.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; @@ -227,7 +227,13 @@ contract UOCallBufferTest is AccountTestBase { ); vm.prank(address(entryPoint)); - vm.expectRevert(bytes(hex"abcdabcd")); + vm.expectRevert( + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(validationHooks[0]), 0), + bytes(hex"abcdabcd") + ) + ); account1.validateUserOp(userOp, userOpHash, 1 wei); vm.clearMockedCalls(); diff --git a/test/account/ValidationAssocHooks.t.sol b/test/account/ValidationAssocHooks.t.sol index 41d2995b..dae37a37 100644 --- a/test/account/ValidationAssocHooks.t.sol +++ b/test/account/ValidationAssocHooks.t.sol @@ -4,7 +4,9 @@ pragma solidity ^0.8.26; import {ExecutionManifest} from "@erc6900/reference-implementation/interfaces/IExecutionModule.sol"; import {HookConfigLib} from "@erc6900/reference-implementation/libraries/HookConfigLib.sol"; import {ValidationConfigLib} from "@erc6900/reference-implementation/libraries/ValidationConfigLib.sol"; +import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol"; +import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; import {ModuleManagerInternals} from "../../src/account/ModuleManagerInternals.sol"; import {MockModule} from "../mocks/modules/MockModule.sol"; @@ -23,7 +25,7 @@ contract ValidationAssocHooksTest is AccountTestBase { } } - function test_validationAssocHooks_maxValidationHooks() public { + function test_validationAssocHooks_maxValidationHooks() public withSMATest { // Attempt to install 257 validation hooks, expect a revert. bytes[] memory hookInstalls = new bytes[](257); @@ -48,7 +50,7 @@ contract ValidationAssocHooksTest is AccountTestBase { ); } - function test_validationAssocHooks_maxExecHooks() public { + function test_validationAssocHooks_maxExecHooks() public withSMATest { // Attempt to install 257 exec hooks, expect a revert. bytes[] memory hookInstalls = new bytes[](257); @@ -77,4 +79,40 @@ contract ValidationAssocHooksTest is AccountTestBase { hookInstalls ); } + + function test_revertOnMissingExecuteUserOp() public withSMATest { + // install a validation-association execution hook, and expect a revert unless called via `executeUserOp`. + + ExecutionManifest memory m; // empty manifest + + hooks.push(new MockModule(m)); + + bytes[] memory hookInstalls = new bytes[](1); + hookInstalls[0] = abi.encodePacked( + HookConfigLib.packExecHook({_module: address(hooks[0]), _entityId: 0, _hasPre: false, _hasPost: false}) + ); + + account1.installValidation( + ValidationConfigLib.pack({ + _validationFunction: _signerValidation, + _isGlobal: true, + _isSignatureValidation: true, + _isUserOpValidation: true + }), + new bytes4[](0), + "", + hookInstalls + ); + + _runExecUserOp( + makeAddr("target"), + "", + abi.encodeWithSelector( + IEntryPoint.FailedOpWithRevert.selector, + 0, + "AA23 reverted", + abi.encodeWithSelector(ModularAccountBase.RequireUserOperationContext.selector) + ) + ); + } } diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index 65da007e..dc780cd6 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -204,8 +204,9 @@ contract ValidationIntersectionTest is AccountTestBase { vm.expectRevert( abi.encodeWithSelector( ModularAccountBase.UnexpectedAggregator.selector, - address(oneHookModule), - MockBaseUserOpValidationModule.EntityId.PRE_VALIDATION_HOOK_1, + ModuleEntityLib.pack( + address(oneHookModule), uint32(MockBaseUserOpValidationModule.EntityId.PRE_VALIDATION_HOOK_1) + ), badAuthorizer ) ); diff --git a/test/mocks/MockInterface.sol b/test/mocks/MockInterface.sol new file mode 100644 index 00000000..3df7beff --- /dev/null +++ b/test/mocks/MockInterface.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.26; + +interface MockInterface { + function doSomething() external; +} diff --git a/test/mocks/MockRevertingConstructor.sol b/test/mocks/MockRevertingConstructor.sol new file mode 100644 index 00000000..99b8d1ae --- /dev/null +++ b/test/mocks/MockRevertingConstructor.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.26; + +contract MockRevertingConstructor { + constructor() { + revert("Constructor reverts"); + } +} diff --git a/test/modules/AllowlistModule.t.sol b/test/modules/AllowlistModule.t.sol index de9842ab..5afee202 100644 --- a/test/modules/AllowlistModule.t.sol +++ b/test/modules/AllowlistModule.t.sol @@ -10,6 +10,8 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; + +import {ExecutionLib} from "../../src/libraries/ExecutionLib.sol"; import {BaseModule} from "../../src/modules/BaseModule.sol"; import {AllowlistModule} from "../../src/modules/permissions/AllowlistModule.sol"; @@ -314,7 +316,11 @@ contract AllowlistModuleTest is CustomValidationTestBase { IEntryPoint.FailedOpWithRevert.selector, 0, "AA23 reverted", - abi.encodeWithSelector(BaseModule.UnexpectedDataPassed.selector) + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(allowlistModule), HOOK_ENTITY_ID), + abi.encodeWithSelector(BaseModule.UnexpectedDataPassed.selector) + ) ) ); entryPoint.handleOps(userOps, beneficiary); @@ -411,7 +417,11 @@ contract AllowlistModuleTest is CustomValidationTestBase { IEntryPoint.FailedOpWithRevert.selector, 0, "AA23 reverted", - abi.encodeWithSelector(AllowlistModule.SelectorNotAllowed.selector) + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(allowlistModule), HOOK_ENTITY_ID), + abi.encodeWithSelector(AllowlistModule.SelectorNotAllowed.selector) + ) ); } } else { @@ -419,7 +429,11 @@ contract AllowlistModuleTest is CustomValidationTestBase { IEntryPoint.FailedOpWithRevert.selector, 0, "AA23 reverted", - abi.encodeWithSelector(AllowlistModule.AddressNotAllowed.selector) + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(allowlistModule), HOOK_ENTITY_ID), + abi.encodeWithSelector(AllowlistModule.AddressNotAllowed.selector) + ) ); } } @@ -441,17 +455,15 @@ contract AllowlistModuleTest is CustomValidationTestBase { ) ) { return abi.encodeWithSelector( - ModularAccountBase.PreRuntimeValidationHookFailed.selector, - address(allowlistModule), - HOOK_ENTITY_ID, + ExecutionLib.PreRuntimeValidationHookReverted.selector, + ModuleEntityLib.pack(address(allowlistModule), HOOK_ENTITY_ID), abi.encodeWithSelector(AllowlistModule.SelectorNotAllowed.selector) ); } } else { return abi.encodeWithSelector( - ModularAccountBase.PreRuntimeValidationHookFailed.selector, - address(allowlistModule), - HOOK_ENTITY_ID, + ExecutionLib.PreRuntimeValidationHookReverted.selector, + ModuleEntityLib.pack(address(allowlistModule), HOOK_ENTITY_ID), abi.encodeWithSelector(AllowlistModule.AddressNotAllowed.selector) ); } diff --git a/test/modules/NativeTokenLimitModule.t.sol b/test/modules/NativeTokenLimitModule.t.sol index b6fc4be2..25786b8a 100644 --- a/test/modules/NativeTokenLimitModule.t.sol +++ b/test/modules/NativeTokenLimitModule.t.sol @@ -13,6 +13,7 @@ import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntry import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; +import {ExecutionLib} from "../../src/libraries/ExecutionLib.sol"; import {BaseModule} from "../../src/modules/BaseModule.sol"; import {NativeTokenLimitModule} from "../../src/modules/permissions/NativeTokenLimitModule.sol"; @@ -107,7 +108,13 @@ contract NativeTokenLimitModuleTest is AccountTestBase { assertEq(result, expected); // uses 200k + 1 wei of gas - vm.expectRevert(NativeTokenLimitModule.ExceededNativeTokenLimit.selector); + vm.expectRevert( + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(module), entityId), + abi.encodeWithSelector(NativeTokenLimitModule.ExceededNativeTokenLimit.selector) + ) + ); result = account1.validateUserOp(_getPackedUO(100_000, 100_000, 1, 1, _getExecuteWithValue(0)), bytes32(0), 0); @@ -124,13 +131,10 @@ contract NativeTokenLimitModuleTest is AccountTestBase { // uses 5e + 1wei of native tokens vm.expectRevert( - abi.encodePacked( - ModularAccountBase.PreExecHookReverted.selector, - abi.encode( - address(module), - uint32(0), - abi.encodePacked(NativeTokenLimitModule.ExceededNativeTokenLimit.selector) - ) + abi.encodeWithSelector( + ExecutionLib.PreExecHookReverted.selector, + ModuleEntityLib.pack(address(module), uint32(0)), + abi.encodePacked(NativeTokenLimitModule.ExceededNativeTokenLimit.selector) ) ); account1.executeUserOp(_getPackedUO(0, 0, 0, 0, _getExecuteWithValue(5 ether + 1)), bytes32(0)); @@ -286,7 +290,11 @@ contract NativeTokenLimitModuleTest is AccountTestBase { IEntryPoint.FailedOpWithRevert.selector, 0, "AA23 reverted", - abi.encodeWithSelector(BaseModule.UnexpectedDataPassed.selector) + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(module), entityId), + abi.encodeWithSelector(BaseModule.UnexpectedDataPassed.selector) + ) ) ); entryPoint.handleOps(uos, beneficiary); diff --git a/test/modules/PaymasterGuardModule.t.sol b/test/modules/PaymasterGuardModule.t.sol index 50ae30a3..1e84fdf6 100644 --- a/test/modules/PaymasterGuardModule.t.sol +++ b/test/modules/PaymasterGuardModule.t.sol @@ -10,6 +10,7 @@ import {ModuleEntityLib} from "@erc6900/reference-implementation/libraries/Modul import {ValidationConfigLib} from "@erc6900/reference-implementation/libraries/ValidationConfigLib.sol"; import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; +import {ExecutionLib} from "../../src/libraries/ExecutionLib.sol"; import {BaseModule} from "../../src/modules/BaseModule.sol"; import {PaymasterGuardModule} from "../../src/modules/permissions/PaymasterGuardModule.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; @@ -179,7 +180,13 @@ contract PaymasterGuardModuleTest is AccountTestBase { abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v) ); - vm.expectRevert(abi.encodeWithSelector(PaymasterGuardModule.BadPaymasterSpecified.selector)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(module), ENTITY_ID), + abi.encodeWithSelector(PaymasterGuardModule.BadPaymasterSpecified.selector) + ) + ); vm.prank(address(entryPoint)); account1.validateUserOp(userOp, userOpHash, 0); } diff --git a/test/modules/TimeRangeModule.t.sol b/test/modules/TimeRangeModule.t.sol index 539d16e5..d17ddb94 100644 --- a/test/modules/TimeRangeModule.t.sol +++ b/test/modules/TimeRangeModule.t.sol @@ -12,6 +12,7 @@ import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interface import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; +import {ExecutionLib} from "../../src/libraries/ExecutionLib.sol"; import {BaseModule} from "../../src/modules/BaseModule.sol"; import {TimeRangeModule} from "../../src/modules/permissions/TimeRangeModule.sol"; @@ -168,9 +169,8 @@ contract TimeRangeModuleTest is CustomValidationTestBase { vm.expectRevert( abi.encodeWithSelector( - ModularAccountBase.PreRuntimeValidationHookFailed.selector, - timeRangeModule, - HOOK_ENTITY_ID, + ExecutionLib.PreRuntimeValidationHookReverted.selector, + ModuleEntityLib.pack(address(timeRangeModule), HOOK_ENTITY_ID), abi.encodeWithSelector(TimeRangeModule.TimeRangeNotValid.selector) ) ); @@ -209,9 +209,8 @@ contract TimeRangeModuleTest is CustomValidationTestBase { vm.expectRevert( abi.encodeWithSelector( - ModularAccountBase.PreRuntimeValidationHookFailed.selector, - timeRangeModule, - HOOK_ENTITY_ID, + ExecutionLib.PreRuntimeValidationHookReverted.selector, + ModuleEntityLib.pack(address(timeRangeModule), HOOK_ENTITY_ID), abi.encodeWithSelector(TimeRangeModule.TimeRangeNotValid.selector) ) ); @@ -249,7 +248,13 @@ contract TimeRangeModuleTest is CustomValidationTestBase { userOp.signature = _encodeSignature(_signerValidation, GLOBAL_VALIDATION, preValidationHookData, ""); vm.prank(address(entryPoint)); - vm.expectRevert(abi.encodeWithSelector(BaseModule.UnexpectedDataPassed.selector)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutionLib.PreUserOpValidationHookReverted.selector, + ModuleEntityLib.pack(address(timeRangeModule), HOOK_ENTITY_ID), + abi.encodeWithSelector(BaseModule.UnexpectedDataPassed.selector) + ) + ); account1.validateUserOp(userOp, userOpHash, 0); }