Use ICS4Wrapper to send raw IBC packets & fix Fee middleware in wasm stack #1375
Conversation
| @@ -9,6 +9,7 @@ import ( | |||
| codectypes "github.com/cosmos/cosmos-sdk/codec/types" | |||
| sdk "github.com/cosmos/cosmos-sdk/types" | |||
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | |||
| ibctransfertypes "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types" | |||
There was a problem hiding this comment.
The interface was extended in v7 and moved to port/types . Why not cut this dependency and define the interface with the send method only in wasmd expected_keepers.go?
There was a problem hiding this comment.
The reason I used ibctransfertypes.ICS4Wrapper is that it only has the SendPacket function, which is all that we need. I agree that we can inline this interface into wasmd.
| @@ -522,6 +522,7 @@ func NewWasmApp( | |||
| app.BankKeeper, | |||
| app.StakingKeeper, | |||
| app.DistrKeeper, | |||
| app.IBCFeeKeeper, // ISC4 Wrapper: fee IBC middleware | |||
There was a problem hiding this comment.
Good eye and the change towards the interface makes sense!
Looking deeper into the impl, the ibc fee keeper just delegates to the channel keeper that we setup with the fee keeper.
The separation is much better and packet sending can go through a wrapper.
Personally, I find it very hard to configure and chain the keepers and middlewares proper. There are 2 places that need to be configured. IMHO it would be best, if we could just pass the middleware here and let it handle the internals. Unfortunately there are some circular dependencies.
There was a problem hiding this comment.
Yeah, it's an SDK/IBC thing. I'm sure it'll get better.
| @@ -11,6 +11,8 @@ import ( | |||
| paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" | |||
|
|
|||
| "github.com/CosmWasm/wasmd/x/wasm/types" | |||
|
|
|||
| ibctransfertypes "github.com/cosmos/ibc-go/v4/modules/core/05-port/types" | |||
There was a problem hiding this comment.
Same as above: let's move the interface into the project
|
Thanks for the CR. I'll fix everything on Sunday. |
Oh cool, thanks! |
…stack (backport #1375) (#1379) * Use ICS4Wrapper to send raw IBC packets & fix Fee in wasm stack (cherry picked from commit 6dfa5cb) # Conflicts: # app/app.go # x/wasm/keeper/handler_plugin.go # x/wasm/keeper/keeper_cgo.go # x/wasm/keeper/keeper_test.go # x/wasm/keeper/options_test.go # x/wasm/keeper/test_common.go * Fix merge conflicts * Inline ibc packet sender interface and minor chore * Rename IBCPacketSender --------- Co-authored-by: Assaf Morami <assaf.morami@gmail.com> Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
…stack (backport CosmWasm#1375) (CosmWasm#1379) * Use ICS4Wrapper to send raw IBC packets & fix Fee in wasm stack (cherry picked from commit 6dfa5cb) # Conflicts: # app/app.go # x/wasm/keeper/handler_plugin.go # x/wasm/keeper/keeper_cgo.go # x/wasm/keeper/keeper_test.go # x/wasm/keeper/options_test.go # x/wasm/keeper/test_common.go * Fix merge conflicts * Inline ibc packet sender interface and minor chore * Rename IBCPacketSender --------- Co-authored-by: Assaf Morami <assaf.morami@gmail.com> Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
This PR adds an ICS4Wrapper to the wasm keeper, which serves as the interface for sending raw IBC packets. This feature enables the chain to use middlewares with IBC-enabled contracts.
As of v0.31, the Fee middleware is only configured for incoming packets, as shown in this code snippet: link. On the other hand, for outgoing packets, the wasm keeper directly uses the IBC channel keeper, as shown here: link. Consequently, the Fee middleware is bypassed for outgoing packets. Since IBC relayer fees are paid on the sending chains, this means that a relayer can be paid for relaying IBC packets to wasmd but not for relaying packets from wasmd.