Skip to content
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.

Sign up for GitHub

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

Integrate Packet Forward Middleware #3911

Merged
merged 27 commits into from
Feb 8, 2023
Merged

Integrate Packet Forward Middleware #3911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
89b8b23
added initial packet forward middleware integration
nicolaslara Jan 3, 2023
0820107
gofumpt
nicolaslara Jan 3, 2023
b987095
Merge branch 'main' into nicolas/packet-forward-middleware
nicolaslara Jan 4, 2023
aa7dd10
updated go.mod
nicolaslara Jan 4, 2023
f084a4b
correct value
nicolaslara Jan 4, 2023
b2654a1
Merge branch 'main' into nicolas/packet-forward-middleware
nicolaslara Jan 23, 2023
9d87a60
updating to match upstream
nicolaslara Jan 23, 2023
58859b4
added params
nicolaslara Jan 23, 2023
85a936f
gofumpt
nicolaslara Jan 23, 2023
bf161a3
Merge branch 'main' into nicolas/packet-forward-middleware
nicolaslara Jan 24, 2023
0b6df24
added PFM to upgrades
nicolaslara Jan 24, 2023
0e67297
go imports
nicolaslara Jan 24, 2023
19f81c4
added e2e test for PFM+wasm hoks
nicolaslara Jan 26, 2023
dd322be
refactor
nicolaslara Jan 27, 2023
a4f7082
refactored for reusability and added comments
nicolaslara Jan 27, 2023
03b1d09
Merge branch 'main' into nicolas/packet-forward-middleware
nicolaslara Jan 30, 2023
293a16c
updated to latest version that allows native json
nicolaslara Jan 30, 2023
fea6d54
removed unnecessary param
nicolaslara Jan 30, 2023
9168bb8
Merge branch 'main' into nicolas/packet-forward-middleware
nicolaslara Jan 30, 2023
6542dc3
Merge branch 'main' into nicolas/packet-forward-middleware
nicolaslara Feb 7, 2023
a014a31
reordering imports after merge
nicolaslara Feb 7, 2023
8208bfc
added denom check
nicolaslara Feb 7, 2023
66c2ccc
added changelog
nicolaslara Feb 7, 2023
9dbf94d
Update tests/e2e/e2e_test.go
nicolaslara Feb 8, 2023
ef52182
requiring the txs to succeed
nicolaslara Feb 8, 2023
080b7e6
fixed e2e tests
nicolaslara Feb 8, 2023
8b308ac
Merge branch 'main' into nicolas/packet-forward-middleware
nicolaslara Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#4107](https://github.com/osmosis-labs/osmosis/pull/4107) Add superfluid unbond partial amount
* [#4207](https://github.com/osmosis-labs/osmosis/pull/4207) Add support for Async Interchain Queries
* [#4248](https://github.com/osmosis-labs/osmosis/pull/4248) Add panic recovery to `MultihopEstimateInGivenExactAmountOut`, `MultihopEstimateOutGivenExactAmountIn` and `RouteExactAmountOut`
* [#3911](https://github.com/osmosis-labs/osmosis/pull/3911) Add Packet Forward Middleware

## Misc Improvements
* [#4131](https://github.com/osmosis-labs/osmosis/pull/4141) Add GatherValuesFromStorePrefixWithKeyParser function to osmoutils.
Expand Down
45 changes: 39 additions & 6 deletions app/keepers/keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ import (
ibckeeper "github.com/cosmos/ibc-go/v4/modules/core/keeper"
icqkeeper "github.com/strangelove-ventures/async-icq/keeper"

packetforward "github.com/strangelove-ventures/packet-forward-middleware/v4/router"
packetforwardkeeper "github.com/strangelove-ventures/packet-forward-middleware/v4/router/keeper"
packetforwardtypes "github.com/strangelove-ventures/packet-forward-middleware/v4/router/types"

// IBC Transfer: Defines the "transfer" IBC port
transfer "github.com/cosmos/ibc-go/v4/modules/apps/transfer"

Expand Down Expand Up @@ -149,6 +153,7 @@ type AppKeepers struct {
TransferStack *ibchooks.IBCMiddleware
Ics20WasmHooks *ibchooks.WasmHooks
HooksICS4Wrapper ibchooks.ICS4Middleware
PacketForwardKeeper *packetforwardkeeper.Keeper

// keys to access the substores
keys map[string]*sdk.KVStoreKey
Expand Down Expand Up @@ -470,19 +475,24 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
appKeepers.GovKeeper = &govKeeper
}

// Create the IBC Transfer Stack from bottom to top:
// WireICS20PreWasmKeeper Create the IBC Transfer Stack from bottom to top:
//
// * SendPacket. Originates from the transferKeeper and and goes up the stack:
// * SendPacket. Originates from the transferKeeper and goes up the stack:
// transferKeeper.SendPacket -> ibc_rate_limit.SendPacket -> ibc_hooks.SendPacket -> channel.SendPacket
// * RecvPacket, message that originates from core IBC and goes down to app, the flow is the other way
// channel.RecvPacket -> ibc_hooks.OnRecvPacket -> ibc_rate_limit.OnRecvPacket -> transfer.OnRecvPacket
// channel.RecvPacket -> ibc_hooks.OnRecvPacket -> ibc_rate_limit.OnRecvPacket -> forward.OnRecvPacket -> transfer.OnRecvPacket
//
// Note that the forward middleware is only integrated on the "reveive" direction. It can be safely skipped when sending.
// Note also that the forward middleware is called "router", but we are using the name "forward" for clarity
// This may later be renamed upstream: https://github.com/strangelove-ventures/packet-forward-middleware/issues/10
//
// After this, the wasm keeper is required to be set on both
// appkeepers.WasmHooks AND appKeepers.RateLimitingICS4Wrapper
func (appKeepers *AppKeepers) WireICS20PreWasmKeeper(
appCodec codec.Codec,
bApp *baseapp.BaseApp,
hooksKeeper *ibchookskeeper.Keeper) {
hooksKeeper *ibchookskeeper.Keeper,
) {
// Setup the ICS4Wrapper used by the hooks middleware
osmoPrefix := sdk.GetConfig().GetBech32AccountAddrPrefix()
wasmHooks := ibchooks.NewWasmHooks(hooksKeeper, nil, osmoPrefix) // The contract keeper needs to be set later
Expand Down Expand Up @@ -520,10 +530,31 @@ func (appKeepers *AppKeepers) WireICS20PreWasmKeeper(
)
appKeepers.TransferKeeper = &transferKeeper
appKeepers.RawIcs20TransferAppModule = transfer.NewAppModule(*appKeepers.TransferKeeper)
transferIBCModule := transfer.NewIBCModule(*appKeepers.TransferKeeper)

// Packet Forward Middleware
// Initialize packet forward middleware router
appKeepers.PacketForwardKeeper = packetforwardkeeper.NewKeeper(
appCodec,
appKeepers.keys[packetforwardtypes.StoreKey],
appKeepers.GetSubspace(packetforwardtypes.ModuleName),
appKeepers.TransferKeeper,
appKeepers.IBCKeeper.ChannelKeeper,
appKeepers.DistrKeeper,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhh, why does it need this? Community pool?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, the fee can be configured via a param as a percentage of the send and it funds the community pool.

Ideally this should go to stakers IMO, but the fee defaults to 0, so we can change that later

Copy link
Member

@ValarDragon ValarDragon Jan 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RIP, this is bad module design imo, should be a middleware responsibility. (Fee at the base transport layer bad design)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please expand on this @ValarDragon ? Would love to understand your perspective so we can improve it. I'm not sure what you mean, as this is the keeper for the IBC middleware.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why your transport layer for packet sending should have bips of fee bundled in at default. This is equivalent in design practice to saying our API for bank sends should take in a fee parameter as well, and have bips of all sends be captured.

You could in principle want this, but this can be pushed to wrappers of the main object, rather than being the main thing you import. (I'd prefer no capability of this in what we import, than having such a parameter forced)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(NOTE: This is very minor on the overall work going into this. Awesome job on the module! And I think my first comment "bad module design imo" was too harsh for the severity of opinion actually held)

appKeepers.BankKeeper,
// The ICS4Wrapper is replaced by the HooksICS4Wrapper instead of the channel so that sending can be overridden by the middleware
appKeepers.HooksICS4Wrapper,
)
packetForwardMiddleware := packetforward.NewIBCMiddleware(
transfer.NewIBCModule(*appKeepers.TransferKeeper),
appKeepers.PacketForwardKeeper,
0,
packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp,
packetforwardkeeper.DefaultRefundTransferPacketTimeoutTimestamp,
)

// RateLimiting IBC Middleware
rateLimitingTransferModule := ibcratelimit.NewIBCModule(transferIBCModule, appKeepers.RateLimitingICS4Wrapper)
rateLimitingTransferModule := ibcratelimit.NewIBCModule(packetForwardMiddleware, appKeepers.RateLimitingICS4Wrapper)

// Hooks Middleware
hooksTransferModule := ibchooks.NewIBCMiddleware(&rateLimitingTransferModule, &appKeepers.HooksICS4Wrapper)
appKeepers.TransferStack = &hooksTransferModule
Expand Down Expand Up @@ -600,6 +631,7 @@ func (appKeepers *AppKeepers) initParamsKeeper(appCodec codec.BinaryCodec, legac
paramsKeeper.Subspace(ibcratelimittypes.ModuleName)
paramsKeeper.Subspace(concentratedliquiditytypes.ModuleName)
paramsKeeper.Subspace(icqtypes.ModuleName)
paramsKeeper.Subspace(packetforwardtypes.ModuleName).WithKeyTable(packetforwardtypes.ParamKeyTable())

return paramsKeeper
}
Expand Down Expand Up @@ -701,5 +733,6 @@ func KVStoreKeys() []string {
protorevtypes.StoreKey,
ibchookstypes.StoreKey,
icqtypes.StoreKey,
packetforwardtypes.StoreKey,
}
}
2 changes: 2 additions & 0 deletions app/keepers/modules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
transfer "github.com/cosmos/ibc-go/v4/modules/apps/transfer"
ibc "github.com/cosmos/ibc-go/v4/modules/core"
ibcclientclient "github.com/cosmos/ibc-go/v4/modules/core/02-client/client"
"github.com/strangelove-ventures/packet-forward-middleware/v4/router"

"github.com/cosmos/cosmos-sdk/types/module"
"github.com/cosmos/cosmos-sdk/x/auth"
Expand Down Expand Up @@ -103,4 +104,5 @@ var AppModuleBasics = []module.AppModuleBasic{
ica.AppModuleBasic{},
ibc_hooks.AppModuleBasic{},
ibc_rate_limit.AppModuleBasic{},
router.AppModuleBasic{},
}
3 changes: 2 additions & 1 deletion app/upgrades/v15/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package v15
import (
store "github.com/cosmos/cosmos-sdk/store/types"
icqtypes "github.com/strangelove-ventures/async-icq/types"
packetforwardtypes "github.com/strangelove-ventures/packet-forward-middleware/v4/router/types"

"github.com/osmosis-labs/osmosis/v14/app/upgrades"
cltypes "github.com/osmosis-labs/osmosis/v14/x/concentrated-liquidity/types"
Expand All @@ -18,7 +19,7 @@ var Upgrade = upgrades.Upgrade{
UpgradeName: UpgradeName,
CreateUpgradeHandler: CreateUpgradeHandler,
StoreUpgrades: store.StoreUpgrades{
Added: []string{poolmanagertypes.StoreKey, cltypes.StoreKey, valsetpreftypes.StoreKey, protorevtypes.StoreKey, icqtypes.StoreKey},
Added: []string{poolmanagertypes.StoreKey, cltypes.StoreKey, valsetpreftypes.StoreKey, protorevtypes.StoreKey, icqtypes.StoreKey, packetforwardtypes.StoreKey},
Deleted: []string{},
},
}
2 changes: 2 additions & 0 deletions app/upgrades/v15/upgrades.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
packetforwardtypes "github.com/strangelove-ventures/packet-forward-middleware/v4/router/types"

"github.com/osmosis-labs/osmosis/v14/app/keepers"
"github.com/osmosis-labs/osmosis/v14/app/upgrades"
Expand All @@ -23,6 +24,7 @@ func CreateUpgradeHandler(
poolmanagerParams := poolmanagertypes.NewParams(keepers.GAMMKeeper.GetParams(ctx).PoolCreationFee)

keepers.PoolManagerKeeper.SetParams(ctx, poolmanagerParams)
keepers.PacketForwardKeeper.SetParams(ctx, packetforwardtypes.DefaultParams())

// N.B: pool id in gamm is to be deprecated in the future
// Instead,it is moved to poolmanager.
Expand Down
3 changes: 3 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ require (
github.com/golangci/golangci-lint v1.51.1
github.com/gorilla/mux v1.8.0
github.com/grpc-ecosystem/grpc-gateway v1.16.0
github.com/iancoleman/orderedmap v0.2.0
github.com/mattn/go-sqlite3 v1.14.16
github.com/ory/dockertest/v3 v3.9.1
github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3
Expand All @@ -28,6 +29,7 @@ require (
github.com/spf13/viper v1.15.0
// Async ICQ branch: ibc-v4
github.com/strangelove-ventures/async-icq v0.0.0-20230116084035-5609e84dd443
github.com/strangelove-ventures/packet-forward-middleware/v4 v4.0.3
github.com/stretchr/testify v1.8.1
github.com/tendermint/tendermint v0.34.24
github.com/tendermint/tm-db v0.6.8-0.20220506192307-f628bb5dc95b
Expand Down Expand Up @@ -319,4 +321,5 @@ replace (
github.com/tendermint/tendermint => github.com/informalsystems/tendermint v0.34.24
// use grpc compatible with cosmos protobufs
google.golang.org/grpc => google.golang.org/grpc v1.33.2

)
4 changes: 4 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,8 @@ github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpO
github.com/hudl/fargo v1.3.0/go.mod h1:y3CKSmjA+wD2gak7sUSXTAoopbhU08POFhmITJgmKTg=
github.com/huin/goupnp v1.0.0/go.mod h1:n9v9KO1tAxYH82qOn+UTIFQDmx5n1Zxd/ClZDMX7Bnc=
github.com/huin/goutil v0.0.0-20170803182201-1ca381bf3150/go.mod h1:PpLOETDnJ0o3iZrZfqZzyLl6l7F3c6L1oWn7OICBi6o=
github.com/iancoleman/orderedmap v0.2.0 h1:sq1N/TFpYH++aViPcaKjys3bDClUEU7s5B+z6jq8pNA=
github.com/iancoleman/orderedmap v0.2.0/go.mod h1:N0Wam8K1arqPXNWjMo21EXnBPOPp36vB07FNRdD2geA=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/imdario/mergo v0.3.13 h1:lFzP57bqS/wsqKssCGmtLAb8A0wKjLGrve2q3PPVcBk=
Expand Down Expand Up @@ -1061,6 +1063,8 @@ github.com/steakknife/bloomfilter v0.0.0-20180922174646-6819c0d2a570/go.mod h1:8
github.com/steakknife/hamming v0.0.0-20180906055917-c99c65617cd3/go.mod h1:hpGUWaI9xL8pRQCTXQgocU38Qw1g0Us7n5PxxTwTCYU=
github.com/strangelove-ventures/async-icq v0.0.0-20230116084035-5609e84dd443 h1:nRkSfj9IhUrGF+0MRQHs4f8mtHFPe+0li7/k3Y4k0cs=
github.com/strangelove-ventures/async-icq v0.0.0-20230116084035-5609e84dd443/go.mod h1:9dxdEF3Lo8WvLYx1e2knGkz3tmiPVX0ohODzUjGl9G4=
github.com/strangelove-ventures/packet-forward-middleware/v4 v4.0.3 h1:3E7I9C+gM7n0+OkI7JmvWH5PGD6pZOIc1+mUUooh6dI=
github.com/strangelove-ventures/packet-forward-middleware/v4 v4.0.3/go.mod h1:AG8F5pdk3x1h7PlRvPoMem3623W+w8HJHrWYkVJ51kk=
github.com/streadway/amqp v0.0.0-20190404075320-75d898a42a94/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw=
github.com/streadway/amqp v0.0.0-20190827072141-edfb9018d271/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw=
github.com/streadway/handy v0.0.0-20190108123426-d5acb3125c2a/go.mod h1:qNTQ5P5JnDBl6z3cMAg/SywNDC5ABu5ApDIw6lUbRmI=
Expand Down
4 changes: 4 additions & 0 deletions go.work.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,8 @@ github.com/hashicorp/uuid v0.0.0-20160311170451-ebb0a03e909c/go.mod h1:fHzc09Uny
github.com/huandu/xstrings v1.0.0/go.mod h1:4qWG/gcEcfX4z/mBDHJ++3ReCw9ibxbsNJbcucJdbSo=
github.com/huandu/xstrings v1.2.0/go.mod h1:DvyZB1rfVYsBIigL8HwpZgxHwXozlTgGqn63UyNX5k4=
github.com/hudl/fargo v1.4.0/go.mod h1:9Ai6uvFy5fQNq6VPKtg+Ceq1+eTY4nKUlR2JElEOcDo=
github.com/iancoleman/orderedmap v0.2.0 h1:sq1N/TFpYH++aViPcaKjys3bDClUEU7s5B+z6jq8pNA=
github.com/iancoleman/orderedmap v0.2.0/go.mod h1:N0Wam8K1arqPXNWjMo21EXnBPOPp36vB07FNRdD2geA=
github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
github.com/imdario/mergo v0.3.4/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/imdario/mergo v0.3.5/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
Expand Down Expand Up @@ -1282,6 +1284,8 @@ github.com/spf13/viper v1.12.0/go.mod h1:b6COn30jlNxbm/V2IqWiNWkJ+vZNiMNksliPCiu
github.com/spf13/viper v1.13.0/go.mod h1:Icm2xNL3/8uyh/wFuB1jI7TiTNKp8632Nwegu+zgdYw=
github.com/stefanberger/go-pkcs11uri v0.0.0-20201008174630-78d3cae3a980/go.mod h1:AO3tvPzVZ/ayst6UlUKUv6rcPQInYe3IknH3jYhAKu8=
github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
github.com/strangelove-ventures/packet-forward-middleware/v4 v4.0.3 h1:3E7I9C+gM7n0+OkI7JmvWH5PGD6pZOIc1+mUUooh6dI=
github.com/strangelove-ventures/packet-forward-middleware/v4 v4.0.3/go.mod h1:AG8F5pdk3x1h7PlRvPoMem3623W+w8HJHrWYkVJ51kk=
github.com/streadway/amqp v1.0.0/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw=
github.com/streadway/handy v0.0.0-20200128134331-0f66f006fb2e/go.mod h1:qNTQ5P5JnDBl6z3cMAg/SywNDC5ABu5ApDIw6lUbRmI=
github.com/stretchr/objx v0.0.0-20180129172003-8a3f7159479f/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
Expand Down
Loading