-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Add check-fjord script to smoke test live chains #10578
Conversation
Semgrep found 9
No |
Semgrep found 4
Found banned use of |
02d485d
to
34b87cd
Compare
Semgrep found 2 No |
WalkthroughWalkthroughThe recent updates enhance the Fjord upgrade verification in Ethereum. Changes include new check functions and configurations in Changes
Recent Review DetailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 (
|
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
op-chain-ops/cmd/check-fjord/checks/checks.go (2)
72-79
: TheCheckFjordConfig
struct is well-defined and encapsulates all necessary fields for the checks. Consider adding documentation comments to explain the purpose of each field, especially for public or exported types.
364-391
: ThefjordL1Cost
function calculates the L1 cost based on various parameters. The function is well-implemented, but adding inline comments explaining the calculation steps would enhance understanding and maintainability.
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.
Actionable comments posted: 2
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.
This looks great!
I propose to move a few things around.
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.
Actionable comments posted: 3
* Add check-fjord script to smoke test live chains * Fix checkRIP7212 invalid signature test * check-fjord: several fixes * fix callopts * check-fjord: fix bindings import * check-fjord: fix errors & naming * lint * fix commands * Remove unused configuration flags * Add e2e test for check-fjord script * Add test to verify unactivated fjord is properly detected * Refactor check-fjord script and e2e test for cleaner separation * Add tests to ensure all fjord checks error if fork is unactivated * Update op-e2e/check_scripts_test.go --------- Co-authored-by: Sebastian Stammler <seb@oplabs.co>
* Add check-fjord script to smoke test live chains * Fix checkRIP7212 invalid signature test * check-fjord: several fixes * fix callopts * check-fjord: fix bindings import * check-fjord: fix errors & naming * lint * fix commands * Remove unused configuration flags * Add e2e test for check-fjord script * Add test to verify unactivated fjord is properly detected * Refactor check-fjord script and e2e test for cleaner separation * Add tests to ensure all fjord checks error if fork is unactivated * Update op-e2e/check_scripts_test.go --------- Co-authored-by: Sebastian Stammler <seb@oplabs.co>
…0578) * Add check-fjord script to smoke test live chains * Fix checkRIP7212 invalid signature test * check-fjord: several fixes * fix callopts * check-fjord: fix bindings import * check-fjord: fix errors & naming * lint * fix commands * Remove unused configuration flags * Add e2e test for check-fjord script * Add test to verify unactivated fjord is properly detected * Refactor check-fjord script and e2e test for cleaner separation * Add tests to ensure all fjord checks error if fork is unactivated * Update op-e2e/check_scripts_test.go --------- Co-authored-by: Sebastian Stammler <seb@oplabs.co>
Description
Add a "smoke test" script that can be run against live networks to confirm the Fjord hardfork is active and Fjord features are working correctly.
Tests
Added an e2e test that runs all of the
check-fjord
script tests against a mock, test env chain.Metadata