AA-213: Standalone ValidationManager package #153
Conversation
forshtat
changed the title
| // SPDX-License-Identifier: GPL-3.0 | ||
| pragma solidity ^0.8.15; | ||
|
|
||
| contract GetCodeHashes { |
| import { checkUserOpRulesViolations } from '@account-abstraction/validation-manager' | ||
|
|
||
| const userOperation: UserOperation = createUserOp() | ||
| checkUserOpRulesViolations(provider, userOperation, entryPoint) |
There was a problem hiding this comment.
Maybe just validateRulesViolations(UserOperation)?
There was a problem hiding this comment.
- the parameters are required.
- methods should have the verb-subject in their names, so its good idea to have UserOp there.
- the librarian is validation-manager, so it makes sense to name it "validate" and not "check"
There was a problem hiding this comment.
- the parameters are required.
- methods should have the verb-subject in their names, so its good idea to have UserOp there.
- the librarian is validation-manager, so it makes sense to name it "validate" and not "check"
The rules themselves are actually called "validation rules" so I don't want to ever have to say "validate validation rules", so I called the function 'check'.
| import { ReputationManager } from './ReputationManager' | ||
| import Debug from 'debug' | ||
| import { ReferencedCodeHashes, StakeInfo, UserOperation, ValidationErrors } from './Types' | ||
| import { UserOperation } from './Types' |
There was a problem hiding this comment.
maybe UserOperation should be moved to "utils" too ?
| export interface StorageMap { | ||
| [address: string]: string | SlotMap | ||
| } | ||
| export const abi = Object.values([ |
There was a problem hiding this comment.
- don't export generic "abi" from a package.
- specifically, this one is used when decoding callstack during trace - it should be declared there, not passed through a chain of methods and packages.
There was a problem hiding this comment.
But this includes ABIs that are only relevant to our project, won't other users want to supply their own ABIs?
There was a problem hiding this comment.
The test ABIs are not needed, only the "public" ones. They were added for debugging-purposes only, and really can be removed.
| import { UserOperationEventEvent } from '@account-abstraction/contracts/dist/types/EntryPoint' | ||
| import { UserOperationReceipt } from '../src/RpcTypes' | ||
| import { ExecutionManager } from '../src/modules/ExecutionManager' | ||
| import { BundlerReputationParams, ReputationManager } from '../src/modules/ReputationManager' | ||
| import { MempoolManager } from '../src/modules/MempoolManager' | ||
| import { ValidationManager } from '../src/modules/ValidationManager' | ||
| import { ValidationManager } from '../../validation-manager/src/ValidationManager' |
There was a problem hiding this comment.
@account-abstraction/validation-manager
There was a problem hiding this comment.
Tests are executed with ts-node which ignores all scopes and just goes with the flow. Most of the files in tests are imported chaotically. I can fix that if needed.
| import { checkRulesViolations } from '@account-abstraction/validation-manager' | ||
|
|
||
| const userOperation: UserOperation = createUserOp() | ||
| checkRulesViolations(provider, userOperation, entryPoint) |
| import { EntryPoint__factory } from '@account-abstraction/contracts' | ||
| import { parseEther } from 'ethers/lib/utils' | ||
| import { Signer } from 'ethers' | ||
| import { BundlerConfig } from '../BundlerConfig' | ||
| import { EventsManager } from './EventsManager' | ||
| import { getNetworkProvider } from '../Config' | ||
| import { abi } from './Types' |
There was a problem hiding this comment.
this abi is a set of ABIs used to simplify decoding of call stacks. it is not a global parameter.
| @@ -0,0 +1,20 @@ | |||
| { | |||
There was a problem hiding this comment.
why we need 2 tsconfig files?
There was a problem hiding this comment.
I am not sure but this is needed for yarn workspaces. Removing these used to break the project in the past, did not try since.
| */ | ||
| function parseCallStack (tracerResults: BundlerCollectorReturn): CallEntry[] { |
There was a problem hiding this comment.
this is a local parser. no need to "externalize" the abi . Maybe remove unneeded (test) contracts, and add IAccount instead
…extract-rules-validator-package
No description provided.