Problem: ibc-go v9 is not used #1731
Conversation
|
Warning Rate limit exceeded@mmsqe has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 11 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughThis PR primarily upgrades numerous modules from ibc-go v8 to v9. It adds a new "State Machine Breaking" section in the changelog to document the upgrade to ibc-go v9.0.x, updates import paths and function arguments across the core application, tests, integration scripts, third-party proto files, and various x/cronos components, and simplifies upgrade logic by removing deprecated functions and tests. Additionally, dependency versions and build configurations in files such as default.nix, go.mod, and gomod2nix.toml have been updated accordingly. Changes
Possibly related PRs
Suggested labels
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 (
|
Solution: - upgrade to v9 to try new features fix middleware build cleanup upgrade bump version bump deps fix middleware
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1731 +/- ##
===========================================
+ Coverage 16.87% 35.57% +18.69%
===========================================
Files 72 126 +54
Lines 6163 11830 +5667
===========================================
+ Hits 1040 4208 +3168
- Misses 5000 7205 +2205
- Partials 123 417 +294
|
yihuang
requested review from
JayT106 and
thomas-nguy
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
x/cronos/middleware/conversion_middleware.go (1)
237-238: Ensure coverage of single-token refund scenario.
getIbcDenomFromDataForRefundis straightforward, but coverage could verify correctness under various token denominators.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 237-238: x/cronos/middleware/conversion_middleware.go#L237-L238
Added lines #L237 - L238 were not covered by testsx/cronos/events/bindings/src/Relayer.sol (1)
72-72: Remove the "mmsqe" typo in the comment.The commented-out event declaration contains an unnecessary "mmsqe" suffix.
- // event Denomination(Cosmos.Denom denom); mmsqe + // event Denomination(Cosmos.Denom denom);CHANGELOG.md (2)
5-9: Changelog Update – New State Machine Breaking Section
A new section titled "State Machine Breaking" has been added under the UNRELEASED category with an entry for PR #1731 documenting the ibc-go v9.0.x upgrade. This clearly communicates the breaking change and its scheduled release date.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
9-9: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
9-10: Formatting Note on Release Date
The release date is formatted with emphasis (Dec 19, 2024). While this is acceptable in changelogs, you might consider using a heading or alternative style if your project’s markdown guidelines prefer a different format for dates.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
9-9: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
📜 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 (82)
CHANGELOG.md(1 hunks)app/app.go(5 hunks)app/sim_test.go(1 hunks)app/upgrades.go(1 hunks)app/upgrades_test.go(0 hunks)default.nix(1 hunks)go.mod(11 hunks)gomod2nix.toml(16 hunks)integration_tests/configs/ibc_timeout.jsonnet(1 hunks)integration_tests/configs/upgrade-test-package.nix(2 hunks)integration_tests/cosmoscli.py(1 hunks)integration_tests/ibc_utils.py(8 hunks)integration_tests/test_ibc.py(1 hunks)integration_tests/test_ibc_rly.py(10 hunks)integration_tests/test_ibc_timeout.py(1 hunks)integration_tests/test_ibc_update_client.py(1 hunks)integration_tests/test_ica.py(1 hunks)integration_tests/test_upgrade.py(2 hunks)nix/default.nix(1 hunks)nix/sources.json(1 hunks)third_party/proto/ibc/applications/fee/v1/ack.proto(1 hunks)third_party/proto/ibc/applications/fee/v1/fee.proto(1 hunks)third_party/proto/ibc/applications/fee/v1/genesis.proto(2 hunks)third_party/proto/ibc/applications/fee/v1/metadata.proto(1 hunks)third_party/proto/ibc/applications/fee/v1/query.proto(3 hunks)third_party/proto/ibc/applications/fee/v1/tx.proto(2 hunks)third_party/proto/ibc/applications/interchain_accounts/controller/v1/controller.proto(1 hunks)third_party/proto/ibc/applications/interchain_accounts/controller/v1/query.proto(1 hunks)third_party/proto/ibc/applications/interchain_accounts/controller/v1/tx.proto(1 hunks)third_party/proto/ibc/applications/interchain_accounts/genesis/v1/genesis.proto(1 hunks)third_party/proto/ibc/applications/interchain_accounts/host/v1/host.proto(1 hunks)third_party/proto/ibc/applications/interchain_accounts/host/v1/query.proto(1 hunks)third_party/proto/ibc/applications/interchain_accounts/host/v1/tx.proto(2 hunks)third_party/proto/ibc/applications/interchain_accounts/v1/account.proto(1 hunks)third_party/proto/ibc/applications/interchain_accounts/v1/metadata.proto(1 hunks)third_party/proto/ibc/applications/interchain_accounts/v1/packet.proto(1 hunks)third_party/proto/ibc/applications/transfer/v1/authz.proto(2 hunks)third_party/proto/ibc/applications/transfer/v1/denomtrace.proto(1 hunks)third_party/proto/ibc/applications/transfer/v1/genesis.proto(1 hunks)third_party/proto/ibc/applications/transfer/v1/query.proto(1 hunks)third_party/proto/ibc/applications/transfer/v1/transfer.proto(2 hunks)third_party/proto/ibc/applications/transfer/v1/tx.proto(4 hunks)third_party/proto/ibc/applications/transfer/v2/genesis.proto(1 hunks)third_party/proto/ibc/applications/transfer/v2/packet.proto(2 hunks)third_party/proto/ibc/applications/transfer/v2/queryv2.proto(1 hunks)third_party/proto/ibc/applications/transfer/v2/token.proto(1 hunks)third_party/proto/ibc/core/channel/v1/channel.proto(2 hunks)third_party/proto/ibc/core/channel/v1/genesis.proto(1 hunks)third_party/proto/ibc/core/channel/v1/query.proto(1 hunks)third_party/proto/ibc/core/channel/v1/tx.proto(2 hunks)third_party/proto/ibc/core/channel/v1/upgrade.proto(1 hunks)third_party/proto/ibc/core/client/v1/client.proto(2 hunks)third_party/proto/ibc/core/client/v1/genesis.proto(2 hunks)third_party/proto/ibc/core/client/v1/query.proto(3 hunks)third_party/proto/ibc/core/client/v1/tx.proto(2 hunks)third_party/proto/ibc/core/commitment/v1/commitment.proto(1 hunks)third_party/proto/ibc/core/commitment/v2/commitment.proto(1 hunks)third_party/proto/ibc/core/connection/v1/connection.proto(2 hunks)third_party/proto/ibc/core/connection/v1/genesis.proto(1 hunks)third_party/proto/ibc/core/connection/v1/query.proto(1 hunks)third_party/proto/ibc/core/connection/v1/tx.proto(3 hunks)third_party/proto/ibc/core/types/v1/genesis.proto(1 hunks)third_party/proto/ibc/lightclients/localhost/v2/localhost.proto(1 hunks)third_party/proto/ibc/lightclients/solomachine/v2/solomachine.proto(1 hunks)third_party/proto/ibc/lightclients/solomachine/v3/solomachine.proto(1 hunks)third_party/proto/ibc/lightclients/tendermint/v1/tendermint.proto(2 hunks)x/cronos/client/cli/tx.go(1 hunks)x/cronos/events/bindings/cosmos/lib/cosmos_types.abigen.go(2 hunks)x/cronos/events/bindings/cosmos/precompile/relayer/i_relayer_module.abigen.go(5 hunks)x/cronos/events/bindings/src/CosmosTypes.sol(1 hunks)x/cronos/events/bindings/src/Relayer.sol(2 hunks)x/cronos/events/decoders.go(3 hunks)x/cronos/events/events.go(3 hunks)x/cronos/keeper/ibc.go(1 hunks)x/cronos/keeper/keeper.go(2 hunks)x/cronos/keeper/mock/ibckeeper_mock.go(2 hunks)x/cronos/keeper/params.go(2 hunks)x/cronos/keeper/precompiles/ica.go(1 hunks)x/cronos/keeper/precompiles/relayer.go(2 hunks)x/cronos/keeper/precompiles/utils.go(2 hunks)x/cronos/middleware/conversion_middleware.go(6 hunks)x/cronos/types/interfaces.go(2 hunks)
💤 Files with no reviewable changes (1)
- app/upgrades_test.go
✅ Files skipped from review due to trivial changes (33)
- third_party/proto/ibc/lightclients/localhost/v2/localhost.proto
- third_party/proto/ibc/applications/interchain_accounts/v1/metadata.proto
- third_party/proto/ibc/core/channel/v1/channel.proto
- third_party/proto/ibc/applications/interchain_accounts/v1/account.proto
- third_party/proto/ibc/core/connection/v1/connection.proto
- third_party/proto/ibc/applications/transfer/v1/genesis.proto
- third_party/proto/ibc/core/connection/v1/genesis.proto
- third_party/proto/ibc/applications/fee/v1/tx.proto
- third_party/proto/ibc/applications/interchain_accounts/host/v1/host.proto
- third_party/proto/ibc/lightclients/solomachine/v3/solomachine.proto
- third_party/proto/ibc/applications/fee/v1/genesis.proto
- third_party/proto/ibc/applications/fee/v1/ack.proto
- default.nix
- third_party/proto/ibc/core/connection/v1/query.proto
- third_party/proto/ibc/applications/fee/v1/fee.proto
- third_party/proto/ibc/applications/interchain_accounts/v1/packet.proto
- third_party/proto/ibc/core/channel/v1/upgrade.proto
- third_party/proto/ibc/core/channel/v1/query.proto
- third_party/proto/ibc/applications/interchain_accounts/controller/v1/query.proto
- third_party/proto/ibc/applications/interchain_accounts/controller/v1/tx.proto
- third_party/proto/ibc/lightclients/tendermint/v1/tendermint.proto
- third_party/proto/ibc/lightclients/solomachine/v2/solomachine.proto
- third_party/proto/ibc/core/types/v1/genesis.proto
- third_party/proto/ibc/applications/interchain_accounts/controller/v1/controller.proto
- third_party/proto/ibc/core/channel/v1/genesis.proto
- third_party/proto/ibc/applications/fee/v1/metadata.proto
- third_party/proto/ibc/applications/interchain_accounts/host/v1/query.proto
- third_party/proto/ibc/applications/interchain_accounts/genesis/v1/genesis.proto
- x/cronos/client/cli/tx.go
- x/cronos/keeper/precompiles/ica.go
- x/cronos/keeper/ibc.go
- third_party/proto/ibc/applications/fee/v1/query.proto
- third_party/proto/ibc/core/channel/v1/tx.proto
🧰 Additional context used
🪛 Buf (1.47.2)
third_party/proto/ibc/core/commitment/v2/commitment.proto
3-3: Files with package "ibc.core.commitment.v2" must be within a directory "ibc/core/commitment/v2" relative to root but were in directory "third_party/proto/ibc/core/commitment/v2".
(PACKAGE_DIRECTORY_MATCH)
third_party/proto/ibc/applications/transfer/v1/denomtrace.proto
3-3: Files with package "ibc.applications.transfer.v1" must be within a directory "ibc/applications/transfer/v1" relative to root but were in directory "third_party/proto/ibc/applications/transfer/v1".
(PACKAGE_DIRECTORY_MATCH)
third_party/proto/ibc/applications/transfer/v2/genesis.proto
7-7: import "ibc/applications/transfer/v1/transfer.proto": file does not exist
(COMPILE)
third_party/proto/ibc/applications/transfer/v1/authz.proto
7-7: import "cosmos_proto/cosmos.proto": file does not exist
(COMPILE)
third_party/proto/ibc/applications/transfer/v2/token.proto
7-7: import "ibc/applications/transfer/v1/transfer.proto": file does not exist
(COMPILE)
third_party/proto/ibc/applications/transfer/v1/transfer.proto
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
third_party/proto/ibc/applications/transfer/v2/packet.proto
7-7: import "ibc/applications/transfer/v2/token.proto": file does not exist
(COMPILE)
third_party/proto/ibc/core/client/v1/query.proto
7-7: import "cosmos/base/query/v1beta1/pagination.proto": file does not exist
(COMPILE)
third_party/proto/ibc/applications/transfer/v2/queryv2.proto
7-7: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
🪛 GitHub Check: codecov/patch
x/cronos/keeper/precompiles/utils.go
[warning] 37-40: x/cronos/keeper/precompiles/utils.go#L37-L40
Added lines #L37 - L40 were not covered by tests
[warning] 45-45: x/cronos/keeper/precompiles/utils.go#L45
Added line #L45 was not covered by tests
x/cronos/keeper/mock/ibckeeper_mock.go
[warning] 34-34: x/cronos/keeper/mock/ibckeeper_mock.go#L34
Added line #L34 was not covered by tests
app/upgrades.go
[warning] 12-12: app/upgrades.go#L12
Added line #L12 was not covered by tests
x/cronos/events/decoders.go
[warning] 65-65: x/cronos/events/decoders.go#L65
Added line #L65 was not covered by tests
[warning] 88-98: x/cronos/events/decoders.go#L88-L98
Added lines #L88 - L98 were not covered by tests
[warning] 101-122: x/cronos/events/decoders.go#L101-L122
Added lines #L101 - L122 were not covered by tests
[warning] 124-124: x/cronos/events/decoders.go#L124
Added line #L124 was not covered by tests
x/cronos/keeper/precompiles/relayer.go
[warning] 163-164: x/cronos/keeper/precompiles/relayer.go#L163-L164
Added lines #L163 - L164 were not covered by tests
x/cronos/events/bindings/cosmos/lib/cosmos_types.abigen.go
[warning] 261-267: x/cronos/events/bindings/cosmos/lib/cosmos_types.abigen.go#L261-L267
Added lines #L261 - L267 were not covered by tests
[warning] 269-269: x/cronos/events/bindings/cosmos/lib/cosmos_types.abigen.go#L269
Added line #L269 was not covered by tests
[warning] 276-277: x/cronos/events/bindings/cosmos/lib/cosmos_types.abigen.go#L276-L277
Added lines #L276 - L277 were not covered by tests
[warning] 283-284: x/cronos/events/bindings/cosmos/lib/cosmos_types.abigen.go#L283-L284
Added lines #L283 - L284 were not covered by tests
x/cronos/middleware/conversion_middleware.go
[warning] 105-105: x/cronos/middleware/conversion_middleware.go#L105
Added line #L105 was not covered by tests
[warning] 107-107: x/cronos/middleware/conversion_middleware.go#L107
Added line #L107 was not covered by tests
[warning] 112-126: x/cronos/middleware/conversion_middleware.go#L112-L126
Added lines #L112 - L126 were not covered by tests
[warning] 142-142: x/cronos/middleware/conversion_middleware.go#L142
Added line #L142 was not covered by tests
[warning] 151-151: x/cronos/middleware/conversion_middleware.go#L151
Added line #L151 was not covered by tests
[warning] 155-166: x/cronos/middleware/conversion_middleware.go#L155-L166
Added lines #L155 - L166 were not covered by tests
[warning] 181-181: x/cronos/middleware/conversion_middleware.go#L181
Added line #L181 was not covered by tests
[warning] 184-184: x/cronos/middleware/conversion_middleware.go#L184
Added line #L184 was not covered by tests
[warning] 188-199: x/cronos/middleware/conversion_middleware.go#L188-L199
Added lines #L188 - L199 were not covered by tests
[warning] 212-212: x/cronos/middleware/conversion_middleware.go#L212
Added line #L212 was not covered by tests
[warning] 214-214: x/cronos/middleware/conversion_middleware.go#L214
Added line #L214 was not covered by tests
[warning] 217-217: x/cronos/middleware/conversion_middleware.go#L217
Added line #L217 was not covered by tests
[warning] 221-221: x/cronos/middleware/conversion_middleware.go#L221
Added line #L221 was not covered by tests
[warning] 223-223: x/cronos/middleware/conversion_middleware.go#L223
Added line #L223 was not covered by tests
[warning] 237-238: x/cronos/middleware/conversion_middleware.go#L237-L238
Added lines #L237 - L238 were not covered by tests
[warning] 244-247: x/cronos/middleware/conversion_middleware.go#L244-L247
Added lines #L244 - L247 were not covered by tests
[warning] 251-253: x/cronos/middleware/conversion_middleware.go#L251-L253
Added lines #L251 - L253 were not covered by tests
🪛 Ruff (0.8.2)
integration_tests/test_ibc_rly.py
120-120: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
9-9: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
🔇 Additional comments (115)
app/upgrades.go (1)
12-12: Consider adding a test scenario for the new upgrade plan.Currently, the plan name
"v1.5"is not covered by tests according to the static analysis hints. It would be beneficial to write a test that confirms the upgrade handler is invoked as expected and that no regressions occur for subsequent upgrades.Would you like me to provide a boilerplate test to verify the upgrade plan execution?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 12-12: app/upgrades.go#L12
Added line #L12 was not covered by testsx/cronos/middleware/conversion_middleware.go (6)
9-12: No immediate issues with the updated imports.Upgrading imports to
v9is correct and aligns with the PR objective of adopting ibc-go v9. Ensure there are no conflicting references left behind.
99-132: Enhance coverage and consider testing multi-token scenarios.The loop processes multiple tokens (
data.Tokens), but these lines are not covered by tests according to the static analysis hints. Verify that each token is properly converted and that failures for one token don't inadvertently affect others.A specialized unit test with multiple tokens and partial-failure edge cases would be beneficial.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 105-105: x/cronos/middleware/conversion_middleware.go#L105
Added line #L105 was not covered by tests
[warning] 107-107: x/cronos/middleware/conversion_middleware.go#L107
Added line #L107 was not covered by tests
[warning] 112-126: x/cronos/middleware/conversion_middleware.go#L112-L126
Added lines #L112 - L126 were not covered by tests
135-173: Add acknowledgement error-path tests.When an error acknowledgement is received, the logic triggers a refund loop for multiple tokens. It's crucial to test partial and full refund scenarios to ensure correctness.
I can generate a test snippet to cover various packet acknowledgement outcomes if needed.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 142-142: x/cronos/middleware/conversion_middleware.go#L142
Added line #L142 was not covered by tests
[warning] 151-151: x/cronos/middleware/conversion_middleware.go#L151
Added line #L151 was not covered by tests
[warning] 155-166: x/cronos/middleware/conversion_middleware.go#L155-L166
Added lines #L155 - L166 were not covered by tests
175-203: Confirm timeout refund logic with multi-token support.Similarly, ensure each token is refunded correctly on timeouts. Consider boundary cases, such as zero tokens or extremely large token amounts, to guarantee no overflow or logic errors.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 181-181: x/cronos/middleware/conversion_middleware.go#L181
Added line #L181 was not covered by tests
[warning] 184-184: x/cronos/middleware/conversion_middleware.go#L184
Added line #L184 was not covered by tests
[warning] 188-199: x/cronos/middleware/conversion_middleware.go#L188-L199
Added lines #L188 - L199 were not covered by tests
205-226: Validate conversion logic when parsing 'amount'.
convertVouchersnow relies on a string for the token amount. Confirm behavior for odd or invalid amounts (e.g., non-numeric strings) is adequately tested.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 212-212: x/cronos/middleware/conversion_middleware.go#L212
Added line #L212 was not covered by tests
[warning] 214-214: x/cronos/middleware/conversion_middleware.go#L214
Added line #L214 was not covered by tests
[warning] 217-217: x/cronos/middleware/conversion_middleware.go#L217
Added line #L217 was not covered by tests
[warning] 221-221: x/cronos/middleware/conversion_middleware.go#L221
Added line #L221 was not covered by tests
[warning] 223-223: x/cronos/middleware/conversion_middleware.go#L223
Added line #L223 was not covered by tests
244-253: Check updated denom-trace logic for correctness.Here, the function manipulates traces based on whether the source port/channel matches. Consider adding a test verifying it correctly derives IBC denoms across diverse path configurations.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 244-247: x/cronos/middleware/conversion_middleware.go#L244-L247
Added lines #L244 - L247 were not covered by tests
[warning] 251-253: x/cronos/middleware/conversion_middleware.go#L251-L253
Added lines #L251 - L253 were not covered by testsapp/app.go (4)
117-135: Imports upgraded to ibc-go v9 look consistent.These references align with the overall ibc-go v9 migration. Confirm that no usage of the old v8 imports remains, to avoid potential conflicts.
645-645: Verify gRPC query router usage for ICAHostKeeper.This additional parameter helps in query management, but ensure relevant gRPC endpoints are tested to confirm correct host-keeper behavior.
749-750: Include end-to-end test for the new ICA Controller stack initialization.The updated middleware references the ICS29 fee module and
ICAControllerKeeper. Ensure the new stack is tested, particularly for multi-hop or advanced scenarios.
771-775: Add coverage for light client route registration.Registering the Tendermint light client module is essential for IBC functionality, yet code coverage tools indicate no coverage. A dedicated integration or unit test verifying correct route setup is advisable.
x/cronos/events/bindings/src/CosmosTypes.sol (2)
9-20: LGTM! Well-structured token model implementation.The new structs (
Hop,Denom,Token) are well-designed and align with the ibc-go v9 token model. The implementation follows Solidity best practices.
25-25: LGTM! Function signature matches the new token model.The
tokenfunction is correctly added to handle the new token model.integration_tests/configs/ibc_timeout.jsonnet (2)
14-16: LGTM! Added IBC timeout parameter.The
ibc_timeoutparameter is correctly added to the cronos params.
23-29: LGTM! Enhanced fee grant configuration.The new
feegrantsstructure provides more granular control with well-defined fields for managing fee grants.x/cronos/keeper/mock/ibckeeper_mock.go (1)
8-8: LGTM! Successfully migrated to ibc-go v9's token model.The changes correctly implement the new token model with proper import path and method signature updates.
Please ensure test coverage for the default case (line 34). Consider adding a test case where
denomTraceHashdoesn't match any of the predefined values.Also applies to: 17-35
integration_tests/configs/upgrade-test-package.nix (1)
24-26: LGTM! Version references updated correctly.The version updates and commit hash changes are consistent with the project's versioning scheme.
Also applies to: 51-51
x/cronos/keeper/precompiles/utils.go (2)
25-28: LGTM! Interface simplification looks good.The removal of the
NativeMessageinterface in favor ofproto.Messagesimplifies the code while maintaining functionality.
37-40: Add test coverage for the new error handling logic.The new error handling for signer retrieval is more robust, but it lacks test coverage.
Please add test cases to cover:
- Valid signer retrieval
- Error case when signer retrieval fails
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 37-40: x/cronos/keeper/precompiles/utils.go#L37-L40
Added lines #L37 - L40 were not covered by testsintegration_tests/test_ibc_timeout.py (1)
21-25: Verify if the ready check is still needed with ibc-go v9.The removal of
assert_ready(ibc)might affect test reliability. Please confirm:
- Is this check no longer necessary with ibc-go v9?
- Are there any new recommended ways to verify IBC readiness?
✅ Verification successful
Review Comment: Removal of
assert_readyin test_ibc_timeout ConfirmedOur search shows that the only reference to
assert_readyexists inintegration_tests/ibc_utils.py—it’s defined and invoked only there. There is no usage of the check in other test files (including intest_ibc_timeout.py), which indicates that with the move to ibc-go v9 the explicit readiness check has been removed intentionally. This suggests that IBC readiness is now either implicitly guaranteed or handled differently.
- The only occurrence of
assert_readyis inintegration_tests/ibc_utils.py.- No alternative or supplementary IBC readiness verification is found in the test files.
- It is likely that the removal reflects updated practices with ibc-go v9 rather than an oversight.
Based on the current evidence, the removal does not appear to negatively impact test reliability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other uses of assert_ready in test files rg "assert_ready\(" integration_tests/Length of output: 147
x/cronos/keeper/params.go (1)
8-8: LGTM! Import path updated correctly.The import path has been updated to use ibc-go v9.
nix/default.nix (1)
68-75: LGTM! Build configuration updates look good.The changes appropriately update:
- Go build module version to 1.23
- Vendor hash for updated dependencies
- GOWORK environment variable syntax
x/cronos/events/bindings/src/Relayer.sol (1)
61-64: LGTM: Updated FungibleTokenPacket event signature.The event signature has been updated to use
Cosmos.Token[]to support multiple tokens, aligning with ibc-go v9's token structure.x/cronos/events/events.go (2)
6-8: LGTM: Updated import paths to ibc-go v9.Import paths have been correctly updated to use ibc-go v9 modules.
21-22: LGTM: Updated RelayerValueDecoders for new token structure.The decoders have been updated to handle the new token structure in ibc-go v9:
- Added
ConvertTokensforAttributeKeyTokens- Added
ConvertAmountforsdk.AttributeKeyAmount- Updated refund token handling
Also applies to: 40-40
integration_tests/test_ibc_update_client.py (1)
44-45: Provide rationale for skipping the test.The test is being skipped with a comment suggesting to use
ibc_update_client_with_headerinstead. Please:
- Document why the test is being skipped
- Provide a replacement test implementation using
ibc_update_client_with_header- Or create a follow-up issue to track this if it's a temporary measure
x/cronos/events/decoders.go (3)
88-99: Add test coverage for ConvertTokens function.The new
ConvertTokensfunction lacks test coverage. Please add unit tests to verify:
- Successful token conversion
- Error handling for invalid JSON
- Error handling for token conversion failures
Would you like me to help generate the unit tests for this function?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 88-98: x/cronos/events/decoders.go#L88-L98
Added lines #L88 - L98 were not covered by tests
101-125: Add test coverage for sdkTokensToEvmTokens function.The new
sdkTokensToEvmTokensfunction lacks test coverage. Please add unit tests to verify:
- Successful conversion of single and multiple tokens
- Error handling for invalid amounts
- Correct handling of denomination traces
Would you like me to help generate the unit tests for this function?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 101-122: x/cronos/events/decoders.go#L101-L122
Added lines #L101 - L122 were not covered by tests
[warning] 124-124: x/cronos/events/decoders.go#L124
Added line #L124 was not covered by tests
65-75: LGTM: Updated ConvertAmount function.The function has been simplified by removing the unused
indexedparameter.Please add test coverage for this modified function.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 65-65: x/cronos/events/decoders.go#L65
Added line #L65 was not covered by testsx/cronos/types/interfaces.go (2)
11-11: LGTM: Import paths updated to ibc-go v9.The import paths have been correctly updated from v8 to v9 as part of the ibc-go upgrade.
Also applies to: 18-21
40-40: Breaking change: GetDenomTrace replaced with GetDenom.The method signature change from
GetDenomTracetoGetDenomwith a different return type is a breaking change. Please ensure all callers are updated accordingly.Run the following script to verify all callers have been updated:
✅ Verification successful
I'll run an additional search (case-insensitive) to double-check if any instance of
GetDenomTraceremains in the codebase.If this search produces no output, it verifies that all callers have been updated to use
GetDenomas expected. Please run this script and review its output. If nothing is found, you can conclude that the breaking change has been fully applied.Next Step:
- Run the above script and confirm that no output is produced.
Reference Update Verified: All callers have been updated to use
GetDenomas expected.
- No instances of
GetDenomTracewere found in any part of the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find any remaining references to GetDenomTrace # Search for GetDenomTrace usage rg -A 5 'GetDenomTrace'Length of output: 23
Script:
#!/bin/bash # Re-run ripgrep to search for any instance of "GetDenomTrace" (case-insensitive) rg -i 'GetDenomTrace'Length of output: 21
integration_tests/test_ica.py (1)
119-120: Verify the impact of removing channel upgrade logic.The channel upgrade functionality has been removed with a comment indicating that
ibc_upgrade_channelsdoes not work. This could impact the test coverage of channel upgrade scenarios.Would you like me to help investigate why
ibc_upgrade_channelsis not working or propose an alternative approach for testing channel upgrades?integration_tests/test_ibc.py (1)
68-69: Consistent with ICA test: Channel upgrade functionality removed.The removal of channel upgrade logic is consistent with the changes in
test_ica.py. This further confirms a broader issue with theibc_upgrade_channelsfunctionality.x/cronos/keeper/precompiles/relayer.go (2)
17-19: LGTM: Import paths updated to ibc-go v9.The import paths have been correctly updated from v8 to v9 as part of the ibc-go upgrade.
163-164: Add test coverage for new receiver chain source logic.The new logic for determining the receiver chain source using
ExtractDenomFromPathandHasPrefixis not covered by tests.Would you like me to help generate test cases to cover this new logic? The test should verify:
- The behavior when the receiver chain is the source
- The behavior when the receiver chain is not the source
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 163-164: x/cronos/keeper/precompiles/relayer.go#L163-L164
Added lines #L163 - L164 were not covered by testsintegration_tests/test_upgrade.py (1)
168-168: LGTM! Upgrade test updated for v1.5.The changes correctly update the upgrade test to handle the new version v1.5, maintaining consistency with the existing upgrade test pattern.
Also applies to: 305-305
x/cronos/keeper/keeper.go (1)
18-22: LGTM! IBC-go upgrade and interface implementation verification.The changes correctly:
- Update IBC-related imports from v8 to v9
- Add type assertion to verify that Keeper implements the ContractKeeper interface
Also applies to: 52-52
integration_tests/test_ibc_rly.py (1)
74-89: LGTM! Enhanced token handling with trace information.The changes improve token handling by:
- Adding structured token representation with trace information
- Updating test cases to use the new token structure
- Maintaining consistency in token denomination handling
Also applies to: 292-292, 310-316, 354-360
x/cronos/events/bindings/cosmos/lib/cosmos_types.abigen.go (1)
38-54: Add test coverage for new Cosmos types.The new Cosmos types and Token method bindings lack test coverage. While this is an auto-generated file, it's important to ensure the generated code works as expected.
Please add test cases that cover:
- Creation and manipulation of CosmosDenom, CosmosHop, and CosmosToken structs
- Invocation of the Token method with various inputs
Would you like me to help generate test cases for these new types and methods?
Also applies to: 258-285
app/sim_test.go (1)
38-40: LGTM! Import paths updated for ibc-go v9.The import paths have been correctly updated from v8 to v9 for the ibc-go modules.
integration_tests/ibc_utils.py (3)
4-4: LGTM! Required import for environment variables.The
osimport is needed for accessing environment variables in the relayer key restoration process.
399-399: Verify test coverage impact of reducing user count.The number of test users has been reduced from 50 to 49, which could impact test coverage.
Could you explain the rationale behind reducing the number of test users? This change might affect the test coverage of edge cases or concurrent operations.
439-439: LGTM! Updated denom trace query to use chainmain node.The denom trace query has been updated to use the chainmain node, which aligns with ibc-go v9 changes.
integration_tests/cosmoscli.py (1)
413-414: Breaking change: ICA packet timeout specification updated.The packet timeout specification has changed from relative to absolute timestamp. This is a breaking change from ibc-go v8 to v9 that affects how IBC packet timeouts are handled.
Please verify that all IBC packet timeout handling in the codebase has been updated accordingly. Run this script to check for any remaining relative timeout references:
✅ Verification successful
Breaking change validated: No remaining relative timeout references found.
Based on our searches, no occurrences of
--relative-packet-timeoutor regex matches for relative timeout references were found across the codebase, confirming the update to absolute timeout timestamps.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining relative packet timeout references # Search for relative packet timeout references echo "Searching for relative packet timeout references..." rg "relative.*packet.*timeout|packet.*timeout.*relative" -A 5Length of output: 176
Script:
#!/bin/bash # Description: Confirm that no --relative-packet-timeout references remain in the codebase. echo "Searching for '--relative-packet-timeout' references..." rg "--relative-packet-timeout" -A 5Length of output: 590
Script:
#!/bin/bash # Description: Correctly search for '--relative-packet-timeout' references using literal mode. echo "Searching for '--relative-packet-timeout' references..." rg -- '--relative-packet-timeout' -A 5Length of output: 250
x/cronos/events/bindings/cosmos/precompile/relayer/i_relayer_module.abigen.go (2)
38-54: LGTM: New token structures align with ibc-go v9 upgrade.The new structs
CosmosDenom,CosmosHop, andCosmosTokenprovide a more structured way to handle token denominations and their traces. The implementation correctly follows the protobuf definitions.
1310-1316: Verify event signature hash after ABI changes.The FungibleTokenPacket event signature has been updated to use the new token structure. Ensure all consumers of this event are updated to handle the new format.
Run this script to verify the event signature:
third_party/proto/ibc/core/commitment/v2/commitment.proto (1)
7-12: LGTM: Clear and well-documented MerklePath message.The MerklePath message is well structured with:
- Clear documentation explaining its purpose
- Appropriate field type (repeated bytes) for key_path
- Root-to-leaf representation noted in comments
third_party/proto/ibc/applications/transfer/v1/denomtrace.proto (1)
7-16: Verify migration plan for deprecated DenomTrace.The DenomTrace message is marked as deprecated. Ensure there's a clear migration path to the new token structures in v2.
Run this script to find usages of the deprecated message:
third_party/proto/ibc/applications/transfer/v2/token.proto (1)
10-24: LGTM: Well-structured token definitions with proper nullability constraints.The Token and Denom messages are well designed with:
- Clear field documentation
- Proper nullability constraints using gogoproto
- Consistent field numbering (note the jump from 1 to 3 in Denom message)
Verify the field numbering in the Denom message:
✅ Verification successful
Verification Complete: Field numbering in the Denom message confirmed.
- The grep output shows no occurrence of field number 2 in the Denom message.
- The Denom message correctly uses field number 1 for
baseand field number 3 fortrace, leaving a deliberate gap.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any field number 2 usage in Denom message # Search for field number 2 in proto files rg "= 2;" third_party/proto/ibc/applications/transfer/v2/Length of output: 419
third_party/proto/ibc/core/commitment/v1/commitment.proto (2)
5-5: Update go_package Option
Thego_packageoption has been updated to use ibc-go v9. Please confirm that the new package path
"github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types"
aligns with the expected module layout in v9.
7-7: Verify gogoproto Import Existence
Static analysis reports that"gogoproto/gogo.proto"might not be found. Please verify that this dependency is available in your build environment or adjust the import path if needed.🧰 Tools
🪛 Buf (1.47.2)
7-7: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
third_party/proto/ibc/applications/transfer/v1/transfer.proto (3)
5-5: Review gogoproto Import
Static analysis warns that"gogoproto/gogo.proto"may not exist. Ensure the dependency is correctly set up or update the import path accordingly.🧰 Tools
🪛 Buf (1.47.2)
5-5: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
7-7: Update go_package Option
Thego_packageoption has been updated to reflect ibc-go v9. Please double-check that"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"is the correct path per the new module specifications.
22-38: Introduce New Forwarding and Hop Messages
The addition of theForwardingandHopmessages introduces new functionality for handling multihop transfers. Verify that the field definitions and options (e.g.,(gogoproto.nullable) = falsefor the repeatedhops) meet the updated protocol requirements and work as expected in the broader application context.third_party/proto/ibc/applications/transfer/v2/genesis.proto (4)
11-11: Review gogoproto Import
Similar to other files, static analysis indicates a potential issue with"gogoproto/gogo.proto". Please verify that this dependency is present or adjust the path accordingly.
13-25: Review New GenesisState Message Definition
The newly addedGenesisStatemessage is clearly defined, with fields forport_id,denoms,params,total_escrowed, andforwarded_packets. Verify that:
- The repeated
Denomfield (with customgogoprotooptions) aligns with the intended serialization behavior.- The reference to
ibc.applications.transfer.v1.Paramsis correct within the upgrade context.
27-31: Review New ForwardedPacket Message
TheForwardedPacketmessage is added for handling the forwarded packets lifecycle. Please confirm that the typesPacketIdandPacket(referenced from"ibc/core/channel/v1/channel.proto") are correctly integrated and that the non-nullable constraints meet the protocol’s requirements.
7-7:⚠️ Potential issueVerify Transfer Proto Import Path
The import"ibc/applications/transfer/v1/transfer.proto"(line 7) is flagged by static analysis as not found. Ensure that the file path is accurate relative to the repository structure or update it if the v1 file has been moved/renamed in the upgrade process.🧰 Tools
🪛 Buf (1.47.2)
7-7: import "ibc/applications/transfer/v1/transfer.proto": file does not exist
(COMPILE)
third_party/proto/ibc/core/client/v1/genesis.proto (3)
5-5: Update go_package Option
The update to thego_packageoption—"github.com/cosmos/ibc-go/v9/modules/core/02-client/types"—is consistent with the v9 upgrade. Ensure that this change is propagated to all relevant client modules.
8-8: Verify gogoproto Import
As in other proto files, confirm that the import"gogoproto/gogo.proto"is properly resolved in your build environment.
28-30: Clarify GenesisMetadata Comment
The updated comment forGenesisMetadatanow indicates that it will be used to export all client store keys that are not client or consensus states. This provides better clarity. Ensure this documentation aligns with the intended use case in your system.third_party/proto/ibc/applications/transfer/v2/queryv2.proto (7)
5-5: Update go_package Option
Thego_packageoption have been updated to"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types", which aligns with the v9 upgrade. Confirm that this path is consistent across related modules.
7-7: Verify gogoproto Import Availability
Static analysis again flags the import"gogoproto/gogo.proto". Please verify that this dependency is available in your project or update the import path if necessary.🧰 Tools
🪛 Buf (1.47.2)
7-7: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
12-23: Review New QueryV2 Service Implementation
The new gRPC serviceQueryV2and its RPC methods (DenomsandDenom) have been introduced. The addition of HTTP GET annotations for these methods facilitates RESTful access. Confirm that the endpoints (/ibc/apps/transfer/v2/denomsand/ibc/apps/transfer/v2/denoms/{hash=**}) integrate seamlessly with your API gateway or HTTP server configuration.
25-30: Validate QueryDenomRequest Message
TheQueryDenomRequestmessage is straightforward, containing a singlestring hashfield intended to accept either a hex format or full denom input. Verify that the input validation on this field is performed where necessary.
32-37: Validate QueryDenomResponse Message
TheQueryDenomResponsecorrectly encapsulates theDenomtype. Please ensure that the referencedDenommessage is defined (presumably in"ibc/applications/transfer/v2/token.proto") and that its serialization satisfies client expectations.
39-44: Review QueryDenomsRequest Message
TheQueryDenomsRequestmessage includes an optional pagination field, which is appropriate for handling potentially large responses. Make sure that the pagination behavior is properly implemented in your query resolver.
46-53: Review QueryDenomsResponse Message
The response message returns a repeated field ofDenomobjects along with pagination information. The customgogoprotooptions applied for the repeated fields should be verified to work with your protobuf serialization library.third_party/proto/ibc/applications/transfer/v1/authz.proto (4)
5-5: Updatego_packageto ibc-go v9The
go_packageoption is now updated to use version 9, which is essential for consistency throughout the upgrade process.
7-7: Review Import Path forcosmos_proto/cosmos.protoStatic analysis indicates that the file imported as
"cosmos_proto/cosmos.proto"might not exist. Please verify that the dependency is available or update the import path accordingly.🧰 Tools
🪛 Buf (1.47.2)
7-7: import "cosmos_proto/cosmos.proto": file does not exist
(COMPILE)
10-10: Add Import for Updated Transfer DefinitionsA new import is added for
"ibc/applications/transfer/v1/transfer.proto". Confirm that this file exists at the specified path and that it provides the expected definitions.
26-27: Introduce AllowedForwarding in AllocationThe addition of the
AllowedForwardingfield (and its associated message) in theAllocationdefinition enriches the forwarding capability. Ensure that downstream logic handles this new structure correctly.third_party/proto/ibc/applications/interchain_accounts/host/v1/tx.proto (2)
5-5: Updatego_packagefor Interchain Accounts HostThe
go_packageoption now points to"github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/host/types", which aligns with the ibc-go v9 upgrade. This change should integrate seamlessly with other module updates.
31-31: Enforce Non-nullable Query RequestsMarking the
requestsfield as non-nullable in theMsgModuleQuerySafemessage ensures that all query requests are provided explicitly. Verify that clients and middleware are updated to honor this constraint.third_party/proto/ibc/applications/transfer/v2/packet.proto (4)
5-5: Updatego_packageto ibc-go v9The
go_packageoption has been updated to use the version 9 package path, maintaining consistency across the protocol definitions.
7-7: Check Existence oftoken.protoImportStatic analysis reports a potential issue with the import
"ibc/applications/transfer/v2/token.proto". Please verify that this file exists or adjust the import path if it has been relocated.🧰 Tools
🪛 Buf (1.47.2)
7-7: import "ibc/applications/transfer/v2/token.proto": file does not exist
(COMPILE)
27-41: New Message: FungibleTokenPacketDataV2The introduction of
FungibleTokenPacketDataV2supports multiple tokens along with an optional forwarding field. Ensure that any processing logic or middleware handling token packets is updated to accommodate these new structures.
43-47: New Message: ForwardingPacketDataThe
ForwardingPacketDatamessage introduces the mechanism for token packet forwarding (with an optional destination memo and a repeated list of hops). Review how this integrates with the existing transfer flow to ensure smooth interoperability.third_party/proto/ibc/core/client/v1/client.proto (2)
5-6: Updatego_packagefor IBC Core ClientThe
go_packageoption is now updated to"github.com/cosmos/ibc-go/v9/modules/core/02-client/types", reflecting the migration to ibc-go v9.
55-57: Refine Height Message with JSON TagsIn the
Heightmessage, therevision_numberandrevision_heightfields now include explicit JSON tags. This change guarantees that zero values are always marshaled, which is critical for consistent client behavior.third_party/proto/ibc/applications/transfer/v1/query.proto (2)
10-10: Updatego_packageto Use ibc-go v9The update of the
go_packageoption to"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"aligns with the overall upgrade from v8 to v9. This change is straightforward and necessary.
12-33: Review Removal of DenomTrace-related RPCs and MessagesThe removal of RPC methods and message types related to denomination traces (e.g.,
DenomTracesandDenomTrace) is consistent with the new design in ibc-go v9. Verify that no remaining portions of the codebase still depend on these deprecated endpoints.third_party/proto/ibc/applications/transfer/v1/tx.proto (3)
5-5: Update go_package option for ibc-go v9
Thego_packageoption has been updated to reference the v9 module for the transfer types. Please ensure that all consumers of this proto now reference the new package path.
38-39: Deprecate the legacy token field in MsgTransfer
Thetokenfield is now marked as deprecated with an updated annotation. Verify that clients are updated to use the newtokensfield instead.
52-55: Introduce new tokens and forwarding fields in MsgTransfer
A new repeated fieldtokens(for transferring multiple tokens) and an optionalforwardingfield have been added. Ensure that downstream handlers and documentation are updated to support these new fields.third_party/proto/ibc/core/connection/v1/tx.proto (3)
5-5: Update go_package option for connection types
Thego_packageoption now points to the v9 module for core connection types. Confirm that all related components use the new import path.
60-80: Deprecate legacy fields in MsgConnectionOpenTry
Several fields inMsgConnectionOpenTry(includingprevious_connection_id,client_state,proof_client,proof_consensus,consensus_height, andhost_consensus_state_proof) are now marked as deprecated. Ensure that any legacy logic relying on these fields is updated or removed in accordance with the ibc-go v9 changes.
93-110: Upgrade MsgConnectionOpenAck for ibc-go v9
InMsgConnectionOpenAck, fields such asclient_state,proof_client,proof_consensus,consensus_height, andhost_consensus_state_proofhave been deprecated. Please verify that any logic processing this message is compatible with these deprecations.nix/sources.json (1)
126-132: Update relayer source information
The relayer details have been updated to point to the new repository undermmsqe(with updatedhomepage,owner,rev,sha256, andurl). Verify that these new details are correct and that the build system accepts the new source.third_party/proto/ibc/core/client/v1/tx.proto (2)
5-5: Update go_package option for client types
Thego_packageoption now reflects ibc-go v9 for client types. Confirm consistency with other updated proto files in the client module.
55-59: Add client_id field to MsgCreateClientResponse
A new fieldclient_idhas been introduced in theMsgCreateClientResponsemessage to convey the created client's identifier. This upgrade aligns with the updated protocol specifications for ibc-go v9.third_party/proto/ibc/core/client/v1/query.proto (4)
5-5: Update go_package for client query proto
Thego_packageoption is updated to reference the v9 module. Ensure that query service implementations and client integrations are aligned with this change.
7-7: Verify pagination.proto import path
The file importscosmos/base/query/v1beta1/pagination.proto, but static analysis indicates that this file might not exist in the expected location. Please verify the correct path or update the import accordingly.🧰 Tools
🪛 Buf (1.47.2)
7-7: import "cosmos/base/query/v1beta1/pagination.proto": file does not exist
(COMPILE)
10-10: Update commitment proto import to v2
The import for the commitment definitions has been updated to"ibc/core/commitment/v2/commitment.proto", aligning with the v9 changes. Confirm that related dependency versions are updated accordingly.
221-238: Refactor QueryVerifyMembershipRequest for updated commitment
TheQueryVerifyMembershipRequestmessage now reserves the deprecated field and adds a newmerkle_pathfield of typeibc.core.commitment.v2.MerklePath. Make sure clients adapt to this change when verifying proofs.go.mod (8)
3-3: Updated Go Version: "go 1.23.3"
The go version has been updated to 1.23.3; please verify that the rest of the tooling and any CI scripts are compatible with this version.
25-27: ibc-go Upgrade: Modules and Version Update
The dependencies for bothgithub.com/cosmos/ibc-go/modules/apps/callbacksandgithub.com/cosmos/ibc-go/v9have been updated to reflect the v9 upgrade (v9.0.2). Confirm that all usages of ibc-go APIs in the codebase have been adapted to support the new version.
64-64: Dependency Version Bump for go-winio
The Microsoft go-winio dependency is now at v0.6.2. Ensure that any platform-specific (Windows) logic is tested with the new version.
71-71: Upgrade of bgentry/speakeasy
Thebgentry/speakeasydependency has been updated to v0.2.0. It is advisable to double-check any integration tests involving password prompts or terminal interactions for compatibility with the new version.
77-77: Backoff Package Update
The upgrade ofgithub.com/cenkalti/backoff/v4to v4.3.0 is noted. Please verify if any changes in the backoff API need adjustments in your retry or timeout logic.
82-82: Cockroachdb FIFO Dependency Update
Thegithub.com/cockroachdb/fifodependency now points to commitv0.0.0-20240616162244-4768e80dfb9a. Ensure this commit is stable and that integration tests covering queue or FIFO operations pass reliably.
100-100: Wincred Version Bump
github.com/danieljoos/wincredhas been updated to v1.2.1. It is recommended to run tests on Windows platforms (if applicable) to ensure that the update does not introduce any regressions.
273-274: Replace Block – Ethermint Dependency Revision
Within the replace block, the dependency forgithub.com/evmos/etherminthas changed tov0.6.1-0.20250114082733-11b246d20511(lines 273–274). Make sure that this updated version is compatible with the rest of the dependency tree and passes all relevant integration tests.gomod2nix.toml (12)
89-90: Module Update for go-winio
The entry forgithub.com/Microsoft/go-winiohas been updated to v0.6.2 with a new hash. This change is consistent with the update in go.mod; please verify that the hash is correct.
110-111: Speakeasy Module Update
The dependencygithub.com/bgentry/speakeasyhas been bumped to v0.2.0 alongside a new hash. Confirm that the update is reflected correctly in the build and dependency resolution.
127-129: Backoff Module Upgrade
The update forgithub.com/cenkalti/backoff/v4to version v4.3.0 with the corresponding new hash is applied. Ensure that any API changes are accounted for in your code.
143-145: Cockroachdb FIFO Update
The FIFO dependency now uses commitv0.0.0-20240616162244-4768e80dfb9awith an updated hash. Please ensure compatibility with any features using FIFO queues.
223-225: Wincred Update Verification
Thegithub.com/danieljoos/wincredmodule has been updated to v1.2.1, and the new hash is provided. Validate this update during your dependency audit.
258-260: Jose2go Dependency Bump
github.com/dvsekhvalnov/jose2gois now updated to v1.7.0 with a new hash. Please review any impacted cryptographic operations, noting potential API changes.
259-261: Emicklei/dot Update
The dependency forgithub.com/emicklei/dothas been updated to v1.6.2 with its corresponding hash. Confirm that any DOT graph generation functionalities are unimpacted.
269-271: Fatih/color Module Upgrade
The version ofgithub.com/fatih/colorhas been bumped to v1.17.0 with an updated hash. This should improve or maintain colorized output consistency; please verify on different terminal setups if required.
474-476: Go-Runewidth Update
The dependencygithub.com/mattn/go-runewidthis now set to v0.0.15 with a new hash, ensuring better support for character width calculations.
531-533: Rivo/uniseg Version Upgrade
Thegithub.com/rivo/unisegmodule is updated to v0.2.0 with a new hash. Confirm that any string segmentation or Unicode handling in the project remains correct.
716-717: nhooyr/websocket Upgrade
The update fornhooyr.io/websocketto v1.8.11 (with updated hash) is consistent with go.mod. Ensure websocket functionalities are tested across supported platforms.
543-544: Sagikazarmark/locafero Update
The modulegithub.com/sagikazarmark/locaferois now updated to v0.6.0 with a new hash. Validate that filesystem abstractions depending on this package continue to work as expected.
| denomTrace, exists := k.transferKeeper.GetDenom(ctx, hexDenomBytes) | ||
| if !exists { | ||
| return "", errors.Wrapf(types.ErrIbcCroDenomInvalid, "%s is invalid", ibcVoucherDenom) | ||
| } | ||
|
|
||
| // the path has for format port/channelId | ||
| return strings.Split(denomTrace.Path, "/")[1], nil | ||
| return denomTrace.Trace[0].ChannelId, nil |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify array bounds for Trace access.
The direct access to Trace[0] assumes the array is non-empty. Consider adding a bounds check.
- return denomTrace.Trace[0].ChannelId, nil
+ if len(denomTrace.Trace) == 0 {
+ return "", errors.Wrapf(types.ErrIbcCroDenomInvalid, "empty trace for %s", ibcVoucherDenom)
+ }
+ return denomTrace.Trace[0].ChannelId, nil📝 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.
| denomTrace, exists := k.transferKeeper.GetDenom(ctx, hexDenomBytes) | |
| if !exists { | |
| return "", errors.Wrapf(types.ErrIbcCroDenomInvalid, "%s is invalid", ibcVoucherDenom) | |
| } | |
| // the path has for format port/channelId | |
| return strings.Split(denomTrace.Path, "/")[1], nil | |
| return denomTrace.Trace[0].ChannelId, nil | |
| denomTrace, exists := k.transferKeeper.GetDenom(ctx, hexDenomBytes) | |
| if !exists { | |
| return "", errors.Wrapf(types.ErrIbcCroDenomInvalid, "%s is invalid", ibcVoucherDenom) | |
| } | |
| // the path has for format port/channelId | |
| if len(denomTrace.Trace) == 0 { | |
| return "", errors.Wrapf(types.ErrIbcCroDenomInvalid, "empty trace for %s", ibcVoucherDenom) | |
| } | |
| return denomTrace.Trace[0].ChannelId, nil |
| "denom": denom, | ||
| "amount": amt, | ||
| } | ||
| def fungible(dst, src, amt, denom, trace=[]): |
There was a problem hiding this comment.
Fix mutable default argument.
Using mutable data structures (empty list []) as default arguments in Python can lead to unexpected behavior because the same list object is shared across all function calls.
Apply this diff to fix the issue:
-def fungible(dst, src, amt, denom, trace=[]):
+def fungible(dst, src, amt, denom, trace=None):
+ if trace is None:
+ trace = []📝 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.
| def fungible(dst, src, amt, denom, trace=[]): | |
| def fungible(dst, src, amt, denom, trace=None): | |
| if trace is None: | |
| trace = [] | |
| # ... (rest of the function implementation) ... |
🧰 Tools
🪛 Ruff (0.8.2)
120-120: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
| @@ -148,7 +149,7 @@ def prepare_network( | |||
| connection_only=False, | |||
| grantee=None, | |||
| need_relayer_caller=False, | |||
| relayer=cluster.Relayer.HERMES.value, | |||
| relayer=cluster.Relayer.RLY.value, | |||
There was a problem hiding this comment.
💡 Codebase verification
Impact of Changing Default Relayer: Inconsistent HERMES References Detected
The default relayer in integration_tests/ibc_utils.py has been updated to use RLY, but we still found several references to HERMES in the codebase. Specifically:
- In integration_tests/network.py, the relayer is still explicitly set to
cluster.Relayer.HERMES.value. - In integration_tests/ibc_utils.py, there’s a conditional check (
is_hermes = relayer == cluster.Relayer.HERMES.value) that continues to reference HERMES.
This inconsistency could lead to unintended behavior:
- Tests or code paths relying on the HERMES logic might not be executed when using the new default, resulting in untested or orphaned logic.
- The dual existence of HERMES- and RLY-specific code may confuse test outcomes or impact IBC functionality if not harmonized.
It’s important to review whether these legacy references are still required (perhaps for backward compatibility or explicit test scenarios) or if they should be refactored to consistently use RLY as the default.
🔗 Analysis chain
Verify the impact of changing the default relayer.
The default relayer has been changed from HERMES to RLY. This is a significant change that could affect IBC functionality.
Run the following script to check for any remaining HERMES relayer references:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining HERMES relayer references that might need updating
# Search for HERMES references in Python files
echo "Searching for HERMES references in Python files..."
rg -A 5 "HERMES" --type py
# Search for hermes configuration or setup
echo "Searching for hermes configuration files..."
fd -e toml "hermes"Length of output: 922
| syntax = "proto3"; | ||
|
|
||
| package ibc.core.commitment.v2; | ||
|
|
||
| option go_package = "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types/v2"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix package directory structure.
The package directory structure doesn't match the proto package name. According to the Buf linter:
- Current:
third_party/proto/ibc/core/commitment/v2 - Expected:
ibc/core/commitment/v2
🧰 Tools
🪛 Buf (1.47.2)
3-3: Files with package "ibc.core.commitment.v2" must be within a directory "ibc/core/commitment/v2" relative to root but were in directory "third_party/proto/ibc/core/commitment/v2".
(PACKAGE_DIRECTORY_MATCH)
| syntax = "proto3"; | ||
|
|
||
| package ibc.applications.transfer.v1; | ||
|
|
||
| option go_package = "github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix package directory structure.
The package directory structure doesn't match the proto package name. According to the Buf linter:
- Current:
third_party/proto/ibc/applications/transfer/v1 - Expected:
ibc/applications/transfer/v1
🧰 Tools
🪛 Buf (1.47.2)
3-3: Files with package "ibc.applications.transfer.v1" must be within a directory "ibc/applications/transfer/v1" relative to root but were in directory "third_party/proto/ibc/applications/transfer/v1".
(PACKAGE_DIRECTORY_MATCH)
|
|
||
| option go_package = "github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"; | ||
|
|
||
| import "ibc/applications/transfer/v1/transfer.proto"; |
There was a problem hiding this comment.
Fix missing import file.
The import "ibc/applications/transfer/v1/transfer.proto" is missing. This needs to be resolved to ensure proper compilation.
🧰 Tools
🪛 Buf (1.47.2)
7-7: import "ibc/applications/transfer/v1/transfer.proto": file does not exist
(COMPILE)
Solution:
TODO: fix ibc conversion middleware
👮🏻👮🏻👮🏻 !!!! 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
New Features
Enhancements
Refactor