refactor: ibc port router for app callbacks #6314
Conversation
|
Important Review SkippedDraft detected. 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 (
|
| timeoutHeight clienttypes.Height, | ||
| timeoutTimestamp uint64, |
There was a problem hiding this comment.
we should remove timeout info from the interface. Left for now since it breaks callbacks interface, but I don't see a reason to leave it for callbacks
| } | ||
|
|
||
| // TODO: the packet sender is now passed in, it is possible to remove `GetPacketSender` in favour of this arg |
There was a problem hiding this comment.
we can remove the GetPacketSender interface now! Wahoo!
|
The ics20 code needs to be taken care of and some tests refactored. Just did what was needed to resolve the merge conflicts and get most tests passing. |
| for _, prefix := range k.Keys() { | ||
| if strings.Contains(module, prefix) { | ||
| return k.AppRouter.GetRoute(prefix) | ||
| } | ||
| } |
There was a problem hiding this comment.
lets make this well documented since the implications are subtle cc @damiannolan
* not working yet * More tests fixed. * added message error * commented out tests * Wire approuter to e2e testing simapp * Delete obsolete tests * remove possibly unneeded call * Removed tests no longer needed * test: fix commented/removed tests --------- Co-authored-by: Colin Axnér <[email protected]>
#7010) * chore: adding updates to composite router and scaffolding for IBCLegacyModule * chore: support construction of LegacyIBCModules over consecutive calls to AddRoute * chore: removed ack handhsake impl * chore: fix linter * chore: lint * Linter * chore: addressing PR feedback * chore: routeToLegacyModule returns a single item * Update modules/core/05-port/types/router_v2.go * Update modules/core/05-port/types/router_v2.go * Update modules/core/05-port/types/router_v2.go * chore: fix errors from merge conflict --------- Co-authored-by: bznein <[email protected]> Co-authored-by: Colin Axnér <[email protected]>
* chore: adding updates to composite router and scaffolding for IBCLegacyModule * chore: support construction of LegacyIBCModules over consecutive calls to AddRoute * chore: removed ack handhsake impl * chore: fix linter * chore: lint * Linter * chore: addressing PR feedback * chore: rename IBCModule to ClassicIBCModule and add new IBCModule interface * fix: e2e for fee wrapped channels --------- Co-authored-by: bznein <[email protected]> Co-authored-by: Colin Axnér <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for prefix := range rtr.routes { | ||
| if strings.Contains(module, prefix) { | ||
| return rtr.routes[prefix], true | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for prefix := range rtr.legacyRoutes { | ||
| if strings.Contains(module, prefix) { | ||
| return rtr.legacyRoutes[prefix], true | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
* chore: adding wrapping functions to ics29 * chore: added docstring to VersionWrapper interface * chore: fix linter * refactor: use unsafe unwrap func in OnChanOpenInit * chore: fixing wiring in app.go and fee tests * fix: fee middleware tests * fix: ics27 controller tests * wip: debugging callbacks * chore: fix linter * chore: fix miswiring in callbacks module * chore: modifications to binary simapp * chore: do not unwrap a version if it is an empty string * Update modules/apps/27-interchain-accounts/controller/ibc_middleware.go * Apply suggestions from code review * chore: add more code docs --------- Co-authored-by: Colin Axnér <[email protected]>
* chore: add fee implementation for OnChanOpenTry * chore: fix wiring in app.gos * chore: updated callbacks tests to handle new OnChanOpenTry * chore: lint, correct error string * chore: remove commented code * chore: addressing PR feedback
* refactor: use new port router for OnChanOpenAck * fix: transfer test * fix: controller tests * fix: legacy ibc module handler on disabling
* chore: add fee implementation for OnChanOpenTry * chore: fix wiring in app.gos * chore: updated callbacks tests to handle new OnChanOpenTry * chore: lint, correct error string * chore: remove commented code * chore: implementing OnChanUpgradeInit * chore: linter * chore: removed redundant test * chore: addressing PR feedback
* deps: bump cosmos-sdk to v0.50.9 (#6828) * deps: bump cosmos-sdk to v0.50.8 * chore: update changelog * deps: bump cosmossdk.io/client to v2.0.0-beta.3. bump x/upgrade to v0.1.4 * chore: make tidy-all * test: bump to 3f6796fba413cca for testing purposes. * deps: bump cosmos sdk to 0.50.9 * Update CHANGELOG.md * chore: update CHANGELOG for submodules. --------- Co-authored-by: DimitrisJim <[email protected]> * chore(api!): move checks from Transfer to OnSendPacket * Fix merge error * fix to test * Add Require() back * Update modules/apps/transfer/ibc_module.go Co-authored-by: colin axnér <[email protected]> * Linter * Removed wrong merge results * reintroduce test * Move check for isSendEnabledCoins to keeper * new tests * fixed test * linter * Removed checks and added comment * removed extra parentheses * Revert "Merge remote-tracking branch 'origin' into bznein/6949/msgTransferWrapperToSendPacket" This reverts commit be9f558, reversing changes made to 36e4b05. * Delete commented out code --------- Co-authored-by: Damian Nolan <[email protected]> Co-authored-by: DimitrisJim <[email protected]> Co-authored-by: colin axnér <[email protected]>
* chore: implement OnChanUpgradeTry * wip: partial implementation for OnChanUpgradeAck * chore: updated tests to work with new OnChanUpgradeAck implementation
* chore: implement OnChanUpgradeTry * wip: partial implementation for OnChanUpgradeAck * chore: updated tests to work with new OnChanUpgradeAck implementation * chore: implemented OnChanUpgradeOpen * chore: addressing PR feedback
) * refactor: update OnRecvPacket to use result type in favour of ack interface * refactor: update recv packet handling in interchain accounts, update event func arg to use concrete type * api! remove Acknowledgement interface from exported pkg * refactor: modifications to test code for api removal * lint: fix duplicate imports * refactor: use concrete acknowledgement in callbacks test * doc: update inline godocs for OnRecvPacket in ibccallbacks * fix: remove wrapping of result ack in callbacks recv packet * test: refactor writeAck tests to use expected error format * chore: add godocs and stringer impl for new recv result types * chore: add issue link for async acknowledgements in 29-fee * doc: update godocs for RecvPacketResult and status enums * chore: address pr comments, test updates and inline docs
* test not working * refactored test and removed unneeded caese
* refactor: use new port router for OnAcknowledgementPacket * fix: msg server update * fix: ics27 host tests * fix: ics27 controller tests * lint * chore: docs * add type assertions * add 29 type assertion
* initial commit * revert change * loop callbacks * fix loop * wip * chore: fixing remainder of timeout test failures * chore: check type assertion * PR feedbacK --------- Co-authored-by: chatton <[email protected]>
…7142) * chore: updated handlers to use reverseCallbacks function * chore: add reconstructVersion to im * chore: remove duplicate import * chore: pr feedback
|
Closing this pr. It was used to do investigative work on redesigning the port router such that we could support ibc applications to be written with a cleaner interface and without the use of the confusing middleware design and ICS4Wrapper. It was determined making these changes in a backwards compatible way was not reasonable and thus these changes are being actively investigated in the ibc eureka / v2 work. Capabilities have been removed! And any other work in this pr will be upstreamed as necessary. v1 ibc applications will remain as is and a new design approach will be enabled for v2 great work by all involved in diving into these investigations! |
Description
closes: #6941
closes: #3371
closes: #4427
closes: #4444
closes: #5774
closes: #5814
closes: #71
closes: #3312
closes: #3817
closes: #3727
closes: #41
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.