Conversation
masihyeganeh
left a comment
There was a problem hiding this comment.
@masihyeganeh reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @akhlopiachyi, @metalarm10, @miladz68, @TxCorpi0x, and @ysv).
x/deterministicgas/config.go line 357 at r1 (raw file):
// ibc/core/channel/v2 // TODO: MsgSendPacket is the user-facing message that initiates the transfer, so it should be deterministic.
Should we calculate this one?
TxCorpi0x
left a comment
There was a problem hiding this comment.
@TxCorpi0x made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @akhlopiachyi, @metalarm10, @miladz68, and @ysv).
x/deterministicgas/config.go line 357 at r1 (raw file):
Previously, masihyeganeh (Masih Yeganeh) wrote…
Should we calculate this one?
I am not sure, May be @ysv or @miladz68 are the best person to ask.
IMO this would be hard to estimate at the moment, since the current Hermes version does not support v2 transfers, i am not sure how we are going to estimate it in a working blockchain. We can do this by just running the send packet and measure it. but i think it is better to define a task for the future so we can rethink this
miladz68
left a comment
There was a problem hiding this comment.
@miladz68 partially reviewed 2 files and made 3 comments.
Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on akhlopiachyi, masihyeganeh, metalarm10, TxCorpi0x, and ysv).
x/deterministicgas/config.go line 357 at r1 (raw file):
Previously, TxCorpi0x wrote…
I am not sure, May be @ysv or @miladz68 are the best person to ask.
IMO this would be hard to estimate at the moment, since the current Hermes version does not support v2 transfers, i am not sure how we are going to estimate it in a working blockchain. We can do this by just running the send packet and measure it. but i think it is better to define a task for the future so we can rethink this
I think they should be un-deterministic.
a discussion (no related file):
let's merge this PR after v6 release.
integration-tests/ibc/ibc_v2_test.go line 70 at r1 (raw file):
gaiaChain := chains.Gaia // --- Step 1: Relayer accounts ---
please remove numbered steps, it makes future maintenance harder.
miladz68
left a comment
There was a problem hiding this comment.
@miladz68 partially reviewed 2 files.
Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on akhlopiachyi, masihyeganeh, metalarm10, TxCorpi0x, and ysv).
TxCorpi0x
left a comment
There was a problem hiding this comment.
@TxCorpi0x made 3 comments and resolved 1 discussion.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on akhlopiachyi, masihyeganeh, metalarm10, miladz68, and ysv).
a discussion (no related file):
Previously, miladz68 (milad) wrote…
let's merge this PR after v6 release.
Sure
integration-tests/ibc/ibc_v2_test.go line 70 at r1 (raw file):
Previously, miladz68 (milad) wrote…
please remove numbered steps, it makes future maintenance harder.
Done
x/deterministicgas/config.go line 357 at r1 (raw file):
Previously, miladz68 (milad) wrote…
I think they should be un-deterministic.
Done, Todo removed
miladz68
left a comment
There was a problem hiding this comment.
@miladz68 reviewed 4 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on akhlopiachyi, metalarm10, and ysv).
x/deterministicgas/config.go line 357 at r1 (raw file):
Previously, TxCorpi0x wrote…
Done, Todo removed
Maybe I have misunderstood, MsgSendPacket replaces MsgTransfer ? I thought MsgSendPacket is handled by relayers.
Description
This pull request introduces support for IBC v2 (Inter-Blockchain Communication version 2) in the application, particularly focusing on the transfer module and related middleware. It updates imports, configures new IBC v2 stacks and routers, and ensures deterministic gas configuration for new IBC v2 channel messages.
IBC v2 Integration and Configuration:
Added imports for IBC v2 modules, including
ibccallbacksv2,transferv2, and the new IBC API, to support v2 functionality inapp/app.go.Created and configured the IBC v2 transfer stack and middleware, and set up a dedicated IBC v2 router using
SetRouterV2in the application initialization logic inapp/app.go.Created Integration Test for the IBC v2 complete flow.
Reviewers checklist:
Authors checklist
This change is