-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[Cleanup] Avoid importing op-e2e package from op-node #9765
[Cleanup] Avoid importing op-e2e package from op-node #9765
Conversation
WalkthroughWalkthroughThe changes involve refactoring and streamlining the process of proving withdrawal parameters across several files. This includes the introduction of a new package import, adjustments in function calls to utilize a more direct approach, and the division of a key function into two specialized versions for enhanced clarity. The modifications aim to improve code modularity and reduce dependencies, particularly removing unnecessary parameters and simplifying the logic for withdrawal proving. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Is there any reason that whole |
ah, while that would clean up the code, it wouldn't help your withdrawer... Otherwise it does look like it works (at least |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this code with the completely separate paths for fault proof and l2oo withdrawals is strictly better, so will approve and merge this. I am still keen to move all this code out of op-node though and properly reflect that it is actually part of our e2e testing framework.
Interested to hear thoughts on how that impacts the withdrawer and what the right option is to support that. It definitely looks like a useful tool...
8974714
Description
(Package import cleanup PR, no logic changes).
Avoid importing the
op-e2e
package fromop-node
. This package is primarily for E2E testing and should not be depended on from production code.Also the
op-e2e/config
dir contains aninit
function that breaks everything when not in a testing environment.Tests
Existing tests.
Additional context
https://github.com/base-org/withdrawer depends on the
op-node/withdrawals
package and is currently broken due to theinit
function that is imported as a side-effect from thee2eutils.UseFPAC()
call.