AA-40: Gaschecks with geth (eth-infinitism#102)

drortirosh · web-flow · commit d1b316012eb3 · 2022-07-28T10:04:27.000+03:00
* A-32-enforce-checkin-gas-reports
* use geth for local gas checks
* changed "big" to 10 (avoid getting close to 128k tx limit)
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -15,6 +15,7 @@ jobs:
 
   test:
     runs-on: ubuntu-latest
+
     steps:
       - uses: actions/setup-node@v1
         with:
@@ -29,6 +30,25 @@ jobs:
 
       - run: yarn run ci
 
+  gas-checks:
+    runs-on: ubuntu-latest
+    services:
+      localgeth:
+        image: dtr22/geth-dev
+
+    steps:
+      - uses: actions/setup-node@v1
+        with:
+          node-version: '14'
+      - uses: actions/checkout@v1
+      - uses: actions/cache@v2
+        with:
+          path: node_modules
+          key: ${{ runner.os }}-${{ hashFiles('yarn.lock') }}
+      - run: yarn install
+      - run: yarn compile
+      - run: yarn ci-gas-calc
+
   lint:
     runs-on: ubuntu-latest
     steps:
diff --git a/contracts/test/TestPaymasterAcceptAll.sol b/contracts/test/TestPaymasterAcceptAll.sol
@@ -8,8 +8,13 @@ import "../BasePaymaster.sol";
  */
 contract TestPaymasterAcceptAll is BasePaymaster {
 
-    // solhint-disable-next-line no-empty-blocks
-    constructor(EntryPoint _entryPoint) BasePaymaster(_entryPoint) {}
+    constructor(EntryPoint _entryPoint) BasePaymaster(_entryPoint) {
+        // to support "deterministic address" deployer
+        // solhint-disable avoid-tx-origin
+        if (tx.origin != msg.sender) {
+            _transferOwnership(tx.origin);
+        }
+    }
 
     function validatePaymasterUserOp(UserOperation calldata userOp, bytes32 requestId, uint maxCost) external virtual override view returns (bytes memory context) {
         (userOp, requestId, maxCost);
diff --git a/gascalc/1-simple-wallet.gas.ts b/gascalc/1-simple-wallet.gas.ts
@@ -9,9 +9,9 @@ context('simple wallet', function () {
     await g.addTestRow({ title: 'simple - diff from previous', count: 2, diffLastGas: true })
   })
 
-  it('simple 20', async function () {
+  it('simple 10', async function () {
     if (g.skipLong()) this.skip()
-    await g.addTestRow({ title: 'simple', count: 20, diffLastGas: false })
-    await g.addTestRow({ title: 'simple - diff from previous', count: 21, diffLastGas: true })
+    await g.addTestRow({ title: 'simple', count: 10, diffLastGas: false })
+    await g.addTestRow({ title: 'simple - diff from previous', count: 11, diffLastGas: true })
   })
 })
diff --git a/gascalc/2-paymaster.gas.ts b/gascalc/2-paymaster.gas.ts
@@ -2,6 +2,8 @@ import { parseEther } from 'ethers/lib/utils'
 import { TestPaymasterAcceptAll__factory } from '../typechain'
 import { ethers } from 'hardhat'
 import { GasChecker } from './GasChecker'
+import { Create2Factory } from '../src/Create2Factory'
+import { hexValue } from '@ethersproject/bytes'
 
 const ethersSigner = ethers.provider.getSigner()
 
@@ -11,8 +13,9 @@ context('Minimal Paymaster', function () {
 
   let paymasterAddress: string
   before(async () => {
-    const paymaster = await new TestPaymasterAcceptAll__factory(ethersSigner).deploy(g.entryPoint().address)
-    paymasterAddress = paymaster.address
+    const paymasterInit = hexValue(new TestPaymasterAcceptAll__factory(ethersSigner).getDeployTransaction(g.entryPoint().address).data!)
+    const paymasterAddress = await new Create2Factory(ethers.provider, ethersSigner).deploy(paymasterInit, 0)
+    const paymaster = TestPaymasterAcceptAll__factory.connect(paymasterAddress, ethersSigner)
     await paymaster.addStake(0, { value: 1 })
     await g.entryPoint().depositTo(paymaster.address, { value: parseEther('10') })
   })
@@ -26,13 +29,13 @@ context('Minimal Paymaster', function () {
     })
   })
 
-  it('simple paymaster 20', async function () {
+  it('simple paymaster 10', async function () {
     if (g.skipLong()) this.skip()
 
-    await g.addTestRow({ title: 'simple paymaster', count: 20, paymaster: paymasterAddress, diffLastGas: false })
+    await g.addTestRow({ title: 'simple paymaster', count: 10, paymaster: paymasterAddress, diffLastGas: false })
     await g.addTestRow({
       title: 'simple paymaster with diff',
-      count: 21,
+      count: 11,
       paymaster: paymasterAddress,
       diffLastGas: true
     })
diff --git a/gascalc/3-huge-tx-gas.ts b/gascalc/3-huge-tx-gas.ts
@@ -1,17 +1,17 @@
 import { DefaultGasTestInfo, GasChecker } from './GasChecker'
 
-context('huge tx', function () {
+context('huge tx - 5k', function () {
   this.timeout(60000)
-  const huge = DefaultGasTestInfo.destCallData!.padEnd(20480, 'f')
+  const huge = DefaultGasTestInfo.destCallData!.padEnd(10240, 'f')
   const g = new GasChecker()
 
-  it('big tx', async () => {
-    await g.addTestRow({ title: 'big tx 10k', count: 1, destCallData: huge, diffLastGas: false })
+  it('big tx 5k', async () => {
+    await g.addTestRow({ title: 'big tx 5k', count: 1, destCallData: huge, diffLastGas: false })
     await g.addTestRow({ title: 'big tx - diff from previous', count: 2, destCallData: huge, diffLastGas: true })
   })
-  it('big tx 20', async function () {
+  it('big tx 10', async function () {
     if (g.skipLong()) this.skip()
-    await g.addTestRow({ title: 'big tx', count: 20, destCallData: huge, diffLastGas: false })
-    await g.addTestRow({ title: 'big tx - diff from previous', count: 21, destCallData: huge, diffLastGas: true })
+    await g.addTestRow({ title: 'big tx 5k', count: 10, destCallData: huge, diffLastGas: false })
+    await g.addTestRow({ title: 'big tx - diff from previous', count: 11, destCallData: huge, diffLastGas: true })
   })
 })
diff --git a/gascalc/GasChecker.ts b/gascalc/GasChecker.ts
@@ -199,6 +199,8 @@ export class GasChecker {
         return op
       }))
 
+    const txdata = GasCheckCollector.inst.entryPoint.interface.encodeFunctionData('handleOps', [userOps, info.beneficiary])
+    console.log('=== encoded data=', txdata.length)
     const gasEst = await GasCheckCollector.inst.entryPoint.estimateGas.handleOps(
       userOps, info.beneficiary, {}
     )
diff --git a/hardhat.config.ts b/hardhat.config.ts
@@ -36,6 +36,8 @@ const config: HardhatUserConfig = {
   },
   networks: {
     dev: { url: 'http://localhost:8545' },
+    // github action starts localgeth service, for gas calculations
+    localgeth: { url: 'http://localgeth:8545' },
     goerli: getNetwork('goerli'),
     proxy: getNetwork1('http://localhost:8545'),
     kovan: getNetwork('kovan')
diff --git a/package.json b/package.json
@@ -10,12 +10,16 @@
     "lint:js": "eslint -f unix .",
     "lint-fix": "eslint -f unix . --fix",
     "lint:sol": "solhint -f unix \"contracts/**/*.sol\" --max-warnings 0",
-    "gas-calc": "npx ts-mocha gascalc/*",
+    "gas-calc": "./scripts/gascalc",
+    "mocha-gascalc": "TS_NODE_TRANSPILE_ONLY=1 npx ts-mocha gascalc/*",
     "test": "./scripts/hh-wrapper test",
     "coverage": "COVERAGE=1 hardhat coverage",
     "deploy": "./scripts/hh-wrapper deploy",
     "test-dev": "hardhat test --network dev",
-    "ci": "yarn compile && hardhat test && yarn gas-calc && yarn run runop",
+    "ci": "yarn compile && hardhat test && yarn run runop",
+    "ci-gas-calc": "yarn gas-calc && yarn check-gas-reports",
+    "check-gas-reports": "./scripts/check-gas-reports",
+
     "runop": "hardhat run src/runop.ts ",
     "runop-goerli": "AA_URL=https://account-abstraction-goerli.nethermind.io yarn runop --network goerli",
     "runop3": "hardhat run src/runop3.ts "
diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt
@@ -2,34 +2,34 @@
    the destination is "wallet.nonce()", which is known to be "hot" address used by this wallet
    it little higher than EOA call: its an exec from entrypoint (or wallet owner) into wallet contract, verifying msg.sender and exec to target)
 - gas estimate "simple" - 27751
-- gas estimate "big tx 10k" - 218388
+- gas estimate "big tx 5k" - 122740
 ╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗
 ║ handleOps description          │ count │ total gasUsed │ per UserOp gas │ per UserOp overhead ║
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │      wallet.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         74114 │                │                     ║
+║ simple                         │     1 │         74094 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          42075 │               14324 ║
+║ simple - diff from previous    │     2 │               │          42135 │               14384 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    20 │        874669 │                │                     ║
+║ simple                         │    10 │        453128 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    21 │               │          42428 │               14677 ║
+║ simple - diff from previous    │    11 │               │          42132 │               14381 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         80231 │                │                     ║
+║ simple paymaster               │     1 │         74094 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          40972 │               13221 ║
+║ simple paymaster with diff     │     2 │               │          42115 │               14364 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    20 │        858952 │                │                     ║
+║ simple paymaster               │    10 │        453124 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    21 │               │          41261 │               13510 ║
+║ simple paymaster with diff     │    11 │               │          42244 │               14493 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 10k                     │     1 │        274578 │                │                     ║
+║ big tx 5k                      │     1 │        173728 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         241826 │               23438 ║
+║ big tx - diff from previous    │     2 │               │         141370 │               18630 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx                         │    20 │       4955417 │                │                     ║
+║ big tx 5k                      │    10 │       1451288 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    21 │               │         251739 │               33351 ║
+║ big tx - diff from previous    │    11 │               │         142832 │               20092 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 
diff --git a/scripts/check-gas-reports b/scripts/check-gas-reports
@@ -0,0 +1,17 @@
+#!/bin/bash
+# make sure gas reports are checked in with this commit
+# dump report diff
+# exit with "1" if there is a diff, zero if no diff
+
+folder=${1:-reports}
+git diff --color=always $folder
+git diff ${folder} | grep -q .
+
+if [ "$?" == 1 ]; then
+  #diff with no error - ok
+  exit
+else
+  echo ERROR: found above unchecked reports. 
+  exit 1
+fi
+
diff --git a/scripts/docker-gascalc b/scripts/docker-gascalc
@@ -0,0 +1,5 @@
+# run "yarn gas-calc" using geth with docker.
+# (if you have geth running on localhost:8545, its faster with "HARDHAT_NETWORK=dev yarn gas-calc")
+docker-compose -f `dirname $0`/docker-gascalc.yml up --abort-on-container-exit
+docker-compose -f `dirname $0`/docker-gascalc.yml down
+
diff --git a/scripts/docker-gascalc.yml b/scripts/docker-gascalc.yml
@@ -0,0 +1,20 @@
+version: '2'
+
+services:
+  test:
+    image: node
+    container_name: gascalc
+    depends_on:
+      - localgeth
+    volumes:
+      - ..:/app
+    working_dir: /app
+    restart: "no"
+    environment:
+      - HARDHAT_NETWORK=localgeth
+    command: "yarn mocha-gascalc"
+
+  #configuration is a copy of github/.workflows/build.xml
+  localgeth:
+    image: dtr22/geth-dev
+    container_name: localgeth
diff --git a/scripts/gascalc b/scripts/gascalc
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+# run gascalc, assuming "geth" is running on localhost, port 8545
+cd `dirname $0`/..
+function getClientVersion() {
+  curl -m 1 -s -d '{"method":"web3_clientVersion","params":[],"id":1234,"jsonrpc":"2.0"}' -H content-type:application/json localhost:8545
+}
+
+if [[ `getClientVersion` =~ "Geth" ]]; then
+  echo Using GETH on localhost:8545
+  HARDHAT_NETWORK=dev yarn mocha-gascalc
+else
+  echo No GETH running on localhost:8545. Using docker..
+  ./scripts/docker-gascalc
+fi

Original file line number	Diff line number	Diff line change
`@@ -199,6 +199,8 @@ export class GasChecker {`
`199`	`199`	`return op`
`200`	`200`	`}))`
`201`	`201`
	`202`	`+ const txdata = GasCheckCollector.inst.entryPoint.interface.encodeFunctionData('handleOps', [userOps, info.beneficiary])`
	`203`	`+ console.log('=== encoded data=', txdata.length)`
`202`	`204`	`const gasEst = await GasCheckCollector.inst.entryPoint.estimateGas.handleOps(`
`203`	`205`	`userOps, info.beneficiary, {}`
`204`	`206`	`)`