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.

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

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

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

Conversation

drortirosh
Copy link

add a definition for the term "accountable", used in some EREP rules...

@drortirosh drortirosh requested review from yoavw, shahafn and forshtat May 22, 2024 09:57
@@ -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

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants