diff --git a/src/test/ModuleKitHelpers.sol b/src/test/ModuleKitHelpers.sol index f49e091e..af188a29 100644 --- a/src/test/ModuleKitHelpers.sol +++ b/src/test/ModuleKitHelpers.sol @@ -170,7 +170,6 @@ library ModuleKitHelpers { address module ) internal - view returns (bool) { return HelperBase(instance.accountHelper).isModuleInstalled(instance, moduleTypeId, module); @@ -183,7 +182,6 @@ library ModuleKitHelpers { bytes memory data ) internal - view returns (bool) { return HelperBase(instance.accountHelper).isModuleInstalled( diff --git a/src/test/helpers/KernelHelpers.sol b/src/test/helpers/KernelHelpers.sol index a837df23..42531e2b 100644 --- a/src/test/helpers/KernelHelpers.sol +++ b/src/test/helpers/KernelHelpers.sol @@ -394,7 +394,6 @@ contract KernelHelpers is HelperBase { bytes memory data ) public - view virtual override deployAccountForAction(instance) diff --git a/src/test/helpers/SafeHelpers.sol b/src/test/helpers/SafeHelpers.sol index 6a0eeeec..8e485f0e 100644 --- a/src/test/helpers/SafeHelpers.sol +++ b/src/test/helpers/SafeHelpers.sol @@ -245,7 +245,6 @@ contract SafeHelpers is HelperBase { bytes memory data ) public - view virtual override deployAccountForAction(instance) diff --git a/src/test/utils/ERC4337Helpers.sol b/src/test/utils/ERC4337Helpers.sol index af5d12d3..2dcd0746 100644 --- a/src/test/utils/ERC4337Helpers.sol +++ b/src/test/utils/ERC4337Helpers.sol @@ -26,8 +26,6 @@ import { InstalledModule } from "./Storage.sol"; -import "forge-std/console2.sol"; - library ERC4337Helpers { using Simulator for PackedUserOperation; @@ -75,15 +73,15 @@ library ERC4337Helpers { abi.decode(logs[i].data, (uint256, bool, uint256, uint256)); totalUserOpGas = actualGasUsed; if (!userOpSuccess) { + bytes32 userOpHash = logs[i].topics[1]; if (isExpectRevert == 0) { - bytes32 userOpHash = logs[i].topics[1]; bytes memory revertReason = getUserOpRevertReason(logs, userOpHash); revert UserOperationReverted( userOpHash, address(bytes20(logs[i].topics[2])), nonce, revertReason ); } else { if (isExpectRevert == 2) { - checkRevertMessage(getUserOpRevertReason(logs, bytes32(0))); + checkRevertMessage(getUserOpRevertReason(logs, userOpHash)); } clearExpectRevert(); } @@ -149,7 +147,7 @@ library ERC4337Helpers { function getUserOpRevertReason( VmSafe.Log[] memory logs, - bytes32 /* userOpHash */ + bytes32 userOpHash ) internal pure @@ -160,6 +158,7 @@ library ERC4337Helpers { if ( logs[i].topics[0] == 0x1c4fada7374c0a9ee8841fc38afe82932dc0f8e69012e927f061a8bae611a201 + && logs[i].topics[1] == userOpHash ) { (, revertReason) = abi.decode(logs[i].data, (uint256, bytes)); } @@ -168,8 +167,6 @@ library ERC4337Helpers { function checkRevertMessage(bytes memory actualReason) internal view { bytes memory revertMessage = getExpectRevertMessage(); - console2.logBytes(revertMessage); - console2.logBytes(actualReason); if (revertMessage.length == 4) { bytes4 expected = bytes4(revertMessage); diff --git a/test/Diff.t.sol b/test/Diff.t.sol index f667a857..3047a41b 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -13,12 +13,14 @@ import { } from "src/external/ERC7579.sol"; import { getAccountType, InstalledModule } from "src/test/utils/Storage.sol"; import { toString } from "src/test/utils/Vm.sol"; +import { MockValidatorFalse } from "test/mocks/MockValidatorFalse.sol"; contract ERC7579DifferentialModuleKitLibTest is BaseTest { using ModuleKitHelpers for *; using ModuleKitUserOp for *; MockValidator internal validator; + MockValidatorFalse internal validatorFalse; MockExecutor internal executor; MockFallback internal fallbackHandler; MockHook internal hook; @@ -33,6 +35,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { // Setup modules validator = new MockValidator(); + validatorFalse = new MockValidatorFalse(); hook = new MockHook(); executor = new MockExecutor(); fallbackHandler = new MockFallback(); @@ -107,13 +110,11 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { _revertWhen__ValidationFails(""); // Revert selector - _revertWhen__ValidationReverts(abi.encodePacked(bytes4(0x65c8fd4d))); + _revertWhen__ValidationFails(abi.encodePacked(bytes4(0x220266b6))); // Revert message - _revertWhen__ValidationReverts( - abi.encodeWithSignature( - "FailedOpWithRevert(uint256,string,bytes)", 0, "AA23 reverted", "" - ) + _revertWhen__ValidationFails( + abi.encodeWithSignature("FailedOp(uint256,string)", 0, "AA24 signature error") ); } @@ -122,22 +123,55 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { _revertWhen__ValidationReverts(""); // Revert selector - _revertWhen__ValidationReverts(abi.encodePacked(bytes4(hex"ffffffff"))); + _revertWhen__ValidationReverts(abi.encodePacked(bytes4(0x65c8fd4d))); // Revert message + bytes memory revertMessage; + + AccountType env = ModuleKitHelpers.getAccountType(); + if (env == AccountType.SAFE) { + revertMessage = abi.encodeWithSignature( + "FailedOpWithRevert(uint256,string,bytes)", + 0, + "AA23 reverted", + abi.encode(bytes4(0xacfdb444)) + ); + } else { + revertMessage = abi.encodeWithSignature( + "FailedOpWithRevert(uint256,string,bytes)", 0, "AA23 reverted", "" + ); + } + + _revertWhen__ValidationReverts(revertMessage); } function testexec__RevertWhen__UserOperationFails() public { + // Deploy the account first + testexec__Given__TwoInputs(); + // No revert reason _revertWhen__UserOperationFails(""); + bytes memory revertSelector; + bytes memory revertMessage; + + AccountType env = ModuleKitHelpers.getAccountType(); + if (env == AccountType.SAFE) { + revertSelector = abi.encodePacked(bytes4(0xacfdb444)); + revertMessage = abi.encodePacked(bytes4(0xacfdb444)); + } else if (env == AccountType.KERNEL) { + revertSelector = abi.encodePacked(bytes4(0xf21e646b)); + revertMessage = abi.encodePacked(bytes4(0xf21e646b)); + } else { + revertSelector = abi.encodePacked(bytes4(0x08c379a0)); + revertMessage = abi.encodeWithSignature("Error(string)", "MockTarget: not authorized"); + } + // Revert selector - _revertWhen__UserOperationFails(abi.encodePacked(bytes4(0x08c379a0))); + _revertWhen__UserOperationFails(revertSelector); // Revert message - _revertWhen__UserOperationFails( - abi.encodeWithSignature("Error(string)", "MockTarget: not authorized") - ); + _revertWhen__UserOperationFails(revertMessage); } /*////////////////////////////////////////////////////////////////////////// @@ -602,6 +636,14 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { uint256 value = 10 gwei; bytes memory callData = ""; + if (!instance.isModuleInstalled(MODULE_TYPE_VALIDATOR, address(validatorFalse))) { + instance.installModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: address(validatorFalse), + data: "" + }); + } + // Expect the revert if (revertReason.length == 0) { instance.expect4337Revert(); @@ -616,21 +658,24 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { target: receiver, value: value, callData: callData, - txValidator: makeAddr("invalidValidator") + txValidator: address(validatorFalse) }).execUserOps(); } function _revertWhen__ValidationReverts(bytes memory revertReason) public { address revertingValidator = makeAddr("revertingValidator"); - vm.etch(revertingValidator, address(validator).code); - instance.installModule({ - moduleTypeId: MODULE_TYPE_VALIDATOR, - module: revertingValidator, - data: "" - }); + if (!instance.isModuleInstalled(MODULE_TYPE_VALIDATOR, revertingValidator)) { + vm.etch(revertingValidator, address(validator).code); + + instance.installModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: revertingValidator, + data: "" + }); - vm.etch(revertingValidator, hex"fd"); + vm.etch(revertingValidator, hex"fd"); + } // Create userOperation fields address receiver = makeAddr("receiver"); diff --git a/test/mocks/MockValidatorFalse.sol b/test/mocks/MockValidatorFalse.sol new file mode 100644 index 00000000..0b032c64 --- /dev/null +++ b/test/mocks/MockValidatorFalse.sol @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.23; + +/* solhint-disable no-unused-vars */ +import { ERC7579ValidatorBase } from "src/Modules.sol"; +import { PackedUserOperation } from "src/external/ERC4337.sol"; + +contract MockValidatorFalse is ERC7579ValidatorBase { + function onInstall(bytes calldata data) external virtual override { } + + function onUninstall(bytes calldata data) external virtual override { } + + function validateUserOp( + PackedUserOperation calldata userOp, + bytes32 userOpHash + ) + external + virtual + override + returns (ValidationData) + { + return _packValidationData({ sigFailed: true, validUntil: type(uint48).max, validAfter: 0 }); + } + + function isValidSignatureWithSender( + address sender, + bytes32 hash, + bytes calldata data + ) + external + view + virtual + override + returns (bytes4) + { + return EIP1271_FAILED; + } + + function isModuleType(uint256 typeID) external pure override returns (bool) { + return typeID == TYPE_VALIDATOR; + } + + function isInitialized(address smartAccount) external pure returns (bool) { + return false; + } +}