V9 #468
V9 #468
Conversation
Co-authored-by: Luca Graziotti <[email protected]> Co-authored-by: Julian Kocher <[email protected]> Co-authored-by: Dan Kanefsky <[email protected]>
Co-authored-by: John Letey <[email protected]>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces a series of updates across multiple areas of the codebase. Enhancements include improved ICA channel communication, dependency upgrades, and several new modules (Wormhole, Dollar, Swap) with associated keeper integrations. The changes also update module/version paths to v9, simplify Makefile file searching, and adjust license headers throughout. Additionally, upgrade handler functionality is enhanced with new configuration functions and the genesis script is modified to include wormhole-related settings. Changes
Sequence Diagram(s)sequenceDiagram
participant UH as UpgradeHandler
participant App as Application
participant WM as WormholeModule
participant DM as DollarModule
UH->>App: Trigger upgrade migration
App->>WM: ConfigureWormholeModule(ctx, wormholeKeeper)
WM-->>App: Return configuration status
App->>DM: ConfigureDollarModule(ctx, dollarKeeper)
DM-->>App: Return configuration status
App->>UH: Complete upgrade process
Possibly related PRs
Suggested reviewers
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
upgrade/constants.go (1)
20-33: Upgrade naming and ASCII
Renaming to "argentum" and introducing the ASCII art fosters a distinctive upgrade identity. If you plan to expand the art, consider organizing it in a separate file or resource.local.sh (1)
20-21: New Genesis Account Addition for USDC
The addition of the genesis account fornoble1cyyzpxplxdzkeea7kwsydadg87357qnah9s9cvwith a balance of1000000uusdcis clear and appears to be part of the enhanced module integrations. Ensure that this account and the provided balance are correctly aligned with the intended setup for the Wormhole module or any related financial configuration. A brief inline comment explaining the purpose of this account might help future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
.github/license.ymlis excluded by!**/*.yml.github/workflows/docker-publish.yamlis excluded by!**/*.yaml.github/workflows/draft-release.yamlis excluded by!**/*.yamlapp.yamlis excluded by!**/*.yamle2e/go.modis excluded by!**/*.mode2e/go.sumis excluded by!**/*.sum,!**/*.sumgo.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (34)
.changelog/unreleased/bug-fixes/432-fix-ica.md(1 hunks).changelog/unreleased/dependencies/455-bump-ftf.md(1 hunks).changelog/unreleased/dependencies/464-autocli-maps.md(1 hunks).changelog/unreleased/features/444-integrate-wormhole.md(1 hunks).changelog/unreleased/features/448-integrate-dollar.md(1 hunks).changelog/unreleased/features/449-integrate-swap.md(1 hunks).changelog/unreleased/improvements/443-module-path.md(1 hunks)Makefile(1 hunks)ante.go(1 hunks)app.go(7 hunks)cmd/commands.go(2 hunks)cmd/init.go(2 hunks)cmd/nobled/main.go(2 hunks)cmd/root.go(1 hunks)e2e/cctp_deposit_for_burn_test.go(1 hunks)e2e/cctp_deposit_for_burn_with_caller_test.go(1 hunks)e2e/cctp_receive_message_test.go(1 hunks)e2e/cctp_receive_message_with_caller_test.go(1 hunks)e2e/cctp_replace_deposit_for_burn_test.go(1 hunks)e2e/cctp_roles_test.go(1 hunks)e2e/conformance_test.go(1 hunks)e2e/fiat_tf_test.go(1 hunks)e2e/ibc_utils.go(1 hunks)e2e/module_check_test.go(1 hunks)e2e/upgrade_test.go(4 hunks)e2e/upgrade_utils.go(1 hunks)e2e/utils.go(1 hunks)export.go(1 hunks)legacy.go(3 hunks)local.sh(2 hunks)tools.go(1 hunks)upgrade/constants.go(2 hunks)upgrade/store.go(2 hunks)upgrade/upgrade.go(4 hunks)
✅ Files skipped from review due to trivial changes (20)
- .changelog/unreleased/dependencies/455-bump-ftf.md
- .changelog/unreleased/improvements/443-module-path.md
- ante.go
- e2e/cctp_deposit_for_burn_with_caller_test.go
- e2e/conformance_test.go
- tools.go
- e2e/ibc_utils.go
- e2e/cctp_roles_test.go
- e2e/cctp_receive_message_test.go
- e2e/module_check_test.go
- cmd/commands.go
- e2e/utils.go
- cmd/root.go
- e2e/cctp_receive_message_with_caller_test.go
- e2e/upgrade_utils.go
- export.go
- e2e/cctp_deposit_for_burn_test.go
- cmd/nobled/main.go
- cmd/init.go
- e2e/fiat_tf_test.go
⏰ Context from checks skipped due to timeout of 90000ms (33)
- GitHub Check: test (TestChainUpgrade)
- GitHub Check: test (TestRestrictedModules)
- GitHub Check: test (TestFiatTFIBCIn)
- GitHub Check: test (TestFiatTFIBCOut)
- GitHub Check: test (TestFiatTFBankSend)
- GitHub Check: test (TestFiatTFAuthzSend)
- GitHub Check: test (TestFiatTFAuthzGrant)
- GitHub Check: test (TestFiatTFAuth)
- GitHub Check: test (TestFiatTFBurn)
- GitHub Check: test (TestFiatTFMint)
- GitHub Check: test (TestFiatTFUnblacklist)
- GitHub Check: test (TestFiatTFBlacklist)
- GitHub Check: test (TestFiatTFUpdateBlacklister)
- GitHub Check: test (TestFiatTFUnpause)
- GitHub Check: test (TestFiatTFPause)
- GitHub Check: test (TestFiatTFUpdatePauser)
- GitHub Check: test (TestFiatTFRemoveMinter)
- GitHub Check: test (TestFiatTFConfigureMinter)
- GitHub Check: test (TestFiatTFRemoveMinterController)
- GitHub Check: test (TestFiatTFConfigureMinterController)
- GitHub Check: test (TestFiatTFUpdateMasterMinter)
- GitHub Check: test (TestFiatTFAcceptOwner)
- GitHub Check: test (TestFiatTFUpdateOwner)
- GitHub Check: test (TestConformance)
- GitHub Check: test (TestCCTP_UpdateTokenController)
- GitHub Check: test (TestCCTP_UpdatePauser)
- GitHub Check: test (TestCCTP_UpdateAttesterManager)
- GitHub Check: test (TestCCTP_UpdateOwner)
- GitHub Check: test (TestCCTP_ReplaceDepositForBurn)
- GitHub Check: test (TestCCTP_ReceiveMessageWithCaller)
- GitHub Check: test (TestCCTP_ReceiveMessage)
- GitHub Check: test (TestCCTP_DepositForBurnWithCaller)
- GitHub Check: test (TestCCTP_DepositForBurn)
🔇 Additional comments (29)
upgrade/upgrade.go (8)
1-3: SPDX license header adoption
The updated license header and year reflect best practices for open licensing and compliance.
21-21: New imports
These imports (e.g.,fmt,cosmossdk.io/errors,cosmossdk.io/log,go-ethereum/common, wormhole, and dollar modules) appear purpose-driven and are used effectively in the subsequent module configuration logic.Also applies to: 23-24, 28-28, 35-36, 38-39
45-45: Expanded function signature
Introducinglogger,dollarKeeper, andwormholeKeeperintoCreateUpgradeHandleraligns with your new module configuration calls and ensures clear parameter usage.Also applies to: 47-48
60-63: Wormhole module configuration call
Delegating initialization toConfigureWormholeModuleneatly encapsulates your testnet vs. mainnet logic, making the upgrade process more modular.
64-67: Dollar module configuration call
Similarly, routing Dollar module installation toConfigureDollarModulehelps keep the upgrade logic self-contained and maintainable.
68-69: Informative logging
Adding succinct, celebratory logs can enrich operator experience and provide clear confirmation of a successful upgrade.
102-135: ConfigureWormholeModule function
• Branching on chain ID to set wormhole configuration is well-structured.
• For Testnet, referencing established addresses is good; consider using config or environment variables if these might change.
• The placeholder for Mainnet is clear, but remember to finalize it before release.Would you like a script to search the codebase and ensure no other wormhole config references are missed?
136-162: ConfigureDollarModule function
• Similar approach for chain ID branching and error handling.
• Keep in mind to fill in the Mainnet case soon.
• Validating addresses via environment config or external references can help prevent misconfigurations.upgrade/store.go (3)
1-3: License header
The new year and SPDX updates look consistent.
24-26: Additional module imports
Bringing indollartypes,wormholetypes, andswaptypessupports your new upgrade paths and is consistent with changes elsewhere.
30-37: Store upgrade entries
Listing Dollar, Swap, and Wormhole modules instoreUpgradesensures stores are properly initialized at upgrade. Verify that their store keys don’t conflict with existing modules.upgrade/constants.go (2)
1-3: SPDX license header
Maintaining accurate licensing information is important; these changes are correct.
34-38: Chain ID constants
DefiningTestnetChainIDandMainnetChainIDcentralizes environment checks and prevents accidental typos or mismatches.e2e/upgrade_test.go (3)
42-42: LGTM: Version update for upgrade image.The upgrade image version has been updated from v8.0.0 to v8.0.5.
52-52: LGTM: Fixed typo in comment.Corrected "inital" to "initial" in the comment.
99-99: LGTM: Improved upgrade naming.Changed upgrade name from version-based "v8.1" to semantic name "argentum", which is more meaningful and follows better naming conventions.
legacy.go (2)
45-46: LGTM: Clean IBC router configuration.The IBC router configuration is properly structured with clear route definitions.
Also applies to: 136-137
142-143: LGTM: Proper Wormhole keeper setup.The Wormhole keeper is correctly configured with necessary IBC keepers and scoped keeper.
e2e/cctp_replace_deposit_for_burn_test.go (1)
198-198: Fixed error check function name.Changed from
require.Zeroftorequire.Zerofor consistency.app.go (3)
44-45: LGTM: Clean module imports.New modules (Dollar, Wormhole, Swap) are properly imported.
Also applies to: 61-63
157-161: LGTM: Proper keeper integration.New keepers (Dollar, Swap, Wormhole) are correctly added to the App struct.
321-324: LGTM: Updated upgrade handler.The upgrade handler is properly configured with new keepers (Dollar, Wormhole).
.changelog/unreleased/dependencies/464-autocli-maps.md (1)
1-2: Clear Documentation for Dependency Update.
The changelog entry clearly documents the bump ofcosmossdk.io/client/v2to support returning maps inside AutoCLI queries ([#464]). The message is concise and informative..changelog/unreleased/features/449-integrate-swap.md (1)
1-2: Clear Documentation for Swap Module Integration.
This entry concisely notes the integration of the custom Swap module for token exchange with the appropriate PR reference ([#449]). The information is clear and follows the established format..changelog/unreleased/features/444-integrate-wormhole.md (1)
1-2: Clear Documentation for Wormhole Module Integration.
The changelog entry effectively details the integration of the custom Wormhole module for enabling IBC messaging, with proper referencing to PR ([#444]). The note is clear and consistent with our documentation style..changelog/unreleased/features/448-integrate-dollar.md (1)
1-2: Clear Documentation for Dollar Module Integration.
This changelog entry clearly documents the integration of the custom Dollar module for issuing Noble's stablecoin $USDN along with the related PR reference ([#448]). The entry is straightforward and well formatted..changelog/unreleased/bug-fixes/432-fix-ica.md (1)
1-2: Clear Documentation for ICA Channels Bug-Fix.
The entry clearly communicates the update to the ICA channel capabilities from the ICA Controller module back to the ICA Host module, referencing PR ([#432]). The description is succinct and effective.Makefile (1)
67-67: Review of FILES variable assignment update
The new assignmentFILES := $(shell find . -name "*.go")now collects all Go files recursively, without filtering out generated files (e.g.*.pb.go,*.pb.gw.go,*.pulsar.go). This broader scope might inadvertently include files that are not meant for license checks or linting, so please verify that this behavior is intentional and will not introduce false positives in downstream tooling.local.sh (1)
30-33: Wormhole Module Genesis Configuration Update
The new jq commands updating the genesis configuration to set the wormhole parameters (chain_id,gov_chain,gov_address, andguardian_sets) are correctly added. Please verify that the hardcoded values (e.g., chain id4009, gov chain1, and the specific gov_address and guardian set values) are as intended for your deployment environment. If these values may change based on different deployment scenarios, consider parameterizing them or at least adding clarifying comments for maintainability.
Co-authored-by: John Letey <[email protected]> Co-authored-by: Luca Graziotti <[email protected]>
Co-authored-by: John Letey <[email protected]>
This PR brings the v9 work into main 🥳
IMPORTANT: It should be rebased and not squashed so that the commit history is kept from the development branch!