fix: [QS-S3] Minor gas optimizations and consistency

gas-snapshots/ModularAccount.json

@@ -1,16 +1,16 @@
 {
   "Runtime_AccountCreation": "176129",
-  "Runtime_BatchTransfers": "93045",
+  "Runtime_BatchTransfers": "93000",
   "Runtime_Erc20Transfer": "78454",
-  "Runtime_InstallSessionKey_Case1": "423189",
+  "Runtime_InstallSessionKey_Case1": "423213",
   "Runtime_NativeTransfer": "54603",
   "Runtime_UseSessionKey_Case1_Counter": "78653",
   "Runtime_UseSessionKey_Case1_Token": "111966",
-  "UserOp_BatchTransfers": "198366",
-  "UserOp_Erc20Transfer": "185331",
+  "UserOp_BatchTransfers": "198311",
+  "UserOp_Erc20Transfer": "185319",
   "UserOp_InstallSessionKey_Case1": "531396",
-  "UserOp_NativeTransfer": "161588",
-  "UserOp_UseSessionKey_Case1_Counter": "195233",
-  "UserOp_UseSessionKey_Case1_Token": "226217",
-  "UserOp_deferredValidation": "258080"
+  "UserOp_NativeTransfer": "161576",
+  "UserOp_UseSessionKey_Case1_Counter": "195221",
+  "UserOp_UseSessionKey_Case1_Token": "226205",
+  "UserOp_deferredValidation": "258068"
 }

gas-snapshots/SemiModularAccount.json

@@ -1,16 +1,16 @@
 {
   "Runtime_AccountCreation": "97767",
-  "Runtime_BatchTransfers": "89031",
+  "Runtime_BatchTransfers": "88986",
   "Runtime_Erc20Transfer": "74488",
-  "Runtime_InstallSessionKey_Case1": "421741",
+  "Runtime_InstallSessionKey_Case1": "421765",
   "Runtime_NativeTransfer": "50647",
   "Runtime_UseSessionKey_Case1_Counter": "79003",
   "Runtime_UseSessionKey_Case1_Token": "112316",
-  "UserOp_BatchTransfers": "193522",
-  "UserOp_Erc20Transfer": "180558",
-  "UserOp_InstallSessionKey_Case1": "528919",
+  "UserOp_BatchTransfers": "193491",
+  "UserOp_Erc20Transfer": "180546",
+  "UserOp_InstallSessionKey_Case1": "528931",
   "UserOp_NativeTransfer": "156833",
   "UserOp_UseSessionKey_Case1_Counter": "195473",
   "UserOp_UseSessionKey_Case1_Token": "226457",
-  "UserOp_deferredValidation": "254131"
+  "UserOp_deferredValidation": "254119"
 }

gas/modular-account/ModularAccountBenchmarkBase.sol

@@ -18,6 +18,7 @@ import {ModularAccountBase} from "../../src/account/ModularAccountBase.sol";
 import {SemiModularAccountBytecode} from "../../src/account/SemiModularAccountBytecode.sol";
 import {AccountFactory} from "../../src/factory/AccountFactory.sol";
 import {FALLBACK_VALIDATION} from "../../src/helpers/Constants.sol";
+import {ExecutionInstallDelegate} from "../../src/helpers/ExecutionInstallDelegate.sol";
 import {AllowlistModule} from "../../src/modules/permissions/AllowlistModule.sol";
 import {TimeRangeModule} from "../../src/modules/permissions/TimeRangeModule.sol";
 import {SingleSignerValidationModule} from "../../src/modules/validation/SingleSignerValidationModule.sol";
@@ -34,6 +35,7 @@ abstract contract ModularAccountBenchmarkBase is BenchmarkBase, ModuleSignatureU
     ModularAccount public accountImpl;
     SemiModularAccountBytecode public semiModularImpl;
     SingleSignerValidationModule public singleSignerValidationModule;
+    ExecutionInstallDelegate public executionInstallDelegate;
 
     AllowlistModule public allowlistModule;
     TimeRangeModule public timeRangeModule;
@@ -49,8 +51,9 @@ abstract contract ModularAccountBenchmarkBase is BenchmarkBase, ModuleSignatureU
     constructor(string memory accountImplName) BenchmarkBase(accountImplName) {
         (sessionSigner1, sessionSigner1Key) = makeAddrAndKey("session1");
 
-        accountImpl = _deployModularAccount(IEntryPoint(entryPoint));
-        semiModularImpl = _deploySemiModularAccountBytecode(IEntryPoint(entryPoint));
+        executionInstallDelegate = new ExecutionInstallDelegate();
+        accountImpl = _deployModularAccount(IEntryPoint(entryPoint), executionInstallDelegate);
+        semiModularImpl = _deploySemiModularAccountBytecode(IEntryPoint(entryPoint), executionInstallDelegate);
         singleSignerValidationModule = _deploySingleSignerValidationModule();
 
         factory = new AccountFactory(

src/account/AccountBase.sol

@@ -17,8 +17,8 @@ abstract contract AccountBase is IAccount {
 
     error NotEntryPoint();
 
-    constructor(IEntryPoint anEntryPoint) {
-        _ENTRY_POINT = anEntryPoint;
+    constructor(IEntryPoint _entryPoint) {
+        _ENTRY_POINT = _entryPoint;
     }
 
     /// @inheritdoc IAccount

src/account/ModularAccount.sol

@@ -6,14 +6,17 @@ import {
 } from "@erc6900/reference-implementation/interfaces/IModularAccount.sol";
 import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol";
 
+import {ExecutionInstallDelegate} from "../helpers/ExecutionInstallDelegate.sol";
 import {ModularAccountBase} from "./ModularAccountBase.sol";
 
 /// @title Modular Account
 /// @author Alchemy
 /// @notice This contract allows initializing with a validation config (of a validation module) to be installed on
 /// the account.
 contract ModularAccount is ModularAccountBase {
-    constructor(IEntryPoint anEntryPoint) ModularAccountBase(anEntryPoint) {}
+    constructor(IEntryPoint entryPoint, ExecutionInstallDelegate executionInstallDelegate)
+        ModularAccountBase(entryPoint, executionInstallDelegate)
+    {}
 
     /// @notice Initializes the account with a validation function.
     /// @dev This function is only callable once.

src/account/ModularAccountBase.sol

@@ -120,10 +120,12 @@ abstract contract ModularAccountBase is
         ExecutionLib.doCachedPostHooks(postHookData);
     }
 
-    constructor(IEntryPoint anEntryPoint) AccountBase(anEntryPoint) {
+    constructor(IEntryPoint entryPoint, ExecutionInstallDelegate executionInstallDelegate)
+        AccountBase(entryPoint)
+    {
         _disableInitializers();
 
-        _EXECUTION_INSTALL_DELEGATE = address(new ExecutionInstallDelegate());
+        _EXECUTION_INSTALL_DELEGATE = address(executionInstallDelegate);
     }
 
     // EXTERNAL FUNCTIONS
@@ -234,20 +236,20 @@ abstract contract ModularAccountBase is
         wrapNativeFunction
         returns (bytes[] memory results)
     {
-        bool needReturnData = (msg.sender != address(_ENTRY_POINT));
-
         uint256 callsLength = calls.length;
 
-        if (needReturnData) {
+        if (msg.sender != address(_ENTRY_POINT)) {
             results = new bytes[](callsLength);
-        }
 
-        for (uint256 i = 0; i < callsLength; ++i) {
-            ExecutionLib.callBubbleOnRevertTransient(calls[i].target, calls[i].value, calls[i].data);
+            for (uint256 i = 0; i < callsLength; ++i) {
+                ExecutionLib.callBubbleOnRevertTransient(calls[i].target, calls[i].value, calls[i].data);
 
-            if (needReturnData) {
                 results[i] = ExecutionLib.collectReturnData();
             }
+        } else {
+            for (uint256 i = 0; i < callsLength; ++i) {
+                ExecutionLib.callBubbleOnRevertTransient(calls[i].target, calls[i].value, calls[i].data);
+            }
         }
     }
 
@@ -346,6 +348,13 @@ abstract contract ModularAccountBase is
         emit DeferredActionNonceInvalidated(nonce);
     }
 
+    /// @inheritdoc IERC1271
+    function isValidSignature(bytes32 hash, bytes calldata signature) external view override returns (bytes4) {
+        ModuleEntity sigValidation = ModuleEntity.wrap(bytes24(signature));
+        signature = signature[24:];
+        return _isValidSignature(sigValidation, hash, signature);
+    }
+
     /// @inheritdoc IERC165
     /// @notice ERC-165 introspection
     /// @dev returns true for `IERC165.interfaceId` and false for `0xFFFFFFFF`
@@ -381,13 +390,6 @@ abstract contract ModularAccountBase is
         super.upgradeToAndCall(newImplementation, data);
     }
 
-    /// @inheritdoc IERC1271
-    function isValidSignature(bytes32 hash, bytes calldata signature) public view override returns (bytes4) {
-        ModuleEntity sigValidation = ModuleEntity.wrap(bytes24(signature));
-        signature = signature[24:];
-        return _isValidSignature(sigValidation, hash, signature);
-    }
-
     // INTERNAL FUNCTIONS
 
     // Parent function validateUserOp enforces that this call can only be made by the EntryPoint

src/account/ModuleManagerInternals.sol

@@ -49,10 +49,10 @@ abstract contract ModuleManagerInternals is IModularAccount {
         bytes calldata installData,
         bytes[] calldata hooks
     ) internal {
-        ValidationStorage storage _validationStorage =
-            getAccountStorage().validationStorage[validationConfig.moduleEntity()];
         ModuleEntity moduleEntity = validationConfig.moduleEntity();
 
+        ValidationStorage storage _validationStorage = getAccountStorage().validationStorage[moduleEntity];
+
         uint256 length = hooks.length;
         for (uint256 i = 0; i < length; ++i) {
             HookConfig hookConfig = HookConfig.wrap(bytes25(hooks[i][:25]));

src/account/SemiModularAccount7702.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.26;
 import {IModularAccount} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol";
 import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol";
 
+import {ExecutionInstallDelegate} from "../helpers/ExecutionInstallDelegate.sol";
 import {SemiModularAccountBase} from "./SemiModularAccountBase.sol";
 
 /// @title Semi-Modular Account for EIP-7702 EOAs
@@ -14,7 +15,9 @@ import {SemiModularAccountBase} from "./SemiModularAccountBase.sol";
 contract SemiModularAccount7702 is SemiModularAccountBase {
     error UpgradeNotAllowed();
 
-    constructor(IEntryPoint anEntryPoint) SemiModularAccountBase(anEntryPoint) {}
+    constructor(IEntryPoint entryPoint, ExecutionInstallDelegate executionInstallDelegate)
+        SemiModularAccountBase(entryPoint, executionInstallDelegate)
+    {}
 
     /// @inheritdoc IModularAccount
     function accountId() external pure override returns (string memory) {

src/account/SemiModularAccountBase.sol

@@ -10,6 +10,7 @@ import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/Messa
 import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
 
 import {FALLBACK_VALIDATION} from "../helpers/Constants.sol";
+import {ExecutionInstallDelegate} from "../helpers/ExecutionInstallDelegate.sol";
 import {SignatureType} from "../helpers/SignatureType.sol";
 import {RTCallBuffer, SigCallBuffer, UOCallBuffer} from "../libraries/ExecutionLib.sol";
 import {ModularAccountBase} from "./ModularAccountBase.sol";
@@ -46,7 +47,9 @@ abstract contract SemiModularAccountBase is ModularAccountBase {
     error FallbackSignerDisabled();
     error InvalidSignatureType();
 
-    constructor(IEntryPoint anEntryPoint) ModularAccountBase(anEntryPoint) {}
+    constructor(IEntryPoint entryPoint, ExecutionInstallDelegate executionInstallDelegate)
+        ModularAccountBase(entryPoint, executionInstallDelegate)
+    {}
 
     /// @notice Updates the fallback signer data in storage.
     /// @param fallbackSigner The new signer to set.

src/account/SemiModularAccountBytecode.sol

@@ -5,6 +5,7 @@ import {IModularAccount} from "@erc6900/reference-implementation/interfaces/IMod
 import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol";
 import {LibClone} from "solady/utils/LibClone.sol";
 
+import {ExecutionInstallDelegate} from "../helpers/ExecutionInstallDelegate.sol";
 import {SemiModularAccountBase} from "./SemiModularAccountBase.sol";
 
 /// @title Semi-Modular Account Bytecode
@@ -15,7 +16,9 @@ import {SemiModularAccountBase} from "./SemiModularAccountBase.sol";
 /// ERC1967WithImmutableArgs bytecode with a bytecode-appended address (should be encodePacked) to be used as the
 /// fallback signer.
 contract SemiModularAccountBytecode is SemiModularAccountBase {
-    constructor(IEntryPoint anEntryPoint) SemiModularAccountBase(anEntryPoint) {}
+    constructor(IEntryPoint entryPoint, ExecutionInstallDelegate executionInstallDelegate)
+        SemiModularAccountBase(entryPoint, executionInstallDelegate)
+    {}
 
     /// @inheritdoc IModularAccount
     function accountId() external pure override returns (string memory) {

src/account/SemiModularAccountStorageOnly.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.26;
 import {IModularAccount} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol";
 import {IEntryPoint} from "@eth-infinitism/account-abstraction/interfaces/IEntryPoint.sol";
 
+import {ExecutionInstallDelegate} from "../helpers/ExecutionInstallDelegate.sol";
 import {SemiModularAccountBase} from "./SemiModularAccountBase.sol";
 
 /// @title Semi-Modular Account Storage Only
@@ -14,7 +15,9 @@ import {SemiModularAccountBase} from "./SemiModularAccountBase.sol";
 /// `upgradeToAndCall()`. Use the `SemiModularAccountBytecode` instead for new accounts, this implementation should
 /// only be used for account upgrades.
 contract SemiModularAccountStorageOnly is SemiModularAccountBase {
-    constructor(IEntryPoint anEntryPoint) SemiModularAccountBase(anEntryPoint) {}
+    constructor(IEntryPoint entryPoint, ExecutionInstallDelegate executionInstallDelegate)
+        SemiModularAccountBase(entryPoint, executionInstallDelegate)
+    {}
 
     function initialize(address initialSigner) external initializer {
         SemiModularAccountStorage storage smaStorage = _getSemiModularAccountStorage();

src/libraries/ExecutionLib.sol

@@ -833,7 +833,7 @@ library ExecutionLib {
         // Run the post hooks.
         // This is tricky, unlike normal, we must traverse the data backwards, because the post exec hooks should
         // be executed in reverse order of the pre exec hooks.
-        for (uint256 i = 0; i < postHookCount; i++) {
+        for (uint256 i = 0; i < postHookCount; ++i) {
             bool success;
 
             address moduleAddress;

src/modules/permissions/AllowlistModule.sol

@@ -133,7 +133,7 @@ contract AllowlistModule is IExecutionHookModule, IValidationHookModule, ModuleB
             _decrementLimitIfApplies(entityId, token, innerCalldata);
         } else if (selector == IModularAccount.executeBatch.selector) {
             Call[] memory calls = abi.decode(callData, (Call[]));
-            for (uint256 i = 0; i < calls.length; i++) {
+            for (uint256 i = 0; i < calls.length; ++i) {
                 _decrementLimitIfApplies(entityId, calls[i].target, calls[i].data);
             }
         } else {
@@ -195,11 +195,11 @@ contract AllowlistModule is IExecutionHookModule, IValidationHookModule, ModuleB
     /// @param entityId The entity ID to initialize the allowlist for.
     /// @param inputs The allowlist inputs data to update.
     function updateAllowlist(uint32 entityId, AllowlistInput[] memory inputs) public {
-        for (uint256 i = 0; i < inputs.length; i++) {
+        for (uint256 i = 0; i < inputs.length; ++i) {
             AllowlistInput memory input = inputs[i];
             if (input.target == address(0)) {
                 // wildcard case for selectors, any address can be called for the selector
-                for (uint256 j = 0; j < input.selectors.length; j++) {
+                for (uint256 j = 0; j < input.selectors.length; ++j) {
                     setSelectorAllowlist(entityId, address(0), input.selectors[j], true);
                 }
             } else {
@@ -209,7 +209,7 @@ contract AllowlistModule is IExecutionHookModule, IValidationHookModule, ModuleB
                 updateLimits(entityId, input.target, input.hasERC20SpendLimit, input.erc20SpendLimit);
 
                 if (input.hasSelectorAllowlist) {
-                    for (uint256 j = 0; j < input.selectors.length; j++) {
+                    for (uint256 j = 0; j < input.selectors.length; ++j) {
                         setSelectorAllowlist(entityId, input.target, input.selectors[j], true);
                     }
                 }
@@ -222,19 +222,19 @@ contract AllowlistModule is IExecutionHookModule, IValidationHookModule, ModuleB
     /// @param inputs The allowlist inputs data to update.
     /// Note flag will be set to false despite passed different values.
     function deleteAllowlist(uint32 entityId, AllowlistInput[] memory inputs) public {
-        for (uint256 i = 0; i < inputs.length; i++) {
+        for (uint256 i = 0; i < inputs.length; ++i) {
             AllowlistInput memory input = inputs[i];
             if (input.target == address(0)) {
                 // wildcard case for selectors, any address can be called for the selector
-                for (uint256 j = 0; j < input.selectors.length; j++) {
+                for (uint256 j = 0; j < input.selectors.length; ++j) {
                     setSelectorAllowlist(entityId, input.target, input.selectors[j], false);
                 }
             } else {
                 setAddressAllowlist(entityId, input.target, false, false, false);
                 updateLimits(entityId, input.target, false, 0);
 
                 if (input.hasSelectorAllowlist) {
-                    for (uint256 j = 0; j < input.selectors.length; j++) {
+                    for (uint256 j = 0; j < input.selectors.length; ++j) {
                         setSelectorAllowlist(entityId, input.target, input.selectors[j], false);
                     }
                 }
@@ -289,7 +289,7 @@ contract AllowlistModule is IExecutionHookModule, IValidationHookModule, ModuleB
         } else if (bytes4(callData[:4]) == IModularAccount.executeBatch.selector) {
             Call[] memory calls = abi.decode(callData[4:], (Call[]));
 
-            for (uint256 i = 0; i < calls.length; i++) {
+            for (uint256 i = 0; i < calls.length; ++i) {
                 _checkCallPermission(entityId, msg.sender, calls[i].target, calls[i].data);
             }
         }

src/modules/permissions/NativeTokenLimitModule.sol

@@ -98,7 +98,7 @@ contract NativeTokenLimitModule is ModuleBase, IExecutionHookModule, IValidation
             (, value) = abi.decode(callData, (address, uint256));
         } else if (selector == IModularAccount.executeBatch.selector) {
             Call[] memory calls = abi.decode(callData, (Call[]));
-            for (uint256 i = 0; i < calls.length; i++) {
+            for (uint256 i = 0; i < calls.length; ++i) {
                 value += calls[i].value;
             }
         } else if (selector == ModularAccountBase.performCreate.selector) {

test/account/ModularAccount.t.sol

@@ -407,7 +407,7 @@ contract ModularAccountTest is AccountTestBase {
 
     function test_upgradeToAndCall() public withSMATest {
         vm.startPrank(address(entryPoint));
-        ModularAccount account3 = new ModularAccount(entryPoint);
+        ModularAccount account3 = new ModularAccount(entryPoint, executionInstallDelegate);
         bytes32 slot = account3.proxiableUUID();
 
         // account has impl from factory
@@ -564,12 +564,12 @@ contract ModularAccountTest is AccountTestBase {
         vm.stopPrank();
     }
 
-    function test_performCreate() public withSMATest {
+    function test_performCreate_create() public withSMATest {
         address expectedAddr = vm.computeCreateAddress(address(account1), vm.getNonce(address(account1)));
         vm.prank(address(entryPoint));
         address returnedAddr = account1.performCreate(
             0,
-            abi.encodePacked(type(ModularAccount).creationCode, abi.encode(address(entryPoint))),
+            abi.encodePacked(type(ModularAccount).creationCode, abi.encode(entryPoint, executionInstallDelegate)),
             false,
             bytes32(0)
         );
@@ -585,7 +585,7 @@ contract ModularAccountTest is AccountTestBase {
 
     function test_performCreate_create2() public withSMATest {
         bytes memory initCode =
-            abi.encodePacked(type(ModularAccount).creationCode, abi.encode(address(entryPoint)));
+            abi.encodePacked(type(ModularAccount).creationCode, abi.encode(entryPoint, executionInstallDelegate));
         bytes32 initCodeHash = keccak256(initCode);
         bytes32 salt = bytes32(hex"01234b");
 

test/account/UpgradeToSma.t.sol

@@ -46,7 +46,7 @@ contract UpgradeToSmaTest is AccountTestBase {
 
     function setUp() public override {
         _revertSnapshot = vm.snapshotState();
-        smaStorageImpl = address(new SemiModularAccountStorageOnly(entryPoint));
+        smaStorageImpl = address(new SemiModularAccountStorageOnly(entryPoint, executionInstallDelegate));
         (owner2, owner2Key) = makeAddrAndKey("owner2");
         transferAmount = 0.1 ether;
     }