From 7b51ebdd740a6fd924cf85d2388982f676898da8 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 3 Aug 2021 12:08:02 +0200 Subject: [PATCH 01/21] do handshake logic, create test file --- modules/apps/29-fee/module.go | 45 ++++++++++++++----- modules/apps/29-fee/module_test.go | 0 modules/apps/29-fee/types/errors.go | 6 ++- modules/apps/29-fee/types/keys.go | 2 + modules/core/04-channel/types/version.go | 19 ++++++++ modules/core/04-channel/types/version_test.go | 43 ++++++++++++++++++ 6 files changed, 103 insertions(+), 12 deletions(-) create mode 100644 modules/apps/29-fee/module_test.go create mode 100644 modules/core/04-channel/types/version.go create mode 100644 modules/core/04-channel/types/version_test.go diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index c0645e3db78..0ef821e1a37 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -1,6 +1,5 @@ package fee -/* import ( "context" "encoding/json" @@ -90,12 +89,14 @@ func (AppModuleBasic) GetQueryCmd() *cobra.Command { type AppModule struct { AppModuleBasic keeper keeper.Keeper + app porttypes.IBCModule } // NewAppModule creates a new 29-fee module -func NewAppModule(k keeper.Keeper) AppModule { +func NewAppModule(k keeper.Keeper, app porttypes.IBCModule) AppModule { return AppModule{ keeper: k, + app: app, } } @@ -191,7 +192,13 @@ func (am AppModule) OnChanOpenInit( counterparty channeltypes.Counterparty, version string, ) error { - return nil + feeVersion, appVersion := channeltypes.SplitChannelVersion(version) + if feeVersion != types.Version { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) + } + // call underlying app's OnChanOpenInit callback with the appVersion + return am.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, + chanCap, counterparty, appVersion) } // OnChanOpenTry implements the IBCModule interface @@ -206,7 +213,18 @@ func (am AppModule) OnChanOpenTry( version, counterpartyVersion string, ) error { - return nil + feeVersion, appVersion := channeltypes.SplitChannelVersion(version) + cpFeeVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) + + if feeVersion != types.Version { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) + } + if cpFeeVersion != feeVersion { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) + } + // call underlying app's OnChanOpenTry callback with the app versions + return am.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, + chanCap, counterparty, appVersion, cpAppVersion) } // OnChanOpenAck implements the IBCModule interface @@ -216,7 +234,13 @@ func (am AppModule) OnChanOpenAck( channelID string, counterpartyVersion string, ) error { - return nil + cpFeeVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) + + if cpFeeVersion != types.Version { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) + } + // call underlying app's OnChanOpenAck callback with the counterparty app version. + return am.app.OnChanOpenAck(ctx, portID, channelID, cpAppVersion) } // OnChanOpenConfirm implements the IBCModule interface @@ -225,7 +249,8 @@ func (am AppModule) OnChanOpenConfirm( portID, channelID string, ) error { - return nil + // call underlying app's OnChanOpenConfirm callback. + return am.app.OnChanOpenConfirm(ctx, portID, channelID) } // OnChanCloseInit implements the IBCModule interface @@ -234,8 +259,8 @@ func (am AppModule) OnChanCloseInit( portID, channelID string, ) error { - // Disallow user-initiated channel closing for 29-fee channels - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "user cannot close channel") + // TODO: Unescrow all remaining funds for unprocessed packets + return am.app.OnChanCloseInit(ctx, portID, channelID) } // OnChanCloseConfirm implements the IBCModule interface @@ -244,7 +269,8 @@ func (am AppModule) OnChanCloseConfirm( portID, channelID string, ) error { - return nil + // TODO: Unescrow all remaining funds for unprocessed packets + return am.app.OnChanCloseConfirm(ctx, portID, channelID) } // OnRecvPacket implements the IBCModule interface. @@ -274,4 +300,3 @@ func (am AppModule) OnTimeoutPacket( ) error { return nil } -*/ diff --git a/modules/apps/29-fee/module_test.go b/modules/apps/29-fee/module_test.go new file mode 100644 index 00000000000..e69de29bb2d diff --git a/modules/apps/29-fee/types/errors.go b/modules/apps/29-fee/types/errors.go index 9ed5755a9a0..5536d326fc0 100644 --- a/modules/apps/29-fee/types/errors.go +++ b/modules/apps/29-fee/types/errors.go @@ -1,8 +1,10 @@ package types import ( -// sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) // 29-fee sentinel errors -var () +var ( + ErrInvalidVersion = sdkerrors.Register(ModuleName, 2, "invalid ICS29 middleware version") +) diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index 769113f504a..8a6de3b77ba 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -12,4 +12,6 @@ const ( // QuerierRoute is the querier route for IBC transfer QuerierRoute = ModuleName + + Version = "fee29-1" ) diff --git a/modules/core/04-channel/types/version.go b/modules/core/04-channel/types/version.go new file mode 100644 index 00000000000..f739cf9de27 --- /dev/null +++ b/modules/core/04-channel/types/version.go @@ -0,0 +1,19 @@ +package types + +import "strings" + +// SplitChannelVersion middleware version will split the channel version string +// into the outermost middleware version and the underlying app version. +// It will use the default delimiter `:` for middleware versions. +// In case there's no delimeter, this function returns an empty string for the middleware version (first return argument), +// and the full input as the second underlying app version. +func SplitChannelVersion(version string) (middlewareVersion, appVersion string) { + // only split out the first middleware version + splitVersions := strings.Split(version, ":") + if len(splitVersions) == 1 { + return "", version + } + middlewareVersion = splitVersions[0] + appVersion = strings.Join(splitVersions[1:], ":") + return +} diff --git a/modules/core/04-channel/types/version_test.go b/modules/core/04-channel/types/version_test.go new file mode 100644 index 00000000000..13f86a96528 --- /dev/null +++ b/modules/core/04-channel/types/version_test.go @@ -0,0 +1,43 @@ +package types_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/ibc-go/modules/core/04-channel/types" +) + +func TestSplitVersions(t *testing.T) { + testCases := []struct { + name string + version string + mwVersion string + appVersion string + }{ + { + "single wrapped middleware", + "fee29-1:ics20-1", + "fee29-1", + "ics20-1", + }, + { + "multiple wrapped middleware", + "fee29-1:whitelist:ics20-1", + "fee29-1", + "whitelist:ics20-1", + }, + { + "no middleware", + "ics20-1", + "", + "ics20-1", + }, + } + + for _, tc := range testCases { + mwVersion, appVersion := types.SplitChannelVersion(tc.version) + require.Equal(t, tc.mwVersion, mwVersion, "middleware version is unexpected for case: %s", tc.name) + require.Equal(t, tc.appVersion, appVersion, "app version is unexpected for case: %s", tc.name) + } +} From 150211de65fdcd23addc654b338fe07cc928d0a7 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 5 Aug 2021 15:00:05 +0200 Subject: [PATCH 02/21] do cap logic and fix build --- modules/apps/29-fee/keeper/keeper.go | 17 +--- modules/apps/29-fee/module.go | 111 +++++++++++++++++++-------- modules/apps/29-fee/module_test.go | 1 + modules/apps/29-fee/types/keys.go | 8 ++ 4 files changed, 87 insertions(+), 50 deletions(-) diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index db304dd4cf9..c57b81ee10e 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -1,13 +1,11 @@ package keeper -/* import ( "github.com/tendermint/tendermint/libs/log" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" - capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" "github.com/cosmos/ibc-go/modules/apps/transfer/types" host "github.com/cosmos/ibc-go/modules/core/24-host" @@ -25,7 +23,6 @@ type Keeper struct { scopedKeeper capabilitykeeper.ScopedKeeper } - // NewKeeper creates a new 29-fee Keeper instance func NewKeeper( cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace, @@ -59,7 +56,7 @@ func (k Keeper) IsBound(ctx sdk.Context, portID string) bool { // order to expose it to module's InitGenesis function func (k Keeper) BindPort(ctx sdk.Context, portID string) error { cap := k.portKeeper.BindPort(ctx, portID) - return k.ClaimCapability(ctx, cap, host.PortPath(portID)) + return k.scopedKeeper.ClaimCapability(ctx, cap, host.PortPath(portID)) } // GetPort returns the portID for the transfer module. Used in ExportGenesis @@ -73,15 +70,3 @@ func (k Keeper) SetPort(ctx sdk.Context, portID string) { store := ctx.KVStore(k.storeKey) store.Set(types.PortKey, []byte(portID)) } - -// AuthenticateCapability wraps the scopedKeeper's AuthenticateCapability function -func (k Keeper) AuthenticateCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) bool { - return k.scopedKeeper.AuthenticateCapability(ctx, cap, name) -} - -// ClaimCapability allows the transfer module that can claim a capability that IBC module -// passes to it -func (k Keeper) ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) error { - return k.scopedKeeper.ClaimCapability(ctx, cap, name) -} -*/ diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index 0ef821e1a37..ef2e9fcb50c 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -3,7 +3,6 @@ package fee import ( "context" "encoding/json" - "fmt" "math/rand" "github.com/cosmos/cosmos-sdk/client" @@ -19,12 +18,17 @@ import ( "github.com/spf13/cobra" abci "github.com/tendermint/tendermint/abci/types" - "github.com/cosmos/ibc-go/modules/apps/transfer/client/cli" - "github.com/cosmos/ibc-go/modules/apps/transfer/keeper" - "github.com/cosmos/ibc-go/modules/apps/transfer/simulation" - "github.com/cosmos/ibc-go/modules/apps/transfer/types" + capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" + "github.com/cosmos/ibc-go/modules/apps/29-fee/client/cli" + "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" + "github.com/cosmos/ibc-go/modules/apps/29-fee/types" + + // "github.com/cosmos/ibc-go/modules/apps/29-fee/client/cli" + // "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" + // "github.com/cosmos/ibc-go/modules/apps/29-fee/simulation" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/modules/core/05-port/types" + host "github.com/cosmos/ibc-go/modules/core/24-host" ibcexported "github.com/cosmos/ibc-go/modules/core/exported" ) @@ -47,30 +51,32 @@ func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {} // RegisterInterfaces registers module concrete types into protobuf Any. func (AppModuleBasic) RegisterInterfaces(registry codectypes.InterfaceRegistry) { - types.RegisterInterfaces(registry) + // types.RegisterInterfaces(registry) } // DefaultGenesis returns default genesis state as raw bytes for the ibc -// transfer module. +// 29-fee module. func (AppModuleBasic) DefaultGenesis(cdc codec.JSONCodec) json.RawMessage { - return cdc.MustMarshalJSON(types.DefaultGenesisState()) + // return cdc.MustMarshalJSON(types.DefaultGenesisState()) + return nil } // ValidateGenesis performs genesis state validation for the 29-fee module. func (AppModuleBasic) ValidateGenesis(cdc codec.JSONCodec, config client.TxEncodingConfig, bz json.RawMessage) error { - var gs types.GenesisState - if err := cdc.UnmarshalJSON(bz, &gs); err != nil { - return fmt.Errorf("failed to unmarshal %s genesis state: %w", types.ModuleName, err) - } + // var gs types.GenesisState + // if err := cdc.UnmarshalJSON(bz, &gs); err != nil { + // return fmt.Errorf("failed to unmarshal %s genesis state: %w", types.ModuleName, err) + // } - return gs.Validate() + // return gs.Validate() + return nil } // RegisterRESTRoutes implements AppModuleBasic interface func (AppModuleBasic) RegisterRESTRoutes(clientCtx client.Context, rtr *mux.Router) { } -// RegisterGRPCGatewayRoutes registers the gRPC Gateway routes for the ibc-transfer module. +// RegisterGRPCGatewayRoutes registers the gRPC Gateway routes for the ibc-29-fee module. func (AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *runtime.ServeMux) { types.RegisterQueryHandlerClient(context.Background(), mux, types.NewQueryClient(clientCtx)) } @@ -88,8 +94,9 @@ func (AppModuleBasic) GetQueryCmd() *cobra.Command { // AppModule represents the AppModule for this module type AppModule struct { AppModuleBasic - keeper keeper.Keeper - app porttypes.IBCModule + keeper keeper.Keeper + scopedKeeper capabilitykeeper.ScopedKeeper + app porttypes.IBCModule } // NewAppModule creates a new 29-fee module @@ -122,24 +129,25 @@ func (am AppModule) LegacyQuerierHandler(*codec.LegacyAmino) sdk.Querier { // RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { - types.RegisterMsgServer(cfg.MsgServer(), am.keeper) - types.RegisterQueryServer(cfg.QueryServer(), am.keeper) + // types.RegisterMsgServer(cfg.MsgServer(), am.keeper) + // types.RegisterQueryServer(cfg.QueryServer(), am.keeper) } -// InitGenesis performs genesis initialization for the ibc-transfer module. It returns +// InitGenesis performs genesis initialization for the ibc-29-fee module. It returns // no validator updates. func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, data json.RawMessage) []abci.ValidatorUpdate { - var genesisState types.GenesisState - cdc.MustUnmarshalJSON(data, &genesisState) - am.keeper.InitGenesis(ctx, genesisState) + // var genesisState types.GenesisState + // cdc.MustUnmarshalJSON(data, &genesisState) + // am.keeper.InitGenesis(ctx, genesisState) return []abci.ValidatorUpdate{} } -// ExportGenesis returns the exported genesis state as raw bytes for the ibc-transfer +// ExportGenesis returns the exported genesis state as raw bytes for the ibc-29-fee // module. func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.RawMessage { - gs := am.keeper.ExportGenesis(ctx) - return cdc.MustMarshalJSON(gs) + // gs := am.keeper.ExportGenesis(ctx) + // return cdc.MustMarshalJSON(gs) + return nil } // ConsensusVersion implements AppModule/ConsensusVersion. @@ -156,9 +164,9 @@ func (am AppModule) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) []abci.V // AppModuleSimulation functions -// GenerateGenesisState creates a randomized GenState of the transfer module. +// GenerateGenesisState creates a randomized GenState of the 29-fee module. func (AppModule) GenerateGenesisState(simState *module.SimulationState) { - simulation.RandomizedGenState(simState) + // simulation.RandomizedGenState(simState) } // ProposalContents doesn't return any content functions for governance proposals. @@ -166,17 +174,18 @@ func (AppModule) ProposalContents(_ module.SimulationState) []simtypes.WeightedP return nil } -// RandomizedParams creates randomized ibc-transfer param changes for the simulator. +// RandomizedParams creates randomized ibc-29-fee param changes for the simulator. func (AppModule) RandomizedParams(r *rand.Rand) []simtypes.ParamChange { - return simulation.ParamChanges(r) + // return simulation.ParamChanges(r) + return nil } -// RegisterStoreDecoder registers a decoder for transfer module's types +// RegisterStoreDecoder registers a decoder for 29-fee module's types func (am AppModule) RegisterStoreDecoder(sdr sdk.StoreDecoderRegistry) { - sdr[types.StoreKey] = simulation.NewDecodeStore(am.keeper) + // sdr[types.StoreKey] = simulation.NewDecodeStore(am.keeper) } -// WeightedOperations returns the all the transfer module operations with their respective weights. +// WeightedOperations returns the all the 29-fee module operations with their respective weights. func (am AppModule) WeightedOperations(_ module.SimulationState) []simtypes.WeightedOperation { return nil } @@ -196,9 +205,19 @@ func (am AppModule) OnChanOpenInit( if feeVersion != types.Version { return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) } + // Claim channel capability passed back by IBC module + if err := am.scopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { + return err + } + + appCap, err := am.scopedKeeper.NewCapability(ctx, types.AppCapabilityName(channelID, portID)) + if err != nil { + return sdkerrors.Wrap(err, "could not create capability for underlying application") + } + // call underlying app's OnChanOpenInit callback with the appVersion return am.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, - chanCap, counterparty, appVersion) + appCap, counterparty, appVersion) } // OnChanOpenTry implements the IBCModule interface @@ -222,9 +241,33 @@ func (am AppModule) OnChanOpenTry( if cpFeeVersion != feeVersion { return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) } + var ( + appCap *capabilitytypes.Capability + err error + ok bool + ) + // Module may have already claimed capability in OnChanOpenInit in the case of crossing hellos + // (ie chainA and chainB both call ChanOpenInit before one of them calls ChanOpenTry) + // If module can already authenticate the capability then module already owns it so we don't need to claim + // Otherwise, module does not have channel capability and we must claim it from IBC + if !am.scopedKeeper.AuthenticateCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)) { + // Only claim channel capability passed back by IBC module if we do not already own it + if err := am.scopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { + return err + } + appCap, err = am.scopedKeeper.NewCapability(ctx, types.AppCapabilityName(channelID, portID)) + if err != nil { + return sdkerrors.Wrap(err, "could not create capability for underlying app") + } + } + appCap, ok = am.scopedKeeper.GetCapability(ctx, types.AppCapabilityName(channelID, portID)) + if !ok { + return sdkerrors.Wrap(capabilitytypes.ErrCapabilityNotFound, + "could not find app capability on OnChanOpenTry even after OnChanOpenInit called on this chain first (crossing hellos)") + } // call underlying app's OnChanOpenTry callback with the app versions return am.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, - chanCap, counterparty, appVersion, cpAppVersion) + appCap, counterparty, appVersion, cpAppVersion) } // OnChanOpenAck implements the IBCModule interface diff --git a/modules/apps/29-fee/module_test.go b/modules/apps/29-fee/module_test.go index e69de29bb2d..d8ed3f5b3d3 100644 --- a/modules/apps/29-fee/module_test.go +++ b/modules/apps/29-fee/module_test.go @@ -0,0 +1 @@ +package fee_test diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index 8a6de3b77ba..6d538733031 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -1,5 +1,7 @@ package types +import "fmt" + const ( // ModuleName defines the 29-fee name ModuleName = "ibcfee" @@ -14,4 +16,10 @@ const ( QuerierRoute = ModuleName Version = "fee29-1" + + KeyAppCapability = "app_capabilities" ) + +func AppCapabilityName(channelID, portID string) string { + return fmt.Sprintf("%s/%s/%s", KeyAppCapability, channelID, portID) +} From ae319eaccf5a0108bf7c00ab97e36a711d1d6733 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 2 Sep 2021 11:23:15 +0200 Subject: [PATCH 03/21] open handshake implementation and tests --- docs/ibc/proto-docs.md | 2 +- modules/apps/29-fee/fee_test.go | 38 +++ modules/apps/29-fee/module.go | 23 +- modules/apps/29-fee/module_test.go | 248 ++++++++++++++++++ modules/apps/29-fee/types/keys.go | 4 +- modules/apps/transfer/keeper/genesis.go | 18 +- modules/apps/transfer/keeper/genesis_test.go | 2 +- modules/apps/transfer/keeper/keeper.go | 16 +- modules/apps/transfer/module.go | 21 +- modules/apps/transfer/module_test.go | 40 ++- modules/apps/transfer/simulation/genesis.go | 2 +- .../apps/transfer/simulation/genesis_test.go | 2 +- modules/apps/transfer/types/genesis.go | 12 +- modules/apps/transfer/types/genesis.pb.go | 78 +++--- modules/apps/transfer/types/genesis_test.go | 4 +- modules/apps/transfer/types/keys.go | 3 + .../applications/transfer/v1/genesis.proto | 2 +- testing/simapp/app.go | 16 +- 18 files changed, 442 insertions(+), 89 deletions(-) create mode 100644 modules/apps/29-fee/fee_test.go diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md index f571101508f..c1b639d7c3e 100644 --- a/docs/ibc/proto-docs.md +++ b/docs/ibc/proto-docs.md @@ -1048,7 +1048,7 @@ GenesisState defines the ibc-transfer genesis state | Field | Type | Label | Description | | ----- | ---- | ----- | ----------- | -| `port_id` | [string](#string) | | | +| `port_ids` | [string](#string) | repeated | | | `denom_traces` | [DenomTrace](#ibc.applications.transfer.v1.DenomTrace) | repeated | | | `params` | [Params](#ibc.applications.transfer.v1.Params) | | | diff --git a/modules/apps/29-fee/fee_test.go b/modules/apps/29-fee/fee_test.go new file mode 100644 index 00000000000..47644661071 --- /dev/null +++ b/modules/apps/29-fee/fee_test.go @@ -0,0 +1,38 @@ +package fee_test + +import ( + "testing" + + fee "github.com/cosmos/ibc-go/modules/apps/29-fee" + feekeeper "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" + "github.com/cosmos/ibc-go/modules/apps/transfer" + ibctesting "github.com/cosmos/ibc-go/testing" + "github.com/stretchr/testify/suite" +) + +type FeeTestSuite struct { + suite.Suite + + coordinator *ibctesting.Coordinator + + chainA *ibctesting.TestChain + chainB *ibctesting.TestChain + + module fee.AppModule + keeper feekeeper.Keeper +} + +func (suite *FeeTestSuite) SetupTest() { + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2) + suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(0)) + suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1)) + + suite.keeper = suite.chainA.GetSimApp().IBCFeeKeeper + + transferModule := transfer.NewAppModule(suite.chainA.GetSimApp().TransferKeeper) + suite.module = fee.NewAppModule(suite.keeper, suite.chainA.GetSimApp().ScopedIBCFeeKeeper, transferModule) +} + +func TestIBCFeeTestSuite(t *testing.T) { + suite.Run(t, new(FeeTestSuite)) +} diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index ef2e9fcb50c..09dc17336fd 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -24,7 +24,6 @@ import ( "github.com/cosmos/ibc-go/modules/apps/29-fee/types" // "github.com/cosmos/ibc-go/modules/apps/29-fee/client/cli" - // "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" // "github.com/cosmos/ibc-go/modules/apps/29-fee/simulation" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/modules/core/05-port/types" @@ -100,10 +99,11 @@ type AppModule struct { } // NewAppModule creates a new 29-fee module -func NewAppModule(k keeper.Keeper, app porttypes.IBCModule) AppModule { +func NewAppModule(k keeper.Keeper, scopedKeeper capabilitykeeper.ScopedKeeper, app porttypes.IBCModule) AppModule { return AppModule{ - keeper: k, - app: app, + keeper: k, + scopedKeeper: scopedKeeper, + app: app, } } @@ -210,7 +210,7 @@ func (am AppModule) OnChanOpenInit( return err } - appCap, err := am.scopedKeeper.NewCapability(ctx, types.AppCapabilityName(channelID, portID)) + appCap, err := am.scopedKeeper.NewCapability(ctx, types.AppCapabilityName(portID, channelID)) if err != nil { return sdkerrors.Wrap(err, "could not create capability for underlying application") } @@ -255,15 +255,16 @@ func (am AppModule) OnChanOpenTry( if err := am.scopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { return err } - appCap, err = am.scopedKeeper.NewCapability(ctx, types.AppCapabilityName(channelID, portID)) + appCap, err = am.scopedKeeper.NewCapability(ctx, types.AppCapabilityName(portID, channelID)) if err != nil { return sdkerrors.Wrap(err, "could not create capability for underlying app") } - } - appCap, ok = am.scopedKeeper.GetCapability(ctx, types.AppCapabilityName(channelID, portID)) - if !ok { - return sdkerrors.Wrap(capabilitytypes.ErrCapabilityNotFound, - "could not find app capability on OnChanOpenTry even after OnChanOpenInit called on this chain first (crossing hellos)") + } else { + appCap, ok = am.scopedKeeper.GetCapability(ctx, types.AppCapabilityName(portID, channelID)) + if !ok { + return sdkerrors.Wrap(capabilitytypes.ErrCapabilityNotFound, + "could not find app capability on OnChanOpenTry even after OnChanOpenInit called on this chain first (crossing hellos)") + } } // call underlying app's OnChanOpenTry callback with the app versions return am.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, diff --git a/modules/apps/29-fee/module_test.go b/modules/apps/29-fee/module_test.go index d8ed3f5b3d3..372dc5d9e93 100644 --- a/modules/apps/29-fee/module_test.go +++ b/modules/apps/29-fee/module_test.go @@ -1 +1,249 @@ package fee_test + +import ( + "fmt" + + "github.com/cosmos/ibc-go/modules/apps/29-fee/types" + transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" + host "github.com/cosmos/ibc-go/modules/core/24-host" +) + +func (suite *FeeTestSuite) TestOnChanOpenInit() { + testCases := []struct { + name string + version string + expPass bool + }{ + { + "valid fee middleware and transfer version", + "fee29-1:ics20-1", + true, + }, + { + "invalid fee middleware version", + "otherfee28-1:ics20-1", + false, + }, + { + "invalid transfer version", + "fee29-1:wrongics20-1", + false, + }, + { + "incorrect wrapping delimiter", + "fee29-1//ics20-1", + false, + }, + { + "transfer version not wrapped", + "fee29-1", + false, + }, + { + "fee version not included", + "ics20-1", + false, + }, + { + "hanging delimiter", + "fee29-1:ics20-1:", + false, + }, + } + + for _, tc := range testCases { + ctx := suite.chainA.GetContext() + fmt.Println(ctx) + fmt.Println(tc.name) + cap, err := suite.chainA.GetSimApp().ScopedIBCKeeper.NewCapability(ctx, tc.name) + suite.Require().NoError(err) + err = suite.module.OnChanOpenInit( + ctx, + channeltypes.UNORDERED, + []string{"connection-1"}, + transfertypes.FeePortID, + "channel-1", + cap, + channeltypes.NewCounterparty(transfertypes.FeePortID, "channel-2"), + tc.version, + ) + + if tc.expPass { + suite.Require().NoError(err, "unexpected error from version: %s", tc.version) + + // check that capabilities are properly claimed and issued + ctx := suite.chainA.GetContext() + ibcCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) + suite.Require().NotNil(ibcCap, "IBC capability is nil on fee keeper") + suite.Require().True(ok) + + appFeeCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, types.AppCapabilityName(transfertypes.FeePortID, "channel-1")) + suite.Require().NotNil(appFeeCap, "App capability not created or owned by ibc fee keeper") + suite.Require().True(ok) + transferCap, ok := suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) + suite.Require().NotNil(transferCap, "App capability not claimed by transfer keeper") + suite.Require().True(ok) + suite.Require().Equal(appFeeCap, transferCap, "app capabilities not equal") + } else { + suite.Require().Error(err, "error not returned for version: %s", tc.version) + } + } +} + +func (suite *FeeTestSuite) TestOnChanOpenTry() { + testCases := []struct { + name string + version string + cpVersion string + capExists bool + expPass bool + }{ + { + "valid fee middleware and transfer version", + "fee29-1:ics20-1", + "fee29-1:ics20-1", + false, + true, + }, + { + "valid fee middleware and transfer version, crossing hellos", + "fee29-1:ics20-1", + "fee29-1:ics20-1", + true, + true, + }, + { + "invalid fee middleware version", + "otherfee28-1:ics20-1", + "fee29-1:ics20-1", + false, + false, + }, + { + "invalid counterparty fee middleware version", + "fee29-1:ics20-1", + "wrongfee29-1:ics20-1", + false, + false, + }, + { + "invalid transfer version", + "fee29-1:wrongics20-1", + "fee29-1:ics20-1", + false, + false, + }, + { + "invalid counterparty transfer version", + "fee29-1:ics20-1", + "fee29-1:wrongics20-1", + false, + false, + }, + { + "transfer version not wrapped", + "fee29-1", + "fee29-1:ics20-1", + false, + false, + }, + { + "counterparty transfer version not wrapped", + "fee29-1:ics20-1", + "fee29-1", + false, + false, + }, + { + "fee version not included", + "ics20-1", + "fee29-1:ics20-1", + false, + false, + }, + { + "transfer version not included", + "fee29-1:ics20-1", + "ics20-1", + false, + false, + }, + } + + for _, tc := range testCases { + ctx := suite.chainA.GetContext() + cap, err := suite.chainA.GetSimApp().ScopedIBCKeeper.NewCapability(ctx, tc.name) + suite.Require().NoError(err) + // claim capability if capability already exists in fee keeper before ChanOpenTry + if tc.capExists { + suite.chainA.GetSimApp().ScopedIBCFeeKeeper.ClaimCapability(ctx, cap, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) + } + + err = suite.module.OnChanOpenTry( + ctx, + channeltypes.UNORDERED, + []string{"connection-1"}, + transfertypes.FeePortID, + "channel-1", + cap, + channeltypes.NewCounterparty(transfertypes.FeePortID, "channel-0"), + tc.version, + tc.cpVersion, + ) + + if tc.expPass { + suite.Require().NoError(err, "unexpected error from version: %s", tc.version) + + // check that capabilities are properly claimed and issued + ctx := suite.chainA.GetContext() + ibcCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) + suite.Require().NotNil(ibcCap, "IBC capability is nil on fee keeper") + suite.Require().True(ok) + + appFeeCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, types.AppCapabilityName(transfertypes.FeePortID, "channel-1")) + suite.Require().NotNil(appFeeCap, "App capability not created or owned by ibc fee keeper") + suite.Require().True(ok) + transferCap, ok := suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) + suite.Require().NotNil(transferCap, "App capability not claimed by transfer keeper") + suite.Require().True(ok) + suite.Require().Equal(appFeeCap, transferCap, "app capabilities not equal") + } else { + suite.Require().Error(err, "error not returned for version: %s", tc.version) + } + } +} + +func (suite *FeeTestSuite) TestOnChanOpenAck() { + testCases := []struct { + name string + cpVersion string + expPass bool + }{ + { + "success", + "fee29-1:ics20-1", + true, + }, + { + "invalid fee version", + "fee29-3:ics20-1", + false, + }, + { + "invalid transfer version", + "fee29-1:ics20-4", + false, + }, + } + + for _, tc := range testCases { + ctx := suite.chainA.GetContext() + err := suite.module.OnChanOpenAck(ctx, transfertypes.FeePortID, "channel-1", tc.cpVersion) + if tc.expPass { + suite.Require().NoError(err, "unexpected error for case: %s", tc.name) + } else { + suite.Require().Error(err, "%s expected error but returned none", tc.name) + } + } +} diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index 6d538733031..08dfbc6bb7f 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -4,7 +4,7 @@ import "fmt" const ( // ModuleName defines the 29-fee name - ModuleName = "ibcfee" + ModuleName = "feeibc" // StoreKey is the store key string for IBC transfer StoreKey = ModuleName @@ -20,6 +20,6 @@ const ( KeyAppCapability = "app_capabilities" ) -func AppCapabilityName(channelID, portID string) string { +func AppCapabilityName(portID, channelID string) string { return fmt.Sprintf("%s/%s/%s", KeyAppCapability, channelID, portID) } diff --git a/modules/apps/transfer/keeper/genesis.go b/modules/apps/transfer/keeper/genesis.go index 7050a2c5512..6ddc2242985 100644 --- a/modules/apps/transfer/keeper/genesis.go +++ b/modules/apps/transfer/keeper/genesis.go @@ -9,7 +9,7 @@ import ( // InitGenesis initializes the ibc-transfer state and binds to PortID. func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) { - k.SetPort(ctx, state.PortId) + k.SetPorts(ctx, state.PortIds) for _, trace := range state.DenomTraces { k.SetDenomTrace(ctx, trace) @@ -17,12 +17,14 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) { // Only try to bind to port if it is not already bound, since we may already own // port capability from capability InitGenesis - if !k.IsBound(ctx, state.PortId) { - // transfer module binds to the transfer port on InitChain - // and claims the returned capability - err := k.BindPort(ctx, state.PortId) - if err != nil { - panic(fmt.Sprintf("could not claim port capability: %v", err)) + for _, port := range state.PortIds { + if !k.IsBound(ctx, port) { + // transfer module binds to the transfer port on InitChain + // and claims the returned capability + err := k.BindPort(ctx, port) + if err != nil { + panic(fmt.Sprintf("could not claim port capability: %v", err)) + } } } @@ -38,7 +40,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) { // ExportGenesis exports ibc-transfer module's portID and denom trace info into its genesis state. func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { return &types.GenesisState{ - PortId: k.GetPort(ctx), + PortIds: k.GetPorts(ctx), DenomTraces: k.GetAllDenomTraces(ctx), Params: k.GetParams(ctx), } diff --git a/modules/apps/transfer/keeper/genesis_test.go b/modules/apps/transfer/keeper/genesis_test.go index 19e5dfe4a18..aa2d044de22 100644 --- a/modules/apps/transfer/keeper/genesis_test.go +++ b/modules/apps/transfer/keeper/genesis_test.go @@ -30,7 +30,7 @@ func (suite *KeeperTestSuite) TestGenesis() { genesis := suite.chainA.GetSimApp().TransferKeeper.ExportGenesis(suite.chainA.GetContext()) - suite.Require().Equal(types.PortID, genesis.PortId) + suite.Require().Equal([]string{types.PortID, types.FeePortID}, genesis.PortIds) suite.Require().Equal(traces.Sort(), genesis.DenomTraces) suite.Require().NotPanics(func() { diff --git a/modules/apps/transfer/keeper/keeper.go b/modules/apps/transfer/keeper/keeper.go index 08c75a26a0e..b89fb810cf0 100644 --- a/modules/apps/transfer/keeper/keeper.go +++ b/modules/apps/transfer/keeper/keeper.go @@ -1,6 +1,8 @@ package keeper import ( + "strings" + tmbytes "github.com/tendermint/tendermint/libs/bytes" "github.com/tendermint/tendermint/libs/log" @@ -93,16 +95,18 @@ func (k Keeper) BindPort(ctx sdk.Context, portID string) error { return k.ClaimCapability(ctx, cap, host.PortPath(portID)) } -// GetPort returns the portID for the transfer module. Used in ExportGenesis -func (k Keeper) GetPort(ctx sdk.Context) string { +// GetPorts returns the bound portIDs for the transfer module. Used in ExportGenesis +func (k Keeper) GetPorts(ctx sdk.Context) []string { store := ctx.KVStore(k.storeKey) - return string(store.Get(types.PortKey)) + portsStr := string(store.Get(types.PortKey)) + return strings.Split(portsStr, "/") } -// SetPort sets the portID for the transfer module. Used in InitGenesis -func (k Keeper) SetPort(ctx sdk.Context, portID string) { +// SetPorts sets the bound portIDs the transfer module. Used in InitGenesis +func (k Keeper) SetPorts(ctx sdk.Context, portIDs []string) { store := ctx.KVStore(k.storeKey) - store.Set(types.PortKey, []byte(portID)) + portsStr := strings.Join(portIDs, "/") + store.Set(types.PortKey, []byte(portsStr)) } // GetDenomTrace retreives the full identifiers trace and base denomination from the store. diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go index a9a1aa4f875..a8698b97564 100644 --- a/modules/apps/transfer/module.go +++ b/modules/apps/transfer/module.go @@ -190,6 +190,7 @@ func ValidateTransferChannelParams( ctx sdk.Context, keeper keeper.Keeper, order channeltypes.Order, + counterparty channeltypes.Counterparty, portID string, channelID string, version string, @@ -208,9 +209,19 @@ func ValidateTransferChannelParams( } // Require portID is the portID transfer module is bound to - boundPort := keeper.GetPort(ctx) - if boundPort != portID { - return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid port: %s, expected %s", portID, boundPort) + boundPorts := keeper.GetPorts(ctx) + var portOk bool + for _, boundPort := range boundPorts { + if boundPort == portID { + portOk = true + } + } + if !portOk { + return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid port: %s not bound by transfer module", portID) + } + + if portID != counterparty.PortId { + return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "portID: %s do not match counterparty portID: %s", portID, counterparty.PortId) } if version != types.Version { @@ -230,7 +241,7 @@ func (am AppModule) OnChanOpenInit( counterparty channeltypes.Counterparty, version string, ) error { - if err := ValidateTransferChannelParams(ctx, am.keeper, order, portID, channelID, version); err != nil { + if err := ValidateTransferChannelParams(ctx, am.keeper, order, counterparty, portID, channelID, version); err != nil { return err } @@ -254,7 +265,7 @@ func (am AppModule) OnChanOpenTry( version, counterpartyVersion string, ) error { - if err := ValidateTransferChannelParams(ctx, am.keeper, order, portID, channelID, version); err != nil { + if err := ValidateTransferChannelParams(ctx, am.keeper, order, counterparty, portID, channelID, version); err != nil { return err } diff --git a/modules/apps/transfer/module_test.go b/modules/apps/transfer/module_test.go index 63d610de84a..acfe513ab0e 100644 --- a/modules/apps/transfer/module_test.go +++ b/modules/apps/transfer/module_test.go @@ -12,9 +12,10 @@ import ( func (suite *TransferTestSuite) TestOnChanOpenInit() { var ( - channel *channeltypes.Channel - path *ibctesting.Path - chanCap *capabilitytypes.Capability + channel *channeltypes.Channel + path *ibctesting.Path + chanCap *capabilitytypes.Capability + counterparty channeltypes.Counterparty ) testCases := []struct { @@ -26,6 +27,18 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { { "success", func() {}, true, }, + { + "success with fee port", func() { + path.EndpointA.ChannelConfig.PortID = types.FeePortID + counterparty.PortId = types.FeePortID + }, true, + }, + { + "mismatched ports", func() { + path.EndpointA.ChannelConfig.PortID = types.PortID + counterparty.PortId = types.FeePortID + }, false, + }, { "max channels reached", func() { path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1) @@ -63,7 +76,7 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { suite.coordinator.SetupConnections(path) path.EndpointA.ChannelID = ibctesting.FirstChannelID - counterparty := channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + counterparty = channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) channel = &channeltypes.Channel{ State: channeltypes.INIT, Ordering: channeltypes.UNORDERED, @@ -84,7 +97,7 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { tc.malleate() // explicitly change fields in channel and testChannel err = cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), - path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.GetVersion(), + path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, counterparty, channel.GetVersion(), ) if tc.expPass { @@ -102,6 +115,7 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { channel *channeltypes.Channel chanCap *capabilitytypes.Capability path *ibctesting.Path + counterparty channeltypes.Counterparty counterpartyVersion string ) @@ -114,6 +128,18 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { { "success", func() {}, true, }, + { + "success with fee port", func() { + path.EndpointA.ChannelConfig.PortID = types.FeePortID + counterparty.PortId = types.FeePortID + }, true, + }, + { + "mismatched ports", func() { + path.EndpointA.ChannelConfig.PortID = types.PortID + counterparty.PortId = types.FeePortID + }, false, + }, { "max channels reached", func() { path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1) @@ -157,7 +183,7 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { suite.coordinator.SetupConnections(path) path.EndpointA.ChannelID = ibctesting.FirstChannelID - counterparty := channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + counterparty = channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) channel = &channeltypes.Channel{ State: channeltypes.TRYOPEN, Ordering: channeltypes.UNORDERED, @@ -179,7 +205,7 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { tc.malleate() // explicitly change fields in channel and testChannel err = cbs.OnChanOpenTry(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), - path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.GetVersion(), counterpartyVersion, + path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, counterparty, channel.GetVersion(), counterpartyVersion, ) if tc.expPass { diff --git a/modules/apps/transfer/simulation/genesis.go b/modules/apps/transfer/simulation/genesis.go index b300e8919ad..fa3a450a76c 100644 --- a/modules/apps/transfer/simulation/genesis.go +++ b/modules/apps/transfer/simulation/genesis.go @@ -40,7 +40,7 @@ func RandomizedGenState(simState *module.SimulationState) { ) transferGenesis := types.GenesisState{ - PortId: portID, + PortIds: []string{portID}, DenomTraces: types.Traces{}, Params: types.NewParams(sendEnabled, receiveEnabled), } diff --git a/modules/apps/transfer/simulation/genesis_test.go b/modules/apps/transfer/simulation/genesis_test.go index c4bd61035ab..13da29a483d 100644 --- a/modules/apps/transfer/simulation/genesis_test.go +++ b/modules/apps/transfer/simulation/genesis_test.go @@ -39,7 +39,7 @@ func TestRandomizedGenState(t *testing.T) { var ibcTransferGenesis types.GenesisState simState.Cdc.MustUnmarshalJSON(simState.GenState[types.ModuleName], &ibcTransferGenesis) - require.Equal(t, "euzxpfgkqegqiqwixnku", ibcTransferGenesis.PortId) + require.Equal(t, "euzxpfgkqegqiqwixnku", ibcTransferGenesis.PortIds[0]) require.True(t, ibcTransferGenesis.Params.SendEnabled) require.True(t, ibcTransferGenesis.Params.ReceiveEnabled) require.Len(t, ibcTransferGenesis.DenomTraces, 0) diff --git a/modules/apps/transfer/types/genesis.go b/modules/apps/transfer/types/genesis.go index 1a17bc474ce..01dcb257e62 100644 --- a/modules/apps/transfer/types/genesis.go +++ b/modules/apps/transfer/types/genesis.go @@ -5,9 +5,9 @@ import ( ) // NewGenesisState creates a new ibc-transfer GenesisState instance. -func NewGenesisState(portID string, denomTraces Traces, params Params) *GenesisState { +func NewGenesisState(portIDs []string, denomTraces Traces, params Params) *GenesisState { return &GenesisState{ - PortId: portID, + PortIds: portIDs, DenomTraces: denomTraces, Params: params, } @@ -16,7 +16,7 @@ func NewGenesisState(portID string, denomTraces Traces, params Params) *GenesisS // DefaultGenesisState returns a GenesisState with "transfer" as the default PortID. func DefaultGenesisState() *GenesisState { return &GenesisState{ - PortId: PortID, + PortIds: []string{PortID, FeePortID}, DenomTraces: Traces{}, Params: DefaultParams(), } @@ -25,8 +25,10 @@ func DefaultGenesisState() *GenesisState { // Validate performs basic genesis state validation returning an error upon any // failure. func (gs GenesisState) Validate() error { - if err := host.PortIdentifierValidator(gs.PortId); err != nil { - return err + for _, port := range gs.PortIds { + if err := host.PortIdentifierValidator(port); err != nil { + return err + } } if err := gs.DenomTraces.Validate(); err != nil { return err diff --git a/modules/apps/transfer/types/genesis.pb.go b/modules/apps/transfer/types/genesis.pb.go index 94eb0108c5d..1a6c57395e3 100644 --- a/modules/apps/transfer/types/genesis.pb.go +++ b/modules/apps/transfer/types/genesis.pb.go @@ -25,9 +25,9 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // GenesisState defines the ibc-transfer genesis state type GenesisState struct { - PortId string `protobuf:"bytes,1,opt,name=port_id,json=portId,proto3" json:"port_id,omitempty" yaml:"port_id"` - DenomTraces Traces `protobuf:"bytes,2,rep,name=denom_traces,json=denomTraces,proto3,castrepeated=Traces" json:"denom_traces" yaml:"denom_traces"` - Params Params `protobuf:"bytes,3,opt,name=params,proto3" json:"params"` + PortIds []string `protobuf:"bytes,1,rep,name=port_ids,json=portIds,proto3" json:"port_ids,omitempty" yaml:"port_ids"` + DenomTraces Traces `protobuf:"bytes,2,rep,name=denom_traces,json=denomTraces,proto3,castrepeated=Traces" json:"denom_traces" yaml:"denom_traces"` + Params Params `protobuf:"bytes,3,opt,name=params,proto3" json:"params"` } func (m *GenesisState) Reset() { *m = GenesisState{} } @@ -63,11 +63,11 @@ func (m *GenesisState) XXX_DiscardUnknown() { var xxx_messageInfo_GenesisState proto.InternalMessageInfo -func (m *GenesisState) GetPortId() string { +func (m *GenesisState) GetPortIds() []string { if m != nil { - return m.PortId + return m.PortIds } - return "" + return nil } func (m *GenesisState) GetDenomTraces() Traces { @@ -94,26 +94,26 @@ func init() { var fileDescriptor_a4f788affd5bea89 = []byte{ // 323 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x90, 0xc1, 0x4a, 0xf3, 0x40, - 0x14, 0x85, 0x33, 0x7f, 0x7f, 0x22, 0xa6, 0xc5, 0x45, 0x74, 0x51, 0x8a, 0x24, 0x25, 0x28, 0x04, - 0x8b, 0x33, 0xb4, 0xba, 0x72, 0x19, 0x04, 0x71, 0x23, 0x52, 0x5d, 0xb9, 0x29, 0x93, 0xc9, 0x18, - 0x07, 0x9a, 0xdc, 0x30, 0x77, 0x5a, 0xe8, 0x5b, 0xf8, 0x1c, 0x3e, 0x49, 0x97, 0x5d, 0xba, 0xaa, - 0xd2, 0xbe, 0x41, 0x7d, 0x01, 0x49, 0x5a, 0x4b, 0x57, 0xdd, 0x1d, 0x66, 0xbe, 0x73, 0xce, 0xe5, - 0x38, 0x17, 0x2a, 0x16, 0x8c, 0x17, 0xc5, 0x50, 0x09, 0x6e, 0x14, 0xe4, 0xc8, 0x8c, 0xe6, 0x39, - 0xbe, 0x4a, 0xcd, 0xc6, 0x5d, 0x96, 0xca, 0x5c, 0xa2, 0x42, 0x5a, 0x68, 0x30, 0xe0, 0x9e, 0xaa, - 0x58, 0xd0, 0x5d, 0x96, 0xfe, 0xb1, 0x74, 0xdc, 0x6d, 0x75, 0xf6, 0x26, 0x6d, 0xc9, 0x2a, 0xaa, - 0x75, 0x92, 0x42, 0x0a, 0x95, 0x64, 0xa5, 0x5a, 0xbf, 0x06, 0x3f, 0xc4, 0x69, 0xdc, 0xad, 0x2b, - 0x9f, 0x0c, 0x37, 0xd2, 0xed, 0x38, 0x07, 0x05, 0x68, 0x33, 0x50, 0x49, 0x93, 0xb4, 0x49, 0x78, - 0x18, 0xb9, 0xab, 0xb9, 0x7f, 0x34, 0xe1, 0xd9, 0xf0, 0x26, 0xd8, 0x7c, 0x04, 0x7d, 0xbb, 0x54, - 0xf7, 0x89, 0xab, 0x9d, 0x46, 0x22, 0x73, 0xc8, 0x06, 0x46, 0x73, 0x21, 0xb1, 0xf9, 0xaf, 0x5d, - 0x0b, 0xeb, 0xbd, 0x90, 0xee, 0xbb, 0x9a, 0xde, 0x96, 0x8e, 0xe7, 0xd2, 0x10, 0x9d, 0x4f, 0xe7, - 0xbe, 0xb5, 0x9a, 0xfb, 0xc7, 0xeb, 0xfc, 0xdd, 0xac, 0xe0, 0xe3, 0xcb, 0xb7, 0x2b, 0x0a, 0xfb, - 0xf5, 0x64, 0x6b, 0x41, 0x37, 0x72, 0xec, 0x82, 0x6b, 0x9e, 0x61, 0xb3, 0xd6, 0x26, 0x61, 0xbd, - 0x77, 0xb6, 0xbf, 0xed, 0xb1, 0x62, 0xa3, 0xff, 0x65, 0x53, 0x7f, 0xe3, 0x8c, 0x1e, 0xa6, 0x0b, - 0x8f, 0xcc, 0x16, 0x1e, 0xf9, 0x5e, 0x78, 0xe4, 0x7d, 0xe9, 0x59, 0xb3, 0xa5, 0x67, 0x7d, 0x2e, - 0x3d, 0xeb, 0xe5, 0x3a, 0x55, 0xe6, 0x6d, 0x14, 0x53, 0x01, 0x19, 0x13, 0x80, 0x19, 0x20, 0x53, - 0xb1, 0xb8, 0x4c, 0x81, 0x65, 0x90, 0x8c, 0x86, 0x12, 0xcb, 0xbd, 0x77, 0x76, 0x36, 0x93, 0x42, - 0x62, 0x6c, 0x57, 0x63, 0x5e, 0xfd, 0x06, 0x00, 0x00, 0xff, 0xff, 0xb1, 0x5d, 0xce, 0xa9, 0xdb, + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x90, 0x31, 0x4b, 0xc3, 0x40, + 0x1c, 0xc5, 0x13, 0x2b, 0x55, 0xd3, 0x82, 0x90, 0x3a, 0x94, 0x22, 0x49, 0x09, 0x0a, 0x41, 0xf1, + 0x8e, 0x56, 0x27, 0xc7, 0x20, 0x88, 0x8b, 0x48, 0x75, 0x72, 0x29, 0x97, 0xcb, 0x19, 0x0f, 0x9a, + 0xfc, 0xc3, 0xfd, 0xaf, 0x85, 0x7e, 0x0b, 0x3f, 0x87, 0x9f, 0xa4, 0x63, 0x47, 0xa7, 0x2a, 0xed, + 0x37, 0xe8, 0xe0, 0x2c, 0x97, 0xda, 0xd2, 0xa9, 0xdb, 0xe3, 0xee, 0xf7, 0xde, 0xfb, 0xf3, 0x9c, + 0x0b, 0x19, 0x73, 0xca, 0x8a, 0x62, 0x20, 0x39, 0xd3, 0x12, 0x72, 0xa4, 0x5a, 0xb1, 0x1c, 0xdf, + 0x84, 0xa2, 0xa3, 0x0e, 0x4d, 0x45, 0x2e, 0x50, 0x22, 0x29, 0x14, 0x68, 0x70, 0x4f, 0x65, 0xcc, + 0xc9, 0x36, 0x4b, 0xd6, 0x2c, 0x19, 0x75, 0x5a, 0x97, 0x3b, 0x93, 0x36, 0x64, 0x19, 0xd5, 0x3a, + 0x49, 0x21, 0x85, 0x52, 0x52, 0xa3, 0x56, 0xaf, 0xc1, 0xaf, 0xed, 0xd4, 0xef, 0x57, 0x95, 0xcf, + 0x9a, 0x69, 0xe1, 0x12, 0xe7, 0xb0, 0x00, 0xa5, 0xfb, 0x32, 0xc1, 0xa6, 0xdd, 0xae, 0x84, 0x47, + 0x51, 0x63, 0x39, 0xf3, 0x8f, 0xc7, 0x2c, 0x1b, 0xdc, 0x06, 0xeb, 0x9f, 0xa0, 0x77, 0x60, 0xe4, + 0x43, 0x82, 0xae, 0x72, 0xea, 0x89, 0xc8, 0x21, 0xeb, 0x6b, 0xc5, 0xb8, 0xc0, 0xe6, 0x5e, 0xbb, + 0x12, 0xd6, 0xba, 0x21, 0xd9, 0x75, 0x38, 0xb9, 0x33, 0x8e, 0x17, 0x63, 0x88, 0xce, 0x27, 0x33, + 0xdf, 0x5a, 0xce, 0xfc, 0xc6, 0xaa, 0x61, 0x3b, 0x2b, 0xf8, 0xfc, 0xf6, 0xab, 0x25, 0x85, 0xbd, + 0x5a, 0xb2, 0xb1, 0xa0, 0x1b, 0x39, 0xd5, 0x82, 0x29, 0x96, 0x61, 0xb3, 0xd2, 0xb6, 0xc3, 0x5a, + 0xf7, 0x6c, 0x77, 0xdb, 0x53, 0xc9, 0x46, 0xfb, 0xa6, 0xa9, 0xf7, 0xef, 0x8c, 0x1e, 0x27, 0x73, + 0xcf, 0x9e, 0xce, 0x3d, 0xfb, 0x67, 0xee, 0xd9, 0x1f, 0x0b, 0xcf, 0x9a, 0x2e, 0x3c, 0xeb, 0x6b, + 0xe1, 0x59, 0xaf, 0x37, 0xa9, 0xd4, 0xef, 0xc3, 0x98, 0x70, 0xc8, 0x28, 0x07, 0xcc, 0x00, 0xa9, + 0x8c, 0xf9, 0x55, 0x0a, 0x34, 0x83, 0x64, 0x38, 0x10, 0x68, 0x26, 0xdf, 0x9a, 0x5a, 0x8f, 0x0b, + 0x81, 0x71, 0xb5, 0xdc, 0xf3, 0xfa, 0x2f, 0x00, 0x00, 0xff, 0xff, 0x61, 0x40, 0x1b, 0xdb, 0xde, 0x01, 0x00, 0x00, } @@ -161,12 +161,14 @@ func (m *GenesisState) MarshalToSizedBuffer(dAtA []byte) (int, error) { dAtA[i] = 0x12 } } - if len(m.PortId) > 0 { - i -= len(m.PortId) - copy(dAtA[i:], m.PortId) - i = encodeVarintGenesis(dAtA, i, uint64(len(m.PortId))) - i-- - dAtA[i] = 0xa + if len(m.PortIds) > 0 { + for iNdEx := len(m.PortIds) - 1; iNdEx >= 0; iNdEx-- { + i -= len(m.PortIds[iNdEx]) + copy(dAtA[i:], m.PortIds[iNdEx]) + i = encodeVarintGenesis(dAtA, i, uint64(len(m.PortIds[iNdEx]))) + i-- + dAtA[i] = 0xa + } } return len(dAtA) - i, nil } @@ -188,9 +190,11 @@ func (m *GenesisState) Size() (n int) { } var l int _ = l - l = len(m.PortId) - if l > 0 { - n += 1 + l + sovGenesis(uint64(l)) + if len(m.PortIds) > 0 { + for _, s := range m.PortIds { + l = len(s) + n += 1 + l + sovGenesis(uint64(l)) + } } if len(m.DenomTraces) > 0 { for _, e := range m.DenomTraces { @@ -240,7 +244,7 @@ func (m *GenesisState) Unmarshal(dAtA []byte) error { switch fieldNum { case 1: if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field PortId", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field PortIds", wireType) } var stringLen uint64 for shift := uint(0); ; shift += 7 { @@ -268,7 +272,7 @@ func (m *GenesisState) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - m.PortId = string(dAtA[iNdEx:postIndex]) + m.PortIds = append(m.PortIds, string(dAtA[iNdEx:postIndex])) iNdEx = postIndex case 2: if wireType != 2 { diff --git a/modules/apps/transfer/types/genesis_test.go b/modules/apps/transfer/types/genesis_test.go index 23305ae1d07..02e9c8b1d8e 100644 --- a/modules/apps/transfer/types/genesis_test.go +++ b/modules/apps/transfer/types/genesis_test.go @@ -22,14 +22,14 @@ func TestValidateGenesis(t *testing.T) { { "valid genesis", &types.GenesisState{ - PortId: "portidone", + PortIds: []string{"portidone", "portidtwo", "portidthree"}, }, true, }, { "invalid client", &types.GenesisState{ - PortId: "(INVALIDPORT)", + PortIds: []string{"(INVALIDPORT)"}, }, false, }, diff --git a/modules/apps/transfer/types/keys.go b/modules/apps/transfer/types/keys.go index c156af3fd88..f8e780c4339 100644 --- a/modules/apps/transfer/types/keys.go +++ b/modules/apps/transfer/types/keys.go @@ -18,6 +18,9 @@ const ( // PortID is the default port id that transfer module binds to PortID = "transfer" + // FeePortID is the port id that is wrapped by fee middleware + FeePortID = "feetransfer" + // StoreKey is the store key string for IBC transfer StoreKey = ModuleName diff --git a/proto/ibc/applications/transfer/v1/genesis.proto b/proto/ibc/applications/transfer/v1/genesis.proto index 9c6b78ac7b1..1c27652085c 100644 --- a/proto/ibc/applications/transfer/v1/genesis.proto +++ b/proto/ibc/applications/transfer/v1/genesis.proto @@ -9,7 +9,7 @@ import "gogoproto/gogo.proto"; // GenesisState defines the ibc-transfer genesis state message GenesisState { - string port_id = 1 [(gogoproto.moretags) = "yaml:\"port_id\""]; + repeated string port_ids = 1 [(gogoproto.moretags) = "yaml:\"port_ids\""]; repeated DenomTrace denom_traces = 2 [ (gogoproto.castrepeated) = "Traces", (gogoproto.nullable) = false, diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 817996c4c4c..1ea25c58ba6 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -81,6 +81,9 @@ import ( upgradeclient "github.com/cosmos/cosmos-sdk/x/upgrade/client" upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" + ibcfee "github.com/cosmos/ibc-go/modules/apps/29-fee" + ibcfeekeeper "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" + ibcfeetypes "github.com/cosmos/ibc-go/modules/apps/29-fee/types" transfer "github.com/cosmos/ibc-go/modules/apps/transfer" ibctransferkeeper "github.com/cosmos/ibc-go/modules/apps/transfer/keeper" ibctransfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" @@ -133,6 +136,7 @@ var ( ibcmock.AppModuleBasic{}, authzmodule.AppModuleBasic{}, vesting.AppModuleBasic{}, + ibcfee.AppModuleBasic{}, ) // module account permissions @@ -185,11 +189,13 @@ type SimApp struct { EvidenceKeeper evidencekeeper.Keeper TransferKeeper ibctransferkeeper.Keeper FeeGrantKeeper feegrantkeeper.Keeper + IBCFeeKeeper ibcfeekeeper.Keeper // make scoped keepers public for test purposes ScopedIBCKeeper capabilitykeeper.ScopedKeeper ScopedTransferKeeper capabilitykeeper.ScopedKeeper ScopedIBCMockKeeper capabilitykeeper.ScopedKeeper + ScopedIBCFeeKeeper capabilitykeeper.ScopedKeeper // the module manager mm *module.Manager @@ -231,7 +237,7 @@ func NewSimApp( minttypes.StoreKey, distrtypes.StoreKey, slashingtypes.StoreKey, govtypes.StoreKey, paramstypes.StoreKey, ibchost.StoreKey, upgradetypes.StoreKey, feegrant.StoreKey, evidencetypes.StoreKey, ibctransfertypes.StoreKey, capabilitytypes.StoreKey, - authzkeeper.StoreKey, + authzkeeper.StoreKey, ibcfeetypes.StoreKey, ) tkeys := sdk.NewTransientStoreKeys(paramstypes.TStoreKey) memKeys := sdk.NewMemoryStoreKeys(capabilitytypes.MemStoreKey) @@ -259,6 +265,7 @@ func NewSimApp( // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // note replicate if you do not need to test core IBC or light clients. scopedIBCMockKeeper := app.CapabilityKeeper.ScopeToModule(ibcmock.ModuleName) + app.ScopedIBCFeeKeeper = app.CapabilityKeeper.ScopeToModule(ibcfeetypes.ModuleName) // add keepers app.AccountKeeper = authkeeper.NewAccountKeeper( @@ -321,6 +328,12 @@ func NewSimApp( ) transferModule := transfer.NewAppModule(app.TransferKeeper) + app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), + app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, + app.AccountKeeper, app.BankKeeper, app.ScopedIBCFeeKeeper, + ) + feeTransferModule := ibcfee.NewAppModule(app.IBCFeeKeeper, app.ScopedIBCFeeKeeper, transferModule) + // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // note replicate if you do not need to test core IBC or light clients. mockModule := ibcmock.NewAppModule(scopedIBCMockKeeper, &app.IBCKeeper.PortKeeper) @@ -368,6 +381,7 @@ func NewSimApp( params.NewAppModule(app.ParamsKeeper), authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry), transferModule, + feeTransferModule, mockModule, ) From b8f2a7a04fdf368dbde06c3d5acd087d20954eb0 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 2 Sep 2021 11:27:50 +0200 Subject: [PATCH 04/21] remove prints --- modules/apps/29-fee/module_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/modules/apps/29-fee/module_test.go b/modules/apps/29-fee/module_test.go index 372dc5d9e93..870ad189621 100644 --- a/modules/apps/29-fee/module_test.go +++ b/modules/apps/29-fee/module_test.go @@ -1,8 +1,6 @@ package fee_test import ( - "fmt" - "github.com/cosmos/ibc-go/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" @@ -54,8 +52,6 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { for _, tc := range testCases { ctx := suite.chainA.GetContext() - fmt.Println(ctx) - fmt.Println(tc.name) cap, err := suite.chainA.GetSimApp().ScopedIBCKeeper.NewCapability(ctx, tc.name) suite.Require().NoError(err) err = suite.module.OnChanOpenInit( From 1f62964e10cdce6ad6a9b5cfb7ac488e18a02dc2 Mon Sep 17 00:00:00 2001 From: Aditya Date: Fri, 3 Sep 2021 13:08:33 +0200 Subject: [PATCH 05/21] Update modules/apps/29-fee/module.go Co-authored-by: Sean King --- modules/apps/29-fee/module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index 09dc17336fd..a6840c6b476 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -236,7 +236,7 @@ func (am AppModule) OnChanOpenTry( cpFeeVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) if feeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected version: %s, got: %s", types.Version, feeVersion) } if cpFeeVersion != feeVersion { return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) From 47117740ad9c4a4a778c18121e5d2ee1c311a570 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 10 Sep 2021 13:04:28 +0200 Subject: [PATCH 06/21] debugging progress --- modules/apps/29-fee/fee_test.go | 16 ++- modules/apps/29-fee/module.go | 10 +- modules/apps/29-fee/module_test.go | 140 ++++++++++---------- modules/apps/transfer/keeper/keeper_test.go | 7 + modules/apps/transfer/module.go | 1 + modules/core/keeper/msg_server.go | 4 + testing/simapp/app.go | 1 + 7 files changed, 105 insertions(+), 74 deletions(-) diff --git a/modules/apps/29-fee/fee_test.go b/modules/apps/29-fee/fee_test.go index 47644661071..be702977e7c 100644 --- a/modules/apps/29-fee/fee_test.go +++ b/modules/apps/29-fee/fee_test.go @@ -4,8 +4,8 @@ import ( "testing" fee "github.com/cosmos/ibc-go/modules/apps/29-fee" - feekeeper "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" "github.com/cosmos/ibc-go/modules/apps/transfer" + transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" ibctesting "github.com/cosmos/ibc-go/testing" "github.com/stretchr/testify/suite" ) @@ -18,8 +18,9 @@ type FeeTestSuite struct { chainA *ibctesting.TestChain chainB *ibctesting.TestChain - module fee.AppModule - keeper feekeeper.Keeper + path *ibctesting.Path + + moduleA fee.AppModule } func (suite *FeeTestSuite) SetupTest() { @@ -27,10 +28,15 @@ func (suite *FeeTestSuite) SetupTest() { suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(0)) suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1)) - suite.keeper = suite.chainA.GetSimApp().IBCFeeKeeper + keeper := suite.chainA.GetSimApp().IBCFeeKeeper transferModule := transfer.NewAppModule(suite.chainA.GetSimApp().TransferKeeper) - suite.module = fee.NewAppModule(suite.keeper, suite.chainA.GetSimApp().ScopedIBCFeeKeeper, transferModule) + suite.moduleA = fee.NewAppModule(keeper, suite.chainA.GetSimApp().ScopedIBCFeeKeeper, transferModule) + + path := ibctesting.NewPath(suite.chainA, suite.chainB) + path.EndpointA.ChannelConfig.PortID = transfertypes.FeePortID + path.EndpointB.ChannelConfig.PortID = transfertypes.FeePortID + suite.path = path } func TestIBCFeeTestSuite(t *testing.T) { diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index 09dc17336fd..81a6068a947 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -3,6 +3,7 @@ package fee import ( "context" "encoding/json" + "fmt" "math/rand" "github.com/cosmos/cosmos-sdk/client" @@ -232,6 +233,7 @@ func (am AppModule) OnChanOpenTry( version, counterpartyVersion string, ) error { + fmt.Println("FEE TRANSFER") feeVersion, appVersion := channeltypes.SplitChannelVersion(version) cpFeeVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) @@ -246,20 +248,24 @@ func (am AppModule) OnChanOpenTry( err error ok bool ) + fmt.Println("why") + fmt.Println(host.ChannelCapabilityPath(portID, channelID)) // Module may have already claimed capability in OnChanOpenInit in the case of crossing hellos // (ie chainA and chainB both call ChanOpenInit before one of them calls ChanOpenTry) // If module can already authenticate the capability then module already owns it so we don't need to claim // Otherwise, module does not have channel capability and we must claim it from IBC if !am.scopedKeeper.AuthenticateCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)) { + fmt.Println("hello") // Only claim channel capability passed back by IBC module if we do not already own it if err := am.scopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { - return err + return sdkerrors.Wrap(err, "fee middleware could not claim capability from caller") } appCap, err = am.scopedKeeper.NewCapability(ctx, types.AppCapabilityName(portID, channelID)) if err != nil { - return sdkerrors.Wrap(err, "could not create capability for underlying app") + return sdkerrors.Wrap(err, "could not create capability for underlying application") } } else { + fmt.Println("bye") appCap, ok = am.scopedKeeper.GetCapability(ctx, types.AppCapabilityName(portID, channelID)) if !ok { return sdkerrors.Wrap(capabilitytypes.ErrCapabilityNotFound, diff --git a/modules/apps/29-fee/module_test.go b/modules/apps/29-fee/module_test.go index 870ad189621..bcbc74e5d87 100644 --- a/modules/apps/29-fee/module_test.go +++ b/modules/apps/29-fee/module_test.go @@ -1,6 +1,8 @@ package fee_test import ( + "fmt" + "github.com/cosmos/ibc-go/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" @@ -51,39 +53,46 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { } for _, tc := range testCases { - ctx := suite.chainA.GetContext() - cap, err := suite.chainA.GetSimApp().ScopedIBCKeeper.NewCapability(ctx, tc.name) - suite.Require().NoError(err) - err = suite.module.OnChanOpenInit( - ctx, - channeltypes.UNORDERED, - []string{"connection-1"}, - transfertypes.FeePortID, - "channel-1", - cap, - channeltypes.NewCounterparty(transfertypes.FeePortID, "channel-2"), - tc.version, - ) + tc := tc - if tc.expPass { - suite.Require().NoError(err, "unexpected error from version: %s", tc.version) + suite.Run(tc.name, func() { + // reset suite + suite.SetupTest() - // check that capabilities are properly claimed and issued ctx := suite.chainA.GetContext() - ibcCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) - suite.Require().NotNil(ibcCap, "IBC capability is nil on fee keeper") - suite.Require().True(ok) - - appFeeCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, types.AppCapabilityName(transfertypes.FeePortID, "channel-1")) - suite.Require().NotNil(appFeeCap, "App capability not created or owned by ibc fee keeper") - suite.Require().True(ok) - transferCap, ok := suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) - suite.Require().NotNil(transferCap, "App capability not claimed by transfer keeper") - suite.Require().True(ok) - suite.Require().Equal(appFeeCap, transferCap, "app capabilities not equal") - } else { - suite.Require().Error(err, "error not returned for version: %s", tc.version) - } + cap, err := suite.chainA.GetSimApp().ScopedIBCKeeper.NewCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) + suite.Require().NoError(err) + err = suite.moduleA.OnChanOpenInit( + ctx, + channeltypes.UNORDERED, + []string{"connection-1"}, + transfertypes.FeePortID, + "channel-1", + cap, + channeltypes.NewCounterparty(transfertypes.FeePortID, ""), + tc.version, + ) + + if tc.expPass { + suite.Require().NoError(err, "unexpected error from version: %s", tc.version) + + // check that capabilities are properly claimed and issued + ctx := suite.chainA.GetContext() + ibcCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) + suite.Require().NotNil(ibcCap, "IBC capability is nil on fee keeper") + suite.Require().True(ok) + + appFeeCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, types.AppCapabilityName(transfertypes.FeePortID, "channel-1")) + suite.Require().NotNil(appFeeCap, "App capability not created or owned by ibc fee keeper") + suite.Require().True(ok) + transferCap, ok := suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) + suite.Require().NotNil(transferCap, "App capability not claimed by transfer keeper") + suite.Require().True(ok) + suite.Require().Equal(appFeeCap, transferCap, "app capabilities not equal") + } else { + suite.Require().Error(err, "error not returned for version: %s", tc.version) + } + }) } } @@ -168,45 +177,42 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { } for _, tc := range testCases { - ctx := suite.chainA.GetContext() - cap, err := suite.chainA.GetSimApp().ScopedIBCKeeper.NewCapability(ctx, tc.name) - suite.Require().NoError(err) - // claim capability if capability already exists in fee keeper before ChanOpenTry - if tc.capExists { - suite.chainA.GetSimApp().ScopedIBCFeeKeeper.ClaimCapability(ctx, cap, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) - } + tc := tc - err = suite.module.OnChanOpenTry( - ctx, - channeltypes.UNORDERED, - []string{"connection-1"}, - transfertypes.FeePortID, - "channel-1", - cap, - channeltypes.NewCounterparty(transfertypes.FeePortID, "channel-0"), - tc.version, - tc.cpVersion, - ) + suite.Run(tc.name, func() { + // reset suite + suite.SetupTest() + suite.coordinator.SetupClients(suite.path) + suite.coordinator.SetupConnections(suite.path) + suite.path.EndpointB.ChanOpenInit() - if tc.expPass { - suite.Require().NoError(err, "unexpected error from version: %s", tc.version) + if tc.capExists { + suite.path.EndpointA.ChanOpenInit() + } - // check that capabilities are properly claimed and issued - ctx := suite.chainA.GetContext() - ibcCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) - suite.Require().NotNil(ibcCap, "IBC capability is nil on fee keeper") - suite.Require().True(ok) - - appFeeCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, types.AppCapabilityName(transfertypes.FeePortID, "channel-1")) - suite.Require().NotNil(appFeeCap, "App capability not created or owned by ibc fee keeper") - suite.Require().True(ok) - transferCap, ok := suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) - suite.Require().NotNil(transferCap, "App capability not claimed by transfer keeper") - suite.Require().True(ok) - suite.Require().Equal(appFeeCap, transferCap, "app capabilities not equal") - } else { - suite.Require().Error(err, "error not returned for version: %s", tc.version) - } + fmt.Println("port:", suite.path.EndpointA.ChannelConfig.PortID) + err := suite.path.EndpointA.ChanOpenTry() + + if tc.expPass { + suite.Require().NoError(err, "unexpected error from version: %s", tc.version) + + // check that capabilities are properly claimed and issued + ctx := suite.chainA.GetContext() + ibcCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, suite.path.EndpointA.ChannelID)) + suite.Require().NotNil(ibcCap, "IBC capability is nil on fee keeper: %s", host.ChannelCapabilityPath(transfertypes.FeePortID, suite.path.EndpointA.ChannelID)) + suite.Require().True(ok) + + appFeeCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, types.AppCapabilityName(transfertypes.FeePortID, suite.path.EndpointA.ChannelID)) + suite.Require().NotNil(appFeeCap, "App capability not created or owned by ibc fee keeper") + suite.Require().True(ok) + transferCap, ok := suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, suite.path.EndpointA.ChannelID)) + suite.Require().NotNil(transferCap, "App capability not claimed by transfer keeper") + suite.Require().True(ok) + suite.Require().Equal(appFeeCap, transferCap, "app capabilities not equal") + } else { + suite.Require().Error(err, "error not returned for version: %s", tc.version) + } + }) } } @@ -235,7 +241,7 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { for _, tc := range testCases { ctx := suite.chainA.GetContext() - err := suite.module.OnChanOpenAck(ctx, transfertypes.FeePortID, "channel-1", tc.cpVersion) + err := suite.moduleA.OnChanOpenAck(ctx, transfertypes.FeePortID, "channel-1", tc.cpVersion) if tc.expPass { suite.Require().NoError(err, "unexpected error for case: %s", tc.name) } else { diff --git a/modules/apps/transfer/keeper/keeper_test.go b/modules/apps/transfer/keeper/keeper_test.go index de3902df669..60f52a88cb5 100644 --- a/modules/apps/transfer/keeper/keeper_test.go +++ b/modules/apps/transfer/keeper/keeper_test.go @@ -54,6 +54,13 @@ func (suite *KeeperTestSuite) TestGetTransferAccount() { suite.Require().Equal(expectedMaccAddr, macc.GetAddress()) } +func (suite *KeeperTestSuite) TestGetPorts() { + expectedPorts := []string{"transfer", "feetransfer", "testport"} + suite.chainA.GetSimApp().TransferKeeper.SetPorts(suite.chainA.GetContext(), expectedPorts) + gotPorts := suite.chainA.GetSimApp().TransferKeeper.GetPorts(suite.chainA.GetContext()) + suite.Require().Equal(expectedPorts, gotPorts, "ports set in store is not the same as ports retrieved from store") +} + func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go index a8698b97564..d9a351c977b 100644 --- a/modules/apps/transfer/module.go +++ b/modules/apps/transfer/module.go @@ -214,6 +214,7 @@ func ValidateTransferChannelParams( for _, boundPort := range boundPorts { if boundPort == portID { portOk = true + break } } if !portOk { diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 69bde98b687..9d3b30f238f 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" "github.com/armon/go-metrics" @@ -301,6 +302,8 @@ func (k Keeper) ChannelOpenTry(goCtx context.Context, msg *channeltypes.MsgChann if err != nil { return nil, sdkerrors.Wrap(err, "could not retrieve module from port-id") } + fmt.Println("PORTID: ", msg.PortId) + fmt.Println("MODULE: ", module) channelID, cap, err := k.ChannelKeeper.ChanOpenTry(ctx, msg.Channel.Ordering, msg.Channel.ConnectionHops, msg.PortId, msg.PreviousChannelId, portCap, msg.Channel.Counterparty, msg.Channel.Version, msg.CounterpartyVersion, msg.ProofInit, msg.ProofHeight, @@ -484,6 +487,7 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke if err != nil { return nil, sdkerrors.Wrap(err, "could not retrieve module from port-id") } + fmt.Println("MODULE: ", module) // Retrieve callbacks from router cbs, ok := k.Router.GetRoute(module) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 1ea25c58ba6..f37fb412a75 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -342,6 +342,7 @@ func NewSimApp( ibcRouter := porttypes.NewRouter() ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferModule) ibcRouter.AddRoute(ibcmock.ModuleName, mockModule) + ibcRouter.AddRoute(ibcfeetypes.ModuleName, feeTransferModule) app.IBCKeeper.SetRouter(ibcRouter) // create evidence keeper with router From dac74bc58c30d3ad0541dc5794525da51f9e41df Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 16 Sep 2021 11:23:03 +0200 Subject: [PATCH 07/21] fee enabled flag --- modules/apps/29-fee/keeper/keeper.go | 28 +++++++++++++++++++++------- modules/apps/29-fee/types/keys.go | 4 ++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index c57b81ee10e..764862bb6e6 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -7,7 +7,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" - "github.com/cosmos/ibc-go/modules/apps/transfer/types" + "github.com/cosmos/ibc-go/modules/apps/29-fee/types" host "github.com/cosmos/ibc-go/modules/core/24-host" ) @@ -59,14 +59,28 @@ func (k Keeper) BindPort(ctx sdk.Context, portID string) error { return k.scopedKeeper.ClaimCapability(ctx, cap, host.PortPath(portID)) } -// GetPort returns the portID for the transfer module. Used in ExportGenesis -func (k Keeper) GetPort(ctx sdk.Context) string { +// // GetPort returns the portID for the transfer module. Used in ExportGenesis +// func (k Keeper) GetPort(ctx sdk.Context) string { +// store := ctx.KVStore(k.storeKey) +// return string(store.Get(types.PortKey)) +// } + +// // SetPort sets the portID for the transfer module. Used in InitGenesis +// func (k Keeper) SetPort(ctx sdk.Context, portID string) { +// store := ctx.KVStore(k.storeKey) +// store.Set(types.PortKey, []byte(portID)) +// } + +// SetFeeEnabled sets a flag to determine if fee handling logic should run for the given channel +// identified by channel and port identifiers. +func (k Keeper) SetFeeEnabled(ctx sdk.Context, portID, channelID string) { store := ctx.KVStore(k.storeKey) - return string(store.Get(types.PortKey)) + store.Set(types.FeeEnabledKey(portID, channelID), []byte{1}) } -// SetPort sets the portID for the transfer module. Used in InitGenesis -func (k Keeper) SetPort(ctx sdk.Context, portID string) { +// IsFeeEnabled returns whether fee handling logic should be run for the given port by checking the +// fee enabled flag for the given port and channel identifiers +func (k Keeper) IsFeeEnabled(ctx sdk.Context, portID, channelID string) bool { store := ctx.KVStore(k.storeKey) - store.Set(types.PortKey, []byte(portID)) + return store.Get(types.FeeEnabledKey(portID, channelID)) != nil } diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index 08dfbc6bb7f..01d9ada3a2d 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -23,3 +23,7 @@ const ( func AppCapabilityName(portID, channelID string) string { return fmt.Sprintf("%s/%s/%s", KeyAppCapability, channelID, portID) } + +func FeeEnabledKey(portID, channelID string) []byte { + return []byte(fmt.Sprintf("fee_enabled/%s/%s", portID, channelID)) +} From d190a19d6830d07c46a4739925e1d9aeb43b08eb Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 20 Sep 2021 11:30:30 +0200 Subject: [PATCH 08/21] cleanup handshake logic --- docs/ibc/proto-docs.md | 2 +- modules/apps/29-fee/fee_test.go | 15 +- modules/apps/29-fee/keeper/keeper.go | 39 +++-- modules/apps/29-fee/module.go | 76 ++++------ modules/apps/29-fee/module_test.go | 143 ++++++++++-------- modules/apps/29-fee/types/expected_keepers.go | 29 ---- modules/apps/29-fee/types/keys.go | 6 - modules/apps/transfer/keeper/genesis.go | 18 +-- modules/apps/transfer/keeper/genesis_test.go | 2 +- modules/apps/transfer/keeper/keeper.go | 16 +- modules/apps/transfer/keeper/keeper_test.go | 7 - modules/apps/transfer/module.go | 13 +- modules/apps/transfer/module_test.go | 24 --- modules/apps/transfer/simulation/genesis.go | 2 +- .../apps/transfer/simulation/genesis_test.go | 2 +- modules/apps/transfer/types/genesis.go | 12 +- modules/apps/transfer/types/genesis.pb.go | 78 +++++----- modules/apps/transfer/types/genesis_test.go | 4 +- modules/apps/transfer/types/keys.go | 3 - modules/core/04-channel/types/version.go | 12 +- .../applications/transfer/v1/genesis.proto | 2 +- testing/simapp/app.go | 21 +-- 22 files changed, 234 insertions(+), 292 deletions(-) diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md index c1b639d7c3e..f571101508f 100644 --- a/docs/ibc/proto-docs.md +++ b/docs/ibc/proto-docs.md @@ -1048,7 +1048,7 @@ GenesisState defines the ibc-transfer genesis state | Field | Type | Label | Description | | ----- | ---- | ----- | ----------- | -| `port_ids` | [string](#string) | repeated | | +| `port_id` | [string](#string) | | | | `denom_traces` | [DenomTrace](#ibc.applications.transfer.v1.DenomTrace) | repeated | | | `params` | [Params](#ibc.applications.transfer.v1.Params) | | | diff --git a/modules/apps/29-fee/fee_test.go b/modules/apps/29-fee/fee_test.go index be702977e7c..80f5d4e1cd7 100644 --- a/modules/apps/29-fee/fee_test.go +++ b/modules/apps/29-fee/fee_test.go @@ -4,8 +4,9 @@ import ( "testing" fee "github.com/cosmos/ibc-go/modules/apps/29-fee" - "github.com/cosmos/ibc-go/modules/apps/transfer" + "github.com/cosmos/ibc-go/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/testing" "github.com/stretchr/testify/suite" ) @@ -28,14 +29,12 @@ func (suite *FeeTestSuite) SetupTest() { suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(0)) suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1)) - keeper := suite.chainA.GetSimApp().IBCFeeKeeper - - transferModule := transfer.NewAppModule(suite.chainA.GetSimApp().TransferKeeper) - suite.moduleA = fee.NewAppModule(keeper, suite.chainA.GetSimApp().ScopedIBCFeeKeeper, transferModule) - path := ibctesting.NewPath(suite.chainA, suite.chainB) - path.EndpointA.ChannelConfig.PortID = transfertypes.FeePortID - path.EndpointB.ChannelConfig.PortID = transfertypes.FeePortID + feeTransferVersion := channeltypes.MergeChannelVersions(types.Version, transfertypes.Version) + path.EndpointA.ChannelConfig.Version = feeTransferVersion + path.EndpointB.ChannelConfig.Version = feeTransferVersion + path.EndpointA.ChannelConfig.PortID = transfertypes.PortID + path.EndpointB.ChannelConfig.PortID = transfertypes.PortID suite.path = path } diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index 764862bb6e6..d9cd5673442 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -6,9 +6,12 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" + capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/modules/core/24-host" + ibcexported "github.com/cosmos/ibc-go/modules/core/exported" ) // Keeper defines the IBC fungible transfer keeper @@ -18,8 +21,6 @@ type Keeper struct { channelKeeper types.ChannelKeeper portKeeper types.PortKeeper - authKeeper types.AccountKeeper - bankKeeper types.BankKeeper scopedKeeper capabilitykeeper.ScopedKeeper } @@ -27,7 +28,7 @@ type Keeper struct { func NewKeeper( cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace, channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper, - authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, scopedKeeper capabilitykeeper.ScopedKeeper, + scopedKeeper capabilitykeeper.ScopedKeeper, ) Keeper { return Keeper{ @@ -35,8 +36,6 @@ func NewKeeper( storeKey: key, channelKeeper: channelKeeper, portKeeper: portKeeper, - authKeeper: authKeeper, - bankKeeper: bankKeeper, scopedKeeper: scopedKeeper, } } @@ -52,11 +51,27 @@ func (k Keeper) IsBound(ctx sdk.Context, portID string) bool { return ok } -// BindPort defines a wrapper function for the ort Keeper's function in +// BindPort defines a wrapper function for the port Keeper's function in // order to expose it to module's InitGenesis function -func (k Keeper) BindPort(ctx sdk.Context, portID string) error { - cap := k.portKeeper.BindPort(ctx, portID) - return k.scopedKeeper.ClaimCapability(ctx, cap, host.PortPath(portID)) +func (k Keeper) BindPort(ctx sdk.Context, portID string) *capabilitytypes.Capability { + return k.portKeeper.BindPort(ctx, portID) +} + +// ChanCloseInit wraps the channel keeper's function in order to expose it to underlying app. +func (k Keeper) ChanCloseInit(ctx sdk.Context, portID, channelID string, chanCap *capabilitytypes.Capability) error { + return k.channelKeeper.ChanCloseInit(ctx, portID, channelID, chanCap) +} + +func (k Keeper) GetChannel(ctx sdk.Context, portID, channelID string) (channeltypes.Channel, bool) { + return k.channelKeeper.GetChannel(ctx, portID, channelID) +} + +func (k Keeper) GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool) { + return k.channelKeeper.GetNextSequenceSend(ctx, portID, channelID) +} + +func (k Keeper) SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI) error { + return k.channelKeeper.SendPacket(ctx, chanCap, packet) } // // GetPort returns the portID for the transfer module. Used in ExportGenesis @@ -78,6 +93,12 @@ func (k Keeper) SetFeeEnabled(ctx sdk.Context, portID, channelID string) { store.Set(types.FeeEnabledKey(portID, channelID), []byte{1}) } +// DeleteFeeEnabled deletes the fee enabled flag for a given portID and channelID +func (k Keeper) DeleteFeeEnabled(ctx sdk.Context, portID, channelID string) { + store := ctx.KVStore(k.storeKey) + store.Delete(types.FeeEnabledKey(portID, channelID)) +} + // IsFeeEnabled returns whether fee handling logic should be run for the given port by checking the // fee enabled flag for the given port and channel identifiers func (k Keeper) IsFeeEnabled(ctx sdk.Context, portID, channelID string) bool { diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index 81a6068a947..688aeb4bb05 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -3,7 +3,6 @@ package fee import ( "context" "encoding/json" - "fmt" "math/rand" "github.com/cosmos/cosmos-sdk/client" @@ -28,7 +27,6 @@ import ( // "github.com/cosmos/ibc-go/modules/apps/29-fee/simulation" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/modules/core/05-port/types" - host "github.com/cosmos/ibc-go/modules/core/24-host" ibcexported "github.com/cosmos/ibc-go/modules/core/exported" ) @@ -203,22 +201,17 @@ func (am AppModule) OnChanOpenInit( version string, ) error { feeVersion, appVersion := channeltypes.SplitChannelVersion(version) - if feeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) - } - // Claim channel capability passed back by IBC module - if err := am.scopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { - return err - } + if feeVersion != "" { + if feeVersion != types.Version { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) + } - appCap, err := am.scopedKeeper.NewCapability(ctx, types.AppCapabilityName(portID, channelID)) - if err != nil { - return sdkerrors.Wrap(err, "could not create capability for underlying application") + am.keeper.SetFeeEnabled(ctx, portID, channelID) } // call underlying app's OnChanOpenInit callback with the appVersion return am.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, - appCap, counterparty, appVersion) + chanCap, counterparty, appVersion) } // OnChanOpenTry implements the IBCModule interface @@ -233,48 +226,25 @@ func (am AppModule) OnChanOpenTry( version, counterpartyVersion string, ) error { - fmt.Println("FEE TRANSFER") feeVersion, appVersion := channeltypes.SplitChannelVersion(version) cpFeeVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) - if feeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) - } - if cpFeeVersion != feeVersion { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) - } - var ( - appCap *capabilitytypes.Capability - err error - ok bool - ) - fmt.Println("why") - fmt.Println(host.ChannelCapabilityPath(portID, channelID)) - // Module may have already claimed capability in OnChanOpenInit in the case of crossing hellos - // (ie chainA and chainB both call ChanOpenInit before one of them calls ChanOpenTry) - // If module can already authenticate the capability then module already owns it so we don't need to claim - // Otherwise, module does not have channel capability and we must claim it from IBC - if !am.scopedKeeper.AuthenticateCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)) { - fmt.Println("hello") - // Only claim channel capability passed back by IBC module if we do not already own it - if err := am.scopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { - return sdkerrors.Wrap(err, "fee middleware could not claim capability from caller") + if feeVersion != "" || cpFeeVersion != "" { + if feeVersion != types.Version { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) } - appCap, err = am.scopedKeeper.NewCapability(ctx, types.AppCapabilityName(portID, channelID)) - if err != nil { - return sdkerrors.Wrap(err, "could not create capability for underlying application") - } - } else { - fmt.Println("bye") - appCap, ok = am.scopedKeeper.GetCapability(ctx, types.AppCapabilityName(portID, channelID)) - if !ok { - return sdkerrors.Wrap(capabilitytypes.ErrCapabilityNotFound, - "could not find app capability on OnChanOpenTry even after OnChanOpenInit called on this chain first (crossing hellos)") + if cpFeeVersion != feeVersion { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) } + + am.keeper.SetFeeEnabled(ctx, portID, channelID) + } else if am.keeper.IsFeeEnabled(ctx, portID, channelID) { + // return error if a previous ChanInit set fee enabled but subsequent OpenTry does not have fee enabled + return sdkerrors.Wrapf(types.ErrInvalidVersion, "previous INIT call (crossing hellos) had fee version set but OpenTry call does not set fee version: %s", version) } // call underlying app's OnChanOpenTry callback with the app versions return am.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, - appCap, counterparty, appVersion, cpAppVersion) + chanCap, counterparty, appVersion, cpAppVersion) } // OnChanOpenAck implements the IBCModule interface @@ -284,10 +254,14 @@ func (am AppModule) OnChanOpenAck( channelID string, counterpartyVersion string, ) error { - cpFeeVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) + cpAppVersion := counterpartyVersion + if am.keeper.IsFeeEnabled(ctx, portID, channelID) { + var cpFeeVersion string + cpFeeVersion, cpAppVersion = channeltypes.SplitChannelVersion(counterpartyVersion) - if cpFeeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) + if cpFeeVersion != types.Version { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) + } } // call underlying app's OnChanOpenAck callback with the counterparty app version. return am.app.OnChanOpenAck(ctx, portID, channelID, cpAppVersion) @@ -310,6 +284,7 @@ func (am AppModule) OnChanCloseInit( channelID string, ) error { // TODO: Unescrow all remaining funds for unprocessed packets + am.keeper.DeleteFeeEnabled(ctx, portID, channelID) return am.app.OnChanCloseInit(ctx, portID, channelID) } @@ -320,6 +295,7 @@ func (am AppModule) OnChanCloseConfirm( channelID string, ) error { // TODO: Unescrow all remaining funds for unprocessed packets + am.keeper.DeleteFeeEnabled(ctx, portID, channelID) return am.app.OnChanCloseConfirm(ctx, portID, channelID) } diff --git a/modules/apps/29-fee/module_test.go b/modules/apps/29-fee/module_test.go index bcbc74e5d87..ed03748fe5f 100644 --- a/modules/apps/29-fee/module_test.go +++ b/modules/apps/29-fee/module_test.go @@ -1,12 +1,10 @@ package fee_test import ( - "fmt" - - "github.com/cosmos/ibc-go/modules/apps/29-fee/types" - transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" + capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/modules/core/24-host" + ibctesting "github.com/cosmos/ibc-go/testing" ) func (suite *FeeTestSuite) TestOnChanOpenInit() { @@ -20,6 +18,11 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { "fee29-1:ics20-1", true, }, + { + "fee version not included, only perform transfer logic", + "ics20-1", + true, + }, { "invalid fee middleware version", "otherfee28-1:ics20-1", @@ -40,11 +43,6 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { "fee29-1", false, }, - { - "fee version not included", - "ics20-1", - false, - }, { "hanging delimiter", "fee29-1:ics20-1:", @@ -58,37 +56,34 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { suite.Run(tc.name, func() { // reset suite suite.SetupTest() + suite.coordinator.SetupClients(suite.path) + suite.coordinator.SetupConnections(suite.path) + + suite.path.EndpointA.ChannelID = ibctesting.FirstChannelID + + counterparty := channeltypes.NewCounterparty(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID) + channel := &channeltypes.Channel{ + State: channeltypes.INIT, + Ordering: channeltypes.UNORDERED, + Counterparty: counterparty, + ConnectionHops: []string{suite.path.EndpointA.ConnectionID}, + Version: tc.version, + } - ctx := suite.chainA.GetContext() - cap, err := suite.chainA.GetSimApp().ScopedIBCKeeper.NewCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort) suite.Require().NoError(err) - err = suite.moduleA.OnChanOpenInit( - ctx, - channeltypes.UNORDERED, - []string{"connection-1"}, - transfertypes.FeePortID, - "channel-1", - cap, - channeltypes.NewCounterparty(transfertypes.FeePortID, ""), - tc.version, - ) - if tc.expPass { - suite.Require().NoError(err, "unexpected error from version: %s", tc.version) + chanCap, err := suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, suite.path.EndpointA.ChannelID)) + suite.Require().NoError(err) - // check that capabilities are properly claimed and issued - ctx := suite.chainA.GetContext() - ibcCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) - suite.Require().NotNil(ibcCap, "IBC capability is nil on fee keeper") - suite.Require().True(ok) + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) - appFeeCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, types.AppCapabilityName(transfertypes.FeePortID, "channel-1")) - suite.Require().NotNil(appFeeCap, "App capability not created or owned by ibc fee keeper") - suite.Require().True(ok) - transferCap, ok := suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, "channel-1")) - suite.Require().NotNil(transferCap, "App capability not claimed by transfer keeper") - suite.Require().True(ok) - suite.Require().Equal(appFeeCap, transferCap, "app capabilities not equal") + err = cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), + suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, chanCap, counterparty, channel.Version) + + if tc.expPass { + suite.Require().NoError(err, "unexpected error from version: %s", tc.version) } else { suite.Require().Error(err, "error not returned for version: %s", tc.version) } @@ -111,6 +106,13 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { false, true, }, + { + "valid transfer version on try and counterparty", + "ics20-1", + "ics20-1", + false, + true, + }, { "valid fee middleware and transfer version, crossing hellos", "fee29-1:ics20-1", @@ -161,7 +163,7 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { false, }, { - "fee version not included", + "fee version not included on try, but included in counterparty", "ics20-1", "fee29-1:ics20-1", false, @@ -186,29 +188,42 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { suite.coordinator.SetupConnections(suite.path) suite.path.EndpointB.ChanOpenInit() + var ( + chanCap *capabilitytypes.Capability + ok bool + err error + ) if tc.capExists { suite.path.EndpointA.ChanOpenInit() + chanCap, ok = suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, suite.path.EndpointA.ChannelID)) + suite.Require().True(ok) + } else { + chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, suite.path.EndpointA.ChannelID)) + suite.Require().NoError(err) } - fmt.Println("port:", suite.path.EndpointA.ChannelConfig.PortID) - err := suite.path.EndpointA.ChanOpenTry() + suite.path.EndpointA.ChannelID = ibctesting.FirstChannelID - if tc.expPass { - suite.Require().NoError(err, "unexpected error from version: %s", tc.version) + counterparty := channeltypes.NewCounterparty(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID) + channel := &channeltypes.Channel{ + State: channeltypes.INIT, + Ordering: channeltypes.UNORDERED, + Counterparty: counterparty, + ConnectionHops: []string{suite.path.EndpointA.ConnectionID}, + Version: tc.version, + } - // check that capabilities are properly claimed and issued - ctx := suite.chainA.GetContext() - ibcCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, suite.path.EndpointA.ChannelID)) - suite.Require().NotNil(ibcCap, "IBC capability is nil on fee keeper: %s", host.ChannelCapabilityPath(transfertypes.FeePortID, suite.path.EndpointA.ChannelID)) - suite.Require().True(ok) + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort) + suite.Require().NoError(err) - appFeeCap, ok := suite.chainA.GetSimApp().ScopedIBCFeeKeeper.GetCapability(ctx, types.AppCapabilityName(transfertypes.FeePortID, suite.path.EndpointA.ChannelID)) - suite.Require().NotNil(appFeeCap, "App capability not created or owned by ibc fee keeper") - suite.Require().True(ok) - transferCap, ok := suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(ctx, host.ChannelCapabilityPath(transfertypes.FeePortID, suite.path.EndpointA.ChannelID)) - suite.Require().NotNil(transferCap, "App capability not claimed by transfer keeper") - suite.Require().True(ok) - suite.Require().Equal(appFeeCap, transferCap, "app capabilities not equal") + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + err = cbs.OnChanOpenTry(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), + suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, chanCap, counterparty, tc.version, tc.cpVersion) + + if tc.expPass { + suite.Require().NoError(err, "unexpected error from version: %s", tc.version) } else { suite.Require().Error(err, "error not returned for version: %s", tc.version) } @@ -240,12 +255,22 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { } for _, tc := range testCases { - ctx := suite.chainA.GetContext() - err := suite.moduleA.OnChanOpenAck(ctx, transfertypes.FeePortID, "channel-1", tc.cpVersion) - if tc.expPass { - suite.Require().NoError(err, "unexpected error for case: %s", tc.name) - } else { - suite.Require().Error(err, "%s expected error but returned none", tc.name) - } + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + suite.coordinator.SetupClients(suite.path) + suite.coordinator.SetupConnections(suite.path) + + suite.path.EndpointA.ChanOpenInit() + suite.path.EndpointB.ChanOpenTry() + + suite.path.EndpointB.ChannelConfig.Version = tc.cpVersion + err := suite.path.EndpointA.ChanOpenAck() + if tc.expPass { + suite.Require().NoError(err, "unexpected error for case: %s", tc.name) + } else { + suite.Require().Error(err, "%s expected error but returned none", tc.name) + } + }) } } diff --git a/modules/apps/29-fee/types/expected_keepers.go b/modules/apps/29-fee/types/expected_keepers.go index 68513b476a5..8e49bbe8ab0 100644 --- a/modules/apps/29-fee/types/expected_keepers.go +++ b/modules/apps/29-fee/types/expected_keepers.go @@ -1,30 +1,12 @@ package types -/* import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth/types" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" - connectiontypes "github.com/cosmos/ibc-go/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ibcexported "github.com/cosmos/ibc-go/modules/core/exported" ) -// AccountKeeper defines the contract required for account APIs. -type AccountKeeper interface { - GetModuleAddress(name string) sdk.AccAddress - GetModuleAccount(ctx sdk.Context, name string) types.ModuleAccountI -} - -// BankKeeper defines the expected bank keeper -type BankKeeper interface { - SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error - MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error - BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error - SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error - SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error -} - // ChannelKeeper defines the expected IBC channel keeper type ChannelKeeper interface { GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channeltypes.Channel, found bool) @@ -33,18 +15,7 @@ type ChannelKeeper interface { ChanCloseInit(ctx sdk.Context, portID, channelID string, chanCap *capabilitytypes.Capability) error } -// ClientKeeper defines the expected IBC client keeper -type ClientKeeper interface { - GetClientConsensusState(ctx sdk.Context, clientID string) (connection ibcexported.ConsensusState, found bool) -} - -// ConnectionKeeper defines the expected IBC connection keeper -type ConnectionKeeper interface { - GetConnection(ctx sdk.Context, connectionID string) (connection connectiontypes.ConnectionEnd, found bool) -} - // PortKeeper defines the expected IBC port keeper type PortKeeper interface { BindPort(ctx sdk.Context, portID string) *capabilitytypes.Capability } -*/ diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index 01d9ada3a2d..ee8926560a5 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -16,14 +16,8 @@ const ( QuerierRoute = ModuleName Version = "fee29-1" - - KeyAppCapability = "app_capabilities" ) -func AppCapabilityName(portID, channelID string) string { - return fmt.Sprintf("%s/%s/%s", KeyAppCapability, channelID, portID) -} - func FeeEnabledKey(portID, channelID string) []byte { return []byte(fmt.Sprintf("fee_enabled/%s/%s", portID, channelID)) } diff --git a/modules/apps/transfer/keeper/genesis.go b/modules/apps/transfer/keeper/genesis.go index 6ddc2242985..7050a2c5512 100644 --- a/modules/apps/transfer/keeper/genesis.go +++ b/modules/apps/transfer/keeper/genesis.go @@ -9,7 +9,7 @@ import ( // InitGenesis initializes the ibc-transfer state and binds to PortID. func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) { - k.SetPorts(ctx, state.PortIds) + k.SetPort(ctx, state.PortId) for _, trace := range state.DenomTraces { k.SetDenomTrace(ctx, trace) @@ -17,14 +17,12 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) { // Only try to bind to port if it is not already bound, since we may already own // port capability from capability InitGenesis - for _, port := range state.PortIds { - if !k.IsBound(ctx, port) { - // transfer module binds to the transfer port on InitChain - // and claims the returned capability - err := k.BindPort(ctx, port) - if err != nil { - panic(fmt.Sprintf("could not claim port capability: %v", err)) - } + if !k.IsBound(ctx, state.PortId) { + // transfer module binds to the transfer port on InitChain + // and claims the returned capability + err := k.BindPort(ctx, state.PortId) + if err != nil { + panic(fmt.Sprintf("could not claim port capability: %v", err)) } } @@ -40,7 +38,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) { // ExportGenesis exports ibc-transfer module's portID and denom trace info into its genesis state. func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { return &types.GenesisState{ - PortIds: k.GetPorts(ctx), + PortId: k.GetPort(ctx), DenomTraces: k.GetAllDenomTraces(ctx), Params: k.GetParams(ctx), } diff --git a/modules/apps/transfer/keeper/genesis_test.go b/modules/apps/transfer/keeper/genesis_test.go index aa2d044de22..19e5dfe4a18 100644 --- a/modules/apps/transfer/keeper/genesis_test.go +++ b/modules/apps/transfer/keeper/genesis_test.go @@ -30,7 +30,7 @@ func (suite *KeeperTestSuite) TestGenesis() { genesis := suite.chainA.GetSimApp().TransferKeeper.ExportGenesis(suite.chainA.GetContext()) - suite.Require().Equal([]string{types.PortID, types.FeePortID}, genesis.PortIds) + suite.Require().Equal(types.PortID, genesis.PortId) suite.Require().Equal(traces.Sort(), genesis.DenomTraces) suite.Require().NotPanics(func() { diff --git a/modules/apps/transfer/keeper/keeper.go b/modules/apps/transfer/keeper/keeper.go index b89fb810cf0..08c75a26a0e 100644 --- a/modules/apps/transfer/keeper/keeper.go +++ b/modules/apps/transfer/keeper/keeper.go @@ -1,8 +1,6 @@ package keeper import ( - "strings" - tmbytes "github.com/tendermint/tendermint/libs/bytes" "github.com/tendermint/tendermint/libs/log" @@ -95,18 +93,16 @@ func (k Keeper) BindPort(ctx sdk.Context, portID string) error { return k.ClaimCapability(ctx, cap, host.PortPath(portID)) } -// GetPorts returns the bound portIDs for the transfer module. Used in ExportGenesis -func (k Keeper) GetPorts(ctx sdk.Context) []string { +// GetPort returns the portID for the transfer module. Used in ExportGenesis +func (k Keeper) GetPort(ctx sdk.Context) string { store := ctx.KVStore(k.storeKey) - portsStr := string(store.Get(types.PortKey)) - return strings.Split(portsStr, "/") + return string(store.Get(types.PortKey)) } -// SetPorts sets the bound portIDs the transfer module. Used in InitGenesis -func (k Keeper) SetPorts(ctx sdk.Context, portIDs []string) { +// SetPort sets the portID for the transfer module. Used in InitGenesis +func (k Keeper) SetPort(ctx sdk.Context, portID string) { store := ctx.KVStore(k.storeKey) - portsStr := strings.Join(portIDs, "/") - store.Set(types.PortKey, []byte(portsStr)) + store.Set(types.PortKey, []byte(portID)) } // GetDenomTrace retreives the full identifiers trace and base denomination from the store. diff --git a/modules/apps/transfer/keeper/keeper_test.go b/modules/apps/transfer/keeper/keeper_test.go index 60f52a88cb5..de3902df669 100644 --- a/modules/apps/transfer/keeper/keeper_test.go +++ b/modules/apps/transfer/keeper/keeper_test.go @@ -54,13 +54,6 @@ func (suite *KeeperTestSuite) TestGetTransferAccount() { suite.Require().Equal(expectedMaccAddr, macc.GetAddress()) } -func (suite *KeeperTestSuite) TestGetPorts() { - expectedPorts := []string{"transfer", "feetransfer", "testport"} - suite.chainA.GetSimApp().TransferKeeper.SetPorts(suite.chainA.GetContext(), expectedPorts) - gotPorts := suite.chainA.GetSimApp().TransferKeeper.GetPorts(suite.chainA.GetContext()) - suite.Require().Equal(expectedPorts, gotPorts, "ports set in store is not the same as ports retrieved from store") -} - func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go index d9a351c977b..2661681f577 100644 --- a/modules/apps/transfer/module.go +++ b/modules/apps/transfer/module.go @@ -209,16 +209,9 @@ func ValidateTransferChannelParams( } // Require portID is the portID transfer module is bound to - boundPorts := keeper.GetPorts(ctx) - var portOk bool - for _, boundPort := range boundPorts { - if boundPort == portID { - portOk = true - break - } - } - if !portOk { - return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid port: %s not bound by transfer module", portID) + boundPort := keeper.GetPort(ctx) + if boundPort != portID { + return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid port: %s, expected %s", portID, boundPort) } if portID != counterparty.PortId { diff --git a/modules/apps/transfer/module_test.go b/modules/apps/transfer/module_test.go index acfe513ab0e..954babec3f0 100644 --- a/modules/apps/transfer/module_test.go +++ b/modules/apps/transfer/module_test.go @@ -27,18 +27,6 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { { "success", func() {}, true, }, - { - "success with fee port", func() { - path.EndpointA.ChannelConfig.PortID = types.FeePortID - counterparty.PortId = types.FeePortID - }, true, - }, - { - "mismatched ports", func() { - path.EndpointA.ChannelConfig.PortID = types.PortID - counterparty.PortId = types.FeePortID - }, false, - }, { "max channels reached", func() { path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1) @@ -128,18 +116,6 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { { "success", func() {}, true, }, - { - "success with fee port", func() { - path.EndpointA.ChannelConfig.PortID = types.FeePortID - counterparty.PortId = types.FeePortID - }, true, - }, - { - "mismatched ports", func() { - path.EndpointA.ChannelConfig.PortID = types.PortID - counterparty.PortId = types.FeePortID - }, false, - }, { "max channels reached", func() { path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1) diff --git a/modules/apps/transfer/simulation/genesis.go b/modules/apps/transfer/simulation/genesis.go index fa3a450a76c..b300e8919ad 100644 --- a/modules/apps/transfer/simulation/genesis.go +++ b/modules/apps/transfer/simulation/genesis.go @@ -40,7 +40,7 @@ func RandomizedGenState(simState *module.SimulationState) { ) transferGenesis := types.GenesisState{ - PortIds: []string{portID}, + PortId: portID, DenomTraces: types.Traces{}, Params: types.NewParams(sendEnabled, receiveEnabled), } diff --git a/modules/apps/transfer/simulation/genesis_test.go b/modules/apps/transfer/simulation/genesis_test.go index 13da29a483d..c4bd61035ab 100644 --- a/modules/apps/transfer/simulation/genesis_test.go +++ b/modules/apps/transfer/simulation/genesis_test.go @@ -39,7 +39,7 @@ func TestRandomizedGenState(t *testing.T) { var ibcTransferGenesis types.GenesisState simState.Cdc.MustUnmarshalJSON(simState.GenState[types.ModuleName], &ibcTransferGenesis) - require.Equal(t, "euzxpfgkqegqiqwixnku", ibcTransferGenesis.PortIds[0]) + require.Equal(t, "euzxpfgkqegqiqwixnku", ibcTransferGenesis.PortId) require.True(t, ibcTransferGenesis.Params.SendEnabled) require.True(t, ibcTransferGenesis.Params.ReceiveEnabled) require.Len(t, ibcTransferGenesis.DenomTraces, 0) diff --git a/modules/apps/transfer/types/genesis.go b/modules/apps/transfer/types/genesis.go index 01dcb257e62..1a17bc474ce 100644 --- a/modules/apps/transfer/types/genesis.go +++ b/modules/apps/transfer/types/genesis.go @@ -5,9 +5,9 @@ import ( ) // NewGenesisState creates a new ibc-transfer GenesisState instance. -func NewGenesisState(portIDs []string, denomTraces Traces, params Params) *GenesisState { +func NewGenesisState(portID string, denomTraces Traces, params Params) *GenesisState { return &GenesisState{ - PortIds: portIDs, + PortId: portID, DenomTraces: denomTraces, Params: params, } @@ -16,7 +16,7 @@ func NewGenesisState(portIDs []string, denomTraces Traces, params Params) *Genes // DefaultGenesisState returns a GenesisState with "transfer" as the default PortID. func DefaultGenesisState() *GenesisState { return &GenesisState{ - PortIds: []string{PortID, FeePortID}, + PortId: PortID, DenomTraces: Traces{}, Params: DefaultParams(), } @@ -25,10 +25,8 @@ func DefaultGenesisState() *GenesisState { // Validate performs basic genesis state validation returning an error upon any // failure. func (gs GenesisState) Validate() error { - for _, port := range gs.PortIds { - if err := host.PortIdentifierValidator(port); err != nil { - return err - } + if err := host.PortIdentifierValidator(gs.PortId); err != nil { + return err } if err := gs.DenomTraces.Validate(); err != nil { return err diff --git a/modules/apps/transfer/types/genesis.pb.go b/modules/apps/transfer/types/genesis.pb.go index 1a6c57395e3..94eb0108c5d 100644 --- a/modules/apps/transfer/types/genesis.pb.go +++ b/modules/apps/transfer/types/genesis.pb.go @@ -25,9 +25,9 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // GenesisState defines the ibc-transfer genesis state type GenesisState struct { - PortIds []string `protobuf:"bytes,1,rep,name=port_ids,json=portIds,proto3" json:"port_ids,omitempty" yaml:"port_ids"` - DenomTraces Traces `protobuf:"bytes,2,rep,name=denom_traces,json=denomTraces,proto3,castrepeated=Traces" json:"denom_traces" yaml:"denom_traces"` - Params Params `protobuf:"bytes,3,opt,name=params,proto3" json:"params"` + PortId string `protobuf:"bytes,1,opt,name=port_id,json=portId,proto3" json:"port_id,omitempty" yaml:"port_id"` + DenomTraces Traces `protobuf:"bytes,2,rep,name=denom_traces,json=denomTraces,proto3,castrepeated=Traces" json:"denom_traces" yaml:"denom_traces"` + Params Params `protobuf:"bytes,3,opt,name=params,proto3" json:"params"` } func (m *GenesisState) Reset() { *m = GenesisState{} } @@ -63,11 +63,11 @@ func (m *GenesisState) XXX_DiscardUnknown() { var xxx_messageInfo_GenesisState proto.InternalMessageInfo -func (m *GenesisState) GetPortIds() []string { +func (m *GenesisState) GetPortId() string { if m != nil { - return m.PortIds + return m.PortId } - return nil + return "" } func (m *GenesisState) GetDenomTraces() Traces { @@ -94,26 +94,26 @@ func init() { var fileDescriptor_a4f788affd5bea89 = []byte{ // 323 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x90, 0x31, 0x4b, 0xc3, 0x40, - 0x1c, 0xc5, 0x13, 0x2b, 0x55, 0xd3, 0x82, 0x90, 0x3a, 0x94, 0x22, 0x49, 0x09, 0x0a, 0x41, 0xf1, - 0x8e, 0x56, 0x27, 0xc7, 0x20, 0x88, 0x8b, 0x48, 0x75, 0x72, 0x29, 0x97, 0xcb, 0x19, 0x0f, 0x9a, - 0xfc, 0xc3, 0xfd, 0xaf, 0x85, 0x7e, 0x0b, 0x3f, 0x87, 0x9f, 0xa4, 0x63, 0x47, 0xa7, 0x2a, 0xed, - 0x37, 0xe8, 0xe0, 0x2c, 0x97, 0xda, 0xd2, 0xa9, 0xdb, 0xe3, 0xee, 0xf7, 0xde, 0xfb, 0xf3, 0x9c, - 0x0b, 0x19, 0x73, 0xca, 0x8a, 0x62, 0x20, 0x39, 0xd3, 0x12, 0x72, 0xa4, 0x5a, 0xb1, 0x1c, 0xdf, - 0x84, 0xa2, 0xa3, 0x0e, 0x4d, 0x45, 0x2e, 0x50, 0x22, 0x29, 0x14, 0x68, 0x70, 0x4f, 0x65, 0xcc, - 0xc9, 0x36, 0x4b, 0xd6, 0x2c, 0x19, 0x75, 0x5a, 0x97, 0x3b, 0x93, 0x36, 0x64, 0x19, 0xd5, 0x3a, - 0x49, 0x21, 0x85, 0x52, 0x52, 0xa3, 0x56, 0xaf, 0xc1, 0xaf, 0xed, 0xd4, 0xef, 0x57, 0x95, 0xcf, - 0x9a, 0x69, 0xe1, 0x12, 0xe7, 0xb0, 0x00, 0xa5, 0xfb, 0x32, 0xc1, 0xa6, 0xdd, 0xae, 0x84, 0x47, - 0x51, 0x63, 0x39, 0xf3, 0x8f, 0xc7, 0x2c, 0x1b, 0xdc, 0x06, 0xeb, 0x9f, 0xa0, 0x77, 0x60, 0xe4, - 0x43, 0x82, 0xae, 0x72, 0xea, 0x89, 0xc8, 0x21, 0xeb, 0x6b, 0xc5, 0xb8, 0xc0, 0xe6, 0x5e, 0xbb, - 0x12, 0xd6, 0xba, 0x21, 0xd9, 0x75, 0x38, 0xb9, 0x33, 0x8e, 0x17, 0x63, 0x88, 0xce, 0x27, 0x33, - 0xdf, 0x5a, 0xce, 0xfc, 0xc6, 0xaa, 0x61, 0x3b, 0x2b, 0xf8, 0xfc, 0xf6, 0xab, 0x25, 0x85, 0xbd, - 0x5a, 0xb2, 0xb1, 0xa0, 0x1b, 0x39, 0xd5, 0x82, 0x29, 0x96, 0x61, 0xb3, 0xd2, 0xb6, 0xc3, 0x5a, - 0xf7, 0x6c, 0x77, 0xdb, 0x53, 0xc9, 0x46, 0xfb, 0xa6, 0xa9, 0xf7, 0xef, 0x8c, 0x1e, 0x27, 0x73, - 0xcf, 0x9e, 0xce, 0x3d, 0xfb, 0x67, 0xee, 0xd9, 0x1f, 0x0b, 0xcf, 0x9a, 0x2e, 0x3c, 0xeb, 0x6b, - 0xe1, 0x59, 0xaf, 0x37, 0xa9, 0xd4, 0xef, 0xc3, 0x98, 0x70, 0xc8, 0x28, 0x07, 0xcc, 0x00, 0xa9, - 0x8c, 0xf9, 0x55, 0x0a, 0x34, 0x83, 0x64, 0x38, 0x10, 0x68, 0x26, 0xdf, 0x9a, 0x5a, 0x8f, 0x0b, - 0x81, 0x71, 0xb5, 0xdc, 0xf3, 0xfa, 0x2f, 0x00, 0x00, 0xff, 0xff, 0x61, 0x40, 0x1b, 0xdb, 0xde, + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x90, 0xc1, 0x4a, 0xf3, 0x40, + 0x14, 0x85, 0x33, 0x7f, 0x7f, 0x22, 0xa6, 0xc5, 0x45, 0x74, 0x51, 0x8a, 0x24, 0x25, 0x28, 0x04, + 0x8b, 0x33, 0xb4, 0xba, 0x72, 0x19, 0x04, 0x71, 0x23, 0x52, 0x5d, 0xb9, 0x29, 0x93, 0xc9, 0x18, + 0x07, 0x9a, 0xdc, 0x30, 0x77, 0x5a, 0xe8, 0x5b, 0xf8, 0x1c, 0x3e, 0x49, 0x97, 0x5d, 0xba, 0xaa, + 0xd2, 0xbe, 0x41, 0x7d, 0x01, 0x49, 0x5a, 0x4b, 0x57, 0xdd, 0x1d, 0x66, 0xbe, 0x73, 0xce, 0xe5, + 0x38, 0x17, 0x2a, 0x16, 0x8c, 0x17, 0xc5, 0x50, 0x09, 0x6e, 0x14, 0xe4, 0xc8, 0x8c, 0xe6, 0x39, + 0xbe, 0x4a, 0xcd, 0xc6, 0x5d, 0x96, 0xca, 0x5c, 0xa2, 0x42, 0x5a, 0x68, 0x30, 0xe0, 0x9e, 0xaa, + 0x58, 0xd0, 0x5d, 0x96, 0xfe, 0xb1, 0x74, 0xdc, 0x6d, 0x75, 0xf6, 0x26, 0x6d, 0xc9, 0x2a, 0xaa, + 0x75, 0x92, 0x42, 0x0a, 0x95, 0x64, 0xa5, 0x5a, 0xbf, 0x06, 0x3f, 0xc4, 0x69, 0xdc, 0xad, 0x2b, + 0x9f, 0x0c, 0x37, 0xd2, 0xed, 0x38, 0x07, 0x05, 0x68, 0x33, 0x50, 0x49, 0x93, 0xb4, 0x49, 0x78, + 0x18, 0xb9, 0xab, 0xb9, 0x7f, 0x34, 0xe1, 0xd9, 0xf0, 0x26, 0xd8, 0x7c, 0x04, 0x7d, 0xbb, 0x54, + 0xf7, 0x89, 0xab, 0x9d, 0x46, 0x22, 0x73, 0xc8, 0x06, 0x46, 0x73, 0x21, 0xb1, 0xf9, 0xaf, 0x5d, + 0x0b, 0xeb, 0xbd, 0x90, 0xee, 0xbb, 0x9a, 0xde, 0x96, 0x8e, 0xe7, 0xd2, 0x10, 0x9d, 0x4f, 0xe7, + 0xbe, 0xb5, 0x9a, 0xfb, 0xc7, 0xeb, 0xfc, 0xdd, 0xac, 0xe0, 0xe3, 0xcb, 0xb7, 0x2b, 0x0a, 0xfb, + 0xf5, 0x64, 0x6b, 0x41, 0x37, 0x72, 0xec, 0x82, 0x6b, 0x9e, 0x61, 0xb3, 0xd6, 0x26, 0x61, 0xbd, + 0x77, 0xb6, 0xbf, 0xed, 0xb1, 0x62, 0xa3, 0xff, 0x65, 0x53, 0x7f, 0xe3, 0x8c, 0x1e, 0xa6, 0x0b, + 0x8f, 0xcc, 0x16, 0x1e, 0xf9, 0x5e, 0x78, 0xe4, 0x7d, 0xe9, 0x59, 0xb3, 0xa5, 0x67, 0x7d, 0x2e, + 0x3d, 0xeb, 0xe5, 0x3a, 0x55, 0xe6, 0x6d, 0x14, 0x53, 0x01, 0x19, 0x13, 0x80, 0x19, 0x20, 0x53, + 0xb1, 0xb8, 0x4c, 0x81, 0x65, 0x90, 0x8c, 0x86, 0x12, 0xcb, 0xbd, 0x77, 0x76, 0x36, 0x93, 0x42, + 0x62, 0x6c, 0x57, 0x63, 0x5e, 0xfd, 0x06, 0x00, 0x00, 0xff, 0xff, 0xb1, 0x5d, 0xce, 0xa9, 0xdb, 0x01, 0x00, 0x00, } @@ -161,14 +161,12 @@ func (m *GenesisState) MarshalToSizedBuffer(dAtA []byte) (int, error) { dAtA[i] = 0x12 } } - if len(m.PortIds) > 0 { - for iNdEx := len(m.PortIds) - 1; iNdEx >= 0; iNdEx-- { - i -= len(m.PortIds[iNdEx]) - copy(dAtA[i:], m.PortIds[iNdEx]) - i = encodeVarintGenesis(dAtA, i, uint64(len(m.PortIds[iNdEx]))) - i-- - dAtA[i] = 0xa - } + if len(m.PortId) > 0 { + i -= len(m.PortId) + copy(dAtA[i:], m.PortId) + i = encodeVarintGenesis(dAtA, i, uint64(len(m.PortId))) + i-- + dAtA[i] = 0xa } return len(dAtA) - i, nil } @@ -190,11 +188,9 @@ func (m *GenesisState) Size() (n int) { } var l int _ = l - if len(m.PortIds) > 0 { - for _, s := range m.PortIds { - l = len(s) - n += 1 + l + sovGenesis(uint64(l)) - } + l = len(m.PortId) + if l > 0 { + n += 1 + l + sovGenesis(uint64(l)) } if len(m.DenomTraces) > 0 { for _, e := range m.DenomTraces { @@ -244,7 +240,7 @@ func (m *GenesisState) Unmarshal(dAtA []byte) error { switch fieldNum { case 1: if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field PortIds", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field PortId", wireType) } var stringLen uint64 for shift := uint(0); ; shift += 7 { @@ -272,7 +268,7 @@ func (m *GenesisState) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - m.PortIds = append(m.PortIds, string(dAtA[iNdEx:postIndex])) + m.PortId = string(dAtA[iNdEx:postIndex]) iNdEx = postIndex case 2: if wireType != 2 { diff --git a/modules/apps/transfer/types/genesis_test.go b/modules/apps/transfer/types/genesis_test.go index 02e9c8b1d8e..23305ae1d07 100644 --- a/modules/apps/transfer/types/genesis_test.go +++ b/modules/apps/transfer/types/genesis_test.go @@ -22,14 +22,14 @@ func TestValidateGenesis(t *testing.T) { { "valid genesis", &types.GenesisState{ - PortIds: []string{"portidone", "portidtwo", "portidthree"}, + PortId: "portidone", }, true, }, { "invalid client", &types.GenesisState{ - PortIds: []string{"(INVALIDPORT)"}, + PortId: "(INVALIDPORT)", }, false, }, diff --git a/modules/apps/transfer/types/keys.go b/modules/apps/transfer/types/keys.go index f8e780c4339..c156af3fd88 100644 --- a/modules/apps/transfer/types/keys.go +++ b/modules/apps/transfer/types/keys.go @@ -18,9 +18,6 @@ const ( // PortID is the default port id that transfer module binds to PortID = "transfer" - // FeePortID is the port id that is wrapped by fee middleware - FeePortID = "feetransfer" - // StoreKey is the store key string for IBC transfer StoreKey = ModuleName diff --git a/modules/core/04-channel/types/version.go b/modules/core/04-channel/types/version.go index f739cf9de27..a2696d291ed 100644 --- a/modules/core/04-channel/types/version.go +++ b/modules/core/04-channel/types/version.go @@ -2,6 +2,8 @@ package types import "strings" +const ChannelVersionDelimiter = ":" + // SplitChannelVersion middleware version will split the channel version string // into the outermost middleware version and the underlying app version. // It will use the default delimiter `:` for middleware versions. @@ -9,11 +11,17 @@ import "strings" // and the full input as the second underlying app version. func SplitChannelVersion(version string) (middlewareVersion, appVersion string) { // only split out the first middleware version - splitVersions := strings.Split(version, ":") + splitVersions := strings.Split(version, ChannelVersionDelimiter) if len(splitVersions) == 1 { return "", version } middlewareVersion = splitVersions[0] - appVersion = strings.Join(splitVersions[1:], ":") + appVersion = strings.Join(splitVersions[1:], ChannelVersionDelimiter) return } + +// MergeChannelVersions merges the provided versions together with the channel version delimiter +// the versions should be passed in from the highest-level middleware to the base application +func MergeChannelVersions(versions ...string) string { + return strings.Join(versions, ChannelVersionDelimiter) +} diff --git a/proto/ibc/applications/transfer/v1/genesis.proto b/proto/ibc/applications/transfer/v1/genesis.proto index 1c27652085c..9c6b78ac7b1 100644 --- a/proto/ibc/applications/transfer/v1/genesis.proto +++ b/proto/ibc/applications/transfer/v1/genesis.proto @@ -9,7 +9,7 @@ import "gogoproto/gogo.proto"; // GenesisState defines the ibc-transfer genesis state message GenesisState { - repeated string port_ids = 1 [(gogoproto.moretags) = "yaml:\"port_ids\""]; + string port_id = 1 [(gogoproto.moretags) = "yaml:\"port_id\""]; repeated DenomTrace denom_traces = 2 [ (gogoproto.castrepeated) = "Traces", (gogoproto.nullable) = false, diff --git a/testing/simapp/app.go b/testing/simapp/app.go index f37fb412a75..324965ac2ff 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -320,29 +320,30 @@ func NewSimApp( &stakingKeeper, govRouter, ) - // Create Transfer Keepers + // Create Transfer Keeper + app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), + app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, + app.ScopedIBCFeeKeeper, + ) + app.TransferKeeper = ibctransferkeeper.NewKeeper( appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName), - app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, + app.IBCFeeKeeper, &app.IBCFeeKeeper, app.AccountKeeper, app.BankKeeper, scopedTransferKeeper, ) transferModule := transfer.NewAppModule(app.TransferKeeper) - app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), - app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, - app.AccountKeeper, app.BankKeeper, app.ScopedIBCFeeKeeper, - ) - feeTransferModule := ibcfee.NewAppModule(app.IBCFeeKeeper, app.ScopedIBCFeeKeeper, transferModule) + feeTransferModule := ibcfee.NewAppModule(app.IBCFeeKeeper, app.ScopedTransferKeeper, transferModule) // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // note replicate if you do not need to test core IBC or light clients. mockModule := ibcmock.NewAppModule(scopedIBCMockKeeper, &app.IBCKeeper.PortKeeper) + feeMockModule := ibcfee.NewAppModule(app.IBCFeeKeeper, scopedIBCMockKeeper, mockModule) // Create static IBC router, add transfer route, then set and seal it ibcRouter := porttypes.NewRouter() - ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferModule) - ibcRouter.AddRoute(ibcmock.ModuleName, mockModule) - ibcRouter.AddRoute(ibcfeetypes.ModuleName, feeTransferModule) + ibcRouter.AddRoute(ibctransfertypes.ModuleName, feeTransferModule) + ibcRouter.AddRoute(ibcmock.ModuleName, feeMockModule) app.IBCKeeper.SetRouter(ibcRouter) // create evidence keeper with router From f49f0b4ac01e964f9f5255f76d04370d16270e8f Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 20 Sep 2021 14:55:04 +0200 Subject: [PATCH 09/21] fix tests --- modules/apps/29-fee/module.go | 9 ++++++--- modules/apps/29-fee/module_test.go | 9 +++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index 688aeb4bb05..fd16c495c11 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -305,7 +305,8 @@ func (am AppModule) OnRecvPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) ibcexported.Acknowledgement { - return nil + // TODO: Implement fee specific logic if fee is enabled for the given channel + return am.app.OnRecvPacket(ctx, packet, relayer) } // OnAcknowledgementPacket implements the IBCModule interface @@ -315,7 +316,8 @@ func (am AppModule) OnAcknowledgementPacket( acknowledgement []byte, relayer sdk.AccAddress, ) error { - return nil + // TODO: Implement fee specific logic if fee is enabled for the given channel + return am.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) } // OnTimeoutPacket implements the IBCModule interface @@ -324,5 +326,6 @@ func (am AppModule) OnTimeoutPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) error { - return nil + // TODO: Implement fee specific logic if fee is enabled for the given channel + return am.app.OnTimeoutPacket(ctx, packet, relayer) } diff --git a/modules/apps/29-fee/module_test.go b/modules/apps/29-fee/module_test.go index ed03748fe5f..56637d28a51 100644 --- a/modules/apps/29-fee/module_test.go +++ b/modules/apps/29-fee/module_test.go @@ -264,8 +264,13 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { suite.path.EndpointA.ChanOpenInit() suite.path.EndpointB.ChanOpenTry() - suite.path.EndpointB.ChannelConfig.Version = tc.cpVersion - err := suite.path.EndpointA.ChanOpenAck() + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + err = cbs.OnChanOpenAck(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, tc.cpVersion) if tc.expPass { suite.Require().NoError(err, "unexpected error for case: %s", tc.name) } else { From 4a73249faf32f5f77aa690877706fdb7b868885e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 20 Sep 2021 17:57:58 +0200 Subject: [PATCH 10/21] much cleaner simapp --- modules/apps/29-fee/fee_test.go | 3 -- modules/apps/29-fee/keeper/keeper.go | 22 -------- modules/apps/29-fee/module.go | 75 ++++++++++++++++------------ modules/core/keeper/msg_server.go | 2 - testing/simapp/app.go | 9 ++-- 5 files changed, 46 insertions(+), 65 deletions(-) diff --git a/modules/apps/29-fee/fee_test.go b/modules/apps/29-fee/fee_test.go index 80f5d4e1cd7..71d435472a5 100644 --- a/modules/apps/29-fee/fee_test.go +++ b/modules/apps/29-fee/fee_test.go @@ -3,7 +3,6 @@ package fee_test import ( "testing" - fee "github.com/cosmos/ibc-go/modules/apps/29-fee" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" @@ -20,8 +19,6 @@ type FeeTestSuite struct { chainB *ibctesting.TestChain path *ibctesting.Path - - moduleA fee.AppModule } func (suite *FeeTestSuite) SetupTest() { diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index d9cd5673442..7be0e42808e 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -5,7 +5,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" @@ -21,14 +20,12 @@ type Keeper struct { channelKeeper types.ChannelKeeper portKeeper types.PortKeeper - scopedKeeper capabilitykeeper.ScopedKeeper } // NewKeeper creates a new 29-fee Keeper instance func NewKeeper( cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace, channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper, - scopedKeeper capabilitykeeper.ScopedKeeper, ) Keeper { return Keeper{ @@ -36,7 +33,6 @@ func NewKeeper( storeKey: key, channelKeeper: channelKeeper, portKeeper: portKeeper, - scopedKeeper: scopedKeeper, } } @@ -45,12 +41,6 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", "x/"+host.ModuleName+"-"+types.ModuleName) } -// IsBound checks if the transfer module is already bound to the desired port -func (k Keeper) IsBound(ctx sdk.Context, portID string) bool { - _, ok := k.scopedKeeper.GetCapability(ctx, host.PortPath(portID)) - return ok -} - // BindPort defines a wrapper function for the port Keeper's function in // order to expose it to module's InitGenesis function func (k Keeper) BindPort(ctx sdk.Context, portID string) *capabilitytypes.Capability { @@ -74,18 +64,6 @@ func (k Keeper) SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability, return k.channelKeeper.SendPacket(ctx, chanCap, packet) } -// // GetPort returns the portID for the transfer module. Used in ExportGenesis -// func (k Keeper) GetPort(ctx sdk.Context) string { -// store := ctx.KVStore(k.storeKey) -// return string(store.Get(types.PortKey)) -// } - -// // SetPort sets the portID for the transfer module. Used in InitGenesis -// func (k Keeper) SetPort(ctx sdk.Context, portID string) { -// store := ctx.KVStore(k.storeKey) -// store.Set(types.PortKey, []byte(portID)) -// } - // SetFeeEnabled sets a flag to determine if fee handling logic should run for the given channel // identified by channel and port identifiers. func (k Keeper) SetFeeEnabled(ctx sdk.Context, portID, channelID string) { diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index fd16c495c11..59292500a64 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -18,7 +18,6 @@ import ( "github.com/spf13/cobra" abci "github.com/tendermint/tendermint/abci/types" - capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" "github.com/cosmos/ibc-go/modules/apps/29-fee/client/cli" "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" @@ -32,7 +31,7 @@ import ( var ( _ module.AppModule = AppModule{} - _ porttypes.IBCModule = AppModule{} + _ porttypes.IBCModule = IBCModule{} _ module.AppModuleBasic = AppModuleBasic{} ) @@ -92,17 +91,13 @@ func (AppModuleBasic) GetQueryCmd() *cobra.Command { // AppModule represents the AppModule for this module type AppModule struct { AppModuleBasic - keeper keeper.Keeper - scopedKeeper capabilitykeeper.ScopedKeeper - app porttypes.IBCModule + keeper keeper.Keeper } // NewAppModule creates a new 29-fee module -func NewAppModule(k keeper.Keeper, scopedKeeper capabilitykeeper.ScopedKeeper, app porttypes.IBCModule) AppModule { +func NewAppModule(k keeper.Keeper) AppModule { return AppModule{ - keeper: k, - scopedKeeper: scopedKeeper, - app: app, + keeper: k, } } @@ -189,8 +184,22 @@ func (am AppModule) WeightedOperations(_ module.SimulationState) []simtypes.Weig return nil } +// IBCModule implements the ICS26 callbacks for the fee middleware given the fee keeper and the underlying application. +type IBCModule struct { + keeper keeper.Keeper + app porttypes.IBCModule +} + +// NewIBCModule creates a new IBCModule given the keeper and underlying application +func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule) IBCModule { + return IBCModule{ + keeper: k, + app: app, + } +} + // OnChanOpenInit implements the IBCModule interface -func (am AppModule) OnChanOpenInit( +func (im IBCModule) OnChanOpenInit( ctx sdk.Context, order channeltypes.Order, connectionHops []string, @@ -206,16 +215,16 @@ func (am AppModule) OnChanOpenInit( return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) } - am.keeper.SetFeeEnabled(ctx, portID, channelID) + im.keeper.SetFeeEnabled(ctx, portID, channelID) } // call underlying app's OnChanOpenInit callback with the appVersion - return am.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, + return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, appVersion) } // OnChanOpenTry implements the IBCModule interface -func (am AppModule) OnChanOpenTry( +func (im IBCModule) OnChanOpenTry( ctx sdk.Context, order channeltypes.Order, connectionHops []string, @@ -237,25 +246,25 @@ func (am AppModule) OnChanOpenTry( return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) } - am.keeper.SetFeeEnabled(ctx, portID, channelID) - } else if am.keeper.IsFeeEnabled(ctx, portID, channelID) { + im.keeper.SetFeeEnabled(ctx, portID, channelID) + } else if im.keeper.IsFeeEnabled(ctx, portID, channelID) { // return error if a previous ChanInit set fee enabled but subsequent OpenTry does not have fee enabled return sdkerrors.Wrapf(types.ErrInvalidVersion, "previous INIT call (crossing hellos) had fee version set but OpenTry call does not set fee version: %s", version) } // call underlying app's OnChanOpenTry callback with the app versions - return am.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, + return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, appVersion, cpAppVersion) } // OnChanOpenAck implements the IBCModule interface -func (am AppModule) OnChanOpenAck( +func (im IBCModule) OnChanOpenAck( ctx sdk.Context, portID, channelID string, counterpartyVersion string, ) error { cpAppVersion := counterpartyVersion - if am.keeper.IsFeeEnabled(ctx, portID, channelID) { + if im.keeper.IsFeeEnabled(ctx, portID, channelID) { var cpFeeVersion string cpFeeVersion, cpAppVersion = channeltypes.SplitChannelVersion(counterpartyVersion) @@ -264,68 +273,68 @@ func (am AppModule) OnChanOpenAck( } } // call underlying app's OnChanOpenAck callback with the counterparty app version. - return am.app.OnChanOpenAck(ctx, portID, channelID, cpAppVersion) + return im.app.OnChanOpenAck(ctx, portID, channelID, cpAppVersion) } // OnChanOpenConfirm implements the IBCModule interface -func (am AppModule) OnChanOpenConfirm( +func (im IBCModule) OnChanOpenConfirm( ctx sdk.Context, portID, channelID string, ) error { // call underlying app's OnChanOpenConfirm callback. - return am.app.OnChanOpenConfirm(ctx, portID, channelID) + return im.app.OnChanOpenConfirm(ctx, portID, channelID) } // OnChanCloseInit implements the IBCModule interface -func (am AppModule) OnChanCloseInit( +func (im IBCModule) OnChanCloseInit( ctx sdk.Context, portID, channelID string, ) error { // TODO: Unescrow all remaining funds for unprocessed packets - am.keeper.DeleteFeeEnabled(ctx, portID, channelID) - return am.app.OnChanCloseInit(ctx, portID, channelID) + im.keeper.DeleteFeeEnabled(ctx, portID, channelID) + return im.app.OnChanCloseInit(ctx, portID, channelID) } // OnChanCloseConfirm implements the IBCModule interface -func (am AppModule) OnChanCloseConfirm( +func (im IBCModule) OnChanCloseConfirm( ctx sdk.Context, portID, channelID string, ) error { // TODO: Unescrow all remaining funds for unprocessed packets - am.keeper.DeleteFeeEnabled(ctx, portID, channelID) - return am.app.OnChanCloseConfirm(ctx, portID, channelID) + im.keeper.DeleteFeeEnabled(ctx, portID, channelID) + return im.app.OnChanCloseConfirm(ctx, portID, channelID) } // OnRecvPacket implements the IBCModule interface. -func (am AppModule) OnRecvPacket( +func (im IBCModule) OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, ) ibcexported.Acknowledgement { // TODO: Implement fee specific logic if fee is enabled for the given channel - return am.app.OnRecvPacket(ctx, packet, relayer) + return im.app.OnRecvPacket(ctx, packet, relayer) } // OnAcknowledgementPacket implements the IBCModule interface -func (am AppModule) OnAcknowledgementPacket( +func (im IBCModule) OnAcknowledgementPacket( ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, ) error { // TODO: Implement fee specific logic if fee is enabled for the given channel - return am.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) } // OnTimeoutPacket implements the IBCModule interface -func (am AppModule) OnTimeoutPacket( +func (im IBCModule) OnTimeoutPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, ) error { // TODO: Implement fee specific logic if fee is enabled for the given channel - return am.app.OnTimeoutPacket(ctx, packet, relayer) + return im.app.OnTimeoutPacket(ctx, packet, relayer) } diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 9d3b30f238f..f9803691caa 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -302,8 +302,6 @@ func (k Keeper) ChannelOpenTry(goCtx context.Context, msg *channeltypes.MsgChann if err != nil { return nil, sdkerrors.Wrap(err, "could not retrieve module from port-id") } - fmt.Println("PORTID: ", msg.PortId) - fmt.Println("MODULE: ", module) channelID, cap, err := k.ChannelKeeper.ChanOpenTry(ctx, msg.Channel.Ordering, msg.Channel.ConnectionHops, msg.PortId, msg.PreviousChannelId, portCap, msg.Channel.Counterparty, msg.Channel.Version, msg.CounterpartyVersion, msg.ProofInit, msg.ProofHeight, diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 4b66bcbe937..2eeefec11c4 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -264,7 +264,6 @@ func NewSimApp( // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // note replicate if you do not need to test core IBC or light clients. scopedIBCMockKeeper := app.CapabilityKeeper.ScopeToModule(ibcmock.ModuleName) - app.ScopedIBCFeeKeeper = app.CapabilityKeeper.ScopeToModule(ibcfeetypes.ModuleName) // seal capability keeper after scoping modules app.CapabilityKeeper.Seal() @@ -325,7 +324,6 @@ func NewSimApp( // Create Transfer Keeper app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, - app.ScopedIBCFeeKeeper, ) app.TransferKeeper = ibctransferkeeper.NewKeeper( @@ -334,13 +332,14 @@ func NewSimApp( app.AccountKeeper, app.BankKeeper, scopedTransferKeeper, ) transferModule := transfer.NewAppModule(app.TransferKeeper) + feeTransferModule := ibcfee.NewIBCModule(app.IBCFeeKeeper, transferModule) - feeTransferModule := ibcfee.NewAppModule(app.IBCFeeKeeper, app.ScopedTransferKeeper, transferModule) + feeModule := ibcfee.NewAppModule(app.IBCFeeKeeper) // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // note replicate if you do not need to test core IBC or light clients. mockModule := ibcmock.NewAppModule(scopedIBCMockKeeper, &app.IBCKeeper.PortKeeper) - feeMockModule := ibcfee.NewAppModule(app.IBCFeeKeeper, scopedIBCMockKeeper, mockModule) + feeMockModule := ibcfee.NewIBCModule(app.IBCFeeKeeper, mockModule) // Create static IBC router, add transfer route, then set and seal it ibcRouter := porttypes.NewRouter() @@ -385,7 +384,7 @@ func NewSimApp( params.NewAppModule(app.ParamsKeeper), authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry), transferModule, - feeTransferModule, + feeModule, mockModule, ) From d6229166635cf320d8f4f4b5fe1902395df8c147 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 20 Sep 2021 18:04:05 +0200 Subject: [PATCH 11/21] split module.go file --- modules/apps/29-fee/ibc_module.go | 167 ++++++++++++++++++ .../{module_test.go => ibc_module_test.go} | 0 modules/apps/29-fee/module.go | 160 +---------------- 3 files changed, 168 insertions(+), 159 deletions(-) create mode 100644 modules/apps/29-fee/ibc_module.go rename modules/apps/29-fee/{module_test.go => ibc_module_test.go} (100%) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go new file mode 100644 index 00000000000..e4e183fedf5 --- /dev/null +++ b/modules/apps/29-fee/ibc_module.go @@ -0,0 +1,167 @@ +package fee + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" + "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" + "github.com/cosmos/ibc-go/modules/apps/29-fee/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" + porttypes "github.com/cosmos/ibc-go/modules/core/05-port/types" + ibcexported "github.com/cosmos/ibc-go/modules/core/exported" +) + +// IBCModule implements the ICS26 callbacks for the fee middleware given the fee keeper and the underlying application. +type IBCModule struct { + keeper keeper.Keeper + app porttypes.IBCModule +} + +// NewIBCModule creates a new IBCModule given the keeper and underlying application +func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule) IBCModule { + return IBCModule{ + keeper: k, + app: app, + } +} + +// OnChanOpenInit implements the IBCModule interface +func (im IBCModule) OnChanOpenInit( + ctx sdk.Context, + order channeltypes.Order, + connectionHops []string, + portID string, + channelID string, + chanCap *capabilitytypes.Capability, + counterparty channeltypes.Counterparty, + version string, +) error { + feeVersion, appVersion := channeltypes.SplitChannelVersion(version) + if feeVersion != "" { + if feeVersion != types.Version { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) + } + + im.keeper.SetFeeEnabled(ctx, portID, channelID) + } + + // call underlying app's OnChanOpenInit callback with the appVersion + return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, + chanCap, counterparty, appVersion) +} + +// OnChanOpenTry implements the IBCModule interface +func (im IBCModule) OnChanOpenTry( + ctx sdk.Context, + order channeltypes.Order, + connectionHops []string, + portID, + channelID string, + chanCap *capabilitytypes.Capability, + counterparty channeltypes.Counterparty, + version, + counterpartyVersion string, +) error { + feeVersion, appVersion := channeltypes.SplitChannelVersion(version) + cpFeeVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) + + if feeVersion != "" || cpFeeVersion != "" { + if feeVersion != types.Version { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) + } + if cpFeeVersion != feeVersion { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) + } + + im.keeper.SetFeeEnabled(ctx, portID, channelID) + } else if im.keeper.IsFeeEnabled(ctx, portID, channelID) { + // return error if a previous ChanInit set fee enabled but subsequent OpenTry does not have fee enabled + return sdkerrors.Wrapf(types.ErrInvalidVersion, "previous INIT call (crossing hellos) had fee version set but OpenTry call does not set fee version: %s", version) + } + // call underlying app's OnChanOpenTry callback with the app versions + return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, + chanCap, counterparty, appVersion, cpAppVersion) +} + +// OnChanOpenAck implements the IBCModule interface +func (im IBCModule) OnChanOpenAck( + ctx sdk.Context, + portID, + channelID string, + counterpartyVersion string, +) error { + cpAppVersion := counterpartyVersion + if im.keeper.IsFeeEnabled(ctx, portID, channelID) { + var cpFeeVersion string + cpFeeVersion, cpAppVersion = channeltypes.SplitChannelVersion(counterpartyVersion) + + if cpFeeVersion != types.Version { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) + } + } + // call underlying app's OnChanOpenAck callback with the counterparty app version. + return im.app.OnChanOpenAck(ctx, portID, channelID, cpAppVersion) +} + +// OnChanOpenConfirm implements the IBCModule interface +func (im IBCModule) OnChanOpenConfirm( + ctx sdk.Context, + portID, + channelID string, +) error { + // call underlying app's OnChanOpenConfirm callback. + return im.app.OnChanOpenConfirm(ctx, portID, channelID) +} + +// OnChanCloseInit implements the IBCModule interface +func (im IBCModule) OnChanCloseInit( + ctx sdk.Context, + portID, + channelID string, +) error { + // TODO: Unescrow all remaining funds for unprocessed packets + im.keeper.DeleteFeeEnabled(ctx, portID, channelID) + return im.app.OnChanCloseInit(ctx, portID, channelID) +} + +// OnChanCloseConfirm implements the IBCModule interface +func (im IBCModule) OnChanCloseConfirm( + ctx sdk.Context, + portID, + channelID string, +) error { + // TODO: Unescrow all remaining funds for unprocessed packets + im.keeper.DeleteFeeEnabled(ctx, portID, channelID) + return im.app.OnChanCloseConfirm(ctx, portID, channelID) +} + +// OnRecvPacket implements the IBCModule interface. +func (im IBCModule) OnRecvPacket( + ctx sdk.Context, + packet channeltypes.Packet, + relayer sdk.AccAddress, +) ibcexported.Acknowledgement { + // TODO: Implement fee specific logic if fee is enabled for the given channel + return im.app.OnRecvPacket(ctx, packet, relayer) +} + +// OnAcknowledgementPacket implements the IBCModule interface +func (im IBCModule) OnAcknowledgementPacket( + ctx sdk.Context, + packet channeltypes.Packet, + acknowledgement []byte, + relayer sdk.AccAddress, +) error { + // TODO: Implement fee specific logic if fee is enabled for the given channel + return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) +} + +// OnTimeoutPacket implements the IBCModule interface +func (im IBCModule) OnTimeoutPacket( + ctx sdk.Context, + packet channeltypes.Packet, + relayer sdk.AccAddress, +) error { + // TODO: Implement fee specific logic if fee is enabled for the given channel + return im.app.OnTimeoutPacket(ctx, packet, relayer) +} diff --git a/modules/apps/29-fee/module_test.go b/modules/apps/29-fee/ibc_module_test.go similarity index 100% rename from modules/apps/29-fee/module_test.go rename to modules/apps/29-fee/ibc_module_test.go diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index 59292500a64..d974ba08df7 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -9,10 +9,8 @@ import ( "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" - capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" "github.com/gorilla/mux" "github.com/grpc-ecosystem/grpc-gateway/runtime" "github.com/spf13/cobra" @@ -24,9 +22,8 @@ import ( // "github.com/cosmos/ibc-go/modules/apps/29-fee/client/cli" // "github.com/cosmos/ibc-go/modules/apps/29-fee/simulation" - channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" + porttypes "github.com/cosmos/ibc-go/modules/core/05-port/types" - ibcexported "github.com/cosmos/ibc-go/modules/core/exported" ) var ( @@ -183,158 +180,3 @@ func (am AppModule) RegisterStoreDecoder(sdr sdk.StoreDecoderRegistry) { func (am AppModule) WeightedOperations(_ module.SimulationState) []simtypes.WeightedOperation { return nil } - -// IBCModule implements the ICS26 callbacks for the fee middleware given the fee keeper and the underlying application. -type IBCModule struct { - keeper keeper.Keeper - app porttypes.IBCModule -} - -// NewIBCModule creates a new IBCModule given the keeper and underlying application -func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule) IBCModule { - return IBCModule{ - keeper: k, - app: app, - } -} - -// OnChanOpenInit implements the IBCModule interface -func (im IBCModule) OnChanOpenInit( - ctx sdk.Context, - order channeltypes.Order, - connectionHops []string, - portID string, - channelID string, - chanCap *capabilitytypes.Capability, - counterparty channeltypes.Counterparty, - version string, -) error { - feeVersion, appVersion := channeltypes.SplitChannelVersion(version) - if feeVersion != "" { - if feeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) - } - - im.keeper.SetFeeEnabled(ctx, portID, channelID) - } - - // call underlying app's OnChanOpenInit callback with the appVersion - return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, - chanCap, counterparty, appVersion) -} - -// OnChanOpenTry implements the IBCModule interface -func (im IBCModule) OnChanOpenTry( - ctx sdk.Context, - order channeltypes.Order, - connectionHops []string, - portID, - channelID string, - chanCap *capabilitytypes.Capability, - counterparty channeltypes.Counterparty, - version, - counterpartyVersion string, -) error { - feeVersion, appVersion := channeltypes.SplitChannelVersion(version) - cpFeeVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) - - if feeVersion != "" || cpFeeVersion != "" { - if feeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) - } - if cpFeeVersion != feeVersion { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) - } - - im.keeper.SetFeeEnabled(ctx, portID, channelID) - } else if im.keeper.IsFeeEnabled(ctx, portID, channelID) { - // return error if a previous ChanInit set fee enabled but subsequent OpenTry does not have fee enabled - return sdkerrors.Wrapf(types.ErrInvalidVersion, "previous INIT call (crossing hellos) had fee version set but OpenTry call does not set fee version: %s", version) - } - // call underlying app's OnChanOpenTry callback with the app versions - return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, - chanCap, counterparty, appVersion, cpAppVersion) -} - -// OnChanOpenAck implements the IBCModule interface -func (im IBCModule) OnChanOpenAck( - ctx sdk.Context, - portID, - channelID string, - counterpartyVersion string, -) error { - cpAppVersion := counterpartyVersion - if im.keeper.IsFeeEnabled(ctx, portID, channelID) { - var cpFeeVersion string - cpFeeVersion, cpAppVersion = channeltypes.SplitChannelVersion(counterpartyVersion) - - if cpFeeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) - } - } - // call underlying app's OnChanOpenAck callback with the counterparty app version. - return im.app.OnChanOpenAck(ctx, portID, channelID, cpAppVersion) -} - -// OnChanOpenConfirm implements the IBCModule interface -func (im IBCModule) OnChanOpenConfirm( - ctx sdk.Context, - portID, - channelID string, -) error { - // call underlying app's OnChanOpenConfirm callback. - return im.app.OnChanOpenConfirm(ctx, portID, channelID) -} - -// OnChanCloseInit implements the IBCModule interface -func (im IBCModule) OnChanCloseInit( - ctx sdk.Context, - portID, - channelID string, -) error { - // TODO: Unescrow all remaining funds for unprocessed packets - im.keeper.DeleteFeeEnabled(ctx, portID, channelID) - return im.app.OnChanCloseInit(ctx, portID, channelID) -} - -// OnChanCloseConfirm implements the IBCModule interface -func (im IBCModule) OnChanCloseConfirm( - ctx sdk.Context, - portID, - channelID string, -) error { - // TODO: Unescrow all remaining funds for unprocessed packets - im.keeper.DeleteFeeEnabled(ctx, portID, channelID) - return im.app.OnChanCloseConfirm(ctx, portID, channelID) -} - -// OnRecvPacket implements the IBCModule interface. -func (im IBCModule) OnRecvPacket( - ctx sdk.Context, - packet channeltypes.Packet, - relayer sdk.AccAddress, -) ibcexported.Acknowledgement { - // TODO: Implement fee specific logic if fee is enabled for the given channel - return im.app.OnRecvPacket(ctx, packet, relayer) -} - -// OnAcknowledgementPacket implements the IBCModule interface -func (im IBCModule) OnAcknowledgementPacket( - ctx sdk.Context, - packet channeltypes.Packet, - acknowledgement []byte, - relayer sdk.AccAddress, -) error { - // TODO: Implement fee specific logic if fee is enabled for the given channel - return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) -} - -// OnTimeoutPacket implements the IBCModule interface -func (im IBCModule) OnTimeoutPacket( - ctx sdk.Context, - packet channeltypes.Packet, - relayer sdk.AccAddress, -) error { - // TODO: Implement fee specific logic if fee is enabled for the given channel - return im.app.OnTimeoutPacket(ctx, packet, relayer) -} From 094fef933fea960f7f42a33becf24dc699ce4142 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 20 Sep 2021 18:12:52 +0200 Subject: [PATCH 12/21] cleanup and docs --- modules/core/keeper/msg_server.go | 2 -- testing/simapp/app.go | 9 +++++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index f9803691caa..69bde98b687 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -2,7 +2,6 @@ package keeper import ( "context" - "fmt" "github.com/armon/go-metrics" @@ -485,7 +484,6 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke if err != nil { return nil, sdkerrors.Wrap(err, "could not retrieve module from port-id") } - fmt.Println("MODULE: ", module) // Retrieve callbacks from router cbs, ok := k.Router.GetRoute(module) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 2eeefec11c4..83cdf6ff2a7 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -321,27 +321,32 @@ func NewSimApp( &stakingKeeper, govRouter, ) - // Create Transfer Keeper app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, ) + // Create Transfer Keeper and pass IBCFeeKeeper as expected Channel and PortKeeper + // since fee middleware will wrap the IBCKeeper for underlying application. app.TransferKeeper = ibctransferkeeper.NewKeeper( appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName), app.IBCFeeKeeper, &app.IBCFeeKeeper, app.AccountKeeper, app.BankKeeper, scopedTransferKeeper, ) transferModule := transfer.NewAppModule(app.TransferKeeper) + // create fee-wrapped transfer module feeTransferModule := ibcfee.NewIBCModule(app.IBCFeeKeeper, transferModule) feeModule := ibcfee.NewAppModule(app.IBCFeeKeeper) // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // note replicate if you do not need to test core IBC or light clients. - mockModule := ibcmock.NewAppModule(scopedIBCMockKeeper, &app.IBCKeeper.PortKeeper) + // Pass IBCFeeKeeper for PortKeeper since fee middleware will wrap the IBCKeeper for underlying application. + mockModule := ibcmock.NewAppModule(scopedIBCMockKeeper, &app.IBCFeeKeeper) + // create fee wrapped mock module feeMockModule := ibcfee.NewIBCModule(app.IBCFeeKeeper, mockModule) // Create static IBC router, add transfer route, then set and seal it + // pass in top-level (fully-wrapped) IBCModules to IBC Router ibcRouter := porttypes.NewRouter() ibcRouter.AddRoute(ibctransfertypes.ModuleName, feeTransferModule) ibcRouter.AddRoute(ibcmock.ModuleName, feeMockModule) From 763159a00a1f4ea80a46c3cf0c15c333705d301a Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 20 Sep 2021 18:24:09 +0200 Subject: [PATCH 13/21] assert IBC interfaces are fulfilled in middleware --- modules/apps/29-fee/keeper/keeper.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index 7be0e42808e..b72b1ac3ae8 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -13,6 +13,13 @@ import ( ibcexported "github.com/cosmos/ibc-go/modules/core/exported" ) +// Middleware must implement types.ChannelKeeper and types.PortKeeper expected interfaces +// so that it can wrap IBC channel and port logic for underlying application. +var ( + _ types.ChannelKeeper = Keeper{} + _ types.PortKeeper = Keeper{} +) + // Keeper defines the IBC fungible transfer keeper type Keeper struct { storeKey sdk.StoreKey From f60897b137a535b2f760b328e570ad06a6399c2c Mon Sep 17 00:00:00 2001 From: Aditya Date: Mon, 20 Sep 2021 18:27:29 +0200 Subject: [PATCH 14/21] Update modules/apps/transfer/module.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- modules/apps/transfer/module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go index 2661681f577..90e8456381a 100644 --- a/modules/apps/transfer/module.go +++ b/modules/apps/transfer/module.go @@ -215,7 +215,7 @@ func ValidateTransferChannelParams( } if portID != counterparty.PortId { - return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "portID: %s do not match counterparty portID: %s", portID, counterparty.PortId) + return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "portID: %s does not match counterparty portID: %s", portID, counterparty.PortId) } if version != types.Version { From bc90365eed904add94eee0f9c61ce126cf41b762 Mon Sep 17 00:00:00 2001 From: Aditya Date: Thu, 23 Sep 2021 10:56:32 +0200 Subject: [PATCH 15/21] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- modules/apps/29-fee/ibc_module.go | 1 + modules/apps/29-fee/module.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index e4e183fedf5..65f9f9e91e5 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -4,6 +4,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" + "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index d974ba08df7..6e73a3b9b08 100644 --- a/modules/apps/29-fee/module.go +++ b/modules/apps/29-fee/module.go @@ -70,7 +70,7 @@ func (AppModuleBasic) ValidateGenesis(cdc codec.JSONCodec, config client.TxEncod func (AppModuleBasic) RegisterRESTRoutes(clientCtx client.Context, rtr *mux.Router) { } -// RegisterGRPCGatewayRoutes registers the gRPC Gateway routes for the ibc-29-fee module. +// RegisterGRPCGatewayRoutes registers the gRPC Gateway routes for ics29 fee module. func (AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *runtime.ServeMux) { types.RegisterQueryHandlerClient(context.Background(), mux, types.NewQueryClient(clientCtx)) } From b9028a26aafd866c91d6e03ad21a0027e4d92d0e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 24 Sep 2021 15:34:05 +0200 Subject: [PATCH 16/21] fix unnecessary crossing hello logic --- modules/apps/29-fee/ibc_module.go | 3 -- .../{module_test.go => ibc_module_test.go} | 53 ++++++++----------- modules/core/04-channel/keeper/handshake.go | 3 +- 3 files changed, 24 insertions(+), 35 deletions(-) rename modules/apps/29-fee/{module_test.go => ibc_module_test.go} (88%) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index e4e183fedf5..d36d163e974 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -74,9 +74,6 @@ func (im IBCModule) OnChanOpenTry( } im.keeper.SetFeeEnabled(ctx, portID, channelID) - } else if im.keeper.IsFeeEnabled(ctx, portID, channelID) { - // return error if a previous ChanInit set fee enabled but subsequent OpenTry does not have fee enabled - return sdkerrors.Wrapf(types.ErrInvalidVersion, "previous INIT call (crossing hellos) had fee version set but OpenTry call does not set fee version: %s", version) } // call underlying app's OnChanOpenTry callback with the app versions return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, diff --git a/modules/apps/29-fee/module_test.go b/modules/apps/29-fee/ibc_module_test.go similarity index 88% rename from modules/apps/29-fee/module_test.go rename to modules/apps/29-fee/ibc_module_test.go index 56637d28a51..3ed04ea4b25 100644 --- a/modules/apps/29-fee/module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -92,88 +92,87 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { } func (suite *FeeTestSuite) TestOnChanOpenTry() { + var ( + chanCap *capabilitytypes.Capability + ok bool + err error + ) + testCases := []struct { name string version string cpVersion string - capExists bool + malleate func(suite *FeeTestSuite) expPass bool }{ { "valid fee middleware and transfer version", "fee29-1:ics20-1", "fee29-1:ics20-1", - false, + func(suite *FeeTestSuite) {}, true, }, { "valid transfer version on try and counterparty", "ics20-1", "ics20-1", - false, - true, - }, - { - "valid fee middleware and transfer version, crossing hellos", - "fee29-1:ics20-1", - "fee29-1:ics20-1", - true, + func(suite *FeeTestSuite) {}, true, }, { "invalid fee middleware version", "otherfee28-1:ics20-1", "fee29-1:ics20-1", - false, + func(suite *FeeTestSuite) {}, false, }, { "invalid counterparty fee middleware version", "fee29-1:ics20-1", "wrongfee29-1:ics20-1", - false, + func(suite *FeeTestSuite) {}, false, }, { "invalid transfer version", "fee29-1:wrongics20-1", "fee29-1:ics20-1", - false, + func(suite *FeeTestSuite) {}, false, }, { "invalid counterparty transfer version", "fee29-1:ics20-1", "fee29-1:wrongics20-1", - false, + func(suite *FeeTestSuite) {}, false, }, { "transfer version not wrapped", "fee29-1", "fee29-1:ics20-1", - false, + func(suite *FeeTestSuite) {}, false, }, { "counterparty transfer version not wrapped", "fee29-1:ics20-1", "fee29-1", - false, + func(suite *FeeTestSuite) {}, false, }, { "fee version not included on try, but included in counterparty", "ics20-1", "fee29-1:ics20-1", - false, + func(suite *FeeTestSuite) {}, false, }, { "transfer version not included", "fee29-1:ics20-1", "ics20-1", - false, + func(suite *FeeTestSuite) {}, false, }, } @@ -188,19 +187,11 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { suite.coordinator.SetupConnections(suite.path) suite.path.EndpointB.ChanOpenInit() - var ( - chanCap *capabilitytypes.Capability - ok bool - err error - ) - if tc.capExists { - suite.path.EndpointA.ChanOpenInit() - chanCap, ok = suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, suite.path.EndpointA.ChannelID)) - suite.Require().True(ok) - } else { - chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, suite.path.EndpointA.ChannelID)) - suite.Require().NoError(err) - } + chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, suite.path.EndpointA.ChannelID)) + suite.Require().NoError(err) + + // malleate test case + tc.malleate(suite) suite.path.EndpointA.ChannelID = ibctesting.FirstChannelID diff --git a/modules/core/04-channel/keeper/handshake.go b/modules/core/04-channel/keeper/handshake.go index 5f9d70ad090..adad943b81c 100644 --- a/modules/core/04-channel/keeper/handshake.go +++ b/modules/core/04-channel/keeper/handshake.go @@ -127,7 +127,8 @@ func (k Keeper) ChanOpenTry( channelID := previousChannelID - // empty channel identifier indicates continuing a previous channel handshake + // non-empty channel identifier indicates continuing a previous channel handshake + // where ChanOpenINIT has already been called on the executing chain. if previousChannelID != "" { // channel identifier and connection hop length checked on msg.ValidateBasic() // ensure that the previous channel exists From d37a7f769859eb8792dae2695abbee46cafa92a5 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 24 Sep 2021 16:16:17 +0200 Subject: [PATCH 17/21] fix version negotiation bugs and improve tests --- modules/apps/29-fee/fee_test.go | 3 +- modules/apps/29-fee/ibc_module.go | 41 +++++++++----- modules/apps/29-fee/ibc_module_test.go | 77 ++++++++++++++++++-------- modules/apps/29-fee/keeper/keeper.go | 11 +++- modules/apps/29-fee/types/keys.go | 7 ++- 5 files changed, 95 insertions(+), 44 deletions(-) diff --git a/modules/apps/29-fee/fee_test.go b/modules/apps/29-fee/fee_test.go index 71d435472a5..9ecb400f710 100644 --- a/modules/apps/29-fee/fee_test.go +++ b/modules/apps/29-fee/fee_test.go @@ -3,11 +3,12 @@ package fee_test import ( "testing" + "github.com/stretchr/testify/suite" + "github.com/cosmos/ibc-go/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/testing" - "github.com/stretchr/testify/suite" ) type FeeTestSuite struct { diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index d36d163e974..752083f9a6a 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -36,13 +36,17 @@ func (im IBCModule) OnChanOpenInit( counterparty channeltypes.Counterparty, version string, ) error { - feeVersion, appVersion := channeltypes.SplitChannelVersion(version) - if feeVersion != "" { - if feeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) - } - + mwVersion, appVersion := channeltypes.SplitChannelVersion(version) + // Since it is valid for fee version to not be specified, the above middleware version may be for a middleware + // lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying + // application. + // If an invalid fee version was passed, we expect the underlying application to fail on its version negotiation. + if mwVersion == types.Version { im.keeper.SetFeeEnabled(ctx, portID, channelID) + } else { + // middleware version is not the expected version for this midddleware. Pass the full version string along, + // if it not valid version for any other lower middleware, an error will be returned by base application. + appVersion = version } // call underlying app's OnChanOpenInit callback with the appVersion @@ -62,19 +66,26 @@ func (im IBCModule) OnChanOpenTry( version, counterpartyVersion string, ) error { - feeVersion, appVersion := channeltypes.SplitChannelVersion(version) - cpFeeVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) - - if feeVersion != "" || cpFeeVersion != "" { - if feeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected: %s, got: %s", types.Version, feeVersion) - } - if cpFeeVersion != feeVersion { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, cpFeeVersion) + mwVersion, appVersion := channeltypes.SplitChannelVersion(version) + cpMwVersion, cpAppVersion := channeltypes.SplitChannelVersion(counterpartyVersion) + + // Since it is valid for fee version to not be specified, the above middleware version may be for a middleware + // lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying + // application. + // If an invalid fee version was passed, we expect the underlying application to fail on its version negotiation. + if mwVersion == types.Version || cpMwVersion == types.Version { + if cpMwVersion != mwVersion { + return sdkerrors.Wrapf(types.ErrInvalidVersion, "fee versions do not match. self version: %s, counterparty version: %s", mwVersion, cpMwVersion) } im.keeper.SetFeeEnabled(ctx, portID, channelID) + } else { + // middleware versions are not the expected version for this midddleware. Pass the full version strings along, + // if it not valid version for any other lower middleware, an error will be returned by base application. + appVersion = version + cpAppVersion = counterpartyVersion } + // call underlying app's OnChanOpenTry callback with the app versions return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, appVersion, cpAppVersion) diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index 3ed04ea4b25..825cc06cdaf 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -7,6 +7,7 @@ import ( ibctesting "github.com/cosmos/ibc-go/testing" ) +// Tests OnChanOpenInit on ChainA func (suite *FeeTestSuite) TestOnChanOpenInit() { testCases := []struct { name string @@ -91,88 +92,90 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { } } +// Tests OnChanOpenTry on ChainA func (suite *FeeTestSuite) TestOnChanOpenTry() { - var ( - chanCap *capabilitytypes.Capability - ok bool - err error - ) - testCases := []struct { name string version string cpVersion string - malleate func(suite *FeeTestSuite) + crossing bool expPass bool }{ { "valid fee middleware and transfer version", "fee29-1:ics20-1", "fee29-1:ics20-1", - func(suite *FeeTestSuite) {}, + false, true, }, { "valid transfer version on try and counterparty", "ics20-1", "ics20-1", - func(suite *FeeTestSuite) {}, + false, + true, + }, + { + "valid fee middleware and transfer version, crossing hellos", + "fee29-1:ics20-1", + "fee29-1:ics20-1", + true, true, }, { "invalid fee middleware version", "otherfee28-1:ics20-1", "fee29-1:ics20-1", - func(suite *FeeTestSuite) {}, + false, false, }, { "invalid counterparty fee middleware version", "fee29-1:ics20-1", "wrongfee29-1:ics20-1", - func(suite *FeeTestSuite) {}, + false, false, }, { "invalid transfer version", "fee29-1:wrongics20-1", "fee29-1:ics20-1", - func(suite *FeeTestSuite) {}, + false, false, }, { "invalid counterparty transfer version", "fee29-1:ics20-1", "fee29-1:wrongics20-1", - func(suite *FeeTestSuite) {}, + false, false, }, { "transfer version not wrapped", "fee29-1", "fee29-1:ics20-1", - func(suite *FeeTestSuite) {}, + false, false, }, { "counterparty transfer version not wrapped", "fee29-1:ics20-1", "fee29-1", - func(suite *FeeTestSuite) {}, + false, false, }, { "fee version not included on try, but included in counterparty", "ics20-1", "fee29-1:ics20-1", - func(suite *FeeTestSuite) {}, + false, false, }, { - "transfer version not included", + "fee version not included", "fee29-1:ics20-1", "ics20-1", - func(suite *FeeTestSuite) {}, + false, false, }, } @@ -183,15 +186,22 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { suite.Run(tc.name, func() { // reset suite suite.SetupTest() - suite.coordinator.SetupClients(suite.path) suite.coordinator.SetupConnections(suite.path) suite.path.EndpointB.ChanOpenInit() - chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, suite.path.EndpointA.ChannelID)) - suite.Require().NoError(err) - - // malleate test case - tc.malleate(suite) + var ( + chanCap *capabilitytypes.Capability + ok bool + err error + ) + if tc.crossing { + suite.path.EndpointA.ChanOpenInit() + chanCap, ok = suite.chainA.GetSimApp().ScopedTransferKeeper.GetCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, suite.path.EndpointA.ChannelID)) + suite.Require().True(ok) + } else { + chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, suite.path.EndpointA.ChannelID)) + suite.Require().NoError(err) + } suite.path.EndpointA.ChannelID = ibctesting.FirstChannelID @@ -222,25 +232,41 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { } } +// Tests OnChanOpenAck on ChainA func (suite *FeeTestSuite) TestOnChanOpenAck() { testCases := []struct { name string cpVersion string + malleate func(suite *FeeTestSuite) expPass bool }{ { "success", "fee29-1:ics20-1", + func(suite *FeeTestSuite) {}, true, }, { "invalid fee version", "fee29-3:ics20-1", + func(suite *FeeTestSuite) {}, false, }, { "invalid transfer version", "fee29-1:ics20-4", + func(suite *FeeTestSuite) {}, + false, + }, + { + "previous INIT set without fee, however counterparty set fee version", // note this can only happen with incompetent or malicious counterparty chain + "fee29-1:ics20-1", + func(suite *FeeTestSuite) { + // do the first steps without fee version, then pass the fee version as counterparty version + // in ChanOpenACK + suite.path.EndpointA.ChannelConfig.Version = "ics20-1" + suite.path.EndpointB.ChannelConfig.Version = "ics20-1" + }, false, }, } @@ -252,6 +278,9 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { suite.coordinator.SetupClients(suite.path) suite.coordinator.SetupConnections(suite.path) + // malleate test case + tc.malleate(suite) + suite.path.EndpointA.ChanOpenInit() suite.path.EndpointB.ChanOpenTry() diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index 1d79a79a996..2841c2464d2 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -5,12 +5,14 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" - "github.com/cosmos/ibc-go/modules/apps/29-fee/types" - channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" - "github.com/tendermint/tendermint/libs/log" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/modules/core/24-host" ibcexported "github.com/cosmos/ibc-go/modules/core/exported" + + "github.com/tendermint/tendermint/libs/log" + + "github.com/cosmos/ibc-go/modules/apps/29-fee/types" ) // Middleware must implement types.ChannelKeeper and types.PortKeeper expected interfaces @@ -59,14 +61,17 @@ func (k Keeper) ChanCloseInit(ctx sdk.Context, portID, channelID string, chanCap return k.channelKeeper.ChanCloseInit(ctx, portID, channelID, chanCap) } +// GetChannel wraps IBC ChannelKeeper's GetChannel function func (k Keeper) GetChannel(ctx sdk.Context, portID, channelID string) (channeltypes.Channel, bool) { return k.channelKeeper.GetChannel(ctx, portID, channelID) } +// GetNextSequenceSend wraps IBC ChannelKeeper's GetNextSequenceSend function func (k Keeper) GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool) { return k.channelKeeper.GetNextSequenceSend(ctx, portID, channelID) } +// SendPacket wraps IBC ChannelKeeper's SendPacket function func (k Keeper) SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI) error { return k.channelKeeper.SendPacket(ctx, chanCap, packet) } diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index 96ec8a75160..d270ab9d7e9 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -20,12 +20,17 @@ const ( Version = "fee29-1" + // FeeEnabledPrefix is the key prefix for storing fee enabled flag + FeeEnabledKeyPrefix = "fee_enabled" + // RelayerAddressKeyPrefix is the key prefix for relayer address mapping RelayerAddressKeyPrefix = "relayerAddress" ) +// FeeEnabledKey returns the key that stores a flag to determine if fee logic should +// be enabled for the given port and channel identifiers. func FeeEnabledKey(portID, channelID string) []byte { - return []byte(fmt.Sprintf("fee_enabled/%s/%s", portID, channelID)) + return []byte(fmt.Sprintf("%s/%s/%s", FeeEnabledKeyPrefix, portID, channelID)) } // KeyRelayerAddress returns the key for relayer address -> counteryparty address mapping From 131e95a8ef0e38bcec247d058812989e17ba8d1a Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 24 Sep 2021 16:30:24 +0200 Subject: [PATCH 18/21] cleanup tests --- modules/apps/29-fee/ibc_module_test.go | 78 ++++++++++--------- modules/core/04-channel/types/version_test.go | 34 ++++++++ 2 files changed, 75 insertions(+), 37 deletions(-) diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index 825cc06cdaf..c3b649014f1 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -1,7 +1,12 @@ package fee_test import ( + "fmt" + capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" + + "github.com/cosmos/ibc-go/modules/apps/29-fee/types" + transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/modules/core/24-host" ibctesting "github.com/cosmos/ibc-go/testing" @@ -16,37 +21,37 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { }{ { "valid fee middleware and transfer version", - "fee29-1:ics20-1", + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), true, }, { "fee version not included, only perform transfer logic", - "ics20-1", + transfertypes.Version, true, }, { "invalid fee middleware version", - "otherfee28-1:ics20-1", + channeltypes.MergeChannelVersions("otherfee28-1", transfertypes.Version), false, }, { "invalid transfer version", - "fee29-1:wrongics20-1", + channeltypes.MergeChannelVersions(types.Version, "wrongics20-1"), false, }, { "incorrect wrapping delimiter", - "fee29-1//ics20-1", + fmt.Sprintf("%s//%s", types.Version, transfertypes.Version), false, }, { "transfer version not wrapped", - "fee29-1", + types.Version, false, }, { "hanging delimiter", - "fee29-1:ics20-1:", + fmt.Sprintf("%s:%s:", types.Version, transfertypes.Version), false, }, } @@ -103,78 +108,78 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { }{ { "valid fee middleware and transfer version", - "fee29-1:ics20-1", - "fee29-1:ics20-1", + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), false, true, }, { "valid transfer version on try and counterparty", - "ics20-1", - "ics20-1", + transfertypes.Version, + transfertypes.Version, false, true, }, { "valid fee middleware and transfer version, crossing hellos", - "fee29-1:ics20-1", - "fee29-1:ics20-1", + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), true, true, }, { "invalid fee middleware version", - "otherfee28-1:ics20-1", - "fee29-1:ics20-1", + channeltypes.MergeChannelVersions("otherfee28-1", transfertypes.Version), + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), false, false, }, { "invalid counterparty fee middleware version", - "fee29-1:ics20-1", - "wrongfee29-1:ics20-1", + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), + channeltypes.MergeChannelVersions("wrongfee29-1", transfertypes.Version), false, false, }, { "invalid transfer version", - "fee29-1:wrongics20-1", - "fee29-1:ics20-1", + channeltypes.MergeChannelVersions(types.Version, "wrongics20-1"), + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), false, false, }, { "invalid counterparty transfer version", - "fee29-1:ics20-1", - "fee29-1:wrongics20-1", + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), + channeltypes.MergeChannelVersions(types.Version, "wrongics20-1"), false, false, }, { "transfer version not wrapped", - "fee29-1", - "fee29-1:ics20-1", + types.Version, + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), false, false, }, { "counterparty transfer version not wrapped", - "fee29-1:ics20-1", - "fee29-1", + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), + types.Version, false, false, }, { "fee version not included on try, but included in counterparty", - "ics20-1", - "fee29-1:ics20-1", + transfertypes.Version, + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), false, false, }, { "fee version not included", - "fee29-1:ics20-1", - "ics20-1", + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), + transfertypes.Version, false, false, }, @@ -242,30 +247,29 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { }{ { "success", - "fee29-1:ics20-1", + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), func(suite *FeeTestSuite) {}, true, }, { "invalid fee version", - "fee29-3:ics20-1", + channeltypes.MergeChannelVersions("fee29-A", transfertypes.Version), func(suite *FeeTestSuite) {}, false, }, { "invalid transfer version", - "fee29-1:ics20-4", + channeltypes.MergeChannelVersions(types.Version, "ics20-4"), func(suite *FeeTestSuite) {}, false, }, { "previous INIT set without fee, however counterparty set fee version", // note this can only happen with incompetent or malicious counterparty chain - "fee29-1:ics20-1", + channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), func(suite *FeeTestSuite) { - // do the first steps without fee version, then pass the fee version as counterparty version - // in ChanOpenACK - suite.path.EndpointA.ChannelConfig.Version = "ics20-1" - suite.path.EndpointB.ChannelConfig.Version = "ics20-1" + // do the first steps without fee version, then pass the fee version as counterparty version in ChanOpenACK + suite.path.EndpointA.ChannelConfig.Version = transfertypes.Version + suite.path.EndpointB.ChannelConfig.Version = transfertypes.Version }, false, }, diff --git a/modules/core/04-channel/types/version_test.go b/modules/core/04-channel/types/version_test.go index 13f86a96528..fda5a570f2f 100644 --- a/modules/core/04-channel/types/version_test.go +++ b/modules/core/04-channel/types/version_test.go @@ -41,3 +41,37 @@ func TestSplitVersions(t *testing.T) { require.Equal(t, tc.appVersion, appVersion, "app version is unexpected for case: %s", tc.name) } } + +func TestMergeVersions(t *testing.T) { + testCases := []struct { + name string + versions []string + merged string + }{ + { + "single version", + []string{"ics20-1"}, + "ics20-1", + }, + { + "empty version", + []string{}, + "", + }, + { + "two versions", + []string{"fee29-1", "ics20-1"}, + "fee29-1:ics20-1", + }, + { + "multiple versions", + []string{"fee29-1", "whitelist", "ics20-1"}, + "fee29-1:whitelist:ics20-1", + }, + } + + for _, tc := range testCases { + actual := types.MergeChannelVersions(tc.versions...) + require.Equal(t, tc.merged, actual, "merged versions string does not equal expected value") + } +} From 40783bf8882bd6a4d508c7223253df261683093a Mon Sep 17 00:00:00 2001 From: Aditya Date: Fri, 24 Sep 2021 16:55:24 +0200 Subject: [PATCH 19/21] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- modules/apps/29-fee/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index 2841c2464d2..7c71cc4d5e9 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -89,7 +89,7 @@ func (k Keeper) DeleteFeeEnabled(ctx sdk.Context, portID, channelID string) { store.Delete(types.FeeEnabledKey(portID, channelID)) } -// IsFeeEnabled returns whether fee handling logic should be run for the given port by checking the +// IsFeeEnabled returns whether fee handling logic should be run for the given port. It will check the // fee enabled flag for the given port and channel identifiers func (k Keeper) IsFeeEnabled(ctx sdk.Context, portID, channelID string) bool { store := ctx.KVStore(k.storeKey) From 24d2a20167a5b06c3690ab829a57ee366e4d83c8 Mon Sep 17 00:00:00 2001 From: Aditya Date: Mon, 27 Sep 2021 14:15:03 +0200 Subject: [PATCH 20/21] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- modules/apps/29-fee/ibc_module_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index c3b649014f1..915fdaebf79 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -62,9 +62,7 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { suite.Run(tc.name, func() { // reset suite suite.SetupTest() - suite.coordinator.SetupClients(suite.path) suite.coordinator.SetupConnections(suite.path) - suite.path.EndpointA.ChannelID = ibctesting.FirstChannelID counterparty := channeltypes.NewCounterparty(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID) @@ -279,7 +277,6 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { tc := tc suite.Run(tc.name, func() { suite.SetupTest() - suite.coordinator.SetupClients(suite.path) suite.coordinator.SetupConnections(suite.path) // malleate test case From 6b4501539b2b8a2d323f451ac70c612f1d58ec01 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 27 Sep 2021 14:29:22 +0200 Subject: [PATCH 21/21] address rest of colin comments --- modules/apps/29-fee/ibc_module.go | 4 +++- modules/apps/29-fee/keeper/keeper.go | 7 +++---- modules/apps/29-fee/types/keys.go | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index 6afd0a1085d..0c22fb19d8d 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -4,7 +4,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" - + "github.com/cosmos/ibc-go/modules/apps/29-fee/keeper" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" @@ -99,6 +99,8 @@ func (im IBCModule) OnChanOpenAck( channelID string, counterpartyVersion string, ) error { + // If handshake was initialized with fee enabled it must complete with fee enabled. + // If handshake was initialized with fee disabled it must complete with fee disabled. cpAppVersion := counterpartyVersion if im.keeper.IsFeeEnabled(ctx, portID, channelID) { var cpFeeVersion string diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index 2841c2464d2..aa2c2cdba5a 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -6,13 +6,12 @@ import ( capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" - channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" - host "github.com/cosmos/ibc-go/modules/core/24-host" - ibcexported "github.com/cosmos/ibc-go/modules/core/exported" - "github.com/tendermint/tendermint/libs/log" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" + host "github.com/cosmos/ibc-go/modules/core/24-host" + ibcexported "github.com/cosmos/ibc-go/modules/core/exported" ) // Middleware must implement types.ChannelKeeper and types.PortKeeper expected interfaces diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index d270ab9d7e9..39bbfe5ff85 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -21,7 +21,7 @@ const ( Version = "fee29-1" // FeeEnabledPrefix is the key prefix for storing fee enabled flag - FeeEnabledKeyPrefix = "fee_enabled" + FeeEnabledKeyPrefix = "feeEnabled" // RelayerAddressKeyPrefix is the key prefix for relayer address mapping RelayerAddressKeyPrefix = "relayerAddress"