-
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
feat: add packet forward middleware #2381
Conversation
WalkthroughThe codebase has been updated to include a new middleware for packet forwarding in the Changes
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (1)
- app/app.go (11 hunks)
Additional comments: 11
app/app.go (11)
104-106: The import paths for the packet forwarding middleware have been added correctly and there are no conflicts with existing imports.
209-209: The
packetforward
module has been correctly added to themoduleBasics
slice.276-285: The keepers for the packet forwarding middleware are declared correctly, following the established naming conventions and pointer usage.
346-346: The store key for the packet forwarding middleware has been added correctly and follows the naming conventions.
567-574: The comments added provide a clear description of the middleware stack for ICS20, including the packet forwarding middleware's position in the stack.
588-611: The packet forwarding middleware is correctly integrated into the IBC middleware stack with appropriate parameters for retries and timeouts.
743-743: The packet forwarding middleware's AppModule is registered correctly within the module manager.
784-784: The packet forwarding middleware is added to the BeginBlockers in the correct sequence.
803-803: The packet forwarding middleware is added to the EndBlockers in the correct sequence.
827-827: The packet forwarding middleware is added to the InitGenesis in the correct sequence.
1173-1173: The subspace for the packet forwarding middleware is correctly initialized in the ParamsKeeper with the appropriate key table.
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.
Thanks for the contribution!
@@ -1138,6 +1170,7 @@ func initParamsKeeper( | |||
paramsKeeper.Subspace(ibctransfertypes.ModuleName) | |||
paramsKeeper.Subspace(ibcexported.ModuleName) | |||
paramsKeeper.Subspace(icahosttypes.SubModuleName) | |||
paramsKeeper.Subspace(packetforwardtypes.ModuleName).WithKeyTable(packetforwardtypes.ParamKeyTable()) |
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.
This is not required in v7.1.1 (x/params is migrated out due to SDK v0.51 deprecation).
It is fine being here, just not used
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.
Thanks for the comment @Reecepbcups .
Paramspace is still working in v0.47 and v0.50. I'm not sure if the IBC requires it or not.
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.
I've checked Osmosis, which migrated to v0.47 recently, and they also setup PFM
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.
I mean, their Osmosis PFM integration also uses params keeper.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.mod
is excluded by:!**/*.mod
go.sum
is excluded by:!**/*.sum
Files selected for processing (1)
- app/app.go (12 hunks)
Additional comments: 12
app/app.go (12)
- 104-106: The import statements for the packet forward middleware and related packages have been added correctly.
- 209-209: The
packetforward.AppModuleBasic{}
has been correctly added to themoduleBasics
slice.- 276-285: The
PacketForwardKeeper
has been correctly added to theUmeeApp
struct as a pointer, which is consistent with the other keepers.- 346-346: The
packetforwardtypes.StoreKey
has been correctly added to thestoreKeys
slice.- 586-615: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [567-612]
The integration of the packet forward middleware into the IBC module stack is done correctly, with attention to the middleware order in the stack.
- 589-600: The
PacketForwardKeeper
is initialized properly with the necessary dependencies and configuration.- 744-744: The
packetforward.NewAppModule
is correctly added to the module manager.- 785-785: The
packetforwardtypes.ModuleName
is correctly added to thebeginBlockers
slice.- 804-804: The
packetforwardtypes.ModuleName
is correctly added to theendBlockers
slice.- 828-828: The
packetforwardtypes.ModuleName
is correctly added to theinitGenesis
slice.- 845-845: The
packetforwardtypes.ModuleName
is correctly added to theorderMigrations
slice.- 1175-1175: The previous comments regarding the
packetforwardtypes.ModuleName
subspace are still valid and no further action is required.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2381 +/- ##
==========================================
- Coverage 75.38% 68.50% -6.89%
==========================================
Files 100 180 +80
Lines 8025 13321 +5296
==========================================
+ Hits 6050 9125 +3075
- Misses 1589 3591 +2002
- Partials 386 605 +219
|
Description
Adds the packet forward middleware. Useful for applications like https://ibc.fun/.
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
New Features
Enhancements