diff --git a/packages/bundler/test/DebugMethodHandler.test.ts b/packages/bundler/test/DebugMethodHandler.test.ts index dba69c73..0964cf9e 100644 --- a/packages/bundler/test/DebugMethodHandler.test.ts +++ b/packages/bundler/test/DebugMethodHandler.test.ts @@ -52,7 +52,7 @@ describe('#DebugMethodHandler', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) - const validMgr = new ValidationManager(entryPoint, repMgr, config.unsafe) + const validMgr = new ValidationManager(entryPoint, config.unsafe) const eventsManager = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, eventsManager, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/UserOpMethodHandler.test.ts b/packages/bundler/test/UserOpMethodHandler.test.ts index 53695e2a..6af33f55 100644 --- a/packages/bundler/test/UserOpMethodHandler.test.ts +++ b/packages/bundler/test/UserOpMethodHandler.test.ts @@ -18,7 +18,14 @@ import { TestRulesAccount__factory } from '../src/types' import { ValidationManager, supportsDebugTraceCall } from '@account-abstraction/validation-manager' -import { packUserOp, resolveHexlify, UserOperation, waitFor } from '@account-abstraction/utils' +import { + AddressZero, + packUserOp, + resolveHexlify, + simulationRpcParams, + UserOperation, + waitFor +} from '@account-abstraction/utils' import { UserOperationEventEvent } from '@account-abstraction/contracts/dist/types/EntryPoint' import { UserOperationReceipt } from '../src/RpcTypes' import { ExecutionManager } from '../src/modules/ExecutionManager' @@ -131,7 +138,7 @@ describe('UserOpMethodHandler', function () { }) }) - describe.only('sendUserOperation', function () { + describe('sendUserOperation', function () { let userOperation: UserOperation let accountAddress: string diff --git a/packages/utils/src/decodeRevertReason.ts b/packages/utils/src/decodeRevertReason.ts index e69de29b..6cded77b 100644 --- a/packages/utils/src/decodeRevertReason.ts +++ b/packages/utils/src/decodeRevertReason.ts @@ -0,0 +1,69 @@ +import { Interface } from '@ethersproject/abi' +import { + EntryPoint__factory, + EntryPointSimulations__factory, + TokenPaymaster__factory +} from '@account-abstraction/contracts' +import { ethers } from 'ethers' + +const decodeRevertReasonContracts = new Interface([ + ...EntryPointSimulations__factory.createInterface().fragments, + ...TokenPaymaster__factory.createInterface().fragments, + // ...TestERC20__factory.createInterface().fragments, // for OZ errors, + 'error ECDSAInvalidSignature()' +]) // .filter(f => f.type === 'error')) + +/** + * helper to decode revert data into its string representation + * @param data revert data or an exception thrown by eth_call + * @param nullIfNoMatch true to return null if not found. otherwise, return input data as-is + */ +export function decodeRevertReason (data: string | Error, nullIfNoMatch = true): string | null { + if (typeof data !== 'string') { + const err = data as any + data = (err.data?.data ?? err.data ?? err.error.data) as string + } + const methodSig = data.slice(0, 10) + const dataParams = '0x' + data.slice(10) + + try { + // would be nice to add these to above "decodeRevertReasonContracts", but we can't add Error(string) to xface... + if (methodSig === '0x08c379a0') { + const [err] = ethers.utils.defaultAbiCoder.decode(['string'], dataParams) + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + return `Error(${err})` + } else if (methodSig === '0x4e487b71') { + const [code] = ethers.utils.defaultAbiCoder.decode(['uint256'], dataParams) + return `Panic(${panicCodes[code] ?? code} + ')` + } + const err = decodeRevertReasonContracts.parseError(data) + // treat any error "bytes" argument as possible error to decode (e.g. FailedOpWithRevert, PostOpReverted) + const args = err.args.map((arg: any, index) => { + switch (err.errorFragment.inputs[index].type) { + case 'bytes' : return decodeRevertReason(arg, false) + case 'string': return `"${(arg as string)}"` + default: return arg + } + }) + return `${err.name}(${args.join(',')})` + } catch (e) { + // throw new Error('unsupported errorSig ' + data) + if (!nullIfNoMatch) { + return data + } + return null + } +} + +const panicCodes: { [key: number]: string } = { + // from https://docs.soliditylang.org/en/v0.8.0/control-structures.html + 0x01: 'assert(false)', + 0x11: 'arithmetic overflow/underflow', + 0x12: 'divide by zero', + 0x21: 'invalid enum value', + 0x22: 'storage byte array that is incorrectly encoded', + 0x31: '.pop() on an empty array.', + 0x32: 'array sout-of-bounds or negative index', + 0x41: 'memory overflow', + 0x51: 'zero-initialized variable of internal function type' +} diff --git a/packages/utils/src/rpcSimulateValidations.ts b/packages/utils/src/rpcSimulateValidations.ts index c32eb249..54c34b8a 100644 --- a/packages/utils/src/rpcSimulateValidations.ts +++ b/packages/utils/src/rpcSimulateValidations.ts @@ -12,8 +12,8 @@ export const entryPointSimulationsInterface = EntryPointSimulations__factory.cre * @param userOp * @param extraOptions optional added tracer settings */ -export function simulationRpcParams (methodName: string, entryPointAddress: string, userOp: UserOperation, extraOptions: any = {}): any[] { - const data = entryPointSimulationsInterface.encodeFunctionData(methodName as any, [packUserOp(userOp)]) +export function simulationRpcParams (methodName: string, entryPointAddress: string, userOp: UserOperation, extraParams: any[] = [], extraOptions: any = {}): any[] { + const data = entryPointSimulationsInterface.encodeFunctionData(methodName as any, [packUserOp(userOp), ...extraParams] as any) const tx = { to: entryPointAddress, data diff --git a/packages/validation-manager/src/BundlerCollectorTracer.ts b/packages/validation-manager/src/BundlerCollectorTracer.ts index cc4d0fe6..426d65d7 100644 --- a/packages/validation-manager/src/BundlerCollectorTracer.ts +++ b/packages/validation-manager/src/BundlerCollectorTracer.ts @@ -93,6 +93,7 @@ interface RelevantStepData { */ interface BundlerCollectorTracer extends LogTracer, BundlerTracerResult { lastOp: string + topLevelAddress?: string lastThreeOpcodes: RelevantStepData[] stopCollectingTopic: string stopCollecting: boolean @@ -211,6 +212,9 @@ export function bundlerCollectorTracer (): BundlerCollectorTracer { } if (log.getDepth() === 1) { + if (this.topLevelAddress == null) { + this.topLevelAddress = toHex(log.contract.getAddress()) + } if (opcode === 'CALL' || opcode === 'STATICCALL') { // stack.peek(0) - gas const addr = toAddress(log.stack.peek(1).toString(16)) @@ -264,29 +268,32 @@ export function bundlerCollectorTracer (): BundlerCollectorTracer { // this.debug.push(`isPrecompiled address=${addrHex} addressInt=${addressInt}`) return addressInt > 0 && addressInt < 10 } - // [OP-041] - if (opcode.match(/^(EXT.*|CALL|CALLCODE|DELEGATECALL|STATICCALL)$/) != null) { - const idx = opcode.startsWith('EXT') ? 0 : 1 - const addr = toAddress(log.stack.peek(idx).toString(16)) - const addrHex = toHex(addr) - // this.debug.push('op=' + opcode + ' last=' + this.lastOp + ' stacksize=' + log.stack.length() + ' addr=' + addrHex) - if (this.currentLevel.contractSize[addrHex] == null && !isAllowedPrecompiled(addr)) { - this.currentLevel.contractSize[addrHex] = { - contractSize: db.getCode(addr).length, - opcode + // ignore opcodes at top-level contract (EntryPointSimulations) + if (this.topLevelAddress !== toHex(log.contract.getAddress())) { + // [OP-041] + if (opcode.match(/^(EXT.*|CALL|CALLCODE|DELEGATECALL|STATICCALL)$/) != null) { + const idx = opcode.startsWith('EXT') ? 0 : 1 + const addr = toAddress(log.stack.peek(idx).toString(16)) + const addrHex = toHex(addr) + // this.debug.push('op=' + opcode + ' last=' + this.lastOp + ' stacksize=' + log.stack.length() + ' addr=' + addrHex) + if (this.currentLevel.contractSize[addrHex] == null && !isAllowedPrecompiled(addr)) { + this.currentLevel.contractSize[addrHex] = { + contractSize: db.getCode(addr).length, + opcode + } } } - } - // [OP-012] - if (this.lastOp === 'GAS' && !opcode.includes('CALL')) { - // count "GAS" opcode only if not followed by "CALL" - this.countSlot(this.currentLevel.opcodes, 'GAS') - } - if (opcode !== 'GAS') { - // ignore "unimportant" opcodes: - if (opcode.match(/^(DUP\d+|PUSH\d+|SWAP\d+|POP|ADD|SUB|MUL|DIV|EQ|LTE?|S?GTE?|SLT|SH[LR]|AND|OR|NOT|ISZERO)$/) == null) { - this.countSlot(this.currentLevel.opcodes, opcode) + // [OP-012] + if (this.lastOp === 'GAS' && !opcode.includes('CALL')) { + // count "GAS" opcode only if not followed by "CALL" + this.countSlot(this.currentLevel.opcodes, 'GAS') + } + if (opcode !== 'GAS') { + // ignore "unimportant" opcodes: + if (opcode.match(/^(DUP\d+|PUSH\d+|SWAP\d+|POP|ADD|SUB|MUL|DIV|EQ|LTE?|S?GTE?|SLT|SH[LR]|AND|OR|NOT|ISZERO)$/) == null) { + this.countSlot(this.currentLevel.opcodes, opcode) + } } } this.lastOp = opcode diff --git a/packages/validation-manager/src/ValidationManager.ts b/packages/validation-manager/src/ValidationManager.ts index ce71c1c2..b8b6839f 100644 --- a/packages/validation-manager/src/ValidationManager.ts +++ b/packages/validation-manager/src/ValidationManager.ts @@ -1,4 +1,4 @@ -import { BigNumber, BigNumberish, BytesLike, ethers } from 'ethers' +import { BigNumber, BigNumberish, ethers } from 'ethers' import { JsonRpcProvider } from '@ethersproject/providers' import Debug from 'debug' @@ -24,12 +24,25 @@ import { BundlerTracerResult, bundlerCollectorTracer, ExitInfo } from './Bundler import { debug_traceCall } from './GethTracer' import EntryPointSimulationsJson from '@account-abstraction/contracts/artifacts/EntryPointSimulations.json' +import { decodeRevertReason } from '@account-abstraction/utils/dist/src/decodeRevertReason' const debug = Debug('aa.mgr.validate') // how much time into the future a UserOperation must be valid in order to be accepted const VALID_UNTIL_FUTURE_SECONDS = 30 +// extract address from "data" (first 20 bytes) +// add it as "addr" member to the "stakeinfo" struct +// if no address, then return "undefined" instead of struct. +function fillEntity (addr: string | undefined, info: StakeInfo): StakeInfo | undefined { + return addr == null || addr === AddressZero || addr === '' + ? undefined + : { + ...info, + addr + } +} + /** * result from successful simulateValidation */ @@ -89,11 +102,9 @@ export class ValidationManager { aggregatorInfo: res.aggregatorInfo.aggregator === AddressZero ? undefined : { ...res.aggregatorInfo.stakeInfo, addr: res.aggregatorInfo?.aggregator } } } catch (error: any) { - const revertData = error?.data - // todo: fix error - if (revertData != null) { - // note: this line throws the revert reason instead of returning it - entryPointSimulations.decodeFunctionResult('simulateValidation', revertData) + const decodedError = decodeRevertReason(error) + if (decodedError != null) { + throw new Error(decodedError) } throw error } @@ -130,27 +141,14 @@ export class ValidationManager { aggregatorInfo // may be missing (exists only SimulationResultWithAggregator } = errorResult.errorArgs - // extract address from "data" (first 20 bytes) - // add it as "addr" member to the "stakeinfo" struct - // if no address, then return "undefined" instead of struct. - function fillEntity (data: BytesLike, info: StakeInfo): StakeInfo | undefined { - const addr = getAddr(data) - return addr == null - ? undefined - : { - ...info, - addr - } - } - return { returnInfo, senderInfo: { ...senderInfo, addr: userOp.sender }, - factoryInfo: fillEntity(userOp.factory ?? '', factoryInfo), - paymasterInfo: fillEntity(userOp.paymaster ?? '', paymasterInfo), + factoryInfo: fillEntity(userOp.factory, factoryInfo), + paymasterInfo: fillEntity(userOp.paymaster, paymasterInfo), aggregatorInfo: fillEntity(aggregatorInfo?.actualAggregator, aggregatorInfo?.stakeInfo) } } @@ -176,28 +174,39 @@ export class ValidationManager { }) const lastResult = tracerResult.calls.slice(-1)[0] - if (lastResult.type !== 'REVERT') { - throw new Error('Invalid response. simulateCall must revert') - } const data = (lastResult as ExitInfo).data + if (lastResult.type === 'REVERT') { + throw new Error(decodeRevertReason(data, false) as string) + } // Hack to handle SELFDESTRUCT until we fix entrypoint if (data === '0x') { return [data as any, tracerResult] } try { - const { - name: errorName, - args: errorArgs - } = this.entryPoint.interface.parseError(data) - const errFullName = `${errorName}(${errorArgs.toString()})` - const errorResult = this._parseErrorResult(userOp, { - errorName, - errorArgs - }) - if (!errorName.includes('Result')) { - // a real error, not a result. - throw new Error(errFullName) + const [decodedSimulations] = entryPointSimulations.decodeFunctionResult('simulateValidation', data) + const validationResult = { + returnInfo: decodedSimulations.returnInfo, + senderInfo: { + ...decodedSimulations.senderInfo, + addr: userOp.sender + }, + factoryInfo: fillEntity(userOp.factory ?? '', decodedSimulations.factoryInfo), + paymasterInfo: fillEntity(userOp.paymaster ?? '', decodedSimulations.paymasterInfo), + aggregatorInfo: fillEntity(decodedSimulations.aggregatorInfo.aggregator, decodedSimulations.aggregatorInfo.stakeInfo) } + // const { + // name: errorName, + // args: errorArgs + // } = this.entryPoint.interface.parseError(data) + // const errFullName = `${errorName}(${errorArgs.toString()})` + // const errorResult = this._parseErrorResult(userOp, { + // errorName, + // errorArgs + // }) + // if (!errorName.includes('Result')) { + // // a real error, not a result. + // throw new Error(errFullName) + // } debug('==dump tree=', JSON.stringify(tracerResult, null, 2) .replace(new RegExp(userOp.sender.toLowerCase()), '{sender}') .replace(new RegExp(getAddr(userOp.paymaster) ?? '--no-paymaster--'), '{paymaster}') @@ -205,7 +214,7 @@ export class ValidationManager { ) // console.log('==debug=', ...tracerResult.numberLevels.forEach(x=>x.access), 'sender=', userOp.sender, 'paymaster=', hexlify(userOp.paymasterAndData)?.slice(0, 42)) // errorResult is "ValidationResult" - return [errorResult, tracerResult] + return [validationResult as any, tracerResult] } catch (e: any) { // if already parsed, throw as is if (e.code != null) { @@ -239,7 +248,9 @@ export class ValidationManager { let storageMap: StorageMap = {} if (!this.unsafe) { let tracerResult: BundlerTracerResult - [res, tracerResult] = await this._geth_traceCall_SimulateValidation(userOp) + [res, tracerResult] = await this._geth_traceCall_SimulateValidation(userOp).catch(e => { + throw e + }) let contractAddresses: string[] [contractAddresses, storageMap] = tracerResultParser(userOp, tracerResult, res, this.entryPoint) // if no previous contract hashes, then calculate hashes of contracts @@ -333,7 +344,7 @@ export class ValidationManager { requireAddressAndFields(userOp, 'factory', ['factoryData']) const calcPreVerificationGas1 = calcPreVerificationGas(userOp) - requireCond(userOp.preVerificationGas >= calcPreVerificationGas1, + requireCond(BigNumber.from(userOp.preVerificationGas).gte(calcPreVerificationGas1), `preVerificationGas too low: expected at least ${calcPreVerificationGas1}`, ValidationErrors.InvalidFields) }