-
Notifications
You must be signed in to change notification settings - Fork 668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: migration docs for transfer multidenom #6393
docs: migration docs for transfer multidenom #6393
Conversation
WalkthroughThe recent updates focus on enhancing the ICS20 - Transfer module to support multi-token transfers. This involves modifying the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant ICS20_Module as ICS20 - Transfer Module
participant EventSystem as Event System
User->>ICS20_Module: Call NewMsgTransfer with multiple tokens
ICS20_Module-->>User: Return MsgTransfer object
ICS20_Module->>EventSystem: Emit ibc_transfer event with tokens attribute
EventSystem-->>User: Notify about ibc_transfer event
Possibly related issues
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
docs/docs/05-migrations/13-v8-to-v9.md (1)
Line range hint
136-136
: Add an article for clarity.- as reference + as a reference
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/docs/05-migrations/13-v8-to-v9.md (1 hunks)
Additional Context Used
LanguageTool (1)
docs/docs/05-migrations/13-v8-to-v9.md (1)
Near line 136: You might be missing the article “a” here.
Context: ...r07-tendermint
may also be useful as reference. ### 06-solomachine TheInitialize
,...
Rule ID: AI_EN_LECTOR_MISSING_DETERMINER_A
Markdownlint (4)
docs/docs/05-migrations/13-v8-to-v9.md (4)
35: Expected: 0 or 2; Actual: 1
Trailing spaces
36: Expected: 0 or 2; Actual: 1
Trailing spaces
97: Column: 34
Hard tabs
8: null
Multiple top-level headings in the same document
Path-based Instructions (1)
docs/docs/05-migrations/13-v8-to-v9.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (1)
docs/docs/05-migrations/13-v8-to-v9.md (1)
76-85
: Ensure the updated function signature is correctly reflected across all relevant documentation and code.Verification successful
The updated function signature for
NewMsgTransfer
is correctly reflected across all relevant documentation and code.
- All instances of
NewMsgTransfer
in the codebase usesdk.NewCoins
, which matches the new function signature.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `NewMsgTransfer` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'NewMsgTransfer'Length of output: 10110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
docs/docs/05-migrations/13-v8-to-v9.md (3)
Line range hint
8-8
: Multiple top-level headings detected.Consider using only one top-level heading to improve document structure and readability.
Line range hint
35-36
: Remove trailing spaces.Trailing spaces on lines 35 and 36 should be removed to adhere to markdown best practices.
Line range hint
97-97
: Replace hard tabs with spaces.Hard tabs detected on line 97. Replace with spaces for consistency in indentation.
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/docs/05-migrations/13-v8-to-v9.md (1 hunks)
Additional Context Used
Markdownlint (4)
docs/docs/05-migrations/13-v8-to-v9.md (4)
35: Expected: 0 or 2; Actual: 1
Trailing spaces
36: Expected: 0 or 2; Actual: 1
Trailing spaces
97: Column: 34
Hard tabs
8: null
Multiple top-level headings in the same document
Path-based Instructions (1)
docs/docs/05-migrations/13-v8-to-v9.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (1)
docs/docs/05-migrations/13-v8-to-v9.md (1)
76-85
: Ensure the updated function signature is reflected across all codebases.Verification successful
All instances of
NewMsgTransfer
in the codebase match the new function signature, usingsdk.Coins
instead ofsdk.Coin
for thetokens
parameter.
modules/apps/transfer/transfer_test.go
modules/apps/transfer/types/transfer_authorization_test.go
modules/apps/transfer/types/msgs_test.go
modules/apps/transfer/keeper/relay_test.go
modules/apps/transfer/keeper/msg_server_test.go
modules/apps/transfer/keeper/mbt_relay_test.go
modules/apps/transfer/keeper/invariants_test.go
modules/apps/transfer/client/cli/tx.go
modules/apps/transfer/types/msgs.go
modules/apps/callbacks/transfer_test.go
modules/apps/callbacks/replay_test.go
modules/apps/callbacks/ibc_middleware_test.go
modules/apps/29-fee/transfer_test.go
modules/apps/29-fee/keeper/events_test.go
modules/apps/27-interchain-accounts/host/keeper/relay_test.go
e2e/tests/transfer/incentivized_test.go
e2e/tests/transfer/upgrades_test.go
e2e/tests/upgrades/upgrade_test.go
e2e/tests/transfer/authz_test.go
e2e/testsuite/tx.go
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `NewMsgTransfer` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'NewMsgTransfer'Length of output: 10110
) | ||
``` | ||
|
||
- The `ibc_transfer` and `fungible_token_packet` events do not include the attributes `denom` and `amount` anymore; instead they include the attribute `tokens` with the list of coins transferred in the packet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update event documentation to reflect attribute changes.
The documentation mentions that ibc_transfer
and fungible_token_packet
events now include the tokens
attribute instead of denom
and amount
. However, both attributes are still present in the codebase. Update the documentation or the code to ensure consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional Context Used
Path-based Instructions (1)
CHANGELOG.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thank you @crodriguezvega! Appreciate the attention to detail!
Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
Description
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.Summary by CodeRabbit
New Features
Documentation