Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

erc-7562.md - define "accountable" #3

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

erc-7562.md - define "accountable" #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ERCS/erc-7562.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,11 @@ The following reputation rules apply for all staked entities, and for unstaked p

### Entity-specific Rules

definition:
**accountable**:an entity that will be banned if the UserOperation revert when creating a bundle (GREP-040)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Accountable" only applies to banning, and not throttling cases? According to this definition a paymaster that returns "valid" for two UserOps separately and then causes a bundle revert, it is "accountable" but a paymaster that just causes a UserOp (or any number of UserOps) to become invalid separately after being valid in 1st simulation (e.g. by flipping a flag in its own storage) is not "accountable". Is that the intention here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are 2 separate mechanisms

  1. opsSeen/opsIncluded on every entity in the userop, that collects "reputation" over time. over a long time, too many opsSeen (w/o opsIncluded, for whatever reason) eventually throttle and ban the entity.
    1a. special case for PM [EREP-015]: its opsSeen is not included for any 2nd validation failure caused by account/factory
  2. [GREP-040] Direct ban, by failing bundle creation (which sets opsSeen=10000, opsIncluded = 0 - a ban for few days)

the question: are there any other case?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are currently the only cases. My question was about the "accountable" definition. Is an entity accountable for any invalidation it causes, or only for the 2nd case?

Copy link
Author

@drortirosh drortirosh May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • the original idea was to manage opsSeen/opsIncluded separated for each entity, regardless of who caused the error (as in some cases, we couldn't really tell)
  • then we added a specific rule (EREP-015) for paymaster
  • maybe we should try to do it for other entities too. That is: with "accountable" entity, we "undo" the increment of other entities.

the flow:

  • sendUserOp: increase opsSeen count for sender, paymaster, factory
  • 2nd validation: depending on staked entity and error cause, undo some opsSeen (need to create a table...)
  • createBundle: if crashes, perform the opsSeen undo above, but also ban the offending entity.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct?

  • when adding a UserOp, increase opsSeen of all entities
  • when handling 2nd validation error:
    • when an error is in factory or account, undo the pm opsSeen

In the table below:

  • n/a - impossible cases
  • There is always "blame": if not in table, then its current column
  • (can't have both factory and sender staked)
staked: AA1 fact AA2 acct AA3 pm
no fact, no stake n/a undo PM
no stakes undo PM blame fact
undo PM
fact undo PM blame fact
undo PM
acct n/a blame acct
undo PM		 blame acct
fact + pm undo PM blame fact
undo PM
acct + pm n/a undo PM

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yoavw , @shahafn , @forshtat - comments?

(opsSeen/opsIncluded cover failures that cause 2nd validation to fail, and over time,
will get banned)

* **[EREP-010]** For each `paymaster`, the mempool must maintain the total gas `UserOperations` using this `paymaster` may consume.
* Do not add a `UserOperation` to the mempool if the maximum total gas cost, including the new `UserOperation`, is above the deposit of the `paymaster` at the current gas price.
* **[EREP-011]** REMOVED
Expand Down