AA-161 validate Nonce in EntryPoint

contracts/core/BaseAccount.sol

@@ -2,8 +2,7 @@
 pragma solidity ^0.8.12;
 
 /* solhint-disable avoid-low-level-calls */
-/* solhint-disable no-inline-assembly */
-/* solhint-disable reason-string */
+/* solhint-disable no-empty-blocks */
 
 import "../interfaces/IAccount.sol";
 import "../interfaces/IEntryPoint.sol";
@@ -22,10 +21,13 @@ abstract contract BaseAccount is IAccount {
     uint256 constant internal SIG_VALIDATION_FAILED = 1;
 
     /**
-     * return the account nonce.
-     * subclass should return a nonce value that is used both by _validateAndUpdateNonce, and by the external provider (to read the current nonce)
+     * Return the account nonce.
+     * This method returns the next sequential nonce.
+     * For a nonce of a specific key, use `entrypoint.getNonce(account, key)`
      */
-    function nonce() public view virtual returns (uint256);
+    function getNonce() public view virtual returns (uint256) {
+        return entryPoint().getNonce(address(this), 0);
+    }
 
     /**
      * return the entryPoint used by this account.
@@ -41,9 +43,7 @@ abstract contract BaseAccount is IAccount {
     external override virtual returns (uint256 validationData) {
         _requireFromEntryPoint();
         validationData = _validateSignature(userOp, userOpHash);
-        if (userOp.initCode.length == 0) {
-            _validateAndUpdateNonce(userOp);
-        }
+        _validateNonce(userOp.nonce);
         _payPrefund(missingAccountFunds);
     }
 
@@ -71,12 +71,23 @@ abstract contract BaseAccount is IAccount {
     internal virtual returns (uint256 validationData);
 
     /**
-     * validate the current nonce matches the UserOperation nonce.
-     * then it should update the account's state to prevent replay of this UserOperation.
-     * called only if initCode is empty (since "nonce" field is used as "salt" on account creation)
-     * @param userOp the op to validate.
+     * Validate the nonce of the UserOperation.
+     * This method may validate the nonce requirement of this account.
+     * e.g.
+     * To limit the nonce to use sequenced UserOps only (no "out of order" UserOps):
+     *      `require(nonce < type(uint64).max)`
+     * For a hypothetical account that *requires* the nonce to be out-of-order:
+     *      `require(nonce & type(uint64).max == 0)`
+     *
+     * The actual nonce uniqueness is managed by the EntryPoint, and thus no other
+     * action is needed by the account itself.
+     *
+     * @param nonce to validate
+     *
+     * solhint-disable-next-line no-empty-blocks
      */
-    function _validateAndUpdateNonce(UserOperation calldata userOp) internal virtual;
+    function _validateNonce(uint256 nonce) internal view virtual {
+    }
 
     /**
      * sends to the entrypoint (msg.sender) the missing funds for this transaction.

contracts/core/EntryPoint.sol

@@ -16,8 +16,9 @@ import "../utils/Exec.sol";
 import "./StakeManager.sol";
 import "./SenderCreator.sol";
 import "./Helpers.sol";
+import "./NonceManager.sol";
 
-contract EntryPoint is IEntryPoint, StakeManager {
+contract EntryPoint is IEntryPoint, StakeManager, NonceManager {
 
     using UserOperationLib for UserOperation;
 
@@ -349,7 +350,8 @@ contract EntryPoint is IEntryPoint, StakeManager {
      * @param initCode the constructor code to be passed into the UserOperation.
      */
     function getSenderAddress(bytes calldata initCode) public {
-        revert SenderAddressResult(senderCreator.createSender(initCode));
+        address sender = senderCreator.createSender(initCode);
+        revert SenderAddressResult(sender);
     }
 
     function _simulationOnlyValidations(UserOperation calldata userOp) internal view {
@@ -511,6 +513,11 @@ contract EntryPoint is IEntryPoint, StakeManager {
         uint256 gasUsedByValidateAccountPrepayment;
         (uint256 requiredPreFund) = _getRequiredPrefund(mUserOp);
         (gasUsedByValidateAccountPrepayment, validationData) = _validateAccountPrepayment(opIndex, userOp, outOpInfo, requiredPreFund);
+
+        if (!_validateAndUpdateNonce(mUserOp.sender, mUserOp.nonce)) {
+            revert FailedOp(opIndex, "AA25 invalid account nonce");
+        }
+
         //a "marker" where account opcode validation is done and paymaster opcode validation is about to start
         // (used only by off-chain simulateValidation)
         numberMarker();

contracts/core/NonceManager.sol

@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity ^0.8.12;
+
+import "../interfaces/IEntryPoint.sol";
+
+/**
+ * nonce management functionality
+ */
+contract NonceManager is INonceManager {
+
+    /**
+     * The next valid sequence number for a given nonce key.
+     */
+    mapping(address => mapping(uint192 => uint256)) public nonceSequenceNumber;
+
+    function getNonce(address sender, uint192 key)
+    public view override returns (uint256 nonce) {
+        return nonceSequenceNumber[sender][key] | (uint256(key) << 64);
+    }
+
+    // allow an account to manually increment its own nonce.
+    // (mainly so that during construction nonce can be made non-zero,
+    // to "absorb" the gas cost of first nonce increment to 1st transaction (construction),
+    // not to 2nd transaction)
+    function incrementNonce(uint192 key) public override {
+        nonceSequenceNumber[msg.sender][key]++;
+    }
+
+    /**
+     * validate nonce uniqueness for this account.
+     * called just after validateUserOp()
+     */
+    function _validateAndUpdateNonce(address sender, uint256 nonce) internal returns (bool) {
+
+        uint192 key = uint192(nonce >> 64);
+        uint64 seq = uint64(nonce);
+        return nonceSequenceNumber[sender][key]++ == seq;
+    }
+
+}

contracts/interfaces/IEntryPoint.sol

@@ -12,8 +12,9 @@ pragma solidity ^0.8.12;
 import "./UserOperation.sol";
 import "./IStakeManager.sol";
 import "./IAggregator.sol";
+import "./INonceManager.sol";
 
-interface IEntryPoint is IStakeManager {
+interface IEntryPoint is IStakeManager, INonceManager {
 
     /***
      * An event emitted after each successful request

contracts/interfaces/INonceManager.sol

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity ^0.8.12;
+
+interface INonceManager {
+
+    /**
+     * Return the next nonce for this sender.
+     * Within a given key, the nonce values are sequenced (starting with zero, and incremented by one on each userop)
+     * But UserOp with different keys can come with arbitrary order.
+     *
+     * @param sender the account address
+     * @param key the high 192 bit of the nonce
+     * @return nonce a full nonce to pass for next UserOp with this sender.
+     */
+    function getNonce(address sender, uint192 key)
+    external view returns (uint256 nonce);
+
+    /**
+     * Manually increment the nonce of the sender.
+     * This method is exposed just for completeness..
+     * Account does NOT need to call it, neither during validation, nor elsewhere,
+     * as the EntryPoint will update the nonce regardless.
+     * Possible use-case is call it with various keys to "initialize" their nonces to one, so that future
+     * UserOperations will not pay extra for the first transaction with a given key.
+     */
+    function incrementNonce(uint192 key) external;
+}

contracts/samples/SimpleAccount.sol

@@ -21,12 +21,6 @@ import "./callback/TokenCallbackHandler.sol";
 contract SimpleAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, Initializable {
     using ECDSA for bytes32;
 
-    //filler member, to push the nonce and owner to the same slot
-    // the "Initializeble" class takes 2 bytes in the first slot
-    bytes28 private _filler;
-
-    //explicit sizes of nonce, to fit a single storage cell with "owner"
-    uint96 private _nonce;
     address public owner;
 
     IEntryPoint private immutable _entryPoint;
@@ -38,11 +32,6 @@ contract SimpleAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, In
         _;
     }
 
-    /// @inheritdoc BaseAccount
-    function nonce() public view virtual override returns (uint256) {
-        return _nonce;
-    }
-
     /// @inheritdoc BaseAccount
     function entryPoint() public view virtual override returns (IEntryPoint) {
         return _entryPoint;
@@ -100,11 +89,6 @@ contract SimpleAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, In
         require(msg.sender == address(entryPoint()) || msg.sender == owner, "account: not Owner or EntryPoint");
     }
 
-    /// implement template method of BaseAccount
-    function _validateAndUpdateNonce(UserOperation calldata userOp) internal override {
-        require(_nonce++ == userOp.nonce, "account: invalid nonce");
-    }
-
     /// implement template method of BaseAccount
     function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash)
     internal override virtual returns (uint256 validationData) {

contracts/samples/gnosis/EIP4337Fallback.sol

@@ -50,6 +50,14 @@ contract EIP4337Fallback is DefaultCallbackHandler, IAccount, IERC1271 {
         return abi.decode(ret, (uint256));
     }
 
+    /**
+     * Helper for wallet to get the next nonce.
+     */
+    function getNonce() public returns (uint256 nonce) {
+        bytes memory ret = delegateToManager();
+        (nonce) = abi.decode(ret, (uint256));
+    }
+
     /**
      * called from the Safe. delegate actual work to EIP4337Manager
      */
@@ -78,4 +86,4 @@ contract EIP4337Fallback is DefaultCallbackHandler, IAccount, IERC1271 {
             return 0xffffffff;
         }
     }
-}
+}

contracts/samples/gnosis/EIP4337Manager.sol

@@ -55,10 +55,8 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor {
             validationData = SIG_VALIDATION_FAILED;
         }
 
-        if (userOp.initCode.length == 0) {
-            require(uint256(nonce) == userOp.nonce, "account: invalid nonce");
-            nonce = bytes32(uint256(nonce) + 1);
-        }
+        // mimic normal Safe nonce behaviour: prevent parallel nonces
+        require(userOp.nonce < type(uint64).max, "account: nonsequential nonce");
 
         if (missingAccountFunds > 0) {
             //Note: MAY pay more than the minimum, to deposit for future transactions
@@ -105,6 +103,12 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor {
         }
     }
 
+    /**
+     * Helper for wallet to get the next nonce.
+     */
+    function getNonce() public view returns (uint256) {
+        return IEntryPoint(entryPoint).getNonce(address(this), 0);
+    }
 
     /**
      * set up a safe as EIP-4337 enabled.
@@ -158,7 +162,8 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor {
         sig[64] = bytes1(uint8(27));
         sig[2] = bytes1(uint8(1));
         sig[35] = bytes1(uint8(1));
-        UserOperation memory userOp = UserOperation(address(safe), uint256(nonce), "", "", 0, 1000000, 0, 0, 0, "", sig);
+        uint256 nonce = uint256(IEntryPoint(manager.entryPoint()).getNonce(address(safe), 0));
+        UserOperation memory userOp = UserOperation(address(safe), nonce, "", "", 0, 1000000, 0, 0, 0, "", sig);
         UserOperation[] memory userOps = new UserOperation[](1);
         userOps[0] = userOp;
         IEntryPoint _entryPoint = IEntryPoint(payable(manager.entryPoint()));

contracts/test/MaliciousAccount.sol

@@ -12,11 +12,12 @@ contract MaliciousAccount is IAccount {
     function validateUserOp(UserOperation calldata userOp, bytes32, uint256 missingAccountFunds)
     external returns (uint256 validationData) {
         ep.depositTo{value : missingAccountFunds}(address(this));
-        // Now calculate basefee per EntryPoint.getUserOpGasPrice() and compare it to the basefe we pass off-chain as nonce
+        // Now calculate basefee per EntryPoint.getUserOpGasPrice() and compare it to the basefe we pass off-chain in the signature
+        uint256 externalBaseFee = abi.decode(userOp.signature, (uint256));
         uint256 requiredGas = userOp.callGasLimit + userOp.verificationGasLimit + userOp.preVerificationGas;
         uint256 gasPrice = missingAccountFunds / requiredGas;
         uint256 basefee = gasPrice - userOp.maxPriorityFeePerGas;
-        require (basefee == userOp.nonce, "Revert after first validation");
+        require (basefee == externalBaseFee, "Revert after first validation");
         return 0;
     }
 }

deploy/2_deploy_SimpleAccountFactory.ts

@@ -12,6 +12,7 @@ const deploySimpleAccountFactory: DeployFunction = async function (hre: HardhatR
       from,
       args: [entrypoint.address],
       gasLimit: 6e6,
+      log: true,
       deterministicDeployment: true
     })
   console.log('==SimpleAccountFactory addr=', ret.address)

eip/EIPS/eip-4337.md

@@ -51,7 +51,7 @@ To avoid Ethereum consensus changes, we do not attempt to create new transaction
 | Field | Type | Description
 | - | - | - |
 | `sender` | `address` | The account making the operation |
-| `nonce` | `uint256` | Anti-replay parameter; also used as the salt for first-time account creation |
+| `nonce` | `uint256` | Anti-replay parameter |
 | `initCode` | `bytes` | The initCode of the account (needed if and only if the account is not yet on-chain and needs to be created) |
 | `callData` | `bytes` | The data to pass to the `sender` during the main execution call |
 | `callGasLimit` | `uint256` | The amount of gas to allocate the main execution call |
@@ -128,6 +128,7 @@ The account:
 * MUST validate the caller is a trusted EntryPoint
 * If the account does not support signature aggregation, it MUST validate the signature is a valid signature of the `userOpHash`, and
   SHOULD return SIG_VALIDATION_FAILED (and not revert) on signature mismatch. Any other error should revert.
+* The MAY check the nonce field, but should not implement the replay protection mechanism: the EntryPoint maintains uniqueness of nonces per user account.
 * MUST pay the entryPoint (caller) at least the "missingAccountFunds" (which might be zero, in case current account's deposit is high enough)
 * The account MAY pay more than this minimum, to cover future transactions (it can always issue `withdrawTo` to retrieve it)
 * The return value MUST be packed of `authorizer`, `validUntil` and `validAfter` timestamps.
@@ -182,6 +183,7 @@ The entry point's `handleOps` function must perform the following steps (we firs
 * **Create the account if it does not yet exist**, using the initcode provided in the `UserOperation`. If the account does not exist, _and_ the initcode is empty, or does not deploy a contract at the "sender" address, the call must fail.
 * **Call `validateUserOp` on the account**, passing in the `UserOperation`, the required fee and aggregator (if there is one). The account should verify the operation's signature, and pay the fee if the account considers the operation valid. If any `validateUserOp` call fails, `handleOps` must skip execution of at least that operation, and may revert entirely.
 * Validate the account's deposit in the entryPoint is high enough to cover the max possible cost (cover the already-done verification and max execution gas)
+* Validate the nonce uniqueness. see [Keep Nonce Uniqueness](#keep-nonce-uniqueness) below
 
 In the execution loop, the `handleOps` call must perform the following steps for each `UserOperation`:
 
@@ -192,6 +194,17 @@ In the execution loop, the `handleOps` call must perform the following steps for
 Before accepting a `UserOperation`, bundlers should use an RPC method to locally call the `simulateValidation` function of the entry point, to verify that the signature is correct and the operation actually pays fees; see the [Simulation section below](#simulation) for details.
 A node/bundler SHOULD drop (not add to the mempool) a `UserOperation` that fails the validation
 
+### Keep Nonce Uniqueness
+
+The EntryPoint maintains nonce uniqueness for each submitted UserOperation using the following algorithm:
+* The nonce is treated as 2 separate fields: 
+  * 64-bit "sequence"
+  * 192-bit "key"
+* Within each "key", the "sequence" value must have consecutive values, starting with zero.
+* That is, a nonce with a new "key" value is allowed, as long as the "sequence" part is zero. The next nonce for that key must be "1", and so on.
+* The EntryPoint exports a method `getNonce(address sender, uint192 key)` to return the next valid nonce for this key.
+* The behaviour of a "classic" sequential nonce can be achieved by validating that the "key" part is always zero.
+
 ### Extension: paymasters
 
 We extend the entry point logic to support **paymasters** that can sponsor transactions for other users. This feature can be used to allow application developers to subsidize fees for their users, allow users to pay fees with [ERC-20](./eip-20.md) tokens and many other use cases. When the paymaster is not equal to the zero address, the entry point implements a different flow:
@@ -907,7 +920,7 @@ See `https://github.com/eth-infinitism/account-abstraction/tree/main/contracts`
 
 ## Security Considerations
 
-The entry point contract will need to be very heavily audited and formally verified, because it will serve as a central trust point for _all_ [ERC-4337](./eip-4337.md). In total, this architecture reduces auditing and formal verification load for the ecosystem, because the amount of work that individual _accounts_ have to do becomes much smaller (they need only verify the `validateUserOp` function and its "check signature, increment nonce and pay fees" logic) and check that other functions are `msg.sender == ENTRY_POINT` gated (perhaps also allowing `msg.sender == self`), but it is nevertheless the case that this is done precisely by concentrating security risk in the entry point contract that needs to be verified to be very robust.
+The entry point contract will need to be very heavily audited and formally verified, because it will serve as a central trust point for _all_ [EIP-4337](./eip-4337.md). In total, this architecture reduces auditing and formal verification load for the ecosystem, because the amount of work that individual _accounts_ have to do becomes much smaller (they need only verify the `validateUserOp` function and its "check signature, and pay fees" logic) and check that other functions are `msg.sender == ENTRY_POINT` gated (perhaps also allowing `msg.sender == self`), but it is nevertheless the case that this is done precisely by concentrating security risk in the entry point contract that needs to be verified to be very robust.
 
 Verification would need to cover two primary claims (not including claims needed to protect paymasters, and claims needed to establish p2p-level DoS resistance):