-
Notifications
You must be signed in to change notification settings - Fork 237
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
Problem: no legacy evm param after multi upgrades #1371
Conversation
WalkthroughThis update encompasses a series of enhancements and fixes focused on improving EVM compatibility, refining configurations, and strengthening testing frameworks. Key changes include a critical RPC bug fix in EVM, module version updates, configuration restructuring for cosmovisor, and the integration of new testing capabilities to ensure upgrade stability and parameter updates. These collective efforts aim to streamline application functionality and enhance operational robustness. 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1371 +/- ##
===========================================
+ Coverage 15.37% 36.55% +21.17%
===========================================
Files 80 129 +49
Lines 6138 9597 +3459
===========================================
+ Hits 944 3508 +2564
- Misses 5118 5712 +594
- Partials 76 377 +301 |
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: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.mod
is excluded by!**/*.mod
go.sum
is excluded by!**/*.sum
Files selected for processing (8)
- CHANGELOG.md (1 hunks)
- app/app.go (2 hunks)
- gomod2nix.toml (1 hunks)
- integration_tests/configs/cosmovisor.jsonnet (1 hunks)
- integration_tests/configs/upgrade-test-package.nix (2 hunks)
- integration_tests/cosmoscli.py (2 hunks)
- integration_tests/test_gov_update_params.py (1 hunks)
- integration_tests/test_upgrade.py (4 hunks)
Files not reviewed due to errors (1)
- (no review received)
Additional comments not posted (11)
integration_tests/configs/upgrade-test-package.nix (2)
3-9
: The addition offetchFlake0
function is correctly implemented for fetching specific GitHub repository revisions.
24-25
: The modifications topkgs.linkFarm
call, including the newreleased0
path for "genesis" and the addition of "v1.1.0", are correctly implemented for upgrade testing.integration_tests/configs/cosmovisor.jsonnet (1)
13-45
: The restructuring of the configuration, including the addition of governance parameters and adjustments to bank parameters, is logically implemented and improves the clarity and functionality of the configuration.integration_tests/test_gov_update_params.py (1)
44-45
: The added assertions for verifying the updates tomerge_netsplit_block
andshanghai_time
in the EVM configuration are correctly implemented and essential for testing the expected behavior.integration_tests/test_upgrade.py (3)
19-19
: The addition of theget_send_enable
function is correctly implemented for verifying send enable parameters as part of the upgrade testing process.
120-143
: Thedo_upgrade
function correctly refactors the upgrade process, improving maintainability and readability. Its implementation, including governance proposal handling and binary switching, is well-executed.
228-231
: The verification of EVM parameters pre and post-upgrades usingcli.query_params
at specific heights is correctly implemented and essential for ensuring the integrity of the EVM configuration throughout the upgrade process.CHANGELOG.md (1)
7-7
: The changelog entry accurately reflects the addition made to the project. It's clear and provides a direct link to the PR for more details.app/app.go (2)
131-131
: The addition ofv0evmtypes
and its usage ininitParamsKeeper
to ensure compatibility with older EVM parameters aligns with the PR's objectives. However, please ensure that the suppression of static analysis warnings with//nolint: staticcheck
is justified and documented to avoid potential issues in the future.Also applies to: 1172-1172
128-134
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-1171]
Ensure thorough testing and verification of the changes, especially how the newly introduced legacy EVM parameters interact with the current system functionalities. It's crucial to confirm that there are no conflicts or regressions introduced by these changes.
Also applies to: 1173-1310
integration_tests/cosmoscli.py (1)
1064-1073
: The addition of**kwargs
toquery_params
enhances method flexibility by allowing the passing of arbitrary keyword arguments. This is a positive change, assumingself.raw
is designed to handle these appropriately.
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: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .github/workflows/build.yml (1 hunks)
- .github/workflows/test.yml (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/test.yml
Additional comments not posted (1)
.github/workflows/build.yml (1)
89-89
: Update ofcodecov/codecov-action
tov4
is approved.Ensure to verify the compatibility of the new action version with the workflow's requirements and check the action's documentation for any breaking changes or new features that could be leveraged.
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: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .github/workflows/lint.yml (1 hunks)
- CHANGELOG.md (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/lint.yml
Additional comments not posted (1)
CHANGELOG.md (1)
9-9
: The documentation of the addition of a parameter keytable in EVM for an old upgrade underv1.2.0-rc1
is clear and accurately reflects the PR objectives.
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make
)make test
)go fmt
)golangci-lint run
)go list -json -m all | nancy sleuth
)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
gomod2nix.toml
to newer versions for better stability and performance.