AA-161 validate Nonce in EntryPoint

contracts/core/BaseAccount.sol

@@ -23,9 +23,10 @@ abstract contract BaseAccount is IAccount {
 
     /**
      * 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)
      */
-    function nonce() public view virtual returns (uint256);
+    function nonce() public view virtual returns (uint256) {
+        return entryPoint().getNonce(address(this), 0);
+    }
 
     /**
      * return the entryPoint used by this account.
@@ -41,9 +42,6 @@ abstract contract BaseAccount is IAccount {
     external override virtual returns (uint256 validationData) {
         _requireFromEntryPoint();
         validationData = _validateSignature(userOp, userOpHash);
-        if (userOp.initCode.length == 0) {
-            _validateAndUpdateNonce(userOp);
-        }
         _payPrefund(missingAccountFunds);
     }
 
@@ -70,14 +68,6 @@ abstract contract BaseAccount is IAccount {
     function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash)
     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.
-     */
-    function _validateAndUpdateNonce(UserOperation calldata userOp) internal virtual;
-
     /**
      * sends to the entrypoint (msg.sender) the missing funds for this transaction.
      * subclass MAY override this method for better funds management

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;
 
@@ -345,11 +346,13 @@ contract EntryPoint is IEntryPoint, StakeManager {
     /**
      * Get counterfactual sender address.
      *  Calculate the sender contract address that will be generated by the initCode and salt in the UserOperation.
-     * this method always revert, and returns the address in SenderAddressResult error
+     * this method always revert, and returns the address (and the nonce) in SenderAddressResult error
      * @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);
+        uint256 nonce = getNonce(sender, 0);
+        revert SenderAddressResult(sender, nonce);
     }
 
     function _simulationOnlyValidations(UserOperation calldata userOp) internal view {
@@ -511,6 +514,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,37 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity ^0.8.12;
+
+import "../interfaces/IEntryPoint.sol";
+
+/**
+ * nonce management functionality
+ */
+contract NonceManager is INonceManager {
+
+    mapping(address => mapping(uint192 => uint256)) public nonces;
+
+    function getNonce(address sender, uint192 key)
+    public view override returns (uint256 nonce) {
+        return nonces[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 {
+        nonces[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 nonces[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
@@ -93,7 +94,7 @@ interface IEntryPoint is IStakeManager {
     /**
      * return value of getSenderAddress
      */
-    error SenderAddressResult(address sender);
+    error SenderAddressResult(address sender, uint nonce);
 
     /**
      * return value of simulateHandleOp
@@ -174,7 +175,7 @@ interface IEntryPoint is IStakeManager {
     /**
      * Get counterfactual sender address.
      *  Calculate the sender contract address that will be generated by the initCode and salt in the UserOperation.
-     * this method always revert, and returns the address in SenderAddressResult error
+     * this method always revert, and returns the address (and nonce) in SenderAddressResult error
      * @param initCode the constructor code to be passed into the UserOperation.
      */
     function getSenderAddress(bytes memory initCode) external;

contracts/interfaces/INonceManager.sol

@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity ^0.8.12;
+
+interface INonceManager {
+
+    /**
+     * return the next nonce for this sender
+     * @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);
+
+    function incrementNonce(uint192 key) external;
+}

contracts/samples/SimpleAccount.sol

@@ -24,8 +24,6 @@ contract SimpleAccount is BaseAccount, UUPSUpgradeable, Initializable {
     // 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;
@@ -37,11 +35,6 @@ contract SimpleAccount is BaseAccount, UUPSUpgradeable, Initializable {
         _;
     }
 
-    /// @inheritdoc BaseAccount
-    function nonce() public view virtual override returns (uint256) {
-        return _nonce;
-    }
-
     /// @inheritdoc BaseAccount
     function entryPoint() public view virtual override returns (IEntryPoint) {
         return _entryPoint;
@@ -99,11 +92,6 @@ contract SimpleAccount is BaseAccount, UUPSUpgradeable, Initializable {
         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,16 @@ contract EIP4337Fallback is DefaultCallbackHandler, IAccount, IERC1271 {
         return abi.decode(ret, (uint256));
     }
 
+    /**
+     * Helper for wallet to get the next nonce.
+     * (NOTE: can't be named "nonce()", since it overrides the GnosisSafe internal "nonce" function, which we ignore,
+     * as we use the EntryPoint's 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 +88,4 @@ contract EIP4337Fallback is DefaultCallbackHandler, IAccount, IERC1271 {
             return 0xffffffff;
         }
     }
-}
+}

contracts/samples/gnosis/EIP4337Manager.sol

@@ -55,11 +55,6 @@ 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);
-        }
-
         if (missingAccountFunds > 0) {
             //Note: MAY pay more than the minimum, to deposit for future transactions
             (bool success,) = payable(msgSender).call{value : missingAccountFunds}("");
@@ -105,6 +100,14 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor {
         }
     }
 
+    /**
+     * Helper for wallet to get the next nonce.
+     * (NOTE: can't be named "nonce()", since it overrides the GnosisSafe internal "nonce" function, which we ignore,
+     * as we use the EntryPoint's nonce)
+     */
+    function getNonce() public view returns (uint256) {
+        return IEntryPoint(entryPoint).getNonce(address(this), 0);
+    }
 
     /**
      * set up a safe as EIP-4337 enabled.
@@ -158,7 +161,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);
+        uint 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)

gascalc/GasChecker.ts

@@ -14,12 +14,13 @@ import {
 } from '../typechain'
 import { BigNumberish, Wallet } from 'ethers'
 import hre from 'hardhat'
-import { fillAndSign } from '../test/UserOp'
+import { fillAndSign, fillUserOp, signUserOp } from '../test/UserOp'
 import { TransactionReceipt } from '@ethersproject/abstract-provider'
 import { table, TableUserConfig } from 'table'
 import { Create2Factory } from '../src/Create2Factory'
 import * as fs from 'fs'
 import { SimpleAccountInterface } from '../typechain/contracts/samples/SimpleAccount'
+import { UserOperation } from '../test/UserOperation'
 
 const gasCheckerLogFile = './reports/gas-checker.txt'
 
@@ -111,6 +112,8 @@ export class GasChecker {
     ])
   }
 
+  createdAccounts = new Set<string>()
+
   /**
    * create accounts up to this counter.
    * make sure they all have balance.
@@ -127,11 +130,29 @@ export class GasChecker {
     console.log('factaddr', factoryAddress)
     const fact = SimpleAccountFactory__factory.connect(factoryAddress, ethersSigner)
     // create accounts
+    const creationOps: UserOperation[] = []
     for (const n of range(count)) {
       const salt = n
       // const initCode = this.accountInitCode(fact, salt)
 
       const addr = await fact.getAddress(this.accountOwner.address, salt)
+
+      if (!this.createdAccounts.has(addr)) {
+        // explicit call to fillUseROp with no "entryPoint", to make sure we manually fill everything and
+        // not attempt to fill from blockchain.
+        const op = signUserOp(await fillUserOp({
+          sender: addr,
+          nonce: 0,
+          callGasLimit: 30000,
+          verificationGasLimit: 1000000,
+          // paymasterAndData: paymaster,
+          preVerificationGas: 1,
+          maxFeePerGas: 0
+        }), this.accountOwner, this.entryPoint().address, await provider.getNetwork().then(net => net.chainId))
+        creationOps.push(op)
+        this.createdAccounts.add(addr)
+      }
+
       this.accounts[addr] = this.accountOwner
       // deploy if not already deployed.
       await fact.createAccount(this.accountOwner.address, salt)
@@ -140,6 +161,9 @@ export class GasChecker {
         await GasCheckCollector.inst.entryPoint.depositTo(addr, { value: minDepositOrBalance.mul(5) })
       }
     }
+    console.log('ops=', creationOps)
+    await this.entryPoint().handleOps(creationOps, ethersSigner.getAddress())
+    console.log('post-op')
   }
 
   /**
@@ -238,7 +262,9 @@ export class GasChecker {
       title: info.title
       // receipt: rcpt
     }
-    if (info.diffLastGas) { ret1.gasDiff = gasDiff }
+    if (info.diffLastGas) {
+      ret1.gasDiff = gasDiff
+    }
     console.debug(ret1)
     return ret1
   }