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

op-e2e: Additional Isthmus fee tests #14626

Closed
sebastianst opened this issue Mar 4, 2025 · 3 comments · Fixed by #14752
Closed

op-e2e: Additional Isthmus fee tests #14626

sebastianst opened this issue Mar 4, 2025 · 3 comments · Fixed by #14752
Assignees
Labels
A-integration Area: integration tests H-isthmus Hardfork: change is planned for isthmus upgrade

Comments

@sebastianst
Copy link
Member

The Isthmus Operator Fee is implemented in

We need additional testing to call these implementations production ready, so extend the existing
op-e2e/actions/proofs/operator_fee_test.go tests to have

  • have a transaction which causes a state refund (e.g. SSTORE that sets a storage location that is currently >0 to 0).
  • have a user deposit tx, so that we can assert how user deposit txs are charged (and that the operator fee vault doesn't get the fees in this case)
  • cover the Isthmus transition block, because of the special case of it isthmus: operator fee op-geth#388 (comment).

Ideally the existing tests are parameterized to cover different scenarios, instead of copy-pasting.

It should also be explored in how far the existing fees test in op-e2e/system/fees/fees_test.go can be moved into the actions/proofs package and be made a proofs test, and replace the above test. Those tests already employ parameterization. Maybe it's worth adding an additional parameter for the state refund and user deposit scenarios.

@sebastianst sebastianst added A-integration Area: integration tests H-isthmus Hardfork: change is planned for isthmus upgrade labels Mar 4, 2025
@tynes
Copy link
Contributor

tynes commented Mar 4, 2025

@tynes
Copy link
Contributor

tynes commented Mar 6, 2025

@leruaa do you mind owning this?

@leruaa
Copy link
Contributor

leruaa commented Mar 7, 2025

@tynes Yes I will

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-integration Area: integration tests H-isthmus Hardfork: change is planned for isthmus upgrade
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants