From 4bcbe267e5351db77ef62d285d9a32cd2f083ce3 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Thu, 28 Sep 2023 03:00:09 +0300 Subject: [PATCH] WIP enforce reputation rules --- packages/bundler/src/DebugMethodHandler.ts | 3 - .../bundler/src/modules/ExecutionManager.ts | 6 +- .../bundler/src/modules/MempoolManager.ts | 58 +++++++++++++++++-- .../bundler/src/modules/ReputationManager.ts | 31 +++++++++- 4 files changed, 85 insertions(+), 13 deletions(-) diff --git a/packages/bundler/src/DebugMethodHandler.ts b/packages/bundler/src/DebugMethodHandler.ts index 98580de4..0d0fb72f 100644 --- a/packages/bundler/src/DebugMethodHandler.ts +++ b/packages/bundler/src/DebugMethodHandler.ts @@ -51,9 +51,6 @@ export class DebugMethodHandler { } setReputation (param: any): ReputationDump { - if (param.reputation == null) { - throw new Error('expected structure { reputation: {addr:{opsSeen:1, opsIncluded:2} }') - } return this.repManager.setReputation(param) } diff --git a/packages/bundler/src/modules/ExecutionManager.ts b/packages/bundler/src/modules/ExecutionManager.ts index ea9fb61d..8a50416f 100644 --- a/packages/bundler/src/modules/ExecutionManager.ts +++ b/packages/bundler/src/modules/ExecutionManager.ts @@ -40,9 +40,11 @@ export class ExecutionManager { this.mempoolManager.addUserOp(userOp, userOpHash, validationResult.returnInfo.prefund, - validationResult.senderInfo, validationResult.referencedContracts, - validationResult.aggregatorInfo?.addr) + validationResult.senderInfo, + validationResult.paymasterInfo, + validationResult.factoryInfo, + validationResult.aggregatorInfo) await this.attemptBundle(false) }) } diff --git a/packages/bundler/src/modules/MempoolManager.ts b/packages/bundler/src/modules/MempoolManager.ts index 4750b77c..46e082ab 100644 --- a/packages/bundler/src/modules/MempoolManager.ts +++ b/packages/bundler/src/modules/MempoolManager.ts @@ -1,7 +1,7 @@ import { BigNumber, BigNumberish } from 'ethers' import { getAddr } from './moduleUtils' import { requireCond } from '../utils' -import { ReputationManager } from './ReputationManager' +import { ReputationManager, ReputationStatus } from './ReputationManager' import Debug from 'debug' import { ReferencedCodeHashes, StakeInfo, UserOperation, ValidationErrors } from './Types' @@ -37,13 +37,22 @@ export class MempoolManager { // add userOp into the mempool, after initial validation. // replace existing, if any (and if new gas is higher) // revets if unable to add UserOp to mempool (too many UserOps with this sender) - addUserOp (userOp: UserOperation, userOpHash: string, prefund: BigNumberish, senderInfo: StakeInfo, referencedContracts: ReferencedCodeHashes, aggregator?: string): void { + addUserOp ( + userOp: UserOperation, + userOpHash: string, + prefund: BigNumberish, + referencedContracts: ReferencedCodeHashes, + senderInfo: StakeInfo, + paymasterInfo?: StakeInfo, + factoryInfo?: StakeInfo, + aggregatorInfo?: StakeInfo + ): void { const entry: MempoolEntry = { userOp, userOpHash, prefund, referencedContracts, - aggregator + aggregator: aggregatorInfo?.addr } const index = this._findBySenderNonce(userOp.sender, userOp.nonce) if (index !== -1) { @@ -54,10 +63,11 @@ export class MempoolManager { } else { debug('add userOp', userOp.sender, userOp.nonce) this.entryCount[userOp.sender] = (this.entryCount[userOp.sender] ?? 0) + 1 - this.checkSenderCountInMempool(userOp, senderInfo) + // this.checkSenderCountInMempool(userOp, senderInfo) + this.checkReputationStatus(senderInfo, paymasterInfo, factoryInfo, aggregatorInfo) this.mempool.push(entry) } - this.updateSeenStatus(aggregator, userOp) + this.updateSeenStatus(aggregatorInfo?.addr, userOp) } private updateSeenStatus (aggregator: string | undefined, userOp: UserOperation): void { @@ -66,6 +76,44 @@ export class MempoolManager { this.reputationManager.updateSeenStatus(getAddr(userOp.initCode)) } + + // TODO: de-duplicate code + // TODO 2: use configuration parameters instead of hard-coded constants + private checkReputationStatus ( + senderInfo: StakeInfo, + paymasterInfo?: StakeInfo, + factoryInfo?: StakeInfo, + aggregatorInfo?: StakeInfo): void { + this.reputationManager.checkBanned('account', senderInfo) + if ((this.entryCount[senderInfo.addr] ?? 0) > MAX_MEMPOOL_USEROPS_PER_SENDER) { + this.reputationManager.checkThrottled('account', senderInfo) + } + + if (paymasterInfo != null) { + this.reputationManager.checkBanned('paymaster', paymasterInfo) + if ((this.entryCount[paymasterInfo.addr] ?? 0) > MAX_MEMPOOL_USEROPS_PER_SENDER) { + this.reputationManager.checkThrottled('paymaster', paymasterInfo) + } + } + + if (factoryInfo != null) { + this.reputationManager.checkBanned('deployer', factoryInfo) + if ((this.entryCount[factoryInfo.addr] ?? 0) > MAX_MEMPOOL_USEROPS_PER_SENDER) { + this.reputationManager.checkThrottled('deployer', factoryInfo) + } + } + + if (aggregatorInfo != null) { + this.reputationManager.checkBanned('aggregator', aggregatorInfo) + if ((this.entryCount[aggregatorInfo.addr] ?? 0) > MAX_MEMPOOL_USEROPS_PER_SENDER) { + this.reputationManager.checkThrottled('aggregator', aggregatorInfo) + } + } + + // const paymaster = getAddr(userOp.paymasterAndData) + // const factory = getAddr(userOp.initCode) + } + // check if there are already too many entries in mempool for that sender. // (allow 4 entities if unstaked, or any number if staked) private checkSenderCountInMempool (userOp: UserOperation, senderInfo: StakeInfo): void { diff --git a/packages/bundler/src/modules/ReputationManager.ts b/packages/bundler/src/modules/ReputationManager.ts index 780b0001..781abc2a 100644 --- a/packages/bundler/src/modules/ReputationManager.ts +++ b/packages/bundler/src/modules/ReputationManager.ts @@ -21,7 +21,7 @@ export interface ReputationParams { export const BundlerReputationParams: ReputationParams = { minInclusionDenominator: 10, - throttlingSlack: 10, + throttlingSlack: 5, banSlack: 10 } @@ -58,7 +58,9 @@ export class ReputationManager { * debug: dump reputation map (with updated "status" for each entry) */ dump (): ReputationDump { - return Object.values(this.entries) + const reputationDump = Object.values(this.entries) + console.log(`Reputations dump:\n${JSON.stringify(reputationDump)}`) + return reputationDump } /** @@ -85,6 +87,7 @@ export class ReputationManager { } _getOrCreate (addr: string): ReputationEntry { + addr = addr.toLowerCase() let entry = this.entries[addr] if (entry == null) { this.entries[addr] = entry = { @@ -126,6 +129,7 @@ export class ReputationManager { // https://github.com/eth-infinitism/account-abstraction/blob/develop/eip/EIPS/eip-4337.md#reputation-scoring-and-throttlingbanning-for-paymasters getStatus (addr?: string): ReputationStatus { + addr = addr?.toLowerCase() if (addr == null || this.whitelist.has(addr)) { return ReputationStatus.OK } @@ -175,7 +179,7 @@ export class ReputationManager { */ setReputation (reputations: ReputationDump): ReputationDump { reputations.forEach(rep => { - this.entries[rep.address] = { + this.entries[rep.address.toLowerCase()] = { address: rep.address, opsSeen: rep.opsSeen, opsIncluded: rep.opsIncluded @@ -184,6 +188,27 @@ export class ReputationManager { return this.dump() } + + /** + * check the given address (account/paymaster/deployer/aggregator) is banned + * unlike {@link checkStake} does not check whitelist or stake + */ + checkBanned (title: 'account' | 'paymaster' | 'aggregator' | 'deployer', info: StakeInfo): void { + requireCond(this.getStatus(info.addr) !== ReputationStatus.BANNED, + `${title} ${info.addr} is banned`, + ValidationErrors.Reputation, { [title]: info.addr }) + } + + /** + * check the given address (account/paymaster/deployer/aggregator) is throttled + * unlike {@link checkStake} does not check whitelist or stake + */ + checkThrottled (title: 'account' | 'paymaster' | 'aggregator' | 'deployer', info: StakeInfo): void { + requireCond(this.getStatus(info.addr) !== ReputationStatus.THROTTLED, + `${title} ${info.addr} is throttled`, + ValidationErrors.Reputation, { [title]: info.addr }) + } + /** * check the given address (account/paymaster/deployer/aggregator) is staked * @param title the address title (field name to put into the "data" element)