fix: [CL ALCHEMY-003] validation function uniqueness in nonce (#293)

alchemyplatform · Nov 24, 2024 · bf9e53a · bf9e53a
1 parent e7ea249
commit bf9e53a
Show file tree

Hide file tree

Showing 38 changed files with 1,075 additions and 555 deletions.
diff --git a/gas-snapshots/ModularAccount.json b/gas-snapshots/ModularAccount.json
@@ -1,16 +1,16 @@
 {
-  "Runtime_AccountCreation": "176129",
-  "Runtime_BatchTransfers": "93000",
-  "Runtime_Erc20Transfer": "78454",
-  "Runtime_InstallSessionKey_Case1": "423213",
-  "Runtime_NativeTransfer": "54603",
-  "Runtime_UseSessionKey_Case1_Counter": "78653",
-  "Runtime_UseSessionKey_Case1_Token": "111966",
-  "UserOp_BatchTransfers": "198311",
-  "UserOp_Erc20Transfer": "185319",
-  "UserOp_InstallSessionKey_Case1": "531396",
-  "UserOp_NativeTransfer": "161576",
-  "UserOp_UseSessionKey_Case1_Counter": "195221",
-  "UserOp_UseSessionKey_Case1_Token": "226205",
-  "UserOp_deferredValidation": "258068"
+  "Runtime_AccountCreation": "176376",
+  "Runtime_BatchTransfers": "92817",
+  "Runtime_Erc20Transfer": "78271",
+  "Runtime_InstallSessionKey_Case1": "423293",
+  "Runtime_NativeTransfer": "54420",
+  "Runtime_UseSessionKey_Case1_Counter": "78542",
+  "Runtime_UseSessionKey_Case1_Token": "111855",
+  "UserOp_BatchTransfers": "197788",
+  "UserOp_Erc20Transfer": "184784",
+  "UserOp_InstallSessionKey_Case1": "531124",
+  "UserOp_NativeTransfer": "161029",
+  "UserOp_UseSessionKey_Case1_Counter": "194521",
+  "UserOp_UseSessionKey_Case1_Token": "225505",
+  "UserOp_deferredValidation": "257223"
 }
diff --git a/gas-snapshots/SemiModularAccount.json b/gas-snapshots/SemiModularAccount.json
@@ -1,16 +1,16 @@
 {
   "Runtime_AccountCreation": "97767",
-  "Runtime_BatchTransfers": "88986",
-  "Runtime_Erc20Transfer": "74488",
-  "Runtime_InstallSessionKey_Case1": "421765",
-  "Runtime_NativeTransfer": "50647",
-  "Runtime_UseSessionKey_Case1_Counter": "79003",
-  "Runtime_UseSessionKey_Case1_Token": "112316",
-  "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": "254119"
+  "Runtime_BatchTransfers": "89019",
+  "Runtime_Erc20Transfer": "74521",
+  "Runtime_InstallSessionKey_Case1": "422024",
+  "Runtime_NativeTransfer": "50680",
+  "Runtime_UseSessionKey_Case1_Counter": "78809",
+  "Runtime_UseSessionKey_Case1_Token": "112122",
+  "UserOp_BatchTransfers": "193097",
+  "UserOp_Erc20Transfer": "180176",
+  "UserOp_InstallSessionKey_Case1": "528787",
+  "UserOp_NativeTransfer": "156451",
+  "UserOp_UseSessionKey_Case1_Counter": "194821",
+  "UserOp_UseSessionKey_Case1_Token": "225805",
+  "UserOp_deferredValidation": "253169"
 }
diff --git a/gas/modular-account/ModularAccount.gas.t.sol b/gas/modular-account/ModularAccount.gas.t.sol
@@ -1,9 +1,8 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.26;
 
-import {ModuleEntity} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol";
-
 import {Call} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol";
+import {ModuleEntity, ModuleEntityLib} from "@erc6900/reference-implementation/libraries/ModuleEntityLib.sol";
 import {
     ValidationConfig,
     ValidationConfigLib
@@ -81,7 +80,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(signerValidation, GLOBAL_V, 0),
             initCode: "",
             callData: abi.encodeCall(ModularAccountBase.execute, (recipient, 0.1 ether, "")),
             // don't over-estimate by a lot here, otherwise a fee is assessed.
@@ -94,8 +93,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature =
-            _encodeSignature(signerValidation, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -138,7 +136,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(signerValidation, GLOBAL_V, 0),
             initCode: "",
             callData: abi.encodeCall(
                 ModularAccountBase.execute,
@@ -154,8 +152,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature =
-            _encodeSignature(signerValidation, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -214,7 +211,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(signerValidation, GLOBAL_V, 0),
             initCode: "",
             callData: abi.encodeCall(ModularAccountBase.executeBatch, (calls)),
             // don't over-estimate by a lot here, otherwise a fee is assessed.
@@ -227,8 +224,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature =
-            _encodeSignature(signerValidation, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -244,12 +240,14 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
         vm.deal(address(account1), 1 ether);
 
         SingleSignerValidationModule newValidationModule = _deploySingleSignerValidationModule();
-        uint32 newEntityId = 0;
+        uint32 newEntityId = 1;
         (address owner2, uint256 owner2Key) = makeAddrAndKey("owner2");
 
+        ModuleEntity newUOValidationEntity = ModuleEntityLib.pack(address(newValidationModule), newEntityId);
+
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonceDefAction(newUOValidationEntity, GLOBAL_V, 0),
             initCode: "",
             callData: abi.encodeCall(ModularAccountBase.execute, (recipient, 0.1 ether, "")),
             // don't over-estimate by a lot here, otherwise a fee is assessed.
@@ -286,8 +284,6 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
         );
 
         userOp.signature = _encodeDeferredInstallUOSignature(
-            ValidationConfigLib.moduleEntity(newUOValidation),
-            GLOBAL_VALIDATION,
             _packDeferredInstallData(
                 deferredInstallNonce,
                 deferredInstallDeadline,
@@ -329,7 +325,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(signerValidation, GLOBAL_V, 0),
             initCode: "",
             callData: _getInstallDataSessionKeyCase1(),
             // don't over-estimate by a lot here, otherwise a fee is assessed.
@@ -342,8 +338,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature =
-            _encodeSignature(signerValidation, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -392,7 +387,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(sessionKeyValidation, SELECTOR_ASSOCIATED_V, 0),
             initCode: "",
             callData: abi.encodePacked(
                 ModularAccountBase.executeUserOp.selector,
@@ -411,9 +406,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) =
             vm.sign(sessionSigner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature = _encodeSignature(
-            sessionKeyValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v)
-        );
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -466,7 +459,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(sessionKeyValidation, SELECTOR_ASSOCIATED_V, 0),
             initCode: "",
             callData: abi.encodePacked(
                 ModularAccountBase.executeUserOp.selector,
@@ -486,9 +479,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("ModularAccount")
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) =
             vm.sign(sessionSigner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature = _encodeSignature(
-            sessionKeyValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v)
-        );
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 

diff --git a/gas/modular-account/SemiModularAccount.gas.t.sol b/gas/modular-account/SemiModularAccount.gas.t.sol
@@ -1,9 +1,8 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.26;
 
-import {ModuleEntity} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol";
-
 import {Call} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol";
+import {ModuleEntity, ModuleEntityLib} from "@erc6900/reference-implementation/libraries/ModuleEntityLib.sol";
 import {
     ValidationConfig,
     ValidationConfigLib
@@ -75,7 +74,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(signerValidation, GLOBAL_V, 0),
             initCode: "",
             callData: abi.encodeCall(ModularAccountBase.execute, (recipient, 0.1 ether, "")),
             // don't over-estimate by a lot here, otherwise a fee is assessed.
@@ -88,8 +87,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature =
-            _encodeSignature(signerValidation, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -132,7 +130,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(signerValidation, GLOBAL_V, 0),
             initCode: "",
             callData: abi.encodeCall(
                 ModularAccountBase.execute,
@@ -148,8 +146,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature =
-            _encodeSignature(signerValidation, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -208,7 +205,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(signerValidation, GLOBAL_V, 0),
             initCode: "",
             callData: abi.encodeCall(ModularAccountBase.executeBatch, (calls)),
             // don't over-estimate by a lot here, otherwise a fee is assessed.
@@ -221,8 +218,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature =
-            _encodeSignature(signerValidation, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -238,12 +234,14 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
         vm.deal(address(account1), 1 ether);
 
         SingleSignerValidationModule newValidationModule = _deploySingleSignerValidationModule();
-        uint32 newEntityId = 0;
+        uint32 newEntityId = 1;
         (address owner2, uint256 owner2Key) = makeAddrAndKey("owner2");
 
+        ModuleEntity newUOValidationEntity = ModuleEntityLib.pack(address(newValidationModule), newEntityId);
+
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonceDefAction(newUOValidationEntity, GLOBAL_V, 0),
             initCode: "",
             callData: abi.encodeCall(ModularAccountBase.execute, (recipient, 0.1 ether, "")),
             // don't over-estimate by a lot here, otherwise a fee is assessed.
@@ -276,8 +274,6 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
         bytes memory deferredValidationSig = _signRawHash(vm, owner1Key, digest);
 
         userOp.signature = _encodeDeferredInstallUOSignature(
-            ValidationConfigLib.moduleEntity(newUOValidation),
-            GLOBAL_VALIDATION,
             _packDeferredInstallData(
                 deferredInstallNonce,
                 deferredInstallDeadline,
@@ -319,7 +315,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(signerValidation, GLOBAL_V, 0),
             initCode: "",
             callData: _getInstallDataSessionKeyCase1(),
             // don't over-estimate by a lot here, otherwise a fee is assessed.
@@ -332,8 +328,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature =
-            _encodeSignature(signerValidation, GLOBAL_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -382,7 +377,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(sessionKeyValidation, SELECTOR_ASSOCIATED_V, 0),
             initCode: "",
             callData: abi.encodePacked(
                 ModularAccountBase.executeUserOp.selector,
@@ -401,9 +396,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) =
             vm.sign(sessionSigner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature = _encodeSignature(
-            sessionKeyValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v)
-        );
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 
@@ -456,7 +449,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
 
         PackedUserOperation memory userOp = PackedUserOperation({
             sender: address(account1),
-            nonce: 0,
+            nonce: _encodeNonce(sessionKeyValidation, SELECTOR_ASSOCIATED_V, 0),
             initCode: "",
             callData: abi.encodePacked(
                 ModularAccountBase.executeUserOp.selector,
@@ -476,9 +469,7 @@ contract ModularAccountGasTest is ModularAccountBenchmarkBase("SemiModularAccoun
         bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
         (uint8 v, bytes32 r, bytes32 s) =
             vm.sign(sessionSigner1Key, MessageHashUtils.toEthSignedMessageHash(userOpHash));
-        userOp.signature = _encodeSignature(
-            sessionKeyValidation, SELECTOR_ASSOCIATED_VALIDATION, abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v)
-        );
+        userOp.signature = _encodeSignature(abi.encodePacked(EOA_TYPE_SIGNATURE, r, s, v));
 
         uint256 gasUsed = _userOpBenchmark(userOp);
 

diff --git a/src/account/AccountStorage.sol b/src/account/AccountStorage.sol
@@ -1,9 +1,10 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.26;
 
-import {HookConfig, ModuleEntity} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol";
+import {HookConfig} from "@erc6900/reference-implementation/interfaces/IModularAccount.sol";
 
 import {LinkedListSet, SetValue} from "../libraries/LinkedListSetLib.sol";
+import {ValidationLookupKey} from "../libraries/ValidationLocatorLib.sol";
 
 // ERC-7201 derived storage slot.
 // keccak256(abi.encode(uint256(keccak256("Alchemy.ModularAccount.Storage_V2")) - 1)) & ~bytes32(uint256(0xff))
@@ -27,6 +28,8 @@ struct ExecutionStorage {
 
 /// @notice Represents data associated with a specific validation function.
 struct ValidationStorage {
+    // The address of the validation module.
+    address module;
     // Whether or not this validation can be used as a global validation function.
     bool isGlobal;
     // Whether or not this validation is allowed to validate ERC-1271 signatures.
@@ -55,7 +58,7 @@ struct AccountStorage {
     // Execution functions and their associated functions.
     mapping(bytes4 selector => ExecutionStorage) executionStorage;
     // Validation functions and their associated functions.
-    mapping(ModuleEntity validationFunction => ValidationStorage) validationStorage;
+    mapping(ValidationLookupKey lookupKey => ValidationStorage) validationStorage;
     // Module-defined ERC-165 interfaces installed on the account.
     mapping(bytes4 => uint256) supportedIfaces;
     // Nonce usage state for deferred actions.