AA-110-check-codehash (eth-infinitism#42)

* AA-110-check-codehash

check code of all accessed code:
- must not be zero-sized (unless it is a precompile)
- are not modified before the 2nd validation (that is, fail the userOp before 2nd validation if a contract code was modified)

sendBundleNow returns { transactionHash, userOpHashes }

Author: drortirosh

.idea/aa-bundler.iml

@@ -1 +0,0 @@
-ignores: ["solidity-string-utils"]

packages/bundler/.depcheckrc

@@ -1,31 +1,22 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity ^0.8.15;
 
-import "@account-abstraction/contracts/core/EntryPoint.sol";
-import "solidity-string-utils/StringUtils.sol";
+import "@account-abstraction/contracts/interfaces/IEntryPoint.sol";
 
 contract BundlerHelper {
-    using StringUtils for *;
-
-    /**
-     * run handleop. require to get refund for the used gas.
-     */
-    function handleOps(uint expectedPaymentGas, EntryPoint ep, UserOperation[] calldata ops, address payable beneficiary)
-    public returns (uint paid, uint gasPrice, bytes memory errorReason){
-        gasPrice = tx.gasprice;
-        uint expectedPayment = expectedPaymentGas * gasPrice;
-        uint preBalance = beneficiary.balance;
-        try ep.handleOps(ops, beneficiary) {
-        } catch (bytes memory err) {
-            errorReason = err;
+    function getUserOpHashes(IEntryPoint entryPoint, UserOperation[] memory userOps) external view returns (bytes32[] memory ret) {
+        ret = new bytes32[](userOps.length);
+        for (uint i = 0; i < userOps.length; i++) {
+            ret[i] = entryPoint.getUserOpHash(userOps[i]);
         }
-        paid = beneficiary.balance - preBalance;
-        if (paid < expectedPayment) {
-            revert(string.concat(
-                "didn't pay enough: paid ", paid.toString(),
-                " expected ", expectedPayment.toString(),
-                " gasPrice ", gasPrice.toString()
-            ));
+    }
+
+    function getCodeHashes(address[] memory addresses) public view returns (bytes32) {
+        bytes32[] memory hashes = new bytes32[](addresses.length);
+        for (uint i = 0; i < addresses.length; i++) {
+            hashes[i] = addresses[i].codehash;
         }
+        bytes memory data = abi.encode(hashes);
+        return keccak256(data);
     }
 }

packages/bundler/contracts/BundlerHelper.sol

@@ -56,7 +56,6 @@
     "hardhat": "^2.11.0",
     "hardhat-deploy": "^0.11.11",
     "solidity-coverage": "^0.7.21",
-    "solidity-string-utils": "^0.0.8-0",
     "ts-node": ">=8.0.0",
     "typechain": "^8.1.0"
   }

packages/bundler/package.json

@@ -9,6 +9,9 @@ import { LogCallFrame, LogContext, LogDb, LogFrameResult, LogStep, LogTracer } f
 declare function toHex (a: any): string
 
 declare function toAddress (a: any): string
+
+declare function isPrecompiled (addr: any): boolean
+
 /**
  * return type of our BundlerCollectorTracer.
  * collect access and opcodes, split into "levels" based on NUMBER opcode
@@ -42,6 +45,7 @@ export interface ExitInfo {
   gasUsed: number
   data: string
 }
+
 export interface NumberLevelInfo {
   opcodes: { [opcode: string]: number }
   access: { [address: string]: AccessInfo }
@@ -151,6 +155,16 @@ export function bundlerCollectorTracer (): BundlerCollectorTracer {
         }
       }
 
+      if (opcode.match(/^(EXT.*|CALL|CALLCODE|DELEGATECALL|STATICCALL)$/) != null) {
+        // this.debug.push('op=' + opcode + ' last=' + this.lastOp + ' stacksize=' + log.stack.length())
+        const idx = opcode.startsWith('EXT') ? 0 : 1
+        const addr = toAddress(log.stack.peek(idx).toString(16))
+        const addrHex = toHex(addr)
+        if ((this.currentLevel.contractSize[addrHex] ?? 0) === 0 && !isPrecompiled(addr)) {
+          this.currentLevel.contractSize[addrHex] = db.getCode(addr).length
+        }
+      }
+
       if (log.getDepth() === 1) {
         // NUMBER opcode at top level split levels
         if (opcode === 'NUMBER') this.numberCounter++
@@ -175,15 +189,6 @@ export function bundlerCollectorTracer (): BundlerCollectorTracer {
           this.countSlot(this.currentLevel.opcodes, opcode)
         }
       }
-      if (opcode.match(/^(EXT.*|CALL|CALLCODE|DELEGATECALL|STATICCALL|CREATE2)$/) != null) {
-        // this.debug.push('op=' + opcode + ' last=' + this.lastOp + ' stacksize=' + log.stack.length())
-        const idx = opcode.startsWith('EXT') ? 0 : 1
-        const addr = toAddress(log.stack.peek(idx).toString(16))
-        const addrHex = toHex(addr)
-        if (this.currentLevel.contractSize[addrHex] == null) {
-          this.currentLevel.contractSize[addrHex] = db.getCode(addr).length
-        }
-      }
       this.lastOp = opcode
 
       if (opcode === 'SLOAD' || opcode === 'SSTORE') {

packages/bundler/src/BundlerCollectorTracer.ts

@@ -4,6 +4,7 @@ import ow from 'ow'
 export interface BundlerConfig {
   beneficiary: string
   entryPoint: string
+  bundlerHelper: string
   gasFactor: string
   minBalance: string
   mnemonic: string
@@ -24,6 +25,7 @@ export interface BundlerConfig {
 export const BundlerConfigShape = {
   beneficiary: ow.string,
   entryPoint: ow.string,
+  bundlerHelper: ow.string,
   gasFactor: ow.string,
   minBalance: ow.string,
   mnemonic: ow.string,
@@ -45,5 +47,6 @@ export const BundlerConfigShape = {
 export const bundlerConfigDefault: Partial<BundlerConfig> = {
   port: '3000',
   entryPoint: '0x1306b01bC3e4AD202612D3843387e94737673F53',
+  bundlerHelper: '0x3ac2913fd3ed9a2c6eb7757bcfc6f9cd49cbfea4',
   unsafe: false
 }

packages/bundler/src/BundlerConfig.ts

@@ -171,8 +171,7 @@ export class BundlerServer {
         result = 'ok'
         break
       case 'debug_bundler_sendBundleNow':
-        await this.debugHandler.sendBundleNow()
-        result = 'ok'
+        result = await this.debugHandler.sendBundleNow()
         break
       default:
         throw new RpcError(`Method ${method} is not supported`, -32601)

packages/bundler/src/BundlerServer.ts

@@ -1,6 +1,7 @@
 import { ExecutionManager } from './modules/ExecutionManager'
 import { ReputationDump, ReputationManager } from './modules/ReputationManager'
 import { MempoolManager } from './modules/MempoolManager'
+import { SendBundleReturn } from './modules/BundleManager'
 
 export class DebugMethodHandler {
   constructor (
@@ -29,8 +30,8 @@ export class DebugMethodHandler {
     }
   }
 
-  async sendBundleNow (): Promise<void> {
-    await this.execManager.attemptBundle(true)
+  async sendBundleNow (): Promise<SendBundleReturn | undefined> {
+    return await this.execManager.attemptBundle(true)
   }
 
   clearState (): void {

packages/bundler/src/DebugMethodHandler.ts

@@ -8,16 +8,23 @@ import Debug from 'debug'
 import { ReputationManager, ReputationStatus } from './ReputationManager'
 import { AddressZero } from '@account-abstraction/utils'
 import { Mutex } from 'async-mutex'
+import { BundlerHelper } from '../types'
 
 const debug = Debug('aa.cron')
 
+export interface SendBundleReturn {
+  transactionHash: string
+  userOpHashes: string[]
+}
+
 export class BundleManager {
   provider: JsonRpcProvider
   signer: JsonRpcSigner
   mutex = new Mutex()
 
   constructor (
     readonly entryPoint: EntryPoint,
+    readonly bundlerHelper: BundlerHelper,
     readonly mempoolManager: MempoolManager,
     readonly validationManager: ValidationManager,
     readonly reputationManager: ReputationManager,
@@ -34,30 +41,38 @@ export class BundleManager {
    * collect UserOps from mempool into a bundle
    * send this bundle.
    */
-  async sendNextBundle (): Promise<void> {
-    await this.mutex.runExclusive(async () => {
+  async sendNextBundle (): Promise<SendBundleReturn | undefined> {
+    return await this.mutex.runExclusive(async () => {
       debug('sendNextBundle')
 
       const bundle = await this.createBundle()
       if (bundle.length === 0) {
         debug('sendNextBundle - no bundle to send')
       } else {
         const beneficiary = await this._selectBeneficiary()
-        await this.sendBundle(bundle, beneficiary)
+        const ret = await this.sendBundle(bundle, beneficiary)
         debug(`sendNextBundle exit - after sent a bundle of ${bundle.length} `)
+        return ret
       }
     })
   }
 
   /**
    * submit a bundle.
    * after submitting the bundle, remove all UserOps from the mempool
+   * @return SendBundleReturn the transaction and UserOp hashes on successful transaction, or null on failed transaction
    */
-  async sendBundle (userOps: UserOperation[], beneficiary: string): Promise<void> {
+  async sendBundle (userOps: UserOperation[], beneficiary: string): Promise<SendBundleReturn | undefined> {
     try {
-      await this.entryPoint.handleOps(userOps, beneficiary)
+      const ret = await this.entryPoint.handleOps(userOps, beneficiary)
       debug('sent handleOps with', userOps.length, 'ops. removing from mempool')
       this.mempoolManager.removeAllUserOps(userOps)
+      // hashes are needed for debug rpc only.
+      const hashes = await this.bundlerHelper.getUserOpHashes(this.entryPoint.address, userOps)
+      return {
+        transactionHash: ret.hash,
+        userOpHashes: hashes
+      }
     } catch (e: any) {
       // failed to handleOp. use FailedOp to detect by
       if (e.errorName !== 'FailedOp') {
@@ -119,9 +134,9 @@ export class BundleManager {
       let validationResult: ValidationResult
       try {
         // re-validate UserOp. no need to check stake, since it cannot be reduced between first and 2nd validation
-        validationResult = await this.validationManager.validateUserOp(entry.userOp, false)
+        validationResult = await this.validationManager.validateUserOp(entry.userOp, entry.referencedContracts, false)
       } catch (e: any) {
-        debug('failed 2nd validation', e.message)
+        debug('failed 2nd validation:', e.message)
         // failed validation. don't try anymore
         this.mempoolManager.removeUserOp(entry.userOp)
         continue

packages/bundler/src/modules/BundleManager.ts

@@ -1,7 +1,7 @@
 import { ReputationManager } from './ReputationManager'
 import { clearInterval } from 'timers'
 import { MempoolManager } from './MempoolManager'
-import { BundleManager } from './BundleManager'
+import { BundleManager, SendBundleReturn } from './BundleManager'
 import Debug from 'debug'
 import { UserOperation } from './moduleUtils'
 import { ValidationManager } from './ValidationManager'
@@ -39,10 +39,11 @@ export class ExecutionManager {
     await this.mutex.runExclusive(async () => {
       debug('sendUserOperation')
       this.validationManager.validateInputParameters(userOp, entryPointInput)
-      const validationResult = await this.validationManager.validateUserOp(userOp)
+      const validationResult = await this.validationManager.validateUserOp(userOp, undefined)
       this.mempoolManager.addUserOp(userOp,
         validationResult.returnInfo.prefund,
         validationResult.senderInfo,
+        validationResult.referencedContracts,
         validationResult.aggregatorInfo?.addr)
       await this.attemptBundle(false)
     })
@@ -80,10 +81,10 @@ export class ExecutionManager {
    * attempt to send a bundle now.
    * @param force
    */
-  async attemptBundle (force = true): Promise<void> {
+  async attemptBundle (force = true): Promise<SendBundleReturn | undefined> {
     debug('attemptBundle force=', force, 'count=', this.mempoolManager.count(), 'max=', this.maxMempoolSize)
     if (force || this.mempoolManager.count() >= this.maxMempoolSize) {
-      await this.bundleManager.sendNextBundle()
+      return await this.bundleManager.sendNextBundle()
     }
   }
 }

packages/bundler/src/modules/ExecutionManager.ts

@@ -1,7 +1,7 @@
 import { BigNumber, BigNumberish } from 'ethers'
 import { getAddr, UserOperation } from './moduleUtils'
 import { requireCond } from '../utils'
-import { StakeInfo, ValidationErrors } from './ValidationManager'
+import { ReferencedCodeHashes, StakeInfo, ValidationErrors } from './ValidationManager'
 import { ReputationManager } from './ReputationManager'
 import Debug from 'debug'
 
@@ -10,6 +10,7 @@ const debug = Debug('aa.mempool')
 export interface MempoolEntry {
   userOp: UserOperation
   prefund: BigNumberish
+  referencedContracts: ReferencedCodeHashes
   // aggregator, if one was found during simulation
   aggregator?: string
 }
@@ -35,10 +36,11 @@ export class MempoolManager {
   // add userOp into the mempool, after initial validation.
   // replace existing, if any (and if new gas is higher)
   // revets if unable to add UserOp to mempool (too many UserOps with this sender)
-  addUserOp (userOp: UserOperation, prefund: BigNumberish, senderInfo: StakeInfo, aggregator?: string): void {
+  addUserOp (userOp: UserOperation, prefund: BigNumberish, senderInfo: StakeInfo, referencedContracts: ReferencedCodeHashes, aggregator?: string): void {
     const entry: MempoolEntry = {
       userOp,
       prefund,
+      referencedContracts,
       aggregator
     }
     const index = this._find(userOp)