From 851cbd6d8e311bc953a5b698e84c1e899c7db1f1 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 7 Jan 2025 15:18:25 +0100 Subject: [PATCH 01/29] AA-227: Define JSON schema for the alt-mempool configuration --- .../utils/src/altmempool/AltMempoolConfig.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 packages/utils/src/altmempool/AltMempoolConfig.ts diff --git a/packages/utils/src/altmempool/AltMempoolConfig.ts b/packages/utils/src/altmempool/AltMempoolConfig.ts new file mode 100644 index 00000000..50ad7047 --- /dev/null +++ b/packages/utils/src/altmempool/AltMempoolConfig.ts @@ -0,0 +1,50 @@ +type Role = 'sender' | 'paymaster' | 'factory' + +type EnterOpcode = 'CALL' | 'DELEGATECALL' | 'CALLCODE' | 'STATICCALL' | 'CREATE' | 'CREATE2' + +export interface AltMempoolRuleExceptionBase { + role?: Role + address?: string + depths?: number[] + enterOpcode?: EnterOpcode[] + enterMethodSelector?: `0x${string}` +} + +export interface AltMempoolRuleExceptionBannedOpcode extends AltMempoolRuleExceptionBase { + opcodes: string[] + slots: Array<`0x${string}`> +} + +type RuleException = `0x${string}` | Role | AltMempoolRuleExceptionBase | AltMempoolRuleExceptionBannedOpcode + +export interface BaseAltMempoolRule { + enabled?: boolean + exceptions?: RuleException[] +} + +// TODO: define all current rules +type RuleERC7562 = 'erep010' | 'erep020' + +export interface AltMempoolConfig { + [mempoolId: number]: { [rule in RuleERC7562]?: BaseAltMempoolRule } +} + +const config: AltMempoolConfig = { + 1: { + erep010: { + enabled: true, + exceptions: [ + 'sender', + '0xdeadbeef', + { + depths: [3], + enterOpcode: ['CALL'], + opcodes: ['SSTORE', 'SLOAD'], + slots: ['0xdeadbeef'] + } + ] + } + } +} + +console.log(config) From cfa7fc985dc549af9a8206d23521da8d3a413898 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Sun, 12 Jan 2025 20:43:16 +0100 Subject: [PATCH 02/29] WIP: Rewrite the ERC-7562 parser in a modular configurable way --- .../src/AccountAbstractionEntity.ts | 11 ++ .../validation-manager/src/ERC7562Rule.ts | 37 +++++++ .../src/ERC7562RuleViolation.ts | 17 +++ .../src/ERC7562TracerParser.ts | 102 ++++++++++++++++++ 4 files changed, 167 insertions(+) create mode 100644 packages/validation-manager/src/AccountAbstractionEntity.ts create mode 100644 packages/validation-manager/src/ERC7562Rule.ts create mode 100644 packages/validation-manager/src/ERC7562RuleViolation.ts create mode 100644 packages/validation-manager/src/ERC7562TracerParser.ts diff --git a/packages/validation-manager/src/AccountAbstractionEntity.ts b/packages/validation-manager/src/AccountAbstractionEntity.ts new file mode 100644 index 00000000..b96ee075 --- /dev/null +++ b/packages/validation-manager/src/AccountAbstractionEntity.ts @@ -0,0 +1,11 @@ +export const enum AccountAbstractionEntity { + sender = 'Sender', + paymaster = 'Paymaster', + factory = 'Factory', + aggregator = 'Aggregator', + senderCreator = 'SenderCreator', + entryPoint = 'EntryPoint', + // TODO: leaving 'fixme' entity for future refactor + // (some rules are checked in a way that makes it hard to find entity) + fixme = 'fixme' +} diff --git a/packages/validation-manager/src/ERC7562Rule.ts b/packages/validation-manager/src/ERC7562Rule.ts new file mode 100644 index 00000000..e453fe2d --- /dev/null +++ b/packages/validation-manager/src/ERC7562Rule.ts @@ -0,0 +1,37 @@ +export const enum ERC7562Rule { + op011 = 'OP-011', + op012 = 'OP-012', + op013 = 'OP-013', + op020 = 'OP-020', + op031 = 'OP-031', + op041 = 'OP-041', + op042 = 'OP-042', + op051 = 'OP-051', + op052 = 'OP-052', + op053 = 'OP-053', + op054 = 'OP-054', + op061 = 'OP-061', + op062 = 'OP-062', + op070 = 'OP-070', + op080 = 'OP-080', + cod010 = 'COD-010', + sto010 = 'STO-010', + sto021 = 'STO-021', + sto022 = 'STO-022', + sto031 = 'STO-031', + sto032 = 'STO-032', + sto033 = 'STO-033', + sto040 = 'STO-040', + sto041 = 'STO-041', + grep010 = 'GREP-010', + grep020 = 'GREP-020', + grep040 = 'GREP-040', + grep050 = 'GREP-050', + srep010 = 'SREP-010', + srep040 = 'SREP-040', + erep010 = 'EREP-010', + erep015 = 'EREP-015', + erep020 = 'EREP-020', + erep030 = 'EREP-030', + erep040 = 'EREP-040' +} diff --git a/packages/validation-manager/src/ERC7562RuleViolation.ts b/packages/validation-manager/src/ERC7562RuleViolation.ts new file mode 100644 index 00000000..3fba2bf4 --- /dev/null +++ b/packages/validation-manager/src/ERC7562RuleViolation.ts @@ -0,0 +1,17 @@ +import { ValidationErrors } from '@account-abstraction/utils' + +import { ERC7562Rule } from './ERC7562Rule' +import { AccountAbstractionEntity } from './AccountAbstractionEntity' + +export interface ERC7562RuleViolation { + rule: ERC7562Rule + depth: number + entity: AccountAbstractionEntity + address: string + errorCode: ValidationErrors + description: string + conflict?: string + opcode?: string + value?: string + slot?: string +} diff --git a/packages/validation-manager/src/ERC7562TracerParser.ts b/packages/validation-manager/src/ERC7562TracerParser.ts new file mode 100644 index 00000000..e2428e5e --- /dev/null +++ b/packages/validation-manager/src/ERC7562TracerParser.ts @@ -0,0 +1,102 @@ +import { ERC7562RuleViolation } from './ERC7562RuleViolation' +import { OperationBase, ValidationErrors } from '@account-abstraction/utils' +import { BundlerTracerResult, MethodInfo } from './BundlerCollectorTracer' +import { ERC7562Rule } from './ERC7562Rule' +import { AltMempoolConfig } from '@account-abstraction/utils/dist/src/altmempool/AltMempoolConfig' +import { AccountAbstractionEntity } from './AccountAbstractionEntity' +import { BigNumber } from 'ethers' + +export class ERC7562TracerParser { + constructor ( + readonly mempoolConfig: AltMempoolConfig, + readonly entryPointAddress: string + ) { + + } + + private _isCallToEntryPoint (call: MethodInfo): boolean { + return call.to?.toLowerCase() === this.entryPointAddress?.toLowerCase() && + call.from?.toLowerCase() !== this.entryPointAddress?.toLowerCase() + } + + /** + * Validates the UserOperation and throws an exception in case current mempool configuration rules were violated. + */ + requireCompliance ( + userOp: OperationBase, + tracerResults: BundlerTracerResult + ): void { + const violations = this.parseResults(userOp, tracerResults) + if (violations.length > 0) { + // TODO: human-readable description of which rules were violated. + throw new Error('Rules Violated') + } + } + + parseResults ( + userOp: OperationBase, + tracerResults: BundlerTracerResult + ): ERC7562RuleViolation[] { + return [] + } + + checkSanity (tracerResults: BundlerTracerResult): void { + if (Object.values(tracerResults.callsFromEntryPoint).length < 1) { + throw new Error('Unexpected traceCall result: no calls from entrypoint.') + } + } + + /** + * OP-052: May call `depositTo(sender)` with any value from either the `sender` or `factory`. + * OP-053: May call the fallback function from the `sender` with any value. + * OP-054: Any other access to the EntryPoint is forbidden. + */ + checkOp054 (tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { + const callStack = tracerResults.calls.filter((call: any) => call.topLevelTargetAddress == null) as MethodInfo[] + const callInfoEntryPoint = callStack.filter(call => { + const isCallToEntryPoint = this._isCallToEntryPoint(call) + const isEntryPointCallAllowedOP052 = call.method === 'depositTo' + const isEntryPointCallAllowedOP053 = call.method === '0x' + const isEntryPointCallAllowed = isEntryPointCallAllowedOP052 || isEntryPointCallAllowedOP053 + return isCallToEntryPoint && !isEntryPointCallAllowed + }) + return callInfoEntryPoint.map((it: MethodInfo): ERC7562RuleViolation => { + return { + rule: ERC7562Rule.op054, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: it.from, + opcode: it.type, + value: it.value, + errorCode: ValidationErrors.OpcodeValidation, + description: `illegal call into EntryPoint during validation ${it?.method}` + } + }) + } + + /** + * OP-061: CALL with value is forbidden. The only exception is a call to the EntryPoint. + */ + checkOp061 (tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { + const callStack = tracerResults.calls.filter((call: any) => call.topLevelTargetAddress == null) as MethodInfo[] + const illegalNonZeroValueCall = callStack.filter( + call => + !this._isCallToEntryPoint(call) && + !BigNumber.from(call.value ?? 0).eq(0) + ) + return illegalNonZeroValueCall.map((it: MethodInfo): ERC7562RuleViolation => { + return { + rule: ERC7562Rule.op061, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: it.from, + opcode: it.type, + value: it.value, + errorCode: ValidationErrors.OpcodeValidation, + description: 'May not may CALL with value' + } + }) + } +} From 449e29a86f97088c3f3b3488cdf02b0768d2aa44 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Sun, 12 Jan 2025 22:03:32 +0100 Subject: [PATCH 03/29] WIP: Add checks for OP-011 and OP-080 --- .../src/ERC7562TracerParser.ts | 94 ++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/packages/validation-manager/src/ERC7562TracerParser.ts b/packages/validation-manager/src/ERC7562TracerParser.ts index e2428e5e..343567fc 100644 --- a/packages/validation-manager/src/ERC7562TracerParser.ts +++ b/packages/validation-manager/src/ERC7562TracerParser.ts @@ -1,10 +1,11 @@ import { ERC7562RuleViolation } from './ERC7562RuleViolation' import { OperationBase, ValidationErrors } from '@account-abstraction/utils' -import { BundlerTracerResult, MethodInfo } from './BundlerCollectorTracer' +import { BundlerTracerResult, MethodInfo, TopLevelCallInfo } from './BundlerCollectorTracer' import { ERC7562Rule } from './ERC7562Rule' import { AltMempoolConfig } from '@account-abstraction/utils/dist/src/altmempool/AltMempoolConfig' import { AccountAbstractionEntity } from './AccountAbstractionEntity' import { BigNumber } from 'ethers' +import { bannedOpCodes, opcodesOnlyInStakedEntities } from './TracerResultParser' export class ERC7562TracerParser { constructor ( @@ -37,6 +38,10 @@ export class ERC7562TracerParser { userOp: OperationBase, tracerResults: BundlerTracerResult ): ERC7562RuleViolation[] { + this.checkSanity(tracerResults) + this.checkOp054(tracerResults) + this.checkOp061(tracerResults) + this.checkOp011(tracerResults) return [] } @@ -99,4 +104,91 @@ export class ERC7562TracerParser { } }) } + + /** + * OP-020: Revert on "out of gas" is forbidden as it can "leak" the gas limit or the current call stack depth. + */ + checkOp020 (tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { + const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) + const entityCallsWithOOG = entityCallsFromEntryPoint.filter((it: TopLevelCallInfo) => it.oog) + return entityCallsWithOOG.map((it: TopLevelCallInfo) => { + const entityTitle = 'fixme' + return { + rule: ERC7562Rule.op020, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: it.from ?? 'n/a', + opcode: it.type ?? 'n/a', + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `${entityTitle} internally reverts on oog` + } + }) + } + + /** + * OP-011: Blocked opcodes + * OP-080: `BALANCE` (0x31) and `SELFBALANCE` (0x47) are allowed only from a staked entity, else they are blocked + */ + checkOp011 (tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { + const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) + const violations: ERC7562RuleViolation[] = [] + for (const topLevelCallInfo of entityCallsFromEntryPoint) { + const opcodes = topLevelCallInfo.opcodes + const bannedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { + return bannedOpCodes.has(opcode) + }) + // TODO: TBD: Creating an object for each violation may be wasteful but makes it easier to choose a right mempool. + const bannedOpcodesViolations: ERC7562RuleViolation[] = + bannedOpCodeUsed + .map( + (opcode: string): ERC7562RuleViolation => { + const entityTitle = 'fixme' + return { + rule: ERC7562Rule.op011, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: topLevelCallInfo.from ?? 'n/a', + opcode, + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `${entityTitle} uses banned opcode: ${opcode}` + } + } + ) + violations.push(...bannedOpcodesViolations) + + // TODO: Deduplicate code in an elegant way + // TODO: Extract OP-080 into a separate function + const onlyStakedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { + return opcodesOnlyInStakedEntities.has(opcode) && !this._isEntityStaked(topLevelCallInfo) + }) + const onlyStakedOpcodesViolations: ERC7562RuleViolation[] = + onlyStakedOpCodeUsed + .map( + (opcode: string): ERC7562RuleViolation => { + const entityTitle = 'fixme' + return { + rule: ERC7562Rule.op011, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: topLevelCallInfo.from ?? 'n/a', + opcode, + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `unstaked ${entityTitle} uses banned opcode: ${opcode}` + } + } + ) + violations.push(...onlyStakedOpcodesViolations) + } + return violations + } + + private _isEntityStaked (topLevelCallInfo: TopLevelCallInfo): boolean { + throw new Error('Method not implemented.') + } } From c43b1b92bdda0f0ed1bab911157909f4bd87d595 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Sun, 12 Jan 2025 23:20:39 +0100 Subject: [PATCH 04/29] WIP: Check OP-031 rules violations --- .../src/ERC7562TracerParser.ts | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/validation-manager/src/ERC7562TracerParser.ts b/packages/validation-manager/src/ERC7562TracerParser.ts index 343567fc..37322987 100644 --- a/packages/validation-manager/src/ERC7562TracerParser.ts +++ b/packages/validation-manager/src/ERC7562TracerParser.ts @@ -1,5 +1,5 @@ import { ERC7562RuleViolation } from './ERC7562RuleViolation' -import { OperationBase, ValidationErrors } from '@account-abstraction/utils' +import { OperationBase, requireCond, ValidationErrors } from '@account-abstraction/utils' import { BundlerTracerResult, MethodInfo, TopLevelCallInfo } from './BundlerCollectorTracer' import { ERC7562Rule } from './ERC7562Rule' import { AltMempoolConfig } from '@account-abstraction/utils/dist/src/altmempool/AltMempoolConfig' @@ -188,6 +188,40 @@ export class ERC7562TracerParser { return violations } + /** + * OP-031: CREATE2 is allowed exactly once in the deployment phase and must deploy code for the "sender" address + */ + checkOp031 ( + userOp: OperationBase, + tracerResults: BundlerTracerResult + ): ERC7562RuleViolation[] { + const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) + const violations: ERC7562RuleViolation[] = [] + for (const topLevelCallInfo of entityCallsFromEntryPoint) { + if (topLevelCallInfo.type !== 'CREATE2') { + continue + } + const entityTitle = 'fixme' as string + const factoryStaked = false + const isAllowedCreateByOP032 = entityTitle === 'account' && factoryStaked && topLevelCallInfo.from === userOp.sender.toLowerCase() + const isAllowedCreateByEREP060 = entityTitle === 'factory' && topLevelCallInfo.from === userOp.factory && factoryStaked + const isAllowedCreateSenderByFactory = entityTitle === 'factory' && topLevelCallInfo.to === userOp.sender.toLowerCase() + if (!(isAllowedCreateByOP032 || isAllowedCreateByEREP060 || isAllowedCreateSenderByFactory)) { + violations.push({ + rule: ERC7562Rule.op011, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: topLevelCallInfo.from ?? 'n/a', + opcode: 'CREATE2', + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `${entityTitle} uses banned opcode: CREATE2` + }) + } + } + return violations + } private _isEntityStaked (topLevelCallInfo: TopLevelCallInfo): boolean { throw new Error('Method not implemented.') } From 23a64c3ad16b730e766c37eec88adc0ba540f9cc Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 13 Jan 2025 14:40:30 +0100 Subject: [PATCH 05/29] WIP: Express some STO rules in readable format --- .../src/BundlerCollectorTracer.ts | 14 ++-- .../src/ERC7562TracerParser.ts | 83 +++++++++++++++++-- 2 files changed, 84 insertions(+), 13 deletions(-) diff --git a/packages/validation-manager/src/BundlerCollectorTracer.ts b/packages/validation-manager/src/BundlerCollectorTracer.ts index ac93d1ae..583cfdcc 100644 --- a/packages/validation-manager/src/BundlerCollectorTracer.ts +++ b/packages/validation-manager/src/BundlerCollectorTracer.ts @@ -51,13 +51,17 @@ export interface ExitInfo { data: string } +export type StorageAccessInfos = { [address: string]: AccessInfo } +export type OpcodeInfos = { [address: string]: number } +export type ContractSizes = { [address: string]: ContractSizeInfo } +export type ExtCodeAccessInfos = { [address: string]: string } + export interface TopLevelCallInfo { - // topLevelMethodSig: string topLevelTargetAddress: string - opcodes: { [opcode: string]: number } - access: { [address: string]: AccessInfo } - contractSize: { [addr: string]: ContractSizeInfo } - extCodeAccessInfo: { [addr: string]: string } + opcodes: OpcodeInfos + access: StorageAccessInfos + contractSize: ContractSizes + extCodeAccessInfo: ExtCodeAccessInfos oog?: boolean calls?: [] type?: string diff --git a/packages/validation-manager/src/ERC7562TracerParser.ts b/packages/validation-manager/src/ERC7562TracerParser.ts index 37322987..32ebd2ed 100644 --- a/packages/validation-manager/src/ERC7562TracerParser.ts +++ b/packages/validation-manager/src/ERC7562TracerParser.ts @@ -1,11 +1,17 @@ import { ERC7562RuleViolation } from './ERC7562RuleViolation' import { OperationBase, requireCond, ValidationErrors } from '@account-abstraction/utils' -import { BundlerTracerResult, MethodInfo, TopLevelCallInfo } from './BundlerCollectorTracer' +import { + AccessInfo, + BundlerTracerResult, + MethodInfo, + StorageAccessInfos, + TopLevelCallInfo +} from './BundlerCollectorTracer' import { ERC7562Rule } from './ERC7562Rule' import { AltMempoolConfig } from '@account-abstraction/utils/dist/src/altmempool/AltMempoolConfig' import { AccountAbstractionEntity } from './AccountAbstractionEntity' import { BigNumber } from 'ethers' -import { bannedOpCodes, opcodesOnlyInStakedEntities } from './TracerResultParser' +import { associatedWith, bannedOpCodes, opcodesOnlyInStakedEntities } from './TracerResultParser' export class ERC7562TracerParser { constructor ( @@ -20,6 +26,14 @@ export class ERC7562TracerParser { call.from?.toLowerCase() !== this.entryPointAddress?.toLowerCase() } + private _isEntityStaked (topLevelCallInfo: TopLevelCallInfo): boolean { + throw new Error('Method not implemented.') + } + + private _getEntity (userOp: OperationBase, address: string): AccountAbstractionEntity { + return AccountAbstractionEntity.fixme + } + /** * Validates the UserOperation and throws an exception in case current mempool configuration rules were violated. */ @@ -42,6 +56,9 @@ export class ERC7562TracerParser { this.checkOp054(tracerResults) this.checkOp061(tracerResults) this.checkOp011(tracerResults) + this.checkOp020(tracerResults) + this.checkOp031(userOp, tracerResults) + this.checkStorage(userOp, tracerResults) return [] } @@ -198,13 +215,16 @@ export class ERC7562TracerParser { const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) const violations: ERC7562RuleViolation[] = [] for (const topLevelCallInfo of entityCallsFromEntryPoint) { - if (topLevelCallInfo.type !== 'CREATE2') { + if ( + topLevelCallInfo.type !== 'CREATE' && + topLevelCallInfo.type !== 'CREATE2' + ) { continue } const entityTitle = 'fixme' as string - const factoryStaked = false - const isAllowedCreateByOP032 = entityTitle === 'account' && factoryStaked && topLevelCallInfo.from === userOp.sender.toLowerCase() - const isAllowedCreateByEREP060 = entityTitle === 'factory' && topLevelCallInfo.from === userOp.factory && factoryStaked + const isFactoryStaked = false + const isAllowedCreateByOP032 = entityTitle === 'account' && isFactoryStaked && topLevelCallInfo.from === userOp.sender.toLowerCase() + const isAllowedCreateByEREP060 = entityTitle === 'factory' && topLevelCallInfo.from === userOp.factory && isFactoryStaked const isAllowedCreateSenderByFactory = entityTitle === 'factory' && topLevelCallInfo.to === userOp.sender.toLowerCase() if (!(isAllowedCreateByOP032 || isAllowedCreateByEREP060 || isAllowedCreateSenderByFactory)) { violations.push({ @@ -222,7 +242,54 @@ export class ERC7562TracerParser { } return violations } - private _isEntityStaked (topLevelCallInfo: TopLevelCallInfo): boolean { - throw new Error('Method not implemented.') + + checkStorage (userOp: OperationBase, tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { + this.checkStorageInternal(userOp, tracerResults.callsFromEntryPoint[0].access) + return [] + } + + checkStorageInternal (userOp: OperationBase, access: StorageAccessInfos): ERC7562RuleViolation[] { + Object.entries(access).forEach(([address, accessInfo]) => { + this.checkStorageInternalInternal(userOp, address, accessInfo) + }) + return [] + } + + checkStorageInternalInternal ( + userOp: OperationBase, + address: string, + accessInfo: AccessInfo + ): ERC7562RuleViolation[] { + const violations: ERC7562RuleViolation[] = [] + const allSlots: string[] = [ + ...Object.keys(accessInfo.writes), + ...Object.keys(accessInfo.reads), + ...Object.keys(accessInfo.transientWrites ?? {}), + ...Object.keys(accessInfo.transientReads ?? {}) + ] + const entitySlots = {} // TODO: restore + const entityAddress = '' // TODO: restore + const isEntityStaked = false // TODO + const isFactoryStaked = false // TODO + const isSenderCreation = false // TODO + for (const slot of allSlots) { + const isSenderAssociated: boolean = associatedWith(slot, userOp.sender.toLowerCase(), entitySlots) + const isEntityInternalSTO031: boolean = address.toLowerCase() === entityAddress.toLowerCase() + const isEntityAssociatedSTO032: boolean = associatedWith(slot, entityAddress, entitySlots) + const isReadOnlyAccessSTO033: boolean = accessInfo.writes[slot] == null && accessInfo.transientWrites[slot] == null + + const allowedST031ST032ST033: boolean = + (isEntityInternalSTO031 || isEntityAssociatedSTO032 || isReadOnlyAccessSTO033) && isEntityStaked + + const allowedSTO021: boolean = isSenderAssociated && !isSenderCreation + const allowedSTO022: boolean = isSenderAssociated && isSenderCreation && isFactoryStaked + const allowed = allowedSTO021 || allowedSTO022 || allowedST031ST032ST033 + if (!allowed) { + // TODO + // @ts-ignore + violations.push({}) + } + } + return violations } } From 266018f88f14c2536fea3835ffccadbfcccf034c Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 13 Jan 2025 15:58:04 +0100 Subject: [PATCH 06/29] More rules --- .../src/ERC7562TracerParser.ts | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/packages/validation-manager/src/ERC7562TracerParser.ts b/packages/validation-manager/src/ERC7562TracerParser.ts index 32ebd2ed..6848ff95 100644 --- a/packages/validation-manager/src/ERC7562TracerParser.ts +++ b/packages/validation-manager/src/ERC7562TracerParser.ts @@ -54,10 +54,12 @@ export class ERC7562TracerParser { ): ERC7562RuleViolation[] { this.checkSanity(tracerResults) this.checkOp054(tracerResults) + this.checkOp054ExtCode(userOp, tracerResults) this.checkOp061(tracerResults) this.checkOp011(tracerResults) this.checkOp020(tracerResults) this.checkOp031(userOp, tracerResults) + this.checkOp041(userOp, tracerResults) this.checkStorage(userOp, tracerResults) return [] } @@ -287,7 +289,58 @@ export class ERC7562TracerParser { if (!allowed) { // TODO // @ts-ignore - violations.push({}) + violations.push({ + // description: `${entityTitle} has forbidden ${readWrite} ${transientStr}${nameAddr(addr, stakeInfoEntities)} slot ${slot}`, + // description: `unstaked ${entityTitle} accessed ${nameAddr(addr, stakeInfoEntities)} slot ${requireStakeSlot}`, entityTitle, access) + }) + } + } + return violations + } + + checkOp041 ( + userOp: OperationBase, + tracerResults: BundlerTracerResult + ): ERC7562RuleViolation[] { + const entityTitle = 'fixme' + const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) + const violations: ERC7562RuleViolation[] = [] + for (const topLevelCallInfo of entityCallsFromEntryPoint) { + // the only contract we allow to access before its deployment is the "sender" itself, which gets created. + let illegalZeroCodeAccess: any + for (const addr of Object.keys(topLevelCallInfo.contractSize)) { + // [OP-042] + if (addr.toLowerCase() !== userOp.sender.toLowerCase() && addr.toLowerCase() !== this.entryPointAddress.toLowerCase() && topLevelCallInfo.contractSize[addr].contractSize <= 2) { + illegalZeroCodeAccess = topLevelCallInfo.contractSize[addr] + illegalZeroCodeAccess.address = addr + // @ts-ignore + violations.push({ + errorCode: ValidationErrors.OpcodeValidation, + description: `${entityTitle} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}`, + }) + } + } + } + return violations + } + + checkOp054ExtCode ( + userOp: OperationBase, + tracerResults: BundlerTracerResult + ): ERC7562RuleViolation[] { + const entityTitle = 'fixme' + const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) + const violations: ERC7562RuleViolation[] = [] + for (const topLevelCallInfo of entityCallsFromEntryPoint) { + let illegalEntryPointCodeAccess + for (const addr of Object.keys(topLevelCallInfo.extCodeAccessInfo)) { + if (addr.toLowerCase() === this.entryPointAddress.toLowerCase()) { + illegalEntryPointCodeAccess = topLevelCallInfo.extCodeAccessInfo[addr] + // @ts-ignore + violations.push({ + description: `${entityTitle} accesses EntryPoint contract address ${this.entryPointAddress} with opcode ${illegalEntryPointCodeAccess}`, + }) + } } } return violations From a7ce5bb07229a900a84400bbef898ba563bbf1f2 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 13 Jan 2025 16:27:52 +0100 Subject: [PATCH 07/29] Remove the old 'TracerResultsParser' --- .../bundler/src/modules/ExecutionManager.ts | 5 +- packages/bundler/src/modules/initServer.ts | 6 +- packages/bundler/test/BundlerManager.test.ts | 7 +- packages/bundler/test/BundlerServer.test.ts | 4 +- .../bundler/test/DebugMethodHandler.test.ts | 4 +- .../bundler/test/UserOpMethodHandler.test.ts | 4 +- packages/bundler/test/ValidateManager.test.ts | 4 +- packages/utils/src/index.ts | 1 + .../utils/src/interfaces/OperationBase.ts | 2 - .../src/BundlerCollectorTracer.ts | 8 +- .../src/ERC7562BannedOpcodes.ts | 3 + ...RC7562TracerParser.ts => ERC7562Parser.ts} | 73 +++- .../src/TracerResultParser.ts | 394 ------------------ .../src/ValidationManager.ts | 23 +- .../src/ValidationManagerRIP7560.ts | 5 +- packages/validation-manager/src/index.ts | 5 +- 16 files changed, 113 insertions(+), 435 deletions(-) create mode 100644 packages/validation-manager/src/ERC7562BannedOpcodes.ts rename packages/validation-manager/src/{ERC7562TracerParser.ts => ERC7562Parser.ts} (85%) delete mode 100644 packages/validation-manager/src/TracerResultParser.ts diff --git a/packages/bundler/src/modules/ExecutionManager.ts b/packages/bundler/src/modules/ExecutionManager.ts index 89a50e29..49d78337 100644 --- a/packages/bundler/src/modules/ExecutionManager.ts +++ b/packages/bundler/src/modules/ExecutionManager.ts @@ -15,6 +15,7 @@ import { DepositManager } from './DepositManager' import { BigNumberish, Signer } from 'ethers' import { BundlerConfig } from '../BundlerConfig' import { PreVerificationGasCalculator } from '@account-abstraction/sdk' +import { ERC7562Parser } from '@account-abstraction/validation-manager/dist/src/ERC7562Parser' const debug = Debug('aa.exec') @@ -148,10 +149,12 @@ export class ExecutionManager { async _setConfiguration (configOverrides: Partial): Promise { const { configuration, entryPoint, unsafe } = this.validationManager._getDebugConfiguration() const pvgc = new PreVerificationGasCalculator(Object.assign({}, configuration, configOverrides)) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address) this.validationManager = new ValidationManager( entryPoint, unsafe, - pvgc + pvgc, + erc7562Parser ) return pvgc } diff --git a/packages/bundler/src/modules/initServer.ts b/packages/bundler/src/modules/initServer.ts index 63145e80..6d8b0047 100644 --- a/packages/bundler/src/modules/initServer.ts +++ b/packages/bundler/src/modules/initServer.ts @@ -21,6 +21,7 @@ import { IBundleManager } from './IBundleManager' import { DepositManager } from './DepositManager' import { IRip7560StakeManager__factory } from '@account-abstraction/utils/dist/src/types' import { PreVerificationGasCalculator, ChainConfigs } from '@account-abstraction/sdk' +import { ERC7562Parser } from '@account-abstraction/validation-manager/dist/src/ERC7562Parser' /** * initialize server modules. @@ -35,16 +36,17 @@ export function initServer (config: BundlerConfig, signer: Signer): [ExecutionMa const eventsManager = new EventsManager(entryPoint, mempoolManager, reputationManager) const mergedPvgcConfig = Object.assign({}, ChainConfigs[config.chainId] ?? {}, config) const preVerificationGasCalculator = new PreVerificationGasCalculator(mergedPvgcConfig) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address) let validationManager: IValidationManager let bundleManager: IBundleManager if (!config.rip7560) { const tracerProvider = config.tracerRpcUrl == null ? undefined : getNetworkProvider(config.tracerRpcUrl) - validationManager = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, tracerProvider) + validationManager = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser, tracerProvider) bundleManager = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, signer, eventsManager, mempoolManager, validationManager, reputationManager, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc) } else { const stakeManager = IRip7560StakeManager__factory.connect(AA_STAKE_MANAGER, signer) - validationManager = new ValidationManagerRIP7560(stakeManager, entryPoint.provider as JsonRpcProvider, config.unsafe) + validationManager = new ValidationManagerRIP7560(stakeManager, entryPoint.provider as JsonRpcProvider, erc7562Parser, config.unsafe) bundleManager = new BundleManagerRIP7560(entryPoint.provider as JsonRpcProvider, signer, eventsManager, mempoolManager, validationManager, reputationManager, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc, false) } diff --git a/packages/bundler/test/BundlerManager.test.ts b/packages/bundler/test/BundlerManager.test.ts index a855f33c..fbd2c4dc 100644 --- a/packages/bundler/test/BundlerManager.test.ts +++ b/packages/bundler/test/BundlerManager.test.ts @@ -23,6 +23,7 @@ import { ExecutionManager } from '../src/modules/ExecutionManager' import { EventsManager } from '../src/modules/EventsManager' import { createSigner } from './testUtils' import { DepositManager } from '../src/modules/DepositManager' +import { ERC7562Parser } from '@account-abstraction/validation-manager/dist/src/ERC7562Parser' describe('#BundlerManager', () => { let bm: BundleManager @@ -63,7 +64,8 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) bm = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc) }) @@ -119,7 +121,8 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const validMgr = new ValidationManager(_entryPoint, config.unsafe, preVerificationGasCalculator) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const validMgr = new ValidationManager(_entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(_entryPoint, mempoolMgr, repMgr) bundleMgr = new BundleManager(_entryPoint, _entryPoint.provider as JsonRpcProvider, _entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) const depositManager = new DepositManager(entryPoint, mempoolMgr, bundleMgr) diff --git a/packages/bundler/test/BundlerServer.test.ts b/packages/bundler/test/BundlerServer.test.ts index 7b47c4af..846fbabc 100644 --- a/packages/bundler/test/BundlerServer.test.ts +++ b/packages/bundler/test/BundlerServer.test.ts @@ -23,6 +23,7 @@ import { ExecutionManager } from '../src/modules/ExecutionManager' import { MethodHandlerERC4337 } from '../src/MethodHandlerERC4337' import { BundlerConfig } from '../src/BundlerConfig' import { DepositManager } from '../src/modules/DepositManager' +import { ERC7562Parser } from '@account-abstraction/validation-manager/dist/src/ERC7562Parser' describe('BundleServer', function () { let entryPoint: IEntryPoint @@ -59,7 +60,8 @@ describe('BundleServer', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) const depositManager = new DepositManager(entryPoint, mempoolMgr, bundleMgr) diff --git a/packages/bundler/test/DebugMethodHandler.test.ts b/packages/bundler/test/DebugMethodHandler.test.ts index c5678d4c..d07fbaa0 100644 --- a/packages/bundler/test/DebugMethodHandler.test.ts +++ b/packages/bundler/test/DebugMethodHandler.test.ts @@ -25,6 +25,7 @@ import { MethodHandlerERC4337 } from '../src/MethodHandlerERC4337' import { createSigner } from './testUtils' import { EventsManager } from '../src/modules/EventsManager' import { DepositManager } from '../src/modules/DepositManager' +import { ERC7562Parser } from '@account-abstraction/validation-manager/dist/src/ERC7562Parser' const provider = ethers.provider @@ -69,7 +70,8 @@ describe('#DebugMethodHandler', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const eventsManager = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, 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 84d44af8..4200b4b2 100644 --- a/packages/bundler/test/UserOpMethodHandler.test.ts +++ b/packages/bundler/test/UserOpMethodHandler.test.ts @@ -34,6 +34,7 @@ import { ethers } from 'hardhat' import { createSigner } from './testUtils' import { EventsManager } from '../src/modules/EventsManager' import { DepositManager } from '../src/modules/DepositManager' +import { ERC7562Parser } from '@account-abstraction/validation-manager/dist/src/ERC7562Parser' describe('UserOpMethodHandler', function () { const helloWorld = 'hello world' @@ -87,7 +88,8 @@ describe('UserOpMethodHandler', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) const depositManager = new DepositManager(entryPoint, mempoolMgr, bundleMgr) diff --git a/packages/bundler/test/ValidateManager.test.ts b/packages/bundler/test/ValidateManager.test.ts index 129869bf..46843879 100644 --- a/packages/bundler/test/ValidateManager.test.ts +++ b/packages/bundler/test/ValidateManager.test.ts @@ -35,6 +35,7 @@ import { TestTimeRangeAccountFactory, TestTimeRangeAccountFactory__factory } from '../src/types' +import { ERC7562Parser } from '@account-abstraction/validation-manager/dist/src/ERC7562Parser' const cEmptyUserOp: UserOperation = { sender: AddressZero, @@ -154,7 +155,8 @@ describe('#ValidationManager', () => { const unsafe = !await supportsDebugTraceCall(provider, false) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - vm = new ValidationManager(entryPoint, unsafe, preVerificationGasCalculator) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + vm = new ValidationManager(entryPoint, unsafe, preVerificationGasCalculator, erc7562Parser) if (!await supportsDebugTraceCall(ethers.provider, false)) { console.log('WARNING: opcode banning tests can only run with geth') diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index a8c60d48..98787b9b 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -2,6 +2,7 @@ export { EIP7702Authorization, getEip7702AuthorizationSigner } from './interface export { UserOperation } from './interfaces/UserOperation' export { OperationBase } from './interfaces/OperationBase' export { OperationRIP7560 } from './interfaces/OperationRIP7560' +export { AltMempoolConfig } from './altmempool/AltMempoolConfig' export * from './Version' export * from './decodeRevertReason' export * from './ERC4337Utils' diff --git a/packages/utils/src/interfaces/OperationBase.ts b/packages/utils/src/interfaces/OperationBase.ts index 0523c30d..cddf756f 100644 --- a/packages/utils/src/interfaces/OperationBase.ts +++ b/packages/utils/src/interfaces/OperationBase.ts @@ -1,7 +1,5 @@ import { BigNumberish, BytesLike } from 'ethers' -import { EIP7702Authorization } from './EIP7702Authorization' - /** * The operation interface that is shared by ERC-4337 and RIP-7560 types. */ diff --git a/packages/validation-manager/src/BundlerCollectorTracer.ts b/packages/validation-manager/src/BundlerCollectorTracer.ts index 583cfdcc..60d7dc69 100644 --- a/packages/validation-manager/src/BundlerCollectorTracer.ts +++ b/packages/validation-manager/src/BundlerCollectorTracer.ts @@ -51,10 +51,10 @@ export interface ExitInfo { data: string } -export type StorageAccessInfos = { [address: string]: AccessInfo } -export type OpcodeInfos = { [address: string]: number } -export type ContractSizes = { [address: string]: ContractSizeInfo } -export type ExtCodeAccessInfos = { [address: string]: string } +export interface StorageAccessInfos { [address: string]: AccessInfo } +export interface OpcodeInfos { [address: string]: number } +export interface ContractSizes { [address: string]: ContractSizeInfo } +export interface ExtCodeAccessInfos { [address: string]: string } export interface TopLevelCallInfo { topLevelTargetAddress: string diff --git a/packages/validation-manager/src/ERC7562BannedOpcodes.ts b/packages/validation-manager/src/ERC7562BannedOpcodes.ts new file mode 100644 index 00000000..1b96e4e5 --- /dev/null +++ b/packages/validation-manager/src/ERC7562BannedOpcodes.ts @@ -0,0 +1,3 @@ +export const bannedOpCodes = new Set(['GASPRICE', 'GASLIMIT', 'DIFFICULTY', 'TIMESTAMP', 'BASEFEE', 'BLOCKHASH', 'NUMBER', 'ORIGIN', 'GAS', 'COINBASE', 'SELFDESTRUCT', 'RANDOM', 'PREVRANDAO', 'INVALID']) +// opcodes allowed in staked entities [OP-080] +export const opcodesOnlyInStakedEntities = new Set(['BALANCE', 'SELFBALANCE']) diff --git a/packages/validation-manager/src/ERC7562TracerParser.ts b/packages/validation-manager/src/ERC7562Parser.ts similarity index 85% rename from packages/validation-manager/src/ERC7562TracerParser.ts rename to packages/validation-manager/src/ERC7562Parser.ts index 6848ff95..da69d251 100644 --- a/packages/validation-manager/src/ERC7562TracerParser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -1,5 +1,9 @@ +import { BigNumber } from 'ethers' +import { hexZeroPad } from 'ethers/lib/utils' + +import { OperationBase, StorageMap, ValidationErrors, AltMempoolConfig } from '@account-abstraction/utils' + import { ERC7562RuleViolation } from './ERC7562RuleViolation' -import { OperationBase, requireCond, ValidationErrors } from '@account-abstraction/utils' import { AccessInfo, BundlerTracerResult, @@ -8,12 +12,17 @@ import { TopLevelCallInfo } from './BundlerCollectorTracer' import { ERC7562Rule } from './ERC7562Rule' -import { AltMempoolConfig } from '@account-abstraction/utils/dist/src/altmempool/AltMempoolConfig' import { AccountAbstractionEntity } from './AccountAbstractionEntity' -import { BigNumber } from 'ethers' -import { associatedWith, bannedOpCodes, opcodesOnlyInStakedEntities } from './TracerResultParser' +import { bannedOpCodes, opcodesOnlyInStakedEntities } from './ERC7562BannedOpcodes' +import { ValidationResult } from './IValidationManager' -export class ERC7562TracerParser { +export interface ERC7562ValidationResults { + storageMap: StorageMap + ruleViolations: ERC7562RuleViolation[] + contractAddresses: string[] +} + +export class ERC7562Parser { constructor ( readonly mempoolConfig: AltMempoolConfig, readonly entryPointAddress: string @@ -34,24 +43,47 @@ export class ERC7562TracerParser { return AccountAbstractionEntity.fixme } + private _associatedWith (slot: string, addr: string, entitySlots: { [addr: string]: Set }): boolean { + const addrPadded = hexZeroPad(addr, 32).toLowerCase() + if (slot === addrPadded) { + return true + } + const k = entitySlots[addr] + if (k == null) { + return false + } + const slotN = BigNumber.from(slot) + // scan all slot entries to check of the given slot is within a structure, starting at that offset. + // assume a maximum size on a (static) structure size. + for (const k1 of k.keys()) { + const kn = BigNumber.from(k1) + if (slotN.gte(kn) && slotN.lt(kn.add(128))) { + return true + } + } + return false + } + /** * Validates the UserOperation and throws an exception in case current mempool configuration rules were violated. */ requireCompliance ( userOp: OperationBase, - tracerResults: BundlerTracerResult - ): void { - const violations = this.parseResults(userOp, tracerResults) - if (violations.length > 0) { + tracerResults: BundlerTracerResult, + validationResult: ValidationResult + ): ERC7562ValidationResults { + const results = this.parseResults(userOp, tracerResults) + if (results.ruleViolations.length > 0) { // TODO: human-readable description of which rules were violated. throw new Error('Rules Violated') } + return results } parseResults ( userOp: OperationBase, tracerResults: BundlerTracerResult - ): ERC7562RuleViolation[] { + ): ERC7562ValidationResults { this.checkSanity(tracerResults) this.checkOp054(tracerResults) this.checkOp054ExtCode(userOp, tracerResults) @@ -61,7 +93,9 @@ export class ERC7562TracerParser { this.checkOp031(userOp, tracerResults) this.checkOp041(userOp, tracerResults) this.checkStorage(userOp, tracerResults) - return [] + return { + contractAddresses: [], ruleViolations: [], storageMap: {} + } } checkSanity (tracerResults: BundlerTracerResult): void { @@ -275,17 +309,18 @@ export class ERC7562TracerParser { const isFactoryStaked = false // TODO const isSenderCreation = false // TODO for (const slot of allSlots) { - const isSenderAssociated: boolean = associatedWith(slot, userOp.sender.toLowerCase(), entitySlots) + const isSenderInternalSTO010: boolean = address.toLowerCase() === userOp.sender.toLowerCase() + const isSenderAssociated: boolean = this._associatedWith(slot, userOp.sender.toLowerCase(), entitySlots) const isEntityInternalSTO031: boolean = address.toLowerCase() === entityAddress.toLowerCase() - const isEntityAssociatedSTO032: boolean = associatedWith(slot, entityAddress, entitySlots) + const isEntityAssociatedSTO032: boolean = this._associatedWith(slot, entityAddress, entitySlots) const isReadOnlyAccessSTO033: boolean = accessInfo.writes[slot] == null && accessInfo.transientWrites[slot] == null - const allowedST031ST032ST033: boolean = + const isAllowedST031ST032ST033: boolean = (isEntityInternalSTO031 || isEntityAssociatedSTO032 || isReadOnlyAccessSTO033) && isEntityStaked - const allowedSTO021: boolean = isSenderAssociated && !isSenderCreation - const allowedSTO022: boolean = isSenderAssociated && isSenderCreation && isFactoryStaked - const allowed = allowedSTO021 || allowedSTO022 || allowedST031ST032ST033 + const isAllowedSTO021: boolean = isSenderAssociated && !isSenderCreation + const isAllowedSTO022: boolean = isSenderAssociated && isSenderCreation && isFactoryStaked + const allowed = isSenderInternalSTO010 || isAllowedSTO021 || isAllowedSTO022 || isAllowedST031ST032ST033 if (!allowed) { // TODO // @ts-ignore @@ -316,7 +351,7 @@ export class ERC7562TracerParser { // @ts-ignore violations.push({ errorCode: ValidationErrors.OpcodeValidation, - description: `${entityTitle} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}`, + description: `${entityTitle} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}` }) } } @@ -338,7 +373,7 @@ export class ERC7562TracerParser { illegalEntryPointCodeAccess = topLevelCallInfo.extCodeAccessInfo[addr] // @ts-ignore violations.push({ - description: `${entityTitle} accesses EntryPoint contract address ${this.entryPointAddress} with opcode ${illegalEntryPointCodeAccess}`, + description: `${entityTitle} accesses EntryPoint contract address ${this.entryPointAddress} with opcode ${illegalEntryPointCodeAccess}` }) } } diff --git a/packages/validation-manager/src/TracerResultParser.ts b/packages/validation-manager/src/TracerResultParser.ts deleted file mode 100644 index 7d157b43..00000000 --- a/packages/validation-manager/src/TracerResultParser.ts +++ /dev/null @@ -1,394 +0,0 @@ -// This file contains references to validation rules, in the format [xxx-###] -// where xxx is OP/STO/COD/EP/SREP/EREP/UREP/ALT, and ### is a number -// the validation rules are defined in erc-aa-validation.md -import Debug from 'debug' -import { BigNumber } from 'ethers' -import { hexZeroPad, keccak256 } from 'ethers/lib/utils' -import { inspect } from 'util' - -import { AccessInfo, BundlerTracerResult, MethodInfo, TopLevelCallInfo } from './BundlerCollectorTracer' -import { - OperationBase, - RpcError, - StakeInfo, - StorageMap, - ValidationErrors, - mapOf, - requireCond, - toBytes32, AddressZero, UserOperation -} from '@account-abstraction/utils' - -import { ValidationResult } from './IValidationManager' - -const debug = Debug('aa.handler.opcodes') - -/** - * slots associated with each entity. - * keccak( A || ...) is associated with "A" - * removed rule: keccak( ... || ASSOC ) (for a previously associated hash) is also associated with "A" - * - * @param stakeInfoEntities stake info for (factory, account, paymaster). factory and paymaster can be null. - * @param keccak array of buffers that were given to keccak in the transaction - */ -function parseEntitySlots (stakeInfoEntities: { [addr: string]: StakeInfo | undefined }, keccak: string[]): { - [addr: string]: Set -} { - // for each entity (sender, factory, paymaster), hold the valid slot addresses - // valid: the slot was generated by keccak(entity || ...) - const entitySlots: { [addr: string]: Set } = {} - - keccak.forEach(k => { - Object.values(stakeInfoEntities).forEach(info => { - const addr = info?.addr?.toLowerCase() - if (addr == null) return - const addrPadded = toBytes32(addr) - if (entitySlots[addr] == null) { - entitySlots[addr] = new Set() - } - - const currentEntitySlots = entitySlots[addr] - - // valid slot: the slot was generated by keccak(entityAddr || ...) - if (k.startsWith(addrPadded)) { - // console.log('added mapping (balance) slot', value) - currentEntitySlots.add(keccak256(k)) - } - // disabled 2nd rule: .. or by keccak( ... || OWN) where OWN is previous allowed slot - // if (k.length === 130 && currentEntitySlots.has(k.slice(-64))) { - // // console.log('added double-mapping (allowance) slot', value) - // currentEntitySlots.add(value) - // } - }) - }) - - return entitySlots -} - -// -// // method-signature for calls from entryPoint -// const callsFromEntryPointMethodSigs1: { [key: string]: string } = { -// factory: SenderCreator__factory.createInterface().getSighash('createSender'), -// account: IAccount__factory.createInterface().getSighash('validateUserOp'), -// paymaster: IPaymaster__factory.createInterface().getSighash('validatePaymasterUserOp') -// } -// -// // todo: use a selector for factory or refactor the tracer for RIP-7560 -// const callsFromEntryPointMethodSigs: { [key: string]: string } = { -// account: '0xbf45c166', -// paymaster: '0xe0e6183a', -// } - -function getEntityTitle (userOp: OperationBase, entityAddress: string): string { - if (userOp.sender.toLowerCase() === entityAddress.toLowerCase()) { - return 'account' - } else if (userOp.factory?.toLowerCase() === entityAddress.toLowerCase()) { - return 'factory' - } else if (userOp.paymaster?.toLowerCase() === entityAddress.toLowerCase()) { - return 'paymaster' - } else { - throw new RpcError(`could not find entity name for address ${entityAddress}. This should not happen. This is a bug.`, 0) - } -} - -// opcodes from [OP-011] -// (CREATE, CREATE2 have special rules, OP-031, OP-032) -const bannedOpCodes = new Set(['GASPRICE', 'GASLIMIT', 'DIFFICULTY', 'TIMESTAMP', 'BASEFEE', 'BLOCKHASH', 'NUMBER', 'ORIGIN', 'GAS', 'COINBASE', 'SELFDESTRUCT', 'RANDOM', 'PREVRANDAO', 'INVALID']) -// opcodes allowed in staked entities [OP-080] -const opcodesOnlyInStakedEntities = new Set(['BALANCE', 'SELFBALANCE']) - -/** - * parse collected simulation traces and revert if they break our rules - * @param userOp the userOperation that was used in this simulation - * @param tracerResults the tracer return value - * @param validationResult output from simulateValidation - * @param entryPointAddress the entryPoint that hosted the "simulatedValidation" traced call. - * @return list of contract addresses referenced by this UserOp - */ -export function tracerResultParser ( - userOp: OperationBase, - tracerResults: BundlerTracerResult, - validationResult: ValidationResult, - entryPointAddress: string -): [string[], StorageMap] { - debug('=== simulation result:', inspect(tracerResults, true, 10, true)) - // todo: block access to no-code addresses (might need update to tracer) - - // eslint-disable-next-line @typescript-eslint/no-base-to-string - if (Object.values(tracerResults.callsFromEntryPoint).length < 1) { - throw new Error('Unexpected traceCall result: no calls from entrypoint.') - } - if (tracerResults.calls != null) { - const callStack = tracerResults.calls.filter((call: any) => call.topLevelTargetAddress == null) as MethodInfo[] - // [OP-052], [OP-053] - const callInfoEntryPoint = callStack.find(call => - call.to?.toLowerCase() === entryPointAddress?.toLowerCase() && call.from?.toLowerCase() !== entryPointAddress?.toLowerCase() && - (call.method !== '0x' && call.method !== 'depositTo')) - // [OP-054] - requireCond(callInfoEntryPoint == null, - `illegal call into EntryPoint during validation ${callInfoEntryPoint?.method}`, - ValidationErrors.OpcodeValidation - ) - - // [OP-061] - const illegalNonZeroValueCall = callStack.find( - call => - call.to?.toLowerCase() !== entryPointAddress?.toLowerCase() && - !BigNumber.from(call.value ?? 0).eq(0)) - requireCond( - illegalNonZeroValueCall == null, - 'May not may CALL with value', - ValidationErrors.OpcodeValidation) - } - - const sender = userOp.sender.toLowerCase() - // stake info per "number" level (factory, sender, paymaster) - // we only use stake info if we notice a memory reference that require stake - const stakeInfoEntities = { - [sender]: validationResult.senderInfo - } - const factory = userOp.factory?.toLowerCase() - if (factory != null) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - stakeInfoEntities[factory] = validationResult.factoryInfo! - } - const paymaster = userOp.paymaster?.toLowerCase() - if (paymaster != null) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - stakeInfoEntities[paymaster] = validationResult.paymasterInfo! - } - - const entitySlots: { [addr: string]: Set } = parseEntitySlots(stakeInfoEntities, tracerResults.keccak) - Object.entries(stakeInfoEntities).forEach(([entityAddress, entStakes]) => { - const entityTitle = getEntityTitle(userOp, entityAddress) - const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter( - call => call.topLevelTargetAddress != null && call.topLevelTargetAddress.toLowerCase() === entityAddress.toLowerCase()) - entityCallsFromEntryPoint.forEach((entityCall) => { - if (entityCall == null) { - if (entityAddress.toLowerCase() === userOp.sender.toLowerCase()) { - // should never happen... only factory, paymaster are optional. - throw new RpcError('missing trace into account validation', ValidationErrors.InvalidFields) - } - return - } - processEntityCall(entityCall, entityAddress, entityTitle, entStakes, entitySlots, userOp as UserOperation, stakeInfoEntities, entryPointAddress, tracerResults) - }) - }) - // return list of contract addresses by this UserOp. already known not to contain zero-sized addresses. - const addresses = tracerResults.callsFromEntryPoint.flatMap(level => Object.keys(level.contractSize)) - const storageMap: StorageMap = {} - tracerResults.callsFromEntryPoint.forEach(level => { - Object.keys(level.access).forEach(addr => { - storageMap[addr] = storageMap[addr] ?? level.access[addr].reads - }) - }) - return [addresses, storageMap] -} - -function processEntityCall (entityCall: TopLevelCallInfo, entityAddress: string, entityTitle: string, entStakes: StakeInfo, entitySlots: { [addr: string]: Set }, userOp: UserOperation, stakeInfoEntities: {[addr: string]: StakeInfo}, entryPointAddress: string, tracerResults: BundlerTracerResult, depth = 0): void { - const opcodes = entityCall.opcodes - const access = entityCall.access - - // [OP-020] - requireCond(!(entityCall.oog ?? false), - `${entityTitle} internally reverts on oog`, ValidationErrors.OpcodeValidation) - - // opcodes from [OP-011] - Object.keys(opcodes).forEach(opcode => { - requireCond(!bannedOpCodes.has(opcode), `${entityTitle} uses banned opcode: ${opcode}`, ValidationErrors.OpcodeValidation) - // [OP-080] - requireCond(!opcodesOnlyInStakedEntities.has(opcode) || isStaked(entStakes), - `unstaked ${entityTitle} uses banned opcode: ${opcode}`, ValidationErrors.OpcodeValidation) - }) - - if (entityCall.type === 'CREATE') { - // CREATE is allowed only from the entity (factory, account) itself, not from inner contract - // top-level is "handleOps" (or simulateValidation) itself - requireCond(depth === 1, - `${entityTitle} uses banned opcode: CREATE from inner call depth=${depth}`, ValidationErrors.OpcodeValidation) - - // [OP-032] If there is a `factory` (even unstaked), the `sender` contract is allowed to use `CREATE` opcode - requireCond( - (entityTitle === 'account' && userOp.factory != null) || - (entityTitle === 'factory' && isStaked(entStakes)), - `${entityTitle} uses banned opcode: CREATE`, ValidationErrors.OpcodeValidation - ) - } - if (entityCall.type === 'CREATE2') { - const factory = userOp.factory?.toLowerCase() ?? '' - const factoryStaked = isStaked(stakeInfoEntities[factory]) - console.log(`=== CREATE2: title:${entityTitle} factory-staked:${factoryStaked} callfromsender:${entityCall.from === userOp.sender.toLowerCase()}`) - console.log({ - stakeInfoEntities, - userOp - }) - requireCond( - (entityTitle === 'account' && factoryStaked && entityCall.from === userOp.sender.toLowerCase()) || // OP-032 - (entityTitle === 'factory' && ( - entityCall.to === userOp.sender.toLowerCase() || - (entityCall.from === userOp.factory && isStaked(entStakes)) - )), - `${entityTitle} uses banned opcode: CREATE2`, ValidationErrors.OpcodeValidation - ) - } - - Object.entries(access).forEach(([addr, { - reads, - writes, - transientReads, - transientWrites - }]) => { - // testing read/write access on contract "addr" - if (addr.toLowerCase() === userOp.sender.toLowerCase()) { - // allowed to access sender's storage - // [STO-010] - return - } - - if (addr.toLowerCase() === entryPointAddress.toLowerCase()) { - // ignore storage access on entryPoint (balance/deposit of entities. - // we block them on method calls: only allowed to deposit, never to read - return - } - - debug('dump keccak calculations and reads', { - entityTitle, - entityAddress, - k: mapOf(tracerResults.keccak, k => keccak256(k)), - reads - }) - - // [OP-070]: treat transient storage (TLOAD/TSTORE) just like storage. - // scan all slots. find a referenced slot - // at the end of the scan, we will check if the entity has stake, and report that slot if not. - let requireStakeSlot: string | undefined - [ - ...Object.keys(writes), - ...Object.keys(reads), - ...Object.keys(transientWrites ?? {}), - ...Object.keys(transientReads ?? {}) - ].forEach(slot => { - // slot associated with sender is allowed (e.g. token.balanceOf(sender) - // but during initial UserOp (where there is an initCode), it is allowed only for staked entity - if (associatedWith(slot, userOp.sender.toLowerCase(), entitySlots)) { - if (userOp.factory != null && userOp.factory !== AddressZero) { - // special case: account.validateUserOp is allowed to use assoc storage if factory is staked. - // [STO-022], [STO-021] - if (!(entityAddress.toLowerCase() === userOp.sender.toLowerCase() && isStaked(stakeInfoEntities[userOp.factory.toLowerCase()]))) { - requireStakeSlot = slot - } - } - } else if (associatedWith(slot, entityAddress, entitySlots)) { - // [STO-032] - // accessing a slot associated with entityAddr (e.g. token.balanceOf(paymaster) - requireStakeSlot = slot - } else if (addr.toLowerCase() === entityAddress.toLowerCase()) { - // [STO-031] - // accessing storage member of entity itself requires stake. - requireStakeSlot = slot - } else if (writes[slot] == null && transientWrites[slot] == null) { - // [STO-033]: staked entity have read-only access to any storage in non-entity contract. - requireStakeSlot = slot - } else { - // accessing arbitrary storage of another contract is not allowed - const isWrite = Object.keys(writes).includes(slot) || Object.keys(transientWrites ?? {}).includes(slot) - const isTransient = Object.keys(transientReads ?? {}).includes(slot) || Object.keys(transientWrites ?? {}).includes(slot) - const readWrite = isWrite ? 'write to' : 'read from' - const transientStr = isTransient ? 'transient ' : '' - requireCond(false, - `${entityTitle} has forbidden ${readWrite} ${transientStr}${nameAddr(addr, stakeInfoEntities)} slot ${slot}`, - ValidationErrors.OpcodeValidation, { [entityTitle]: entStakes?.addr }) - } - }) - - requireCondAndStake(requireStakeSlot != null, entStakes, - `unstaked ${entityTitle} accessed ${nameAddr(addr, stakeInfoEntities)} slot ${requireStakeSlot}`, entityTitle, access) - }) - - // the only contract we allow to access before its deployment is the "sender" itself, which gets created. - let illegalZeroCodeAccess: any - for (const addr of Object.keys(entityCall.contractSize)) { - // [OP-042] - if (addr.toLowerCase() !== userOp.sender.toLowerCase() && addr.toLowerCase() !== entryPointAddress.toLowerCase() && entityCall.contractSize[addr].contractSize <= 2) { - illegalZeroCodeAccess = entityCall.contractSize[addr] - illegalZeroCodeAccess.address = addr - break - } - } - // [OP-041] - requireCond( - illegalZeroCodeAccess == null, - `${entityTitle} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}`, - ValidationErrors.OpcodeValidation) - - let illegalEntryPointCodeAccess - for (const addr of Object.keys(entityCall.extCodeAccessInfo)) { - if (addr.toLowerCase() === entryPointAddress.toLowerCase()) { - illegalEntryPointCodeAccess = entityCall.extCodeAccessInfo[addr] - break - } - } - requireCond( - illegalEntryPointCodeAccess == null, - `${entityTitle} accesses EntryPoint contract address ${entryPointAddress} with opcode ${illegalEntryPointCodeAccess}`, - ValidationErrors.OpcodeValidation) - - // Recursively handling all subcalls to check validation rules - if (entityCall.calls != null) { - entityCall.calls.forEach((call: any) => { - processEntityCall(call, entityAddress, entityTitle, entStakes, entitySlots, userOp, stakeInfoEntities, entryPointAddress, tracerResults, depth + 1) - }) - } -} - -// helper method: if condition is true, then entity must be staked. -function requireCondAndStake (cond: boolean, entStake: StakeInfo | undefined, failureMessage: string, entityTitle: string, access: { [address: string]: AccessInfo }): void { - if (!cond) { - return - } - if (entStake == null) { - throw new Error(`internal: ${entityTitle} not in userOp, but has storage accesses in ${JSON.stringify(access)}`) - } - requireCond(isStaked(entStake), - failureMessage, ValidationErrors.OpcodeValidation, { [entityTitle]: entStake?.addr }) - - // TODO: check real minimum stake values -} - -// check if the given entity is staked -function isStaked (entStake?: StakeInfo): boolean { - return entStake != null && BigNumber.from(1).lte(entStake.stake) && BigNumber.from(1).lte(entStake.unstakeDelaySec) -} - -// if addr is current account/paymaster/factory, then return that title -// otherwise, return addr as-is -function nameAddr (addr: string, stakeInfoEntities: {[addr: string]: StakeInfo}): string { - const [title] = Object.entries(stakeInfoEntities).find(([title, info]) => - info?.addr.toLowerCase() === addr.toLowerCase()) ?? [] - - return title ?? addr -} - -// return true if the given slot is associated with the given address, given the known keccak operations: -// @param slot the SLOAD/SSTORE slot address we're testing -// @param addr - the address we try to check for association with -// @param reverseKeccak - a mapping we built for keccak values that contained the address -function associatedWith (slot: string, addr: string, entitySlots: { [addr: string]: Set }): boolean { - const addrPadded = hexZeroPad(addr, 32).toLowerCase() - if (slot === addrPadded) { - return true - } - const k = entitySlots[addr] - if (k == null) { - return false - } - const slotN = BigNumber.from(slot) - // scan all slot entries to check of the given slot is within a structure, starting at that offset. - // assume a maximum size on a (static) structure size. - for (const k1 of k.keys()) { - const kn = BigNumber.from(k1) - if (slotN.gte(kn) && slotN.lt(kn.add(128))) { - return true - } - } - return false -} diff --git a/packages/validation-manager/src/ValidationManager.ts b/packages/validation-manager/src/ValidationManager.ts index c6e92d8a..344cfe33 100644 --- a/packages/validation-manager/src/ValidationManager.ts +++ b/packages/validation-manager/src/ValidationManager.ts @@ -31,13 +31,13 @@ import { runContractScript, getAuthorizationList, SenderCreator__factory, IEntryPoint__factory, IPaymaster__factory } from '@account-abstraction/utils' -import { tracerResultParser } from './TracerResultParser' import { bundlerCollectorTracer, BundlerTracerResult, ExitInfo } from './BundlerCollectorTracer' import { debug_traceCall } from './GethTracer' import EntryPointSimulationsJson from '@account-abstraction/contracts/artifacts/EntryPointSimulations.json' import { IValidationManager, ValidateUserOpResult, ValidationResult } from './IValidationManager' import { Interface } from 'ethers/lib/utils' +import { ERC7562Parser } from './ERC7562Parser' const debug = Debug('aa.mgr.validate') @@ -61,6 +61,7 @@ export class ValidationManager implements IValidationManager { readonly entryPoint: IEntryPoint, readonly unsafe: boolean, readonly preVerificationGasCalculator: PreVerificationGasCalculator, + readonly erc7562Parser: ERC7562Parser, readonly providerForTracer?: JsonRpcProvider ) { this.provider = this.entryPoint.provider as JsonRpcProvider @@ -268,7 +269,7 @@ export class ValidationManager implements IValidationManager { // console.log('tracer res') // console.dir(tracerResult, { depth: null }) let contractAddresses: string[] - [contractAddresses, storageMap] = tracerResultParser(userOp, tracerResult, res, this.entryPoint.address) + ({ contractAddresses, storageMap } = this.erc7562Parser.requireCompliance(userOp, tracerResult, res)) // if no previous contract hashes, then calculate hashes of contracts if (previousCodeHashes == null) { codeHashes = await this.getCodeHashes(contractAddresses) @@ -410,7 +411,7 @@ export class ValidationManager implements IValidationManager { convertTracerResult (tracerResult: any, userOp: UserOperation): BundlerTracerResult { const SENDER_CREATOR = '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c'.toLowerCase() // Before flattening we add top level addresses for calls from EntryPoint and from SENDER_CREATOR - tracerResult.calls.forEach((call: {calls: any, to: any, topLevelTargetAddress: any}) => { + tracerResult.calls.forEach((call: { calls: any, to: any, topLevelTargetAddress: any }) => { call.topLevelTargetAddress = call.to if (call.to.toLowerCase() === SENDER_CREATOR && call.calls != null) { call.calls.forEach((subcall: any) => { @@ -419,7 +420,20 @@ export class ValidationManager implements IValidationManager { } }) tracerResult.calls = this.flattenCalls(tracerResult.calls) - tracerResult.calls.forEach((call: { topLevelTargetAddress: any, method: any, input: any, to: any, from: any, opcodes: any, usedOpcodes: any, access: any, accessedSlots: any, extCodeAccessInfo: any, outOfGas: any, oog: any }) => { + tracerResult.calls.forEach((call: { + topLevelTargetAddress: any + method: any + input: any + to: any + from: any + opcodes: any + usedOpcodes: any + access: any + accessedSlots: any + extCodeAccessInfo: any + outOfGas: any + oog: any + }) => { call.opcodes = {} if (call.usedOpcodes != null) { Object.keys(call.usedOpcodes).forEach((opcode: string) => { @@ -481,6 +495,7 @@ export class ValidationManager implements IValidationManager { return def } } + const methodSig = call.input.slice(0, 10) const method = callCatch(() => AbiInterfaces.getFunction(methodSig), methodSig) call.method = method.name diff --git a/packages/validation-manager/src/ValidationManagerRIP7560.ts b/packages/validation-manager/src/ValidationManagerRIP7560.ts index 4b1e3351..1367637a 100644 --- a/packages/validation-manager/src/ValidationManagerRIP7560.ts +++ b/packages/validation-manager/src/ValidationManagerRIP7560.ts @@ -15,7 +15,7 @@ import { PreVerificationGasCalculatorConfig } from '@account-abstraction/sdk' import { IValidationManager, ValidateUserOpResult, ValidationResult } from './IValidationManager' import { eth_traceRip7560Validation } from './GethTracer' -import { tracerResultParser } from './TracerResultParser' +import { ERC7562Parser } from './ERC7562Parser' export const AA_ENTRY_POINT = '0x0000000000000000000000000000000000007560' export const AA_STAKE_MANAGER = '0x570Aa568b6cf62ff08c6C3a3b3DB1a0438E871Fb' @@ -24,6 +24,7 @@ export class ValidationManagerRIP7560 implements IValidationManager { constructor ( readonly stakeManager: IRip7560StakeManager, readonly provider: JsonRpcProvider, + readonly erc7562Parser: ERC7562Parser, readonly unsafe: boolean ) { } @@ -101,7 +102,7 @@ export class ValidationManagerRIP7560 implements IValidationManager { // this.parseValidationTracingResult(traceResult) // let contractAddresses: string[] // [contractAddresses, storageMap] = - tracerResultParser(operation, traceResult, validationResult, AA_ENTRY_POINT) + this.erc7562Parser.requireCompliance(operation, traceResult, validationResult) // TODO alex shahaf handle codehashes // if no previous contract hashes, then calculate hashes of contracts if (previousCodeHashes == null) { diff --git a/packages/validation-manager/src/index.ts b/packages/validation-manager/src/index.ts index 3698a76d..b01db014 100644 --- a/packages/validation-manager/src/index.ts +++ b/packages/validation-manager/src/index.ts @@ -12,6 +12,7 @@ import { bundlerJSTracerName, debug_traceCall, eth_traceRip7560Validation } from import { bundlerCollectorTracer } from './BundlerCollectorTracer' import { ValidateUserOpResult } from './IValidationManager' import { ValidationManager } from './ValidationManager' +import { ERC7562Parser } from './ERC7562Parser' export * from './ValidationManager' export * from './ValidationManagerRIP7560' @@ -82,10 +83,12 @@ export async function checkRulesViolations ( throw new Error('This provider does not support stack tracing') } const entryPoint = IEntryPoint__factory.connect(entryPointAddress, provider) + const erc7562Parser = new ERC7562Parser({}, entryPointAddress) const validationManager = new ValidationManager( entryPoint, false, - Object.assign({}) as PreVerificationGasCalculator + Object.assign({}) as PreVerificationGasCalculator, + erc7562Parser ) return await validationManager.validateUserOp(userOperation) } From a9c6fca3a14ea2bac8d1050ce4ee8db893f3b8c4 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 14 Jan 2025 01:17:55 +0100 Subject: [PATCH 08/29] Define and validate parser results schema --- .../validation-manager/src/ERC7562Call.ts | 76 +++++++++++++++++++ .../src/ValidationManager.ts | 2 + 2 files changed, 78 insertions(+) create mode 100644 packages/validation-manager/src/ERC7562Call.ts diff --git a/packages/validation-manager/src/ERC7562Call.ts b/packages/validation-manager/src/ERC7562Call.ts new file mode 100644 index 00000000..26451446 --- /dev/null +++ b/packages/validation-manager/src/ERC7562Call.ts @@ -0,0 +1,76 @@ +import ow from 'ow' + +interface ContractSize { + contractSize: number + opcode: number +} + +interface AccessedSlots { + reads: Record + transientReads: Record + transientWrites: Record + writes: Record +} + +export interface ERC7562Call { + accessedSlots: AccessedSlots + contractSize: Record + error?: string + extCodeAccessInfo: string[] + from: string + gas: string + gasUsed: string + input: string + outOfGas: boolean + output?: string + to: string + type: string + usedOpcodes: Record + value?: string + calls?: ERC7562Call[] + keccak?: string[] +} + +const contractSizeSchema = ow.object.exactShape({ + contractSize: ow.number, + opcode: ow.number +}) + +const accessedSlotsSchema = ow.object.exactShape({ + reads: ow.object.valuesOfType(ow.array.ofType(ow.string)), + transientReads: ow.object, + transientWrites: ow.object, + writes: ow.object.valuesOfType(ow.number) +}) + +const erc7562CallSchema = ow.object.exactShape({ + accessedSlots: accessedSlotsSchema, + contractSize: ow.object.valuesOfType(contractSizeSchema), + error: ow.optional.string, + extCodeAccessInfo: ow.array.ofType(ow.string), + from: ow.string, + gas: ow.string, + gasUsed: ow.string, + input: ow.string, + outOfGas: ow.boolean, + output: ow.optional.string, + to: ow.string, + type: ow.string, + usedOpcodes: ow.object.valuesOfType(ow.number), + value: ow.optional.string, + calls: ow.optional.array, + keccak: ow.optional.array.ofType(ow.string) +}) + +/** + * Recursively check the calls ow schema. + */ +export function validateERC7562Call (value: ERC7562Call): void { + ow(value, erc7562CallSchema) + + if (value.calls) { + for (const call of value.calls) { + validateERC7562Call(call) + } + } +} diff --git a/packages/validation-manager/src/ValidationManager.ts b/packages/validation-manager/src/ValidationManager.ts index 344cfe33..e898aaad 100644 --- a/packages/validation-manager/src/ValidationManager.ts +++ b/packages/validation-manager/src/ValidationManager.ts @@ -38,6 +38,7 @@ import EntryPointSimulationsJson from '@account-abstraction/contracts/artifacts/ import { IValidationManager, ValidateUserOpResult, ValidationResult } from './IValidationManager' import { Interface } from 'ethers/lib/utils' import { ERC7562Parser } from './ERC7562Parser' +import { validateERC7562Call } from './ERC7562Call' const debug = Debug('aa.mgr.validate') @@ -409,6 +410,7 @@ export class ValidationManager implements IValidationManager { // todo fix rest of the code to work with the new tracer result instead of adjusting it here convertTracerResult (tracerResult: any, userOp: UserOperation): BundlerTracerResult { + validateERC7562Call(tracerResult) const SENDER_CREATOR = '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c'.toLowerCase() // Before flattening we add top level addresses for calls from EntryPoint and from SENDER_CREATOR tracerResult.calls.forEach((call: { calls: any, to: any, topLevelTargetAddress: any }) => { From 217e5b1f33df67b546f568a1476888cd3550ea1a Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 14 Jan 2025 01:37:27 +0100 Subject: [PATCH 09/29] Remove new parser's dependency on the old tracer --- .../validation-manager/src/ERC7562Call.ts | 4 +- .../validation-manager/src/ERC7562Parser.ts | 90 ++++++++++--------- 2 files changed, 49 insertions(+), 45 deletions(-) diff --git a/packages/validation-manager/src/ERC7562Call.ts b/packages/validation-manager/src/ERC7562Call.ts index 26451446..57e1cab7 100644 --- a/packages/validation-manager/src/ERC7562Call.ts +++ b/packages/validation-manager/src/ERC7562Call.ts @@ -1,11 +1,11 @@ import ow from 'ow' -interface ContractSize { +export interface ContractSize { contractSize: number opcode: number } -interface AccessedSlots { +export interface AccessedSlots { reads: Record transientReads: Record transientWrites: Record diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index da69d251..a562ffd2 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -4,17 +4,11 @@ import { hexZeroPad } from 'ethers/lib/utils' import { OperationBase, StorageMap, ValidationErrors, AltMempoolConfig } from '@account-abstraction/utils' import { ERC7562RuleViolation } from './ERC7562RuleViolation' -import { - AccessInfo, - BundlerTracerResult, - MethodInfo, - StorageAccessInfos, - TopLevelCallInfo -} from './BundlerCollectorTracer' import { ERC7562Rule } from './ERC7562Rule' import { AccountAbstractionEntity } from './AccountAbstractionEntity' import { bannedOpCodes, opcodesOnlyInStakedEntities } from './ERC7562BannedOpcodes' import { ValidationResult } from './IValidationManager' +import { AccessedSlots, ERC7562Call } from './ERC7562Call' export interface ERC7562ValidationResults { storageMap: StorageMap @@ -30,12 +24,12 @@ export class ERC7562Parser { } - private _isCallToEntryPoint (call: MethodInfo): boolean { + private _isCallToEntryPoint (call: ERC7562Call): boolean { return call.to?.toLowerCase() === this.entryPointAddress?.toLowerCase() && call.from?.toLowerCase() !== this.entryPointAddress?.toLowerCase() } - private _isEntityStaked (topLevelCallInfo: TopLevelCallInfo): boolean { + private _isEntityStaked (topLevelCallInfo: ERC7562Call): boolean { throw new Error('Method not implemented.') } @@ -69,7 +63,7 @@ export class ERC7562Parser { */ requireCompliance ( userOp: OperationBase, - tracerResults: BundlerTracerResult, + tracerResults: ERC7562Call, validationResult: ValidationResult ): ERC7562ValidationResults { const results = this.parseResults(userOp, tracerResults) @@ -82,9 +76,19 @@ export class ERC7562Parser { parseResults ( userOp: OperationBase, - tracerResults: BundlerTracerResult + tracerResults: ERC7562Call ): ERC7562ValidationResults { - this.checkSanity(tracerResults) + if (tracerResults.calls == null || tracerResults.calls.length < 1) { + throw new Error('Unexpected traceCall result: no calls from entrypoint.') + } + return this._innerStepRecursive(userOp, tracerResults) + } + + private _innerStepRecursive ( + userOp: OperationBase, + tracerResults: ERC7562Call + ): ERC7562ValidationResults { + this.checkOp054(tracerResults) this.checkOp054ExtCode(userOp, tracerResults) this.checkOp061(tracerResults) @@ -93,32 +97,31 @@ export class ERC7562Parser { this.checkOp031(userOp, tracerResults) this.checkOp041(userOp, tracerResults) this.checkStorage(userOp, tracerResults) + for (const call of tracerResults.calls ?? []) { + this._innerStepRecursive(userOp, call) + } return { contractAddresses: [], ruleViolations: [], storageMap: {} } } - checkSanity (tracerResults: BundlerTracerResult): void { - if (Object.values(tracerResults.callsFromEntryPoint).length < 1) { - throw new Error('Unexpected traceCall result: no calls from entrypoint.') - } - } - /** * OP-052: May call `depositTo(sender)` with any value from either the `sender` or `factory`. * OP-053: May call the fallback function from the `sender` with any value. * OP-054: Any other access to the EntryPoint is forbidden. */ - checkOp054 (tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { - const callStack = tracerResults.calls.filter((call: any) => call.topLevelTargetAddress == null) as MethodInfo[] + checkOp054 (tracerResults: ERC7562Call): ERC7562RuleViolation[] { + const callStack = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress == null) as ERC7562Call[] const callInfoEntryPoint = callStack.filter(call => { const isCallToEntryPoint = this._isCallToEntryPoint(call) + // @ts-ignore const isEntryPointCallAllowedOP052 = call.method === 'depositTo' + // @ts-ignore const isEntryPointCallAllowedOP053 = call.method === '0x' const isEntryPointCallAllowed = isEntryPointCallAllowedOP052 || isEntryPointCallAllowedOP053 return isCallToEntryPoint && !isEntryPointCallAllowed }) - return callInfoEntryPoint.map((it: MethodInfo): ERC7562RuleViolation => { + return callInfoEntryPoint.map((it: ERC7562Call): ERC7562RuleViolation => { return { rule: ERC7562Rule.op054, // TODO: fill in depth, entity @@ -128,6 +131,7 @@ export class ERC7562Parser { opcode: it.type, value: it.value, errorCode: ValidationErrors.OpcodeValidation, + // @ts-ignore description: `illegal call into EntryPoint during validation ${it?.method}` } }) @@ -136,14 +140,14 @@ export class ERC7562Parser { /** * OP-061: CALL with value is forbidden. The only exception is a call to the EntryPoint. */ - checkOp061 (tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { - const callStack = tracerResults.calls.filter((call: any) => call.topLevelTargetAddress == null) as MethodInfo[] + checkOp061 (tracerResults: ERC7562Call): ERC7562RuleViolation[] { + const callStack = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress == null) as ERC7562Call[] const illegalNonZeroValueCall = callStack.filter( call => !this._isCallToEntryPoint(call) && !BigNumber.from(call.value ?? 0).eq(0) ) - return illegalNonZeroValueCall.map((it: MethodInfo): ERC7562RuleViolation => { + return illegalNonZeroValueCall.map((it: ERC7562Call): ERC7562RuleViolation => { return { rule: ERC7562Rule.op061, // TODO: fill in depth, entity @@ -161,10 +165,10 @@ export class ERC7562Parser { /** * OP-020: Revert on "out of gas" is forbidden as it can "leak" the gas limit or the current call stack depth. */ - checkOp020 (tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { - const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) - const entityCallsWithOOG = entityCallsFromEntryPoint.filter((it: TopLevelCallInfo) => it.oog) - return entityCallsWithOOG.map((it: TopLevelCallInfo) => { + checkOp020 (tracerResults: ERC7562Call): ERC7562RuleViolation[] { + const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) + const entityCallsWithOOG = entityCallsFromEntryPoint.filter((it: ERC7562Call) => it.outOfGas) + return entityCallsWithOOG.map((it: ERC7562Call) => { const entityTitle = 'fixme' return { rule: ERC7562Rule.op020, @@ -184,11 +188,11 @@ export class ERC7562Parser { * OP-011: Blocked opcodes * OP-080: `BALANCE` (0x31) and `SELFBALANCE` (0x47) are allowed only from a staked entity, else they are blocked */ - checkOp011 (tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { - const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) + checkOp011 (tracerResults: ERC7562Call): ERC7562RuleViolation[] { + const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) const violations: ERC7562RuleViolation[] = [] for (const topLevelCallInfo of entityCallsFromEntryPoint) { - const opcodes = topLevelCallInfo.opcodes + const opcodes = topLevelCallInfo.usedOpcodes const bannedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { return bannedOpCodes.has(opcode) }) @@ -246,9 +250,9 @@ export class ERC7562Parser { */ checkOp031 ( userOp: OperationBase, - tracerResults: BundlerTracerResult + tracerResults: ERC7562Call ): ERC7562RuleViolation[] { - const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) + const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) const violations: ERC7562RuleViolation[] = [] for (const topLevelCallInfo of entityCallsFromEntryPoint) { if ( @@ -279,13 +283,13 @@ export class ERC7562Parser { return violations } - checkStorage (userOp: OperationBase, tracerResults: BundlerTracerResult): ERC7562RuleViolation[] { - this.checkStorageInternal(userOp, tracerResults.callsFromEntryPoint[0].access) + checkStorage (userOp: OperationBase, tracerResults: ERC7562Call): ERC7562RuleViolation[] { + this.checkStorageInternal(userOp, tracerResults.calls![0]) return [] } - checkStorageInternal (userOp: OperationBase, access: StorageAccessInfos): ERC7562RuleViolation[] { - Object.entries(access).forEach(([address, accessInfo]) => { + checkStorageInternal (userOp: OperationBase, tracerResults: ERC7562Call): ERC7562RuleViolation[] { + Object.entries(tracerResults.accessedSlots).forEach(([address, accessInfo]) => { this.checkStorageInternalInternal(userOp, address, accessInfo) }) return [] @@ -294,7 +298,7 @@ export class ERC7562Parser { checkStorageInternalInternal ( userOp: OperationBase, address: string, - accessInfo: AccessInfo + accessInfo: AccessedSlots ): ERC7562RuleViolation[] { const violations: ERC7562RuleViolation[] = [] const allSlots: string[] = [ @@ -335,10 +339,10 @@ export class ERC7562Parser { checkOp041 ( userOp: OperationBase, - tracerResults: BundlerTracerResult + tracerResults: ERC7562Call ): ERC7562RuleViolation[] { const entityTitle = 'fixme' - const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) + const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) const violations: ERC7562RuleViolation[] = [] for (const topLevelCallInfo of entityCallsFromEntryPoint) { // the only contract we allow to access before its deployment is the "sender" itself, which gets created. @@ -361,16 +365,16 @@ export class ERC7562Parser { checkOp054ExtCode ( userOp: OperationBase, - tracerResults: BundlerTracerResult + tracerResults: ERC7562Call ): ERC7562RuleViolation[] { const entityTitle = 'fixme' - const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter((call: any) => call.topLevelTargetAddress != null) + const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) const violations: ERC7562RuleViolation[] = [] for (const topLevelCallInfo of entityCallsFromEntryPoint) { let illegalEntryPointCodeAccess for (const addr of Object.keys(topLevelCallInfo.extCodeAccessInfo)) { if (addr.toLowerCase() === this.entryPointAddress.toLowerCase()) { - illegalEntryPointCodeAccess = topLevelCallInfo.extCodeAccessInfo[addr] + illegalEntryPointCodeAccess = topLevelCallInfo.extCodeAccessInfo // @ts-ignore violations.push({ description: `${entityTitle} accesses EntryPoint contract address ${this.entryPointAddress} with opcode ${illegalEntryPointCodeAccess}` From f5804bd84ea19de29ec3b83caca64c7246c87289 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 14 Jan 2025 14:51:47 +0100 Subject: [PATCH 10/29] Remove conversion function and make new tracer recursive as well --- .../bundler/src/modules/ExecutionManager.ts | 2 +- packages/bundler/src/modules/initServer.ts | 2 +- packages/bundler/test/BundlerManager.test.ts | 4 +- packages/bundler/test/BundlerServer.test.ts | 2 +- .../bundler/test/DebugMethodHandler.test.ts | 2 +- .../bundler/test/UserOpMethodHandler.test.ts | 2 +- packages/bundler/test/ValidateManager.test.ts | 2 +- packages/utils/src/index.ts | 1 - .../validation-manager/src/ERC7562Parser.ts | 298 +++++++++--------- ...62RuleViolation.ts => ERC7562Violation.ts} | 8 +- .../src/ValidationManager.ts | 120 +------ .../src/altmempool/AltMempoolConfig.ts | 9 +- .../src/{ => enum}/ERC7562Rule.ts | 2 +- packages/validation-manager/src/index.ts | 5 +- 14 files changed, 169 insertions(+), 290 deletions(-) rename packages/validation-manager/src/{ERC7562RuleViolation.ts => ERC7562Violation.ts} (64%) rename packages/{utils => validation-manager}/src/altmempool/AltMempoolConfig.ts (86%) rename packages/validation-manager/src/{ => enum}/ERC7562Rule.ts (95%) diff --git a/packages/bundler/src/modules/ExecutionManager.ts b/packages/bundler/src/modules/ExecutionManager.ts index 49d78337..244c32e1 100644 --- a/packages/bundler/src/modules/ExecutionManager.ts +++ b/packages/bundler/src/modules/ExecutionManager.ts @@ -149,7 +149,7 @@ export class ExecutionManager { async _setConfiguration (configOverrides: Partial): Promise { const { configuration, entryPoint, unsafe } = this.validationManager._getDebugConfiguration() const pvgc = new PreVerificationGasCalculator(Object.assign({}, configuration, configOverrides)) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) this.validationManager = new ValidationManager( entryPoint, unsafe, diff --git a/packages/bundler/src/modules/initServer.ts b/packages/bundler/src/modules/initServer.ts index 6d8b0047..9d953afa 100644 --- a/packages/bundler/src/modules/initServer.ts +++ b/packages/bundler/src/modules/initServer.ts @@ -36,7 +36,7 @@ export function initServer (config: BundlerConfig, signer: Signer): [ExecutionMa const eventsManager = new EventsManager(entryPoint, mempoolManager, reputationManager) const mergedPvgcConfig = Object.assign({}, ChainConfigs[config.chainId] ?? {}, config) const preVerificationGasCalculator = new PreVerificationGasCalculator(mergedPvgcConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) let validationManager: IValidationManager let bundleManager: IBundleManager if (!config.rip7560) { diff --git a/packages/bundler/test/BundlerManager.test.ts b/packages/bundler/test/BundlerManager.test.ts index fbd2c4dc..be475ee8 100644 --- a/packages/bundler/test/BundlerManager.test.ts +++ b/packages/bundler/test/BundlerManager.test.ts @@ -64,7 +64,7 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) bm = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc) @@ -121,7 +121,7 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) const validMgr = new ValidationManager(_entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(_entryPoint, mempoolMgr, repMgr) bundleMgr = new BundleManager(_entryPoint, _entryPoint.provider as JsonRpcProvider, _entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/BundlerServer.test.ts b/packages/bundler/test/BundlerServer.test.ts index 846fbabc..ba9beefc 100644 --- a/packages/bundler/test/BundlerServer.test.ts +++ b/packages/bundler/test/BundlerServer.test.ts @@ -60,7 +60,7 @@ describe('BundleServer', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/DebugMethodHandler.test.ts b/packages/bundler/test/DebugMethodHandler.test.ts index d07fbaa0..c12a88c2 100644 --- a/packages/bundler/test/DebugMethodHandler.test.ts +++ b/packages/bundler/test/DebugMethodHandler.test.ts @@ -70,7 +70,7 @@ describe('#DebugMethodHandler', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const eventsManager = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, eventsManager, mempoolMgr, validMgr, repMgr, diff --git a/packages/bundler/test/UserOpMethodHandler.test.ts b/packages/bundler/test/UserOpMethodHandler.test.ts index 4200b4b2..616c8069 100644 --- a/packages/bundler/test/UserOpMethodHandler.test.ts +++ b/packages/bundler/test/UserOpMethodHandler.test.ts @@ -88,7 +88,7 @@ describe('UserOpMethodHandler', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/ValidateManager.test.ts b/packages/bundler/test/ValidateManager.test.ts index 46843879..a0307126 100644 --- a/packages/bundler/test/ValidateManager.test.ts +++ b/packages/bundler/test/ValidateManager.test.ts @@ -155,7 +155,7 @@ describe('#ValidationManager', () => { const unsafe = !await supportsDebugTraceCall(provider, false) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) vm = new ValidationManager(entryPoint, unsafe, preVerificationGasCalculator, erc7562Parser) if (!await supportsDebugTraceCall(ethers.provider, false)) { diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 98787b9b..a8c60d48 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -2,7 +2,6 @@ export { EIP7702Authorization, getEip7702AuthorizationSigner } from './interface export { UserOperation } from './interfaces/UserOperation' export { OperationBase } from './interfaces/OperationBase' export { OperationRIP7560 } from './interfaces/OperationRIP7560' -export { AltMempoolConfig } from './altmempool/AltMempoolConfig' export * from './Version' export * from './decodeRevertReason' export * from './ERC4337Utils' diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index a562ffd2..f54e8ef5 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -1,25 +1,29 @@ import { BigNumber } from 'ethers' import { hexZeroPad } from 'ethers/lib/utils' -import { OperationBase, StorageMap, ValidationErrors, AltMempoolConfig } from '@account-abstraction/utils' +import { OperationBase, StorageMap, ValidationErrors } from '@account-abstraction/utils' -import { ERC7562RuleViolation } from './ERC7562RuleViolation' -import { ERC7562Rule } from './ERC7562Rule' +import { ERC7562Violation, toError } from './ERC7562Violation' +import { ERC7562Rule } from './enum/ERC7562Rule' import { AccountAbstractionEntity } from './AccountAbstractionEntity' import { bannedOpCodes, opcodesOnlyInStakedEntities } from './ERC7562BannedOpcodes' import { ValidationResult } from './IValidationManager' import { AccessedSlots, ERC7562Call } from './ERC7562Call' +import { AltMempoolConfig } from './altmempool/AltMempoolConfig' export interface ERC7562ValidationResults { storageMap: StorageMap - ruleViolations: ERC7562RuleViolation[] + ruleViolations: ERC7562Violation[] contractAddresses: string[] } export class ERC7562Parser { + private violations: ERC7562Violation[] = [] + constructor ( readonly mempoolConfig: AltMempoolConfig, - readonly entryPointAddress: string + readonly entryPointAddress: string, + readonly bailOnViolation: boolean ) { } @@ -58,6 +62,13 @@ export class ERC7562Parser { return false } + private _violationDetected (violation: ERC7562Violation) { + this.violations.push(violation) + if (this.bailOnViolation) { + throw toError(violation) + } + } + /** * Validates the UserOperation and throws an exception in case current mempool configuration rules were violated. */ @@ -78,19 +89,22 @@ export class ERC7562Parser { userOp: OperationBase, tracerResults: ERC7562Call ): ERC7562ValidationResults { + this.violations = [] + if (tracerResults.calls == null || tracerResults.calls.length < 1) { throw new Error('Unexpected traceCall result: no calls from entrypoint.') } - return this._innerStepRecursive(userOp, tracerResults) + return this._innerStepRecursive(userOp, tracerResults, 0) } private _innerStepRecursive ( userOp: OperationBase, - tracerResults: ERC7562Call + tracerResults: ERC7562Call, + recursionDepth: number ): ERC7562ValidationResults { this.checkOp054(tracerResults) - this.checkOp054ExtCode(userOp, tracerResults) + this.checkOp054ExtCode(tracerResults) this.checkOp061(tracerResults) this.checkOp011(tracerResults) this.checkOp020(tracerResults) @@ -98,7 +112,7 @@ export class ERC7562Parser { this.checkOp041(userOp, tracerResults) this.checkStorage(userOp, tracerResults) for (const call of tracerResults.calls ?? []) { - this._innerStepRecursive(userOp, call) + this._innerStepRecursive(userOp, call, recursionDepth + 1) } return { contractAddresses: [], ruleViolations: [], storageMap: {} @@ -110,44 +124,40 @@ export class ERC7562Parser { * OP-053: May call the fallback function from the `sender` with any value. * OP-054: Any other access to the EntryPoint is forbidden. */ - checkOp054 (tracerResults: ERC7562Call): ERC7562RuleViolation[] { - const callStack = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress == null) as ERC7562Call[] - const callInfoEntryPoint = callStack.filter(call => { - const isCallToEntryPoint = this._isCallToEntryPoint(call) - // @ts-ignore - const isEntryPointCallAllowedOP052 = call.method === 'depositTo' + checkOp054 (erc7562Call: ERC7562Call): void { + const isCallToEntryPoint = this._isCallToEntryPoint(erc7562Call) + // @ts-ignore + const isEntryPointCallAllowedOP052 = call.method === 'depositTo' + // @ts-ignore + const isEntryPointCallAllowedOP053 = call.method === '0x' + const isEntryPointCallAllowed = isEntryPointCallAllowedOP052 || isEntryPointCallAllowedOP053 + const isRuleViolated = isCallToEntryPoint && !isEntryPointCallAllowed + + this._violationDetected({ + rule: ERC7562Rule.op054, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: erc7562Call.from, + opcode: erc7562Call.type, + value: erc7562Call.value, + errorCode: ValidationErrors.OpcodeValidation, // @ts-ignore - const isEntryPointCallAllowedOP053 = call.method === '0x' - const isEntryPointCallAllowed = isEntryPointCallAllowedOP052 || isEntryPointCallAllowedOP053 - return isCallToEntryPoint && !isEntryPointCallAllowed - }) - return callInfoEntryPoint.map((it: ERC7562Call): ERC7562RuleViolation => { - return { - rule: ERC7562Rule.op054, - // TODO: fill in depth, entity - depth: -1, - entity: AccountAbstractionEntity.fixme, - address: it.from, - opcode: it.type, - value: it.value, - errorCode: ValidationErrors.OpcodeValidation, - // @ts-ignore - description: `illegal call into EntryPoint during validation ${it?.method}` - } + description: `illegal call into EntryPoint during validation ${it?.method}` }) } /** * OP-061: CALL with value is forbidden. The only exception is a call to the EntryPoint. */ - checkOp061 (tracerResults: ERC7562Call): ERC7562RuleViolation[] { + checkOp061 (tracerResults: ERC7562Call): ERC7562Violation[] { const callStack = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress == null) as ERC7562Call[] const illegalNonZeroValueCall = callStack.filter( call => !this._isCallToEntryPoint(call) && !BigNumber.from(call.value ?? 0).eq(0) ) - return illegalNonZeroValueCall.map((it: ERC7562Call): ERC7562RuleViolation => { + return illegalNonZeroValueCall.map((it: ERC7562Call): ERC7562Violation => { return { rule: ERC7562Rule.op061, // TODO: fill in depth, entity @@ -165,7 +175,7 @@ export class ERC7562Parser { /** * OP-020: Revert on "out of gas" is forbidden as it can "leak" the gas limit or the current call stack depth. */ - checkOp020 (tracerResults: ERC7562Call): ERC7562RuleViolation[] { + checkOp020 (tracerResults: ERC7562Call): ERC7562Violation[] { const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) const entityCallsWithOOG = entityCallsFromEntryPoint.filter((it: ERC7562Call) => it.outOfGas) return entityCallsWithOOG.map((it: ERC7562Call) => { @@ -188,61 +198,53 @@ export class ERC7562Parser { * OP-011: Blocked opcodes * OP-080: `BALANCE` (0x31) and `SELFBALANCE` (0x47) are allowed only from a staked entity, else they are blocked */ - checkOp011 (tracerResults: ERC7562Call): ERC7562RuleViolation[] { - const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) - const violations: ERC7562RuleViolation[] = [] - for (const topLevelCallInfo of entityCallsFromEntryPoint) { - const opcodes = topLevelCallInfo.usedOpcodes - const bannedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { - return bannedOpCodes.has(opcode) - }) - // TODO: TBD: Creating an object for each violation may be wasteful but makes it easier to choose a right mempool. - const bannedOpcodesViolations: ERC7562RuleViolation[] = - bannedOpCodeUsed - .map( - (opcode: string): ERC7562RuleViolation => { - const entityTitle = 'fixme' - return { - rule: ERC7562Rule.op011, - // TODO: fill in depth, entity - depth: -1, - entity: AccountAbstractionEntity.fixme, - address: topLevelCallInfo.from ?? 'n/a', - opcode, - value: '0', - errorCode: ValidationErrors.OpcodeValidation, - description: `${entityTitle} uses banned opcode: ${opcode}` - } - } - ) - violations.push(...bannedOpcodesViolations) + checkOp011 (tracerResults: ERC7562Call): void { + const opcodes = tracerResults.usedOpcodes + const bannedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { + return bannedOpCodes.has(opcode) + }) + bannedOpCodeUsed + .forEach( + (opcode: string): void => { + const entityTitle = 'fixme' + this._violationDetected({ + rule: ERC7562Rule.op011, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: tracerResults.from, + opcode, + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `${entityTitle} uses banned opcode: ${opcode}` + }) + } + ) + } - // TODO: Deduplicate code in an elegant way - // TODO: Extract OP-080 into a separate function - const onlyStakedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { - return opcodesOnlyInStakedEntities.has(opcode) && !this._isEntityStaked(topLevelCallInfo) - }) - const onlyStakedOpcodesViolations: ERC7562RuleViolation[] = - onlyStakedOpCodeUsed - .map( - (opcode: string): ERC7562RuleViolation => { - const entityTitle = 'fixme' - return { - rule: ERC7562Rule.op011, - // TODO: fill in depth, entity - depth: -1, - entity: AccountAbstractionEntity.fixme, - address: topLevelCallInfo.from ?? 'n/a', - opcode, - value: '0', - errorCode: ValidationErrors.OpcodeValidation, - description: `unstaked ${entityTitle} uses banned opcode: ${opcode}` - } - } - ) - violations.push(...onlyStakedOpcodesViolations) - } - return violations + checkOp080 (tracerResults: ERC7562Call): void { + const opcodes = tracerResults.usedOpcodes + const isEntityStaked = this._isEntityStaked(tracerResults) + const onlyStakedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { + return opcodesOnlyInStakedEntities.has(opcode) && !isEntityStaked + }) + onlyStakedOpCodeUsed + .forEach( + (opcode: string): void => { + const entityTitle = 'fixme' + this._violationDetected({ + rule: ERC7562Rule.op011, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: tracerResults.from ?? 'n/a', + opcode, + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `unstaked ${entityTitle} uses banned opcode: ${opcode}` + }) + } + ) } /** @@ -251,56 +253,49 @@ export class ERC7562Parser { checkOp031 ( userOp: OperationBase, tracerResults: ERC7562Call - ): ERC7562RuleViolation[] { - const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) - const violations: ERC7562RuleViolation[] = [] - for (const topLevelCallInfo of entityCallsFromEntryPoint) { - if ( - topLevelCallInfo.type !== 'CREATE' && - topLevelCallInfo.type !== 'CREATE2' - ) { - continue - } - const entityTitle = 'fixme' as string - const isFactoryStaked = false - const isAllowedCreateByOP032 = entityTitle === 'account' && isFactoryStaked && topLevelCallInfo.from === userOp.sender.toLowerCase() - const isAllowedCreateByEREP060 = entityTitle === 'factory' && topLevelCallInfo.from === userOp.factory && isFactoryStaked - const isAllowedCreateSenderByFactory = entityTitle === 'factory' && topLevelCallInfo.to === userOp.sender.toLowerCase() - if (!(isAllowedCreateByOP032 || isAllowedCreateByEREP060 || isAllowedCreateSenderByFactory)) { - violations.push({ - rule: ERC7562Rule.op011, - // TODO: fill in depth, entity - depth: -1, - entity: AccountAbstractionEntity.fixme, - address: topLevelCallInfo.from ?? 'n/a', - opcode: 'CREATE2', - value: '0', - errorCode: ValidationErrors.OpcodeValidation, - description: `${entityTitle} uses banned opcode: CREATE2` - }) - } + ): void { + if ( + tracerResults.type !== 'CREATE' && + tracerResults.type !== 'CREATE2' + ) { + return + } + const entityTitle = 'fixme' as string + const isFactoryStaked = false + const isAllowedCreateByOP032 = entityTitle === 'account' && isFactoryStaked && tracerResults.from === userOp.sender.toLowerCase() + const isAllowedCreateByEREP060 = entityTitle === 'factory' && tracerResults.from === userOp.factory && isFactoryStaked + const isAllowedCreateSenderByFactory = entityTitle === 'factory' && tracerResults.to === userOp.sender.toLowerCase() + if (!(isAllowedCreateByOP032 || isAllowedCreateByEREP060 || isAllowedCreateSenderByFactory)) { + this._violationDetected({ + rule: ERC7562Rule.op011, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: tracerResults.from ?? 'n/a', + opcode: 'CREATE2', + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `${entityTitle} uses banned opcode: CREATE2` + }) } - return violations } - checkStorage (userOp: OperationBase, tracerResults: ERC7562Call): ERC7562RuleViolation[] { + checkStorage (userOp: OperationBase, tracerResults: ERC7562Call): void { this.checkStorageInternal(userOp, tracerResults.calls![0]) - return [] } - checkStorageInternal (userOp: OperationBase, tracerResults: ERC7562Call): ERC7562RuleViolation[] { + checkStorageInternal (userOp: OperationBase, tracerResults: ERC7562Call): void { Object.entries(tracerResults.accessedSlots).forEach(([address, accessInfo]) => { this.checkStorageInternalInternal(userOp, address, accessInfo) }) - return [] } checkStorageInternalInternal ( userOp: OperationBase, address: string, accessInfo: AccessedSlots - ): ERC7562RuleViolation[] { - const violations: ERC7562RuleViolation[] = [] + ): ERC7562Violation[] { + const violations: ERC7562Violation[] = [] const allSlots: string[] = [ ...Object.keys(accessInfo.writes), ...Object.keys(accessInfo.reads), @@ -340,48 +335,37 @@ export class ERC7562Parser { checkOp041 ( userOp: OperationBase, tracerResults: ERC7562Call - ): ERC7562RuleViolation[] { + ): void { const entityTitle = 'fixme' - const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) - const violations: ERC7562RuleViolation[] = [] - for (const topLevelCallInfo of entityCallsFromEntryPoint) { - // the only contract we allow to access before its deployment is the "sender" itself, which gets created. - let illegalZeroCodeAccess: any - for (const addr of Object.keys(topLevelCallInfo.contractSize)) { - // [OP-042] - if (addr.toLowerCase() !== userOp.sender.toLowerCase() && addr.toLowerCase() !== this.entryPointAddress.toLowerCase() && topLevelCallInfo.contractSize[addr].contractSize <= 2) { - illegalZeroCodeAccess = topLevelCallInfo.contractSize[addr] - illegalZeroCodeAccess.address = addr - // @ts-ignore - violations.push({ - errorCode: ValidationErrors.OpcodeValidation, - description: `${entityTitle} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}` - }) - } + // the only contract we allow to access before its deployment is the "sender" itself, which gets created. + let illegalZeroCodeAccess: any + for (const addr of Object.keys(tracerResults.contractSize)) { + // [OP-042] + if (addr.toLowerCase() !== userOp.sender.toLowerCase() && addr.toLowerCase() !== this.entryPointAddress.toLowerCase() && tracerResults.contractSize[addr].contractSize <= 2) { + illegalZeroCodeAccess = tracerResults.contractSize[addr] + illegalZeroCodeAccess.address = addr + // @ts-ignore + this._violationDetected({ + errorCode: ValidationErrors.OpcodeValidation, + description: `${entityTitle} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}` + }) } } - return violations } checkOp054ExtCode ( - userOp: OperationBase, tracerResults: ERC7562Call - ): ERC7562RuleViolation[] { + ): void { const entityTitle = 'fixme' - const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) - const violations: ERC7562RuleViolation[] = [] - for (const topLevelCallInfo of entityCallsFromEntryPoint) { - let illegalEntryPointCodeAccess - for (const addr of Object.keys(topLevelCallInfo.extCodeAccessInfo)) { - if (addr.toLowerCase() === this.entryPointAddress.toLowerCase()) { - illegalEntryPointCodeAccess = topLevelCallInfo.extCodeAccessInfo - // @ts-ignore - violations.push({ - description: `${entityTitle} accesses EntryPoint contract address ${this.entryPointAddress} with opcode ${illegalEntryPointCodeAccess}` - }) - } + let illegalEntryPointCodeAccess + for (const addr of Object.keys(tracerResults.extCodeAccessInfo)) { + if (addr.toLowerCase() === this.entryPointAddress.toLowerCase()) { + illegalEntryPointCodeAccess = tracerResults.extCodeAccessInfo + // @ts-ignore + violations.push({ + description: `${entityTitle} accesses EntryPoint contract address ${this.entryPointAddress} with opcode ${illegalEntryPointCodeAccess}` + }) } } - return violations } } diff --git a/packages/validation-manager/src/ERC7562RuleViolation.ts b/packages/validation-manager/src/ERC7562Violation.ts similarity index 64% rename from packages/validation-manager/src/ERC7562RuleViolation.ts rename to packages/validation-manager/src/ERC7562Violation.ts index 3fba2bf4..3d1694e2 100644 --- a/packages/validation-manager/src/ERC7562RuleViolation.ts +++ b/packages/validation-manager/src/ERC7562Violation.ts @@ -1,9 +1,9 @@ import { ValidationErrors } from '@account-abstraction/utils' -import { ERC7562Rule } from './ERC7562Rule' +import { ERC7562Rule } from './enum/ERC7562Rule' import { AccountAbstractionEntity } from './AccountAbstractionEntity' -export interface ERC7562RuleViolation { +export interface ERC7562Violation { rule: ERC7562Rule depth: number entity: AccountAbstractionEntity @@ -15,3 +15,7 @@ export interface ERC7562RuleViolation { value?: string slot?: string } + +export function toError(violation: ERC7562Violation): Error { + return new Error('TODO: VIOLATED!') +} diff --git a/packages/validation-manager/src/ValidationManager.ts b/packages/validation-manager/src/ValidationManager.ts index e898aaad..868101f0 100644 --- a/packages/validation-manager/src/ValidationManager.ts +++ b/packages/validation-manager/src/ValidationManager.ts @@ -31,14 +31,13 @@ import { runContractScript, getAuthorizationList, SenderCreator__factory, IEntryPoint__factory, IPaymaster__factory } from '@account-abstraction/utils' -import { bundlerCollectorTracer, BundlerTracerResult, ExitInfo } from './BundlerCollectorTracer' import { debug_traceCall } from './GethTracer' import EntryPointSimulationsJson from '@account-abstraction/contracts/artifacts/EntryPointSimulations.json' import { IValidationManager, ValidateUserOpResult, ValidationResult } from './IValidationManager' import { Interface } from 'ethers/lib/utils' import { ERC7562Parser } from './ERC7562Parser' -import { validateERC7562Call } from './ERC7562Call' +import { ERC7562Call, validateERC7562Call } from './ERC7562Call' const debug = Debug('aa.mgr.validate') @@ -143,7 +142,7 @@ export class ValidationManager implements IValidationManager { async _geth_traceCall_SimulateValidation ( operation: OperationBase, stateOverride: { [address: string]: { code: string } } - ): Promise<[ValidationResult, BundlerTracerResult]> { + ): Promise<[ValidationResult, ERC7562Call]> { const userOp = operation as UserOperation const provider = this.entryPoint.provider as JsonRpcProvider const simulateCall = entryPointSimulations.encodeFunctionData('simulateValidation', [packUserOp(userOp)]) @@ -158,7 +157,7 @@ export class ValidationManager implements IValidationManager { } let tracer if (!this.usingErc7562NativeTracer()) { - tracer = bundlerCollectorTracer + throw new Error('JavaScript BundlerCollectorTracer is no longer supported') } const tracerResult = await debug_traceCall(provider, { from: AddressZero, @@ -176,7 +175,7 @@ export class ValidationManager implements IValidationManager { if (!this.usingErc7562NativeTracer()) { // Using preState tracer + JS tracer const lastResult = tracerResult.calls.slice(-1)[0] - data = (lastResult as ExitInfo).data + data = (lastResult as any).data if (lastResult.type === 'REVERT') { throw new RpcError(decodeRevertReason(data, false) as string, ValidationErrors.SimulateValidation) } @@ -258,15 +257,10 @@ export class ValidationManager implements IValidationManager { const stateOverrideForEip7702 = await this.getAuthorizationsStateOverride(authorizationList) let storageMap: StorageMap = {} if (!this.unsafe) { - let tracerResult: BundlerTracerResult + let tracerResult: ERC7562Call [res, tracerResult] = await this._geth_traceCall_SimulateValidation(userOp, stateOverrideForEip7702).catch(e => { throw e }) - // console.log('validation res', res) - // todo fix - if (this.usingErc7562NativeTracer()) { - this.convertTracerResult(tracerResult, userOp) - } // console.log('tracer res') // console.dir(tracerResult, { depth: null }) let contractAddresses: string[] @@ -408,110 +402,6 @@ export class ValidationManager implements IValidationManager { return await this.entryPoint.getUserOpHash(packUserOp(userOp as UserOperation)) } - // todo fix rest of the code to work with the new tracer result instead of adjusting it here - convertTracerResult (tracerResult: any, userOp: UserOperation): BundlerTracerResult { - validateERC7562Call(tracerResult) - const SENDER_CREATOR = '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c'.toLowerCase() - // Before flattening we add top level addresses for calls from EntryPoint and from SENDER_CREATOR - tracerResult.calls.forEach((call: { calls: any, to: any, topLevelTargetAddress: any }) => { - call.topLevelTargetAddress = call.to - if (call.to.toLowerCase() === SENDER_CREATOR && call.calls != null) { - call.calls.forEach((subcall: any) => { - subcall.topLevelTargetAddress = subcall.to - }) - } - }) - tracerResult.calls = this.flattenCalls(tracerResult.calls) - tracerResult.calls.forEach((call: { - topLevelTargetAddress: any - method: any - input: any - to: any - from: any - opcodes: any - usedOpcodes: any - access: any - accessedSlots: any - extCodeAccessInfo: any - outOfGas: any - oog: any - }) => { - call.opcodes = {} - if (call.usedOpcodes != null) { - Object.keys(call.usedOpcodes).forEach((opcode: string) => { - call.opcodes[this.getOpcodeName(parseInt(opcode))] = call.usedOpcodes[opcode] - }) - } - - if (call.access == null) { - call.access = {} - } - if (call.accessedSlots != null) { - call.access[call.to] = { - reads: call.accessedSlots.reads ?? {}, - writes: call.accessedSlots.writes ?? {}, - transientReads: call.accessedSlots.transientReads ?? {}, - transientWrites: call.accessedSlots.transientWrites ?? {} - } - Object.keys(call.access[call.to].reads).forEach((slot) => { - if (call.access[call.to].reads[slot] != null && call.access[call.to].reads[slot].length > 0) { - call.access[call.to].reads[slot] = call.access[call.to].reads[slot][0] - } - }) - } - if (call.extCodeAccessInfo == null) { - call.extCodeAccessInfo = {} - } - const newExtCode: any = {} - if (Array.isArray(call.extCodeAccessInfo)) { - call.extCodeAccessInfo.forEach((addr: any) => { - newExtCode[addr] = 1 - }) - } - call.extCodeAccessInfo = newExtCode - call.oog = call.outOfGas - - // Adding method name - if (call.topLevelTargetAddress == null && call.to.toLowerCase() === this.entryPoint.address.toLowerCase()) { - if (call.input.length <= 2) { - call.method = '0x' - } else { - const mergedAbi = Object.values([ - ...SenderCreator__factory.abi, - ...IEntryPoint__factory.abi, - ...IPaymaster__factory.abi - ].reduce((set, entry) => { - const key = `${entry.name}(${entry.inputs.map(i => i.type).join(',')})` - // console.log('key=', key, keccak256(Buffer.from(key)).slice(0,10)) - return { - ...set, - [key]: entry - } - }, {})) as any - const AbiInterfaces = new Interface(mergedAbi) - - function callCatch (x: () => T, def: T1): T | T1 { - try { - return x() - } catch { - return def - } - } - - const methodSig = call.input.slice(0, 10) - const method = callCatch(() => AbiInterfaces.getFunction(methodSig), methodSig) - call.method = method.name - } - } - }) - // TODO: This is a hardcoded address of SenderCreator immutable member in EntryPoint. Any change in EntryPoint's code - // requires a change of this address. - // TODO check why the filter fails test_ban_user_op_access_other_ops_sender_in_bundle - tracerResult.callsFromEntryPoint = tracerResult.calls // .filter((call: { from: string }) => call.from.toLowerCase() === this.entryPoint.address.toLowerCase() || call.from.toLowerCase() === SENDER_CREATOR) - - return tracerResult - } - flattenCalls (calls: any[]): any[] { return calls.reduce((acc: any, call: any) => { acc.push(call) // Add the current call to the accumulator diff --git a/packages/utils/src/altmempool/AltMempoolConfig.ts b/packages/validation-manager/src/altmempool/AltMempoolConfig.ts similarity index 86% rename from packages/utils/src/altmempool/AltMempoolConfig.ts rename to packages/validation-manager/src/altmempool/AltMempoolConfig.ts index 50ad7047..1f422ea6 100644 --- a/packages/utils/src/altmempool/AltMempoolConfig.ts +++ b/packages/validation-manager/src/altmempool/AltMempoolConfig.ts @@ -1,3 +1,5 @@ +import { ERC7562Rule } from '../enum/ERC7562Rule' + type Role = 'sender' | 'paymaster' | 'factory' type EnterOpcode = 'CALL' | 'DELEGATECALL' | 'CALLCODE' | 'STATICCALL' | 'CREATE' | 'CREATE2' @@ -22,16 +24,13 @@ export interface BaseAltMempoolRule { exceptions?: RuleException[] } -// TODO: define all current rules -type RuleERC7562 = 'erep010' | 'erep020' - export interface AltMempoolConfig { - [mempoolId: number]: { [rule in RuleERC7562]?: BaseAltMempoolRule } + [mempoolId: number]: { [rule in ERC7562Rule]?: BaseAltMempoolRule } } const config: AltMempoolConfig = { 1: { - erep010: { + [ERC7562Rule.erep010]: { enabled: true, exceptions: [ 'sender', diff --git a/packages/validation-manager/src/ERC7562Rule.ts b/packages/validation-manager/src/enum/ERC7562Rule.ts similarity index 95% rename from packages/validation-manager/src/ERC7562Rule.ts rename to packages/validation-manager/src/enum/ERC7562Rule.ts index e453fe2d..d36a4301 100644 --- a/packages/validation-manager/src/ERC7562Rule.ts +++ b/packages/validation-manager/src/enum/ERC7562Rule.ts @@ -1,4 +1,4 @@ -export const enum ERC7562Rule { +export enum ERC7562Rule { op011 = 'OP-011', op012 = 'OP-012', op013 = 'OP-013', diff --git a/packages/validation-manager/src/index.ts b/packages/validation-manager/src/index.ts index b01db014..bada1d90 100644 --- a/packages/validation-manager/src/index.ts +++ b/packages/validation-manager/src/index.ts @@ -17,6 +17,9 @@ import { ERC7562Parser } from './ERC7562Parser' export * from './ValidationManager' export * from './ValidationManagerRIP7560' export * from './IValidationManager' +export * from './altmempool/AltMempoolConfig' +export * from './enum/ERC7562Rule' +export * from './enum/EVMOpcodes' export async function supportsNativeTracer (provider: JsonRpcProvider, nativeTracer = bundlerJSTracerName): Promise { try { @@ -83,7 +86,7 @@ export async function checkRulesViolations ( throw new Error('This provider does not support stack tracing') } const entryPoint = IEntryPoint__factory.connect(entryPointAddress, provider) - const erc7562Parser = new ERC7562Parser({}, entryPointAddress) + const erc7562Parser = new ERC7562Parser({}, entryPointAddress, true) const validationManager = new ValidationManager( entryPoint, false, From c496ee23b5118d216948c474c3924a838b558942 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 14 Jan 2025 16:20:17 +0100 Subject: [PATCH 11/29] Clean up parser code --- .../src/ERC7562BannedOpcodes.ts | 25 ++- .../validation-manager/src/ERC7562Call.ts | 8 +- .../validation-manager/src/ERC7562Parser.ts | 164 ++++++++++-------- 3 files changed, 116 insertions(+), 81 deletions(-) diff --git a/packages/validation-manager/src/ERC7562BannedOpcodes.ts b/packages/validation-manager/src/ERC7562BannedOpcodes.ts index 1b96e4e5..66ef6e54 100644 --- a/packages/validation-manager/src/ERC7562BannedOpcodes.ts +++ b/packages/validation-manager/src/ERC7562BannedOpcodes.ts @@ -1,3 +1,24 @@ -export const bannedOpCodes = new Set(['GASPRICE', 'GASLIMIT', 'DIFFICULTY', 'TIMESTAMP', 'BASEFEE', 'BLOCKHASH', 'NUMBER', 'ORIGIN', 'GAS', 'COINBASE', 'SELFDESTRUCT', 'RANDOM', 'PREVRANDAO', 'INVALID']) +export const bannedOpCodes = new Set( + [ + 'GASPRICE', + 'GASLIMIT', + 'DIFFICULTY', + 'TIMESTAMP', + 'BASEFEE', + 'BLOCKHASH', + 'NUMBER', + 'ORIGIN', + 'COINBASE', + 'SELFDESTRUCT', + 'RANDOM', + 'PREVRANDAO', + 'INVALID' + ] +) // opcodes allowed in staked entities [OP-080] -export const opcodesOnlyInStakedEntities = new Set(['BALANCE', 'SELFBALANCE']) +export const opcodesOnlyInStakedEntities = new Set( + [ + 'BALANCE', + 'SELFBALANCE' + ] +) diff --git a/packages/validation-manager/src/ERC7562Call.ts b/packages/validation-manager/src/ERC7562Call.ts index 57e1cab7..c37a46ce 100644 --- a/packages/validation-manager/src/ERC7562Call.ts +++ b/packages/validation-manager/src/ERC7562Call.ts @@ -6,10 +6,10 @@ export interface ContractSize { } export interface AccessedSlots { - reads: Record - transientReads: Record - transientWrites: Record - writes: Record + reads?: Record + transientReads?: Record + transientWrites?: Record + writes?: Record } export interface ERC7562Call { diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index f54e8ef5..a6f35efc 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -1,7 +1,14 @@ -import { BigNumber } from 'ethers' -import { hexZeroPad } from 'ethers/lib/utils' +import { BigNumber, ethers } from 'ethers' +import { hexZeroPad, Interface, FunctionFragment } from 'ethers/lib/utils' -import { OperationBase, StorageMap, ValidationErrors } from '@account-abstraction/utils' +import { + AddressZero, + IEntryPoint__factory, IPaymaster__factory, + OperationBase, + SenderCreator__factory, + StorageMap, + ValidationErrors +} from '@account-abstraction/utils' import { ERC7562Violation, toError } from './ERC7562Violation' import { ERC7562Rule } from './enum/ERC7562Rule' @@ -10,6 +17,7 @@ import { bannedOpCodes, opcodesOnlyInStakedEntities } from './ERC7562BannedOpcod import { ValidationResult } from './IValidationManager' import { AccessedSlots, ERC7562Call } from './ERC7562Call' import { AltMempoolConfig } from './altmempool/AltMempoolConfig' +import { getOpcodeName } from './enum/EVMOpcodes' export interface ERC7562ValidationResults { storageMap: StorageMap @@ -30,7 +38,9 @@ export class ERC7562Parser { private _isCallToEntryPoint (call: ERC7562Call): boolean { return call.to?.toLowerCase() === this.entryPointAddress?.toLowerCase() && - call.from?.toLowerCase() !== this.entryPointAddress?.toLowerCase() + call.from?.toLowerCase() !== this.entryPointAddress?.toLowerCase() && + // skipping the top-level call from address(0) to 'simulateValidations()' + call.from?.toLowerCase() !== AddressZero } private _isEntityStaked (topLevelCallInfo: ERC7562Call): boolean { @@ -62,6 +72,21 @@ export class ERC7562Parser { return false } + private _tryDetectKnownMethod (call: ERC7562Call): string { + const mergedAbi = Object.values([ + ...SenderCreator__factory.abi, + ...IEntryPoint__factory.abi, + ...IPaymaster__factory.abi + ]) + const AbiInterfaces = new Interface(mergedAbi) + const methodSig = call.input.slice(0, 10) + try { + const abiFunction: FunctionFragment = AbiInterfaces.getFunction(methodSig) + return abiFunction.name + } catch (_) {} + return methodSig + } + private _violationDetected (violation: ERC7562Violation) { this.violations.push(violation) if (this.bailOnViolation) { @@ -126,72 +151,67 @@ export class ERC7562Parser { */ checkOp054 (erc7562Call: ERC7562Call): void { const isCallToEntryPoint = this._isCallToEntryPoint(erc7562Call) - // @ts-ignore - const isEntryPointCallAllowedOP052 = call.method === 'depositTo' - // @ts-ignore - const isEntryPointCallAllowedOP053 = call.method === '0x' + const knownMethod = this._tryDetectKnownMethod(erc7562Call) + const isEntryPointCallAllowedOP052 = knownMethod === 'depositTo' + const isEntryPointCallAllowedOP053 = knownMethod === '0x' const isEntryPointCallAllowed = isEntryPointCallAllowedOP052 || isEntryPointCallAllowedOP053 const isRuleViolated = isCallToEntryPoint && !isEntryPointCallAllowed - - this._violationDetected({ - rule: ERC7562Rule.op054, - // TODO: fill in depth, entity - depth: -1, - entity: AccountAbstractionEntity.fixme, - address: erc7562Call.from, - opcode: erc7562Call.type, - value: erc7562Call.value, - errorCode: ValidationErrors.OpcodeValidation, - // @ts-ignore - description: `illegal call into EntryPoint during validation ${it?.method}` - }) + if (isRuleViolated) { + this._violationDetected({ + rule: ERC7562Rule.op054, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: erc7562Call.from, + opcode: erc7562Call.type, + value: erc7562Call.value, + errorCode: ValidationErrors.OpcodeValidation, + // @ts-ignore + description: `illegal call into EntryPoint during validation ${erc7562Call?.method}` + }) + } } /** * OP-061: CALL with value is forbidden. The only exception is a call to the EntryPoint. */ - checkOp061 (tracerResults: ERC7562Call): ERC7562Violation[] { - const callStack = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress == null) as ERC7562Call[] - const illegalNonZeroValueCall = callStack.filter( - call => - !this._isCallToEntryPoint(call) && - !BigNumber.from(call.value ?? 0).eq(0) - ) - return illegalNonZeroValueCall.map((it: ERC7562Call): ERC7562Violation => { - return { + checkOp061 (tracerResults: ERC7562Call): void { + const isIllegalNonZeroValueCall = + !this._isCallToEntryPoint(tracerResults) && + !BigNumber.from(tracerResults.value ?? 0).eq(0) + if (isIllegalNonZeroValueCall) { + this._violationDetected({ rule: ERC7562Rule.op061, // TODO: fill in depth, entity depth: -1, entity: AccountAbstractionEntity.fixme, - address: it.from, - opcode: it.type, - value: it.value, + address: tracerResults.from, + opcode: tracerResults.type, + value: tracerResults.value, errorCode: ValidationErrors.OpcodeValidation, description: 'May not may CALL with value' - } - }) + }) + } } /** * OP-020: Revert on "out of gas" is forbidden as it can "leak" the gas limit or the current call stack depth. */ - checkOp020 (tracerResults: ERC7562Call): ERC7562Violation[] { - const entityCallsFromEntryPoint = tracerResults.calls!.filter((call: any) => call.topLevelTargetAddress != null) - const entityCallsWithOOG = entityCallsFromEntryPoint.filter((it: ERC7562Call) => it.outOfGas) - return entityCallsWithOOG.map((it: ERC7562Call) => { + checkOp020 (tracerResults: ERC7562Call): void { + if (tracerResults.outOfGas) { const entityTitle = 'fixme' - return { + this._violationDetected({ rule: ERC7562Rule.op020, // TODO: fill in depth, entity depth: -1, entity: AccountAbstractionEntity.fixme, - address: it.from ?? 'n/a', - opcode: it.type ?? 'n/a', + address: tracerResults.from, + opcode: tracerResults.type, value: '0', errorCode: ValidationErrors.OpcodeValidation, description: `${entityTitle} internally reverts on oog` - } - }) + }) + } } /** @@ -201,25 +221,25 @@ export class ERC7562Parser { checkOp011 (tracerResults: ERC7562Call): void { const opcodes = tracerResults.usedOpcodes const bannedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { - return bannedOpCodes.has(opcode) + const opcodeName = getOpcodeName(parseInt(opcode)) ?? '' + return bannedOpCodes.has(opcodeName) }) - bannedOpCodeUsed - .forEach( - (opcode: string): void => { - const entityTitle = 'fixme' - this._violationDetected({ - rule: ERC7562Rule.op011, - // TODO: fill in depth, entity - depth: -1, - entity: AccountAbstractionEntity.fixme, - address: tracerResults.from, - opcode, - value: '0', - errorCode: ValidationErrors.OpcodeValidation, - description: `${entityTitle} uses banned opcode: ${opcode}` - }) - } - ) + bannedOpCodeUsed.forEach( + (opcode: string): void => { + const entityTitle = 'fixme' + this._violationDetected({ + rule: ERC7562Rule.op011, + // TODO: fill in depth, entity + depth: -1, + entity: AccountAbstractionEntity.fixme, + address: tracerResults.from, + opcode, + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `${entityTitle} uses banned opcode: ${opcode}` + }) + } + ) } checkOp080 (tracerResults: ERC7562Call): void { @@ -281,24 +301,19 @@ export class ERC7562Parser { } checkStorage (userOp: OperationBase, tracerResults: ERC7562Call): void { - this.checkStorageInternal(userOp, tracerResults.calls![0]) - } - - checkStorageInternal (userOp: OperationBase, tracerResults: ERC7562Call): void { Object.entries(tracerResults.accessedSlots).forEach(([address, accessInfo]) => { - this.checkStorageInternalInternal(userOp, address, accessInfo) + this.checkStorageInternal(userOp, address, accessInfo) }) } - checkStorageInternalInternal ( + checkStorageInternal ( userOp: OperationBase, address: string, accessInfo: AccessedSlots - ): ERC7562Violation[] { - const violations: ERC7562Violation[] = [] + ): void { const allSlots: string[] = [ - ...Object.keys(accessInfo.writes), - ...Object.keys(accessInfo.reads), + ...Object.keys(accessInfo.writes ?? {}), + ...Object.keys(accessInfo.reads ?? {}), ...Object.keys(accessInfo.transientWrites ?? {}), ...Object.keys(accessInfo.transientReads ?? {}) ] @@ -312,7 +327,7 @@ export class ERC7562Parser { const isSenderAssociated: boolean = this._associatedWith(slot, userOp.sender.toLowerCase(), entitySlots) const isEntityInternalSTO031: boolean = address.toLowerCase() === entityAddress.toLowerCase() const isEntityAssociatedSTO032: boolean = this._associatedWith(slot, entityAddress, entitySlots) - const isReadOnlyAccessSTO033: boolean = accessInfo.writes[slot] == null && accessInfo.transientWrites[slot] == null + const isReadOnlyAccessSTO033: boolean = accessInfo.writes?.[slot] == null && accessInfo.transientWrites?.[slot] == null const isAllowedST031ST032ST033: boolean = (isEntityInternalSTO031 || isEntityAssociatedSTO032 || isReadOnlyAccessSTO033) && isEntityStaked @@ -323,13 +338,12 @@ export class ERC7562Parser { if (!allowed) { // TODO // @ts-ignore - violations.push({ + this._violationDetected({ // description: `${entityTitle} has forbidden ${readWrite} ${transientStr}${nameAddr(addr, stakeInfoEntities)} slot ${slot}`, // description: `unstaked ${entityTitle} accessed ${nameAddr(addr, stakeInfoEntities)} slot ${requireStakeSlot}`, entityTitle, access) }) } } - return violations } checkOp041 ( From 1697e56b979b561fda1e5fc42de9426f6913f723 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 14 Jan 2025 22:49:36 +0100 Subject: [PATCH 12/29] Passes banned opcodes spec test --- packages/bundler/src/BundlerConfig.ts | 3 + .../bundler/src/modules/ExecutionManager.ts | 5 +- packages/bundler/src/modules/initServer.ts | 2 +- packages/bundler/test/BundlerManager.test.ts | 6 +- packages/bundler/test/BundlerServer.test.ts | 3 +- .../bundler/test/DebugMethodHandler.test.ts | 3 +- .../bundler/test/UserOpMethodHandler.test.ts | 3 +- packages/bundler/test/ValidateManager.test.ts | 4 +- .../src/AccountAbstractionEntity.ts | 12 +- .../validation-manager/src/ERC7562Call.ts | 2 +- .../validation-manager/src/ERC7562Parser.ts | 127 ++++++++++++------ .../src/ERC7562Violation.ts | 6 +- .../validation-manager/src/enum/EVMOpcodes.ts | 44 ++++++ packages/validation-manager/src/index.ts | 3 +- 14 files changed, 161 insertions(+), 62 deletions(-) create mode 100644 packages/validation-manager/src/enum/EVMOpcodes.ts diff --git a/packages/bundler/src/BundlerConfig.ts b/packages/bundler/src/BundlerConfig.ts index 8feeb01a..aa11bc21 100644 --- a/packages/bundler/src/BundlerConfig.ts +++ b/packages/bundler/src/BundlerConfig.ts @@ -9,6 +9,7 @@ export interface BundlerConfig { chainId: number beneficiary: string entryPoint: string + senderCreator: string gasFactor: string minBalance: string mnemonic: string @@ -48,6 +49,7 @@ export const BundlerConfigShape = { chainId: ow.number, beneficiary: ow.string, entryPoint: ow.string, + senderCreator: ow.string, gasFactor: ow.string, minBalance: ow.string, mnemonic: ow.string, @@ -102,6 +104,7 @@ export const bundlerConfigDefault: Partial = { port: '3000', privateApiPort: '3001', entryPoint: '0x0000000071727De22E5E9d8BAf0edAc6f37da032', + senderCreator: '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c', unsafe: false, conditionalRpc: false, minStake: MIN_STAKE_VALUE, diff --git a/packages/bundler/src/modules/ExecutionManager.ts b/packages/bundler/src/modules/ExecutionManager.ts index 244c32e1..6052c0d0 100644 --- a/packages/bundler/src/modules/ExecutionManager.ts +++ b/packages/bundler/src/modules/ExecutionManager.ts @@ -148,8 +148,9 @@ export class ExecutionManager { async _setConfiguration (configOverrides: Partial): Promise { const { configuration, entryPoint, unsafe } = this.validationManager._getDebugConfiguration() - const pvgc = new PreVerificationGasCalculator(Object.assign({}, configuration, configOverrides)) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) + const mergedConfiguration = Object.assign({}, configuration, configOverrides) + const pvgc = new PreVerificationGasCalculator(mergedConfiguration) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, mergedConfiguration.senderCreator ?? '', true) this.validationManager = new ValidationManager( entryPoint, unsafe, diff --git a/packages/bundler/src/modules/initServer.ts b/packages/bundler/src/modules/initServer.ts index 9d953afa..cbe9678f 100644 --- a/packages/bundler/src/modules/initServer.ts +++ b/packages/bundler/src/modules/initServer.ts @@ -36,7 +36,7 @@ export function initServer (config: BundlerConfig, signer: Signer): [ExecutionMa const eventsManager = new EventsManager(entryPoint, mempoolManager, reputationManager) const mergedPvgcConfig = Object.assign({}, ChainConfigs[config.chainId] ?? {}, config) const preVerificationGasCalculator = new PreVerificationGasCalculator(mergedPvgcConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) let validationManager: IValidationManager let bundleManager: IBundleManager if (!config.rip7560) { diff --git a/packages/bundler/test/BundlerManager.test.ts b/packages/bundler/test/BundlerManager.test.ts index be475ee8..65759fa1 100644 --- a/packages/bundler/test/BundlerManager.test.ts +++ b/packages/bundler/test/BundlerManager.test.ts @@ -41,6 +41,7 @@ describe('#BundlerManager', () => { chainId: 1337, beneficiary: await signer.getAddress(), entryPoint: entryPoint.address, + senderCreator: '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c', gasFactor: '0.2', minBalance: '0', mnemonic: '', @@ -64,7 +65,7 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) bm = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc) @@ -99,6 +100,7 @@ describe('#BundlerManager', () => { chainId: 1337, beneficiary: await bundlerSigner.getAddress(), entryPoint: _entryPoint.address, + senderCreator: '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c', gasFactor: '0.2', minBalance: '0', mnemonic: '', @@ -121,7 +123,7 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(_entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(_entryPoint, mempoolMgr, repMgr) bundleMgr = new BundleManager(_entryPoint, _entryPoint.provider as JsonRpcProvider, _entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/BundlerServer.test.ts b/packages/bundler/test/BundlerServer.test.ts index ba9beefc..4d5dc48c 100644 --- a/packages/bundler/test/BundlerServer.test.ts +++ b/packages/bundler/test/BundlerServer.test.ts @@ -37,6 +37,7 @@ describe('BundleServer', function () { chainId: 1337, beneficiary: await signer.getAddress(), entryPoint: entryPoint.address, + senderCreator: '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c', gasFactor: '0.2', minBalance: '0', mnemonic: '', @@ -60,7 +61,7 @@ describe('BundleServer', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/DebugMethodHandler.test.ts b/packages/bundler/test/DebugMethodHandler.test.ts index c12a88c2..4fd3687e 100644 --- a/packages/bundler/test/DebugMethodHandler.test.ts +++ b/packages/bundler/test/DebugMethodHandler.test.ts @@ -47,6 +47,7 @@ describe('#DebugMethodHandler', () => { chainId: 1337, beneficiary: await signer.getAddress(), entryPoint: entryPoint.address, + senderCreator: '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c', gasFactor: '0.2', minBalance: '0', mnemonic: '', @@ -70,7 +71,7 @@ describe('#DebugMethodHandler', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const eventsManager = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, eventsManager, mempoolMgr, validMgr, repMgr, diff --git a/packages/bundler/test/UserOpMethodHandler.test.ts b/packages/bundler/test/UserOpMethodHandler.test.ts index 616c8069..2052aa5b 100644 --- a/packages/bundler/test/UserOpMethodHandler.test.ts +++ b/packages/bundler/test/UserOpMethodHandler.test.ts @@ -65,6 +65,7 @@ describe('UserOpMethodHandler', function () { chainId: 1337, beneficiary: await signer.getAddress(), entryPoint: entryPoint.address, + senderCreator: '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c', gasFactor: '0.2', minBalance: '0', mnemonic: '', @@ -88,7 +89,7 @@ describe('UserOpMethodHandler', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/ValidateManager.test.ts b/packages/bundler/test/ValidateManager.test.ts index a0307126..35021181 100644 --- a/packages/bundler/test/ValidateManager.test.ts +++ b/packages/bundler/test/ValidateManager.test.ts @@ -155,7 +155,9 @@ describe('#ValidationManager', () => { const unsafe = !await supportsDebugTraceCall(provider, false) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, true) + + const senderCreator = '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c' + const erc7562Parser = new ERC7562Parser({}, entryPoint.address, senderCreator, true) vm = new ValidationManager(entryPoint, unsafe, preVerificationGasCalculator, erc7562Parser) if (!await supportsDebugTraceCall(ethers.provider, false)) { diff --git a/packages/validation-manager/src/AccountAbstractionEntity.ts b/packages/validation-manager/src/AccountAbstractionEntity.ts index b96ee075..97a75597 100644 --- a/packages/validation-manager/src/AccountAbstractionEntity.ts +++ b/packages/validation-manager/src/AccountAbstractionEntity.ts @@ -1,11 +1,9 @@ export const enum AccountAbstractionEntity { - sender = 'Sender', - paymaster = 'Paymaster', - factory = 'Factory', - aggregator = 'Aggregator', + account = 'account', + paymaster = 'paymaster', + factory = 'factory', + aggregator = 'aggregator', senderCreator = 'SenderCreator', entryPoint = 'EntryPoint', - // TODO: leaving 'fixme' entity for future refactor - // (some rules are checked in a way that makes it hard to find entity) - fixme = 'fixme' + none = 'none' } diff --git a/packages/validation-manager/src/ERC7562Call.ts b/packages/validation-manager/src/ERC7562Call.ts index c37a46ce..21b73144 100644 --- a/packages/validation-manager/src/ERC7562Call.ts +++ b/packages/validation-manager/src/ERC7562Call.ts @@ -68,7 +68,7 @@ const erc7562CallSchema = ow.object.exactShape({ export function validateERC7562Call (value: ERC7562Call): void { ow(value, erc7562CallSchema) - if (value.calls) { + if (value.calls != null) { for (const call of value.calls) { validateERC7562Call(call) } diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index a6f35efc..1c83d62f 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -1,11 +1,14 @@ -import { BigNumber, ethers } from 'ethers' -import { hexZeroPad, Interface, FunctionFragment } from 'ethers/lib/utils' +import { BigNumber } from 'ethers' +import { FunctionFragment, hexZeroPad, Interface } from 'ethers/lib/utils' import { AddressZero, - IEntryPoint__factory, IPaymaster__factory, + IEntryPoint__factory, + IPaymaster__factory, OperationBase, + RpcError, SenderCreator__factory, + StakeInfo, StorageMap, ValidationErrors } from '@account-abstraction/utils' @@ -26,11 +29,14 @@ export interface ERC7562ValidationResults { } export class ERC7562Parser { - private violations: ERC7562Violation[] = [] + private ruleViolations: ERC7562Violation[] = [] + private currentEntity: AccountAbstractionEntity = AccountAbstractionEntity.none + private stakeValidationResult!: ValidationResult constructor ( readonly mempoolConfig: AltMempoolConfig, readonly entryPointAddress: string, + readonly senderCreatorAddress: string, readonly bailOnViolation: boolean ) { @@ -43,12 +49,22 @@ export class ERC7562Parser { call.from?.toLowerCase() !== AddressZero } - private _isEntityStaked (topLevelCallInfo: ERC7562Call): boolean { - throw new Error('Method not implemented.') - } - - private _getEntity (userOp: OperationBase, address: string): AccountAbstractionEntity { - return AccountAbstractionEntity.fixme + private _isEntityStaked (): boolean { + let entStake: StakeInfo | undefined + switch (this.currentEntity) { + case AccountAbstractionEntity.account: + entStake = this.stakeValidationResult.senderInfo + break + case AccountAbstractionEntity.factory: + entStake = this.stakeValidationResult.senderInfo + break + case AccountAbstractionEntity.paymaster: + entStake = this.stakeValidationResult.senderInfo + break + default: + break + } + return entStake != null && BigNumber.from(1).lte(entStake.stake) && BigNumber.from(1).lte(entStake.unstakeDelaySec) } private _associatedWith (slot: string, addr: string, entitySlots: { [addr: string]: Set }): boolean { @@ -88,12 +104,31 @@ export class ERC7562Parser { } private _violationDetected (violation: ERC7562Violation) { - this.violations.push(violation) + this.ruleViolations.push(violation) if (this.bailOnViolation) { throw toError(violation) } } + private _detectEntityChange (userOp: OperationBase, call: ERC7562Call): void { + if (call.from.toLowerCase() !== this.entryPointAddress.toLowerCase()) { + return + } + if (userOp.sender.toLowerCase() === call.to.toLowerCase()) { + this.currentEntity = AccountAbstractionEntity.account + } else if (userOp.factory?.toLowerCase() === call.to.toLowerCase()) { + this.currentEntity = AccountAbstractionEntity.factory + } else if (userOp.paymaster?.toLowerCase() === call.to.toLowerCase()) { + this.currentEntity = AccountAbstractionEntity.paymaster + } else if (this.entryPointAddress.toLowerCase() === call.to.toLowerCase()) { + this.currentEntity = AccountAbstractionEntity.entryPoint + } else if (this.senderCreatorAddress.toLowerCase() === call.to.toLowerCase()) { + this.currentEntity = AccountAbstractionEntity.senderCreator + } else { + throw new RpcError(`could not find entity name for address ${call.to}. This should not happen. This is a bug.`, 0) + } + } + /** * Validates the UserOperation and throws an exception in case current mempool configuration rules were violated. */ @@ -102,7 +137,7 @@ export class ERC7562Parser { tracerResults: ERC7562Call, validationResult: ValidationResult ): ERC7562ValidationResults { - const results = this.parseResults(userOp, tracerResults) + const results = this.parseResults(userOp, tracerResults, validationResult) if (results.ruleViolations.length > 0) { // TODO: human-readable description of which rules were violated. throw new Error('Rules Violated') @@ -112,13 +147,16 @@ export class ERC7562Parser { parseResults ( userOp: OperationBase, - tracerResults: ERC7562Call + tracerResults: ERC7562Call, + validationResult: ValidationResult ): ERC7562ValidationResults { - this.violations = [] - if (tracerResults.calls == null || tracerResults.calls.length < 1) { throw new Error('Unexpected traceCall result: no calls from entrypoint.') } + + this.ruleViolations = [] + this.stakeValidationResult = validationResult + return this._innerStepRecursive(userOp, tracerResults, 0) } @@ -127,11 +165,12 @@ export class ERC7562Parser { tracerResults: ERC7562Call, recursionDepth: number ): ERC7562ValidationResults { - + this._detectEntityChange(userOp, tracerResults) this.checkOp054(tracerResults) this.checkOp054ExtCode(tracerResults) this.checkOp061(tracerResults) this.checkOp011(tracerResults) + this.checkOp080(tracerResults) this.checkOp020(tracerResults) this.checkOp031(userOp, tracerResults) this.checkOp041(userOp, tracerResults) @@ -140,7 +179,7 @@ export class ERC7562Parser { this._innerStepRecursive(userOp, call, recursionDepth + 1) } return { - contractAddresses: [], ruleViolations: [], storageMap: {} + contractAddresses: [], ruleViolations: this.ruleViolations, storageMap: {} } } @@ -161,7 +200,7 @@ export class ERC7562Parser { rule: ERC7562Rule.op054, // TODO: fill in depth, entity depth: -1, - entity: AccountAbstractionEntity.fixme, + entity: this.currentEntity, address: erc7562Call.from, opcode: erc7562Call.type, value: erc7562Call.value, @@ -184,7 +223,7 @@ export class ERC7562Parser { rule: ERC7562Rule.op061, // TODO: fill in depth, entity depth: -1, - entity: AccountAbstractionEntity.fixme, + entity: this.currentEntity, address: tracerResults.from, opcode: tracerResults.type, value: tracerResults.value, @@ -199,17 +238,16 @@ export class ERC7562Parser { */ checkOp020 (tracerResults: ERC7562Call): void { if (tracerResults.outOfGas) { - const entityTitle = 'fixme' this._violationDetected({ rule: ERC7562Rule.op020, // TODO: fill in depth, entity depth: -1, - entity: AccountAbstractionEntity.fixme, + entity: this.currentEntity, address: tracerResults.from, opcode: tracerResults.type, value: '0', errorCode: ValidationErrors.OpcodeValidation, - description: `${entityTitle} internally reverts on oog` + description: `${this.currentEntity.toString()} internally reverts on oog` }) } } @@ -220,23 +258,25 @@ export class ERC7562Parser { */ checkOp011 (tracerResults: ERC7562Call): void { const opcodes = tracerResults.usedOpcodes - const bannedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { - const opcodeName = getOpcodeName(parseInt(opcode)) ?? '' - return bannedOpCodes.has(opcodeName) - }) + const bannedOpCodeUsed = Object.keys(opcodes) + .map((opcode: string) => { + return getOpcodeName(parseInt(opcode)) ?? '' + }) + .filter((opcode: string) => { + return bannedOpCodes.has(opcode) + }) bannedOpCodeUsed.forEach( (opcode: string): void => { - const entityTitle = 'fixme' this._violationDetected({ rule: ERC7562Rule.op011, // TODO: fill in depth, entity depth: -1, - entity: AccountAbstractionEntity.fixme, + entity: this.currentEntity, address: tracerResults.from, opcode, value: '0', errorCode: ValidationErrors.OpcodeValidation, - description: `${entityTitle} uses banned opcode: ${opcode}` + description: `${this.currentEntity.toString()} uses banned opcode: ${opcode.toString()}` }) } ) @@ -244,24 +284,23 @@ export class ERC7562Parser { checkOp080 (tracerResults: ERC7562Call): void { const opcodes = tracerResults.usedOpcodes - const isEntityStaked = this._isEntityStaked(tracerResults) + const isEntityStaked = this._isEntityStaked() const onlyStakedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { return opcodesOnlyInStakedEntities.has(opcode) && !isEntityStaked }) onlyStakedOpCodeUsed .forEach( (opcode: string): void => { - const entityTitle = 'fixme' this._violationDetected({ rule: ERC7562Rule.op011, // TODO: fill in depth, entity depth: -1, - entity: AccountAbstractionEntity.fixme, + entity: this.currentEntity, address: tracerResults.from ?? 'n/a', opcode, value: '0', errorCode: ValidationErrors.OpcodeValidation, - description: `unstaked ${entityTitle} uses banned opcode: ${opcode}` + description: `unstaked ${this.currentEntity.toString()} uses banned opcode: ${opcode}` }) } ) @@ -280,22 +319,29 @@ export class ERC7562Parser { ) { return } - const entityTitle = 'fixme' as string const isFactoryStaked = false - const isAllowedCreateByOP032 = entityTitle === 'account' && isFactoryStaked && tracerResults.from === userOp.sender.toLowerCase() - const isAllowedCreateByEREP060 = entityTitle === 'factory' && tracerResults.from === userOp.factory && isFactoryStaked - const isAllowedCreateSenderByFactory = entityTitle === 'factory' && tracerResults.to === userOp.sender.toLowerCase() + const isAllowedCreateByOP032 = + this.currentEntity === AccountAbstractionEntity.account && + tracerResults.from === userOp.sender.toLowerCase() && + isFactoryStaked + const isAllowedCreateByEREP060 = + this.currentEntity === AccountAbstractionEntity.factory && + tracerResults.from === userOp.factory && + isFactoryStaked + const isAllowedCreateSenderByFactory = + this.currentEntity === AccountAbstractionEntity.factory && + tracerResults.to === userOp.sender.toLowerCase() if (!(isAllowedCreateByOP032 || isAllowedCreateByEREP060 || isAllowedCreateSenderByFactory)) { this._violationDetected({ rule: ERC7562Rule.op011, // TODO: fill in depth, entity depth: -1, - entity: AccountAbstractionEntity.fixme, + entity: this.currentEntity, address: tracerResults.from ?? 'n/a', opcode: 'CREATE2', value: '0', errorCode: ValidationErrors.OpcodeValidation, - description: `${entityTitle} uses banned opcode: CREATE2` + description: `${this.currentEntity.toString()} uses banned opcode: CREATE2` }) } } @@ -350,7 +396,6 @@ export class ERC7562Parser { userOp: OperationBase, tracerResults: ERC7562Call ): void { - const entityTitle = 'fixme' // the only contract we allow to access before its deployment is the "sender" itself, which gets created. let illegalZeroCodeAccess: any for (const addr of Object.keys(tracerResults.contractSize)) { @@ -361,7 +406,7 @@ export class ERC7562Parser { // @ts-ignore this._violationDetected({ errorCode: ValidationErrors.OpcodeValidation, - description: `${entityTitle} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}` + description: `${this.currentEntity.toString()} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}` }) } } diff --git a/packages/validation-manager/src/ERC7562Violation.ts b/packages/validation-manager/src/ERC7562Violation.ts index 3d1694e2..fc78d22b 100644 --- a/packages/validation-manager/src/ERC7562Violation.ts +++ b/packages/validation-manager/src/ERC7562Violation.ts @@ -1,4 +1,4 @@ -import { ValidationErrors } from '@account-abstraction/utils' +import { RpcError, ValidationErrors } from '@account-abstraction/utils' import { ERC7562Rule } from './enum/ERC7562Rule' import { AccountAbstractionEntity } from './AccountAbstractionEntity' @@ -16,6 +16,6 @@ export interface ERC7562Violation { slot?: string } -export function toError(violation: ERC7562Violation): Error { - return new Error('TODO: VIOLATED!') +export function toError (violation: ERC7562Violation): Error { + return new RpcError(violation.description, violation.errorCode) } diff --git a/packages/validation-manager/src/enum/EVMOpcodes.ts b/packages/validation-manager/src/enum/EVMOpcodes.ts new file mode 100644 index 00000000..af38b960 --- /dev/null +++ b/packages/validation-manager/src/enum/EVMOpcodes.ts @@ -0,0 +1,44 @@ +export enum EVMOpcodes { + ADDRESS = 0x30, + BALANCE = 0x31, + ORIGIN = 0x32, + GASPRICE = 0x3A, + BLOCKHASH = 0x40, + COINBASE = 0x41, + TIMESTAMP = 0x42, + NUMBER = 0x43, + DIFFICULTY = 0x44, + GASLIMIT = 0x45, + SELFBALANCE = 0x47, + BASEFEE = 0x48, + BLOBHASH = 0x49, + BLOBBASEFEE = 0x4A, + GAS = 0x5A, + CREATE = 0xF0, + SELFDESTRUCT = 0xFF, + EXTCODESIZE = 0x3B, + EXTCODECOPY = 0x3C, + EXTCODEHASH = 0x3F, + CHAINID = 0x46, + MLOAD = 0x51, + MSTORE = 0x52, + MSTORE8 = 0x53, + SLOAD = 0x54, + SSTORE = 0x55, + JUMPDEST = 0x5B, + TLOAD = 0x5C, + TSTORE = 0x5D, + MCOPY = 0x5E, + PUSH0 = 0x5F, + CALL = 0xF1, + RETURN = 0xF3, + DELEGATECALL = 0xF4, + CREATE2 = 0xF5, + STATICCALL = 0xFA, + REVERT = 0xFD, + INVALID = 0xFE, +} + +export function getOpcodeName (value: number): string | undefined { + return Object.entries(EVMOpcodes).find(([_, v]) => v === value)?.[0] +} diff --git a/packages/validation-manager/src/index.ts b/packages/validation-manager/src/index.ts index bada1d90..9ec1a173 100644 --- a/packages/validation-manager/src/index.ts +++ b/packages/validation-manager/src/index.ts @@ -86,7 +86,8 @@ export async function checkRulesViolations ( throw new Error('This provider does not support stack tracing') } const entryPoint = IEntryPoint__factory.connect(entryPointAddress, provider) - const erc7562Parser = new ERC7562Parser({}, entryPointAddress, true) + const senderCreator = '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c' + const erc7562Parser = new ERC7562Parser({}, entryPointAddress, senderCreator, true) const validationManager = new ValidationManager( entryPoint, false, From ec7b480666302fec20a007de64d57e747f115d61 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Sun, 19 Jan 2025 22:33:00 +0100 Subject: [PATCH 13/29] Fix some error reporting logic --- .../src/AccountAbstractionEntity.ts | 2 +- .../validation-manager/src/ERC7562Parser.ts | 121 +++++++++++++----- .../src/ValidationManager.ts | 8 +- 3 files changed, 95 insertions(+), 36 deletions(-) diff --git a/packages/validation-manager/src/AccountAbstractionEntity.ts b/packages/validation-manager/src/AccountAbstractionEntity.ts index 97a75597..e5fe6280 100644 --- a/packages/validation-manager/src/AccountAbstractionEntity.ts +++ b/packages/validation-manager/src/AccountAbstractionEntity.ts @@ -1,4 +1,4 @@ -export const enum AccountAbstractionEntity { +export enum AccountAbstractionEntity { account = 'account', paymaster = 'paymaster', factory = 'factory', diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index 1c83d62f..175c6f75 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -31,6 +31,7 @@ export interface ERC7562ValidationResults { export class ERC7562Parser { private ruleViolations: ERC7562Violation[] = [] private currentEntity: AccountAbstractionEntity = AccountAbstractionEntity.none + private currentEntityAddress: string = '' private stakeValidationResult!: ValidationResult constructor ( @@ -103,7 +104,7 @@ export class ERC7562Parser { return methodSig } - private _violationDetected (violation: ERC7562Violation) { + private _violationDetected (violation: ERC7562Violation): void { this.ruleViolations.push(violation) if (this.bailOnViolation) { throw toError(violation) @@ -111,24 +112,49 @@ export class ERC7562Parser { } private _detectEntityChange (userOp: OperationBase, call: ERC7562Call): void { - if (call.from.toLowerCase() !== this.entryPointAddress.toLowerCase()) { + if (call.from.toLowerCase() !== this.entryPointAddress.toLowerCase() && + call.from.toLowerCase() !== this.senderCreatorAddress.toLowerCase()) { return } if (userOp.sender.toLowerCase() === call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.account - } else if (userOp.factory?.toLowerCase() === call.to.toLowerCase()) { + this.currentEntityAddress = userOp.sender + } else if ( + call.from.toLowerCase() === this.senderCreatorAddress.toLowerCase() && + userOp.factory?.toLowerCase() === call.to.toLowerCase() + ) { this.currentEntity = AccountAbstractionEntity.factory + this.currentEntityAddress = userOp.factory } else if (userOp.paymaster?.toLowerCase() === call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.paymaster + this.currentEntityAddress = userOp.paymaster } else if (this.entryPointAddress.toLowerCase() === call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.entryPoint + this.currentEntityAddress = this.entryPointAddress } else if (this.senderCreatorAddress.toLowerCase() === call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.senderCreator + this.currentEntityAddress = this.senderCreatorAddress } else { throw new RpcError(`could not find entity name for address ${call.to}. This should not happen. This is a bug.`, 0) } } + private _tryGetAddressName (userOp: OperationBase, address: string): AccountAbstractionEntity | string { + const lowerAddress = address.toLowerCase() + if (lowerAddress === userOp.sender.toLowerCase()) { + return AccountAbstractionEntity.account + } else if (userOp.factory?.toLowerCase() === lowerAddress) { + return AccountAbstractionEntity.factory + } else if (userOp.paymaster?.toLowerCase() === lowerAddress) { + return AccountAbstractionEntity.paymaster + } else if (this.entryPointAddress.toLowerCase() === lowerAddress) { + return AccountAbstractionEntity.entryPoint + } else if (this.senderCreatorAddress.toLowerCase() === lowerAddress) { + return AccountAbstractionEntity.senderCreator + } + return address + } + /** * Validates the UserOperation and throws an exception in case current mempool configuration rules were violated. */ @@ -165,9 +191,10 @@ export class ERC7562Parser { tracerResults: ERC7562Call, recursionDepth: number ): ERC7562ValidationResults { + const address = tracerResults.to this._detectEntityChange(userOp, tracerResults) this.checkOp054(tracerResults) - this.checkOp054ExtCode(tracerResults) + this.checkOp054ExtCode(tracerResults, address, recursionDepth) this.checkOp061(tracerResults) this.checkOp011(tracerResults) this.checkOp080(tracerResults) @@ -205,8 +232,7 @@ export class ERC7562Parser { opcode: erc7562Call.type, value: erc7562Call.value, errorCode: ValidationErrors.OpcodeValidation, - // @ts-ignore - description: `illegal call into EntryPoint during validation ${erc7562Call?.method}` + description: `illegal call into EntryPoint during validation ${knownMethod}` }) } } @@ -258,13 +284,15 @@ export class ERC7562Parser { */ checkOp011 (tracerResults: ERC7562Call): void { const opcodes = tracerResults.usedOpcodes - const bannedOpCodeUsed = Object.keys(opcodes) - .map((opcode: string) => { - return getOpcodeName(parseInt(opcode)) ?? '' - }) - .filter((opcode: string) => { - return bannedOpCodes.has(opcode) - }) + const bannedOpCodeUsed = + Object + .keys(opcodes) + .map((opcode: string) => { + return getOpcodeName(parseInt(opcode)) ?? '' + }) + .filter((opcode: string) => { + return bannedOpCodes.has(opcode) + }) bannedOpCodeUsed.forEach( (opcode: string): void => { this._violationDetected({ @@ -285,9 +313,15 @@ export class ERC7562Parser { checkOp080 (tracerResults: ERC7562Call): void { const opcodes = tracerResults.usedOpcodes const isEntityStaked = this._isEntityStaked() - const onlyStakedOpCodeUsed = Object.keys(opcodes).filter((opcode: string) => { - return opcodesOnlyInStakedEntities.has(opcode) && !isEntityStaked - }) + const onlyStakedOpCodeUsed = + Object + .keys(opcodes) + .map((opcode: string) => { + return getOpcodeName(parseInt(opcode)) ?? '' + }) + .filter((opcode: string) => { + return opcodesOnlyInStakedEntities.has(opcode) && !isEntityStaked + }) onlyStakedOpCodeUsed .forEach( (opcode: string): void => { @@ -364,29 +398,45 @@ export class ERC7562Parser { ...Object.keys(accessInfo.transientReads ?? {}) ] const entitySlots = {} // TODO: restore - const entityAddress = '' // TODO: restore + const addressName = this._tryGetAddressName(userOp, address) const isEntityStaked = false // TODO const isFactoryStaked = false // TODO const isSenderCreation = false // TODO for (const slot of allSlots) { const isSenderInternalSTO010: boolean = address.toLowerCase() === userOp.sender.toLowerCase() const isSenderAssociated: boolean = this._associatedWith(slot, userOp.sender.toLowerCase(), entitySlots) - const isEntityInternalSTO031: boolean = address.toLowerCase() === entityAddress.toLowerCase() - const isEntityAssociatedSTO032: boolean = this._associatedWith(slot, entityAddress, entitySlots) + const isEntityInternalSTO031: boolean = address.toLowerCase() === this.currentEntityAddress.toLowerCase() + const isEntityAssociatedSTO032: boolean = this._associatedWith(slot, this.currentEntityAddress, entitySlots) const isReadOnlyAccessSTO033: boolean = accessInfo.writes?.[slot] == null && accessInfo.transientWrites?.[slot] == null - const isAllowedST031ST032ST033: boolean = - (isEntityInternalSTO031 || isEntityAssociatedSTO032 || isReadOnlyAccessSTO033) && isEntityStaked + const isAllowedIfEntityStaked = isEntityInternalSTO031 || isEntityAssociatedSTO032 || isReadOnlyAccessSTO033 + const isAllowedST031ST032ST033: boolean = isAllowedIfEntityStaked && isEntityStaked const isAllowedSTO021: boolean = isSenderAssociated && !isSenderCreation - const isAllowedSTO022: boolean = isSenderAssociated && isSenderCreation && isFactoryStaked + const isAllowedIfFactoryStaked = isSenderAssociated && isSenderCreation + const isAllowedSTO022: boolean = isAllowedIfFactoryStaked && isFactoryStaked const allowed = isSenderInternalSTO010 || isAllowedSTO021 || isAllowedSTO022 || isAllowedST031ST032ST033 if (!allowed) { - // TODO - // @ts-ignore + let description: string + if ( + (isAllowedIfEntityStaked && !isEntityStaked) || + (isAllowedIfFactoryStaked && !isFactoryStaked) + ) { + description = `unstaked ${this.currentEntity.toString()} accessed ${addressName} slot ${slot}` + } else { + const isWrite = Object.keys(accessInfo.writes ?? {}).includes(slot) || Object.keys(accessInfo.transientWrites ?? {}).includes(slot) + const isTransient = Object.keys(accessInfo.transientReads ?? {}).includes(slot) || Object.keys(accessInfo.transientWrites ?? {}).includes(slot) + const readWrite = isWrite ? 'write to' : 'read from' + const transientStr = isTransient ? 'transient ' : '' + description = `${this.currentEntity.toString()} has forbidden ${readWrite} ${transientStr}${addressName} slot ${slot}` + } this._violationDetected({ - // description: `${entityTitle} has forbidden ${readWrite} ${transientStr}${nameAddr(addr, stakeInfoEntities)} slot ${slot}`, - // description: `unstaked ${entityTitle} accessed ${nameAddr(addr, stakeInfoEntities)} slot ${requireStakeSlot}`, entityTitle, access) + address: '', + depth: 0, + entity: this.currentEntity, + errorCode: ValidationErrors.OpcodeValidation, + rule: ERC7562Rule.sto010, + description }) } } @@ -403,8 +453,11 @@ export class ERC7562Parser { if (addr.toLowerCase() !== userOp.sender.toLowerCase() && addr.toLowerCase() !== this.entryPointAddress.toLowerCase() && tracerResults.contractSize[addr].contractSize <= 2) { illegalZeroCodeAccess = tracerResults.contractSize[addr] illegalZeroCodeAccess.address = addr - // @ts-ignore this._violationDetected({ + address: '', + depth: 0, + entity: this.currentEntity, + rule: ERC7562Rule.op041, errorCode: ValidationErrors.OpcodeValidation, description: `${this.currentEntity.toString()} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}` }) @@ -413,16 +466,22 @@ export class ERC7562Parser { } checkOp054ExtCode ( - tracerResults: ERC7562Call + tracerResults: ERC7562Call, + address: string, + recursionDepth: number ): void { const entityTitle = 'fixme' let illegalEntryPointCodeAccess for (const addr of Object.keys(tracerResults.extCodeAccessInfo)) { if (addr.toLowerCase() === this.entryPointAddress.toLowerCase()) { illegalEntryPointCodeAccess = tracerResults.extCodeAccessInfo - // @ts-ignore - violations.push({ - description: `${entityTitle} accesses EntryPoint contract address ${this.entryPointAddress} with opcode ${illegalEntryPointCodeAccess}` + this._violationDetected({ + address, + depth: recursionDepth, + entity: this.currentEntity, + errorCode: ValidationErrors.OpcodeValidation, + rule: ERC7562Rule.op054, + description: `${entityTitle} accesses EntryPoint contract address ${this.entryPointAddress} with opcode $ {'todo'}` }) } } diff --git a/packages/validation-manager/src/ValidationManager.ts b/packages/validation-manager/src/ValidationManager.ts index 868101f0..80a872f3 100644 --- a/packages/validation-manager/src/ValidationManager.ts +++ b/packages/validation-manager/src/ValidationManager.ts @@ -23,21 +23,21 @@ import { decodeErrorReason, decodeRevertReason, getAddr, + getAuthorizationList, getEip7702AuthorizationSigner, mergeValidationDataValues, packUserOp, requireAddressAndFields, requireCond, - runContractScript, getAuthorizationList, SenderCreator__factory, IEntryPoint__factory, IPaymaster__factory + runContractScript } from '@account-abstraction/utils' import { debug_traceCall } from './GethTracer' import EntryPointSimulationsJson from '@account-abstraction/contracts/artifacts/EntryPointSimulations.json' import { IValidationManager, ValidateUserOpResult, ValidationResult } from './IValidationManager' -import { Interface } from 'ethers/lib/utils' import { ERC7562Parser } from './ERC7562Parser' -import { ERC7562Call, validateERC7562Call } from './ERC7562Call' +import { ERC7562Call } from './ERC7562Call' const debug = Debug('aa.mgr.validate') @@ -175,7 +175,7 @@ export class ValidationManager implements IValidationManager { if (!this.usingErc7562NativeTracer()) { // Using preState tracer + JS tracer const lastResult = tracerResult.calls.slice(-1)[0] - data = (lastResult as any).data + data = (lastResult).data if (lastResult.type === 'REVERT') { throw new RpcError(decodeRevertReason(data, false) as string, ValidationErrors.SimulateValidation) } From 0185e2e22ae627aa78f343a81b2c261dc88270a4 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 20 Jan 2025 00:27:19 +0100 Subject: [PATCH 14/29] Fix incorrect stake check --- packages/validation-manager/src/ERC7562Parser.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index 175c6f75..1ece453b 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -57,10 +57,10 @@ export class ERC7562Parser { entStake = this.stakeValidationResult.senderInfo break case AccountAbstractionEntity.factory: - entStake = this.stakeValidationResult.senderInfo + entStake = this.stakeValidationResult.factoryInfo break case AccountAbstractionEntity.paymaster: - entStake = this.stakeValidationResult.senderInfo + entStake = this.stakeValidationResult.paymasterInfo break default: break From 3d8f316da8ca6aef20cfffbf869b1ddb3c02961a Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 20 Jan 2025 01:13:36 +0100 Subject: [PATCH 15/29] Fix EREP-060 and some other bugs --- .../validation-manager/src/ERC7562Parser.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index 1ece453b..fd97a222 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -50,9 +50,9 @@ export class ERC7562Parser { call.from?.toLowerCase() !== AddressZero } - private _isEntityStaked (): boolean { + private _isEntityStaked (entity?: AccountAbstractionEntity): boolean { let entStake: StakeInfo | undefined - switch (this.currentEntity) { + switch (entity ?? this.currentEntity) { case AccountAbstractionEntity.account: entStake = this.stakeValidationResult.senderInfo break @@ -353,18 +353,20 @@ export class ERC7562Parser { ) { return } - const isFactoryStaked = false + const isFactoryStaked = this._isEntityStaked(AccountAbstractionEntity.factory) const isAllowedCreateByOP032 = + tracerResults.type === 'CREATE' && this.currentEntity === AccountAbstractionEntity.account && - tracerResults.from === userOp.sender.toLowerCase() && - isFactoryStaked + tracerResults.from === userOp.sender.toLowerCase() const isAllowedCreateByEREP060 = - this.currentEntity === AccountAbstractionEntity.factory && - tracerResults.from === userOp.factory && + ( + tracerResults.from.toLowerCase() === userOp.sender?.toLowerCase() || + tracerResults.from.toLowerCase() === userOp.factory?.toLowerCase() + ) && isFactoryStaked const isAllowedCreateSenderByFactory = this.currentEntity === AccountAbstractionEntity.factory && - tracerResults.to === userOp.sender.toLowerCase() + tracerResults.to.toLowerCase() === userOp.sender.toLowerCase() if (!(isAllowedCreateByOP032 || isAllowedCreateByEREP060 || isAllowedCreateSenderByFactory)) { this._violationDetected({ rule: ERC7562Rule.op011, From df3ec832a0e3d8b26307b5be3cf8e8709343ed3b Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 20 Jan 2025 14:43:27 +0100 Subject: [PATCH 16/29] Fix associated storage attribution, struct access --- .../validation-manager/src/ERC7562Parser.ts | 84 ++++++++++++++----- 1 file changed, 61 insertions(+), 23 deletions(-) diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index fd97a222..96e12413 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -1,5 +1,5 @@ import { BigNumber } from 'ethers' -import { FunctionFragment, hexZeroPad, Interface } from 'ethers/lib/utils' +import { FunctionFragment, hexZeroPad, Interface, keccak256 } from 'ethers/lib/utils' import { AddressZero, @@ -10,6 +10,7 @@ import { SenderCreator__factory, StakeInfo, StorageMap, + toBytes32, ValidationErrors } from '@account-abstraction/utils' @@ -18,7 +19,7 @@ import { ERC7562Rule } from './enum/ERC7562Rule' import { AccountAbstractionEntity } from './AccountAbstractionEntity' import { bannedOpCodes, opcodesOnlyInStakedEntities } from './ERC7562BannedOpcodes' import { ValidationResult } from './IValidationManager' -import { AccessedSlots, ERC7562Call } from './ERC7562Call' +import { ERC7562Call } from './ERC7562Call' import { AltMempoolConfig } from './altmempool/AltMempoolConfig' import { getOpcodeName } from './enum/EVMOpcodes' @@ -29,6 +30,7 @@ export interface ERC7562ValidationResults { } export class ERC7562Parser { + private keccack: string[] = [] private ruleViolations: ERC7562Violation[] = [] private currentEntity: AccountAbstractionEntity = AccountAbstractionEntity.none private currentEntityAddress: string = '' @@ -112,7 +114,9 @@ export class ERC7562Parser { } private _detectEntityChange (userOp: OperationBase, call: ERC7562Call): void { - if (call.from.toLowerCase() !== this.entryPointAddress.toLowerCase() && + if ( + call.from.toLowerCase() !== AddressZero && + call.from.toLowerCase() !== this.entryPointAddress.toLowerCase() && call.from.toLowerCase() !== this.senderCreatorAddress.toLowerCase()) { return } @@ -155,6 +159,39 @@ export class ERC7562Parser { return address } + /** + * Calculate storage slots associated with each entity. + * keccak( A || ...) is associated with "A" + * + * @param userOp + */ + private _parseEntitySlots ( + userOp: OperationBase + ): { + [addr: string]: Set + } { + // for each entity (sender, factory, paymaster), hold the valid slot addresses + const entityAddresses = [userOp.sender.toLowerCase(), userOp.paymaster?.toLowerCase(), userOp.factory?.toLowerCase()] + const entitySlots: { [addr: string]: Set } = {} + + for (const keccakInput of this.keccack) { + for (const entityAddress of entityAddresses) { + if (entityAddress == null) { + continue + } + const addrPadded = toBytes32(entityAddress) + // valid slot: the slot was generated by keccak(entityAddr || ...) + if (keccakInput.startsWith(addrPadded)) { + if (entitySlots[entityAddress] == null) { + entitySlots[entityAddress] = new Set() + } + entitySlots[entityAddress].add(keccak256(keccakInput)) + } + } + } + return entitySlots + } + /** * Validates the UserOperation and throws an exception in case current mempool configuration rules were violated. */ @@ -180,6 +217,9 @@ export class ERC7562Parser { throw new Error('Unexpected traceCall result: no calls from entrypoint.') } + this.keccack = tracerResults.keccak ?? [] + this.currentEntity = AccountAbstractionEntity.none + this.currentEntityAddress = '' this.ruleViolations = [] this.stakeValidationResult = validationResult @@ -382,34 +422,32 @@ export class ERC7562Parser { } } - checkStorage (userOp: OperationBase, tracerResults: ERC7562Call): void { - Object.entries(tracerResults.accessedSlots).forEach(([address, accessInfo]) => { - this.checkStorageInternal(userOp, address, accessInfo) - }) - } - - checkStorageInternal ( + checkStorage ( userOp: OperationBase, - address: string, - accessInfo: AccessedSlots + tracerResults: ERC7562Call ): void { + if (tracerResults.to.toLowerCase() === this.entryPointAddress.toLowerCase()) { + // Currently inside the EntryPoint deposit code, no access control applies here + return + } const allSlots: string[] = [ - ...Object.keys(accessInfo.writes ?? {}), - ...Object.keys(accessInfo.reads ?? {}), - ...Object.keys(accessInfo.transientWrites ?? {}), - ...Object.keys(accessInfo.transientReads ?? {}) + ...Object.keys(tracerResults.accessedSlots.writes ?? {}), + ...Object.keys(tracerResults.accessedSlots.reads ?? {}), + ...Object.keys(tracerResults.accessedSlots.transientWrites ?? {}), + ...Object.keys(tracerResults.accessedSlots.transientReads ?? {}) ] - const entitySlots = {} // TODO: restore + const address = tracerResults.to + const entitySlots = this._parseEntitySlots(userOp) const addressName = this._tryGetAddressName(userOp, address) - const isEntityStaked = false // TODO - const isFactoryStaked = false // TODO - const isSenderCreation = false // TODO + const isEntityStaked = this._isEntityStaked() + const isFactoryStaked = this._isEntityStaked(AccountAbstractionEntity.factory) + const isSenderCreation = userOp.factory != null for (const slot of allSlots) { const isSenderInternalSTO010: boolean = address.toLowerCase() === userOp.sender.toLowerCase() const isSenderAssociated: boolean = this._associatedWith(slot, userOp.sender.toLowerCase(), entitySlots) const isEntityInternalSTO031: boolean = address.toLowerCase() === this.currentEntityAddress.toLowerCase() const isEntityAssociatedSTO032: boolean = this._associatedWith(slot, this.currentEntityAddress, entitySlots) - const isReadOnlyAccessSTO033: boolean = accessInfo.writes?.[slot] == null && accessInfo.transientWrites?.[slot] == null + const isReadOnlyAccessSTO033: boolean = tracerResults.accessedSlots.writes?.[slot] == null && tracerResults.accessedSlots.transientWrites?.[slot] == null const isAllowedIfEntityStaked = isEntityInternalSTO031 || isEntityAssociatedSTO032 || isReadOnlyAccessSTO033 const isAllowedST031ST032ST033: boolean = isAllowedIfEntityStaked && isEntityStaked @@ -426,8 +464,8 @@ export class ERC7562Parser { ) { description = `unstaked ${this.currentEntity.toString()} accessed ${addressName} slot ${slot}` } else { - const isWrite = Object.keys(accessInfo.writes ?? {}).includes(slot) || Object.keys(accessInfo.transientWrites ?? {}).includes(slot) - const isTransient = Object.keys(accessInfo.transientReads ?? {}).includes(slot) || Object.keys(accessInfo.transientWrites ?? {}).includes(slot) + const isWrite = Object.keys(tracerResults.accessedSlots.writes ?? {}).includes(slot) || Object.keys(tracerResults.accessedSlots.transientWrites ?? {}).includes(slot) + const isTransient = Object.keys(tracerResults.accessedSlots.transientReads ?? {}).includes(slot) || Object.keys(tracerResults.accessedSlots.transientWrites ?? {}).includes(slot) const readWrite = isWrite ? 'write to' : 'read from' const transientStr = isTransient ? 'transient ' : '' description = `${this.currentEntity.toString()} has forbidden ${readWrite} ${transientStr}${addressName} slot ${slot}` From 951758faa73f38cb45a9bbda80487e6794a0f3d8 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 20 Jan 2025 15:13:42 +0100 Subject: [PATCH 17/29] Fix --- packages/validation-manager/src/ERC7562Parser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index 96e12413..c7f6ce1c 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -446,7 +446,7 @@ export class ERC7562Parser { const isSenderInternalSTO010: boolean = address.toLowerCase() === userOp.sender.toLowerCase() const isSenderAssociated: boolean = this._associatedWith(slot, userOp.sender.toLowerCase(), entitySlots) const isEntityInternalSTO031: boolean = address.toLowerCase() === this.currentEntityAddress.toLowerCase() - const isEntityAssociatedSTO032: boolean = this._associatedWith(slot, this.currentEntityAddress, entitySlots) + const isEntityAssociatedSTO032: boolean = this._associatedWith(slot, this.currentEntityAddress.toLowerCase(), entitySlots) const isReadOnlyAccessSTO033: boolean = tracerResults.accessedSlots.writes?.[slot] == null && tracerResults.accessedSlots.transientWrites?.[slot] == null const isAllowedIfEntityStaked = isEntityInternalSTO031 || isEntityAssociatedSTO032 || isReadOnlyAccessSTO033 From 21bc4b0d3059aa2f3a95ec86c2b9c9dea357008b Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 20 Jan 2025 16:12:52 +0100 Subject: [PATCH 18/29] Clean up --- .../src/ERC7562BannedOpcodes.ts | 25 +- .../validation-manager/src/ERC7562Parser.ts | 284 +++++++++--------- 2 files changed, 160 insertions(+), 149 deletions(-) diff --git a/packages/validation-manager/src/ERC7562BannedOpcodes.ts b/packages/validation-manager/src/ERC7562BannedOpcodes.ts index 66ef6e54..e4a55c6d 100644 --- a/packages/validation-manager/src/ERC7562BannedOpcodes.ts +++ b/packages/validation-manager/src/ERC7562BannedOpcodes.ts @@ -1,21 +1,28 @@ +/** + * [OP-011] the opcodes banned for all entities. + */ export const bannedOpCodes = new Set( [ - 'GASPRICE', - 'GASLIMIT', - 'DIFFICULTY', - 'TIMESTAMP', 'BASEFEE', 'BLOCKHASH', + 'COINBASE', + 'DIFFICULTY', + // 'GAS', + 'GASLIMIT', + 'GASPRICE', + 'INVALID', 'NUMBER', 'ORIGIN', - 'COINBASE', - 'SELFDESTRUCT', - 'RANDOM', 'PREVRANDAO', - 'INVALID' + 'RANDOM', + 'SELFDESTRUCT', + 'TIMESTAMP' ] ) -// opcodes allowed in staked entities [OP-080] + +/** + * [OP-080] the opcodes allowed in staked entities. + */ export const opcodesOnlyInStakedEntities = new Set( [ 'BALANCE', diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index c7f6ce1c..e0d4aede 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -233,14 +233,14 @@ export class ERC7562Parser { ): ERC7562ValidationResults { const address = tracerResults.to this._detectEntityChange(userOp, tracerResults) - this.checkOp054(tracerResults) - this.checkOp054ExtCode(tracerResults, address, recursionDepth) - this.checkOp061(tracerResults) this.checkOp011(tracerResults) - this.checkOp080(tracerResults) this.checkOp020(tracerResults) this.checkOp031(userOp, tracerResults) this.checkOp041(userOp, tracerResults) + this.checkOp054(tracerResults) + this.checkOp054ExtCode(tracerResults, address, recursionDepth) + this.checkOp061(tracerResults) + this.checkOp080(tracerResults) this.checkStorage(userOp, tracerResults) for (const call of tracerResults.calls ?? []) { this._innerStepRecursive(userOp, call, recursionDepth + 1) @@ -250,6 +250,126 @@ export class ERC7562Parser { } } + /** + * OP-011: Blocked opcodes + * OP-080: `BALANCE` (0x31) and `SELFBALANCE` (0x47) are allowed only from a staked entity, else they are blocked + */ + checkOp011 (tracerResults: ERC7562Call): void { + const opcodes = tracerResults.usedOpcodes + const bannedOpCodeUsed = + Object + .keys(opcodes) + .map((opcode: string) => { + return getOpcodeName(parseInt(opcode)) ?? '' + }) + .filter((opcode: string) => { + return bannedOpCodes.has(opcode) + }) + bannedOpCodeUsed.forEach( + (opcode: string): void => { + this._violationDetected({ + rule: ERC7562Rule.op011, + // TODO: fill in depth, entity + depth: -1, + entity: this.currentEntity, + address: tracerResults.from, + opcode, + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `${this.currentEntity.toString()} uses banned opcode: ${opcode.toString()}` + }) + } + ) + } + + /** + * OP-020: Revert on "out of gas" is forbidden as it can "leak" the gas limit or the current call stack depth. + */ + checkOp020 (tracerResults: ERC7562Call): void { + if (tracerResults.outOfGas) { + this._violationDetected({ + rule: ERC7562Rule.op020, + // TODO: fill in depth, entity + depth: -1, + entity: this.currentEntity, + address: tracerResults.from, + opcode: tracerResults.type, + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `${this.currentEntity.toString()} internally reverts on oog` + }) + } + } + + /** + * OP-031: CREATE2 is allowed exactly once in the deployment phase and must deploy code for the "sender" address + */ + checkOp031 ( + userOp: OperationBase, + tracerResults: ERC7562Call + ): void { + if ( + tracerResults.type !== 'CREATE' && + tracerResults.type !== 'CREATE2' + ) { + return + } + const isFactoryStaked = this._isEntityStaked(AccountAbstractionEntity.factory) + const isAllowedCreateByOP032 = + userOp.factory != null && + tracerResults.type === 'CREATE' && + this.currentEntity === AccountAbstractionEntity.account && + tracerResults.from.toLowerCase() === userOp.sender.toLowerCase() + const isAllowedCreateByEREP060 = + ( + tracerResults.from.toLowerCase() === userOp.sender?.toLowerCase() || + tracerResults.from.toLowerCase() === userOp.factory?.toLowerCase() + ) && + isFactoryStaked + const isAllowedCreateSenderByFactory = + this.currentEntity === AccountAbstractionEntity.factory && + tracerResults.to.toLowerCase() === userOp.sender.toLowerCase() + if (!(isAllowedCreateByOP032 || isAllowedCreateByEREP060 || isAllowedCreateSenderByFactory)) { + this._violationDetected({ + rule: ERC7562Rule.op011, + // TODO: fill in depth, entity + depth: -1, + entity: this.currentEntity, + address: tracerResults.from ?? 'n/a', + opcode: 'CREATE2', + value: '0', + errorCode: ValidationErrors.OpcodeValidation, + description: `${this.currentEntity.toString()} uses banned opcode: CREATE2` + }) + } + } + + /** + * OP-041: Access to an address without a deployed code is forbidden for EXTCODE* and *CALL opcodes + */ + checkOp041 ( + userOp: OperationBase, + tracerResults: ERC7562Call + ): void { + // the only contract we allow to access before its deployment is the "sender" itself, which gets created. + let illegalZeroCodeAccess: any + for (const addr of Object.keys(tracerResults.contractSize)) { + // [OP-042] + if (addr.toLowerCase() !== userOp.sender.toLowerCase() && addr.toLowerCase() !== this.entryPointAddress.toLowerCase() && tracerResults.contractSize[addr].contractSize <= 2) { + illegalZeroCodeAccess = tracerResults.contractSize[addr] + illegalZeroCodeAccess.address = addr + this._violationDetected({ + address: '', + depth: 0, + entity: this.currentEntity, + rule: ERC7562Rule.op041, + errorCode: ValidationErrors.OpcodeValidation, + description: `${this.currentEntity.toString()} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}` + }) + } + } + } + /** * OP-052: May call `depositTo(sender)` with any value from either the `sender` or `factory`. * OP-053: May call the fallback function from the `sender` with any value. @@ -277,6 +397,25 @@ export class ERC7562Parser { } } + checkOp054ExtCode ( + tracerResults: ERC7562Call, + address: string, + recursionDepth: number + ): void { + for (const addr of tracerResults.extCodeAccessInfo) { + if (addr.toLowerCase() === this.entryPointAddress.toLowerCase()) { + this._violationDetected({ + address, + depth: recursionDepth, + entity: this.currentEntity, + errorCode: ValidationErrors.OpcodeValidation, + rule: ERC7562Rule.op054, + description: `${this.currentEntity} accesses EntryPoint contract address ${this.entryPointAddress} with EXTCODE* opcode` + }) + } + } + } + /** * OP-061: CALL with value is forbidden. The only exception is a call to the EntryPoint. */ @@ -300,56 +439,8 @@ export class ERC7562Parser { } /** - * OP-020: Revert on "out of gas" is forbidden as it can "leak" the gas limit or the current call stack depth. - */ - checkOp020 (tracerResults: ERC7562Call): void { - if (tracerResults.outOfGas) { - this._violationDetected({ - rule: ERC7562Rule.op020, - // TODO: fill in depth, entity - depth: -1, - entity: this.currentEntity, - address: tracerResults.from, - opcode: tracerResults.type, - value: '0', - errorCode: ValidationErrors.OpcodeValidation, - description: `${this.currentEntity.toString()} internally reverts on oog` - }) - } - } - - /** - * OP-011: Blocked opcodes - * OP-080: `BALANCE` (0x31) and `SELFBALANCE` (0x47) are allowed only from a staked entity, else they are blocked + * OP-080: BALANCE (0x31) and SELFBALANCE (0x47) are allowed only from a staked entity, else they are blocked */ - checkOp011 (tracerResults: ERC7562Call): void { - const opcodes = tracerResults.usedOpcodes - const bannedOpCodeUsed = - Object - .keys(opcodes) - .map((opcode: string) => { - return getOpcodeName(parseInt(opcode)) ?? '' - }) - .filter((opcode: string) => { - return bannedOpCodes.has(opcode) - }) - bannedOpCodeUsed.forEach( - (opcode: string): void => { - this._violationDetected({ - rule: ERC7562Rule.op011, - // TODO: fill in depth, entity - depth: -1, - entity: this.currentEntity, - address: tracerResults.from, - opcode, - value: '0', - errorCode: ValidationErrors.OpcodeValidation, - description: `${this.currentEntity.toString()} uses banned opcode: ${opcode.toString()}` - }) - } - ) - } - checkOp080 (tracerResults: ERC7562Call): void { const opcodes = tracerResults.usedOpcodes const isEntityStaked = this._isEntityStaked() @@ -380,48 +471,6 @@ export class ERC7562Parser { ) } - /** - * OP-031: CREATE2 is allowed exactly once in the deployment phase and must deploy code for the "sender" address - */ - checkOp031 ( - userOp: OperationBase, - tracerResults: ERC7562Call - ): void { - if ( - tracerResults.type !== 'CREATE' && - tracerResults.type !== 'CREATE2' - ) { - return - } - const isFactoryStaked = this._isEntityStaked(AccountAbstractionEntity.factory) - const isAllowedCreateByOP032 = - tracerResults.type === 'CREATE' && - this.currentEntity === AccountAbstractionEntity.account && - tracerResults.from === userOp.sender.toLowerCase() - const isAllowedCreateByEREP060 = - ( - tracerResults.from.toLowerCase() === userOp.sender?.toLowerCase() || - tracerResults.from.toLowerCase() === userOp.factory?.toLowerCase() - ) && - isFactoryStaked - const isAllowedCreateSenderByFactory = - this.currentEntity === AccountAbstractionEntity.factory && - tracerResults.to.toLowerCase() === userOp.sender.toLowerCase() - if (!(isAllowedCreateByOP032 || isAllowedCreateByEREP060 || isAllowedCreateSenderByFactory)) { - this._violationDetected({ - rule: ERC7562Rule.op011, - // TODO: fill in depth, entity - depth: -1, - entity: this.currentEntity, - address: tracerResults.from ?? 'n/a', - opcode: 'CREATE2', - value: '0', - errorCode: ValidationErrors.OpcodeValidation, - description: `${this.currentEntity.toString()} uses banned opcode: CREATE2` - }) - } - } - checkStorage ( userOp: OperationBase, tracerResults: ERC7562Call @@ -481,49 +530,4 @@ export class ERC7562Parser { } } } - - checkOp041 ( - userOp: OperationBase, - tracerResults: ERC7562Call - ): void { - // the only contract we allow to access before its deployment is the "sender" itself, which gets created. - let illegalZeroCodeAccess: any - for (const addr of Object.keys(tracerResults.contractSize)) { - // [OP-042] - if (addr.toLowerCase() !== userOp.sender.toLowerCase() && addr.toLowerCase() !== this.entryPointAddress.toLowerCase() && tracerResults.contractSize[addr].contractSize <= 2) { - illegalZeroCodeAccess = tracerResults.contractSize[addr] - illegalZeroCodeAccess.address = addr - this._violationDetected({ - address: '', - depth: 0, - entity: this.currentEntity, - rule: ERC7562Rule.op041, - errorCode: ValidationErrors.OpcodeValidation, - description: `${this.currentEntity.toString()} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}` - }) - } - } - } - - checkOp054ExtCode ( - tracerResults: ERC7562Call, - address: string, - recursionDepth: number - ): void { - const entityTitle = 'fixme' - let illegalEntryPointCodeAccess - for (const addr of Object.keys(tracerResults.extCodeAccessInfo)) { - if (addr.toLowerCase() === this.entryPointAddress.toLowerCase()) { - illegalEntryPointCodeAccess = tracerResults.extCodeAccessInfo - this._violationDetected({ - address, - depth: recursionDepth, - entity: this.currentEntity, - errorCode: ValidationErrors.OpcodeValidation, - rule: ERC7562Rule.op054, - description: `${entityTitle} accesses EntryPoint contract address ${this.entryPointAddress} with opcode $ {'todo'}` - }) - } - } - } } From bb0fadb0f22e88ce7abf8fa089f4ac9515793f3a Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 20 Jan 2025 16:13:58 +0100 Subject: [PATCH 19/29] Undo removing the old parser --- .../src/TracerResultParser.ts | 394 ++++++++++++++++++ 1 file changed, 394 insertions(+) create mode 100644 packages/validation-manager/src/TracerResultParser.ts diff --git a/packages/validation-manager/src/TracerResultParser.ts b/packages/validation-manager/src/TracerResultParser.ts new file mode 100644 index 00000000..7d157b43 --- /dev/null +++ b/packages/validation-manager/src/TracerResultParser.ts @@ -0,0 +1,394 @@ +// This file contains references to validation rules, in the format [xxx-###] +// where xxx is OP/STO/COD/EP/SREP/EREP/UREP/ALT, and ### is a number +// the validation rules are defined in erc-aa-validation.md +import Debug from 'debug' +import { BigNumber } from 'ethers' +import { hexZeroPad, keccak256 } from 'ethers/lib/utils' +import { inspect } from 'util' + +import { AccessInfo, BundlerTracerResult, MethodInfo, TopLevelCallInfo } from './BundlerCollectorTracer' +import { + OperationBase, + RpcError, + StakeInfo, + StorageMap, + ValidationErrors, + mapOf, + requireCond, + toBytes32, AddressZero, UserOperation +} from '@account-abstraction/utils' + +import { ValidationResult } from './IValidationManager' + +const debug = Debug('aa.handler.opcodes') + +/** + * slots associated with each entity. + * keccak( A || ...) is associated with "A" + * removed rule: keccak( ... || ASSOC ) (for a previously associated hash) is also associated with "A" + * + * @param stakeInfoEntities stake info for (factory, account, paymaster). factory and paymaster can be null. + * @param keccak array of buffers that were given to keccak in the transaction + */ +function parseEntitySlots (stakeInfoEntities: { [addr: string]: StakeInfo | undefined }, keccak: string[]): { + [addr: string]: Set +} { + // for each entity (sender, factory, paymaster), hold the valid slot addresses + // valid: the slot was generated by keccak(entity || ...) + const entitySlots: { [addr: string]: Set } = {} + + keccak.forEach(k => { + Object.values(stakeInfoEntities).forEach(info => { + const addr = info?.addr?.toLowerCase() + if (addr == null) return + const addrPadded = toBytes32(addr) + if (entitySlots[addr] == null) { + entitySlots[addr] = new Set() + } + + const currentEntitySlots = entitySlots[addr] + + // valid slot: the slot was generated by keccak(entityAddr || ...) + if (k.startsWith(addrPadded)) { + // console.log('added mapping (balance) slot', value) + currentEntitySlots.add(keccak256(k)) + } + // disabled 2nd rule: .. or by keccak( ... || OWN) where OWN is previous allowed slot + // if (k.length === 130 && currentEntitySlots.has(k.slice(-64))) { + // // console.log('added double-mapping (allowance) slot', value) + // currentEntitySlots.add(value) + // } + }) + }) + + return entitySlots +} + +// +// // method-signature for calls from entryPoint +// const callsFromEntryPointMethodSigs1: { [key: string]: string } = { +// factory: SenderCreator__factory.createInterface().getSighash('createSender'), +// account: IAccount__factory.createInterface().getSighash('validateUserOp'), +// paymaster: IPaymaster__factory.createInterface().getSighash('validatePaymasterUserOp') +// } +// +// // todo: use a selector for factory or refactor the tracer for RIP-7560 +// const callsFromEntryPointMethodSigs: { [key: string]: string } = { +// account: '0xbf45c166', +// paymaster: '0xe0e6183a', +// } + +function getEntityTitle (userOp: OperationBase, entityAddress: string): string { + if (userOp.sender.toLowerCase() === entityAddress.toLowerCase()) { + return 'account' + } else if (userOp.factory?.toLowerCase() === entityAddress.toLowerCase()) { + return 'factory' + } else if (userOp.paymaster?.toLowerCase() === entityAddress.toLowerCase()) { + return 'paymaster' + } else { + throw new RpcError(`could not find entity name for address ${entityAddress}. This should not happen. This is a bug.`, 0) + } +} + +// opcodes from [OP-011] +// (CREATE, CREATE2 have special rules, OP-031, OP-032) +const bannedOpCodes = new Set(['GASPRICE', 'GASLIMIT', 'DIFFICULTY', 'TIMESTAMP', 'BASEFEE', 'BLOCKHASH', 'NUMBER', 'ORIGIN', 'GAS', 'COINBASE', 'SELFDESTRUCT', 'RANDOM', 'PREVRANDAO', 'INVALID']) +// opcodes allowed in staked entities [OP-080] +const opcodesOnlyInStakedEntities = new Set(['BALANCE', 'SELFBALANCE']) + +/** + * parse collected simulation traces and revert if they break our rules + * @param userOp the userOperation that was used in this simulation + * @param tracerResults the tracer return value + * @param validationResult output from simulateValidation + * @param entryPointAddress the entryPoint that hosted the "simulatedValidation" traced call. + * @return list of contract addresses referenced by this UserOp + */ +export function tracerResultParser ( + userOp: OperationBase, + tracerResults: BundlerTracerResult, + validationResult: ValidationResult, + entryPointAddress: string +): [string[], StorageMap] { + debug('=== simulation result:', inspect(tracerResults, true, 10, true)) + // todo: block access to no-code addresses (might need update to tracer) + + // eslint-disable-next-line @typescript-eslint/no-base-to-string + if (Object.values(tracerResults.callsFromEntryPoint).length < 1) { + throw new Error('Unexpected traceCall result: no calls from entrypoint.') + } + if (tracerResults.calls != null) { + const callStack = tracerResults.calls.filter((call: any) => call.topLevelTargetAddress == null) as MethodInfo[] + // [OP-052], [OP-053] + const callInfoEntryPoint = callStack.find(call => + call.to?.toLowerCase() === entryPointAddress?.toLowerCase() && call.from?.toLowerCase() !== entryPointAddress?.toLowerCase() && + (call.method !== '0x' && call.method !== 'depositTo')) + // [OP-054] + requireCond(callInfoEntryPoint == null, + `illegal call into EntryPoint during validation ${callInfoEntryPoint?.method}`, + ValidationErrors.OpcodeValidation + ) + + // [OP-061] + const illegalNonZeroValueCall = callStack.find( + call => + call.to?.toLowerCase() !== entryPointAddress?.toLowerCase() && + !BigNumber.from(call.value ?? 0).eq(0)) + requireCond( + illegalNonZeroValueCall == null, + 'May not may CALL with value', + ValidationErrors.OpcodeValidation) + } + + const sender = userOp.sender.toLowerCase() + // stake info per "number" level (factory, sender, paymaster) + // we only use stake info if we notice a memory reference that require stake + const stakeInfoEntities = { + [sender]: validationResult.senderInfo + } + const factory = userOp.factory?.toLowerCase() + if (factory != null) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + stakeInfoEntities[factory] = validationResult.factoryInfo! + } + const paymaster = userOp.paymaster?.toLowerCase() + if (paymaster != null) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + stakeInfoEntities[paymaster] = validationResult.paymasterInfo! + } + + const entitySlots: { [addr: string]: Set } = parseEntitySlots(stakeInfoEntities, tracerResults.keccak) + Object.entries(stakeInfoEntities).forEach(([entityAddress, entStakes]) => { + const entityTitle = getEntityTitle(userOp, entityAddress) + const entityCallsFromEntryPoint = tracerResults.callsFromEntryPoint.filter( + call => call.topLevelTargetAddress != null && call.topLevelTargetAddress.toLowerCase() === entityAddress.toLowerCase()) + entityCallsFromEntryPoint.forEach((entityCall) => { + if (entityCall == null) { + if (entityAddress.toLowerCase() === userOp.sender.toLowerCase()) { + // should never happen... only factory, paymaster are optional. + throw new RpcError('missing trace into account validation', ValidationErrors.InvalidFields) + } + return + } + processEntityCall(entityCall, entityAddress, entityTitle, entStakes, entitySlots, userOp as UserOperation, stakeInfoEntities, entryPointAddress, tracerResults) + }) + }) + // return list of contract addresses by this UserOp. already known not to contain zero-sized addresses. + const addresses = tracerResults.callsFromEntryPoint.flatMap(level => Object.keys(level.contractSize)) + const storageMap: StorageMap = {} + tracerResults.callsFromEntryPoint.forEach(level => { + Object.keys(level.access).forEach(addr => { + storageMap[addr] = storageMap[addr] ?? level.access[addr].reads + }) + }) + return [addresses, storageMap] +} + +function processEntityCall (entityCall: TopLevelCallInfo, entityAddress: string, entityTitle: string, entStakes: StakeInfo, entitySlots: { [addr: string]: Set }, userOp: UserOperation, stakeInfoEntities: {[addr: string]: StakeInfo}, entryPointAddress: string, tracerResults: BundlerTracerResult, depth = 0): void { + const opcodes = entityCall.opcodes + const access = entityCall.access + + // [OP-020] + requireCond(!(entityCall.oog ?? false), + `${entityTitle} internally reverts on oog`, ValidationErrors.OpcodeValidation) + + // opcodes from [OP-011] + Object.keys(opcodes).forEach(opcode => { + requireCond(!bannedOpCodes.has(opcode), `${entityTitle} uses banned opcode: ${opcode}`, ValidationErrors.OpcodeValidation) + // [OP-080] + requireCond(!opcodesOnlyInStakedEntities.has(opcode) || isStaked(entStakes), + `unstaked ${entityTitle} uses banned opcode: ${opcode}`, ValidationErrors.OpcodeValidation) + }) + + if (entityCall.type === 'CREATE') { + // CREATE is allowed only from the entity (factory, account) itself, not from inner contract + // top-level is "handleOps" (or simulateValidation) itself + requireCond(depth === 1, + `${entityTitle} uses banned opcode: CREATE from inner call depth=${depth}`, ValidationErrors.OpcodeValidation) + + // [OP-032] If there is a `factory` (even unstaked), the `sender` contract is allowed to use `CREATE` opcode + requireCond( + (entityTitle === 'account' && userOp.factory != null) || + (entityTitle === 'factory' && isStaked(entStakes)), + `${entityTitle} uses banned opcode: CREATE`, ValidationErrors.OpcodeValidation + ) + } + if (entityCall.type === 'CREATE2') { + const factory = userOp.factory?.toLowerCase() ?? '' + const factoryStaked = isStaked(stakeInfoEntities[factory]) + console.log(`=== CREATE2: title:${entityTitle} factory-staked:${factoryStaked} callfromsender:${entityCall.from === userOp.sender.toLowerCase()}`) + console.log({ + stakeInfoEntities, + userOp + }) + requireCond( + (entityTitle === 'account' && factoryStaked && entityCall.from === userOp.sender.toLowerCase()) || // OP-032 + (entityTitle === 'factory' && ( + entityCall.to === userOp.sender.toLowerCase() || + (entityCall.from === userOp.factory && isStaked(entStakes)) + )), + `${entityTitle} uses banned opcode: CREATE2`, ValidationErrors.OpcodeValidation + ) + } + + Object.entries(access).forEach(([addr, { + reads, + writes, + transientReads, + transientWrites + }]) => { + // testing read/write access on contract "addr" + if (addr.toLowerCase() === userOp.sender.toLowerCase()) { + // allowed to access sender's storage + // [STO-010] + return + } + + if (addr.toLowerCase() === entryPointAddress.toLowerCase()) { + // ignore storage access on entryPoint (balance/deposit of entities. + // we block them on method calls: only allowed to deposit, never to read + return + } + + debug('dump keccak calculations and reads', { + entityTitle, + entityAddress, + k: mapOf(tracerResults.keccak, k => keccak256(k)), + reads + }) + + // [OP-070]: treat transient storage (TLOAD/TSTORE) just like storage. + // scan all slots. find a referenced slot + // at the end of the scan, we will check if the entity has stake, and report that slot if not. + let requireStakeSlot: string | undefined + [ + ...Object.keys(writes), + ...Object.keys(reads), + ...Object.keys(transientWrites ?? {}), + ...Object.keys(transientReads ?? {}) + ].forEach(slot => { + // slot associated with sender is allowed (e.g. token.balanceOf(sender) + // but during initial UserOp (where there is an initCode), it is allowed only for staked entity + if (associatedWith(slot, userOp.sender.toLowerCase(), entitySlots)) { + if (userOp.factory != null && userOp.factory !== AddressZero) { + // special case: account.validateUserOp is allowed to use assoc storage if factory is staked. + // [STO-022], [STO-021] + if (!(entityAddress.toLowerCase() === userOp.sender.toLowerCase() && isStaked(stakeInfoEntities[userOp.factory.toLowerCase()]))) { + requireStakeSlot = slot + } + } + } else if (associatedWith(slot, entityAddress, entitySlots)) { + // [STO-032] + // accessing a slot associated with entityAddr (e.g. token.balanceOf(paymaster) + requireStakeSlot = slot + } else if (addr.toLowerCase() === entityAddress.toLowerCase()) { + // [STO-031] + // accessing storage member of entity itself requires stake. + requireStakeSlot = slot + } else if (writes[slot] == null && transientWrites[slot] == null) { + // [STO-033]: staked entity have read-only access to any storage in non-entity contract. + requireStakeSlot = slot + } else { + // accessing arbitrary storage of another contract is not allowed + const isWrite = Object.keys(writes).includes(slot) || Object.keys(transientWrites ?? {}).includes(slot) + const isTransient = Object.keys(transientReads ?? {}).includes(slot) || Object.keys(transientWrites ?? {}).includes(slot) + const readWrite = isWrite ? 'write to' : 'read from' + const transientStr = isTransient ? 'transient ' : '' + requireCond(false, + `${entityTitle} has forbidden ${readWrite} ${transientStr}${nameAddr(addr, stakeInfoEntities)} slot ${slot}`, + ValidationErrors.OpcodeValidation, { [entityTitle]: entStakes?.addr }) + } + }) + + requireCondAndStake(requireStakeSlot != null, entStakes, + `unstaked ${entityTitle} accessed ${nameAddr(addr, stakeInfoEntities)} slot ${requireStakeSlot}`, entityTitle, access) + }) + + // the only contract we allow to access before its deployment is the "sender" itself, which gets created. + let illegalZeroCodeAccess: any + for (const addr of Object.keys(entityCall.contractSize)) { + // [OP-042] + if (addr.toLowerCase() !== userOp.sender.toLowerCase() && addr.toLowerCase() !== entryPointAddress.toLowerCase() && entityCall.contractSize[addr].contractSize <= 2) { + illegalZeroCodeAccess = entityCall.contractSize[addr] + illegalZeroCodeAccess.address = addr + break + } + } + // [OP-041] + requireCond( + illegalZeroCodeAccess == null, + `${entityTitle} accesses un-deployed contract address ${illegalZeroCodeAccess?.address as string} with opcode ${illegalZeroCodeAccess?.opcode as string}`, + ValidationErrors.OpcodeValidation) + + let illegalEntryPointCodeAccess + for (const addr of Object.keys(entityCall.extCodeAccessInfo)) { + if (addr.toLowerCase() === entryPointAddress.toLowerCase()) { + illegalEntryPointCodeAccess = entityCall.extCodeAccessInfo[addr] + break + } + } + requireCond( + illegalEntryPointCodeAccess == null, + `${entityTitle} accesses EntryPoint contract address ${entryPointAddress} with opcode ${illegalEntryPointCodeAccess}`, + ValidationErrors.OpcodeValidation) + + // Recursively handling all subcalls to check validation rules + if (entityCall.calls != null) { + entityCall.calls.forEach((call: any) => { + processEntityCall(call, entityAddress, entityTitle, entStakes, entitySlots, userOp, stakeInfoEntities, entryPointAddress, tracerResults, depth + 1) + }) + } +} + +// helper method: if condition is true, then entity must be staked. +function requireCondAndStake (cond: boolean, entStake: StakeInfo | undefined, failureMessage: string, entityTitle: string, access: { [address: string]: AccessInfo }): void { + if (!cond) { + return + } + if (entStake == null) { + throw new Error(`internal: ${entityTitle} not in userOp, but has storage accesses in ${JSON.stringify(access)}`) + } + requireCond(isStaked(entStake), + failureMessage, ValidationErrors.OpcodeValidation, { [entityTitle]: entStake?.addr }) + + // TODO: check real minimum stake values +} + +// check if the given entity is staked +function isStaked (entStake?: StakeInfo): boolean { + return entStake != null && BigNumber.from(1).lte(entStake.stake) && BigNumber.from(1).lte(entStake.unstakeDelaySec) +} + +// if addr is current account/paymaster/factory, then return that title +// otherwise, return addr as-is +function nameAddr (addr: string, stakeInfoEntities: {[addr: string]: StakeInfo}): string { + const [title] = Object.entries(stakeInfoEntities).find(([title, info]) => + info?.addr.toLowerCase() === addr.toLowerCase()) ?? [] + + return title ?? addr +} + +// return true if the given slot is associated with the given address, given the known keccak operations: +// @param slot the SLOAD/SSTORE slot address we're testing +// @param addr - the address we try to check for association with +// @param reverseKeccak - a mapping we built for keccak values that contained the address +function associatedWith (slot: string, addr: string, entitySlots: { [addr: string]: Set }): boolean { + const addrPadded = hexZeroPad(addr, 32).toLowerCase() + if (slot === addrPadded) { + return true + } + const k = entitySlots[addr] + if (k == null) { + return false + } + const slotN = BigNumber.from(slot) + // scan all slot entries to check of the given slot is within a structure, starting at that offset. + // assume a maximum size on a (static) structure size. + for (const k1 of k.keys()) { + const kn = BigNumber.from(k1) + if (slotN.gte(kn) && slotN.lt(kn.add(128))) { + return true + } + } + return false +} From 2310bdc27c9e3159f0141ded7475e8faad366214 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 20 Jan 2025 17:31:21 +0100 Subject: [PATCH 20/29] Skip opcode rules check for the code within EntryPoint --- packages/validation-manager/src/ERC7562BannedOpcodes.ts | 2 +- packages/validation-manager/src/ERC7562Parser.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/validation-manager/src/ERC7562BannedOpcodes.ts b/packages/validation-manager/src/ERC7562BannedOpcodes.ts index e4a55c6d..eb0c4bf9 100644 --- a/packages/validation-manager/src/ERC7562BannedOpcodes.ts +++ b/packages/validation-manager/src/ERC7562BannedOpcodes.ts @@ -7,7 +7,7 @@ export const bannedOpCodes = new Set( 'BLOCKHASH', 'COINBASE', 'DIFFICULTY', - // 'GAS', + 'GAS', 'GASLIMIT', 'GASPRICE', 'INVALID', diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index e0d4aede..205ab649 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -255,6 +255,10 @@ export class ERC7562Parser { * OP-080: `BALANCE` (0x31) and `SELFBALANCE` (0x47) are allowed only from a staked entity, else they are blocked */ checkOp011 (tracerResults: ERC7562Call): void { + if (tracerResults.to.toLowerCase() === this.entryPointAddress.toLowerCase()) { + // Currently inside the EntryPoint deposit code, no access control applies here + return + } const opcodes = tracerResults.usedOpcodes const bannedOpCodeUsed = Object From f295fe574d2fb25c61bf03a76c6bc5db85a00e54 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 20 Jan 2025 17:31:53 +0100 Subject: [PATCH 21/29] Fix typo --- packages/validation-manager/src/ERC7562Parser.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index 205ab649..863e0f2c 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -30,7 +30,7 @@ export interface ERC7562ValidationResults { } export class ERC7562Parser { - private keccack: string[] = [] + private keccak: string[] = [] private ruleViolations: ERC7562Violation[] = [] private currentEntity: AccountAbstractionEntity = AccountAbstractionEntity.none private currentEntityAddress: string = '' @@ -174,7 +174,7 @@ export class ERC7562Parser { const entityAddresses = [userOp.sender.toLowerCase(), userOp.paymaster?.toLowerCase(), userOp.factory?.toLowerCase()] const entitySlots: { [addr: string]: Set } = {} - for (const keccakInput of this.keccack) { + for (const keccakInput of this.keccak) { for (const entityAddress of entityAddresses) { if (entityAddress == null) { continue @@ -217,7 +217,7 @@ export class ERC7562Parser { throw new Error('Unexpected traceCall result: no calls from entrypoint.') } - this.keccack = tracerResults.keccak ?? [] + this.keccak = tracerResults.keccak ?? [] this.currentEntity = AccountAbstractionEntity.none this.currentEntityAddress = '' this.ruleViolations = [] From 5f2c358ecfea4e652f9552112e5070962f40fed6 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Mon, 20 Jan 2025 21:08:14 +0100 Subject: [PATCH 22/29] Fix not returning 'storageMap' and 'contractAddress' results --- .../bundler/src/modules/ExecutionManager.ts | 2 +- packages/bundler/src/modules/initServer.ts | 2 +- packages/bundler/test/BundlerManager.test.ts | 4 +- packages/bundler/test/BundlerServer.test.ts | 2 +- .../bundler/test/DebugMethodHandler.test.ts | 2 +- .../bundler/test/UserOpMethodHandler.test.ts | 2 +- packages/bundler/test/ValidateManager.test.ts | 2 +- .../validation-manager/src/ERC7562Parser.ts | 49 ++++++++++++------- packages/validation-manager/src/index.ts | 2 +- 9 files changed, 39 insertions(+), 28 deletions(-) diff --git a/packages/bundler/src/modules/ExecutionManager.ts b/packages/bundler/src/modules/ExecutionManager.ts index 6052c0d0..d0bb0582 100644 --- a/packages/bundler/src/modules/ExecutionManager.ts +++ b/packages/bundler/src/modules/ExecutionManager.ts @@ -150,7 +150,7 @@ export class ExecutionManager { const { configuration, entryPoint, unsafe } = this.validationManager._getDebugConfiguration() const mergedConfiguration = Object.assign({}, configuration, configOverrides) const pvgc = new PreVerificationGasCalculator(mergedConfiguration) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, mergedConfiguration.senderCreator ?? '', true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, mergedConfiguration.senderCreator ?? '', true) this.validationManager = new ValidationManager( entryPoint, unsafe, diff --git a/packages/bundler/src/modules/initServer.ts b/packages/bundler/src/modules/initServer.ts index cbe9678f..2078edba 100644 --- a/packages/bundler/src/modules/initServer.ts +++ b/packages/bundler/src/modules/initServer.ts @@ -36,7 +36,7 @@ export function initServer (config: BundlerConfig, signer: Signer): [ExecutionMa const eventsManager = new EventsManager(entryPoint, mempoolManager, reputationManager) const mergedPvgcConfig = Object.assign({}, ChainConfigs[config.chainId] ?? {}, config) const preVerificationGasCalculator = new PreVerificationGasCalculator(mergedPvgcConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) let validationManager: IValidationManager let bundleManager: IBundleManager if (!config.rip7560) { diff --git a/packages/bundler/test/BundlerManager.test.ts b/packages/bundler/test/BundlerManager.test.ts index 65759fa1..415f3140 100644 --- a/packages/bundler/test/BundlerManager.test.ts +++ b/packages/bundler/test/BundlerManager.test.ts @@ -65,7 +65,7 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) bm = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc) @@ -123,7 +123,7 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(_entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(_entryPoint, mempoolMgr, repMgr) bundleMgr = new BundleManager(_entryPoint, _entryPoint.provider as JsonRpcProvider, _entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/BundlerServer.test.ts b/packages/bundler/test/BundlerServer.test.ts index 4d5dc48c..b0ce30d8 100644 --- a/packages/bundler/test/BundlerServer.test.ts +++ b/packages/bundler/test/BundlerServer.test.ts @@ -61,7 +61,7 @@ describe('BundleServer', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/DebugMethodHandler.test.ts b/packages/bundler/test/DebugMethodHandler.test.ts index 4fd3687e..463b36d1 100644 --- a/packages/bundler/test/DebugMethodHandler.test.ts +++ b/packages/bundler/test/DebugMethodHandler.test.ts @@ -71,7 +71,7 @@ describe('#DebugMethodHandler', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const eventsManager = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, eventsManager, mempoolMgr, validMgr, repMgr, diff --git a/packages/bundler/test/UserOpMethodHandler.test.ts b/packages/bundler/test/UserOpMethodHandler.test.ts index 2052aa5b..a2509e82 100644 --- a/packages/bundler/test/UserOpMethodHandler.test.ts +++ b/packages/bundler/test/UserOpMethodHandler.test.ts @@ -89,7 +89,7 @@ describe('UserOpMethodHandler', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/ValidateManager.test.ts b/packages/bundler/test/ValidateManager.test.ts index 35021181..a737334b 100644 --- a/packages/bundler/test/ValidateManager.test.ts +++ b/packages/bundler/test/ValidateManager.test.ts @@ -157,7 +157,7 @@ describe('#ValidationManager', () => { const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) const senderCreator = '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c' - const erc7562Parser = new ERC7562Parser({}, entryPoint.address, senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, senderCreator, true) vm = new ValidationManager(entryPoint, unsafe, preVerificationGasCalculator, erc7562Parser) if (!await supportsDebugTraceCall(ethers.provider, false)) { diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index 863e0f2c..0e32042d 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -8,10 +8,11 @@ import { OperationBase, RpcError, SenderCreator__factory, + SlotMap, StakeInfo, StorageMap, - toBytes32, - ValidationErrors + ValidationErrors, + toBytes32 } from '@account-abstraction/utils' import { ERC7562Violation, toError } from './ERC7562Violation' @@ -20,7 +21,6 @@ import { AccountAbstractionEntity } from './AccountAbstractionEntity' import { bannedOpCodes, opcodesOnlyInStakedEntities } from './ERC7562BannedOpcodes' import { ValidationResult } from './IValidationManager' import { ERC7562Call } from './ERC7562Call' -import { AltMempoolConfig } from './altmempool/AltMempoolConfig' import { getOpcodeName } from './enum/EVMOpcodes' export interface ERC7562ValidationResults { @@ -36,13 +36,22 @@ export class ERC7562Parser { private currentEntityAddress: string = '' private stakeValidationResult!: ValidationResult + private contractAddresses: string[] = [] + private storageMap: StorageMap = {} + constructor ( - readonly mempoolConfig: AltMempoolConfig, readonly entryPointAddress: string, readonly senderCreatorAddress: string, readonly bailOnViolation: boolean - ) { + ) {} + private _init (erc7562Call: ERC7562Call) { + this.keccak = erc7562Call.keccak ?? [] + this.ruleViolations = [] + this.currentEntity = AccountAbstractionEntity.none + this.currentEntityAddress = '' + this.contractAddresses = [] + this.storageMap = {} } private _isCallToEntryPoint (call: ERC7562Call): boolean { @@ -210,28 +219,29 @@ export class ERC7562Parser { parseResults ( userOp: OperationBase, - tracerResults: ERC7562Call, + erc7562Call: ERC7562Call, validationResult: ValidationResult ): ERC7562ValidationResults { - if (tracerResults.calls == null || tracerResults.calls.length < 1) { + this._init(erc7562Call) + if (erc7562Call.calls == null || erc7562Call.calls.length < 1) { throw new Error('Unexpected traceCall result: no calls from entrypoint.') } - - this.keccak = tracerResults.keccak ?? [] - this.currentEntity = AccountAbstractionEntity.none - this.currentEntityAddress = '' - this.ruleViolations = [] this.stakeValidationResult = validationResult - - return this._innerStepRecursive(userOp, tracerResults, 0) + this._innerStepRecursive(userOp, erc7562Call, 0) + return { + contractAddresses: this.contractAddresses, + ruleViolations: this.ruleViolations, + storageMap: this.storageMap + } } private _innerStepRecursive ( userOp: OperationBase, tracerResults: ERC7562Call, recursionDepth: number - ): ERC7562ValidationResults { + ): void { const address = tracerResults.to + this.contractAddresses.push(address) this._detectEntityChange(userOp, tracerResults) this.checkOp011(tracerResults) this.checkOp020(tracerResults) @@ -245,9 +255,6 @@ export class ERC7562Parser { for (const call of tracerResults.calls ?? []) { this._innerStepRecursive(userOp, call, recursionDepth + 1) } - return { - contractAddresses: [], ruleViolations: this.ruleViolations, storageMap: {} - } } /** @@ -489,13 +496,17 @@ export class ERC7562Parser { ...Object.keys(tracerResults.accessedSlots.transientWrites ?? {}), ...Object.keys(tracerResults.accessedSlots.transientReads ?? {}) ] - const address = tracerResults.to + const address: string = tracerResults.to const entitySlots = this._parseEntitySlots(userOp) const addressName = this._tryGetAddressName(userOp, address) const isEntityStaked = this._isEntityStaked() const isFactoryStaked = this._isEntityStaked(AccountAbstractionEntity.factory) const isSenderCreation = userOp.factory != null for (const slot of allSlots) { + if (this.storageMap[address] == null) { + this.storageMap[address] = {} + } + (this.storageMap[address] as SlotMap)[slot] = '' // TODO: not clear why were the values relevant const isSenderInternalSTO010: boolean = address.toLowerCase() === userOp.sender.toLowerCase() const isSenderAssociated: boolean = this._associatedWith(slot, userOp.sender.toLowerCase(), entitySlots) const isEntityInternalSTO031: boolean = address.toLowerCase() === this.currentEntityAddress.toLowerCase() diff --git a/packages/validation-manager/src/index.ts b/packages/validation-manager/src/index.ts index 9ec1a173..d45fa5a6 100644 --- a/packages/validation-manager/src/index.ts +++ b/packages/validation-manager/src/index.ts @@ -87,7 +87,7 @@ export async function checkRulesViolations ( } const entryPoint = IEntryPoint__factory.connect(entryPointAddress, provider) const senderCreator = '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c' - const erc7562Parser = new ERC7562Parser({}, entryPointAddress, senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPointAddress, senderCreator, true) const validationManager = new ValidationManager( entryPoint, false, From 646c6869bdc1b2b8e9e339368012d5decbf80cec Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 21 Jan 2025 01:46:39 +0100 Subject: [PATCH 23/29] WIP: Add support for RIP-7560 transaction type --- packages/bundler/src/modules/initServer.ts | 7 +- .../src/AccountAbstractionEntity.ts | 1 + .../validation-manager/src/ERC7562Parser.ts | 233 +++++++++++++++++- packages/validation-manager/src/index.ts | 37 +-- 4 files changed, 240 insertions(+), 38 deletions(-) diff --git a/packages/bundler/src/modules/initServer.ts b/packages/bundler/src/modules/initServer.ts index 2078edba..a40ac3fb 100644 --- a/packages/bundler/src/modules/initServer.ts +++ b/packages/bundler/src/modules/initServer.ts @@ -9,9 +9,11 @@ import { BundlerReputationParams, ReputationManager } from './ReputationManager' import { MempoolManager } from './MempoolManager' import { BundleManager } from './BundleManager' import { + AA_ENTRY_POINT, + AA_STAKE_MANAGER, + IValidationManager, ValidationManager, ValidationManagerRIP7560, - IValidationManager, AA_STAKE_MANAGER } from '@account-abstraction/validation-manager' import { BundlerConfig } from '../BundlerConfig' import { EventsManager } from './EventsManager' @@ -36,15 +38,16 @@ export function initServer (config: BundlerConfig, signer: Signer): [ExecutionMa const eventsManager = new EventsManager(entryPoint, mempoolManager, reputationManager) const mergedPvgcConfig = Object.assign({}, ChainConfigs[config.chainId] ?? {}, config) const preVerificationGasCalculator = new PreVerificationGasCalculator(mergedPvgcConfig) - const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) let validationManager: IValidationManager let bundleManager: IBundleManager if (!config.rip7560) { + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) const tracerProvider = config.tracerRpcUrl == null ? undefined : getNetworkProvider(config.tracerRpcUrl) validationManager = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser, tracerProvider) bundleManager = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, signer, eventsManager, mempoolManager, validationManager, reputationManager, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc) } else { + const erc7562Parser = new ERC7562Parser(AA_ENTRY_POINT, config.senderCreator, true) const stakeManager = IRip7560StakeManager__factory.connect(AA_STAKE_MANAGER, signer) validationManager = new ValidationManagerRIP7560(stakeManager, entryPoint.provider as JsonRpcProvider, erc7562Parser, config.unsafe) bundleManager = new BundleManagerRIP7560(entryPoint.provider as JsonRpcProvider, signer, eventsManager, mempoolManager, validationManager, reputationManager, diff --git a/packages/validation-manager/src/AccountAbstractionEntity.ts b/packages/validation-manager/src/AccountAbstractionEntity.ts index e5fe6280..d50d2b1e 100644 --- a/packages/validation-manager/src/AccountAbstractionEntity.ts +++ b/packages/validation-manager/src/AccountAbstractionEntity.ts @@ -5,5 +5,6 @@ export enum AccountAbstractionEntity { aggregator = 'aggregator', senderCreator = 'SenderCreator', entryPoint = 'EntryPoint', + nativeEntryPoint = 'NativeEntryPoint', none = 'none' } diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index 0e32042d..c801be30 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -23,6 +23,222 @@ import { ValidationResult } from './IValidationManager' import { ERC7562Call } from './ERC7562Call' import { getOpcodeName } from './enum/EVMOpcodes' +// TODO: Use artifact from the submodule +const RIP7560EntryPointABI = [ + { + anonymous: false, + inputs: [ + { + indexed: true, + internalType: 'address', + name: 'sender', + type: 'address' + }, + { + indexed: true, + internalType: 'address', + name: 'paymaster', + type: 'address' + }, + { + indexed: true, + internalType: 'address', + name: 'deployer', + type: 'address' + } + ], + name: 'RIP7560AccountDeployed', + type: 'event' + }, + { + anonymous: false, + inputs: [ + { + indexed: true, + internalType: 'address', + name: 'sender', + type: 'address' + }, + { + indexed: true, + internalType: 'address', + name: 'paymaster', + type: 'address' + }, + { + indexed: false, + internalType: 'uint256', + name: 'nonceKey', + type: 'uint256' + }, + { + indexed: false, + internalType: 'uint256', + name: 'nonceSequence', + type: 'uint256' + }, + { + indexed: false, + internalType: 'uint256', + name: 'executionStatus', + type: 'uint256' + } + ], + name: 'RIP7560TransactionEvent', + type: 'event' + }, + { + anonymous: false, + inputs: [ + { + indexed: true, + internalType: 'address', + name: 'sender', + type: 'address' + }, + { + indexed: true, + internalType: 'address', + name: 'paymaster', + type: 'address' + }, + { + indexed: false, + internalType: 'uint256', + name: 'nonceKey', + type: 'uint256' + }, + { + indexed: false, + internalType: 'uint256', + name: 'nonceSequence', + type: 'uint256' + }, + { + indexed: false, + internalType: 'bytes', + name: 'revertReason', + type: 'bytes' + } + ], + name: 'RIP7560TransactionPostOpRevertReason', + type: 'event' + }, + { + anonymous: false, + inputs: [ + { + indexed: true, + internalType: 'address', + name: 'sender', + type: 'address' + }, + { + indexed: false, + internalType: 'uint256', + name: 'nonceKey', + type: 'uint256' + }, + { + indexed: false, + internalType: 'uint256', + name: 'nonceSequence', + type: 'uint256' + }, + { + indexed: false, + internalType: 'bytes', + name: 'revertReason', + type: 'bytes' + } + ], + name: 'RIP7560TransactionRevertReason', + type: 'event' + }, + { + inputs: [ + { + internalType: 'uint256', + name: 'validAfter', + type: 'uint256' + }, + { + internalType: 'uint256', + name: 'validUntil', + type: 'uint256' + } + ], + name: 'acceptAccount', + outputs: [], + stateMutability: 'nonpayable', + type: 'function' + }, + { + inputs: [ + { + internalType: 'uint256', + name: 'validAfter', + type: 'uint256' + }, + { + internalType: 'uint256', + name: 'validUntil', + type: 'uint256' + }, + { + internalType: 'bytes', + name: 'context', + type: 'bytes' + } + ], + name: 'acceptPaymaster', + outputs: [], + stateMutability: 'nonpayable', + type: 'function' + }, + { + inputs: [ + { + internalType: 'uint256', + name: 'validAfter', + type: 'uint256' + }, + { + internalType: 'uint256', + name: 'validUntil', + type: 'uint256' + } + ], + name: 'sigFailAccount', + outputs: [], + stateMutability: 'nonpayable', + type: 'function' + }, + { + inputs: [ + { + internalType: 'uint256', + name: 'validAfter', + type: 'uint256' + }, + { + internalType: 'uint256', + name: 'validUntil', + type: 'uint256' + }, + { + internalType: 'bytes', + name: 'context', + type: 'bytes' + } + ], + name: 'sigFailPaymaster', + outputs: [], + stateMutability: 'nonpayable', + type: 'function' + } +] + export interface ERC7562ValidationResults { storageMap: StorageMap ruleViolations: ERC7562Violation[] @@ -45,7 +261,7 @@ export class ERC7562Parser { readonly bailOnViolation: boolean ) {} - private _init (erc7562Call: ERC7562Call) { + private _init (erc7562Call: ERC7562Call): void { this.keccak = erc7562Call.keccak ?? [] this.ruleViolations = [] this.currentEntity = AccountAbstractionEntity.none @@ -102,6 +318,7 @@ export class ERC7562Parser { private _tryDetectKnownMethod (call: ERC7562Call): string { const mergedAbi = Object.values([ + ...RIP7560EntryPointABI, ...SenderCreator__factory.abi, ...IEntryPoint__factory.abi, ...IPaymaster__factory.abi @@ -366,7 +583,11 @@ export class ERC7562Parser { let illegalZeroCodeAccess: any for (const addr of Object.keys(tracerResults.contractSize)) { // [OP-042] - if (addr.toLowerCase() !== userOp.sender.toLowerCase() && addr.toLowerCase() !== this.entryPointAddress.toLowerCase() && tracerResults.contractSize[addr].contractSize <= 2) { + if ( + addr.toLowerCase() !== userOp.sender.toLowerCase() && + // addr.toLowerCase() !== AA_ENTRY_POINT && + addr.toLowerCase() !== this.entryPointAddress.toLowerCase() && + tracerResults.contractSize[addr].contractSize <= 2) { illegalZeroCodeAccess = tracerResults.contractSize[addr] illegalZeroCodeAccess.address = addr this._violationDetected({ @@ -389,9 +610,15 @@ export class ERC7562Parser { checkOp054 (erc7562Call: ERC7562Call): void { const isCallToEntryPoint = this._isCallToEntryPoint(erc7562Call) const knownMethod = this._tryDetectKnownMethod(erc7562Call) + const isEntryPointCallAllowedRIP7560 = knownMethod === 'acceptAccount' || + knownMethod === 'acceptPaymaster' || + knownMethod === 'sigFailAccount' || + knownMethod === 'sigFailPaymaster' const isEntryPointCallAllowedOP052 = knownMethod === 'depositTo' const isEntryPointCallAllowedOP053 = knownMethod === '0x' - const isEntryPointCallAllowed = isEntryPointCallAllowedOP052 || isEntryPointCallAllowedOP053 + const isEntryPointCallAllowed = isEntryPointCallAllowedOP052 || + isEntryPointCallAllowedOP053 || + isEntryPointCallAllowedRIP7560 const isRuleViolated = isCallToEntryPoint && !isEntryPointCallAllowed if (isRuleViolated) { this._violationDetected({ diff --git a/packages/validation-manager/src/index.ts b/packages/validation-manager/src/index.ts index d45fa5a6..4a916422 100644 --- a/packages/validation-manager/src/index.ts +++ b/packages/validation-manager/src/index.ts @@ -3,12 +3,12 @@ import { JsonRpcProvider } from '@ethersproject/providers' import { AddressZero, IEntryPoint__factory, - OperationRIP7560, UserOperation } from '@account-abstraction/utils' import { PreVerificationGasCalculator } from '@account-abstraction/sdk' -import { bundlerJSTracerName, debug_traceCall, eth_traceRip7560Validation } from './GethTracer' +import { bundlerJSTracerName, debug_traceCall } from './GethTracer' +// @ts-ignore import { bundlerCollectorTracer } from './BundlerCollectorTracer' import { ValidateUserOpResult } from './IValidationManager' import { ValidationManager } from './ValidationManager' @@ -37,37 +37,8 @@ export async function supportsDebugTraceCall (provider: JsonRpcProvider, rip7560 } if (rip7560) { - // TODO: remove - const defaultsForRip7560Tx: OperationRIP7560 = { - accessList: [], - builderFee: '0x0', - chainId: '0x539', - value: '0x0', - sender: AddressZero, - nonceKey: '0x0', - nonce: '0x0', - executionData: '0x', - callGasLimit: '0x0', - verificationGasLimit: '0x10000', - maxFeePerGas: '0x100000000', - maxPriorityFeePerGas: '0x100000000', - paymaster: AddressZero, - paymasterData: '0x', - factory: AddressZero, - factoryData: '0x', - paymasterVerificationGasLimit: '0x10000', - paymasterPostOpGasLimit: '0x0', - authorizationData: '0x', - authorizationList: [] - }; - - // TODO: align parameter names across 4337 and 7560 - (defaultsForRip7560Tx as any).deployer = defaultsForRip7560Tx.factory; - (defaultsForRip7560Tx as any).deployerData = defaultsForRip7560Tx.factoryData - // make sure we can trace a call. - const ret = await eth_traceRip7560Validation(provider, defaultsForRip7560Tx - ).catch(e => e) - return ret.traceResults != null + // no need to check for the internal RIP-7560 support + return true } // make sure we can trace a call. const ret = await debug_traceCall(provider, From 6f61c92da7b07c301f487be5ec8b755cec1b9e58 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 21 Jan 2025 13:02:59 +0100 Subject: [PATCH 24/29] Configure AA_SENDER_CREATOR for ERC7562Parser --- packages/bundler/src/modules/initServer.ts | 3 ++- packages/validation-manager/src/ValidationManagerRIP7560.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/bundler/src/modules/initServer.ts b/packages/bundler/src/modules/initServer.ts index a40ac3fb..b15761b1 100644 --- a/packages/bundler/src/modules/initServer.ts +++ b/packages/bundler/src/modules/initServer.ts @@ -10,6 +10,7 @@ import { MempoolManager } from './MempoolManager' import { BundleManager } from './BundleManager' import { AA_ENTRY_POINT, + AA_SENDER_CREATOR, AA_STAKE_MANAGER, IValidationManager, ValidationManager, @@ -47,7 +48,7 @@ export function initServer (config: BundlerConfig, signer: Signer): [ExecutionMa bundleManager = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, signer, eventsManager, mempoolManager, validationManager, reputationManager, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc) } else { - const erc7562Parser = new ERC7562Parser(AA_ENTRY_POINT, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(AA_ENTRY_POINT, AA_SENDER_CREATOR, true) const stakeManager = IRip7560StakeManager__factory.connect(AA_STAKE_MANAGER, signer) validationManager = new ValidationManagerRIP7560(stakeManager, entryPoint.provider as JsonRpcProvider, erc7562Parser, config.unsafe) bundleManager = new BundleManagerRIP7560(entryPoint.provider as JsonRpcProvider, signer, eventsManager, mempoolManager, validationManager, reputationManager, diff --git a/packages/validation-manager/src/ValidationManagerRIP7560.ts b/packages/validation-manager/src/ValidationManagerRIP7560.ts index 1367637a..4e6015a6 100644 --- a/packages/validation-manager/src/ValidationManagerRIP7560.ts +++ b/packages/validation-manager/src/ValidationManagerRIP7560.ts @@ -18,6 +18,7 @@ import { eth_traceRip7560Validation } from './GethTracer' import { ERC7562Parser } from './ERC7562Parser' export const AA_ENTRY_POINT = '0x0000000000000000000000000000000000007560' +export const AA_SENDER_CREATOR = '0x00000000000000000000000000000000ffff7560' export const AA_STAKE_MANAGER = '0x570Aa568b6cf62ff08c6C3a3b3DB1a0438E871Fb' export class ValidationManagerRIP7560 implements IValidationManager { From da7f9860e59fcf6075caa6d527114953aa371236 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 21 Jan 2025 16:25:17 +0100 Subject: [PATCH 25/29] Fix some issues with RIP-7560 --- .../bundler/src/modules/ExecutionManager.ts | 2 +- packages/bundler/src/modules/initServer.ts | 5 ++-- packages/bundler/test/BundlerManager.test.ts | 4 ++-- packages/bundler/test/BundlerServer.test.ts | 2 +- .../bundler/test/DebugMethodHandler.test.ts | 2 +- .../bundler/test/UserOpMethodHandler.test.ts | 2 +- packages/bundler/test/ValidateManager.test.ts | 2 +- .../src/AccountAbstractionEntity.ts | 1 + .../validation-manager/src/ERC7562Parser.ts | 24 ++++++++++++------- .../src/ValidationManagerRIP7560.ts | 1 + packages/validation-manager/src/index.ts | 2 +- 11 files changed, 28 insertions(+), 19 deletions(-) diff --git a/packages/bundler/src/modules/ExecutionManager.ts b/packages/bundler/src/modules/ExecutionManager.ts index d0bb0582..a10816fc 100644 --- a/packages/bundler/src/modules/ExecutionManager.ts +++ b/packages/bundler/src/modules/ExecutionManager.ts @@ -150,7 +150,7 @@ export class ExecutionManager { const { configuration, entryPoint, unsafe } = this.validationManager._getDebugConfiguration() const mergedConfiguration = Object.assign({}, configuration, configOverrides) const pvgc = new PreVerificationGasCalculator(mergedConfiguration) - const erc7562Parser = new ERC7562Parser(entryPoint.address, mergedConfiguration.senderCreator ?? '', true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, mergedConfiguration.senderCreator ?? '') this.validationManager = new ValidationManager( entryPoint, unsafe, diff --git a/packages/bundler/src/modules/initServer.ts b/packages/bundler/src/modules/initServer.ts index b15761b1..00df8f64 100644 --- a/packages/bundler/src/modules/initServer.ts +++ b/packages/bundler/src/modules/initServer.ts @@ -10,6 +10,7 @@ import { MempoolManager } from './MempoolManager' import { BundleManager } from './BundleManager' import { AA_ENTRY_POINT, + AA_NONCE_MANAGER, AA_SENDER_CREATOR, AA_STAKE_MANAGER, IValidationManager, @@ -42,13 +43,13 @@ export function initServer (config: BundlerConfig, signer: Signer): [ExecutionMa let validationManager: IValidationManager let bundleManager: IBundleManager if (!config.rip7560) { - const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator) const tracerProvider = config.tracerRpcUrl == null ? undefined : getNetworkProvider(config.tracerRpcUrl) validationManager = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser, tracerProvider) bundleManager = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, signer, eventsManager, mempoolManager, validationManager, reputationManager, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc) } else { - const erc7562Parser = new ERC7562Parser(AA_ENTRY_POINT, AA_SENDER_CREATOR, true) + const erc7562Parser = new ERC7562Parser(AA_ENTRY_POINT, AA_SENDER_CREATOR, AA_NONCE_MANAGER) const stakeManager = IRip7560StakeManager__factory.connect(AA_STAKE_MANAGER, signer) validationManager = new ValidationManagerRIP7560(stakeManager, entryPoint.provider as JsonRpcProvider, erc7562Parser, config.unsafe) bundleManager = new BundleManagerRIP7560(entryPoint.provider as JsonRpcProvider, signer, eventsManager, mempoolManager, validationManager, reputationManager, diff --git a/packages/bundler/test/BundlerManager.test.ts b/packages/bundler/test/BundlerManager.test.ts index 415f3140..9827b7df 100644 --- a/packages/bundler/test/BundlerManager.test.ts +++ b/packages/bundler/test/BundlerManager.test.ts @@ -65,7 +65,7 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) bm = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, config.conditionalRpc) @@ -123,7 +123,7 @@ describe('#BundlerManager', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator) const validMgr = new ValidationManager(_entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(_entryPoint, mempoolMgr, repMgr) bundleMgr = new BundleManager(_entryPoint, _entryPoint.provider as JsonRpcProvider, _entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/BundlerServer.test.ts b/packages/bundler/test/BundlerServer.test.ts index b0ce30d8..cb7e30b4 100644 --- a/packages/bundler/test/BundlerServer.test.ts +++ b/packages/bundler/test/BundlerServer.test.ts @@ -61,7 +61,7 @@ describe('BundleServer', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/DebugMethodHandler.test.ts b/packages/bundler/test/DebugMethodHandler.test.ts index 463b36d1..761d0277 100644 --- a/packages/bundler/test/DebugMethodHandler.test.ts +++ b/packages/bundler/test/DebugMethodHandler.test.ts @@ -71,7 +71,7 @@ describe('#DebugMethodHandler', () => { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) const mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const eventsManager = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, eventsManager, mempoolMgr, validMgr, repMgr, diff --git a/packages/bundler/test/UserOpMethodHandler.test.ts b/packages/bundler/test/UserOpMethodHandler.test.ts index a2509e82..c47dbe96 100644 --- a/packages/bundler/test/UserOpMethodHandler.test.ts +++ b/packages/bundler/test/UserOpMethodHandler.test.ts @@ -89,7 +89,7 @@ describe('UserOpMethodHandler', function () { const repMgr = new ReputationManager(provider, BundlerReputationParams, parseEther(config.minStake), config.minUnstakeDelay) mempoolMgr = new MempoolManager(repMgr) const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) - const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, config.senderCreator) const validMgr = new ValidationManager(entryPoint, config.unsafe, preVerificationGasCalculator, erc7562Parser) const evMgr = new EventsManager(entryPoint, mempoolMgr, repMgr) const bundleMgr = new BundleManager(entryPoint, entryPoint.provider as JsonRpcProvider, entryPoint.signer, evMgr, mempoolMgr, validMgr, repMgr, config.beneficiary, parseEther(config.minBalance), config.maxBundleGas, false) diff --git a/packages/bundler/test/ValidateManager.test.ts b/packages/bundler/test/ValidateManager.test.ts index a737334b..c582b104 100644 --- a/packages/bundler/test/ValidateManager.test.ts +++ b/packages/bundler/test/ValidateManager.test.ts @@ -157,7 +157,7 @@ describe('#ValidationManager', () => { const preVerificationGasCalculator = new PreVerificationGasCalculator(MainnetConfig) const senderCreator = '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c' - const erc7562Parser = new ERC7562Parser(entryPoint.address, senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPoint.address, senderCreator) vm = new ValidationManager(entryPoint, unsafe, preVerificationGasCalculator, erc7562Parser) if (!await supportsDebugTraceCall(ethers.provider, false)) { diff --git a/packages/validation-manager/src/AccountAbstractionEntity.ts b/packages/validation-manager/src/AccountAbstractionEntity.ts index d50d2b1e..9f705600 100644 --- a/packages/validation-manager/src/AccountAbstractionEntity.ts +++ b/packages/validation-manager/src/AccountAbstractionEntity.ts @@ -6,5 +6,6 @@ export enum AccountAbstractionEntity { senderCreator = 'SenderCreator', entryPoint = 'EntryPoint', nativeEntryPoint = 'NativeEntryPoint', + nativeNonceManager = 'NativeNonceManager', none = 'none' } diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index c801be30..f3b2e893 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -255,10 +255,12 @@ export class ERC7562Parser { private contractAddresses: string[] = [] private storageMap: StorageMap = {} + private bailOnViolation: boolean = false + constructor ( readonly entryPointAddress: string, readonly senderCreatorAddress: string, - readonly bailOnViolation: boolean + readonly nonceManagerAddress?: string, ) {} private _init (erc7562Call: ERC7562Call): void { @@ -364,6 +366,9 @@ export class ERC7562Parser { } else if (this.senderCreatorAddress.toLowerCase() === call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.senderCreator this.currentEntityAddress = this.senderCreatorAddress + } else if (this.nonceManagerAddress?.toLowerCase() === call.to.toLowerCase()) { + this.currentEntity = AccountAbstractionEntity.nativeNonceManager + this.currentEntityAddress = this.nonceManagerAddress! } else { throw new RpcError(`could not find entity name for address ${call.to}. This should not happen. This is a bug.`, 0) } @@ -394,8 +399,8 @@ export class ERC7562Parser { private _parseEntitySlots ( userOp: OperationBase ): { - [addr: string]: Set - } { + [addr: string]: Set + } { // for each entity (sender, factory, paymaster), hold the valid slot addresses const entityAddresses = [userOp.sender.toLowerCase(), userOp.paymaster?.toLowerCase(), userOp.factory?.toLowerCase()] const entitySlots: { [addr: string]: Set } = {} @@ -426,11 +431,9 @@ export class ERC7562Parser { tracerResults: ERC7562Call, validationResult: ValidationResult ): ERC7562ValidationResults { + this.bailOnViolation = true const results = this.parseResults(userOp, tracerResults, validationResult) - if (results.ruleViolations.length > 0) { - // TODO: human-readable description of which rules were violated. - throw new Error('Rules Violated') - } + this.bailOnViolation = false return results } @@ -713,8 +716,11 @@ export class ERC7562Parser { userOp: OperationBase, tracerResults: ERC7562Call ): void { - if (tracerResults.to.toLowerCase() === this.entryPointAddress.toLowerCase()) { - // Currently inside the EntryPoint deposit code, no access control applies here + if ( + tracerResults.to.toLowerCase() === this.entryPointAddress.toLowerCase() || + tracerResults.to.toLowerCase() === this.nonceManagerAddress?.toLowerCase() + ) { + // Currently inside system code, no access control applies here return } const allSlots: string[] = [ diff --git a/packages/validation-manager/src/ValidationManagerRIP7560.ts b/packages/validation-manager/src/ValidationManagerRIP7560.ts index 4e6015a6..825fe59a 100644 --- a/packages/validation-manager/src/ValidationManagerRIP7560.ts +++ b/packages/validation-manager/src/ValidationManagerRIP7560.ts @@ -20,6 +20,7 @@ import { ERC7562Parser } from './ERC7562Parser' export const AA_ENTRY_POINT = '0x0000000000000000000000000000000000007560' export const AA_SENDER_CREATOR = '0x00000000000000000000000000000000ffff7560' export const AA_STAKE_MANAGER = '0x570Aa568b6cf62ff08c6C3a3b3DB1a0438E871Fb' +export const AA_NONCE_MANAGER = '0x632fafb21910d6c8b4a3995063dd984f2b829c02' export class ValidationManagerRIP7560 implements IValidationManager { constructor ( diff --git a/packages/validation-manager/src/index.ts b/packages/validation-manager/src/index.ts index 4a916422..bea25cee 100644 --- a/packages/validation-manager/src/index.ts +++ b/packages/validation-manager/src/index.ts @@ -58,7 +58,7 @@ export async function checkRulesViolations ( } const entryPoint = IEntryPoint__factory.connect(entryPointAddress, provider) const senderCreator = '0xefc2c1444ebcc4db75e7613d20c6a62ff67a167c' - const erc7562Parser = new ERC7562Parser(entryPointAddress, senderCreator, true) + const erc7562Parser = new ERC7562Parser(entryPointAddress, senderCreator) const validationManager = new ValidationManager( entryPoint, false, From a85db52536c4147be01035d332d9ad7194b66923 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 21 Jan 2025 16:58:30 +0100 Subject: [PATCH 26/29] Enable legacy tracer and parser for the EIP-7702 tests --- .../src/ValidationManager.ts | 44 ++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/packages/validation-manager/src/ValidationManager.ts b/packages/validation-manager/src/ValidationManager.ts index 80a872f3..7b1629f0 100644 --- a/packages/validation-manager/src/ValidationManager.ts +++ b/packages/validation-manager/src/ValidationManager.ts @@ -38,6 +38,8 @@ import EntryPointSimulationsJson from '@account-abstraction/contracts/artifacts/ import { IValidationManager, ValidateUserOpResult, ValidationResult } from './IValidationManager' import { ERC7562Parser } from './ERC7562Parser' import { ERC7562Call } from './ERC7562Call' +import { bundlerCollectorTracer, BundlerTracerResult } from './BundlerCollectorTracer' +import { tracerResultParser } from './TracerResultParser' const debug = Debug('aa.mgr.validate') @@ -57,6 +59,7 @@ const entryPointSimulations = IEntryPointSimulations__factory.createInterface() */ export class ValidationManager implements IValidationManager { private readonly provider: JsonRpcProvider + constructor ( readonly entryPoint: IEntryPoint, readonly unsafe: boolean, @@ -142,7 +145,7 @@ export class ValidationManager implements IValidationManager { async _geth_traceCall_SimulateValidation ( operation: OperationBase, stateOverride: { [address: string]: { code: string } } - ): Promise<[ValidationResult, ERC7562Call]> { + ): Promise<[ValidationResult, ERC7562Call | null, BundlerTracerResult | null]> { const userOp = operation as UserOperation const provider = this.entryPoint.provider as JsonRpcProvider const simulateCall = entryPointSimulations.encodeFunctionData('simulateValidation', [packUserOp(userOp)]) @@ -157,18 +160,18 @@ export class ValidationManager implements IValidationManager { } let tracer if (!this.usingErc7562NativeTracer()) { - throw new Error('JavaScript BundlerCollectorTracer is no longer supported') + tracer = bundlerCollectorTracer } const tracerResult = await debug_traceCall(provider, { - from: AddressZero, - to: this.entryPoint.address, - data: simulateCall, - gasLimit: simulationGas - }, { - tracer, - stateOverrides - }, - this.providerForTracer + from: AddressZero, + to: this.entryPoint.address, + data: simulateCall, + gasLimit: simulationGas + }, { + tracer, + stateOverrides + }, + this.providerForTracer ) let data: any @@ -201,7 +204,11 @@ export class ValidationManager implements IValidationManager { ) // console.log('==debug=', ...tracerResult.numberLevels.forEach(x=>x.access), 'sender=', userOp.sender, 'paymaster=', hexlify(userOp.paymasterAndData)?.slice(0, 42)) // errorResult is "ValidationResult" - return [validationResult, tracerResult] + if (this.usingErc7562NativeTracer()) { + return [validationResult, tracerResult, null] + } else { + return [validationResult, null, tracerResult as BundlerTracerResult] + } } catch (e: any) { // if already parsed, throw as is if (e.code != null) { @@ -257,14 +264,21 @@ export class ValidationManager implements IValidationManager { const stateOverrideForEip7702 = await this.getAuthorizationsStateOverride(authorizationList) let storageMap: StorageMap = {} if (!this.unsafe) { - let tracerResult: ERC7562Call - [res, tracerResult] = await this._geth_traceCall_SimulateValidation(userOp, stateOverrideForEip7702).catch(e => { + let erc7562Call: ERC7562Call | null + let bundlerTracerResult: BundlerTracerResult | null + [res, erc7562Call, bundlerTracerResult] = await this._geth_traceCall_SimulateValidation(userOp, stateOverrideForEip7702).catch(e => { throw e }) // console.log('tracer res') // console.dir(tracerResult, { depth: null }) let contractAddresses: string[] - ({ contractAddresses, storageMap } = this.erc7562Parser.requireCompliance(userOp, tracerResult, res)) + if (erc7562Call != null) { + ({ contractAddresses, storageMap } = this.erc7562Parser.requireCompliance(userOp, erc7562Call, res)) + } else if (bundlerTracerResult != null) { + [contractAddresses, storageMap] = tracerResultParser(userOp, bundlerTracerResult, res, this.entryPoint.address) + } else { + throw new Error('Tracer result is null for both legacy and modern parser') + } // if no previous contract hashes, then calculate hashes of contracts if (previousCodeHashes == null) { codeHashes = await this.getCodeHashes(contractAddresses) From dd554cdab31ede0bef33ed27016758f8025f86fb Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 21 Jan 2025 17:16:23 +0100 Subject: [PATCH 27/29] Handle the 'depth' field and clean up code for merging --- .../validation-manager/src/ERC7562Parser.ts | 272 ++++++++++-------- .../src/ValidationManager.ts | 18 +- 2 files changed, 155 insertions(+), 135 deletions(-) diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index f3b2e893..15f9dd0f 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -260,9 +260,53 @@ export class ERC7562Parser { constructor ( readonly entryPointAddress: string, readonly senderCreatorAddress: string, - readonly nonceManagerAddress?: string, + readonly nonceManagerAddress?: string ) {} + /** + * Analyzes the tracing results for the given UserOperation. + * Throws an exception in case canonical ERC-7562 rule violation was detected. + * + * In order to get a list of violated rules use {@link parseResults} directly. + * @returns {@link ERC7562ValidationResults} containing addresses and storage slots accessed by the UserOperation. + */ + requireCompliance ( + userOp: OperationBase, + erc7562Call: ERC7562Call, + validationResult: ValidationResult + ): ERC7562ValidationResults { + this.bailOnViolation = true + const results = this.parseResults(userOp, erc7562Call, validationResult) + this.bailOnViolation = false + return results + } + + /** + * Analyzes the tracing results for the given UserOperation. + * + * Unlike {@link requireCompliance}, does not throw an exception in case a rule violation was detected. + * + * @returns {@link ERC7562ValidationResults} containing addresses and storage slots accessed by the UserOperation, + * @returns an array of ERC-7562 rules that were violated by the UserOperation. + */ + parseResults ( + userOp: OperationBase, + erc7562Call: ERC7562Call, + validationResult: ValidationResult + ): ERC7562ValidationResults { + this._init(erc7562Call) + if (erc7562Call.calls == null || erc7562Call.calls.length < 1) { + throw new Error('Unexpected traceCall result: no calls from entrypoint.') + } + this.stakeValidationResult = validationResult + this._innerStepRecursive(userOp, erc7562Call, 0) + return { + contractAddresses: this.contractAddresses, + ruleViolations: this.ruleViolations, + storageMap: this.storageMap + } + } + private _init (erc7562Call: ERC7562Call): void { this.keccak = erc7562Call.keccak ?? [] this.ruleViolations = [] @@ -272,11 +316,11 @@ export class ERC7562Parser { this.storageMap = {} } - private _isCallToEntryPoint (call: ERC7562Call): boolean { - return call.to?.toLowerCase() === this.entryPointAddress?.toLowerCase() && - call.from?.toLowerCase() !== this.entryPointAddress?.toLowerCase() && + private _isCallToEntryPoint (erc7562Call: ERC7562Call): boolean { + return erc7562Call.to?.toLowerCase() === this.entryPointAddress?.toLowerCase() && + erc7562Call.from?.toLowerCase() !== this.entryPointAddress?.toLowerCase() && // skipping the top-level call from address(0) to 'simulateValidations()' - call.from?.toLowerCase() !== AddressZero + erc7562Call.from?.toLowerCase() !== AddressZero } private _isEntityStaked (entity?: AccountAbstractionEntity): boolean { @@ -318,7 +362,7 @@ export class ERC7562Parser { return false } - private _tryDetectKnownMethod (call: ERC7562Call): string { + private _tryDetectKnownMethod (erc7562Call: ERC7562Call): string { const mergedAbi = Object.values([ ...RIP7560EntryPointABI, ...SenderCreator__factory.abi, @@ -326,7 +370,7 @@ export class ERC7562Parser { ...IPaymaster__factory.abi ]) const AbiInterfaces = new Interface(mergedAbi) - const methodSig = call.input.slice(0, 10) + const methodSig = erc7562Call.input.slice(0, 10) try { const abiFunction: FunctionFragment = AbiInterfaces.getFunction(methodSig) return abiFunction.name @@ -341,36 +385,36 @@ export class ERC7562Parser { } } - private _detectEntityChange (userOp: OperationBase, call: ERC7562Call): void { + private _detectEntityChange (userOp: OperationBase, erc7562Call: ERC7562Call): void { if ( - call.from.toLowerCase() !== AddressZero && - call.from.toLowerCase() !== this.entryPointAddress.toLowerCase() && - call.from.toLowerCase() !== this.senderCreatorAddress.toLowerCase()) { + erc7562Call.from.toLowerCase() !== AddressZero && + erc7562Call.from.toLowerCase() !== this.entryPointAddress.toLowerCase() && + erc7562Call.from.toLowerCase() !== this.senderCreatorAddress.toLowerCase()) { return } - if (userOp.sender.toLowerCase() === call.to.toLowerCase()) { + if (userOp.sender.toLowerCase() === erc7562Call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.account this.currentEntityAddress = userOp.sender } else if ( - call.from.toLowerCase() === this.senderCreatorAddress.toLowerCase() && - userOp.factory?.toLowerCase() === call.to.toLowerCase() + erc7562Call.from.toLowerCase() === this.senderCreatorAddress.toLowerCase() && + userOp.factory?.toLowerCase() === erc7562Call.to.toLowerCase() ) { this.currentEntity = AccountAbstractionEntity.factory this.currentEntityAddress = userOp.factory - } else if (userOp.paymaster?.toLowerCase() === call.to.toLowerCase()) { + } else if (userOp.paymaster?.toLowerCase() === erc7562Call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.paymaster this.currentEntityAddress = userOp.paymaster - } else if (this.entryPointAddress.toLowerCase() === call.to.toLowerCase()) { + } else if (this.entryPointAddress.toLowerCase() === erc7562Call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.entryPoint this.currentEntityAddress = this.entryPointAddress - } else if (this.senderCreatorAddress.toLowerCase() === call.to.toLowerCase()) { + } else if (this.senderCreatorAddress.toLowerCase() === erc7562Call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.senderCreator this.currentEntityAddress = this.senderCreatorAddress - } else if (this.nonceManagerAddress?.toLowerCase() === call.to.toLowerCase()) { + } else if (this.nonceManagerAddress?.toLowerCase() === erc7562Call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.nativeNonceManager this.currentEntityAddress = this.nonceManagerAddress! } else { - throw new RpcError(`could not find entity name for address ${call.to}. This should not happen. This is a bug.`, 0) + throw new RpcError(`could not find entity name for address ${erc7562Call.to}. This should not happen. This is a bug.`, 0) } } @@ -399,8 +443,8 @@ export class ERC7562Parser { private _parseEntitySlots ( userOp: OperationBase ): { - [addr: string]: Set - } { + [addr: string]: Set + } { // for each entity (sender, factory, paymaster), hold the valid slot addresses const entityAddresses = [userOp.sender.toLowerCase(), userOp.paymaster?.toLowerCase(), userOp.factory?.toLowerCase()] const entitySlots: { [addr: string]: Set } = {} @@ -423,56 +467,24 @@ export class ERC7562Parser { return entitySlots } - /** - * Validates the UserOperation and throws an exception in case current mempool configuration rules were violated. - */ - requireCompliance ( - userOp: OperationBase, - tracerResults: ERC7562Call, - validationResult: ValidationResult - ): ERC7562ValidationResults { - this.bailOnViolation = true - const results = this.parseResults(userOp, tracerResults, validationResult) - this.bailOnViolation = false - return results - } - - parseResults ( - userOp: OperationBase, - erc7562Call: ERC7562Call, - validationResult: ValidationResult - ): ERC7562ValidationResults { - this._init(erc7562Call) - if (erc7562Call.calls == null || erc7562Call.calls.length < 1) { - throw new Error('Unexpected traceCall result: no calls from entrypoint.') - } - this.stakeValidationResult = validationResult - this._innerStepRecursive(userOp, erc7562Call, 0) - return { - contractAddresses: this.contractAddresses, - ruleViolations: this.ruleViolations, - storageMap: this.storageMap - } - } - private _innerStepRecursive ( userOp: OperationBase, - tracerResults: ERC7562Call, + erc7562Call: ERC7562Call, recursionDepth: number ): void { - const address = tracerResults.to + const address = erc7562Call.to this.contractAddresses.push(address) - this._detectEntityChange(userOp, tracerResults) - this.checkOp011(tracerResults) - this.checkOp020(tracerResults) - this.checkOp031(userOp, tracerResults) - this.checkOp041(userOp, tracerResults) - this.checkOp054(tracerResults) - this.checkOp054ExtCode(tracerResults, address, recursionDepth) - this.checkOp061(tracerResults) - this.checkOp080(tracerResults) - this.checkStorage(userOp, tracerResults) - for (const call of tracerResults.calls ?? []) { + this._detectEntityChange(userOp, erc7562Call) + this._checkOp011(erc7562Call, recursionDepth) + this._checkOp020(erc7562Call, recursionDepth) + this._checkOp031(userOp, erc7562Call, recursionDepth) + this._checkOp041(userOp, erc7562Call, recursionDepth) + this._checkOp054(erc7562Call, recursionDepth) + this._checkOp054ExtCode(erc7562Call, address, recursionDepth) + this._checkOp061(erc7562Call, recursionDepth) + this._checkOp080(erc7562Call, recursionDepth) + this._checkStorage(userOp, erc7562Call, recursionDepth) + for (const call of erc7562Call.calls ?? []) { this._innerStepRecursive(userOp, call, recursionDepth + 1) } } @@ -481,12 +493,12 @@ export class ERC7562Parser { * OP-011: Blocked opcodes * OP-080: `BALANCE` (0x31) and `SELFBALANCE` (0x47) are allowed only from a staked entity, else they are blocked */ - checkOp011 (tracerResults: ERC7562Call): void { - if (tracerResults.to.toLowerCase() === this.entryPointAddress.toLowerCase()) { + private _checkOp011 (erc7562Call: ERC7562Call, recursionDepth: number): void { + if (erc7562Call.to.toLowerCase() === this.entryPointAddress.toLowerCase()) { // Currently inside the EntryPoint deposit code, no access control applies here return } - const opcodes = tracerResults.usedOpcodes + const opcodes = erc7562Call.usedOpcodes const bannedOpCodeUsed = Object .keys(opcodes) @@ -500,10 +512,9 @@ export class ERC7562Parser { (opcode: string): void => { this._violationDetected({ rule: ERC7562Rule.op011, - // TODO: fill in depth, entity - depth: -1, + depth: recursionDepth, entity: this.currentEntity, - address: tracerResults.from, + address: erc7562Call.from, opcode, value: '0', errorCode: ValidationErrors.OpcodeValidation, @@ -516,15 +527,15 @@ export class ERC7562Parser { /** * OP-020: Revert on "out of gas" is forbidden as it can "leak" the gas limit or the current call stack depth. */ - checkOp020 (tracerResults: ERC7562Call): void { - if (tracerResults.outOfGas) { + private _checkOp020 (erc7562Call: ERC7562Call, recursionDepth: number): void { + if (erc7562Call.outOfGas) { this._violationDetected({ rule: ERC7562Rule.op020, // TODO: fill in depth, entity - depth: -1, + depth: recursionDepth, entity: this.currentEntity, - address: tracerResults.from, - opcode: tracerResults.type, + address: erc7562Call.from, + opcode: erc7562Call.type, value: '0', errorCode: ValidationErrors.OpcodeValidation, description: `${this.currentEntity.toString()} internally reverts on oog` @@ -535,38 +546,38 @@ export class ERC7562Parser { /** * OP-031: CREATE2 is allowed exactly once in the deployment phase and must deploy code for the "sender" address */ - checkOp031 ( + private _checkOp031 ( userOp: OperationBase, - tracerResults: ERC7562Call + erc7562Call: ERC7562Call, + recursionDepth: number ): void { if ( - tracerResults.type !== 'CREATE' && - tracerResults.type !== 'CREATE2' + erc7562Call.type !== 'CREATE' && + erc7562Call.type !== 'CREATE2' ) { return } const isFactoryStaked = this._isEntityStaked(AccountAbstractionEntity.factory) const isAllowedCreateByOP032 = userOp.factory != null && - tracerResults.type === 'CREATE' && + erc7562Call.type === 'CREATE' && this.currentEntity === AccountAbstractionEntity.account && - tracerResults.from.toLowerCase() === userOp.sender.toLowerCase() + erc7562Call.from.toLowerCase() === userOp.sender.toLowerCase() const isAllowedCreateByEREP060 = ( - tracerResults.from.toLowerCase() === userOp.sender?.toLowerCase() || - tracerResults.from.toLowerCase() === userOp.factory?.toLowerCase() + erc7562Call.from.toLowerCase() === userOp.sender?.toLowerCase() || + erc7562Call.from.toLowerCase() === userOp.factory?.toLowerCase() ) && isFactoryStaked const isAllowedCreateSenderByFactory = this.currentEntity === AccountAbstractionEntity.factory && - tracerResults.to.toLowerCase() === userOp.sender.toLowerCase() + erc7562Call.to.toLowerCase() === userOp.sender.toLowerCase() if (!(isAllowedCreateByOP032 || isAllowedCreateByEREP060 || isAllowedCreateSenderByFactory)) { this._violationDetected({ rule: ERC7562Rule.op011, - // TODO: fill in depth, entity - depth: -1, + depth: recursionDepth, entity: this.currentEntity, - address: tracerResults.from ?? 'n/a', + address: erc7562Call.from ?? 'n/a', opcode: 'CREATE2', value: '0', errorCode: ValidationErrors.OpcodeValidation, @@ -578,24 +589,25 @@ export class ERC7562Parser { /** * OP-041: Access to an address without a deployed code is forbidden for EXTCODE* and *CALL opcodes */ - checkOp041 ( + private _checkOp041 ( userOp: OperationBase, - tracerResults: ERC7562Call + erc7562Call: ERC7562Call, + recursionDepth: number ): void { // the only contract we allow to access before its deployment is the "sender" itself, which gets created. let illegalZeroCodeAccess: any - for (const addr of Object.keys(tracerResults.contractSize)) { + for (const addr of Object.keys(erc7562Call.contractSize)) { // [OP-042] if ( addr.toLowerCase() !== userOp.sender.toLowerCase() && // addr.toLowerCase() !== AA_ENTRY_POINT && addr.toLowerCase() !== this.entryPointAddress.toLowerCase() && - tracerResults.contractSize[addr].contractSize <= 2) { - illegalZeroCodeAccess = tracerResults.contractSize[addr] + erc7562Call.contractSize[addr].contractSize <= 2) { + illegalZeroCodeAccess = erc7562Call.contractSize[addr] illegalZeroCodeAccess.address = addr this._violationDetected({ address: '', - depth: 0, + depth: recursionDepth, entity: this.currentEntity, rule: ERC7562Rule.op041, errorCode: ValidationErrors.OpcodeValidation, @@ -610,7 +622,10 @@ export class ERC7562Parser { * OP-053: May call the fallback function from the `sender` with any value. * OP-054: Any other access to the EntryPoint is forbidden. */ - checkOp054 (erc7562Call: ERC7562Call): void { + private _checkOp054 ( + erc7562Call: ERC7562Call, + recursionDepth: number + ): void { const isCallToEntryPoint = this._isCallToEntryPoint(erc7562Call) const knownMethod = this._tryDetectKnownMethod(erc7562Call) const isEntryPointCallAllowedRIP7560 = knownMethod === 'acceptAccount' || @@ -626,8 +641,7 @@ export class ERC7562Parser { if (isRuleViolated) { this._violationDetected({ rule: ERC7562Rule.op054, - // TODO: fill in depth, entity - depth: -1, + depth: recursionDepth, entity: this.currentEntity, address: erc7562Call.from, opcode: erc7562Call.type, @@ -638,12 +652,12 @@ export class ERC7562Parser { } } - checkOp054ExtCode ( - tracerResults: ERC7562Call, + private _checkOp054ExtCode ( + erc7562Call: ERC7562Call, address: string, recursionDepth: number ): void { - for (const addr of tracerResults.extCodeAccessInfo) { + for (const addr of erc7562Call.extCodeAccessInfo) { if (addr.toLowerCase() === this.entryPointAddress.toLowerCase()) { this._violationDetected({ address, @@ -660,19 +674,21 @@ export class ERC7562Parser { /** * OP-061: CALL with value is forbidden. The only exception is a call to the EntryPoint. */ - checkOp061 (tracerResults: ERC7562Call): void { + private _checkOp061 ( + erc7562Call: ERC7562Call, + recursionDepth: number + ): void { const isIllegalNonZeroValueCall = - !this._isCallToEntryPoint(tracerResults) && - !BigNumber.from(tracerResults.value ?? 0).eq(0) + !this._isCallToEntryPoint(erc7562Call) && + !BigNumber.from(erc7562Call.value ?? 0).eq(0) if (isIllegalNonZeroValueCall) { this._violationDetected({ rule: ERC7562Rule.op061, - // TODO: fill in depth, entity - depth: -1, + depth: recursionDepth, entity: this.currentEntity, - address: tracerResults.from, - opcode: tracerResults.type, - value: tracerResults.value, + address: erc7562Call.from, + opcode: erc7562Call.type, + value: erc7562Call.value, errorCode: ValidationErrors.OpcodeValidation, description: 'May not may CALL with value' }) @@ -682,8 +698,11 @@ export class ERC7562Parser { /** * OP-080: BALANCE (0x31) and SELFBALANCE (0x47) are allowed only from a staked entity, else they are blocked */ - checkOp080 (tracerResults: ERC7562Call): void { - const opcodes = tracerResults.usedOpcodes + private _checkOp080 ( + erc7562Call: ERC7562Call, + recursionDepth: number + ): void { + const opcodes = erc7562Call.usedOpcodes const isEntityStaked = this._isEntityStaked() const onlyStakedOpCodeUsed = Object @@ -700,9 +719,9 @@ export class ERC7562Parser { this._violationDetected({ rule: ERC7562Rule.op011, // TODO: fill in depth, entity - depth: -1, + depth: recursionDepth, entity: this.currentEntity, - address: tracerResults.from ?? 'n/a', + address: erc7562Call.from ?? 'n/a', opcode, value: '0', errorCode: ValidationErrors.OpcodeValidation, @@ -712,24 +731,25 @@ export class ERC7562Parser { ) } - checkStorage ( + private _checkStorage ( userOp: OperationBase, - tracerResults: ERC7562Call + erc7562Call: ERC7562Call, + recursionDepth: number ): void { if ( - tracerResults.to.toLowerCase() === this.entryPointAddress.toLowerCase() || - tracerResults.to.toLowerCase() === this.nonceManagerAddress?.toLowerCase() + erc7562Call.to.toLowerCase() === this.entryPointAddress.toLowerCase() || + erc7562Call.to.toLowerCase() === this.nonceManagerAddress?.toLowerCase() ) { // Currently inside system code, no access control applies here return } const allSlots: string[] = [ - ...Object.keys(tracerResults.accessedSlots.writes ?? {}), - ...Object.keys(tracerResults.accessedSlots.reads ?? {}), - ...Object.keys(tracerResults.accessedSlots.transientWrites ?? {}), - ...Object.keys(tracerResults.accessedSlots.transientReads ?? {}) + ...Object.keys(erc7562Call.accessedSlots.writes ?? {}), + ...Object.keys(erc7562Call.accessedSlots.reads ?? {}), + ...Object.keys(erc7562Call.accessedSlots.transientWrites ?? {}), + ...Object.keys(erc7562Call.accessedSlots.transientReads ?? {}) ] - const address: string = tracerResults.to + const address: string = erc7562Call.to const entitySlots = this._parseEntitySlots(userOp) const addressName = this._tryGetAddressName(userOp, address) const isEntityStaked = this._isEntityStaked() @@ -744,7 +764,7 @@ export class ERC7562Parser { const isSenderAssociated: boolean = this._associatedWith(slot, userOp.sender.toLowerCase(), entitySlots) const isEntityInternalSTO031: boolean = address.toLowerCase() === this.currentEntityAddress.toLowerCase() const isEntityAssociatedSTO032: boolean = this._associatedWith(slot, this.currentEntityAddress.toLowerCase(), entitySlots) - const isReadOnlyAccessSTO033: boolean = tracerResults.accessedSlots.writes?.[slot] == null && tracerResults.accessedSlots.transientWrites?.[slot] == null + const isReadOnlyAccessSTO033: boolean = erc7562Call.accessedSlots.writes?.[slot] == null && erc7562Call.accessedSlots.transientWrites?.[slot] == null const isAllowedIfEntityStaked = isEntityInternalSTO031 || isEntityAssociatedSTO032 || isReadOnlyAccessSTO033 const isAllowedST031ST032ST033: boolean = isAllowedIfEntityStaked && isEntityStaked @@ -761,15 +781,15 @@ export class ERC7562Parser { ) { description = `unstaked ${this.currentEntity.toString()} accessed ${addressName} slot ${slot}` } else { - const isWrite = Object.keys(tracerResults.accessedSlots.writes ?? {}).includes(slot) || Object.keys(tracerResults.accessedSlots.transientWrites ?? {}).includes(slot) - const isTransient = Object.keys(tracerResults.accessedSlots.transientReads ?? {}).includes(slot) || Object.keys(tracerResults.accessedSlots.transientWrites ?? {}).includes(slot) + const isWrite = Object.keys(erc7562Call.accessedSlots.writes ?? {}).includes(slot) || Object.keys(erc7562Call.accessedSlots.transientWrites ?? {}).includes(slot) + const isTransient = Object.keys(erc7562Call.accessedSlots.transientReads ?? {}).includes(slot) || Object.keys(erc7562Call.accessedSlots.transientWrites ?? {}).includes(slot) const readWrite = isWrite ? 'write to' : 'read from' const transientStr = isTransient ? 'transient ' : '' description = `${this.currentEntity.toString()} has forbidden ${readWrite} ${transientStr}${addressName} slot ${slot}` } this._violationDetected({ address: '', - depth: 0, + depth: recursionDepth, entity: this.currentEntity, errorCode: ValidationErrors.OpcodeValidation, rule: ERC7562Rule.sto010, diff --git a/packages/validation-manager/src/ValidationManager.ts b/packages/validation-manager/src/ValidationManager.ts index 7b1629f0..5a2a0438 100644 --- a/packages/validation-manager/src/ValidationManager.ts +++ b/packages/validation-manager/src/ValidationManager.ts @@ -163,15 +163,15 @@ export class ValidationManager implements IValidationManager { tracer = bundlerCollectorTracer } const tracerResult = await debug_traceCall(provider, { - from: AddressZero, - to: this.entryPoint.address, - data: simulateCall, - gasLimit: simulationGas - }, { - tracer, - stateOverrides - }, - this.providerForTracer + from: AddressZero, + to: this.entryPoint.address, + data: simulateCall, + gasLimit: simulationGas + }, { + tracer, + stateOverrides + }, + this.providerForTracer ) let data: any From 44e16fb1f891b3e5ca9953290141f45c36985ae5 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 21 Jan 2025 19:19:15 +0100 Subject: [PATCH 28/29] Fix lint and depcheck --- packages/validation-manager/package.json | 3 ++- packages/validation-manager/src/ERC7562Parser.ts | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/validation-manager/package.json b/packages/validation-manager/package.json index 6a3d5067..a4458178 100644 --- a/packages/validation-manager/package.json +++ b/packages/validation-manager/package.json @@ -22,7 +22,8 @@ "@ethersproject/properties": "^5.7.0", "@ethersproject/providers": "^5.7.0", "debug": "^4.3.4", - "ethers": "^5.7.0" + "ethers": "^5.7.0", + "ow": "^0.28.1" }, "devDependencies": {} } diff --git a/packages/validation-manager/src/ERC7562Parser.ts b/packages/validation-manager/src/ERC7562Parser.ts index 15f9dd0f..ccdd72ee 100644 --- a/packages/validation-manager/src/ERC7562Parser.ts +++ b/packages/validation-manager/src/ERC7562Parser.ts @@ -392,6 +392,7 @@ export class ERC7562Parser { erc7562Call.from.toLowerCase() !== this.senderCreatorAddress.toLowerCase()) { return } + const nonceManagerAddress = this.nonceManagerAddress if (userOp.sender.toLowerCase() === erc7562Call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.account this.currentEntityAddress = userOp.sender @@ -410,9 +411,12 @@ export class ERC7562Parser { } else if (this.senderCreatorAddress.toLowerCase() === erc7562Call.to.toLowerCase()) { this.currentEntity = AccountAbstractionEntity.senderCreator this.currentEntityAddress = this.senderCreatorAddress - } else if (this.nonceManagerAddress?.toLowerCase() === erc7562Call.to.toLowerCase()) { + } else if ( + nonceManagerAddress != null && + nonceManagerAddress.toLowerCase() === erc7562Call.to.toLowerCase() + ) { this.currentEntity = AccountAbstractionEntity.nativeNonceManager - this.currentEntityAddress = this.nonceManagerAddress! + this.currentEntityAddress = nonceManagerAddress } else { throw new RpcError(`could not find entity name for address ${erc7562Call.to}. This should not happen. This is a bug.`, 0) } From dd53eee88e2f34355f89f8caeb1c9a55b3145ddc Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Tue, 21 Jan 2025 19:22:27 +0100 Subject: [PATCH 29/29] Fix lint --- packages/bundler/src/modules/initServer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/bundler/src/modules/initServer.ts b/packages/bundler/src/modules/initServer.ts index 00df8f64..ad7d0942 100644 --- a/packages/bundler/src/modules/initServer.ts +++ b/packages/bundler/src/modules/initServer.ts @@ -15,7 +15,7 @@ import { AA_STAKE_MANAGER, IValidationManager, ValidationManager, - ValidationManagerRIP7560, + ValidationManagerRIP7560 } from '@account-abstraction/validation-manager' import { BundlerConfig } from '../BundlerConfig' import { EventsManager } from './EventsManager'