-
Notifications
You must be signed in to change notification settings - Fork 170
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
fix: fix ibc transfer msg recv length check #2550
Conversation
WalkthroughThis update primarily focuses on improving error handling and validation within the IBC (Inter-Blockchain Communication) module through additional functions and dependencies. It also enhances documentation formatting and introduces tests for newly added functionality. No exported or public entity declarations were altered. Changes
Sequence Diagram(s)Diagrams are not applicable for this update since the changes primarily involve validation and dependency updates. 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 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.
Pre approving.
- update Release Notes
- Update Changelog
- merge into
main
and then cherry pick into new branch:release/v6.5
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2550 +/- ##
===========================================
- Coverage 75.38% 62.76% -12.63%
===========================================
Files 100 275 +175
Lines 8025 15968 +7943
===========================================
+ Hits 6050 10023 +3973
- Misses 1589 5171 +3582
- Partials 386 774 +388
|
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: 3
Outside diff range and nitpick comments (1)
docs/VALIDATOR.md (1)
Line range hint
46-46
: Consider adding the preposition "on" after "Depending" for grammatical correctness.- Depending where you run your validator node, certain locations may block some endpoints. + Depending on where you run your validator node, certain locations may block some endpoints.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- CHANGELOG.md (1 hunks)
- docs/VALIDATOR.md (1 hunks)
- go.mod (1 hunks)
- tests/e2e/e2e_ibc_test.go (2 hunks)
- tests/e2e/setup/utils.go (1 hunks)
- util/ibc/ibc.go (2 hunks)
- util/ibc/ibc_test.go (2 hunks)
- util/sdkutil/string.go (2 hunks)
Files skipped from review due to trivial changes (2)
- go.mod
- tests/e2e/setup/utils.go
Additional context used
LanguageTool
docs/VALIDATOR.md
[uncategorized] ~45-~45: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...eeder.example.toml). - For the provider config you can use our latest [umee-provider-c...
[grammar] ~46-~46: The verb ‘depend’ requires the preposition ‘on’ (or ‘upon’). (DEPEND_ON)
Context: ...e/umee/umee-provider-config) as is. - Depending where you run your validator node, cert...
[uncategorized] ~60-~60: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...Joining the network Before joining the mainnet you should join a testnet! ### Testnet...CHANGELOG.md
[grammar] ~123-~123: Using ‘plenty’ without ‘of’ is considered to be informal. (PLENTY_OF_NOUNS)
Context: .../pull/2368) Fix inflow amount calculation. Previously, the inflow amount of the t...
[uncategorized] ~124-~124: Possible missing preposition found. (AI_HYDRA_LEO_MISSING_TO)
Context: ...nd SDK account sequence setting changes the calling client. ### API Breaking - [2...
[grammar] ~225-~225: Did you mean “limiting”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun. (ALLOW_TO)
Context: ...veragedLiquidate.MaxRepay` which allows to limit the liquidation size using the leverage...
[uncategorized] ~268-~268: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ub.com//pull/2148) Fix MsgBeginUnbonding counting existing unbondings against ma...
[grammar] ~358-~358: The singular proper name ‘Bridge’ must be used with a third-person or a past tense verb. (HE_VERB_AGR)
Context: ...-network/umee/pull/1967) Gravity Bridge phase out phase-2: disable Umee -> Ethereum t...
[grammar] ~359-~359: The singular proper name ‘Bridge’ must be used with a third-person or a past tense verb. (HE_VERB_AGR)
Context: ...-network/umee/pull/1967) Gravity Bridge phase out phase-2: disable Umee -> Ethereum t...
[grammar] ~409-~409: Make sure that the singular noun after the number ‘4.1’ is correct. (CD_POINT_CD_NN)
Context: ...e/pull/1807) Fixes BNB ibc denom in 4.1 migration - [1812](https://github.com/umee-networ...
[style] ~427-~427: In contexts where ‘if’ is followed by ‘or’, using ‘whether’ may be more appropriate (and formal). (IF_WHETHER)
Context: ...rowand
MsgRepay` won't return errors if there is nothing to withdraw, borrow or...
[uncategorized] ~474-~474: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...of the build process (you must build on same host as you run the binary, or copy the...
[duplication] ~478-~478: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...e/pull/1555) Updates IBC to v5.1.0 that adds adds optional memo field to `FungibleTokenPa...
[uncategorized] ~535-~535: A comma may be missing after the conjunctive/linking adverb ‘Also’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...ableLend, docs, and internal functions. Also QueryLoaned similar queries to QuerySup...
[misspelling] ~539-~539: This word is normally spelled as one. (EN_COMPOUNDS_PROTO_TYPES)
Context: ...roto getters in x/leverage and x/oracle proto types. - [1126](https://github.com/umee-netwo...
[uncategorized] ~560-~560: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...umee/pull/1157) AddedPrintOrErr
util function optimizing the CLI code flow. - [1118](...
[duplication] ~658-~658: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...k/umee/pull/1358/files) Disable Gravity Bridge bridge messages. ### Improvements - [#1355](...
Additional comments not posted (4)
util/ibc/ibc_test.go (1)
36-46
: The new test cases for validatingreceiver
andmemo
fields are well-implemented and cover the expected error scenarios.util/ibc/ibc.go (2)
13-30
: The constants and validation functions forreceiver
andmemo
fields are well-defined. Ensure the arbitrary length limits are justified with domain-specific requirements.
40-50
: The integration of the new validation logic inGetFundsFromPacket
is correctly implemented. This enhances the robustness of packet processing.tests/e2e/e2e_ibc_test.go (1)
15-16
: Imports foribcutil
andsdkutil
have been added to support new test scenarios.
Description
closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
Documentation
Tests
Chores
github.com/onsi/gomega
versionv1.31.1
.