From 931df7e7ebbcc92c0c77094064072deada955dc5 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Wed, 15 Mar 2023 21:05:59 +0200
Subject: [PATCH 01/17] EntryPoint to manage Nonce

- NonceManager to handle 2d nonces
  - _validateAndUpdateNonce() called after validateUserOp
  - getNonce(sender, key)
- remove nonce checking from BaseAccount
- BaseAccount implements nonce() using ep.getNonce
---
 contracts/core/BaseAccount.sol          | 16 +++---------
 contracts/core/EntryPoint.sol           | 12 ++++++++-
 contracts/core/NonceManager.sol         | 33 +++++++++++++++++++++++++
 contracts/interfaces/IEntryPoint.sol    |  3 ++-
 contracts/interfaces/INonceManager.sol  | 15 +++++++++++
 contracts/samples/SimpleAccount.sol     | 12 ---------
 contracts/test/MaliciousAccount.sol     |  5 ++--
 deploy/2_deploy_SimpleAccountFactory.ts |  2 +-
 test/entrypoint.test.ts                 | 13 +++++-----
 test/simple-wallet.test.ts              |  8 ------
 test/y.bls.test.ts                      |  1 -
 11 files changed, 75 insertions(+), 45 deletions(-)
 create mode 100644 contracts/core/NonceManager.sol
 create mode 100644 contracts/interfaces/INonceManager.sol

diff --git a/contracts/core/BaseAccount.sol b/contracts/core/BaseAccount.sol
index 6c06e918..2f573060 100644
--- a/contracts/core/BaseAccount.sol
+++ b/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
diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol
index a25fa7d9..48401b0f 100644
--- a/contracts/core/EntryPoint.sol
+++ b/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 "hardhat/console.sol";
 
-contract EntryPoint is IEntryPoint, StakeManager {
+contract EntryPoint is IEntryPoint, StakeManager, NonceManager {
 
     using UserOperationLib for UserOperation;
 
@@ -229,6 +231,8 @@ contract EntryPoint is IEntryPoint, StakeManager {
     unchecked {
         // handleOps was called with gas limit too low. abort entire bundle.
         if (gasleft() < callGasLimit + mUserOp.verificationGasLimit + 5000) {
+            console.log('left=',gasleft());
+            console.log('needed: ', callGasLimit, mUserOp.verificationGasLimit);
             assembly {
                 mstore(0, INNER_OUT_OF_GAS)
                 revert(0, 32)
@@ -511,6 +515,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();
@@ -523,6 +532,7 @@ contract EntryPoint is IEntryPoint, StakeManager {
         uint256 gasUsed = preGas - gasleft();
 
         if (userOp.verificationGasLimit < gasUsed) {
+            console.log('AA40: verGasLimit %s used %s', mUserOp.verificationGasLimit, gasUsed);
             revert FailedOp(opIndex, "AA40 over verificationGasLimit");
         }
         outOpInfo.prefund = requiredPreFund;
diff --git a/contracts/core/NonceManager.sol b/contracts/core/NonceManager.sol
new file mode 100644
index 00000000..2ce08e0e
--- /dev/null
+++ b/contracts/core/NonceManager.sol
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity ^0.8.12;
+
+import "../interfaces/IEntryPoint.sol";
+import "hardhat/console.sol";
+
+/**
+ * nonce management functionality
+ */
+contract NonceManager is INonceManager {
+
+    mapping(address => mapping(uint192 => uint256)) public nonces;
+
+    function getNonce(address sender, uint192 key)
+    external view override returns (uint256 nonce) {
+//        console.log('getNonce sender %s key %s seq %s', sender, uint(key), uint(nonces[sender][key + 1]));
+        return nonces[sender][key + 1] | (uint256(key) << 64);
+    }
+
+    /**
+     * 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);
+
+        //        console.log('validateNAndUpdate sender %s nonce %s seq %s', sender, uint(nonce), uint(nonces[sender][key+1]));
+        return nonces[sender][key + 1]++ == seq;
+    }
+
+}
diff --git a/contracts/interfaces/IEntryPoint.sol b/contracts/interfaces/IEntryPoint.sol
index c25288b3..a1436197 100644
--- a/contracts/interfaces/IEntryPoint.sol
+++ b/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
diff --git a/contracts/interfaces/INonceManager.sol b/contracts/interfaces/INonceManager.sol
new file mode 100644
index 00000000..20a749cf
--- /dev/null
+++ b/contracts/interfaces/INonceManager.sol
@@ -0,0 +1,15 @@
+// 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);
+}
+
diff --git a/contracts/samples/SimpleAccount.sol b/contracts/samples/SimpleAccount.sol
index 65fe9e68..d3ace1e7 100644
--- a/contracts/samples/SimpleAccount.sol
+++ b/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) {
diff --git a/contracts/test/MaliciousAccount.sol b/contracts/test/MaliciousAccount.sol
index d42e918e..5b840e4f 100644
--- a/contracts/test/MaliciousAccount.sol
+++ b/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;
     }
 }
diff --git a/deploy/2_deploy_SimpleAccountFactory.ts b/deploy/2_deploy_SimpleAccountFactory.ts
index 36002a73..43792ff0 100644
--- a/deploy/2_deploy_SimpleAccountFactory.ts
+++ b/deploy/2_deploy_SimpleAccountFactory.ts
@@ -11,7 +11,7 @@ const deploySimpleAccountFactory: DeployFunction = async function (hre: HardhatR
     'SimpleAccountFactory', {
       from,
       args: [entrypoint.address],
-      gasLimit: 6e6,
+      gasLimit: 6e6, log: true,
       deterministicDeployment: true
     })
   console.log('==SimpleAccountFactory addr=', ret.address)
diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts
index d4502b44..ac2e467a 100644
--- a/test/entrypoint.test.ts
+++ b/test/entrypoint.test.ts
@@ -20,7 +20,8 @@ import {
   TestSignatureAggregator,
   TestSignatureAggregator__factory,
   MaliciousAccount__factory,
-  TestWarmColdAccount__factory
+  TestWarmColdAccount__factory,
+  SimpleAccount__factory
 } from '../typechain'
 import {
   AddressZero,
@@ -233,7 +234,7 @@ describe('EntryPoint', function () {
       // using wrong nonce
       const op = await fillAndSign({ sender: account.address, nonce: 1234 }, accountOwner, entryPoint)
       await expect(entryPoint.callStatic.simulateValidation(op)).to
-        .revertedWith('AA23 reverted: account: invalid nonce')
+        .revertedWith('AA25 invalid account nonce')
     })
 
     it('should report signature failure without revert', async () => {
@@ -411,7 +412,8 @@ describe('EntryPoint', function () {
 
       const userOp: UserOperation = {
         sender: maliciousAccount.address,
-        nonce: block.baseFeePerGas,
+        nonce: await entryPoint.getNonce(maliciousAccount.address,0),
+        signature: defaultAbiCoder.encode(['uint256'], [block.baseFeePerGas]),
         initCode: '0x',
         callData: '0x',
         callGasLimit: '0x' + 1e5.toString(16),
@@ -421,7 +423,6 @@ describe('EntryPoint', function () {
         maxFeePerGas: block.baseFeePerGas.mul(3),
         maxPriorityFeePerGas: block.baseFeePerGas,
         paymasterAndData: '0x',
-        signature: '0x'
       }
       try {
         await expect(entryPoint.simulateValidation(userOp, { gasLimit: 1e6 }))
@@ -447,13 +448,14 @@ describe('EntryPoint', function () {
         sender: testRevertAccount.address,
         callGasLimit: 1e5,
         maxFeePerGas: 1,
+        nonce: await entryPoint.getNonce(testRevertAccount.address, 0),
         verificationGasLimit: 1e5,
         callData: badData.data!
       }
       const beneficiaryAddress = createAddress()
       await expect(entryPoint.simulateValidation(badOp, { gasLimit: 3e5 }))
         .to.revertedWith('ValidationResult')
-      const tx = await entryPoint.handleOps([badOp], beneficiaryAddress, { gasLimit: 3e5 })
+      const tx = await entryPoint.handleOps([badOp], beneficiaryAddress, ) //{ gasLimit: 3e5 })
       const receipt = await tx.wait()
       const userOperationRevertReasonEvent = receipt.events?.find(event => event.event === 'UserOperationRevertReason')
       expect(userOperationRevertReasonEvent?.event).to.equal('UserOperationRevertReason')
@@ -1027,7 +1029,6 @@ describe('EntryPoint', function () {
             await ethersSigner.sendTransaction({ to: addr, value: parseEther('0.1') })
             userOp = await fillAndSign({
               initCode,
-              nonce: 10
             }, accountOwner, entryPoint)
           })
           it('simulateValidation should return aggregator and its stake', async () => {
diff --git a/test/simple-wallet.test.ts b/test/simple-wallet.test.ts
index d5350e0e..28444af4 100644
--- a/test/simple-wallet.test.ts
+++ b/test/simple-wallet.test.ts
@@ -91,14 +91,6 @@ describe('SimpleAccount', function () {
       expect(preBalance - postBalance).to.eql(expectedPay)
     })
 
-    it('should increment nonce', async () => {
-      expect(await account.nonce()).to.equal(1)
-    })
-
-    it('should reject same TX on nonce error', async () => {
-      await expect(account.validateUserOp(userOp, userOpHash, 0)).to.revertedWith('invalid nonce')
-    })
-
     it('should return NO_SIG_VALIDATION on wrong signature', async () => {
       const userOpHash = HashZero
       const deadline = await account.callStatic.validateUserOp({ ...userOp, nonce: 1 }, userOpHash, 0)
diff --git a/test/y.bls.test.ts b/test/y.bls.test.ts
index 16cfcc42..3efe8003 100644
--- a/test/y.bls.test.ts
+++ b/test/y.bls.test.ts
@@ -185,7 +185,6 @@ describe('bls account', function () {
       const userOp = await fillUserOp({
         sender: senderAddress,
         initCode,
-        nonce: 2
       }, entrypoint)
       const requestHash = await blsAgg.getUserOpHash(userOp)
       const sigParts = signer3.sign(requestHash)

From c1290e72279937b6b95443bc454f5517bbbb5c41 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Mon, 20 Mar 2023 19:15:49 +0200
Subject: [PATCH 02/17] getSenderAddress to return nonce

---
 contracts/core/EntryPoint.sol        |   6 +-
 contracts/interfaces/IEntryPoint.sol |   4 +-
 test/UserOp.ts                       | 109 ++++++++++++++-------------
 test/simple-wallet.test.ts           |  55 +++++++++-----
 4 files changed, 97 insertions(+), 77 deletions(-)

diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol
index 48401b0f..6406b743 100644
--- a/contracts/core/EntryPoint.sol
+++ b/contracts/core/EntryPoint.sol
@@ -349,11 +349,13 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager {
     /**
      * 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 {
diff --git a/contracts/interfaces/IEntryPoint.sol b/contracts/interfaces/IEntryPoint.sol
index a1436197..68dd4a12 100644
--- a/contracts/interfaces/IEntryPoint.sol
+++ b/contracts/interfaces/IEntryPoint.sol
@@ -94,7 +94,7 @@ interface IEntryPoint is IStakeManager, INonceManager {
     /**
      * return value of getSenderAddress
      */
-    error SenderAddressResult(address sender);
+    error SenderAddressResult(address sender, uint nonce);
 
     /**
      * return value of simulateHandleOp
@@ -175,7 +175,7 @@ interface IEntryPoint is IStakeManager, INonceManager {
     /**
      * 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;
diff --git a/test/UserOp.ts b/test/UserOp.ts
index 0872d477..6e0293a4 100644
--- a/test/UserOp.ts
+++ b/test/UserOp.ts
@@ -4,16 +4,16 @@ import {
   hexDataSlice,
   keccak256
 } from 'ethers/lib/utils'
-import { BigNumber, Contract, Signer, Wallet } from 'ethers'
-import { AddressZero, callDataCost, rethrow } from './testutils'
-import { ecsign, toRpcSig, keccak256 as keccak256_buffer } from 'ethereumjs-util'
+import {BigNumber, Contract, Signer, Wallet} from 'ethers'
+import {AddressZero, callDataCost, rethrow} from './testutils'
+import {ecsign, toRpcSig, keccak256 as keccak256_buffer} from 'ethereumjs-util'
 import {
   EntryPoint
 } from '../typechain'
-import { UserOperation } from './UserOperation'
-import { Create2Factory } from '../src/Create2Factory'
+import {UserOperation} from './UserOperation'
+import {Create2Factory} from '../src/Create2Factory'
 
-function encode (typevalues: Array<{ type: string, val: any }>, forSignature: boolean): string {
+function encode(typevalues: Array<{ type: string, val: any }>, forSignature: boolean): string {
   const types = typevalues.map(typevalue => typevalue.type === 'bytes' && forSignature ? 'bytes32' : typevalue.type)
   const values = typevalues.map((typevalue) => typevalue.type === 'bytes' && forSignature ? keccak256(typevalue.val) : typevalue.val)
   return defaultAbiCoder.encode(types, values)
@@ -31,51 +31,51 @@ function encode (typevalues: Array<{ type: string, val: any }>, forSignature: bo
 //   return packed
 // }
 
-export function packUserOp (op: UserOperation, forSignature = true): string {
+export function packUserOp(op: UserOperation, forSignature = true): string {
   if (forSignature) {
     // lighter signature scheme (must match UserOperation#pack): do encode a zero-length signature, but strip afterwards the appended zero-length value
     const userOpType = {
       components: [
-        { type: 'address', name: 'sender' },
-        { type: 'uint256', name: 'nonce' },
-        { type: 'bytes', name: 'initCode' },
-        { type: 'bytes', name: 'callData' },
-        { type: 'uint256', name: 'callGasLimit' },
-        { type: 'uint256', name: 'verificationGasLimit' },
-        { type: 'uint256', name: 'preVerificationGas' },
-        { type: 'uint256', name: 'maxFeePerGas' },
-        { type: 'uint256', name: 'maxPriorityFeePerGas' },
-        { type: 'bytes', name: 'paymasterAndData' },
-        { type: 'bytes', name: 'signature' }
+        {type: 'address', name: 'sender'},
+        {type: 'uint256', name: 'nonce'},
+        {type: 'bytes', name: 'initCode'},
+        {type: 'bytes', name: 'callData'},
+        {type: 'uint256', name: 'callGasLimit'},
+        {type: 'uint256', name: 'verificationGasLimit'},
+        {type: 'uint256', name: 'preVerificationGas'},
+        {type: 'uint256', name: 'maxFeePerGas'},
+        {type: 'uint256', name: 'maxPriorityFeePerGas'},
+        {type: 'bytes', name: 'paymasterAndData'},
+        {type: 'bytes', name: 'signature'}
       ],
       name: 'userOp',
       type: 'tuple'
     }
-    let encoded = defaultAbiCoder.encode([userOpType as any], [{ ...op, signature: '0x' }])
+    let encoded = defaultAbiCoder.encode([userOpType as any], [{...op, signature: '0x'}])
     // remove leading word (total length) and trailing word (zero-length signature)
     encoded = '0x' + encoded.slice(66, encoded.length - 64)
     return encoded
   }
   const typevalues = [
-    { type: 'address', val: op.sender },
-    { type: 'uint256', val: op.nonce },
-    { type: 'bytes', val: op.initCode },
-    { type: 'bytes', val: op.callData },
-    { type: 'uint256', val: op.callGasLimit },
-    { type: 'uint256', val: op.verificationGasLimit },
-    { type: 'uint256', val: op.preVerificationGas },
-    { type: 'uint256', val: op.maxFeePerGas },
-    { type: 'uint256', val: op.maxPriorityFeePerGas },
-    { type: 'bytes', val: op.paymasterAndData }
+    {type: 'address', val: op.sender},
+    {type: 'uint256', val: op.nonce},
+    {type: 'bytes', val: op.initCode},
+    {type: 'bytes', val: op.callData},
+    {type: 'uint256', val: op.callGasLimit},
+    {type: 'uint256', val: op.verificationGasLimit},
+    {type: 'uint256', val: op.preVerificationGas},
+    {type: 'uint256', val: op.maxFeePerGas},
+    {type: 'uint256', val: op.maxPriorityFeePerGas},
+    {type: 'bytes', val: op.paymasterAndData}
   ]
   if (!forSignature) {
     // for the purpose of calculating gas cost, also hash signature
-    typevalues.push({ type: 'bytes', val: op.signature })
+    typevalues.push({type: 'bytes', val: op.signature})
   }
   return encode(typevalues, forSignature)
 }
 
-export function packUserOp1 (op: UserOperation): string {
+export function packUserOp1(op: UserOperation): string {
   return defaultAbiCoder.encode([
     'address', // sender
     'uint256', // nonce
@@ -101,7 +101,7 @@ export function packUserOp1 (op: UserOperation): string {
   ])
 }
 
-export function getUserOpHash (op: UserOperation, entryPoint: string, chainId: number): string {
+export function getUserOpHash(op: UserOperation, entryPoint: string, chainId: number): string {
   const userOpHash = keccak256(packUserOp(op, true))
   const enc = defaultAbiCoder.encode(
     ['bytes32', 'address', 'uint256'],
@@ -115,7 +115,7 @@ export const DefaultsForUserOp: UserOperation = {
   initCode: '0x',
   callData: '0x',
   callGasLimit: 0,
-  verificationGasLimit: 100000, // default verification gas. will add create2 cost (3200+200*length) if initCode exists
+  verificationGasLimit: 150000, // default verification gas. will add create2 cost (3200+200*length) if initCode exists
   preVerificationGas: 21000, // should also cover calldata cost.
   maxFeePerGas: 0,
   maxPriorityFeePerGas: 1e9,
@@ -123,7 +123,7 @@ export const DefaultsForUserOp: UserOperation = {
   signature: '0x'
 }
 
-export function signUserOp (op: UserOperation, signer: Wallet, entryPoint: string, chainId: number): UserOperation {
+export function signUserOp(op: UserOperation, signer: Wallet, entryPoint: string, chainId: number): UserOperation {
   const message = getUserOpHash(op, entryPoint, chainId)
   const msg1 = Buffer.concat([
     Buffer.from('\x19Ethereum Signed Message:\n32', 'ascii'),
@@ -140,8 +140,8 @@ export function signUserOp (op: UserOperation, signer: Wallet, entryPoint: strin
   }
 }
 
-export function fillUserOpDefaults (op: Partial<UserOperation>, defaults = DefaultsForUserOp): UserOperation {
-  const partial: any = { ...op }
+export function fillUserOpDefaults(op: Partial<UserOperation>, defaults = DefaultsForUserOp): UserOperation {
+  const partial: any = {...op}
   // we want "item:undefined" to be used from defaults, and not override defaults, so we must explicitly
   // remove those so "merge" will succeed.
   for (const key in partial) {
@@ -150,7 +150,7 @@ export function fillUserOpDefaults (op: Partial<UserOperation>, defaults = Defau
       delete partial[key]
     }
   }
-  const filled = { ...defaults, ...partial }
+  const filled = {...defaults, ...partial}
   return filled
 }
 
@@ -166,24 +166,26 @@ export function fillUserOpDefaults (op: Partial<UserOperation>, defaults = Defau
 // sender - only in case of construction: fill sender from initCode.
 // callGasLimit: VERY crude estimation (by estimating call to account, and add rough entryPoint overhead
 // verificationGasLimit: hard-code default at 100k. should add "create2" cost
-export async function fillUserOp (op: Partial<UserOperation>, entryPoint?: EntryPoint): Promise<UserOperation> {
-  const op1 = { ...op }
+export async function fillUserOp(op: Partial<UserOperation>, entryPoint?: EntryPoint, getNonceFunction = 'nonce'): Promise<UserOperation> {
+  const op1 = {...op}
   const provider = entryPoint?.provider
   if (op.initCode != null) {
     const initAddr = hexDataSlice(op1.initCode!, 0, 20)
     const initCallData = hexDataSlice(op1.initCode!, 20)
-    if (op1.nonce == null) op1.nonce = 0
-    if (op1.sender == null) {
-      // hack: if the init contract is our known deployer, then we know what the address would be, without a view call
-      if (initAddr.toLowerCase() === Create2Factory.contractAddress.toLowerCase()) {
-        const ctr = hexDataSlice(initCallData, 32)
-        const salt = hexDataSlice(initCallData, 0, 32)
-        op1.sender = Create2Factory.getDeployedAddress(ctr, salt)
+    if (op1.sender == null || op1.nonce == null) {
+
+      if (provider == null) throw new Error('no entrypoint/provider')
+      const {
+        sender,
+        nonce
+      } = await entryPoint!.callStatic.getSenderAddress(op1.initCode!).catch(e => e.errorArgs)
+      if (op1.sender == null ) {
+        op1.sender = sender
       } else {
-        // console.log('\t== not our deployer. our=', Create2Factory.contractAddress, 'got', initAddr)
-        if (provider == null) throw new Error('no entrypoint/provider')
-        op1.sender = await entryPoint!.callStatic.getSenderAddress(op1.initCode!).catch(e => e.errorArgs.sender)
+        //should check that sender == op1.sender.
+        // but we use this fill method in tests to test explicitly the on-chain handling of broken sender
       }
+      op1.nonce = nonce
     }
     if (op1.verificationGasLimit == null) {
       if (provider == null) throw new Error('no entrypoint/provider')
@@ -198,8 +200,9 @@ export async function fillUserOp (op: Partial<UserOperation>, entryPoint?: Entry
   }
   if (op1.nonce == null) {
     if (provider == null) throw new Error('must have entryPoint to autofill nonce')
-    const c = new Contract(op.sender!, ['function nonce() view returns(address)'], provider)
-    op1.nonce = await c.nonce().catch(rethrow())
+    const c = new Contract(op.sender!, [`function ${getNonceFunction}() view returns(address)`], provider)
+    op1.nonce = await c[getNonceFunction]().catch(rethrow())
+    console.log('nonce=', op1.nonce)
   }
   if (op1.callGasLimit == null && op.callData != null) {
     if (provider == null) throw new Error('must have entryPoint for callGasLimit estimate')
@@ -232,9 +235,9 @@ export async function fillUserOp (op: Partial<UserOperation>, entryPoint?: Entry
   return op2
 }
 
-export async function fillAndSign (op: Partial<UserOperation>, signer: Wallet | Signer, entryPoint?: EntryPoint): Promise<UserOperation> {
+export async function fillAndSign(op: Partial<UserOperation>, signer: Wallet | Signer, entryPoint?: EntryPoint, getNonceFunction = 'nonce'): Promise<UserOperation> {
   const provider = entryPoint?.provider
-  const op2 = await fillUserOp(op, entryPoint)
+  const op2 = await fillUserOp(op, entryPoint, getNonceFunction)
 
   const chainId = await provider!.getNetwork().then(net => net.chainId)
   const message = arrayify(getUserOpHash(op2, entryPoint!.address, chainId))
diff --git a/test/simple-wallet.test.ts b/test/simple-wallet.test.ts
index 28444af4..6948839e 100644
--- a/test/simple-wallet.test.ts
+++ b/test/simple-wallet.test.ts
@@ -1,32 +1,37 @@
-import { Wallet } from 'ethers'
-import { ethers } from 'hardhat'
-import { expect } from 'chai'
+import {Wallet} from 'ethers'
+import {ethers} from 'hardhat'
+import {expect} from 'chai'
 import {
+  ERC1967Proxy__factory,
   SimpleAccount,
   SimpleAccountFactory__factory,
+  SimpleAccount__factory,
   TestUtil,
   TestUtil__factory
 } from '../typechain'
 import {
+  createAccount,
   createAddress,
   createAccountOwner,
+  deployEntryPoint,
   getBalance,
   isDeployed,
   ONE_ETH,
-  createAccount, HashZero
+  HashZero,
 } from './testutils'
-import { fillUserOpDefaults, getUserOpHash, packUserOp, signUserOp } from './UserOp'
-import { parseEther } from 'ethers/lib/utils'
-import { UserOperation } from './UserOperation'
+import {fillUserOpDefaults, getUserOpHash, packUserOp, signUserOp} from './UserOp'
+import {parseEther} from 'ethers/lib/utils'
+import {UserOperation} from './UserOperation'
 
 describe('SimpleAccount', function () {
-  const entryPoint = '0x'.padEnd(42, '2')
+  let entryPoint: string
   let accounts: string[]
   let testUtil: TestUtil
   let accountOwner: Wallet
   const ethersSigner = ethers.provider.getSigner()
 
   before(async function () {
+    entryPoint = await deployEntryPoint().then(e => e.address)
     accounts = await ethers.provider.listAccounts()
     // ignore in geth.. this is just a sanity test. should be refactored to use a single-account mode..
     if (accounts.length < 2) this.skip()
@@ -35,18 +40,18 @@ describe('SimpleAccount', function () {
   })
 
   it('owner should be able to call transfer', async () => {
-    const { proxy: account } = await createAccount(ethers.provider.getSigner(), accounts[0], entryPoint)
-    await ethersSigner.sendTransaction({ from: accounts[0], to: account.address, value: parseEther('2') })
+    const {proxy: account} = await createAccount(ethers.provider.getSigner(), accounts[0], entryPoint)
+    await ethersSigner.sendTransaction({from: accounts[0], to: account.address, value: parseEther('2')})
     await account.execute(accounts[2], ONE_ETH, '0x')
   })
   it('other account should not be able to call transfer', async () => {
-    const { proxy: account } = await createAccount(ethers.provider.getSigner(), accounts[0], entryPoint)
+    const {proxy: account} = await createAccount(ethers.provider.getSigner(), accounts[0], entryPoint)
     await expect(account.connect(ethers.provider.getSigner(1)).execute(accounts[2], ONE_ETH, '0x'))
       .to.be.revertedWith('account: not Owner or EntryPoint')
   })
 
   it('should pack in js the same as solidity', async () => {
-    const op = await fillUserOpDefaults({ sender: accounts[0] })
+    const op = await fillUserOpDefaults({sender: accounts[0]})
     const packed = packUserOp(op)
     expect(await testUtil.packUserOp(op)).to.equal(packed)
   })
@@ -59,12 +64,21 @@ describe('SimpleAccount', function () {
     let expectedPay: number
 
     const actualGasPrice = 1e9
+    //for testing directly validateUserOp, we initialize the account with EOA as entryPoint.
+    let entryPointEoa: string
 
     before(async () => {
-      // that's the account of ethersSigner
-      const entryPoint = accounts[2];
-      ({ proxy: account } = await createAccount(await ethers.getSigner(entryPoint), accountOwner.address, entryPoint))
-      await ethersSigner.sendTransaction({ from: accounts[0], to: account.address, value: parseEther('0.2') })
+      entryPointEoa = accounts[2];
+      let epAsSigner = await ethers.getSigner(entryPointEoa);
+      const impl = await new SimpleAccount__factory(await epAsSigner).deploy(entryPointEoa)
+      // ({ proxy: account } = await createAccount(await ethers.getSigner(entryPoint), accountOwner.address, entryPoint))
+
+      // cant use "SimpleAccountFactory", since it attempts to increment nonce first
+      const implementation = await new SimpleAccount__factory(ethersSigner).deploy(entryPointEoa)
+      const proxy = await new ERC1967Proxy__factory(ethersSigner).deploy(implementation.address, "0x")
+      account = SimpleAccount__factory.connect(proxy.address, epAsSigner)
+
+      await ethersSigner.sendTransaction({from: accounts[0], to: account.address, value: parseEther('0.2')})
       const callGasLimit = 200000
       const verificationGasLimit = 100000
       const maxFeePerGas = 3e9
@@ -75,14 +89,14 @@ describe('SimpleAccount', function () {
         callGasLimit,
         verificationGasLimit,
         maxFeePerGas
-      }), accountOwner, entryPoint, chainId)
+      }), accountOwner, entryPointEoa, chainId)
 
-      userOpHash = await getUserOpHash(userOp, entryPoint, chainId)
+      userOpHash = await getUserOpHash(userOp, entryPointEoa, chainId)
 
       expectedPay = actualGasPrice * (callGasLimit + verificationGasLimit)
 
       preBalance = await getBalance(account.address)
-      const ret = await account.validateUserOp(userOp, userOpHash, expectedPay, { gasPrice: actualGasPrice })
+      const ret = await account.validateUserOp(userOp, userOpHash, expectedPay, {gasPrice: actualGasPrice})
       await ret.wait()
     })
 
@@ -93,10 +107,11 @@ describe('SimpleAccount', function () {
 
     it('should return NO_SIG_VALIDATION on wrong signature', async () => {
       const userOpHash = HashZero
-      const deadline = await account.callStatic.validateUserOp({ ...userOp, nonce: 1 }, userOpHash, 0)
+      const deadline = await account.callStatic.validateUserOp({...userOp, nonce: 1}, userOpHash, 0)
       expect(deadline).to.eq(1)
     })
   })
+
   context('SimpleAccountFactory', () => {
     it('sanity: check deployer', async () => {
       const ownerAddr = createAddress()

From b2068fa0bd28ea9d05d011544c272af733795542 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Mon, 20 Mar 2023 20:55:11 +0200
Subject: [PATCH 03/17] incrementNonce for gnosis

---
 contracts/core/NonceManager.sol              | 19 +++++++++++++------
 contracts/interfaces/INonceManager.sol       |  3 ++-
 contracts/samples/SimpleAccount.sol          |  4 ++++
 contracts/samples/gnosis/EIP4337Fallback.sol | 12 +++++++++++-
 contracts/samples/gnosis/EIP4337Manager.sol  | 15 +++++++++------
 test/UserOp.ts                               |  1 -
 test/gnosis.test.ts                          | 17 +++++++++--------
 7 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/contracts/core/NonceManager.sol b/contracts/core/NonceManager.sol
index 2ce08e0e..576b9efd 100644
--- a/contracts/core/NonceManager.sol
+++ b/contracts/core/NonceManager.sol
@@ -11,10 +11,19 @@ contract NonceManager is INonceManager {
 
     mapping(address => mapping(uint192 => uint256)) public nonces;
 
+    uint192 constant KEY_OFFSET = 0;
+
     function getNonce(address sender, uint192 key)
-    external view override returns (uint256 nonce) {
-//        console.log('getNonce sender %s key %s seq %s', sender, uint(key), uint(nonces[sender][key + 1]));
-        return nonces[sender][key + 1] | (uint256(key) << 64);
+    public view override returns (uint256 nonce) {
+        return nonces[sender][key + KEY_OFFSET] | (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 + KEY_OFFSET]++;
     }
 
     /**
@@ -25,9 +34,7 @@ contract NonceManager is INonceManager {
 
         uint192 key = uint192(nonce >> 64);
         uint64 seq = uint64(nonce);
-
-        //        console.log('validateNAndUpdate sender %s nonce %s seq %s', sender, uint(nonce), uint(nonces[sender][key+1]));
-        return nonces[sender][key + 1]++ == seq;
+        return nonces[sender][key + KEY_OFFSET]++ == seq;
     }
 
 }
diff --git a/contracts/interfaces/INonceManager.sol b/contracts/interfaces/INonceManager.sol
index 20a749cf..2eb1063c 100644
--- a/contracts/interfaces/INonceManager.sol
+++ b/contracts/interfaces/INonceManager.sol
@@ -11,5 +11,6 @@ interface INonceManager {
      */
     function getNonce(address sender, uint192 key)
     external view returns (uint256 nonce);
-}
 
+    function incrementNonce(uint192 key) external;
+}
diff --git a/contracts/samples/SimpleAccount.sol b/contracts/samples/SimpleAccount.sol
index d3ace1e7..097a207e 100644
--- a/contracts/samples/SimpleAccount.sol
+++ b/contracts/samples/SimpleAccount.sol
@@ -80,6 +80,10 @@ contract SimpleAccount is BaseAccount, UUPSUpgradeable, Initializable {
      */
     function initialize(address anOwner) public virtual initializer {
         _initialize(anOwner);
+        //increment nonce during construction.
+        // This way, the cost of moving from "0" to "1" is absorbed
+        // by first (construction) transaction, instead of during the 2nd transaction
+        entryPoint().incrementNonce(0);
     }
 
     function _initialize(address anOwner) internal virtual {
diff --git a/contracts/samples/gnosis/EIP4337Fallback.sol b/contracts/samples/gnosis/EIP4337Fallback.sol
index fcd55d95..0e4a5c5d 100644
--- a/contracts/samples/gnosis/EIP4337Fallback.sol
+++ b/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;
         }
     }
-}
\ No newline at end of file
+}
diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol
index bc346863..6229ea02 100644
--- a/contracts/samples/gnosis/EIP4337Manager.sol
+++ b/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,7 @@ 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);
+        UserOperation memory userOp = UserOperation(address(safe), uint256(getNonce()), "", "", 0, 1000000, 0, 0, 0, "", sig);
         UserOperation[] memory userOps = new UserOperation[](1);
         userOps[0] = userOp;
         IEntryPoint _entryPoint = IEntryPoint(payable(manager.entryPoint()));
diff --git a/test/UserOp.ts b/test/UserOp.ts
index 6e0293a4..49d8e8d5 100644
--- a/test/UserOp.ts
+++ b/test/UserOp.ts
@@ -202,7 +202,6 @@ export async function fillUserOp(op: Partial<UserOperation>, entryPoint?: EntryP
     if (provider == null) throw new Error('must have entryPoint to autofill nonce')
     const c = new Contract(op.sender!, [`function ${getNonceFunction}() view returns(address)`], provider)
     op1.nonce = await c[getNonceFunction]().catch(rethrow())
-    console.log('nonce=', op1.nonce)
   }
   if (op1.callGasLimit == null && op.callData != null) {
     if (provider == null) throw new Error('must have entryPoint for callGasLimit estimate')
diff --git a/test/gnosis.test.ts b/test/gnosis.test.ts
index 784b7f91..24819edf 100644
--- a/test/gnosis.test.ts
+++ b/test/gnosis.test.ts
@@ -103,7 +103,7 @@ describe('Gnosis Proxy', function () {
   it('should fail from wrong entrypoint', async function () {
     const op = await fillAndSign({
       sender: proxy.address
-    }, owner, entryPoint)
+    }, owner, entryPoint, 'getNonce')
 
     const anotherEntryPoint = await new EntryPoint__factory(ethersSigner).deploy()
 
@@ -116,14 +116,14 @@ describe('Gnosis Proxy', function () {
       nonce: 1234,
       callGasLimit: 1e6,
       callData: safe_execTxCallData
-    }, owner, entryPoint)
-    await expect(entryPoint.handleOps([op], beneficiary)).to.revertedWith('account: invalid nonce')
+    }, owner, entryPoint, 'getNonce')
+    await expect(entryPoint.handleOps([op], beneficiary)).to.revertedWith('AA25 invalid account nonce')
 
     op = await fillAndSign({
       sender: proxy.address,
       callGasLimit: 1e6,
       callData: safe_execTxCallData
-    }, owner, entryPoint)
+    }, owner, entryPoint, 'getNonce')
     // invalidate the signature
     op.callGasLimit = 1
     await expect(entryPoint.handleOps([op], beneficiary)).to.revertedWith('FailedOp(0, "AA24 signature error")')
@@ -134,7 +134,7 @@ describe('Gnosis Proxy', function () {
       sender: proxy.address,
       callGasLimit: 1e6,
       callData: safe_execTxCallData
-    }, owner, entryPoint)
+    }, owner, entryPoint, 'getNonce')
     const rcpt = await entryPoint.handleOps([op], beneficiary).then(async r => r.wait())
     console.log('gasUsed=', rcpt.gasUsed, rcpt.transactionHash)
 
@@ -151,7 +151,8 @@ describe('Gnosis Proxy', function () {
       sender: proxy.address,
       callGasLimit: 1e6,
       callData: safe_execFailTxCallData
-    }, owner, entryPoint)
+    }, owner, entryPoint, 'getNonce')
+
     const rcpt = await entryPoint.handleOps([op], beneficiary).then(async r => r.wait())
     console.log('gasUsed=', rcpt.gasUsed, rcpt.transactionHash)
 
@@ -183,7 +184,7 @@ describe('Gnosis Proxy', function () {
       sender: counterfactualAddress,
       initCode,
       verificationGasLimit: 400000
-    }, owner, entryPoint)
+    }, owner, entryPoint, 'getNonce')
 
     const rcpt = await entryPoint.handleOps([op], beneficiary).then(async r => r.wait())
     console.log('gasUsed=', rcpt.gasUsed, rcpt.transactionHash)
@@ -200,7 +201,7 @@ describe('Gnosis Proxy', function () {
     const op = await fillAndSign({
       sender: counterfactualAddress,
       callData: safe_execTxCallData
-    }, owner, entryPoint)
+    }, owner, entryPoint, 'getNonce')
 
     const rcpt = await entryPoint.handleOps([op], beneficiary).then(async r => r.wait())
     console.log('gasUsed=', rcpt.gasUsed, rcpt.transactionHash)

From 33a8e54498a7febcdad9fd57e753841f76d47a7a Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Mon, 20 Mar 2023 21:55:37 +0200
Subject: [PATCH 04/17] support gnosis

---
 contracts/core/EntryPoint.sol               | 4 ----
 contracts/core/NonceManager.sol             | 3 +--
 contracts/samples/gnosis/EIP4337Manager.sol | 5 +++--
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol
index 6406b743..6c5a7e38 100644
--- a/contracts/core/EntryPoint.sol
+++ b/contracts/core/EntryPoint.sol
@@ -17,7 +17,6 @@ import "./StakeManager.sol";
 import "./SenderCreator.sol";
 import "./Helpers.sol";
 import "./NonceManager.sol";
-import "hardhat/console.sol";
 
 contract EntryPoint is IEntryPoint, StakeManager, NonceManager {
 
@@ -231,8 +230,6 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager {
     unchecked {
         // handleOps was called with gas limit too low. abort entire bundle.
         if (gasleft() < callGasLimit + mUserOp.verificationGasLimit + 5000) {
-            console.log('left=',gasleft());
-            console.log('needed: ', callGasLimit, mUserOp.verificationGasLimit);
             assembly {
                 mstore(0, INNER_OUT_OF_GAS)
                 revert(0, 32)
@@ -534,7 +531,6 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager {
         uint256 gasUsed = preGas - gasleft();
 
         if (userOp.verificationGasLimit < gasUsed) {
-            console.log('AA40: verGasLimit %s used %s', mUserOp.verificationGasLimit, gasUsed);
             revert FailedOp(opIndex, "AA40 over verificationGasLimit");
         }
         outOpInfo.prefund = requiredPreFund;
diff --git a/contracts/core/NonceManager.sol b/contracts/core/NonceManager.sol
index 576b9efd..105ffd44 100644
--- a/contracts/core/NonceManager.sol
+++ b/contracts/core/NonceManager.sol
@@ -2,7 +2,6 @@
 pragma solidity ^0.8.12;
 
 import "../interfaces/IEntryPoint.sol";
-import "hardhat/console.sol";
 
 /**
  * nonce management functionality
@@ -19,7 +18,7 @@ contract NonceManager is INonceManager {
     }
 
     // allow an account to manually increment its own nonce.
-    // (mainly so that during construction nonce can be made non-zero, 
+    // (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 {
diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol
index 6229ea02..0bc7d565 100644
--- a/contracts/samples/gnosis/EIP4337Manager.sol
+++ b/contracts/samples/gnosis/EIP4337Manager.sol
@@ -106,7 +106,7 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor {
      * as we use the EntryPoint's nonce)
      */
     function getNonce() public view returns (uint256) {
-        return  IEntryPoint(entryPoint).getNonce(address(this), 0);
+        return IEntryPoint(entryPoint).getNonce(address(this), 0);
     }
 
     /**
@@ -161,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(getNonce()), "", "", 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()));

From de441b0a2a43b42795f769c4951ce896075932ae Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Mon, 20 Mar 2023 22:02:51 +0200
Subject: [PATCH 05/17] remove key_offset

---
 contracts/core/NonceManager.sol |  8 +++-----
 reports/gas-checker.txt         | 28 ++++++++++++++--------------
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/contracts/core/NonceManager.sol b/contracts/core/NonceManager.sol
index 105ffd44..e0095cf8 100644
--- a/contracts/core/NonceManager.sol
+++ b/contracts/core/NonceManager.sol
@@ -10,11 +10,9 @@ contract NonceManager is INonceManager {
 
     mapping(address => mapping(uint192 => uint256)) public nonces;
 
-    uint192 constant KEY_OFFSET = 0;
-
     function getNonce(address sender, uint192 key)
     public view override returns (uint256 nonce) {
-        return nonces[sender][key + KEY_OFFSET] | (uint256(key) << 64);
+        return nonces[sender][key] | (uint256(key) << 64);
     }
 
     // allow an account to manually increment its own nonce.
@@ -22,7 +20,7 @@ contract NonceManager is INonceManager {
     // 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 + KEY_OFFSET]++;
+        nonces[msg.sender][key]++;
     }
 
     /**
@@ -33,7 +31,7 @@ contract NonceManager is INonceManager {
 
         uint192 key = uint192(nonce >> 64);
         uint64 seq = uint64(nonce);
-        return nonces[sender][key + KEY_OFFSET]++ == seq;
+        return nonces[sender][key]++ == seq;
     }
 
 }
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index b01ac4e8..f381664d 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -1,35 +1,35 @@
 == gas estimate of direct calling the account's "execFromEntryPoint" method
    the destination is "account.nonce()", which is known to be "hot" address used by this account
    it little higher than EOA call: its an exec from entrypoint (or account owner) into account contract, verifying msg.sender and exec to target)
-- gas estimate "simple" - 31033
-- gas estimate "big tx 5k" - 127284
+- gas estimate "simple" - 32055
+- gas estimate "big tx 5k" - 128306
 ╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗
 ║ handleOps description          │ count │ total gasUsed │ per UserOp gas │ per UserOp overhead ║
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │     account.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         75948 │                │                     ║
+║ simple                         │     1 │         78868 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          41449 │               10416 ║
+║ simple - diff from previous    │     2 │               │          44311 │               12256 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        449053 │                │                     ║
+║ simple                         │    10 │        477887 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          41560 │               10527 ║
+║ simple - diff from previous    │    11 │               │          44362 │               12307 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         82245 │                │                     ║
+║ simple paymaster               │     1 │         85153 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          40408 │                9375 ║
+║ simple paymaster with diff     │     2 │               │          43294 │               11239 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        446306 │                │                     ║
+║ simple paymaster               │    10 │        475200 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          40604 │                9571 ║
+║ simple paymaster with diff     │    11 │               │          43346 │               11291 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        177693 │                │                     ║
+║ big tx 5k                      │     1 │        180577 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         142772 │               15488 ║
+║ big tx - diff from previous    │     2 │               │         145646 │               17340 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1468115 │                │                     ║
+║ big tx 5k                      │    10 │       1497057 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         144290 │               17006 ║
+║ big tx - diff from previous    │    11 │               │         147056 │               18750 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 

From 98ee47421ad96ba8341b46c1356199989c3d0c62 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Mon, 20 Mar 2023 22:05:51 +0200
Subject: [PATCH 06/17] lints

---
 deploy/2_deploy_SimpleAccountFactory.ts |  3 +-
 test/UserOp.ts                          | 82 ++++++++++++-------------
 test/entrypoint.test.ts                 | 11 ++--
 test/simple-wallet.test.ts              | 38 ++++++------
 test/y.bls.test.ts                      |  2 +-
 5 files changed, 66 insertions(+), 70 deletions(-)

diff --git a/deploy/2_deploy_SimpleAccountFactory.ts b/deploy/2_deploy_SimpleAccountFactory.ts
index 43792ff0..d1716d4f 100644
--- a/deploy/2_deploy_SimpleAccountFactory.ts
+++ b/deploy/2_deploy_SimpleAccountFactory.ts
@@ -11,7 +11,8 @@ const deploySimpleAccountFactory: DeployFunction = async function (hre: HardhatR
     'SimpleAccountFactory', {
       from,
       args: [entrypoint.address],
-      gasLimit: 6e6, log: true,
+      gasLimit: 6e6,
+      log: true,
       deterministicDeployment: true
     })
   console.log('==SimpleAccountFactory addr=', ret.address)
diff --git a/test/UserOp.ts b/test/UserOp.ts
index 49d8e8d5..4e9c5a99 100644
--- a/test/UserOp.ts
+++ b/test/UserOp.ts
@@ -4,16 +4,15 @@ import {
   hexDataSlice,
   keccak256
 } from 'ethers/lib/utils'
-import {BigNumber, Contract, Signer, Wallet} from 'ethers'
-import {AddressZero, callDataCost, rethrow} from './testutils'
-import {ecsign, toRpcSig, keccak256 as keccak256_buffer} from 'ethereumjs-util'
+import { BigNumber, Contract, Signer, Wallet } from 'ethers'
+import { AddressZero, callDataCost, rethrow } from './testutils'
+import { ecsign, toRpcSig, keccak256 as keccak256_buffer } from 'ethereumjs-util'
 import {
   EntryPoint
 } from '../typechain'
-import {UserOperation} from './UserOperation'
-import {Create2Factory} from '../src/Create2Factory'
+import { UserOperation } from './UserOperation'
 
-function encode(typevalues: Array<{ type: string, val: any }>, forSignature: boolean): string {
+function encode (typevalues: Array<{ type: string, val: any }>, forSignature: boolean): string {
   const types = typevalues.map(typevalue => typevalue.type === 'bytes' && forSignature ? 'bytes32' : typevalue.type)
   const values = typevalues.map((typevalue) => typevalue.type === 'bytes' && forSignature ? keccak256(typevalue.val) : typevalue.val)
   return defaultAbiCoder.encode(types, values)
@@ -31,51 +30,51 @@ function encode(typevalues: Array<{ type: string, val: any }>, forSignature: boo
 //   return packed
 // }
 
-export function packUserOp(op: UserOperation, forSignature = true): string {
+export function packUserOp (op: UserOperation, forSignature = true): string {
   if (forSignature) {
     // lighter signature scheme (must match UserOperation#pack): do encode a zero-length signature, but strip afterwards the appended zero-length value
     const userOpType = {
       components: [
-        {type: 'address', name: 'sender'},
-        {type: 'uint256', name: 'nonce'},
-        {type: 'bytes', name: 'initCode'},
-        {type: 'bytes', name: 'callData'},
-        {type: 'uint256', name: 'callGasLimit'},
-        {type: 'uint256', name: 'verificationGasLimit'},
-        {type: 'uint256', name: 'preVerificationGas'},
-        {type: 'uint256', name: 'maxFeePerGas'},
-        {type: 'uint256', name: 'maxPriorityFeePerGas'},
-        {type: 'bytes', name: 'paymasterAndData'},
-        {type: 'bytes', name: 'signature'}
+        { type: 'address', name: 'sender' },
+        { type: 'uint256', name: 'nonce' },
+        { type: 'bytes', name: 'initCode' },
+        { type: 'bytes', name: 'callData' },
+        { type: 'uint256', name: 'callGasLimit' },
+        { type: 'uint256', name: 'verificationGasLimit' },
+        { type: 'uint256', name: 'preVerificationGas' },
+        { type: 'uint256', name: 'maxFeePerGas' },
+        { type: 'uint256', name: 'maxPriorityFeePerGas' },
+        { type: 'bytes', name: 'paymasterAndData' },
+        { type: 'bytes', name: 'signature' }
       ],
       name: 'userOp',
       type: 'tuple'
     }
-    let encoded = defaultAbiCoder.encode([userOpType as any], [{...op, signature: '0x'}])
+    let encoded = defaultAbiCoder.encode([userOpType as any], [{ ...op, signature: '0x' }])
     // remove leading word (total length) and trailing word (zero-length signature)
     encoded = '0x' + encoded.slice(66, encoded.length - 64)
     return encoded
   }
   const typevalues = [
-    {type: 'address', val: op.sender},
-    {type: 'uint256', val: op.nonce},
-    {type: 'bytes', val: op.initCode},
-    {type: 'bytes', val: op.callData},
-    {type: 'uint256', val: op.callGasLimit},
-    {type: 'uint256', val: op.verificationGasLimit},
-    {type: 'uint256', val: op.preVerificationGas},
-    {type: 'uint256', val: op.maxFeePerGas},
-    {type: 'uint256', val: op.maxPriorityFeePerGas},
-    {type: 'bytes', val: op.paymasterAndData}
+    { type: 'address', val: op.sender },
+    { type: 'uint256', val: op.nonce },
+    { type: 'bytes', val: op.initCode },
+    { type: 'bytes', val: op.callData },
+    { type: 'uint256', val: op.callGasLimit },
+    { type: 'uint256', val: op.verificationGasLimit },
+    { type: 'uint256', val: op.preVerificationGas },
+    { type: 'uint256', val: op.maxFeePerGas },
+    { type: 'uint256', val: op.maxPriorityFeePerGas },
+    { type: 'bytes', val: op.paymasterAndData }
   ]
   if (!forSignature) {
     // for the purpose of calculating gas cost, also hash signature
-    typevalues.push({type: 'bytes', val: op.signature})
+    typevalues.push({ type: 'bytes', val: op.signature })
   }
   return encode(typevalues, forSignature)
 }
 
-export function packUserOp1(op: UserOperation): string {
+export function packUserOp1 (op: UserOperation): string {
   return defaultAbiCoder.encode([
     'address', // sender
     'uint256', // nonce
@@ -101,7 +100,7 @@ export function packUserOp1(op: UserOperation): string {
   ])
 }
 
-export function getUserOpHash(op: UserOperation, entryPoint: string, chainId: number): string {
+export function getUserOpHash (op: UserOperation, entryPoint: string, chainId: number): string {
   const userOpHash = keccak256(packUserOp(op, true))
   const enc = defaultAbiCoder.encode(
     ['bytes32', 'address', 'uint256'],
@@ -123,7 +122,7 @@ export const DefaultsForUserOp: UserOperation = {
   signature: '0x'
 }
 
-export function signUserOp(op: UserOperation, signer: Wallet, entryPoint: string, chainId: number): UserOperation {
+export function signUserOp (op: UserOperation, signer: Wallet, entryPoint: string, chainId: number): UserOperation {
   const message = getUserOpHash(op, entryPoint, chainId)
   const msg1 = Buffer.concat([
     Buffer.from('\x19Ethereum Signed Message:\n32', 'ascii'),
@@ -140,8 +139,8 @@ export function signUserOp(op: UserOperation, signer: Wallet, entryPoint: string
   }
 }
 
-export function fillUserOpDefaults(op: Partial<UserOperation>, defaults = DefaultsForUserOp): UserOperation {
-  const partial: any = {...op}
+export function fillUserOpDefaults (op: Partial<UserOperation>, defaults = DefaultsForUserOp): UserOperation {
+  const partial: any = { ...op }
   // we want "item:undefined" to be used from defaults, and not override defaults, so we must explicitly
   // remove those so "merge" will succeed.
   for (const key in partial) {
@@ -150,7 +149,7 @@ export function fillUserOpDefaults(op: Partial<UserOperation>, defaults = Defaul
       delete partial[key]
     }
   }
-  const filled = {...defaults, ...partial}
+  const filled = { ...defaults, ...partial }
   return filled
 }
 
@@ -166,23 +165,22 @@ export function fillUserOpDefaults(op: Partial<UserOperation>, defaults = Defaul
 // sender - only in case of construction: fill sender from initCode.
 // callGasLimit: VERY crude estimation (by estimating call to account, and add rough entryPoint overhead
 // verificationGasLimit: hard-code default at 100k. should add "create2" cost
-export async function fillUserOp(op: Partial<UserOperation>, entryPoint?: EntryPoint, getNonceFunction = 'nonce'): Promise<UserOperation> {
-  const op1 = {...op}
+export async function fillUserOp (op: Partial<UserOperation>, entryPoint?: EntryPoint, getNonceFunction = 'nonce'): Promise<UserOperation> {
+  const op1 = { ...op }
   const provider = entryPoint?.provider
   if (op.initCode != null) {
     const initAddr = hexDataSlice(op1.initCode!, 0, 20)
     const initCallData = hexDataSlice(op1.initCode!, 20)
     if (op1.sender == null || op1.nonce == null) {
-
       if (provider == null) throw new Error('no entrypoint/provider')
       const {
         sender,
         nonce
       } = await entryPoint!.callStatic.getSenderAddress(op1.initCode!).catch(e => e.errorArgs)
-      if (op1.sender == null ) {
+      if (op1.sender == null) {
         op1.sender = sender
       } else {
-        //should check that sender == op1.sender.
+        // should check that sender == op1.sender.
         // but we use this fill method in tests to test explicitly the on-chain handling of broken sender
       }
       op1.nonce = nonce
@@ -234,7 +232,7 @@ export async function fillUserOp(op: Partial<UserOperation>, entryPoint?: EntryP
   return op2
 }
 
-export async function fillAndSign(op: Partial<UserOperation>, signer: Wallet | Signer, entryPoint?: EntryPoint, getNonceFunction = 'nonce'): Promise<UserOperation> {
+export async function fillAndSign (op: Partial<UserOperation>, signer: Wallet | Signer, entryPoint?: EntryPoint, getNonceFunction = 'nonce'): Promise<UserOperation> {
   const provider = entryPoint?.provider
   const op2 = await fillUserOp(op, entryPoint, getNonceFunction)
 
diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts
index ac2e467a..c3c4efa1 100644
--- a/test/entrypoint.test.ts
+++ b/test/entrypoint.test.ts
@@ -20,8 +20,7 @@ import {
   TestSignatureAggregator,
   TestSignatureAggregator__factory,
   MaliciousAccount__factory,
-  TestWarmColdAccount__factory,
-  SimpleAccount__factory
+  TestWarmColdAccount__factory
 } from '../typechain'
 import {
   AddressZero,
@@ -412,7 +411,7 @@ describe('EntryPoint', function () {
 
       const userOp: UserOperation = {
         sender: maliciousAccount.address,
-        nonce: await entryPoint.getNonce(maliciousAccount.address,0),
+        nonce: await entryPoint.getNonce(maliciousAccount.address, 0),
         signature: defaultAbiCoder.encode(['uint256'], [block.baseFeePerGas]),
         initCode: '0x',
         callData: '0x',
@@ -422,7 +421,7 @@ describe('EntryPoint', function () {
         // we need maxFeeperGas > block.basefee + maxPriorityFeePerGas so requiredPrefund onchain is basefee + maxPriorityFeePerGas
         maxFeePerGas: block.baseFeePerGas.mul(3),
         maxPriorityFeePerGas: block.baseFeePerGas,
-        paymasterAndData: '0x',
+        paymasterAndData: '0x'
       }
       try {
         await expect(entryPoint.simulateValidation(userOp, { gasLimit: 1e6 }))
@@ -455,7 +454,7 @@ describe('EntryPoint', function () {
       const beneficiaryAddress = createAddress()
       await expect(entryPoint.simulateValidation(badOp, { gasLimit: 3e5 }))
         .to.revertedWith('ValidationResult')
-      const tx = await entryPoint.handleOps([badOp], beneficiaryAddress, ) //{ gasLimit: 3e5 })
+      const tx = await entryPoint.handleOps([badOp], beneficiaryAddress) // { gasLimit: 3e5 })
       const receipt = await tx.wait()
       const userOperationRevertReasonEvent = receipt.events?.find(event => event.event === 'UserOperationRevertReason')
       expect(userOperationRevertReasonEvent?.event).to.equal('UserOperationRevertReason')
@@ -1028,7 +1027,7 @@ describe('EntryPoint', function () {
             addr = await entryPoint.callStatic.getSenderAddress(initCode).catch(e => e.errorArgs.sender)
             await ethersSigner.sendTransaction({ to: addr, value: parseEther('0.1') })
             userOp = await fillAndSign({
-              initCode,
+              initCode
             }, accountOwner, entryPoint)
           })
           it('simulateValidation should return aggregator and its stake', async () => {
diff --git a/test/simple-wallet.test.ts b/test/simple-wallet.test.ts
index 6948839e..1a57054d 100644
--- a/test/simple-wallet.test.ts
+++ b/test/simple-wallet.test.ts
@@ -1,6 +1,6 @@
-import {Wallet} from 'ethers'
-import {ethers} from 'hardhat'
-import {expect} from 'chai'
+import { Wallet } from 'ethers'
+import { ethers } from 'hardhat'
+import { expect } from 'chai'
 import {
   ERC1967Proxy__factory,
   SimpleAccount,
@@ -17,11 +17,11 @@ import {
   getBalance,
   isDeployed,
   ONE_ETH,
-  HashZero,
+  HashZero
 } from './testutils'
-import {fillUserOpDefaults, getUserOpHash, packUserOp, signUserOp} from './UserOp'
-import {parseEther} from 'ethers/lib/utils'
-import {UserOperation} from './UserOperation'
+import { fillUserOpDefaults, getUserOpHash, packUserOp, signUserOp } from './UserOp'
+import { parseEther } from 'ethers/lib/utils'
+import { UserOperation } from './UserOperation'
 
 describe('SimpleAccount', function () {
   let entryPoint: string
@@ -40,18 +40,18 @@ describe('SimpleAccount', function () {
   })
 
   it('owner should be able to call transfer', async () => {
-    const {proxy: account} = await createAccount(ethers.provider.getSigner(), accounts[0], entryPoint)
-    await ethersSigner.sendTransaction({from: accounts[0], to: account.address, value: parseEther('2')})
+    const { proxy: account } = await createAccount(ethers.provider.getSigner(), accounts[0], entryPoint)
+    await ethersSigner.sendTransaction({ from: accounts[0], to: account.address, value: parseEther('2') })
     await account.execute(accounts[2], ONE_ETH, '0x')
   })
   it('other account should not be able to call transfer', async () => {
-    const {proxy: account} = await createAccount(ethers.provider.getSigner(), accounts[0], entryPoint)
+    const { proxy: account } = await createAccount(ethers.provider.getSigner(), accounts[0], entryPoint)
     await expect(account.connect(ethers.provider.getSigner(1)).execute(accounts[2], ONE_ETH, '0x'))
       .to.be.revertedWith('account: not Owner or EntryPoint')
   })
 
   it('should pack in js the same as solidity', async () => {
-    const op = await fillUserOpDefaults({sender: accounts[0]})
+    const op = await fillUserOpDefaults({ sender: accounts[0] })
     const packed = packUserOp(op)
     expect(await testUtil.packUserOp(op)).to.equal(packed)
   })
@@ -64,21 +64,19 @@ describe('SimpleAccount', function () {
     let expectedPay: number
 
     const actualGasPrice = 1e9
-    //for testing directly validateUserOp, we initialize the account with EOA as entryPoint.
+    // for testing directly validateUserOp, we initialize the account with EOA as entryPoint.
     let entryPointEoa: string
 
     before(async () => {
-      entryPointEoa = accounts[2];
-      let epAsSigner = await ethers.getSigner(entryPointEoa);
-      const impl = await new SimpleAccount__factory(await epAsSigner).deploy(entryPointEoa)
-      // ({ proxy: account } = await createAccount(await ethers.getSigner(entryPoint), accountOwner.address, entryPoint))
+      entryPointEoa = accounts[2]
+      const epAsSigner = await ethers.getSigner(entryPointEoa)
 
       // cant use "SimpleAccountFactory", since it attempts to increment nonce first
       const implementation = await new SimpleAccount__factory(ethersSigner).deploy(entryPointEoa)
-      const proxy = await new ERC1967Proxy__factory(ethersSigner).deploy(implementation.address, "0x")
+      const proxy = await new ERC1967Proxy__factory(ethersSigner).deploy(implementation.address, '0x')
       account = SimpleAccount__factory.connect(proxy.address, epAsSigner)
 
-      await ethersSigner.sendTransaction({from: accounts[0], to: account.address, value: parseEther('0.2')})
+      await ethersSigner.sendTransaction({ from: accounts[0], to: account.address, value: parseEther('0.2') })
       const callGasLimit = 200000
       const verificationGasLimit = 100000
       const maxFeePerGas = 3e9
@@ -96,7 +94,7 @@ describe('SimpleAccount', function () {
       expectedPay = actualGasPrice * (callGasLimit + verificationGasLimit)
 
       preBalance = await getBalance(account.address)
-      const ret = await account.validateUserOp(userOp, userOpHash, expectedPay, {gasPrice: actualGasPrice})
+      const ret = await account.validateUserOp(userOp, userOpHash, expectedPay, { gasPrice: actualGasPrice })
       await ret.wait()
     })
 
@@ -107,7 +105,7 @@ describe('SimpleAccount', function () {
 
     it('should return NO_SIG_VALIDATION on wrong signature', async () => {
       const userOpHash = HashZero
-      const deadline = await account.callStatic.validateUserOp({...userOp, nonce: 1}, userOpHash, 0)
+      const deadline = await account.callStatic.validateUserOp({ ...userOp, nonce: 1 }, userOpHash, 0)
       expect(deadline).to.eq(1)
     })
   })
diff --git a/test/y.bls.test.ts b/test/y.bls.test.ts
index 3efe8003..6dae29ee 100644
--- a/test/y.bls.test.ts
+++ b/test/y.bls.test.ts
@@ -184,7 +184,7 @@ describe('bls account', function () {
       await fund(senderAddress, '0.01')
       const userOp = await fillUserOp({
         sender: senderAddress,
-        initCode,
+        initCode
       }, entrypoint)
       const requestHash = await blsAgg.getUserOpHash(userOp)
       const sigParts = signer3.sign(requestHash)

From 60e869c92f2f388e4fd8eccfbce9dcb1f7ea54ca Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Mon, 20 Mar 2023 22:31:31 +0200
Subject: [PATCH 07/17] gas calcs

---
 reports/gas-checker.txt | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index f381664d..2c38d7e7 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -1,35 +1,35 @@
 == gas estimate of direct calling the account's "execFromEntryPoint" method
    the destination is "account.nonce()", which is known to be "hot" address used by this account
    it little higher than EOA call: its an exec from entrypoint (or account owner) into account contract, verifying msg.sender and exec to target)
-- gas estimate "simple" - 32055
-- gas estimate "big tx 5k" - 128306
+- gas estimate "simple" - 31996
+- gas estimate "big tx 5k" - 128247
 ╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗
 ║ handleOps description          │ count │ total gasUsed │ per UserOp gas │ per UserOp overhead ║
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │     account.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         78868 │                │                     ║
+║ simple                         │     1 │         78752 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          44311 │               12256 ║
+║ simple - diff from previous    │     2 │               │          44183 │               12187 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        477887 │                │                     ║
+║ simple                         │    10 │        476727 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          44362 │               12307 ║
+║ simple - diff from previous    │    11 │               │          44234 │               12238 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         85153 │                │                     ║
+║ simple paymaster               │     1 │         85025 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          43294 │               11239 ║
+║ simple paymaster with diff     │     2 │               │          43202 │               11206 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        475200 │                │                     ║
+║ simple paymaster               │    10 │        473992 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          43346 │               11291 ║
+║ simple paymaster with diff     │    11 │               │          43302 │               11306 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        180577 │                │                     ║
+║ big tx 5k                      │     1 │        180485 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         145646 │               17340 ║
+║ big tx - diff from previous    │     2 │               │         145518 │               17271 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1497057 │                │                     ║
+║ big tx 5k                      │    10 │       1495813 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         147056 │               18750 ║
+║ big tx - diff from previous    │    11 │               │         146964 │               18717 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 

From d0a9b119f480f4ee6c2e375ce5e40b7658c79770 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Thu, 23 Mar 2023 13:03:12 +0200
Subject: [PATCH 08/17] Remove initial nonce increment on create

create account doesn't increment nonce automatically.
The nonce is incremented if it is created through a UserOp

Side-effect:
if account is created NOT through AA, then first UserOp will cost 20000
gas more.
(if created as a userop, then this extra cost is part of the "creation"
and not on a separate transaction.
---
 contracts/samples/SimpleAccount.sol |  4 ----
 gascalc/GasChecker.ts               | 30 +++++++++++++++++++++++++++--
 reports/gas-checker.txt             | 22 ++++++++++-----------
 3 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/contracts/samples/SimpleAccount.sol b/contracts/samples/SimpleAccount.sol
index 097a207e..d3ace1e7 100644
--- a/contracts/samples/SimpleAccount.sol
+++ b/contracts/samples/SimpleAccount.sol
@@ -80,10 +80,6 @@ contract SimpleAccount is BaseAccount, UUPSUpgradeable, Initializable {
      */
     function initialize(address anOwner) public virtual initializer {
         _initialize(anOwner);
-        //increment nonce during construction.
-        // This way, the cost of moving from "0" to "1" is absorbed
-        // by first (construction) transaction, instead of during the 2nd transaction
-        entryPoint().incrementNonce(0);
     }
 
     function _initialize(address anOwner) internal virtual {
diff --git a/gascalc/GasChecker.ts b/gascalc/GasChecker.ts
index dc32b751..5f976dfc 100644
--- a/gascalc/GasChecker.ts
+++ b/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
   }
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index 2c38d7e7..98fb2b3b 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -10,26 +10,26 @@
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ simple                         │     1 │         78752 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          44183 │               12187 ║
+║ simple - diff from previous    │     2 │               │          44195 │               12199 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        476727 │                │                     ║
+║ simple                         │    10 │        476739 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          44234 │               12238 ║
+║ simple - diff from previous    │    11 │               │          44246 │               12250 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         85025 │                │                     ║
+║ simple paymaster               │     1 │         85037 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          43202 │               11206 ║
+║ simple paymaster with diff     │     2 │               │          43190 │               11194 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        473992 │                │                     ║
+║ simple paymaster               │    10 │        474016 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          43302 │               11306 ║
+║ simple paymaster with diff     │    11 │               │          43230 │               11234 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        180485 │                │                     ║
+║ big tx 5k                      │     1 │        180473 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         145518 │               17271 ║
+║ big tx - diff from previous    │     2 │               │         145530 │               17283 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1495813 │                │                     ║
+║ big tx 5k                      │    10 │       1495885 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         146964 │               18717 ║
+║ big tx - diff from previous    │    11 │               │         146916 │               18669 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 

From 85371d172a0ceecbf2af919ffd880cbd1c5fab4c Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Thu, 23 Mar 2023 16:59:25 +0200
Subject: [PATCH 09/17] PR comments.

---
 contracts/core/EntryPoint.sol               |  3 +--
 contracts/interfaces/IEntryPoint.sol        |  2 +-
 contracts/interfaces/INonceManager.sol      |  8 +++++++
 contracts/samples/gnosis/EIP4337Manager.sol |  5 +++-
 gascalc/GasChecker.ts                       |  2 --
 reports/gas-checker.txt                     | 18 +++++++-------
 test/UserOp.ts                              | 26 ++++++++++-----------
 7 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol
index 6c5a7e38..d9150c34 100644
--- a/contracts/core/EntryPoint.sol
+++ b/contracts/core/EntryPoint.sol
@@ -351,8 +351,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager {
      */
     function getSenderAddress(bytes calldata initCode) public {
         address sender = senderCreator.createSender(initCode);
-        uint256 nonce = getNonce(sender, 0);
-        revert SenderAddressResult(sender, nonce);
+        revert SenderAddressResult(sender);
     }
 
     function _simulationOnlyValidations(UserOperation calldata userOp) internal view {
diff --git a/contracts/interfaces/IEntryPoint.sol b/contracts/interfaces/IEntryPoint.sol
index 68dd4a12..580a1a84 100644
--- a/contracts/interfaces/IEntryPoint.sol
+++ b/contracts/interfaces/IEntryPoint.sol
@@ -94,7 +94,7 @@ interface IEntryPoint is IStakeManager, INonceManager {
     /**
      * return value of getSenderAddress
      */
-    error SenderAddressResult(address sender, uint nonce);
+    error SenderAddressResult(address sender);
 
     /**
      * return value of simulateHandleOp
diff --git a/contracts/interfaces/INonceManager.sol b/contracts/interfaces/INonceManager.sol
index 2eb1063c..fd077df0 100644
--- a/contracts/interfaces/INonceManager.sol
+++ b/contracts/interfaces/INonceManager.sol
@@ -12,5 +12,13 @@ interface INonceManager {
     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;
 }
diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol
index 0bc7d565..eb36b611 100644
--- a/contracts/samples/gnosis/EIP4337Manager.sol
+++ b/contracts/samples/gnosis/EIP4337Manager.sol
@@ -55,6 +55,9 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor {
             validationData = SIG_VALIDATION_FAILED;
         }
 
+        // 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
             (bool success,) = payable(msgSender).call{value : missingAccountFunds}("");
@@ -161,7 +164,7 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor {
         sig[64] = bytes1(uint8(27));
         sig[2] = bytes1(uint8(1));
         sig[35] = bytes1(uint8(1));
-        uint nonce = uint256(IEntryPoint(manager.entryPoint()).getNonce(address(safe), 0));
+        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;
diff --git a/gascalc/GasChecker.ts b/gascalc/GasChecker.ts
index 5f976dfc..f9c6060e 100644
--- a/gascalc/GasChecker.ts
+++ b/gascalc/GasChecker.ts
@@ -161,9 +161,7 @@ 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')
   }
 
   /**
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index 98fb2b3b..c8be1e52 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -10,26 +10,26 @@
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ simple                         │     1 │         78752 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          44195 │               12199 ║
+║ simple - diff from previous    │     2 │               │          44183 │               12187 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        476739 │                │                     ║
+║ simple                         │    10 │        476715 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          44246 │               12250 ║
+║ simple - diff from previous    │    11 │               │          44270 │               12274 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ simple paymaster               │     1 │         85037 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          43190 │               11194 ║
+║ simple paymaster with diff     │     2 │               │          43142 │               11146 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        474016 │                │                     ║
+║ simple paymaster               │    10 │        473992 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          43230 │               11234 ║
+║ simple paymaster with diff     │    11 │               │          43254 │               11258 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        180473 │                │                     ║
+║ big tx 5k                      │     1 │        180485 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ big tx - diff from previous    │     2 │               │         145530 │               17283 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1495885 │                │                     ║
+║ big tx 5k                      │    10 │       1495837 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         146916 │               18669 ║
+║ big tx - diff from previous    │    11 │               │         146976 │               18729 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 
diff --git a/test/UserOp.ts b/test/UserOp.ts
index 4e9c5a99..6fde2265 100644
--- a/test/UserOp.ts
+++ b/test/UserOp.ts
@@ -11,6 +11,7 @@ import {
   EntryPoint
 } from '../typechain'
 import { UserOperation } from './UserOperation'
+import { Create2Factory } from '../src/Create2Factory'
 
 function encode (typevalues: Array<{ type: string, val: any }>, forSignature: boolean): string {
   const types = typevalues.map(typevalue => typevalue.type === 'bytes' && forSignature ? 'bytes32' : typevalue.type)
@@ -81,8 +82,8 @@ export function packUserOp1 (op: UserOperation): string {
     'bytes32', // initCode
     'bytes32', // callData
     'uint256', // callGasLimit
-    'uint', // verificationGasLimit
-    'uint', // preVerificationGas
+    'uint256', // verificationGasLimit
+    'uint256', // preVerificationGas
     'uint256', // maxFeePerGas
     'uint256', // maxPriorityFeePerGas
     'bytes32' // paymasterAndData
@@ -171,19 +172,18 @@ export async function fillUserOp (op: Partial<UserOperation>, entryPoint?: Entry
   if (op.initCode != null) {
     const initAddr = hexDataSlice(op1.initCode!, 0, 20)
     const initCallData = hexDataSlice(op1.initCode!, 20)
-    if (op1.sender == null || op1.nonce == null) {
-      if (provider == null) throw new Error('no entrypoint/provider')
-      const {
-        sender,
-        nonce
-      } = await entryPoint!.callStatic.getSenderAddress(op1.initCode!).catch(e => e.errorArgs)
-      if (op1.sender == null) {
-        op1.sender = sender
+    if (op1.nonce == null) op1.nonce = 0
+    if (op1.sender == null) {
+      // hack: if the init contract is our known deployer, then we know what the address would be, without a view call
+      if (initAddr.toLowerCase() === Create2Factory.contractAddress.toLowerCase()) {
+        const ctr = hexDataSlice(initCallData, 32)
+        const salt = hexDataSlice(initCallData, 0, 32)
+        op1.sender = Create2Factory.getDeployedAddress(ctr, salt)
       } else {
-        // should check that sender == op1.sender.
-        // but we use this fill method in tests to test explicitly the on-chain handling of broken sender
+        // console.log('\t== not our deployer. our=', Create2Factory.contractAddress, 'got', initAddr)
+        if (provider == null) throw new Error('no entrypoint/provider')
+        op1.sender = await entryPoint!.callStatic.getSenderAddress(op1.initCode!).catch(e => e.errorArgs.sender)
       }
-      op1.nonce = nonce
     }
     if (op1.verificationGasLimit == null) {
       if (provider == null) throw new Error('no entrypoint/provider')

From 33cbda98d69494031ccd741cec2e28dbf7187300 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Sun, 26 Mar 2023 14:17:14 +0300
Subject: [PATCH 10/17] eip updates, added test.

---
 eip/EIPS/eip-4337.md    | 17 ++++++++++++--
 test/entrypoint.test.ts | 49 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/eip/EIPS/eip-4337.md b/eip/EIPS/eip-4337.md
index aed39541..67e47fbb 100644
--- a/eip/EIPS/eip-4337.md
+++ b/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):
 
diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts
index c3c4efa1..b30911c3 100644
--- a/test/entrypoint.test.ts
+++ b/test/entrypoint.test.ts
@@ -511,6 +511,55 @@ describe('EntryPoint', function () {
     })
   })
 
+  describe('2d nonces', () => {
+    const beneficiaryAddress = createAddress()
+    let sender: string
+    const key = 1 << 64
+
+    before(async () => {
+      console.log(1)
+      const { proxy } = await createAccount(ethersSigner, accountOwner.address, entryPoint.address)
+      sender = proxy.address
+      await fund(sender)
+      console.log(2)
+    })
+
+    it('should fail nonce with new key and seq!=0', async () => {
+      console.log(3)
+      const op = await fillAndSign({
+        sender,
+        nonce: key + 123
+      }, accountOwner, entryPoint)
+      await expect(entryPoint.callStatic.handleOps([op], beneficiaryAddress)).to.revertedWith('AA25 invalid account nonce')
+    })
+
+    describe('with key=1, seq=1', () => {
+      before(async () => {
+        const op = await fillAndSign({
+          sender,
+          nonce: key
+        }, accountOwner, entryPoint)
+        await entryPoint.handleOps([op], beneficiaryAddress)
+      })
+
+      it('should allow to increment nonce of different key', async () => {
+        const op = await fillAndSign({
+          sender,
+          nonce: key + 1
+        }, accountOwner, entryPoint)
+        await entryPoint.callStatic.handleOps([op], beneficiaryAddress)
+      })
+
+      it('should fail with nonsequential seq', async () => {
+        const op = await fillAndSign({
+          sender,
+          nonce: key + 3
+        }, accountOwner, entryPoint)
+        await expect(entryPoint.callStatic.handleOps([op], beneficiaryAddress)).to.revertedWith('nonce1')
+      })
+    })
+  })
+
   describe('without paymaster (account pays in eth)', () => {
     describe('#handleOps', () => {
       let counter: TestCounter

From ac89c73bbebd30fee92d7b684238329ec18cfcd6 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Sun, 26 Mar 2023 15:08:15 +0300
Subject: [PATCH 11/17] fix tests

---
 test/entrypoint.test.ts | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts
index b30911c3..505e0368 100644
--- a/test/entrypoint.test.ts
+++ b/test/entrypoint.test.ts
@@ -514,21 +514,19 @@ describe('EntryPoint', function () {
   describe('2d nonces', () => {
     const beneficiaryAddress = createAddress()
     let sender: string
-    const key = 1 << 64
+    const key = 1
+    const keyShifted = BigNumber.from(key).shl(64)
 
     before(async () => {
-      console.log(1)
       const { proxy } = await createAccount(ethersSigner, accountOwner.address, entryPoint.address)
       sender = proxy.address
       await fund(sender)
-      console.log(2)
     })
 
     it('should fail nonce with new key and seq!=0', async () => {
-      console.log(3)
       const op = await fillAndSign({
         sender,
-        nonce: key + 123
+        nonce: keyShifted.add(1)
       }, accountOwner, entryPoint)
       await expect(entryPoint.callStatic.handleOps([op], beneficiaryAddress)).to.revertedWith('AA25 invalid account nonce')
     })
@@ -537,25 +535,43 @@ describe('EntryPoint', function () {
       before(async () => {
         const op = await fillAndSign({
           sender,
-          nonce: key
+          nonce: keyShifted
         }, accountOwner, entryPoint)
         await entryPoint.handleOps([op], beneficiaryAddress)
       })
 
+      it('should get next nonce value by getNonce', async () => {
+        expect(await entryPoint.getNonce(sender, key)).to.eql(keyShifted.add(1))
+      })
+
       it('should allow to increment nonce of different key', async () => {
         const op = await fillAndSign({
           sender,
-          nonce: key + 1
+          nonce: await entryPoint.getNonce(sender, key)
         }, accountOwner, entryPoint)
         await entryPoint.callStatic.handleOps([op], beneficiaryAddress)
       })
 
+      it('should allow manual nonce increment', async () => {
+        // must be called from account itself
+        const incNonceKey = 5
+        const incrementCallData = entryPoint.interface.encodeFunctionData('incrementNonce', [incNonceKey])
+        const callData = account.interface.encodeFunctionData('execute', [entryPoint.address, 0, incrementCallData])
+        const op = await fillAndSign({
+          sender,
+          callData,
+          nonce: await entryPoint.getNonce(sender, key)
+        }, accountOwner, entryPoint)
+        await entryPoint.handleOps([op], beneficiaryAddress)
+
+        expect(await entryPoint.getNonce(sender, incNonceKey)).to.equal(BigNumber.from(incNonceKey).shl(64).add(1))
+      })
       it('should fail with nonsequential seq', async () => {
         const op = await fillAndSign({
           sender,
-          nonce: key + 3
+          nonce: keyShifted.add(3)
         }, accountOwner, entryPoint)
-        await expect(entryPoint.callStatic.handleOps([op], beneficiaryAddress)).to.revertedWith('nonce1')
+        await expect(entryPoint.callStatic.handleOps([op], beneficiaryAddress)).to.revertedWith('AA25 invalid account nonce')
       })
     })
   })

From 5fed9631b6429955c586fdc8a903c8d0227f4fb0 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Sun, 26 Mar 2023 15:52:26 +0300
Subject: [PATCH 12/17] PR comment

---
 contracts/core/EntryPoint.sol |  2 +-
 reports/gas-checker.txt       | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol
index d9150c34..1ef39236 100644
--- a/contracts/core/EntryPoint.sol
+++ b/contracts/core/EntryPoint.sol
@@ -346,7 +346,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager {
     /**
      * 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 (and the nonce) in SenderAddressResult error
+     * this method always revert, and returns the address in SenderAddressResult error
      * @param initCode the constructor code to be passed into the UserOperation.
      */
     function getSenderAddress(bytes calldata initCode) public {
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index 07b0558b..2e913746 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -8,19 +8,19 @@
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │     account.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         78807 │                │                     ║
+║ simple                         │     1 │         78819 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          44262 │               12205 ║
+║ simple - diff from previous    │     2 │               │          44250 │               12193 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        477409 │                │                     ║
+║ simple                         │    10 │        477361 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          44301 │               12244 ║
+║ simple - diff from previous    │    11 │               │          44337 │               12280 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ simple paymaster               │     1 │         85104 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          43257 │               11200 ║
+║ simple paymaster with diff     │     2 │               │          43221 │               11164 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        474734 │                │                     ║
+║ simple paymaster               │    10 │        474662 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ simple paymaster with diff     │    11 │               │          43309 │               11252 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
@@ -30,6 +30,6 @@
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ big tx 5k                      │    10 │       1496495 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         147031 │               18723 ║
+║ big tx - diff from previous    │    11 │               │         147019 │               18711 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 

From b64669738d05f313840719b7a72efb4592373436 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Sun, 26 Mar 2023 15:55:35 +0300
Subject: [PATCH 13/17] update nonce comment

---
 contracts/core/BaseAccount.sol       |  4 +++-
 contracts/interfaces/IEntryPoint.sol |  2 +-
 reports/gas-checker.txt              | 16 ++++++++--------
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/contracts/core/BaseAccount.sol b/contracts/core/BaseAccount.sol
index 2f573060..c0eb8e79 100644
--- a/contracts/core/BaseAccount.sol
+++ b/contracts/core/BaseAccount.sol
@@ -22,7 +22,9 @@ abstract contract BaseAccount is IAccount {
     uint256 constant internal SIG_VALIDATION_FAILED = 1;
 
     /**
-     * return the account 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) {
         return entryPoint().getNonce(address(this), 0);
diff --git a/contracts/interfaces/IEntryPoint.sol b/contracts/interfaces/IEntryPoint.sol
index 580a1a84..a1436197 100644
--- a/contracts/interfaces/IEntryPoint.sol
+++ b/contracts/interfaces/IEntryPoint.sol
@@ -175,7 +175,7 @@ interface IEntryPoint is IStakeManager, INonceManager {
     /**
      * 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 (and nonce) in SenderAddressResult error
+     * this method always revert, and returns the address in SenderAddressResult error
      * @param initCode the constructor code to be passed into the UserOperation.
      */
     function getSenderAddress(bytes memory initCode) external;
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index 2e913746..fc42bb26 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -10,26 +10,26 @@
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ simple                         │     1 │         78819 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          44250 │               12193 ║
+║ simple - diff from previous    │     2 │               │          44262 │               12205 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        477361 │                │                     ║
+║ simple                         │    10 │        477409 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          44337 │               12280 ║
+║ simple - diff from previous    │    11 │               │          44301 │               12244 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ simple paymaster               │     1 │         85104 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          43221 │               11164 ║
+║ simple paymaster with diff     │     2 │               │          43257 │               11200 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        474662 │                │                     ║
+║ simple paymaster               │    10 │        474674 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          43309 │               11252 ║
+║ simple paymaster with diff     │    11 │               │          43357 │               11300 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ big tx 5k                      │     1 │        180552 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ big tx - diff from previous    │     2 │               │         145585 │               17277 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1496495 │                │                     ║
+║ big tx 5k                      │    10 │       1496519 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         147019 │               18711 ║
+║ big tx - diff from previous    │    11 │               │         146959 │               18651 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 

From 4618401e7c42fa383bcdb36a193ef2350a44f18c Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Sun, 26 Mar 2023 16:35:29 +0300
Subject: [PATCH 14/17] remove unused "filler"

---
 contracts/samples/SimpleAccount.sol |  4 ----
 reports/gas-checker.txt             | 26 +++++++++++++-------------
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/contracts/samples/SimpleAccount.sol b/contracts/samples/SimpleAccount.sol
index 0bd57ac3..6c1a1982 100644
--- a/contracts/samples/SimpleAccount.sol
+++ b/contracts/samples/SimpleAccount.sol
@@ -21,10 +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;
-
     address public owner;
 
     IEntryPoint private immutable _entryPoint;
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index fc42bb26..d54d1d0f 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -1,35 +1,35 @@
 == gas estimate of direct calling the account's "execFromEntryPoint" method
    the destination is "account.nonce()", which is known to be "hot" address used by this account
    it little higher than EOA call: its an exec from entrypoint (or account owner) into account contract, verifying msg.sender and exec to target)
-- gas estimate "simple" - 32057
+- gas estimate "simple" - 32033
 - gas estimate "big tx 5k" - 128308
 ╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗
 ║ handleOps description          │ count │ total gasUsed │ per UserOp gas │ per UserOp overhead ║
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │     account.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         78819 │                │                     ║
+║ simple                         │     1 │         78830 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          44262 │               12205 ║
+║ simple - diff from previous    │     2 │               │          44273 │               12240 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        477409 │                │                     ║
+║ simple                         │    10 │        477483 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          44301 │               12244 ║
+║ simple - diff from previous    │    11 │               │          44324 │               12291 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         85104 │                │                     ║
+║ simple paymaster               │     1 │         85115 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          43257 │               11200 ║
+║ simple paymaster with diff     │     2 │               │          43268 │               11235 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        474674 │                │                     ║
+║ simple paymaster               │    10 │        474796 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          43357 │               11300 ║
+║ simple paymaster with diff     │    11 │               │          43320 │               11287 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        180552 │                │                     ║
+║ big tx 5k                      │     1 │        180551 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         145585 │               17277 ║
+║ big tx - diff from previous    │     2 │               │         145608 │               17300 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1496519 │                │                     ║
+║ big tx 5k                      │    10 │       1496629 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         146959 │               18651 ║
+║ big tx - diff from previous    │    11 │               │         147018 │               18710 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 

From 69e19f8e9dc7d214bf5a2bb2bfb9e9081dfa2ae9 Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Sun, 26 Mar 2023 17:06:32 +0300
Subject: [PATCH 15/17] fix gas calc

nonce() is no longer "do-nothing" method. use instead account()
---
 gascalc/GasChecker.ts   |  4 ++--
 reports/gas-checker.txt | 30 +++++++++++++++---------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/gascalc/GasChecker.ts b/gascalc/GasChecker.ts
index c25c71ba..dce39c1d 100644
--- a/gascalc/GasChecker.ts
+++ b/gascalc/GasChecker.ts
@@ -53,7 +53,7 @@ interface GasTestInfo {
 export const DefaultGasTestInfo: Partial<GasTestInfo> = {
   dest: 'self', // destination is the account itself.
   destValue: parseEther('0'),
-  destCallData: '0xaffed0e0', // nonce()
+  destCallData: '0x8da5cb5b', // owner()
   gasPrice: 10e9
 }
 
@@ -363,7 +363,7 @@ export class GasCheckCollector {
     }
 
     write('== gas estimate of direct calling the account\'s "execFromEntryPoint" method')
-    write('   the destination is "account.nonce()", which is known to be "hot" address used by this account')
+    write('   the destination is "account.account()", which is known to be "hot" address used by this account')
     write('   it little higher than EOA call: its an exec from entrypoint (or account owner) into account contract, verifying msg.sender and exec to target)')
     Object.values(gasEstimatePerExec).forEach(({ title, accountEst }) => {
       write(`- gas estimate "${title}" - ${accountEst}`)
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index d54d1d0f..642d44e1 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -1,35 +1,35 @@
 == gas estimate of direct calling the account's "execFromEntryPoint" method
-   the destination is "account.nonce()", which is known to be "hot" address used by this account
+   the destination is "account.account()", which is known to be "hot" address used by this account
    it little higher than EOA call: its an exec from entrypoint (or account owner) into account contract, verifying msg.sender and exec to target)
-- gas estimate "simple" - 32033
-- gas estimate "big tx 5k" - 128308
+- gas estimate "simple" - 31074
+- gas estimate "big tx 5k" - 127349
 ╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗
 ║ handleOps description          │ count │ total gasUsed │ per UserOp gas │ per UserOp overhead ║
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │     account.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         78830 │                │                     ║
+║ simple                         │     1 │         77886 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          44273 │               12240 ║
+║ simple - diff from previous    │     2 │               │          43329 │               12255 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        477483 │                │                     ║
+║ simple                         │    10 │        468019 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          44324 │               12291 ║
+║ simple - diff from previous    │    11 │               │          43428 │               12354 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         85115 │                │                     ║
+║ simple paymaster               │     1 │         84159 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          43268 │               11235 ║
+║ simple paymaster with diff     │     2 │               │          42336 │               11262 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        474796 │                │                     ║
+║ simple paymaster               │    10 │        465356 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          43320 │               11287 ║
+║ simple paymaster with diff     │    11 │               │          42412 │               11338 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        180551 │                │                     ║
+║ big tx 5k                      │     1 │        179619 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         145608 │               17300 ║
+║ big tx - diff from previous    │     2 │               │         144664 │               17315 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1496629 │                │                     ║
+║ big tx 5k                      │    10 │       1487213 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         147018 │               18710 ║
+║ big tx - diff from previous    │    11 │               │         146062 │               18713 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 

From 8bc2f63ac147b1eb8016c7b6857f5433fcf0cfca Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Sun, 26 Mar 2023 17:25:36 +0300
Subject: [PATCH 16/17] rename "nonce()" to "getNonce()"

---
 contracts/core/BaseAccount.sol               |  2 +-
 contracts/samples/gnosis/EIP4337Fallback.sol |  2 --
 contracts/samples/gnosis/EIP4337Manager.sol  |  2 --
 reports/gas-checker.txt                      | 28 ++++++++++----------
 test/UserOp.ts                               |  8 +++---
 5 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/contracts/core/BaseAccount.sol b/contracts/core/BaseAccount.sol
index c0eb8e79..49f39a9e 100644
--- a/contracts/core/BaseAccount.sol
+++ b/contracts/core/BaseAccount.sol
@@ -26,7 +26,7 @@ abstract contract BaseAccount is IAccount {
      * 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);
     }
 
diff --git a/contracts/samples/gnosis/EIP4337Fallback.sol b/contracts/samples/gnosis/EIP4337Fallback.sol
index 0e4a5c5d..4cc0978d 100644
--- a/contracts/samples/gnosis/EIP4337Fallback.sol
+++ b/contracts/samples/gnosis/EIP4337Fallback.sol
@@ -52,8 +52,6 @@ contract EIP4337Fallback is DefaultCallbackHandler, IAccount, IERC1271 {
 
     /**
      * 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();
diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol
index eb36b611..2a687ada 100644
--- a/contracts/samples/gnosis/EIP4337Manager.sol
+++ b/contracts/samples/gnosis/EIP4337Manager.sol
@@ -105,8 +105,6 @@ 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);
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index 642d44e1..05f14902 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -1,35 +1,35 @@
 == gas estimate of direct calling the account's "execFromEntryPoint" method
    the destination is "account.account()", which is known to be "hot" address used by this account
    it little higher than EOA call: its an exec from entrypoint (or account owner) into account contract, verifying msg.sender and exec to target)
-- gas estimate "simple" - 31074
-- gas estimate "big tx 5k" - 127349
+- gas estimate "simple" - 31143
+- gas estimate "big tx 5k" - 127394
 ╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗
 ║ handleOps description          │ count │ total gasUsed │ per UserOp gas │ per UserOp overhead ║
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │     account.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         77886 │                │                     ║
+║ simple                         │     1 │         77930 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          43329 │               12255 ║
+║ simple - diff from previous    │     2 │               │          43373 │               12230 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        468019 │                │                     ║
+║ simple                         │    10 │        468531 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          43428 │               12354 ║
+║ simple - diff from previous    │    11 │               │          43412 │               12269 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         84159 │                │                     ║
+║ simple paymaster               │     1 │         84191 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          42336 │               11262 ║
+║ simple paymaster with diff     │     2 │               │          42368 │               11225 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        465356 │                │                     ║
+║ simple paymaster               │    10 │        465772 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          42412 │               11338 ║
+║ simple paymaster with diff     │    11 │               │          42456 │               11313 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        179619 │                │                     ║
+║ big tx 5k                      │     1 │        179663 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         144664 │               17315 ║
+║ big tx - diff from previous    │     2 │               │         144708 │               17314 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1487213 │                │                     ║
+║ big tx 5k                      │    10 │       1487629 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         146062 │               18713 ║
+║ big tx - diff from previous    │    11 │               │         146082 │               18688 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 
diff --git a/test/UserOp.ts b/test/UserOp.ts
index 33156ede..6e5d2600 100644
--- a/test/UserOp.ts
+++ b/test/UserOp.ts
@@ -160,13 +160,13 @@ export function fillUserOpDefaults (op: Partial<UserOperation>, defaults = Defau
 //  - calculate sender by eth_call the deployment code
 //  - default verificationGasLimit estimateGas of deployment code plus default 100000
 // no initCode:
-//  - update nonce from account.nonce()
+//  - update nonce from account.getNonce()
 // entryPoint param is only required to fill in "sender address when specifying "initCode"
-// nonce: assume contract as "nonce()" function, and fill in.
+// nonce: assume contract as "getNonce()" function, and fill in.
 // sender - only in case of construction: fill sender from initCode.
 // callGasLimit: VERY crude estimation (by estimating call to account, and add rough entryPoint overhead
 // verificationGasLimit: hard-code default at 100k. should add "create2" cost
-export async function fillUserOp (op: Partial<UserOperation>, entryPoint?: EntryPoint, getNonceFunction = 'nonce'): Promise<UserOperation> {
+export async function fillUserOp (op: Partial<UserOperation>, entryPoint?: EntryPoint, getNonceFunction = 'getNonce'): Promise<UserOperation> {
   const op1 = { ...op }
   const provider = entryPoint?.provider
   if (op.initCode != null) {
@@ -232,7 +232,7 @@ export async function fillUserOp (op: Partial<UserOperation>, entryPoint?: Entry
   return op2
 }
 
-export async function fillAndSign (op: Partial<UserOperation>, signer: Wallet | Signer, entryPoint?: EntryPoint, getNonceFunction = 'nonce'): Promise<UserOperation> {
+export async function fillAndSign (op: Partial<UserOperation>, signer: Wallet | Signer, entryPoint?: EntryPoint, getNonceFunction = 'getNonce'): Promise<UserOperation> {
   const provider = entryPoint?.provider
   const op2 = await fillUserOp(op, entryPoint, getNonceFunction)
 

From e0ee20ba91ab1711ee59c928bd2643e08c59fedb Mon Sep 17 00:00:00 2001
From: Dror Tirosh <dror@opengsn.org>
Date: Sun, 9 Apr 2023 11:00:20 +0300
Subject: [PATCH 17/17] rename nonces hash

---
 contracts/core/BaseAccount.sol         | 23 +++++++++++++++++++++--
 contracts/core/NonceManager.sol        | 11 +++++++----
 contracts/interfaces/INonceManager.sol |  7 +++++--
 reports/gas-checker.txt                | 24 ++++++++++++------------
 4 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/contracts/core/BaseAccount.sol b/contracts/core/BaseAccount.sol
index 49f39a9e..95b26399 100644
--- a/contracts/core/BaseAccount.sol
+++ b/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";
@@ -44,6 +43,7 @@ abstract contract BaseAccount is IAccount {
     external override virtual returns (uint256 validationData) {
         _requireFromEntryPoint();
         validationData = _validateSignature(userOp, userOpHash);
+        _validateNonce(userOp.nonce);
         _payPrefund(missingAccountFunds);
     }
 
@@ -70,6 +70,25 @@ abstract contract BaseAccount is IAccount {
     function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash)
     internal virtual returns (uint256 validationData);
 
+    /**
+     * 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 _validateNonce(uint256 nonce) internal view virtual {
+    }
+
     /**
      * sends to the entrypoint (msg.sender) the missing funds for this transaction.
      * subclass MAY override this method for better funds management
diff --git a/contracts/core/NonceManager.sol b/contracts/core/NonceManager.sol
index e0095cf8..f0bc20db 100644
--- a/contracts/core/NonceManager.sol
+++ b/contracts/core/NonceManager.sol
@@ -8,11 +8,14 @@ import "../interfaces/IEntryPoint.sol";
  */
 contract NonceManager is INonceManager {
 
-    mapping(address => mapping(uint192 => uint256)) public nonces;
+    /**
+     * 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 nonces[sender][key] | (uint256(key) << 64);
+        return nonceSequenceNumber[sender][key] | (uint256(key) << 64);
     }
 
     // allow an account to manually increment its own nonce.
@@ -20,7 +23,7 @@ contract NonceManager is INonceManager {
     // 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]++;
+        nonceSequenceNumber[msg.sender][key]++;
     }
 
     /**
@@ -31,7 +34,7 @@ contract NonceManager is INonceManager {
 
         uint192 key = uint192(nonce >> 64);
         uint64 seq = uint64(nonce);
-        return nonces[sender][key]++ == seq;
+        return nonceSequenceNumber[sender][key]++ == seq;
     }
 
 }
diff --git a/contracts/interfaces/INonceManager.sol b/contracts/interfaces/INonceManager.sol
index fd077df0..fe649130 100644
--- a/contracts/interfaces/INonceManager.sol
+++ b/contracts/interfaces/INonceManager.sol
@@ -4,7 +4,10 @@ pragma solidity ^0.8.12;
 interface INonceManager {
 
     /**
-     * return the next nonce for this sender
+     * 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.
@@ -13,7 +16,7 @@ interface INonceManager {
     external view returns (uint256 nonce);
 
     /**
-     * manually increment the nonce of the sender.
+     * 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.
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
index 6fbd821b..c72d9427 100644
--- a/reports/gas-checker.txt
+++ b/reports/gas-checker.txt
@@ -12,28 +12,28 @@
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │     account.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         77829 │                │                     ║
+║ simple                         │     1 │         77873 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          43260 │               14246 ║
+║ simple - diff from previous    │     2 │               │          43294 │               14280 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        467509 │                │                     ║
+║ simple                         │    10 │        467715 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          43287 │               14273 ║
+║ simple - diff from previous    │    11 │               │          43357 │               14343 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         84078 │                │                     ║
+║ simple paymaster               │     1 │         84158 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          42267 │               13253 ║
+║ simple paymaster with diff     │     2 │               │          42289 │               13275 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        464810 │                │                     ║
+║ simple paymaster               │    10 │        465040 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          42295 │               13281 ║
+║ simple paymaster with diff     │    11 │               │          42365 │               13351 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        179562 │                │                     ║
+║ big tx 5k                      │     1 │        179606 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         144607 │               19347 ║
+║ big tx - diff from previous    │     2 │               │         144629 │               19369 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1486595 │                │                     ║
+║ big tx 5k                      │    10 │       1486873 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         146005 │               20745 ║
+║ big tx - diff from previous    │    11 │               │         146075 │               20815 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝