Feat/clean refactor

contracts/Nexus.sol

@@ -207,7 +207,6 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U
         else if (moduleTypeId == MODULE_TYPE_EXECUTOR) _uninstallExecutor(module, deInitData);
         else if (moduleTypeId == MODULE_TYPE_FALLBACK) _uninstallFallbackHandler(module, deInitData);
         else if (moduleTypeId == MODULE_TYPE_HOOK) _uninstallHook(module, deInitData);
-        else revert UnsupportedModuleType(moduleTypeId);
     }
 
     function initializeAccount(bytes calldata initData) external payable virtual {
@@ -512,7 +511,7 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U
     function _isModuleInstalled(uint256 moduleTypeId, address module, bytes calldata additionalContext) private view returns (bool) {
         if (moduleTypeId == MODULE_TYPE_VALIDATOR) return _isValidatorInstalled(module);
         else if (moduleTypeId == MODULE_TYPE_EXECUTOR) return _isExecutorInstalled(module);
-        else if (moduleTypeId == MODULE_TYPE_FALLBACK) return _isFallbackHandlerInstalled(abi.decode(additionalContext, (bytes4)), module);
+        else if (moduleTypeId == MODULE_TYPE_FALLBACK) return _isFallbackHandlerInstalled(bytes4(additionalContext[0:4]), module);
         else if (moduleTypeId == MODULE_TYPE_HOOK) return _isHookInstalled(module);
         else return false;
     }

contracts/base/ExecutionHelper.sol

@@ -60,7 +60,7 @@ contract ExecutionHelper is IExecutionHelperEventsAndErrors {
         assembly {
             result := mload(0x40)
             calldatacopy(result, callData.offset, callData.length)
-            success := iszero(call(gas(), target, value, result, callData.length, codesize(), 0x00))
+            success := call(gas(), target, value, result, callData.length, codesize(), 0x00)
             mstore(result, returndatasize()) // Store the length.
             let o := add(result, 0x20)
             returndatacopy(o, 0x00, returndatasize()) // Copy the returndata.

contracts/base/ModuleManager.sol

@@ -225,11 +225,8 @@ contract ModuleManager is Storage, Receiver, IModuleManagerEventsAndErrors {
     /// @param fallbackHandler The address of the fallback handler to uninstall.
     /// @param data The de-initialization data containing the selector.
     function _uninstallFallbackHandler(address fallbackHandler, bytes calldata data) internal virtual {
-        bytes4 selector = bytes4(data[0:4]);
-        bytes memory deInitData = data[4:];
-        if (!_isFallbackHandlerInstalled(selector)) revert FallbackNotInstalledForSelector(selector);
-        _getAccountStorage().fallbacks[selector] = FallbackHandler(address(0), CallType.wrap(0x00));
-        IFallback(fallbackHandler).onUninstall(deInitData);
+        _getAccountStorage().fallbacks[bytes4(data[0:4])] = FallbackHandler(address(0), CallType.wrap(0x00));
+        IFallback(fallbackHandler).onUninstall(data[4:]);
     }
 
     /// @dev Checks if a fallback handler is set for a given selector.

contracts/mocks/MockExecutor.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.23;
+pragma solidity ^0.8.24;
 
 import { IModule } from "contracts/interfaces/modules/IModule.sol";
 import { EncodedModuleTypes } from "contracts/lib/ModuleTypeLib.sol";

contracts/mocks/MockHandler.sol

@@ -1,14 +1,12 @@
 // SPDX-License-Identifier: LGPL-3.0-only
-pragma solidity ^0.8.23;
+pragma solidity ^0.8.24;
 
-import { IERC721Receiver } from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
-import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
-import { IModule } from "contracts/interfaces/modules/IModule.sol";
 import { IFallback } from "contracts/interfaces/modules/IFallback.sol";
-import { EncodedModuleTypes } from "contracts/lib/ModuleTypeLib.sol";
 import { MODULE_TYPE_FALLBACK } from "../../contracts/types/Constants.sol";
 
 contract MockHandler is IFallback {
+
+    uint256 public count;
     string public constant NAME = "Default Handler";
     string public constant VERSION = "1.0.0";
 
@@ -37,4 +35,40 @@ contract MockHandler is IFallback {
     function isInitialized(address) external pure override returns (bool) {
         return false;
     }
+
+    function staticFunction() external pure returns (bytes32) {
+        return keccak256("STATIC_CALL");
+    }
+
+    function stateChangingFunction() external {
+        count++;
+    }
+
+    function singleFunction() external pure returns (bytes32) {
+        return keccak256("SINGLE_CALL");
+    }
+
+    function batchFunction() external pure returns (bytes32) {
+        return keccak256("BATCH_CALL");
+    }
+
+    function revertingStaticFunction() external pure {
+        require(false, "Static call revert reason");
+    }
+
+    function revertingSingleFunction() external pure {
+        require(false, "Single call revert reason");
+    }
+
+    function gasIntensiveFunction() external {
+        while (true) {}
+    }
+
+    function dynamicFunction() external pure returns (bytes32) {
+        return keccak256("DYNAMIC_CALL");
+    }
+
+    function getState() external view returns (uint256) {
+        return count;
+    }
 }

contracts/mocks/MockHook.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.23;
+pragma solidity ^0.8.24;
 
 import { IModule } from "contracts/interfaces/modules/IModule.sol";
 import { EncodedModuleTypes } from "contracts/lib/ModuleTypeLib.sol";

contracts/modules/validators/K1Validator.sol

@@ -82,8 +82,4 @@ contract K1Validator is IValidator {
     function isModuleType(uint256 typeID) external pure returns (bool) {
         return typeID == MODULE_TYPE_VALIDATOR;
     }
-
-    function test() public pure {
-        // TODO To be removed: This function is used to ignore file in coverage report
-    }
 }

contracts/utils/BootstrapUtil.sol

@@ -1,8 +1,7 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.23;
+pragma solidity ^0.8.24;
 
-import { Bootstrap, BootstrapConfig } from "./Bootstrap.sol";
-import { IModule } from "../interfaces/modules/IModule.sol";
+import { BootstrapConfig } from "./Bootstrap.sol";
 
 // Review: can make this a library?
 contract BootstrapUtil {

test/foundry/integration/UpgradeSmartAccountTest.t.sol

@@ -42,4 +42,15 @@ contract UpgradeSmartAccountTest is NexusTest_Base {
         test_currentImplementationAddress();
         test_upgradeImplementation();
     }
+
+    /// @notice Tests the entire upgrade process
+    function test_RevertIf_AccessUnauthorized_upgradeSmartAccount() public {
+        test_proxiableUUIDSlot();
+        test_currentImplementationAddress();
+
+        Nexus newSmartAccount = new Nexus();
+
+        vm.expectRevert(abi.encodeWithSelector(AccountAccessUnauthorized.selector));
+        BOB_ACCOUNT.upgradeToAndCall(address(newSmartAccount), "");
+    }
 }

test/foundry/unit/concrete/accountexecution/TestAccountExecution_ExecuteFromExecutor.t.sol

@@ -348,4 +348,110 @@ contract TestAccountExecution_ExecuteFromExecutor is TestAccountExecution_Base {
             );
         }
     }
+
+
+    /// @notice Tests a batch execution with one failing operation.
+    function test_TryExecuteBatch_SingleFailure() public {
+        // Verify initial state
+        assertEq(counter.getNumber(), 0, "Counter should start at 0");
+
+        // Prepare batch execution with one failing operation
+        Execution[] memory executions = new Execution[](2);
+        executions[0] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.incrementNumber.selector));
+        executions[1] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.revertOperation.selector));
+
+        // Expect the TryExecuteUnsuccessful event to be emitted
+        vm.expectEmit(true, true, true, true);
+        emit TryExecuteUnsuccessful(1, abi.encodeWithSignature("Error(string)", "Counter: Revert operation"));
+
+        // Execute batch operation via MockExecutor
+        mockExecutor.tryExecuteBatchViaAccount(BOB_ACCOUNT, executions);
+
+        // Verify the counter state
+        assertEq(counter.getNumber(), 1, "Counter should have been incremented once");
+    }
+
+    /// @notice Tests a batch execution with one failing operation using prank.
+    function test_TryExecuteBatch_SingleFailure_WithPrank() public {
+        // Verify initial state
+        assertEq(counter.getNumber(), 0, "Counter should start at 0");
+
+        // Prepare batch execution with one failing operation
+        Execution[] memory executions = new Execution[](2);
+        executions[0] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.incrementNumber.selector));
+        executions[1] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.revertOperation.selector));
+
+        // Expect the TryExecuteUnsuccessful event to be emitted
+        vm.expectEmit(true, true, true, true);
+        emit TryExecuteUnsuccessful(1, abi.encodeWithSignature("Error(string)", "Counter: Revert operation"));
+
+        // Prank and execute batch operation via BOB_ACCOUNT
+        prank(address(mockExecutor));
+        BOB_ACCOUNT.executeFromExecutor(ModeLib.encodeTryBatch(), ExecLib.encodeBatch(executions));
+
+        // Verify the counter state
+        assertEq(counter.getNumber(), 1, "Counter should have been incremented once");
+    }
+
+    /// @notice Tests a batch execution with multiple failing operations.
+    function test_TryExecuteBatch_MultipleFailures() public {
+        // Verify initial state
+        assertEq(counter.getNumber(), 0, "Counter should start at 0");
+
+        // Prepare batch execution with multiple failing operations
+        Execution[] memory executions = new Execution[](3);
+        executions[0] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.revertOperation.selector));
+        executions[1] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.revertOperation.selector));
+        executions[2] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.incrementNumber.selector));
+
+        // Expect the TryExecuteUnsuccessful event to be emitted for each failure
+        vm.expectEmit(true, true, true, true);
+        emit TryExecuteUnsuccessful(0, abi.encodeWithSignature("Error(string)", "Counter: Revert operation"));
+        vm.expectEmit(true, true, true, true);
+        emit TryExecuteUnsuccessful(1, abi.encodeWithSignature("Error(string)", "Counter: Revert operation"));
+
+        // Execute batch operation via MockExecutor
+        mockExecutor.tryExecuteBatchViaAccount(BOB_ACCOUNT, executions);
+
+        // Verify the counter state
+        assertEq(counter.getNumber(), 1, "Counter should have been incremented once");
+    }
+
+    /// @notice Tests a batch execution with empty call data.
+    function test_TryExecuteBatch_EmptyCallData() public {
+        // Verify initial state
+        assertEq(counter.getNumber(), 0, "Counter should start at 0");
+
+        // Prepare batch execution with empty call data
+        Execution[] memory executions = new Execution[](1);
+        executions[0] = Execution(address(counter), 0, "");
+
+        // Expect the TryExecuteUnsuccessful event to be emitted
+        vm.expectEmit(true, true, true, true);
+        emit TryExecuteUnsuccessful(0, "");
+
+        // Execute batch operation via MockExecutor
+        mockExecutor.tryExecuteBatchViaAccount(BOB_ACCOUNT, executions);
+
+        // Verify the counter state remains unchanged
+        assertEq(counter.getNumber(), 0, "Counter should remain unchanged");
+    }
+
+    /// @notice Tests a batch execution with insufficient gas.
+    function test_TryExecuteBatch_InsufficientGas() public {
+        // Verify initial state
+        assertEq(counter.getNumber(), 0, "Counter should start at 0");
+
+        // Prepare batch execution with insufficient gas
+        Execution[] memory executions = new Execution[](1);
+        executions[0] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.incrementNumber.selector));
+
+        // Expect revert due to insufficient gas
+        vm.expectRevert();
+
+        // Execute batch operation with limited gas via MockExecutor
+        (bool success, ) = address(mockExecutor).call{ gas: 10000 }(
+            abi.encodeWithSelector(mockExecutor.tryExecuteBatchViaAccount.selector, BOB_ACCOUNT, executions)
+        );
+    }
 }

test/foundry/unit/concrete/accountexecution/TestAccountExecution_TryExecuteBatch.t.sol

@@ -43,12 +43,32 @@ contract TestAccountExecution_TryExecuteBatch is TestAccountExecution_Base {
         PackedUserOperation[] memory userOps = buildPackedUserOperation(BOB, BOB_ACCOUNT, EXECTYPE_TRY, executions, address(VALIDATOR_MODULE));
 
         vm.expectEmit(true, true, true, true);
-        emit TryExecuteUnsuccessful(2, "");
+        emit TryExecuteUnsuccessful(1, abi.encodeWithSignature("Error(string)", "Counter: Revert operation"));
         ENTRYPOINT.handleOps(userOps, payable(BOB.addr));
 
         assertEq(counter.getNumber(), 2, "Counter should have been incremented even after revert operation in batch execution");
     }
 
+    /// @notice Tests handling of a batch operation with one failure.
+    function test_TryExecuteBatch_RevertIf_HandleFailure_WithPrank() public {
+        assertEq(counter.getNumber(), 0, "Counter should start at 0");
+
+        // Preparing a batch execution with three operations: increment, revert, increment
+        Execution[] memory executions = new Execution[](3);
+        executions[0] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.incrementNumber.selector));
+        executions[1] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.revertOperation.selector));
+        executions[2] = Execution(address(counter), 0, abi.encodeWithSelector(Counter.incrementNumber.selector));
+
+        // Execute batch operation
+        prank(address(BOB_ACCOUNT));
+        vm.expectEmit(true, true, true, true);
+        emit TryExecuteUnsuccessful(1, abi.encodeWithSignature("Error(string)", "Counter: Revert operation"));
+
+        BOB_ACCOUNT.execute(ModeLib.encodeTryBatch(), abi.encode(executions));
+
+        assertEq(counter.getNumber(), 2, "Counter should have been incremented even after revert operation in batch execution");
+    }
+
     /// @notice Tests handling of a batch operation with multiple failures.
     function test_TryExecuteBatch_RevertIf_HandleMultipleFailures() public {
         // Preparing a batch execution with three operations: revert, zero address, empty calldata
@@ -61,9 +81,7 @@ contract TestAccountExecution_TryExecuteBatch is TestAccountExecution_Base {
         PackedUserOperation[] memory userOps = buildPackedUserOperation(BOB, BOB_ACCOUNT, EXECTYPE_TRY, executions, address(VALIDATOR_MODULE));
 
         vm.expectEmit(true, true, true, true);
-        emit TryExecuteUnsuccessful(1, "");
-        vm.expectEmit(true, true, true, true);
-        emit TryExecuteUnsuccessful(2, "");
+        emit TryExecuteUnsuccessful(0, abi.encodeWithSignature("Error(string)", "Counter: Revert operation"));
         ENTRYPOINT.handleOps(userOps, payable(BOB.addr));
     }
 

test/foundry/unit/concrete/erc4337account/TestERC4337Account_AddDeposit.t.sol

@@ -99,4 +99,11 @@ contract TestERC4337Account_AddDeposit is NexusTest_Base {
             defaultMaxPercentDelta
         );
     }
+
+    /// @notice Tests that the addDeposit function reverts if calling the wrong entryPoint.
+    function test_AddDeposit_RevertIf_WrongEntryPoint() public {
+        vm.etch(address(0x0000000071727De22E5E9d8BAf0edAc6f37da032), address(VALIDATOR_MODULE).code);
+        vm.expectRevert();
+        BOB_ACCOUNT.addDeposit{ value: defaultDepositAmount }();
+    }
 }

test/foundry/unit/concrete/erc4337account/TestERC4337Account_OnlyEntryPointOrSelf.t.sol

@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.24;
 
-import "../../../utils/Imports.sol";
 import "../../../utils/NexusTest_Base.t.sol";
 
 /// @title TestERC4337Account_OnlyEntryPointOrSelf
@@ -22,7 +21,7 @@ contract TestERC4337Account_OnlyEntryPointOrSelf is NexusTest_Base {
     }
 
     /// @notice Tests execution of user operations from a non-EntryPoint address, expecting failure.
-    function test_ExecuteUserOp_Invalid_FromNonEntryPoint() public {
+    function test_RevertIf_ExecuteUserOp_FromNonEntryPoint() public {
         startPrank(ALICE.addr);
         Execution[] memory execution = new Execution[](1);
         execution[0] = Execution(address(BOB_ACCOUNT), 0, "");
@@ -33,6 +32,14 @@ contract TestERC4337Account_OnlyEntryPointOrSelf is NexusTest_Base {
         stopPrank();
     }
 
+    /// @notice Tests installation of a module from an unauthorized address, expecting failure.
+    function test_RevertIf_InstallModuleFromUnauthorized() public {
+        startPrank(address(ALICE_ACCOUNT));
+        vm.expectRevert(abi.encodeWithSelector(AccountAccessUnauthorized.selector));
+        BOB_ACCOUNT.installModule(MODULE_TYPE_EXECUTOR, address(EXECUTOR_MODULE), "");
+        stopPrank();
+    }
+
     /// @notice Tests installation of a module from the EntryPoint.
     function test_InstallModuleFromEntryPoint_Success() public {
         startPrank(address(ENTRYPOINT));
@@ -44,6 +51,7 @@ contract TestERC4337Account_OnlyEntryPointOrSelf is NexusTest_Base {
     function test_InstallModuleFromSelf_Success() public {
         startPrank(address(BOB_ACCOUNT));
         BOB_ACCOUNT.installModule(MODULE_TYPE_EXECUTOR, address(EXECUTOR_MODULE), "");
+        stopPrank();
     }
 
     /// @notice Tests uninstallation of a module from a non-EntryPoint or self address, expecting failure.
@@ -65,6 +73,7 @@ contract TestERC4337Account_OnlyEntryPointOrSelf is NexusTest_Base {
     function test_WithdrawDepositFromSelf_Success() public {
         startPrank(address(BOB_ACCOUNT));
         BOB_ACCOUNT.withdrawDepositTo(BOB.addr, 0.5 ether);
+        stopPrank();
     }
 
     /// @notice Tests withdrawal of deposit from an unauthorized address, expecting failure.