From 716d960c0838dafa060e9aefb840779aeb2ce50a Mon Sep 17 00:00:00 2001 From: Zer0dot Date: Thu, 3 Oct 2024 18:56:38 -0400 Subject: [PATCH] feat: change fallback validation magic value to zero (#211) --- ...iModularAccount_Runtime_Erc20Transfer.snap | 2 +- ...count_Runtime_InstallSessionKey_Case1.snap | 2 +- ...ModularAccount_Runtime_NativeTransfer.snap | 2 +- ...t_Runtime_UseSessionKey_Case1_Counter.snap | 2 +- ...unt_Runtime_UseSessionKey_Case1_Token.snap | 2 +- ...miModularAccount_UserOp_Erc20Transfer.snap | 2 +- ...ccount_UserOp_InstallSessionKey_Case1.snap | 2 +- ...iModularAccount_UserOp_NativeTransfer.snap | 2 +- ...nt_UserOp_UseSessionKey_Case1_Counter.snap | 2 +- ...ount_UserOp_UseSessionKey_Case1_Token.snap | 2 +- ...ularAccount_UserOp_deferredValidation.snap | 2 +- .github/workflows/test.yml | 6 - package.json | 2 +- src/helpers/Constants.sol | 2 +- .../permissions/PaymasterGuardModule.sol | 8 +- test/account/AccountExecHooks.t.sol | 22 ++-- test/account/AccountFactory.t.sol | 6 +- test/account/AccountReturnData.t.sol | 14 +-- test/account/DeferredValidation.t.sol | 37 +++--- test/account/DirectCallsFromModule.t.sol | 19 +-- test/account/GlobalValidationTest.t.sol | 6 +- test/account/HookOrdering.t.sol | 32 ++--- test/account/ModularAccount.t.sol | 76 ++++++------ test/account/ModularAccountView.t.sol | 8 +- test/account/MultiValidation.t.sol | 16 +-- test/account/PerHookData.t.sol | 46 +++---- test/account/PermittedCallPermissions.t.sol | 6 +- test/account/ReplaceModule.t.sol | 5 +- test/account/SelfCallAuthorization.t.sol | 32 ++--- .../SemiModularAccountDirectCall.t.sol | 10 +- test/account/TokenReceiver.t.sol | 10 +- test/account/UpgradeToSma.t.sol | 114 ++++++++++++++---- test/account/ValidationIntersection.t.sol | 20 +-- test/modules/AllowlistModule.t.sol | 2 +- test/modules/ERC20TokenLimitModule.t.sol | 21 ++-- test/modules/PaymasterGuardModule.t.sol | 18 +-- test/modules/TimeRangeModule.t.sol | 17 ++- test/modules/WebauthnValidationModule.t.sol | 22 ++-- test/utils/AccountTestBase.sol | 45 +++++-- test/utils/CustomValidationTestBase.sol | 23 ++-- test/utils/ModuleSignatureUtils.sol | 12 +- test/utils/TestConstants.sol | 4 +- 42 files changed, 394 insertions(+), 289 deletions(-) diff --git a/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap b/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap index 3e771615..5161f82f 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap @@ -1 +1 @@ -79399 \ No newline at end of file +79113 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap b/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap index cb93f715..2b7f6dca 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap @@ -1 +1 @@ -448696 \ No newline at end of file +448410 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap b/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap index 8345f666..2cfca2ab 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap @@ -1 +1 @@ -55523 \ No newline at end of file +55237 \ 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 4fbf2caa..4d73da37 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -90077 \ No newline at end of file +90071 \ 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 00a040c3..6af4357b 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -123520 \ No newline at end of file +123514 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap b/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap index be3d6913..1401db4d 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap @@ -1 +1 @@ -187114 \ No newline at end of file +186831 \ 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 e0a1cae1..1cd81302 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -557483 \ No newline at end of file +557200 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap b/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap index d932c717..9fea7028 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap @@ -1 +1 @@ -163250 \ No newline at end of file +162967 \ 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 ebb08b9f..debd0dc3 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -210701 \ No newline at end of file +210698 \ 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 575f05f4..e3ea9a98 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -241879 \ No newline at end of file +241876 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap index a6dfcd39..ee7d10fd 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap @@ -1 +1 @@ -265949 \ No newline at end of file +265676 \ No newline at end of file diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 86d14f55..10b5136a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -24,9 +24,6 @@ jobs: - name: Run tests run: FOUNDRY_PROFILE=optimized-test-deep forge test -vvv - - name: Run SMA tests - run: FOUNDRY_PROFILE=optimized-test-deep SMA_TEST=true forge test -vvv - test-default: name: Run Forge Tests (default) runs-on: ubuntu-latest @@ -42,6 +39,3 @@ jobs: - name: Run tests run: forge test -vvv - - - name: Run SMA tests - run: SMA_TEST=true forge test -vvv \ No newline at end of file diff --git a/package.json b/package.json index c9f36909..456cda36 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,6 @@ "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 && pnpm lint && pnpm test && pnpm gas", - "test": "forge test && SMA_TEST=true forge test" + "test": "forge test" } } diff --git a/src/helpers/Constants.sol b/src/helpers/Constants.sol index b83cfd83..6c0e9f76 100644 --- a/src/helpers/Constants.sol +++ b/src/helpers/Constants.sol @@ -13,4 +13,4 @@ uint8 constant MAX_PRE_VALIDATION_HOOKS = type(uint8).max; uint32 constant DIRECT_CALL_VALIDATION_ENTITYID = type(uint32).max; // Magic value for the ModuleEntity of the fallback validation for SemiModularAccount. -ModuleEntity constant FALLBACK_VALIDATION = ModuleEntity.wrap(bytes24(type(uint192).max)); +ModuleEntity constant FALLBACK_VALIDATION = ModuleEntity.wrap(bytes24(0)); diff --git a/src/modules/permissions/PaymasterGuardModule.sol b/src/modules/permissions/PaymasterGuardModule.sol index 6a089a97..4a5b72e4 100644 --- a/src/modules/permissions/PaymasterGuardModule.sol +++ b/src/modules/permissions/PaymasterGuardModule.sol @@ -14,7 +14,7 @@ import {BaseModule, IERC165} from "../BaseModule.sol"; /// used. /// - If this hook is installed, and no paymaster is setup, all requests will revert contract PaymasterGuardModule is BaseModule, IValidationHookModule { - mapping(uint32 entityId => mapping(address account => address paymaster)) public payamsters; + mapping(uint32 entityId => mapping(address account => address paymaster)) public paymasters; error BadPaymasterSpecified(); @@ -23,14 +23,14 @@ contract PaymasterGuardModule is BaseModule, IValidationHookModule { /// validation function onInstall(bytes calldata data) external override { (uint32 entityId, address paymaster) = abi.decode(data, (uint32, address)); - payamsters[entityId][msg.sender] = paymaster; + paymasters[entityId][msg.sender] = paymaster; } /// @inheritdoc IModule /// @param data should be encoded with the entityId of the validation function onUninstall(bytes calldata data) external override { (uint32 entityId) = abi.decode(data, (uint32)); - delete payamsters[entityId][msg.sender]; + delete paymasters[entityId][msg.sender]; } /// @inheritdoc IValidationHookModule @@ -41,7 +41,7 @@ contract PaymasterGuardModule is BaseModule, IValidationHookModule { returns (uint256) { address payingPaymaster = address(bytes20(userOp.paymasterAndData[:20])); - if (payingPaymaster == payamsters[entityId][msg.sender]) { + if (payingPaymaster == paymasters[entityId][msg.sender]) { return 0; } else { revert BadPaymasterSpecified(); diff --git a/test/account/AccountExecHooks.t.sol b/test/account/AccountExecHooks.t.sol index ed8a5faa..56f09ee0 100644 --- a/test/account/AccountExecHooks.t.sol +++ b/test/account/AccountExecHooks.t.sol @@ -27,7 +27,7 @@ contract AccountExecHooksTest is AccountTestBase { // emitted by MockModule event ReceivedCall(bytes msgData, uint256 msgValue); - function setUp() public { + function setUp() public override { _allowTestDirectCalls(); _m1.executionFunctions.push( @@ -39,7 +39,7 @@ contract AccountExecHooksTest is AccountTestBase { ); } - function test_preExecHook_install() public { + function test_preExecHook_install() public withSMATest { _installExecution1WithHooks( ManifestExecutionHook({ executionSelector: _EXEC_SELECTOR, @@ -52,7 +52,7 @@ contract AccountExecHooksTest is AccountTestBase { /// @dev Module 1 hook pair: [1, null] /// Expected execution: [1, null] - function test_preExecHook_run() public { + function test_preExecHook_run() public withSMATest { test_preExecHook_install(); vm.expectEmit(true, true, true, true); @@ -71,13 +71,13 @@ contract AccountExecHooksTest is AccountTestBase { assertTrue(success); } - function test_preExecHook_uninstall() public { + function test_preExecHook_uninstall() public withSMATest { test_preExecHook_install(); _uninstallExecution(mockModule1); } - function test_execHookPair_install() public { + function test_execHookPair_install() public withSMATest { _installExecution1WithHooks( ManifestExecutionHook({ executionSelector: _EXEC_SELECTOR, @@ -90,7 +90,7 @@ contract AccountExecHooksTest is AccountTestBase { /// @dev Module 1 hook pair: [1, 2] /// Expected execution: [1, 2] - function test_execHookPair_run() public { + function test_execHookPair_run() public withSMATest { test_execHookPair_install(); vm.expectEmit(true, true, true, true); @@ -119,13 +119,13 @@ contract AccountExecHooksTest is AccountTestBase { assertTrue(success); } - function test_execHookPair_uninstall() public { + function test_execHookPair_uninstall() public withSMATest { test_execHookPair_install(); _uninstallExecution(mockModule1); } - function test_postOnlyExecHook_install() public { + function test_postOnlyExecHook_install() public withSMATest { _installExecution1WithHooks( ManifestExecutionHook({ executionSelector: _EXEC_SELECTOR, @@ -138,7 +138,7 @@ contract AccountExecHooksTest is AccountTestBase { /// @dev Module 1 hook pair: [null, 2] /// Expected execution: [null, 2] - function test_postOnlyExecHook_run() public { + function test_postOnlyExecHook_run() public withSMATest { test_postOnlyExecHook_install(); vm.expectEmit(true, true, true, true); @@ -151,7 +151,7 @@ contract AccountExecHooksTest is AccountTestBase { assertTrue(success); } - function test_postOnlyExecHook_uninstall() public { + function test_postOnlyExecHook_uninstall() public withSMATest { test_postOnlyExecHook_install(); _uninstallExecution(mockModule1); @@ -166,11 +166,13 @@ contract AccountExecHooksTest is AccountTestBase { vm.expectEmit(true, true, true, true); emit ExecutionInstalled(address(mockModule1), _m1); + // vm.startPrank(owner1); account1.installExecution({ module: address(mockModule1), manifest: mockModule1.executionManifest(), moduleInstallData: bytes("a") }); + // vm.stopPrank(); } function _uninstallExecution(MockModule module) internal { diff --git a/test/account/AccountFactory.t.sol b/test/account/AccountFactory.t.sol index fab7384a..62009edf 100644 --- a/test/account/AccountFactory.t.sol +++ b/test/account/AccountFactory.t.sol @@ -7,13 +7,13 @@ import {AccountTestBase} from "../utils/AccountTestBase.sol"; import {TEST_DEFAULT_VALIDATION_ENTITY_ID} from "../utils/TestConstants.sol"; contract AccountFactoryTest is AccountTestBase { - function test_createAccount() public { + function test_createAccount() public withSMATest { ModularAccount account = factory.createAccount(address(this), 100, TEST_DEFAULT_VALIDATION_ENTITY_ID); assertEq(address(account.entryPoint()), address(entryPoint)); } - function test_createAccountAndGetAddress() public { + function test_createAccountAndGetAddress() public withSMATest { ModularAccount account = factory.createAccount(address(this), 100, TEST_DEFAULT_VALIDATION_ENTITY_ID); assertEq( @@ -21,7 +21,7 @@ contract AccountFactoryTest is AccountTestBase { ); } - function test_multipleDeploy() public { + function test_multipleDeploy() public withSMATest { ModularAccount account = factory.createAccount(address(this), 100, TEST_DEFAULT_VALIDATION_ENTITY_ID); uint256 startGas = gasleft(); diff --git a/test/account/AccountReturnData.t.sol b/test/account/AccountReturnData.t.sol index cdefb330..0ddb9bfd 100644 --- a/test/account/AccountReturnData.t.sol +++ b/test/account/AccountReturnData.t.sol @@ -20,7 +20,7 @@ contract AccountReturnDataTest is AccountTestBase { ResultCreatorModule public resultCreatorModule; ResultConsumerModule public resultConsumerModule; - function setUp() public { + function setUp() public override { _transferOwnershipToTest(); regularResultContract = new RegularResultContract(); @@ -55,14 +55,14 @@ contract AccountReturnDataTest is AccountTestBase { } // Tests the ability to read the result of module execution functions via the account's fallback - function test_returnData_fallback() public view { + function test_returnData_fallback() public withSMATest { bytes32 result = ResultCreatorModule(address(account1)).foo(); assertEq(result, keccak256("bar")); } // Tests the ability to read the results of contracts called via IModularAccount.execute - function test_returnData_singular_execute() public { + function test_returnData_singular_execute() public withSMATest { bytes memory returnData = account1.executeWithRuntimeValidation( abi.encodeCall( account1.execute, @@ -77,7 +77,7 @@ contract AccountReturnDataTest is AccountTestBase { } // Tests the ability to read the results of multiple contract calls via IModularAccount.executeBatch - function test_returnData_executeBatch() public { + function test_returnData_executeBatch() public withSMATest { Call[] memory calls = new Call[](2); calls[0] = Call({ target: address(regularResultContract), @@ -105,14 +105,14 @@ contract AccountReturnDataTest is AccountTestBase { } // Tests the ability to read data via routing to fallback functions - function test_returnData_execFromModule_fallback() public view { + function test_returnData_execFromModule_fallback() public withSMATest { bool result = ResultConsumerModule(address(account1)).checkResultFallback(keccak256("bar")); assertTrue(result); } - // Tests the ability to read data via executeWithRuntimeValidation - function test_returnData_authorized_exec() public { + // Tests the ability to read data via executeWithAuthorization + function test_returnData_authorized_exec() public withSMATest { bool result = ResultConsumerModule(address(account1)).checkResultExecuteWithRuntimeValidation( address(regularResultContract), keccak256("bar") ); diff --git a/test/account/DeferredValidation.t.sol b/test/account/DeferredValidation.t.sol index 7fc7b2dc..b4a440de 100644 --- a/test/account/DeferredValidation.t.sol +++ b/test/account/DeferredValidation.t.sol @@ -10,6 +10,7 @@ import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; import {ModuleEntity, ModuleEntityLib} from "../../src/libraries/ModuleEntityLib.sol"; import {ValidationConfig, ValidationConfigLib} from "../../src/libraries/ValidationConfigLib.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {CODELESS_ADDRESS} from "../utils/TestConstants.sol"; contract DeferredValidationTest is AccountTestBase { using ValidationConfigLib for ValidationConfig; @@ -21,13 +22,11 @@ contract DeferredValidationTest is AccountTestBase { bytes internal _encodedCall; ModuleEntity internal _deferredValidation; - bool internal _isSmaTest; bytes internal _deferredValidationInstallData; - function setUp() external { + function setUp() public override { _encodedCall = abi.encodeCall(ModularAccountBase.execute, (makeAddr("dead"), 0, "")); _deferredValidation = ModuleEntityLib.pack(address(_deployECDSAValidationModule()), 0); - _isSmaTest = vm.envOr("SMA_TEST", false); uint32 entityId = 0; _deferredValidationInstallData = abi.encode(entityId, owner1); @@ -35,7 +34,7 @@ contract DeferredValidationTest is AccountTestBase { // Negatives - function test_fail_deferredValidation_nonceUsed() external { + function test_fail_deferredValidation_nonceUsed() external withSMATest { uint256 nonce = entryPoint.getNonce(address(account1), 0); PackedUserOperation memory userOp = PackedUserOperation({ @@ -57,7 +56,7 @@ contract DeferredValidationTest is AccountTestBase { userOp.signature = _buildFullDeferredInstallSig( vm, owner1Key, - _isSmaTest, + _isSMATest, account1, _signerValidation, _deferredValidation, @@ -79,7 +78,7 @@ contract DeferredValidationTest is AccountTestBase { _sendOp(userOp, expectedRevertData); } - function test_fail_deferredValidation_pastDeadline() external { + function test_fail_deferredValidation_pastDeadline() external withSMATest { // Note that a deadline of 0 implies no expiry vm.warp(2); @@ -104,7 +103,7 @@ contract DeferredValidationTest is AccountTestBase { userOp.signature = _buildFullDeferredInstallSig( vm, owner1Key, - _isSmaTest, + _isSMATest, account1, _signerValidation, _deferredValidation, @@ -120,7 +119,7 @@ contract DeferredValidationTest is AccountTestBase { _sendOp(userOp, expectedRevertData); } - function test_fail_deferredValidation_invalidSig() external { + function test_fail_deferredValidation_invalidSig() external withSMATest { uint256 nonce = entryPoint.getNonce(address(account1), 0); PackedUserOperation memory userOp = PackedUserOperation({ @@ -142,8 +141,8 @@ contract DeferredValidationTest is AccountTestBase { userOp.signature = _buildFullDeferredInstallSig( vm, owner1Key, - _isSmaTest, - ModularAccount(payable(0)), + _isSMATest, + ModularAccount(payable(CODELESS_ADDRESS)), // Invalid account _signerValidation, _deferredValidation, _deferredValidationInstallData, @@ -162,7 +161,7 @@ contract DeferredValidationTest is AccountTestBase { _sendOp(userOp, expectedRevertData); } - function test_fail_deferredValidation_nonceInvalidated() external { + function test_fail_deferredValidation_nonceInvalidated() external withSMATest { vm.prank(address(entryPoint)); account1.invalidateDeferredValidationInstallNonce(0); @@ -187,7 +186,7 @@ contract DeferredValidationTest is AccountTestBase { userOp.signature = _buildFullDeferredInstallSig( vm, owner1Key, - _isSmaTest, + _isSMATest, account1, _signerValidation, _deferredValidation, @@ -207,7 +206,7 @@ contract DeferredValidationTest is AccountTestBase { _sendOp(userOp, expectedRevertData); } - function test_fail_deferredValidation_invalidDeferredValidationSig() external { + function test_fail_deferredValidation_invalidDeferredValidationSig() external withSMATest { uint256 nonce = entryPoint.getNonce(address(account1), 0); PackedUserOperation memory userOp = PackedUserOperation({ @@ -230,7 +229,7 @@ contract DeferredValidationTest is AccountTestBase { userOp.signature = _buildFullDeferredInstallSig( vm, owner1Key, - _isSmaTest, + _isSMATest, account1, _signerValidation, _deferredValidation, @@ -248,7 +247,7 @@ contract DeferredValidationTest is AccountTestBase { // Positives - function test_deferredValidation() external { + function test_deferredValidation() external withSMATest { uint256 nonce = entryPoint.getNonce(address(account1), 0); PackedUserOperation memory userOp = PackedUserOperation({ @@ -270,7 +269,7 @@ contract DeferredValidationTest is AccountTestBase { userOp.signature = _buildFullDeferredInstallSig( vm, owner1Key, - _isSmaTest, + _isSMATest, account1, _signerValidation, _deferredValidation, @@ -283,11 +282,11 @@ contract DeferredValidationTest is AccountTestBase { _sendOp(userOp, ""); } - function test_deferredValidation_initCode() external { + function test_deferredValidation_initCode() external withSMATest { ModularAccount account2; bytes memory initCode; - if (_isSmaTest) { + if (_isSMATest) { account2 = ModularAccount(payable(factory.getAddressSemiModular(owner1, 1))); initCode = abi.encodePacked(address(factory), abi.encodeCall(factory.createSemiModularAccount, (owner1, 1))); @@ -323,7 +322,7 @@ contract DeferredValidationTest is AccountTestBase { userOp.signature = _buildFullDeferredInstallSig( vm, owner1Key, - _isSmaTest, + _isSMATest, account2, _signerValidation, _deferredValidation, diff --git a/test/account/DirectCallsFromModule.t.sol b/test/account/DirectCallsFromModule.t.sol index 708dd0fe..5ab19625 100644 --- a/test/account/DirectCallsFromModule.t.sol +++ b/test/account/DirectCallsFromModule.t.sol @@ -10,6 +10,7 @@ import {ModuleEntity, ModuleEntityLib} from "../../src/libraries/ModuleEntityLib import {ValidationConfig, ValidationConfigLib} from "../../src/libraries/ValidationConfigLib.sol"; import {DirectCallModule} from "../mocks/modules/DirectCallModule.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {CODELESS_ADDRESS} from "../utils/TestConstants.sol"; contract DirectCallsFromModuleTest is AccountTestBase { using ValidationConfigLib for ValidationConfig; @@ -28,7 +29,7 @@ contract DirectCallsFromModuleTest is AccountTestBase { _; } - function setUp() public { + function setUp() public override { _module = new DirectCallModule(); assertFalse(_module.preHookRan()); assertFalse(_module.postHookRan()); @@ -39,10 +40,10 @@ contract DirectCallsFromModuleTest is AccountTestBase { /* Negatives */ /* -------------------------------------------------------------------------- */ - function test_fail_directCallModuleNotInstalled() external { + function test_fail_directCallModuleNotInstalled() external withSMATest { vm.prank(address(_module)); vm.expectRevert(_buildDirectCallDisallowedError(IModularAccount.execute.selector)); - account1.execute(address(0), 0, ""); + account1.execute(CODELESS_ADDRESS, 0, ""); } function testFuzz_fail_directCallModuleUninstalled(bool validationType) @@ -53,10 +54,10 @@ contract DirectCallsFromModuleTest is AccountTestBase { vm.prank(address(_module)); vm.expectRevert(_buildDirectCallDisallowedError(IModularAccount.execute.selector)); - account1.execute(address(0), 0, ""); + account1.execute(CODELESS_ADDRESS, 0, ""); } - function test_fail_directCallModuleCallOtherSelector() external { + function test_fail_directCallModuleCallOtherSelector() external withSMATest { _installValidationSelector(); Call[] memory calls = new Call[](0); @@ -75,7 +76,7 @@ contract DirectCallsFromModuleTest is AccountTestBase { randomizedValidationType(validationType) { vm.prank(address(_module)); - account1.execute(address(0), 0, ""); + account1.execute(CODELESS_ADDRESS, 0, ""); assertTrue(_module.preHookRan()); assertTrue(_module.postHookRan()); @@ -105,7 +106,7 @@ contract DirectCallsFromModuleTest is AccountTestBase { // Install => Succeesfully call => uninstall => fail to call vm.prank(address(_module)); - account1.execute(address(0), 0, ""); + account1.execute(CODELESS_ADDRESS, 0, ""); assertTrue(_module.preHookRan()); assertTrue(_module.postHookRan()); @@ -114,10 +115,10 @@ contract DirectCallsFromModuleTest is AccountTestBase { vm.prank(address(_module)); vm.expectRevert(_buildDirectCallDisallowedError(IModularAccount.execute.selector)); - account1.execute(address(0), 0, ""); + account1.execute(CODELESS_ADDRESS, 0, ""); } - function test_directCallsFromEOA() external { + function test_directCallsFromEOA() external withSMATest { address extraOwner = makeAddr("extraOwner"); bytes4[] memory selectors = new bytes4[](1); diff --git a/test/account/GlobalValidationTest.t.sol b/test/account/GlobalValidationTest.t.sol index 35c06a02..bc66111b 100644 --- a/test/account/GlobalValidationTest.t.sol +++ b/test/account/GlobalValidationTest.t.sol @@ -20,7 +20,7 @@ contract GlobalValidationTest is AccountTestBase { uint256 public owner2Key; ModularAccount public account2; - function setUp() public { + function setUp() public override { (owner2, owner2Key) = makeAddrAndKey("owner2"); // Compute counterfactual address @@ -33,7 +33,7 @@ contract GlobalValidationTest is AccountTestBase { vm.deal(ethRecipient, 1 wei); } - function test_globalValidation_userOp_simple() public { + function test_globalValidation_userOp_simple() public withSMATest { PackedUserOperation memory userOp = PackedUserOperation({ sender: address(account2), nonce: 0, @@ -61,7 +61,7 @@ contract GlobalValidationTest is AccountTestBase { assertEq(ethRecipient.balance, 2 wei); } - function test_globalValidation_runtime_simple() public { + function test_globalValidation_runtime_simple() public withSMATest { // Deploy the account first factory.createAccount(owner2, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID); diff --git a/test/account/HookOrdering.t.sol b/test/account/HookOrdering.t.sol index b54bb172..cbb1b98c 100644 --- a/test/account/HookOrdering.t.sol +++ b/test/account/HookOrdering.t.sol @@ -36,7 +36,7 @@ contract HookOrderingTest is AccountTestBase { ModuleEntity public orderCheckerValidationEntity; - function setUp() public { + function setUp() public override { hookOrderChecker = new HookOrderCheckerModule(); } @@ -88,7 +88,7 @@ contract HookOrderingTest is AccountTestBase { // 6. post exec (validation-assoc) hook 2: post only // 5. post exec (validation-assoc) hook 1: pre only (skipped) - function test_hookOrder_userOp_moduleExecFunction_withAssoc() public { + function test_hookOrder_userOp_moduleExecFunction_withAssoc() public withSMATest { _installOrderCheckerModuleWithValidationAssocExec(4); PackedUserOperation memory userOp = PackedUserOperation({ @@ -113,7 +113,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderWithValidationAssocExec(); } - function test_hookOrder_userOp_moduleExecFunction_noAssoc_regular() public { + function test_hookOrder_userOp_moduleExecFunction_noAssoc_regular() public withSMATest { _installOrderCheckerModuleNoValidationAssocExec(4); PackedUserOperation memory userOp = PackedUserOperation({ @@ -136,7 +136,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderNoValidationAssocExec(); } - function test_hookOrder_userOp_moduleExecFunction_noAssoc_execUO() public { + function test_hookOrder_userOp_moduleExecFunction_noAssoc_execUO() public withSMATest { _installOrderCheckerModuleNoValidationAssocExec(4); PackedUserOperation memory userOp = PackedUserOperation({ @@ -161,7 +161,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderNoValidationAssocExec(); } - function test_hookOrder_runtime_moduleExecFunction() public { + function test_hookOrder_runtime_moduleExecFunction() public withSMATest { _installOrderCheckerModuleWithValidationAssocExec(4); account1.executeWithRuntimeValidation( @@ -172,7 +172,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderWithValidationAssocExec(); } - function test_hookOrder_runtime_moduleExecFunction_noAssoc() public { + function test_hookOrder_runtime_moduleExecFunction_noAssoc() public withSMATest { _installOrderCheckerModuleNoValidationAssocExec(4); account1.executeWithRuntimeValidation( @@ -183,7 +183,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderNoValidationAssocExec(); } - function test_hookOrder_directCall_moduleExecFunction() public { + function test_hookOrder_directCall_moduleExecFunction() public withSMATest { _installOrderCheckerModuleWithValidationAssocExec(DIRECT_CALL_VALIDATION_ENTITYID); vm.prank(address(hookOrderChecker)); @@ -192,7 +192,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderDirectCallWithValidationAssocExec(); } - function test_hookOrder_directCall_moduleExecFunction_noAssoc() public { + function test_hookOrder_directCall_moduleExecFunction_noAssoc() public withSMATest { _installOrderCheckerModuleNoValidationAssocExec(DIRECT_CALL_VALIDATION_ENTITYID); vm.prank(address(hookOrderChecker)); @@ -201,7 +201,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderDirectCallNoValidationAssocExec(); } - function test_hookOrder_userOp_accountNativeFunction_withAssoc() public { + function test_hookOrder_userOp_accountNativeFunction_withAssoc() public withSMATest { _installOrderCheckerModuleWithValidationAssocExec(4); PackedUserOperation memory userOp = PackedUserOperation({ @@ -230,7 +230,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderWithValidationAssocExec(); } - function test_hookOrder_userOp_accountNativeFunction_noAssoc_regular() public { + function test_hookOrder_userOp_accountNativeFunction_noAssoc_regular() public withSMATest { _installOrderCheckerModuleNoValidationAssocExec(4); PackedUserOperation memory userOp = PackedUserOperation({ @@ -255,7 +255,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderNoValidationAssocExec(); } - function test_hookOrder_userOp_accountNativeFunction_noAssoc_execUO() public { + function test_hookOrder_userOp_accountNativeFunction_noAssoc_execUO() public withSMATest { _installOrderCheckerModuleNoValidationAssocExec(4); PackedUserOperation memory userOp = PackedUserOperation({ @@ -284,7 +284,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderNoValidationAssocExec(); } - function test_hookOrder_runtime_accountNativeFunction_regular() public { + function test_hookOrder_runtime_accountNativeFunction_regular() public withSMATest { _installOrderCheckerModuleWithValidationAssocExec(4); account1.executeWithRuntimeValidation( @@ -298,7 +298,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderWithValidationAssocExec(); } - function test_hookOrder_runtime_accountNativeFunction_noAssoc() public { + function test_hookOrder_runtime_accountNativeFunction_noAssoc() public withSMATest { _installOrderCheckerModuleNoValidationAssocExec(4); account1.executeWithRuntimeValidation( @@ -312,7 +312,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderNoValidationAssocExec(); } - function test_hookOrder_directCall_accountNativeFunction() public { + function test_hookOrder_directCall_accountNativeFunction() public withSMATest { _installOrderCheckerModuleWithValidationAssocExec(DIRECT_CALL_VALIDATION_ENTITYID); vm.prank(address(hookOrderChecker)); @@ -321,7 +321,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderDirectCallWithValidationAssocExec(); } - function test_hookOrder_directCall_accountNativeFunction_noAssoc() public { + function test_hookOrder_directCall_accountNativeFunction_noAssoc() public withSMATest { _installOrderCheckerModuleNoValidationAssocExec(DIRECT_CALL_VALIDATION_ENTITYID); vm.prank(address(hookOrderChecker)); @@ -330,7 +330,7 @@ contract HookOrderingTest is AccountTestBase { _checkInvokeOrderDirectCallNoValidationAssocExec(); } - function test_hookOrder_signatureValidation() public { + function test_hookOrder_signatureValidation() public withSMATest { _installOrderCheckerModuleWithValidationAssocExec(4); // Technically, the hooks aren't supposed to make state changes during the signature validation flow diff --git a/test/account/ModularAccount.t.sol b/test/account/ModularAccount.t.sol index b7d1ec91..d890fc14 100644 --- a/test/account/ModularAccount.t.sol +++ b/test/account/ModularAccount.t.sol @@ -25,7 +25,7 @@ import {ComprehensiveModule} from "../mocks/modules/ComprehensiveModule.sol"; import {MockExecutionInstallationModule} from "../mocks/modules/MockExecutionInstallationModule.sol"; import {MockModule} from "../mocks/modules/MockModule.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; -import {TEST_DEFAULT_VALIDATION_ENTITY_ID} from "../utils/TestConstants.sol"; +import {CODELESS_ADDRESS, TEST_DEFAULT_VALIDATION_ENTITY_ID} from "../utils/TestConstants.sol"; contract ModularAccountTest is AccountTestBase { using ECDSA for bytes32; @@ -46,13 +46,13 @@ contract ModularAccountTest is AccountTestBase { event ExecutionUninstalled(address indexed module, bool onUninstallSucceeded, ExecutionManifest manifest); event ReceivedCall(bytes msgData, uint256 msgValue); - function setUp() public { + function setUp() public override { mockExecutionInstallationModule = new MockExecutionInstallationModule(); (owner2, owner2Key) = makeAddrAndKey("owner2"); // Compute counterfactual address - if (vm.envOr("SMA_TEST", false)) { + if (_isSMATest) { account2 = ModularAccount(payable(factory.getAddressSemiModular(owner2, 0))); } else { account2 = ModularAccount(payable(factory.getAddress(owner2, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID))); @@ -60,16 +60,17 @@ contract ModularAccountTest is AccountTestBase { vm.deal(address(account2), 100 ether); ethRecipient = makeAddr("ethRecipient"); + vm.deal(ethRecipient, 1 wei); counter = new Counter(); - counter.increment(); // amoritze away gas cost of zero->nonzero transition + counter.increment(); // amortize away gas cost of zero->nonzero transition } - function test_deployAccount() public { + function test_deployAccount() public withSMATest { factory.createAccount(owner2, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID); } - function test_postDeploy_ethSend() public { + function test_postDeploy_ethSend() public withSMATest { PackedUserOperation memory userOp = PackedUserOperation({ sender: address(account1), nonce: 0, @@ -95,8 +96,8 @@ contract ModularAccountTest is AccountTestBase { assertEq(ethRecipient.balance, 2 wei); } - function test_basicUserOp_withInitCode() public { - bytes memory callData = vm.envOr("SMA_TEST", false) + function test_basicUserOp_withInitCode() public withSMATest { + bytes memory callData = _isSMATest ? abi.encodeCall(SemiModularAccountBytecode(payable(account1)).updateFallbackSigner, (owner2)) : abi.encodeCall( ModularAccountBase.execute, @@ -107,7 +108,7 @@ contract ModularAccountTest is AccountTestBase { ) ); - bytes memory initCode = vm.envOr("SMA_TEST", false) + bytes memory initCode = _isSMATest ? abi.encodePacked(address(factory), abi.encodeCall(factory.createSemiModularAccount, (owner2, 0))) : abi.encodePacked( address(factory), abi.encodeCall(factory.createAccount, (owner2, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID)) @@ -136,8 +137,8 @@ contract ModularAccountTest is AccountTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_standardExecuteEthSend_withInitcode() public { - bytes memory initCode = vm.envOr("SMA_TEST", false) + function test_standardExecuteEthSend_withInitcode() public withSMATest { + bytes memory initCode = _isSMATest ? abi.encodePacked(address(factory), abi.encodeCall(factory.createSemiModularAccount, (owner2, 0))) : abi.encodePacked( address(factory), abi.encodeCall(factory.createAccount, (owner2, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID)) @@ -170,7 +171,7 @@ contract ModularAccountTest is AccountTestBase { assertEq(recipient.balance, 1 wei); } - function test_debug_ModularAccount_storageAccesses() public { + function test_debug_ModularAccount_storageAccesses() public withSMATest { PackedUserOperation memory userOp = PackedUserOperation({ sender: address(account1), nonce: 0, @@ -196,15 +197,12 @@ contract ModularAccountTest is AccountTestBase { _printStorageReadsAndWrites(address(account2)); } - function test_accountId() public view { + function test_accountId() public withSMATest { string memory accountId = account1.accountId(); - assertEq( - accountId, - vm.envOr("SMA_TEST", false) ? "alchemy.semi-modular-account.0.0.1" : "alchemy.modular-account.0.0.1" - ); + assertEq(accountId, _isSMATest ? "alchemy.semi-modular-account.0.0.1" : "alchemy.modular-account.0.0.1"); } - function test_contractInteraction() public { + function test_contractInteraction() public withSMATest { PackedUserOperation memory userOp = PackedUserOperation({ sender: address(account1), nonce: 0, @@ -232,7 +230,7 @@ contract ModularAccountTest is AccountTestBase { assertEq(counter.number(), 2); } - function test_batchExecute() public { + function test_batchExecute() public withSMATest { // Performs both an eth send and a contract interaction with counter Call[] memory calls = new Call[](2); calls[0] = Call({target: ethRecipient, value: 1 wei, data: ""}); @@ -264,7 +262,7 @@ contract ModularAccountTest is AccountTestBase { assertEq(ethRecipient.balance, 2 wei); } - function test_installExecution() public { + function test_installExecution() public withSMATest { vm.startPrank(address(entryPoint)); vm.expectEmit(true, true, true, true); @@ -280,9 +278,10 @@ contract ModularAccountTest is AccountTestBase { ExecutionDataView memory data = account1.getExecutionData(MockExecutionInstallationModule.executionInstallationExecute.selector); assertEq(data.module, address(mockExecutionInstallationModule)); + vm.stopPrank(); } - function test_installExecution_PermittedCallSelectorNotInstalled() public { + function test_installExecution_PermittedCallSelectorNotInstalled() public withSMATest { vm.startPrank(address(entryPoint)); ExecutionManifest memory m; @@ -294,12 +293,13 @@ contract ModularAccountTest is AccountTestBase { manifest: mockModuleWithBadPermittedExec.executionManifest(), moduleInstallData: "" }); + vm.stopPrank(); } - function test_installExecution_interfaceNotSupported() public { + function test_installExecution_interfaceNotSupported() public withSMATest { vm.startPrank(address(entryPoint)); - address badModule = address(1); + address badModule = CODELESS_ADDRESS; vm.expectRevert( abi.encodeWithSelector(ModuleManagerInternals.InterfaceNotSupported.selector, address(badModule)) ); @@ -307,9 +307,10 @@ contract ModularAccountTest is AccountTestBase { ExecutionManifest memory m; account1.installExecution({module: address(badModule), manifest: m, moduleInstallData: "a"}); + vm.stopPrank(); } - function test_installExecution_alreadyInstalled() public { + function test_installExecution_alreadyInstalled() public withSMATest { ExecutionManifest memory m = mockExecutionInstallationModule.executionManifest(); vm.prank(address(entryPoint)); @@ -333,7 +334,7 @@ contract ModularAccountTest is AccountTestBase { }); } - function test_uninstallExecution_default() public { + function test_uninstallExecution_default() public withSMATest { vm.startPrank(address(entryPoint)); ComprehensiveModule module = new ComprehensiveModule(); @@ -353,6 +354,7 @@ contract ModularAccountTest is AccountTestBase { ExecutionDataView memory data = account1.getExecutionData(module.foo.selector); assertEq(data.module, address(0)); + vm.stopPrank(); } function _installExecutionWithExecHooks() internal returns (MockModule module) { @@ -369,13 +371,13 @@ contract ModularAccountTest is AccountTestBase { vm.stopPrank(); } - function test_upgradeToAndCall() public { + function test_upgradeToAndCall() public withSMATest { vm.startPrank(address(entryPoint)); ModularAccount account3 = new ModularAccount(entryPoint); bytes32 slot = account3.proxiableUUID(); // account has impl from factory - if (vm.envOr("SMA_TEST", false)) { + if (_isSMATest) { assertEq( address(semiModularAccountImplementation), address(uint160(uint256(vm.load(address(account1), slot)))) @@ -386,11 +388,12 @@ contract ModularAccountTest is AccountTestBase { account1.upgradeToAndCall(address(account3), bytes("")); // account has new impl assertEq(address(account3), address(uint160(uint256(vm.load(address(account1), slot))))); + vm.stopPrank(); } // TODO: Consider if this test belongs here or in the tests specific to the ECDSAValidationModule - function test_transferOwnership() public { - if (vm.envOr("SMA_TEST", false)) { + function test_transferOwnership() public withSMATest { + if (_isSMATest) { // Note: replaced "owner1" with address(0), this doesn't actually affect the account, but allows the // test to pass by ensuring the signer can be set in the validation. assertEq( @@ -410,10 +413,10 @@ contract ModularAccountTest is AccountTestBase { assertEq(ecdsaValidationModule.signers(TEST_DEFAULT_VALIDATION_ENTITY_ID, address(account1)), owner2); } - function test_isValidSignature() public view { + function test_isValidSignature() public withSMATest { bytes32 message = keccak256("hello world"); - bytes32 replaySafeHash = vm.envOr("SMA_TEST", false) + bytes32 replaySafeHash = _isSMATest ? SemiModularAccountBytecode(payable(account1)).replaySafeHash(message) : ecdsaValidationModule.replaySafeHash(address(account1), message); @@ -427,7 +430,7 @@ contract ModularAccountTest is AccountTestBase { } // Only need a test case for the negative case, as the positive case is covered by the isValidSignature test - function test_signatureValidationFlag_enforce() public { + function test_signatureValidationFlag_enforce() public withSMATest { // Install a new copy of ECDSAValidationModule with the signature validation flag set to false uint32 newEntityId = 2; vm.prank(address(entryPoint)); @@ -440,7 +443,7 @@ contract ModularAccountTest is AccountTestBase { bytes32 message = keccak256("hello world"); - bytes32 replaySafeHash = vm.envOr("SMA_TEST", false) + bytes32 replaySafeHash = _isSMATest ? SemiModularAccountBytecode(payable(account1)).replaySafeHash(message) : ecdsaValidationModule.replaySafeHash(address(account1), message); @@ -458,7 +461,7 @@ contract ModularAccountTest is AccountTestBase { IERC1271(address(account1)).isValidSignature(message, signature); } - function test_userOpValidationFlag_enforce() public { + function test_userOpValidationFlag_enforce() public withSMATest { // Install a new copy of ECDSAValidationModule with the userOp validation flag set to false uint32 newEntityId = 2; vm.prank(address(entryPoint)); @@ -515,9 +518,10 @@ contract ModularAccountTest is AccountTestBase { ); assertEq(ethRecipient.balance, 2 wei); + vm.stopPrank(); } - function test_performCreate() public { + function test_performCreate() public withSMATest { address expectedAddr = vm.computeCreateAddress(address(account1), vm.getNonce(address(account1))); vm.prank(address(entryPoint)); address returnedAddr = account1.performCreate( @@ -528,7 +532,7 @@ contract ModularAccountTest is AccountTestBase { assertEq(address(ModularAccount(payable(expectedAddr)).entryPoint()), address(entryPoint)); } - function test_performCreate2() public { + function test_performCreate2() public withSMATest { bytes memory initCode = abi.encodePacked(type(ModularAccount).creationCode, abi.encode(address(entryPoint))); bytes32 initCodeHash = keccak256(initCode); diff --git a/test/account/ModularAccountView.t.sol b/test/account/ModularAccountView.t.sol index 9797e69f..02b5463d 100644 --- a/test/account/ModularAccountView.t.sol +++ b/test/account/ModularAccountView.t.sol @@ -22,7 +22,7 @@ contract ModularAccountViewTest is CustomValidationTestBase { ModuleEntity public comprehensiveModuleValidation; - function setUp() public { + function setUp() public override { comprehensiveModule = new ComprehensiveModule(); comprehensiveModuleValidation = ModuleEntityLib.pack(address(comprehensiveModule), uint32(ComprehensiveModule.EntityId.VALIDATION)); @@ -34,7 +34,7 @@ contract ModularAccountViewTest is CustomValidationTestBase { vm.stopPrank(); } - function test_moduleView_getExecutionData_native() public view { + function test_moduleView_getExecutionData_native() public withSMATest { bytes4[] memory selectorsToCheck = new bytes4[](5); selectorsToCheck[0] = IModularAccount.execute.selector; @@ -55,7 +55,7 @@ contract ModularAccountViewTest is CustomValidationTestBase { } } - function test_moduleView_getExecutionData_module() public view { + function test_moduleView_getExecutionData_module() public withSMATest { bytes4[] memory selectorsToCheck = new bytes4[](1); address[] memory expectedModuleAddress = new address[](1); @@ -99,7 +99,7 @@ contract ModularAccountViewTest is CustomValidationTestBase { } } - function test_moduleView_getValidationData() public view { + function test_moduleView_getValidationData() public withSMATest { ValidationDataView memory data = account1.getValidationData(comprehensiveModuleValidation); bytes4[] memory selectors = data.selectors; diff --git a/test/account/MultiValidation.t.sol b/test/account/MultiValidation.t.sol index 2bb270a4..60ac8669 100644 --- a/test/account/MultiValidation.t.sol +++ b/test/account/MultiValidation.t.sol @@ -13,7 +13,7 @@ import {ModuleEntityLib} from "../../src/libraries/ModuleEntityLib.sol"; import {ValidationConfigLib} from "../../src/libraries/ValidationConfigLib.sol"; import {ECDSAValidationModule} from "../../src/modules/validation/ECDSAValidationModule.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; -import {TEST_DEFAULT_VALIDATION_ENTITY_ID} from "../utils/TestConstants.sol"; +import {CODELESS_ADDRESS, TEST_DEFAULT_VALIDATION_ENTITY_ID} from "../utils/TestConstants.sol"; contract MultiValidationTest is AccountTestBase { using ECDSA for bytes32; @@ -24,13 +24,13 @@ contract MultiValidationTest is AccountTestBase { address public owner2; uint256 public owner2Key; - function setUp() public { + function setUp() public override { validator2 = new ECDSAValidationModule(); (owner2, owner2Key) = makeAddrAndKey("owner2"); } - function test_overlappingValidationInstall() public { + function test_overlappingValidationInstall() public withSMATest { vm.prank(address(entryPoint)); account1.installValidation( ValidationConfigLib.pack(address(validator2), TEST_DEFAULT_VALIDATION_ENTITY_ID, true, true, true), @@ -51,7 +51,7 @@ contract MultiValidationTest is AccountTestBase { } } - function test_runtimeValidation_specify() public { + function test_runtimeValidation_specify() public withSMATest { test_overlappingValidationInstall(); // Assert that the runtime validation can be specified. @@ -66,7 +66,7 @@ contract MultiValidationTest is AccountTestBase { ) ); account1.executeWithRuntimeValidation( - abi.encodeCall(IModularAccount.execute, (address(0), 0, "")), + abi.encodeCall(IModularAccount.execute, (CODELESS_ADDRESS, 0, "")), _encodeSignature( ModuleEntityLib.pack(address(validator2), TEST_DEFAULT_VALIDATION_ENTITY_ID), GLOBAL_VALIDATION, "" ) @@ -74,14 +74,14 @@ contract MultiValidationTest is AccountTestBase { vm.prank(owner2); account1.executeWithRuntimeValidation( - abi.encodeCall(IModularAccount.execute, (address(0), 0, "")), + abi.encodeCall(IModularAccount.execute, (CODELESS_ADDRESS, 0, "")), _encodeSignature( ModuleEntityLib.pack(address(validator2), TEST_DEFAULT_VALIDATION_ENTITY_ID), GLOBAL_VALIDATION, "" ) ); } - function test_userOpValidation_specify() public { + function test_userOpValidation_specify() public withSMATest { test_overlappingValidationInstall(); // Assert that the userOp validation can be specified. @@ -90,7 +90,7 @@ contract MultiValidationTest is AccountTestBase { sender: address(account1), nonce: 0, initCode: "", - callData: abi.encodeCall(ModularAccountBase.execute, (address(0), 0, "")), + callData: abi.encodeCall(ModularAccountBase.execute, (CODELESS_ADDRESS, 0, "")), accountGasLimits: _encodeGas(VERIFICATION_GAS_LIMIT, CALL_GAS_LIMIT), preVerificationGas: 0, gasFees: _encodeGas(1, 1), diff --git a/test/account/PerHookData.t.sol b/test/account/PerHookData.t.sol index 14bcd1da..05aa687c 100644 --- a/test/account/PerHookData.t.sol +++ b/test/account/PerHookData.t.sol @@ -25,7 +25,7 @@ contract PerHookDataTest is CustomValidationTestBase { uint32 internal constant _PRE_HOOK_ENTITY_ID_1 = 0; uint32 internal constant _PRE_HOOK_ENTITY_ID_2 = 1; - function setUp() public { + function setUp() public override { _counter = new Counter(); _accessControlHookModule = new MockAccessControlHookModule(); @@ -33,7 +33,7 @@ contract PerHookDataTest is CustomValidationTestBase { _customValidationSetup(); } - function test_passAccessControl_userOp() public { + function test_passAccessControl_userOp() public withSMATest { assertEq(_counter.number(), 0); (PackedUserOperation memory userOp, bytes32 userOpHash) = _getCounterUserOP(); @@ -55,7 +55,7 @@ contract PerHookDataTest is CustomValidationTestBase { assertEq(_counter.number(), 1); } - function test_failAccessControl_badSigData_userOp() public { + function test_failAccessControl_badSigData_userOp() public withSMATest { (PackedUserOperation memory userOp, bytes32 userOpHash) = _getCounterUserOP(); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); @@ -84,7 +84,7 @@ contract PerHookDataTest is CustomValidationTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_failAccessControl_noSigData_userOp() public { + function test_failAccessControl_noSigData_userOp() public withSMATest { (PackedUserOperation memory userOp, bytes32 userOpHash) = _getCounterUserOP(); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); @@ -104,7 +104,7 @@ contract PerHookDataTest is CustomValidationTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_failAccessControl_badIndexProvided_userOp() public { + function test_failAccessControl_badIndexProvided_userOp() public withSMATest { (PackedUserOperation memory userOp, bytes32 userOpHash) = _getCounterUserOP(); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); @@ -130,7 +130,7 @@ contract PerHookDataTest is CustomValidationTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_passAccessControl_twoHooks_userOp() public { + function test_passAccessControl_twoHooks_userOp() public withSMATest { _installSecondPreHook(); assertEq(_counter.number(), 0); @@ -155,7 +155,7 @@ contract PerHookDataTest is CustomValidationTestBase { assertEq(_counter.number(), 1); } - function test_failAccessControl_indexOutOfOrder_userOp() public { + function test_failAccessControl_indexOutOfOrder_userOp() public withSMATest { _installSecondPreHook(); (PackedUserOperation memory userOp, bytes32 userOpHash) = _getCounterUserOP(); @@ -183,7 +183,7 @@ contract PerHookDataTest is CustomValidationTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_failAccessControl_badTarget_userOp() public { + function test_failAccessControl_badTarget_userOp() public withSMATest { PackedUserOperation memory userOp = PackedUserOperation({ sender: address(account1), nonce: 0, @@ -220,7 +220,7 @@ contract PerHookDataTest is CustomValidationTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_failPerHookData_nonCanonicalEncoding_userOp() public { + function test_failPerHookData_nonCanonicalEncoding_userOp() public withSMATest { (PackedUserOperation memory userOp, bytes32 userOpHash) = _getCounterUserOP(); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); @@ -245,7 +245,7 @@ contract PerHookDataTest is CustomValidationTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_failPerHookData_excessData_userOp() public { + function test_failPerHookData_excessData_userOp() public withSMATest { (PackedUserOperation memory userOp, bytes32 userOpHash) = _getCounterUserOP(); (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); @@ -273,7 +273,7 @@ contract PerHookDataTest is CustomValidationTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_passAccessControl_runtime() public { + function test_passAccessControl_runtime() public withSMATest { assertEq(_counter.number(), 0); PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1); @@ -290,7 +290,7 @@ contract PerHookDataTest is CustomValidationTestBase { assertEq(_counter.number(), 1); } - function test_failAccessControl_badSigData_runtime() public { + function test_failAccessControl_badSigData_runtime() public withSMATest { PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1); preValidationHookData[0] = PreValidationHookData({ index: 0, @@ -314,7 +314,7 @@ contract PerHookDataTest is CustomValidationTestBase { ); } - function test_failAccessControl_noSigData_runtime() public { + function test_failAccessControl_noSigData_runtime() public withSMATest { vm.prank(owner1); vm.expectRevert( abi.encodeWithSelector( @@ -332,7 +332,7 @@ contract PerHookDataTest is CustomValidationTestBase { ); } - function test_failAccessControl_badIndexProvided_runtime() public { + function test_failAccessControl_badIndexProvided_runtime() public withSMATest { PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](2); preValidationHookData[0] = PreValidationHookData({index: 0, validationData: abi.encodePacked(_counter)}); preValidationHookData[1] = PreValidationHookData({index: 1, validationData: abi.encodePacked(_counter)}); @@ -349,7 +349,7 @@ contract PerHookDataTest is CustomValidationTestBase { ); } - function test_passAccessControl_twoHooks_runtime() public { + function test_passAccessControl_twoHooks_runtime() public withSMATest { _installSecondPreHook(); assertEq(_counter.number(), 0); @@ -369,7 +369,7 @@ contract PerHookDataTest is CustomValidationTestBase { assertEq(_counter.number(), 1); } - function test_failAccessControl_indexOutOfOrder_runtime() public { + function test_failAccessControl_indexOutOfOrder_runtime() public withSMATest { _installSecondPreHook(); PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](3); @@ -386,7 +386,7 @@ contract PerHookDataTest is CustomValidationTestBase { ); } - function test_failAccessControl_badTarget_runtime() public { + function test_failAccessControl_badTarget_runtime() public withSMATest { PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1); preValidationHookData[0] = PreValidationHookData({index: 0, validationData: abi.encodePacked(beneficiary)}); @@ -405,7 +405,7 @@ contract PerHookDataTest is CustomValidationTestBase { ); } - function test_failPerHookData_nonCanonicalEncoding_runtime() public { + function test_failPerHookData_nonCanonicalEncoding_runtime() public withSMATest { PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1); preValidationHookData[0] = PreValidationHookData({index: 0, validationData: ""}); @@ -419,7 +419,7 @@ contract PerHookDataTest is CustomValidationTestBase { ); } - function test_failPerHookData_excessData_runtime() public { + function test_failPerHookData_excessData_runtime() public withSMATest { PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1); preValidationHookData[0] = PreValidationHookData({index: 0, validationData: abi.encodePacked(_counter)}); @@ -435,7 +435,7 @@ contract PerHookDataTest is CustomValidationTestBase { ); } - function test_pass1271AccessControl() public view { + function test_pass1271AccessControl() public withSMATest { string memory message = "Hello, world!"; bytes32 messageHash = keccak256(abi.encodePacked(message)); @@ -453,7 +453,7 @@ contract PerHookDataTest is CustomValidationTestBase { assertEq(result, bytes4(0x1626ba7e)); } - function test_fail1271AccessControl_badSigData() public { + function test_fail1271AccessControl_badSigData() public withSMATest { string memory message = "Hello, world!"; bytes32 messageHash = keccak256(abi.encodePacked(message)); @@ -472,7 +472,7 @@ contract PerHookDataTest is CustomValidationTestBase { ); } - function test_fail1271AccessControl_noSigData() public { + function test_fail1271AccessControl_noSigData() public withSMATest { string memory message = "Hello, world!"; bytes32 messageHash = keccak256(abi.encodePacked(message)); @@ -529,7 +529,7 @@ contract PerHookDataTest is CustomValidationTestBase { HookConfigLib.packValidationHook(address(_accessControlHookModule), _PRE_HOOK_ENTITY_ID_1), abi.encode(_PRE_HOOK_ENTITY_ID_1, _counter) ); - // patched to also work during SMA tests by differentiating the validation + // patched to work during SMA tests by enforcing that the new validation is not the fallback validation. _signerValidation = ModuleEntityLib.pack(address(ecdsaValidationModule), _VALIDATION_ENTITY_ID); return ( _signerValidation, true, true, true, new bytes4[](0), abi.encode(_VALIDATION_ENTITY_ID, owner1), hooks diff --git a/test/account/PermittedCallPermissions.t.sol b/test/account/PermittedCallPermissions.t.sol index dcd28914..bce71aff 100644 --- a/test/account/PermittedCallPermissions.t.sol +++ b/test/account/PermittedCallPermissions.t.sol @@ -12,7 +12,7 @@ contract PermittedCallPermissionsTest is AccountTestBase { PermittedCallerModule public permittedCallerModule; - function setUp() public { + function setUp() public override { _transferOwnershipToTest(); resultCreatorModule = new ResultCreatorModule(); @@ -36,14 +36,14 @@ contract PermittedCallPermissionsTest is AccountTestBase { vm.stopPrank(); } - function test_permittedCall_Allowed() public view { + function test_permittedCall_Allowed() public withSMATest { bytes memory result = PermittedCallerModule(address(account1)).usePermittedCallAllowed(); bytes32 actual = abi.decode(result, (bytes32)); assertEq(actual, keccak256("bar")); } - function test_permittedCall_NotAllowed() public { + function test_permittedCall_NotAllowed() public withSMATest { vm.expectRevert( abi.encodeWithSelector( ModularAccountBase.ValidationFunctionMissing.selector, ResultCreatorModule.bar.selector diff --git a/test/account/ReplaceModule.t.sol b/test/account/ReplaceModule.t.sol index a5e0fcc2..86c81838 100644 --- a/test/account/ReplaceModule.t.sol +++ b/test/account/ReplaceModule.t.sol @@ -32,7 +32,7 @@ contract UpgradeModuleTest is AccountTestBase { // From MockModule event ReceivedCall(bytes msgData, uint256 msgValue); - function test_upgradeModuleExecutionFunction() public { + function test_upgradeModuleExecutionFunction() public withSMATest { ExecutionManifest memory m; ManifestExecutionFunction[] memory executionFunctions = new ManifestExecutionFunction[](1); executionFunctions[0] = ManifestExecutionFunction({ @@ -96,7 +96,7 @@ contract UpgradeModuleTest is AccountTestBase { TestModule(address(account1)).testFunction(); } - function test_upgradeModuleValidationFunction() public { + function test_upgradeModuleValidationFunction() public withSMATest { // Setup new validaiton with pre validation and execution hooks associated with a validator ECDSAValidationModule validation1 = new ECDSAValidationModule(); ECDSAValidationModule validation2 = new ECDSAValidationModule(); @@ -213,5 +213,6 @@ contract UpgradeModuleTest is AccountTestBase { ); account1.executeWithRuntimeValidation(callData, _encodeSignature(newModuleEntity, GLOBAL_VALIDATION, "")); assertEq(target.balance, 2 * sendAmount); + vm.stopPrank(); } } diff --git a/test/account/SelfCallAuthorization.t.sol b/test/account/SelfCallAuthorization.t.sol index 34287a18..e911bcc5 100644 --- a/test/account/SelfCallAuthorization.t.sol +++ b/test/account/SelfCallAuthorization.t.sol @@ -18,7 +18,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { ModuleEntity public comprehensiveModuleValidation; - function setUp() public { + function setUp() public override { // install the comprehensive module to get new exec functions with different validations configured. comprehensiveModule = new ComprehensiveModule(); @@ -40,7 +40,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { vm.stopPrank(); } - function test_selfCallFails_userOp() public { + function test_selfCallFails_userOp() public withSMATest { // Uses global validation _runUserOp( abi.encodeCall(ComprehensiveModule.foo, ()), @@ -55,7 +55,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { ); } - function test_selfCallFails_execUserOp() public { + function test_selfCallFails_execUserOp() public withSMATest { // Uses global validation _runUserOp( abi.encodePacked(IAccountExecute.executeUserOp.selector, abi.encodeCall(ComprehensiveModule.foo, ())), @@ -70,7 +70,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { ); } - function test_selfCallFails_runtime() public { + function test_selfCallFails_runtime() public withSMATest { // Uses global validation _runtimeCall( abi.encodeCall(ComprehensiveModule.foo, ()), @@ -80,7 +80,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { ); } - function test_selfCallPrivilegeEscalation_prevented_userOp() public { + function test_selfCallPrivilegeEscalation_prevented_userOp() public withSMATest { // Using global validation, self-call bypasses custom validation needed for ComprehensiveModule.foo _runUserOp( abi.encodeCall( @@ -110,7 +110,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { ); } - function test_selfCallPrivilegeEscalation_prevented_execUserOp() public { + function test_selfCallPrivilegeEscalation_prevented_execUserOp() public withSMATest { // Using global validation, self-call bypasses custom validation needed for ComprehensiveModule.foo _runUserOp( abi.encodePacked( @@ -145,7 +145,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { ); } - function test_selfCallPrivilegeEscalation_prevented_runtime() public { + function test_selfCallPrivilegeEscalation_prevented_runtime() public withSMATest { // Using global validation, self-call bypasses custom validation needed for ComprehensiveModule.foo _runtimeCall( abi.encodeCall( @@ -165,7 +165,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { ); } - function test_batchAction_allowed_userOp() public { + function test_batchAction_allowed_userOp() public withSMATest { _enableBatchValidation(); Call[] memory calls = new Call[](2); @@ -178,11 +178,11 @@ contract SelfCallAuthorizationTest is AccountTestBase { PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; - vm.expectCall(address(comprehensiveModule), abi.encodeCall(ComprehensiveModule.foo, ()), 2); + vm.expectCall(address(comprehensiveModule), abi.encodeCall(ComprehensiveModule.foo, ())); entryPoint.handleOps(userOps, beneficiary); } - function test_batchAction_allowed_execUserOp() public { + function test_batchAction_allowed_execUserOp() public withSMATest { _enableBatchValidation(); Call[] memory calls = new Call[](2); @@ -198,25 +198,25 @@ contract SelfCallAuthorizationTest is AccountTestBase { PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; - vm.expectCall(address(comprehensiveModule), abi.encodeCall(ComprehensiveModule.foo, ()), 2); + vm.expectCall(address(comprehensiveModule), abi.encodeCall(ComprehensiveModule.foo, ())); entryPoint.handleOps(userOps, beneficiary); } - function test_batchAction_allowed_runtime() public { + function test_batchAction_allowed_runtime() public withSMATest { _enableBatchValidation(); Call[] memory calls = new Call[](2); calls[0] = Call(address(account1), 0, abi.encodeCall(ComprehensiveModule.foo, ())); calls[1] = Call(address(account1), 0, abi.encodeCall(ComprehensiveModule.foo, ())); - vm.expectCall(address(comprehensiveModule), abi.encodeCall(ComprehensiveModule.foo, ()), 2); + vm.expectCall(address(comprehensiveModule), abi.encodeCall(ComprehensiveModule.foo, ())); account1.executeWithRuntimeValidation( abi.encodeCall(IModularAccount.executeBatch, (calls)), _encodeSignature(comprehensiveModuleValidation, SELECTOR_ASSOCIATED_VALIDATION, "") ); } - function test_recursiveDepthCapped_userOp() public { + function test_recursiveDepthCapped_userOp() public withSMATest { _enableBatchValidation(); Call[] memory innerCalls = new Call[](1); @@ -243,7 +243,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_recursiveDepthCapped_execUserOp() public { + function test_recursiveDepthCapped_execUserOp() public withSMATest { _enableBatchValidation(); Call[] memory innerCalls = new Call[](1); @@ -272,7 +272,7 @@ contract SelfCallAuthorizationTest is AccountTestBase { entryPoint.handleOps(userOps, beneficiary); } - function test_recursiveDepthCapped_runtime() public { + function test_recursiveDepthCapped_runtime() public withSMATest { _enableBatchValidation(); Call[] memory innerCalls = new Call[](1); diff --git a/test/account/SemiModularAccountDirectCall.t.sol b/test/account/SemiModularAccountDirectCall.t.sol index 29a4cb49..4ca5c3d7 100644 --- a/test/account/SemiModularAccountDirectCall.t.sol +++ b/test/account/SemiModularAccountDirectCall.t.sol @@ -17,7 +17,7 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { ModuleEntity internal _directCallModuleEntity; ModuleEntity internal _fallbackDirectCallModuleEntity; - function setUp() external { + function setUp() public override { _module = new MockSMADirectFallbackModule(); _directCallModuleEntity = ModuleEntityLib.pack(address(_module), DIRECT_CALL_VALIDATION_ENTITYID); @@ -30,7 +30,7 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { // Negatives - function test_fail_smaDirectCall_disabledFallbackSigner() external { + function test_fail_smaDirectCall_disabledFallbackSigner() external withSMATest { vm.prank(owner1); SemiModularAccountBase(payable(account1)).setFallbackSignerDisabled(true); @@ -43,7 +43,7 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { account1.execute(_target, 0, ""); } - function test_fail_smaDirectCall_notFallbackSigner() external { + function test_fail_smaDirectCall_notFallbackSigner() external withSMATest { bytes memory expectedRevertData = abi.encodeWithSelector( ModularAccountBase.ValidationFunctionMissing.selector, ModularAccountBase.execute.selector ); @@ -55,7 +55,7 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { // Positives - function test_Flow_smaDirectCall_installedHooksUninstalled() external { + function test_Flow_smaDirectCall_installedHooksUninstalled() external withSMATest { // We install the validation as if it were a direct call validation, but we pass hooks from the // DirectCallModule, and while the validation remains the fallback signer + direct call entityId. bytes[] memory hooks = new bytes[](2); @@ -106,7 +106,7 @@ contract SemiModularAccountDirectCallTest is AccountTestBase { assertFalse(_module.validationHookRan()); } - function test_smaDirectCall() external { + function test_smaDirectCall() external withSMATest { vm.prank(owner1); account1.execute(_target, 0, ""); } diff --git a/test/account/TokenReceiver.t.sol b/test/account/TokenReceiver.t.sol index 4cce2fb2..b4d96804 100644 --- a/test/account/TokenReceiver.t.sol +++ b/test/account/TokenReceiver.t.sol @@ -14,7 +14,7 @@ contract TokenReceiverTest is AccountTestBase { uint256 internal constant _NFT_TOKEN_ID = 0; uint256 internal constant _NFT_TOKEN_COUNT = 10; - function setUp() public { + function setUp() public override { // Compute counterfactual address // account1 = factory.createAccount(owner1, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID); // vm.deal(address(account1), 100 ether); @@ -25,26 +25,26 @@ contract TokenReceiverTest is AccountTestBase { erc1155.mint(owner1, _NFT_TOKEN_ID, _NFT_TOKEN_COUNT); } - function test_supportedInterfaces() public view { + function test_supportedInterfaces() public withSMATest { assertTrue(account1.supportsInterface(type(IERC721Receiver).interfaceId)); assertTrue(account1.supportsInterface(type(IERC1155Receiver).interfaceId)); } - function test_receiveERC721() public { + function test_receiveERC721() public withSMATest { assertEq(owner1, erc721.ownerOf(_NFT_TOKEN_ID)); vm.prank(owner1); erc721.transferFrom(owner1, address(account1), _NFT_TOKEN_ID); assertEq(address(account1), erc721.ownerOf(_NFT_TOKEN_ID)); } - function test_receiveERC1155() public { + function test_receiveERC1155() public withSMATest { assertEq(_NFT_TOKEN_COUNT, erc1155.balanceOf(owner1, _NFT_TOKEN_ID)); vm.prank(owner1); erc1155.safeTransferFrom(owner1, address(account1), _NFT_TOKEN_ID, _NFT_TOKEN_COUNT, ""); assertEq(_NFT_TOKEN_COUNT, erc1155.balanceOf(address(account1), _NFT_TOKEN_ID)); } - function test_receiveERC1155Batch() public { + function test_receiveERC1155Batch() public withSMATest { uint256[] memory ids = new uint256[](1); uint256[] memory values = new uint256[](1); ids[0] = _NFT_TOKEN_ID; diff --git a/test/account/UpgradeToSma.t.sol b/test/account/UpgradeToSma.t.sol index a75b17ea..96564273 100644 --- a/test/account/UpgradeToSma.t.sol +++ b/test/account/UpgradeToSma.t.sol @@ -1,34 +1,40 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.26; +import {LightAccount} from "@alchemy/light-account/src/LightAccount.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"; +import {LibClone} from "solady/utils/LibClone.sol"; import {AccountStorageInitializable} from "../../src/account/AccountStorageInitializable.sol"; import {ModularAccount} from "../../src/account/ModularAccount.sol"; import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; import {SemiModularAccountBase} from "../../src/account/SemiModularAccountBase.sol"; import {SemiModularAccountStorageOnly} from "../../src/account/SemiModularAccountStorageOnly.sol"; +import {FALLBACK_VALIDATION} from "../../src/helpers/Constants.sol"; +import {ModuleEntity, ModuleEntityLib} from "../../src/libraries/ModuleEntityLib.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; -import {LightAccount} from "@alchemy/light-account/src/LightAccount.sol"; -import {LibClone} from "solady/utils/LibClone.sol"; +import {CODELESS_ADDRESS} from "../utils/TestConstants.sol"; contract UpgradeToSmaTest is AccountTestBase { + using ModuleEntityLib for ModuleEntity; + using MessageHashUtils for bytes32; + address public smaStorageImpl; address public owner2; uint256 public owner2Key; + uint256 public transferAmount; - function setUp() external { + function setUp() public override { smaStorageImpl = address(new SemiModularAccountStorageOnly(entryPoint)); (owner2, owner2Key) = makeAddrAndKey("owner2"); + transferAmount = 0.1 ether; } + // This test should only run with an MA, using withSMATest would result in the SMABytecode not being + // initialized and the initialize call not reverting. function test_fail_upgradeToAndCall_initializedMaToSmaStorage() external { - // This should only fail if the contract is initialized, and SMABytecode does not have an initializer, - // so we skip this case by checking the env variable. - if (vm.envOr("SMA_TEST", false)) { - return; - } - // The call should revert with invalid initialization. vm.expectRevert(AccountStorageInitializable.InvalidInitialization.selector); @@ -46,15 +52,31 @@ contract UpgradeToSmaTest is AccountTestBase { smaStorageImpl, abi.encodeCall(SemiModularAccountBase.updateFallbackSigner, owner2) ); + // The previous owner1 validation is still installed, so this should not revert. + _userOpTransfer(address(account1), owner1Key, "", 0, false); + + vm.prank(owner2); + account1.uninstallValidation(_signerValidation, "", new bytes[](0)); + // Build expected revert data for a UO with the original signer. - bytes memory expectedRevertdata = - abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error"); + bytes memory expectedRevertdata = abi.encodeWithSelector( + IEntryPoint.FailedOpWithRevert.selector, + 0, + "AA23 reverted", + abi.encodeWithSelector( + ModularAccountBase.ValidationFunctionMissing.selector, ModularAccountBase.execute.selector + ) + ); - // Attempt to execute a UO with the original signer, anticipating a revert. - _userOpTransfer(address(account1), owner1Key, expectedRevertdata); + // Execute a UO with the original signer and the now uninstalled validation, anticipating a revert. + _userOpTransfer(address(account1), owner1Key, expectedRevertdata, transferAmount, false); - // Attempt to successfully execute a UO with the new signer, which is the fallback signer. - _userOpTransfer(address(account1), owner2Key, ""); + expectedRevertdata = abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error"); + // Execute a UO with the original signer and the fallback validation, anticipating a revert. + _userOpTransfer(address(account1), owner1Key, expectedRevertdata, transferAmount, true); + + // Execute a UO with the new signer, which is the fallback signer. + _userOpTransfer(address(account1), owner2Key, "", transferAmount, true); } function test_upgradeToAndCall_LaToSmaStorage() external { @@ -78,29 +100,75 @@ contract UpgradeToSmaTest is AccountTestBase { abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error"); // Attempt to execute a UO with the original signer, anticipating a revert. - _userOpTransfer(address(newAccount), owner1Key, expectedRevertdata); + _userOpTransfer(address(newAccount), owner1Key, expectedRevertdata, 0, true); // Attempt to successfully execute a UO with the new signer, which is the fallback signer. - _userOpTransfer(newAccount, owner2Key, ""); + _userOpTransfer(newAccount, owner2Key, "", 0, true); } - function _userOpTransfer(address account, uint256 ownerKey, bytes memory expectedRevertData) internal { + // Internal helpers + + function _userOpTransfer( + address account, + uint256 ownerKey, + bytes memory expectedRevertData, + uint256 initialBalance, + bool withFallbackValidation + ) internal { // Pre-fund the account. deal(account, 1 ether); // Generate a target and ensure it has no balance. - address target = makeAddr("4546b"); - assertEq(target.balance, 0, "Target has balance when it shouldn't"); + address target = CODELESS_ADDRESS; + assertEq(target.balance, initialBalance, "Target has balance when it shouldn't"); // Encode a transfer to the target. - bytes memory encodedCall = abi.encodeCall(ModularAccountBase.execute, (target, 0.1 ether, "")); + bytes memory encodedCall = abi.encodeCall(ModularAccountBase.execute, (target, transferAmount, "")); // Run a UO with the encoded call. - _runUserOpFrom(account, ownerKey, encodedCall, expectedRevertData); + if (withFallbackValidation) { + _runUserOpWithFallbackValidation(account, ownerKey, encodedCall, expectedRevertData); + } else { + _runUserOpFrom(account, ownerKey, encodedCall, expectedRevertData); + } // If the call was not supposed to revert, ensure the transfer succeeded. if (expectedRevertData.length == 0) { - assertEq(target.balance, 0.1 ether, "Target missing balance from UO transfer"); + assertEq(target.balance, transferAmount + initialBalance, "Target missing balance from UO transfer"); + } + } + + function _runUserOpWithFallbackValidation( + address account, + uint256 ownerKey, + bytes memory encodedCall, + bytes memory expectedRevertData + ) internal { + uint256 nonce = entryPoint.getNonce(address(account), 0); + + PackedUserOperation memory userOp = PackedUserOperation({ + sender: account, + nonce: nonce, + initCode: hex"", + callData: encodedCall, + accountGasLimits: _encodeGas(VERIFICATION_GAS_LIMIT, CALL_GAS_LIMIT), + preVerificationGas: 0, + gasFees: _encodeGas(1, 1), + paymasterAndData: hex"", + signature: hex"" + }); + + bytes32 userOpHash = entryPoint.getUserOpHash(userOp); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, userOpHash.toEthSignedMessageHash()); + + userOp.signature = _encodeSignature(FALLBACK_VALIDATION, GLOBAL_VALIDATION, abi.encodePacked(r, s, v)); + + PackedUserOperation[] memory userOps = new PackedUserOperation[](1); + userOps[0] = userOp; + + if (expectedRevertData.length > 0) { + vm.expectRevert(expectedRevertData); } + entryPoint.handleOps(userOps, beneficiary); } } diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index 8b8faecb..bc2c35bd 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -26,7 +26,7 @@ contract ValidationIntersectionTest is AccountTestBase { ModuleEntity public oneHookValidation; ModuleEntity public twoHookValidation; - function setUp() public { + function setUp() public override { noHookModule = new MockUserOpValidationModule(); oneHookModule = new MockUserOpValidation1HookModule(); twoHookModule = new MockUserOpValidation2HookModule(); @@ -103,7 +103,7 @@ contract ValidationIntersectionTest is AccountTestBase { assertEq(returnedValidationData, validationData); } - function test_validationIntersect_authorizer_sigfail_validationFunction() public { + function test_validationIntersect_authorizer_sigfail_validationFunction() public withSMATest { oneHookModule.setValidationData( _SIG_VALIDATION_FAILED, 0 // returns OK @@ -121,7 +121,7 @@ contract ValidationIntersectionTest is AccountTestBase { assertEq(uint160(returnedValidationData), _SIG_VALIDATION_FAILED); } - function test_validationIntersect_authorizer_sigfail_hook() public { + function test_validationIntersect_authorizer_sigfail_hook() public withSMATest { oneHookModule.setValidationData( 0, // returns OK _SIG_VALIDATION_FAILED @@ -139,7 +139,7 @@ contract ValidationIntersectionTest is AccountTestBase { assertEq(uint160(returnedValidationData), _SIG_VALIDATION_FAILED); } - function test_validationIntersect_timeBounds_intersect_1() public { + function test_validationIntersect_timeBounds_intersect_1() public withSMATest { uint48 start1 = uint48(10); uint48 end1 = uint48(20); @@ -161,7 +161,7 @@ contract ValidationIntersectionTest is AccountTestBase { assertEq(returnedValidationData, _packValidationRes(address(0), start2, end1)); } - function test_validationIntersect_timeBounds_intersect_2() public { + function test_validationIntersect_timeBounds_intersect_2() public withSMATest { uint48 start1 = uint48(10); uint48 end1 = uint48(20); @@ -183,7 +183,7 @@ contract ValidationIntersectionTest is AccountTestBase { assertEq(returnedValidationData, _packValidationRes(address(0), start2, end1)); } - function test_validationIntersect_revert_unexpectedAuthorizer() public { + function test_validationIntersect_revert_unexpectedAuthorizer() public withSMATest { address badAuthorizer = makeAddr("badAuthorizer"); oneHookModule.setValidationData( @@ -209,7 +209,7 @@ contract ValidationIntersectionTest is AccountTestBase { account1.validateUserOp(userOp, uoHash, 1 wei); } - function test_validationIntersect_validAuthorizer() public { + function test_validationIntersect_validAuthorizer() public withSMATest { address goodAuthorizer = makeAddr("goodAuthorizer"); oneHookModule.setValidationData( @@ -228,7 +228,7 @@ contract ValidationIntersectionTest is AccountTestBase { assertEq(address(uint160(returnedValidationData)), goodAuthorizer); } - function test_validationIntersect_authorizerAndTimeRange() public { + function test_validationIntersect_authorizerAndTimeRange() public withSMATest { uint48 start1 = uint48(10); uint48 end1 = uint48(20); @@ -251,7 +251,7 @@ contract ValidationIntersectionTest is AccountTestBase { assertEq(returnedValidationData, _packValidationRes(goodAuthorizer, start2, end1)); } - function test_validationIntersect_multiplePreValidationHooksIntersect() public { + function test_validationIntersect_multiplePreValidationHooksIntersect() public withSMATest { uint48 start1 = uint48(10); uint48 end1 = uint48(20); @@ -275,7 +275,7 @@ contract ValidationIntersectionTest is AccountTestBase { assertEq(returnedValidationData, _packValidationRes(address(0), start2, end1)); } - function test_validationIntersect_multiplePreValidationHooksSigFail() public { + function test_validationIntersect_multiplePreValidationHooksSigFail() public withSMATest { twoHookModule.setValidationData( 0, // returns OK 0, // returns OK diff --git a/test/modules/AllowlistModule.t.sol b/test/modules/AllowlistModule.t.sol index 4e4902d3..eee8cd02 100644 --- a/test/modules/AllowlistModule.t.sol +++ b/test/modules/AllowlistModule.t.sol @@ -32,7 +32,7 @@ contract AllowlistModuleTest is CustomValidationTestBase { uint32 indexed entityId, address indexed account, bytes24 indexed targetAndSelector, bool allowed ); - function setUp() public { + function setUp() public override { _signerValidation = ModuleEntityLib.pack(address(ecdsaValidationModule), TEST_DEFAULT_VALIDATION_ENTITY_ID); allowlistModule = new AllowlistModule(); diff --git a/test/modules/ERC20TokenLimitModule.t.sol b/test/modules/ERC20TokenLimitModule.t.sol index 01aecb78..88aa8d6c 100644 --- a/test/modules/ERC20TokenLimitModule.t.sol +++ b/test/modules/ERC20TokenLimitModule.t.sol @@ -18,9 +18,10 @@ import {ERC20TokenLimitModule} from "../../src/modules/permissions/ERC20TokenLim import {MockModule} from "../mocks/modules/MockModule.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {CODELESS_ADDRESS} from "../utils/TestConstants.sol"; contract ERC20TokenLimitModuleTest is AccountTestBase { - address public recipient = address(1); + address public recipient = CODELESS_ADDRESS; MockERC20 public erc20; address payable public bundler = payable(address(2)); ExecutionManifest internal _m; @@ -31,7 +32,7 @@ contract ERC20TokenLimitModuleTest is AccountTestBase { ERC20TokenLimitModule public module = new ERC20TokenLimitModule(); uint256 public spendLimit = 10 ether; - function setUp() public { + function setUp() public override { // Set up a validator with hooks from the erc20 spend limit module attached acct = factory.createAccount(address(this), 0, 0); @@ -75,7 +76,7 @@ contract ERC20TokenLimitModuleTest is AccountTestBase { ); } - function test_userOp_executeLimit() public { + function test_userOp_executeLimit() public withSMATest { vm.startPrank(address(entryPoint)); (, uint256 limit) = module.limits(0, address(erc20), address(acct)); @@ -85,9 +86,10 @@ contract ERC20TokenLimitModuleTest is AccountTestBase { (, limit) = module.limits(0, address(erc20), address(acct)); assertEq(limit, 5 ether); + vm.stopPrank(); } - function test_userOp_executeBatchLimit() public { + function test_userOp_executeBatchLimit() public withSMATest { Call[] memory calls = new Call[](3); calls[0] = Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.transfer, (recipient, 1 wei))}); @@ -106,9 +108,10 @@ contract ERC20TokenLimitModuleTest is AccountTestBase { (, limit) = module.limits(0, address(erc20), address(acct)); assertEq(limit, 10 ether - 6 ether - 100_001); + vm.stopPrank(); } - function test_userOp_executeBatch_approveAndTransferLimit() public { + function test_userOp_executeBatch_approveAndTransferLimit() public withSMATest { Call[] memory calls = new Call[](3); calls[0] = Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.approve, (recipient, 1 wei))}); @@ -127,9 +130,10 @@ contract ERC20TokenLimitModuleTest is AccountTestBase { (, limit) = module.limits(0, address(erc20), address(acct)); assertEq(limit, 10 ether - 6 ether - 100_001); + vm.stopPrank(); } - function test_userOp_executeBatch_approveAndTransferLimit_fail() public { + function test_userOp_executeBatch_approveAndTransferLimit_fail() public withSMATest { Call[] memory calls = new Call[](3); calls[0] = Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.approve, (recipient, 1 wei))}); @@ -151,9 +155,10 @@ contract ERC20TokenLimitModuleTest is AccountTestBase { (, limit) = module.limits(0, address(erc20), address(acct)); assertEq(limit, 10 ether); + vm.stopPrank(); } - function test_runtime_executeLimit() public { + function test_runtime_executeLimit() public withSMATest { (, uint256 limit) = module.limits(0, address(erc20), address(acct)); assertEq(limit, 10 ether); acct.executeWithRuntimeValidation( @@ -165,7 +170,7 @@ contract ERC20TokenLimitModuleTest is AccountTestBase { assertEq(limit, 5 ether); } - function test_runtime_executeBatchLimit() public { + function test_runtime_executeBatchLimit() public withSMATest { Call[] memory calls = new Call[](3); calls[0] = Call({target: address(erc20), value: 0, data: abi.encodeCall(IERC20.approve, (recipient, 1 wei))}); diff --git a/test/modules/PaymasterGuardModule.t.sol b/test/modules/PaymasterGuardModule.t.sol index 09d03dfe..57eaa19d 100644 --- a/test/modules/PaymasterGuardModule.t.sol +++ b/test/modules/PaymasterGuardModule.t.sol @@ -15,27 +15,27 @@ contract PaymasterGuardModuleTest is AccountTestBase { address public paymaster2; uint32 public constant ENTITY_ID = 1; - function setUp() public { + function setUp() public override { account = payable(makeAddr("account")); paymaster1 = payable(makeAddr("paymaster1")); paymaster2 = payable(makeAddr("paymaster2")); } - function test_onInstall() public { + function test_onInstall() public withSMATest { vm.startPrank(address(account)); module.onInstall(abi.encode(ENTITY_ID, paymaster1)); - assertEq(paymaster1, module.payamsters(ENTITY_ID, account)); + assertEq(paymaster1, module.paymasters(ENTITY_ID, account)); } - function test_onUinstall() public { + function test_onUninstall() public withSMATest { vm.startPrank(address(account)); module.onUninstall(abi.encode(ENTITY_ID)); - assertEq(address(0), module.payamsters(ENTITY_ID, account)); + assertEq(address(0), module.paymasters(ENTITY_ID, account)); } - function test_preUserOpValidationHook_success() public { + function test_preUserOpValidationHook_success() public withSMATest { PackedUserOperation memory uo = _packUO(abi.encodePacked(paymaster1, "")); vm.startPrank(address(account)); @@ -46,7 +46,7 @@ contract PaymasterGuardModuleTest is AccountTestBase { assertEq(res, 0); } - function test_preUserOpValidationHook_failWithInvalidData() public { + function test_preUserOpValidationHook_failWithInvalidData() public withSMATest { PackedUserOperation memory uo = _packUO(""); vm.startPrank(address(account)); @@ -56,7 +56,7 @@ contract PaymasterGuardModuleTest is AccountTestBase { module.preUserOpValidationHook(ENTITY_ID, uo, ""); } - function test_preUserOpValidationHook_fail() public { + function test_preUserOpValidationHook_fail() public withSMATest { PackedUserOperation memory uo = _packUO(abi.encodePacked(paymaster1, "")); vm.startPrank(address(account)); @@ -67,7 +67,7 @@ contract PaymasterGuardModuleTest is AccountTestBase { module.preUserOpValidationHook(ENTITY_ID, uo, ""); } - function test_preRuntimeValidationHook_success() public { + function test_preRuntimeValidationHook_success() public withSMATest { vm.startPrank(address(account)); module.preRuntimeValidationHook(ENTITY_ID, address(0), 0, "", ""); diff --git a/test/modules/TimeRangeModule.t.sol b/test/modules/TimeRangeModule.t.sol index 4926ec6e..d95f6096 100644 --- a/test/modules/TimeRangeModule.t.sol +++ b/test/modules/TimeRangeModule.t.sol @@ -26,7 +26,7 @@ contract TimeRangeModuleTest is CustomValidationTestBase { uint48 public validUntil; uint48 public validAfter; - function setUp() public { + function setUp() public override { _signerValidation = ModuleEntityLib.pack(address(ecdsaValidationModule), TEST_DEFAULT_VALIDATION_ENTITY_ID); timeRangeModule = new TimeRangeModule(); @@ -38,7 +38,7 @@ contract TimeRangeModuleTest is CustomValidationTestBase { assertEq(timeRangeModule.moduleId(), "alchemy.timerange-module.0.0.1"); } - function test_timeRangeModule_install() public { + function test_timeRangeModule_install() public withSMATest { validUntil = 1000; validAfter = 100; @@ -64,7 +64,7 @@ contract TimeRangeModuleTest is CustomValidationTestBase { assertEq(retrievedValidAfter, validAfter); } - function test_timeRangeModule_uninstall() public { + function test_timeRangeModule_uninstall() public withSMATest { test_timeRangeModule_install(); // Uninstall the module @@ -73,8 +73,7 @@ contract TimeRangeModuleTest is CustomValidationTestBase { vm.expectCall({ callee: address(timeRangeModule), - data: abi.encodeCall(TimeRangeModule.onUninstall, (hookUninstallDatas[0])), - count: 1 + data: abi.encodeCall(TimeRangeModule.onUninstall, (hookUninstallDatas[0])) }); vm.prank(address(account1)); account1.uninstallValidation(_signerValidation, "", hookUninstallDatas); @@ -154,7 +153,7 @@ contract TimeRangeModuleTest is CustomValidationTestBase { ); } - function test_timeRangeModule_runtime_before() public { + function test_timeRangeModule_runtime_before() public withSMATest { validUntil = 1000; validAfter = 100; @@ -178,7 +177,7 @@ contract TimeRangeModuleTest is CustomValidationTestBase { ); } - function test_timeRangeModule_runtime_during() public { + function test_timeRangeModule_runtime_during() public withSMATest { validUntil = 1000; validAfter = 100; @@ -187,7 +186,7 @@ contract TimeRangeModuleTest is CustomValidationTestBase { // Attempt during the valid time range, expect success vm.warp(101); - vm.expectCall({callee: makeAddr("recipient"), msgValue: 0 wei, data: "", count: 1}); + vm.expectCall({callee: makeAddr("recipient"), msgValue: 0 wei, data: ""}); vm.prank(owner1); account1.executeWithRuntimeValidation( abi.encodeCall(ModularAccountBase.execute, (makeAddr("recipient"), 0 wei, "")), @@ -195,7 +194,7 @@ contract TimeRangeModuleTest is CustomValidationTestBase { ); } - function test_timeRangeModule_runtime_after() public { + function test_timeRangeModule_runtime_after() public withSMATest { validUntil = 1000; validAfter = 100; diff --git a/test/modules/WebauthnValidationModule.t.sol b/test/modules/WebauthnValidationModule.t.sol index e23eb27d..8b5a713c 100644 --- a/test/modules/WebauthnValidationModule.t.sol +++ b/test/modules/WebauthnValidationModule.t.sol @@ -4,8 +4,6 @@ pragma solidity ^0.8.26; import {ModuleEntityLib} from "@erc6900/reference-implementation/helpers/ModuleEntityLib.sol"; import {ValidationConfigLib} from "@erc6900/reference-implementation/helpers/ValidationConfigLib.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; - -import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {WebAuthn} from "webauthn-sol/src/WebAuthn.sol"; import {Utils, WebAuthnInfo} from "webauthn-sol/test/Utils.sol"; @@ -14,6 +12,7 @@ import {ModularAccount} from "../../src/account/ModularAccount.sol"; import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol"; import {WebauthnValidationModule} from "../../src/modules/validation/WebauthnValidationModule.sol"; import {AccountTestBase} from "../utils/AccountTestBase.sol"; +import {CODELESS_ADDRESS} from "../utils/TestConstants.sol"; contract WebauthnValidationModuleTest is AccountTestBase { using MessageHashUtils for bytes32; @@ -32,10 +31,11 @@ contract WebauthnValidationModuleTest is AccountTestBase { uint256 internal constant _SIG_VALIDATION_PASSED = 0; uint256 internal constant _SIG_VALIDATION_FAILED = 1; - function setUp() external { + function setUp() public override { module = new WebauthnValidationModule(); - account = payable(address(new ERC1967Proxy(address(new ModularAccount(entryPoint)), ""))); - ModularAccount(payable(account)).initializeWithValidation( + account = payable(account1); + vm.prank(address(entryPoint)); + ModularAccount(account).installValidation( ValidationConfigLib.pack(address(module), entityId, true, true, true), new bytes4[](0), abi.encode(entityId, x, y), @@ -53,7 +53,7 @@ contract WebauthnValidationModuleTest is AccountTestBase { ); } - function test_isValidSignature_shouldFail(bytes32 message, uint256 sigR, uint256 sigS) external view { + function test_fail_isValidSignature(bytes32 message, uint256 sigR, uint256 sigS) external view { bytes32 challenge = module.replaySafeHash(account, message); // make sure r, s values isn't the right one by accident. checking 1 should be enough @@ -93,22 +93,22 @@ contract WebauthnValidationModuleTest is AccountTestBase { ); } - function test_uoValidation() external { + function test_uoValidation() external withSMATest { PackedUserOperation memory uo; uo.sender = account; - uo.callData = abi.encodeCall(ModularAccountBase.execute, (address(0), 0, new bytes(0))); + uo.callData = abi.encodeCall(ModularAccountBase.execute, (CODELESS_ADDRESS, 0, new bytes(0))); bytes32 uoHash = entryPoint.getUserOpHash(uo); uo.signature = _getUOSigForChallenge(uoHash.toEthSignedMessageHash(), 0, 0); - vm.startPrank(address(entryPoint)); + vm.prank(address(entryPoint)); assertEq(ModularAccountBase(account).validateUserOp(uo, uoHash, 0), _SIG_VALIDATION_PASSED); } function test_uoValidaton_shouldFail(uint256 sigR, uint256 sigS) external { PackedUserOperation memory uo; uo.sender = account; - uo.callData = abi.encodeCall(ModularAccountBase.execute, (address(0), 0, new bytes(0))); + uo.callData = abi.encodeCall(ModularAccountBase.execute, (CODELESS_ADDRESS, 0, new bytes(0))); bytes32 uoHash = entryPoint.getUserOpHash(uo); // make sure r, s values isn't the right one by accident. checking 1 should be enough @@ -120,7 +120,7 @@ contract WebauthnValidationModuleTest is AccountTestBase { vm.assume(sigR != 0); // because we special case r=0 and s=0 in the helper function uo.signature = _getUOSigForChallenge(uoHash.toEthSignedMessageHash(), sigR, sigS); - vm.startPrank(address(entryPoint)); + vm.prank(address(entryPoint)); assertEq(ModularAccountBase(account).validateUserOp(uo, uoHash, 0), _SIG_VALIDATION_FAILED); } diff --git a/test/utils/AccountTestBase.sol b/test/utils/AccountTestBase.sol index 89c24c93..f60aa33d 100644 --- a/test/utils/AccountTestBase.sol +++ b/test/utils/AccountTestBase.sol @@ -1,16 +1,16 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.26; +import {Call, IModularAccount} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol"; import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; -import {Call, IModularAccount} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; - import {ModularAccount} from "../../src/account/ModularAccount.sol"; import {SemiModularAccountBytecode} from "../../src/account/SemiModularAccountBytecode.sol"; import {AccountFactory} from "../../src/factory/AccountFactory.sol"; import {DIRECT_CALL_VALIDATION_ENTITYID} from "../../src/helpers/Constants.sol"; +import {FALLBACK_VALIDATION} from "../../src/helpers/Constants.sol"; import {ModuleEntity, ModuleEntityLib} from "../../src/libraries/ModuleEntityLib.sol"; import {ValidationConfigLib} from "../../src/libraries/ValidationConfigLib.sol"; import {ECDSAValidationModule} from "../../src/modules/validation/ECDSAValidationModule.sol"; @@ -40,7 +40,10 @@ abstract contract AccountTestBase is OptimizedTest, ModuleSignatureUtils { uint256 public owner1Key; ModularAccount public account1; + bool internal _isSMATest; + ModuleEntity internal _signerValidation; + uint256 internal _revertSnapshot; // Re-declare the constant to prevent derived test contracts from having to import it uint32 public constant TEST_DEFAULT_VALIDATION_ENTITY_ID = EXT_CONST_TEST_DEFAULT_VALIDATION_ENTITY_ID; @@ -48,18 +51,38 @@ abstract contract AccountTestBase is OptimizedTest, ModuleSignatureUtils { uint256 public constant CALL_GAS_LIMIT = 100_000; uint256 public constant VERIFICATION_GAS_LIMIT = 1_200_000; + function setUp() public virtual { + // Intentionally left blank + // This should be overriden when needed and will be called again by the `withSMATest` modifier. + } + + modifier withSMATest() { + _; + + vm.revertTo(_revertSnapshot); + + _isSMATest = true; + account1 = ModularAccount(payable(factory.createSemiModularAccount(owner1, 0))); + vm.deal(address(account1), 100 ether); + _signerValidation = FALLBACK_VALIDATION; + + setUp(); + + _; + } + constructor() { entryPoint = _deployEntryPoint070(); (owner1, owner1Key) = makeAddrAndKey("owner1"); factoryOwner = makeAddr("factoryOwner"); beneficiary = payable(makeAddr("beneficiary")); - address deployedECDSAValidationModule = address(_deployECDSAValidationModule()); + // address deployedECDSAValidationModule = address(_deployECDSAValidationModule()); // We etch the single signer validation to the max address, such that it coincides with the fallback // validation module entity for semi modular account tests. - ecdsaValidationModule = ECDSAValidationModule(address(type(uint160).max)); - vm.etch(address(ecdsaValidationModule), deployedECDSAValidationModule.code); + ecdsaValidationModule = _deployECDSAValidationModule(); + // vm.etch(address(0), deployedECDSAValidationModule.code); accountImplementation = _deployModularAccount(entryPoint); @@ -74,15 +97,13 @@ abstract contract AccountTestBase is OptimizedTest, ModuleSignatureUtils { factoryOwner ); - if (vm.envOr("SMA_TEST", false)) { - account1 = ModularAccount(payable(factory.createSemiModularAccount(owner1, 0))); - } else { - account1 = factory.createAccount(owner1, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID); - } + account1 = factory.createAccount(owner1, 0, TEST_DEFAULT_VALIDATION_ENTITY_ID); vm.deal(address(account1), 100 ether); _signerValidation = ModuleEntityLib.pack(address(ecdsaValidationModule), TEST_DEFAULT_VALIDATION_ENTITY_ID); + + _revertSnapshot = vm.snapshot(); } function _runExecUserOp(address target, bytes memory callData) internal { @@ -194,13 +215,15 @@ abstract contract AccountTestBase is OptimizedTest, ModuleSignatureUtils { function _transferOwnershipToTest() internal { // Transfer ownership to test contract for easier invocation. vm.prank(owner1); - if (vm.envOr("SMA_TEST", false)) { + + if (_isSMATest) { account1.executeWithRuntimeValidation( abi.encodeCall(SemiModularAccountBytecode(payable(account1)).updateFallbackSigner, (address(this))), _encodeSignature(_signerValidation, GLOBAL_VALIDATION, "") ); return; } + account1.executeWithRuntimeValidation( abi.encodeCall( account1.execute, diff --git a/test/utils/CustomValidationTestBase.sol b/test/utils/CustomValidationTestBase.sol index 42b0ab61..8806841c 100644 --- a/test/utils/CustomValidationTestBase.sol +++ b/test/utils/CustomValidationTestBase.sol @@ -4,9 +4,8 @@ pragma solidity ^0.8.26; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import {ModularAccount} from "../../src/account/ModularAccount.sol"; -import {ModuleEntity} from "../../src/libraries/ModuleEntityLib.sol"; +import {ModuleEntity, ModuleEntityLib} from "../../src/libraries/ModuleEntityLib.sol"; import {ValidationConfigLib} from "../../src/libraries/ValidationConfigLib.sol"; - import {AccountTestBase} from "./AccountTestBase.sol"; /// @dev This test contract base is used to test custom validation logic. @@ -14,6 +13,8 @@ import {AccountTestBase} from "./AccountTestBase.sol"; /// Then, call _customValidationSetup in the test setup. /// Make sure to do so after any state variables that `_initialValidationConfig` relies on are set. abstract contract CustomValidationTestBase is AccountTestBase { + using ModuleEntityLib for ModuleEntity; + function _customValidationSetup() internal { ( ModuleEntity validationFunction, @@ -25,13 +26,17 @@ abstract contract CustomValidationTestBase is AccountTestBase { bytes[] memory hooks ) = _initialValidationConfig(); - account1 = ModularAccount(payable(new ERC1967Proxy{salt: 0}(address(accountImplementation), ""))); - - _beforeInstallStep(address(account1)); - - if (vm.envOr("SMA_TEST", false)) { - vm.prank(address(entryPoint)); + if (_isSMATest) { + // Short circuit because you cannot install hooks to the fallback validation. + // if (validationFunction.eq(FALLBACK_VALIDATION)) { + // revert("the fuck"); + // return; + // } + account1 = + ModularAccount(payable(new ERC1967Proxy{salt: 0}(address(semiModularAccountImplementation), ""))); + _beforeInstallStep(address(account1)); // The initializer doesn't work on the SMA + vm.prank(address(entryPoint)); account1.installValidation( ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation, isUserOpValidation), selectors, @@ -39,6 +44,8 @@ abstract contract CustomValidationTestBase is AccountTestBase { hooks ); } else { + account1 = ModularAccount(payable(new ERC1967Proxy{salt: 0}(address(accountImplementation), ""))); + _beforeInstallStep(address(account1)); account1.initializeWithValidation( ValidationConfigLib.pack(validationFunction, isGlobal, isSignatureValidation, isUserOpValidation), selectors, diff --git a/test/utils/ModuleSignatureUtils.sol b/test/utils/ModuleSignatureUtils.sol index 4f841e2e..14ffaed6 100644 --- a/test/utils/ModuleSignatureUtils.sol +++ b/test/utils/ModuleSignatureUtils.sol @@ -115,7 +115,7 @@ contract ModuleSignatureUtils { function _buildFullDeferredInstallSig( Vm vm, uint256 ownerKey, - bool isSmaTest, + bool isSMATest, ModularAccount account, ModuleEntity outerECDSAValidation, ModuleEntity deferredValidation, @@ -140,7 +140,7 @@ contract ModuleSignatureUtils { bytes memory deferredInstallSig = _getDeferredInstallSig( vm, ownerKey, - isSmaTest, + isSMATest, account, outerECDSAValidation, deferredConfig, @@ -165,7 +165,7 @@ contract ModuleSignatureUtils { } function _getReplaySafeHash( - bool isSmaTest, + bool isSMATest, ModularAccount account, ModuleEntity outerECDSAValidation, ValidationConfig deferredConfig, @@ -197,7 +197,7 @@ contract ModuleSignatureUtils { (address outerECDSAValidationAddr,) = outerECDSAValidation.unpack(); - bytes32 replaySafeHash = isSmaTest + bytes32 replaySafeHash = isSMATest ? _getSmaReplaySafeHash(account, typedDataHash) : ECDSAValidationModule(outerECDSAValidationAddr).replaySafeHash(address(account), typedDataHash); @@ -230,7 +230,7 @@ contract ModuleSignatureUtils { function _getDeferredInstallSig( Vm vm, uint256 ownerKey, - bool isSmaTest, + bool isSMATest, ModularAccount account, ModuleEntity outerECDSAValidation, ValidationConfig deferredConfig, @@ -239,7 +239,7 @@ contract ModuleSignatureUtils { uint48 deadline ) internal view returns (bytes memory) { bytes32 replaySafeHash = _getReplaySafeHash( - isSmaTest, + isSMATest, account, outerECDSAValidation, deferredConfig, diff --git a/test/utils/TestConstants.sol b/test/utils/TestConstants.sol index 2cd8c2cf..ea890497 100644 --- a/test/utils/TestConstants.sol +++ b/test/utils/TestConstants.sol @@ -1,4 +1,6 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.26; -uint32 constant TEST_DEFAULT_VALIDATION_ENTITY_ID = type(uint32).max; +uint32 constant TEST_DEFAULT_VALIDATION_ENTITY_ID = 0; //type(uint32).max; + +address constant CODELESS_ADDRESS = address(0x4546b);