Conversation
WalkthroughThis update upgrades the IBC module from v8 to v10 and removes several dependencies and features. Specifically, references to NFT, Oracle, IBC Fee, Rate Limit, Capability, and CosmWasm integrations have been removed from the application code and configuration. The configuration system has been refactored with new initialization functions, and the genesis configuration has been simplified by deleting NFT and oracle sections. Additionally, various Go dependencies have been updated and unused parameters and functions eliminated to streamline the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RootCmd
participant Config
participant App
User->>RootCmd: Execute command
RootCmd->>Config: initAppConfig() / NewDefaultConfig()
RootCmd->>Config: initCometBFTConfig()
RootCmd->>App: Instantiate application (without WASM options)
App->>App: Set up ante handler & call initializeABCIExtensions()
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
app/ante.go (1)
9-10: 💡 Verification agent🧩 Analysis chain
Inconsistent CosmWasm removal.
While the PR objective is to remove CosmWasm ("no wasm in good cosmos chains"), there are still references to
wasmkeeperandwasmTypesin this file, including in theHandlerOptionsstruct and the ante handler decorators. This suggests that the CosmWasm removal might not be complete.Also applies to: 22-24, 64-67
🏁 Script executed:
#!/bin/bash # Check if there are other files still referencing wasmkeeper or related imports echo "Checking for other files referencing wasmkeeper..." rg --type go "wasmkeeper|wasmTypes|wasmConfig" -A 3 -B 3Length of output: 4400
Critical: Incomplete CosmWasm Removal
The PR’s objective is to eliminate CosmWasm integration entirely, yet multiple files still reference it. In particular, in
app/ante.gothe imports forwasmkeeperandwasmTypes(lines 9–10) continue to be used in theHandlerOptionsstruct and in the ante decorators. Additionally, our search shows that similar references exist in:
- app/queries/queries.go: Usage of
wasmkeeperpersists in the registration of custom query plugins.- app/test_helpers.go: References to
wasmkeeperremain during the test application setup.Please update the PR to remove all CosmWasm dependencies (including related structs, functions, and decorators) across these files. This will fully align the changes with the objective of “no wasm in good cosmos chains.”
🧰 Tools
🪛 golangci-lint (1.64.8)
9-9: could not import github.com/CosmWasm/wasmd/x/wasm/keeper (-: # github.com/CosmWasm/wasmd/x/wasm/keeper
/tmp/go/.go-mod-cache/github.com/!cosm!wasm/wasmd@v0.53.0/x/wasm/keeper/test_common.go:386:3: cannot use runtime.NewKVStoreService(keys[types.StoreKey]) (value of interface type "cosmossdk.io/core/store".KVStoreService) as "cosmossdk.io/store/types".StoreKey value in argument to NewKeeper: "cosmossdk.io/core/store".KVStoreService does not implement "cosmossdk.io/store/types".StoreKey (missing method Name)
/tmp/go/.go-mod-cache/github.com/!cosm!wasm/wasmd@v0.53.0/x/wasm/keeper/test_common.go:400:3: cannot use availableCapabilities (variable of type []string) as string value in argument to NewKeeper)(typecheck)
🧹 Nitpick comments (8)
cmd/horsed/cmd/config.go (1)
7-9: Added new AppConfig type that embeds the server config.This simplifies the configuration structure by leveraging the server's default configuration as a base. Consider adding documentation that explains the purpose of this type and how it extends the server configuration.
type AppConfig struct { + // AppConfig embeds the default server configuration and can be extended with + // application-specific configuration settings if needed. serverconfig.Config }app/test_helpers.go (2)
68-68: Ensure consistency with removed WASM parameters.
This removal streamlines theNewfunction call by droppingopts. Double-check that any leftover WASM references in surrounding code have also been removed.
94-94: Drop unusedWasmOptsin SetupOptions if no longer needed.
Although WASM parameters are removed here,WasmOptsis still present inSetupOptions(line 50). Consider removing it if it is no longer in use anywhere.-type SetupOptions struct { - Logger log.Logger - DB *dbm.MemDB - AppOpts servertypes.AppOptions - WasmOpts []wasmkeeper.Option -} +type SetupOptions struct { + Logger log.Logger + DB *dbm.MemDB + AppOpts servertypes.AppOptions +}cmd/horsed/cmd/commands.go (1)
10-10: Remove unused or unrelated imports.
The import on this line appears new or altered but is not obviously referenced. If it's unused, consider removing it for cleanliness.app/app.go (4)
284-284: Review memory store usage.
Ensure that any modules or features using the newly declared memory store keys are properly finalized or removed if unnecessary.
401-401: Set circuit breaker.
This call ensures the newly introduced circuit breaker is active. Confirm that you have the correct fallback behavior if the breaker toggles.
702-702: Confirm the need for ABCI extension initialization.
app.initializeABCIExtensions()is newly introduced but empty. If this is a placeholder, clarify its responsibilities or remove it until ready.
992-995: Implement or remove stub function.
initializeABCIExtensions()is an empty placeholder. Consider implementing it or removing it to avoid confusion.Would you like help drafting this implementation, or should I open a new issue to track it?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
app/ante.go(1 hunks)app/app.go(12 hunks)app/encoding.go(1 hunks)app/genesis.go(0 hunks)app/oracle.go(0 hunks)app/test_helpers.go(3 hunks)app/test_support.go(1 hunks)cmd/horsed/cmd/commands.go(1 hunks)cmd/horsed/cmd/config.go(1 hunks)cmd/horsed/cmd/root.go(2 hunks)go.mod(14 hunks)networks/horse/genesis.json(0 hunks)networks/horse/genesis.narwhal.json(0 hunks)
💤 Files with no reviewable changes (4)
- networks/horse/genesis.narwhal.json
- networks/horse/genesis.json
- app/genesis.go
- app/oracle.go
🧰 Additional context used
🧬 Code Definitions (2)
cmd/horsed/cmd/root.go (2)
cmd/horsed/cmd/config.go (3)
AppConfig(7-9)DefaultConfigTemplate(17-19)NewDefaultConfig(11-15)app/app.go (1)
New(233-724)
app/test_helpers.go (1)
app/app.go (1)
New(233-724)
🔇 Additional comments (34)
app/test_support.go (1)
8-8: IBC module has been upgraded from v8 to v10.This change is part of a broader effort to upgrade the IBC module throughout the codebase. Ensure that your application is compatible with this newer version and that all IBC-related functionality remains intact.
app/ante.go (1)
13-14: IBC module has been upgraded from v8 to v10.The imports for both
ibcanteandkeeperhave been updated to use v10 of the IBC module, which is consistent with changes in other files.cmd/horsed/cmd/root.go (3)
23-26: Added new function to initialize app config.This function encapsulates the application configuration initialization logic, improving modularity. It uses the newly defined
AppConfigtype fromcmd/horsed/cmd/config.go.
28-31: Added new function to initialize CometBFT config.This function encapsulates the CometBFT configuration initialization logic, improving modularity and code organization.
38-38: Simplified tempApp instantiation by removing wasmkeeper options.This change aligns with the PR objective of removing CosmWasm from the project. The
tempAppis now created without any WASM-specific options, which is consistent with the overall goal of removing CosmWasm dependencies.cmd/horsed/cmd/config.go (2)
11-15: Added function to create a new default AppConfig.This function creates a new
AppConfiginstance with default server configuration settings, which is a good practice for providing sensible defaults.
17-19:Details
❓ Verification inconclusive
Simplified DefaultConfigTemplate function.
The function now directly returns the default configuration template from the server config package, removing any custom templates that might have been included before (such as WASM and Oracle templates according to the AI summary).
🏁 Script executed:
#!/bin/bash # Check if there are any references to custom config templates that might have been removed echo "Checking for references to custom config templates..." rg "WasmTemplate|OracleTemplate" --type goLength of output: 161
DefaultConfigTemplate Simplification – Confirm Custom Template Removal
The updated function now directly returns the default configuration template from the server config package, with any legacy custom templates (e.g., WASM and Oracle templates) removed. Our verification script did not find any references to "WasmTemplate" or "OracleTemplate" across the Go code, indicating that these custom templates have likely been removed. However, please manually verify that no other remnants exist before final approval.
- File:
cmd/horsed/cmd/config.go(Lines 17–19)- Code Snippet:
func DefaultConfigTemplate() string { return serverconfig.DefaultConfigTemplate }app/encoding.go (1)
17-17: Confirm removal of WASM options argument.
This line removes the previously used WASM options. Verify that no remaining code references these WASM options.app/test_helpers.go (1)
265-265: Confirm initialization without WASM options.
This refactor removes the WASM-specific argument. Ensure that no calls to WASM functionality remain.app/app.go (5)
104-109: Upgrade IBC dependencies to v10.
These lines confirm the transition to the latest IBC modules (v10). Verify that other IBC references (e.g., configuration, default settings, Dockerfiles) are also updated to match the new version.
154-155: Check module account permissions.
You're granting minter and burner permissions to bothibctransfertypes.ModuleNameandtokenfactorytypes.ModuleName. Ensure this aligns with your business logic and that neither module inadvertently burns or mints tokens incorrectly.
203-204: Adding IBC keepers.
AddingIBCKeeperand a separateTransferKeeperis a good approach. Confirm that any leftover WASM keeper references have been fully removed.
277-280: Validate migrated store keys.
These lines confirm that store keys for IBC and Transfer are kept, while WASM keys are removed. Double-check that nothing references the old WASM store keys.
736-737: Maintain minimal dependencies in ante handler.
PassingIBCKeeperinto the ante handler is valid if you need IBC checks in transaction processing. Confirm no leftover WASM references are included.go.mod (20)
3-5: Updated Go Version and Toolchain
Thegodirective has been updated to1.23.6and the toolchain is now set togo1.24.1. Please verify that this new toolchain and patch version are compatible with your existing build, CI configuration, and deployment pipelines.
32-33: CosmWasm Dependencies Presence
Despite the PR’s intent to remove CosmWasm integrations, the dependenciesgithub.com/CosmWasm/wasmdandgithub.com/CosmWasm/wasmvm/v2are still present in thego.modfile. Please confirm if these should be removed to fully align with the PR objectives or retained for transitional purposes.
35-41: Core Module Dependency Updates
The core dependencies have been version bumped as follows:
github.com/cometbft/cometbftupdated to v0.38.15github.com/cosmos/cosmos-sdkupdated to v0.50.13github.com/cosmos/ibc-go/v10updated to v10.1.0
These changes are significant and may include breaking changes. Please ensure that thorough integration and regression testing are performed to validate compatibility with the rest of the blockchain application.
55-59: Google and X Tooling Updates
The following dependencies have been updated:
golang.org/x/tools→ v0.29.0google.golang.org/genproto/googleapis/api→ v0.0.0-20241015192408-796eee8c2d53google.golang.org/grpc→ v1.69.0google.golang.org/protobuf→ v1.36.5
Please check that these updates do not introduce new deprecation warnings or compatibility issues with your current implementation.
74-74: Cloud Compute Metadata Dependency Update
The dependencycloud.google.com/go/compute/metadatahas been updated to v0.5.2. Verify that cloud metadata fetching and related functionalities work correctly with this new version.
84-84: DataDog Dependency Update
github.com/DataDog/datadog-gohas been updated to v4.8.3+incompatible. Please ensure that this version aligns with your monitoring and logging integrations, and confirm that there are no breaking changes in its API usage.
92-93: Minor Dependency Updates
The dependencies for:
github.com/bgentry/speakeasy→ updated to v0.2.0github.com/bits-and-blooms/bitset→ updated to v1.17.0
have been updated. It is advisable to run functional tests to confirm that these upgrades do not adversely affect any area of your application that relies on them.
102-102: CockroachDB FIFO Dependency Bump
The modulegithub.com/cockroachdb/fifohas been updated to a newer version. Please validate that the FIFO queue semantics remain intact and that any dependent components function as expected under this update.
130-131: Cryptography and Timer Dependency Updates
Updates have been applied to:
github.com/decred/dcrd/dcrec/secp256k1/v4→ v4.3.0github.com/desertbit/timer→ v1.0.1
These changes are critical for cryptographic operations and timer functionality respectively. Ensure that all related processes—including security checks and time-based operations—are rigorously tested after these upgrades.
144-144: Ethereum Go-Ethereum Dependency Update
github.com/ethereum/go-ethereumis updated to v1.15.5. Confirm that any Ethereum-related utilities or integrations that your project might be using continue to work as expected.
149-149: Sentry Dependency Update
The update ofgithub.com/getsentry/sentry-goto v0.28.1 may affect error monitoring and logging configurations. Please verify that Sentry integration within the application functions correctly with this new version.
235-235: Petermattis Goid Dependency Update
github.com/petermattis/goidhas been updated. Verify that this change does not negatively impact any functionality relying on accurate goroutine or execution context tracking.
270-270: BoltDB Dependency Update (Alpha Release)
go.etcd.io/bboltis now at v1.4.0-alpha.1. Since this is an alpha release, please be cautious and perform comprehensive testing to detect any instabilities or regressions before deploying in production environments.
278-280: OpenTelemetry Dependency Updates
The OpenTelemetry modules –go.opentelemetry.io/otel,otel/metric, andotel/trace– are updated to v1.31.0. Make sure that these telemetry updates integrate fully with your monitoring setup and that data collection remains accurate.
286-286: Go Crypto Dependency Update
golang.org/x/cryptohas been updated to v0.32.0. Confirm that all cryptographic routines continue to operate securely and that there are no unexpected deprecations or API changes impacting security-related code.
288-290: Module, Net, and OAuth2 Dependency Updates
The following updates have been applied:
golang.org/x/mod→ v0.22.0golang.org/x/net→ v0.34.0golang.org/x/oauth2→ v0.23.0
Given the widespread usage of these modules, please verify that module resolution, network operations, and OAuth2 functionalities are not adversely affected.
292-293: System and Terminal Dependency Updates
golang.org/x/sysandgolang.org/x/termhave been updated. It is important to confirm that any system-level interactions or terminal I/O operations behave correctly after these changes.
295-295: Time Dependency Update
golang.org/x/timeis now updated to v0.9.0. Verify that any scheduling or time-based functionality continues to perform as expected.
298-298: Google Genproto RPC Dependency Update
The modulegoogle.golang.org/genproto/googleapis/rpchas been updated to a new version. Please ensure that service definitions and gRPC interactions remain compatible following this update.
303-303: Websocket Dependency Update
nhooyr.io/websocketis updated to v1.8.11. Confirm that websocket-related functionalities throughout the application continue to operate as expected.
| // initialize BaseApp | ||
| app.SetInitChainer(app.InitChainer) | ||
| app.SetPreBlocker(app.PreBlocker) | ||
| app.SetBeginBlocker(app.BeginBlocker) | ||
| app.SetEndBlocker(app.EndBlocker) | ||
|
|
||
| app.MarketMapKeeper.SetHooks(app.OracleKeeper.Hooks()) | ||
| // set denom resolver to test variant. | ||
| // app.FeeMarketKeeper.SetDenomResolver(&ante.DenomResolverImpl{ | ||
| // StakingKeeper: &app.StakingKeeper, | ||
| // }) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid repeating initialization logic.
These lines appear to replicate the BaseApp initialization steps (SetInitChainer, etc.) that are already done earlier. If duplication is unintentional, remove or consolidate them.
-// initialize BaseApp
-app.SetInitChainer(app.InitChainer)
-app.SetPreBlocker(app.PreBlocker)
-app.SetBeginBlocker(app.BeginBlocker)
-app.SetEndBlocker(app.EndBlocker)
+// Initialization of BaseApp is already set above. Remove duplicate calls if unnecessary.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // initialize BaseApp | |
| app.SetInitChainer(app.InitChainer) | |
| app.SetPreBlocker(app.PreBlocker) | |
| app.SetBeginBlocker(app.BeginBlocker) | |
| app.SetEndBlocker(app.EndBlocker) | |
| app.MarketMapKeeper.SetHooks(app.OracleKeeper.Hooks()) | |
| // set denom resolver to test variant. | |
| // app.FeeMarketKeeper.SetDenomResolver(&ante.DenomResolverImpl{ | |
| // StakingKeeper: &app.StakingKeeper, | |
| // }) | |
| // Initialization of BaseApp is already set above. Remove duplicate calls if unnecessary. | |
| // set denom resolver to test variant. | |
| // app.FeeMarketKeeper.SetDenomResolver(&ante.DenomResolverImpl{ | |
| // StakingKeeper: &app.StakingKeeper, | |
| // }) |
1 participant
Summary by CodeRabbit
Refactor
Chores