Sync Feature Branch With Main #6359
Conversation
* test: added packet data backwards compatibility test * imp: fixed test case * imp: use expError --------- Co-authored-by: Carlos Rodriguez <[email protected]> Co-authored-by: Colin Axnér <[email protected]>
updated-dependencies: - dependency-name: bufbuild/buf-setup-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* docs: updates due to 02-client routing refactoring * lint * fix up grammar suggestions * Update 02-light-client-module.md * remove empty space --------- Co-authored-by: Charly <[email protected]> Co-authored-by: Charly <[email protected]>
chatton
requested review from
colin-axner,
AdityaSripal,
crodriguezvega,
charleenfei,
damiannolan,
DimitrisJim and
srdtrk
as code owners
| path.EndpointA.ChannelConfig.Version = types.V1 | ||
| path.EndpointB.ChannelConfig.Version = types.V1 |
There was a problem hiding this comment.
the packet data in this test is V1, since we default to V2 now, we need to explicitly update to V1 for this packet data to be treated correctly.
There was a problem hiding this comment.
should update this to also check for v2 (this can be done as part of #6215 possibly.
There was a problem hiding this comment.
I think we can rework the test to be a bit more specific as well. I'd primarily be interested in a test which ensures new fields or improperly formatted packet datas return error acks and not a panic (so if a new field is added in a future version, if it is accidentally sent to a chain that doesn't support it, a refund occurs)
There was a problem hiding this comment.
i'll open issue to keep track.
|
Important Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 (
|
| func() { | ||
| packetData = []byte("invalid packet data") | ||
| }, | ||
| ibcerrors.ErrInvalidType, |
There was a problem hiding this comment.
this error was updated based on the return value of the new unmarshal fn
| jsonString := fmt.Sprintf(`{"amount":"100","sender":%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) | ||
| packetData = []byte(jsonString) | ||
| }, | ||
| ibcerrors.ErrInvalidType, |
|
Description
One of the new tests that made its way into main needed to be updated to work with ics20-v2.
Left comments on the fixes
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/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.