[AA]: fix inconsistent userOpHash

packages/boba/account-abstraction/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.

packages/boba/account-abstraction/contracts/core/EntryPoint.sol

@@ -16,8 +16,10 @@ import "../utils/Exec.sol";
 import "./StakeManager.sol";
 import "./SenderCreator.sol";
 import "./Helpers.sol";
+import "./NonceManager.sol";
+import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
 
-contract EntryPoint is IEntryPoint, StakeManager {
+contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard {
 
     using UserOperationLib for UserOperation;
 
@@ -87,7 +89,7 @@ contract EntryPoint is IEntryPoint, StakeManager {
      * @param ops the operations to execute
      * @param beneficiary the address to receive the fees
      */
-    function handleOps(UserOperation[] calldata ops, address payable beneficiary) public {
+    function handleOps(UserOperation[] calldata ops, address payable beneficiary) public nonReentrant {
 
         uint256 opslen = ops.length;
         UserOpInfo[] memory opInfos = new UserOpInfo[](opslen);
@@ -100,6 +102,7 @@ contract EntryPoint is IEntryPoint, StakeManager {
         }
 
         uint256 collected = 0;
+        emit BeforeExecution();
 
         for (uint256 i = 0; i < opslen; i++) {
             collected += _executeUserOp(i, ops[i], opInfos[i]);
@@ -117,7 +120,7 @@ contract EntryPoint is IEntryPoint, StakeManager {
     function handleAggregatedOps(
         UserOpsPerAggregator[] calldata opsPerAggregator,
         address payable beneficiary
-    ) public {
+    ) public nonReentrant {
 
         uint256 opasLen = opsPerAggregator.length;
         uint256 totalOps = 0;
@@ -142,6 +145,8 @@ contract EntryPoint is IEntryPoint, StakeManager {
 
         UserOpInfo[] memory opInfos = new UserOpInfo[](totalOps);
 
+        emit BeforeExecution();
+
         uint256 opIndex = 0;
         for (uint256 a = 0; a < opasLen; a++) {
             UserOpsPerAggregator calldata opa = opsPerAggregator[a];
@@ -349,7 +354,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 +517,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();

packages/boba/account-abstraction/contracts/core/Helpers.sol

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity ^0.8.12;
 
+/* solhint-disable no-inline-assembly */
+
 /**
  * returned data from validateUserOp.
  * validateUserOp returns a uint256, with is created by `_packedValidationData` and parsed by `_parseValidationData`
@@ -63,3 +65,16 @@ pragma solidity ^0.8.12;
     function _packValidationData(bool sigFailed, uint48 validUntil, uint48 validAfter) pure returns (uint256) {
         return (sigFailed ? 1 : 0) | (uint256(validUntil) << 160) | (uint256(validAfter) << (160 + 48));
     }
+
+/**
+ * keccak function over calldata.
+ * @dev copy calldata into memory, do keccak and drop allocated memory. Strangely, this is more efficient than letting solidity do it.
+ */
+    function calldataKeccak(bytes calldata data) pure returns (bytes32 ret) {
+        assembly {
+            let mem := mload(0x40)
+            let len := data.length
+            calldatacopy(mem, data.offset, len)
+            ret := keccak256(mem, len)
+        }
+    }

packages/boba/account-abstraction/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;
+    }
+
+}

packages/boba/account-abstraction/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
@@ -45,6 +46,12 @@ interface IEntryPoint is IStakeManager {
      */
     event UserOperationRevertReason(bytes32 indexed userOpHash, address indexed sender, uint256 nonce, bytes revertReason);
 
+    /**
+     * an event emitted by handleOps(), before starting the execution loop.
+     * any event emitted before this event, is part of the validation.
+     */
+    event BeforeExecution();
+
     /**
      * signature aggregator used by the following UserOperationEvents within this bundle.
      */

packages/boba/account-abstraction/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;
+}

packages/boba/account-abstraction/contracts/interfaces/UserOperation.sol

@@ -3,6 +3,8 @@ pragma solidity ^0.8.12;
 
 /* solhint-disable no-inline-assembly */
 
+import {calldataKeccak} from "../core/Helpers.sol";
+
     /**
      * User Operation struct
      * @param sender the sender account of this request.
@@ -59,19 +61,24 @@ library UserOperationLib {
     }
 
     function pack(UserOperation calldata userOp) internal pure returns (bytes memory ret) {
-        //lighter signature scheme. must match UserOp.ts#packUserOp
-        bytes calldata sig = userOp.signature;
-        // copy directly the userOp from calldata up to (but not including) the signature.
-        // this encoding depends on the ABI encoding of calldata, but is much lighter to copy
-        // than referencing each field separately.
-        assembly {
-            let ofs := userOp
-            let len := sub(sub(sig.offset, ofs), 32)
-            ret := mload(0x40)
-            mstore(0x40, add(ret, add(len, 32)))
-            mstore(ret, len)
-            calldatacopy(add(ret, 32), ofs, len)
-        }
+        address sender = getSender(userOp);
+        uint256 nonce = userOp.nonce;
+        bytes32 hashInitCode = calldataKeccak(userOp.initCode);
+        bytes32 hashCallData = calldataKeccak(userOp.callData);
+        uint256 callGasLimit = userOp.callGasLimit;
+        uint256 verificationGasLimit = userOp.verificationGasLimit;
+        uint256 preVerificationGas = userOp.preVerificationGas;
+        uint256 maxFeePerGas = userOp.maxFeePerGas;
+        uint256 maxPriorityFeePerGas = userOp.maxPriorityFeePerGas;
+        bytes32 hashPaymasterAndData = calldataKeccak(userOp.paymasterAndData);
+
+        return abi.encode(
+            sender, nonce,
+            hashInitCode, hashCallData,
+            callGasLimit, verificationGasLimit, preVerificationGas,
+            maxFeePerGas, maxPriorityFeePerGas,
+            hashPaymasterAndData
+        );
     }
 
     function hash(UserOperation calldata userOp) internal pure returns (bytes32) {

packages/boba/account-abstraction/contracts/samples/SimpleAccount.sol

@@ -10,22 +10,17 @@ import "@openzeppelin/contracts/proxy/utils/Initializable.sol";
 import "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
 
 import "../core/BaseAccount.sol";
+import "./callback/TokenCallbackHandler.sol";
 
 /**
   * minimal account.
   *  this is sample minimal account.
   *  has execute, eth handling methods
   *  has a single signer that can send requests through the entryPoint.
   */
-contract SimpleAccount is BaseAccount, UUPSUpgradeable, Initializable {
+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;
@@ -37,11 +32,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 +89,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) {