From 0fbc6bbfab8676d6223cb99dd10a864c3cb7c93f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 12 Jan 2022 13:04:31 +0100 Subject: [PATCH] fix build --- modules/apps/29-fee/fee_test.go | 7 +- modules/apps/29-fee/ibc_module.go | 23 ++--- modules/apps/29-fee/ibc_module_test.go | 86 ++++++------------- modules/apps/29-fee/keeper/genesis_test.go | 2 +- modules/apps/29-fee/keeper/grpc_query_test.go | 2 +- modules/apps/29-fee/keeper/keeper_test.go | 6 +- modules/apps/29-fee/types/genesis_test.go | 2 +- modules/apps/29-fee/types/keys_test.go | 2 +- .../apps/transfer/types/expected_keepers.go | 2 +- testing/simapp/app.go | 8 +- 10 files changed, 53 insertions(+), 87 deletions(-) diff --git a/modules/apps/29-fee/fee_test.go b/modules/apps/29-fee/fee_test.go index 1b930db6456..147bd0b01a4 100644 --- a/modules/apps/29-fee/fee_test.go +++ b/modules/apps/29-fee/fee_test.go @@ -3,15 +3,14 @@ package fee_test import ( "testing" - "github.com/stretchr/testify/suite" - sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" - ibctesting "github.com/cosmos/ibc-go/testing" + ibctesting "github.com/cosmos/ibc-go/v3/testing" ) type FeeTestSuite struct { @@ -47,7 +46,7 @@ func (suite *FeeTestSuite) CreateICS20Packet(coin sdk.Coin) channeltypes.Packet fungibleTokenPacket := transfertypes.NewFungibleTokenPacketData( coin.Denom, - sdk.NewInt(100).Uint64(), + sdk.NewInt(100).String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), ) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index 230c7da2cd0..d9044200528 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -64,32 +64,33 @@ func (im IBCModule) OnChanOpenTry( channelID string, chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, - version, counterpartyVersion string, -) error { - mwVersion, appVersion := channeltypes.SplitChannelVersion(version) +) (string, error) { 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) - } - + if cpMwVersion == types.Version { 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) + appVersion, err := im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, cpAppVersion) + if err != nil { + return "", err + } + + if im.keeper.IsFeeEnabled(ctx, portID, channelID) { + return channeltypes.MergeChannelVersions(types.Version, appVersion), nil + } + + return appVersion, nil } // OnChanOpenAck implements the IBCModule interface diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index bee6b693661..08087dbc9cb 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -10,8 +10,8 @@ import ( transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v3/modules/core/24-host" - ibctesting "github.com/cosmos/ibc-go/testing" - "github.com/cosmos/ibc-go/testing/simapp" + ibctesting "github.com/cosmos/ibc-go/v3/testing" + "github.com/cosmos/ibc-go/v3/testing/simapp" ) var ( @@ -107,42 +107,30 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { func (suite *FeeTestSuite) TestOnChanOpenTry() { testCases := []struct { name string - version string cpVersion string crossing bool expPass bool }{ { - "valid fee middleware and transfer version", - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), + "valid fee middleware version", channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), false, true, }, { - "valid transfer version on try and counterparty", - transfertypes.Version, + "valid transfer version", transfertypes.Version, false, true, }, { - "valid fee middleware and transfer version, crossing hellos", - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), + "crossing hellos: valid fee middleware", channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), true, true, }, { "invalid fee middleware version", - channeltypes.MergeChannelVersions("otherfee28-1", transfertypes.Version), - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), - false, - false, - }, - { - "invalid counterparty fee middleware version", - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), channeltypes.MergeChannelVersions("wrongfee29-1", transfertypes.Version), false, false, @@ -150,42 +138,6 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { { "invalid transfer version", channeltypes.MergeChannelVersions(types.Version, "wrongics20-1"), - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), - false, - false, - }, - { - "invalid counterparty transfer version", - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), - channeltypes.MergeChannelVersions(types.Version, "wrongics20-1"), - false, - false, - }, - { - "transfer version not wrapped", - types.Version, - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), - false, - false, - }, - { - "counterparty transfer version not wrapped", - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), - types.Version, - false, - false, - }, - { - "fee version not included on try, but included in counterparty", - transfertypes.Version, - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), - false, - false, - }, - { - "fee version not included", - channeltypes.MergeChannelVersions(types.Version, transfertypes.Version), - transfertypes.Version, false, false, }, @@ -222,7 +174,7 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { Ordering: channeltypes.UNORDERED, Counterparty: counterparty, ConnectionHops: []string{suite.path.EndpointA.ConnectionID}, - Version: tc.version, + Version: tc.cpVersion, } module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort) @@ -231,13 +183,13 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() { 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) + _, err = cbs.OnChanOpenTry(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), + suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, chanCap, counterparty, tc.cpVersion) if tc.expPass { - suite.Require().NoError(err, "unexpected error from version: %s", tc.version) + suite.Require().NoError(err) } else { - suite.Require().Error(err, "error not returned for version: %s", tc.version) + suite.Require().Error(err) } }) } @@ -656,17 +608,22 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() { if tc.expPass { suite.Require().NoError(err, "unexpected error for case: %s", tc.name) + + expectedAmt, ok := sdk.NewIntFromString("10000000000000000000") + suite.Require().True(ok) suite.Require().Equal( sdk.Coin{ Denom: ibctesting.TestCoin.Denom, - Amount: sdk.NewInt(100000000000000), + Amount: expectedAmt, }, suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) } else { + expectedAmt, ok := sdk.NewIntFromString("9999999999999999400") + suite.Require().True(ok) suite.Require().Equal( sdk.Coin{ Denom: ibctesting.TestCoin.Denom, - Amount: sdk.NewInt(99999999999400), + Amount: expectedAmt, }, suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) } @@ -762,17 +719,22 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { if tc.expPass { suite.Require().NoError(err, "unexpected error for case: %s", tc.name) + + expectedAmt, ok := sdk.NewIntFromString("10000000000000000100") + suite.Require().True(ok) suite.Require().Equal( sdk.Coin{ Denom: ibctesting.TestCoin.Denom, - Amount: sdk.NewInt(100000000000100), + Amount: expectedAmt, }, suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) } else { + expectedAmt, ok := sdk.NewIntFromString("9999999999999999500") + suite.Require().True(ok) suite.Require().Equal( sdk.Coin{ Denom: ibctesting.TestCoin.Denom, - Amount: sdk.NewInt(99999999999500), + Amount: expectedAmt, }, suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) } diff --git a/modules/apps/29-fee/keeper/genesis_test.go b/modules/apps/29-fee/keeper/genesis_test.go index 8332658cecd..6833ed72816 100644 --- a/modules/apps/29-fee/keeper/genesis_test.go +++ b/modules/apps/29-fee/keeper/genesis_test.go @@ -4,7 +4,7 @@ import ( "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" - ibctesting "github.com/cosmos/ibc-go/testing" + ibctesting "github.com/cosmos/ibc-go/v3/testing" ) func (suite *KeeperTestSuite) TestInitGenesis() { diff --git a/modules/apps/29-fee/keeper/grpc_query_test.go b/modules/apps/29-fee/keeper/grpc_query_test.go index a2904234871..bef02669b04 100644 --- a/modules/apps/29-fee/keeper/grpc_query_test.go +++ b/modules/apps/29-fee/keeper/grpc_query_test.go @@ -9,7 +9,7 @@ import ( "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" - ibctesting "github.com/cosmos/ibc-go/testing" + ibctesting "github.com/cosmos/ibc-go/v3/testing" ) func (suite *KeeperTestSuite) TestQueryIncentivizedPacketI() { diff --git a/modules/apps/29-fee/keeper/keeper_test.go b/modules/apps/29-fee/keeper/keeper_test.go index 40623262158..40eeaed8d40 100644 --- a/modules/apps/29-fee/keeper/keeper_test.go +++ b/modules/apps/29-fee/keeper/keeper_test.go @@ -4,14 +4,14 @@ import ( "fmt" "testing" - "github.com/stretchr/testify/suite" - "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" + "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" - ibctesting "github.com/cosmos/ibc-go/testing" + ibctesting "github.com/cosmos/ibc-go/v3/testing" ) var ( diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go index 72382501442..190edaab220 100644 --- a/modules/apps/29-fee/types/genesis_test.go +++ b/modules/apps/29-fee/types/genesis_test.go @@ -10,7 +10,7 @@ import ( "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" - ibctesting "github.com/cosmos/ibc-go/testing" + ibctesting "github.com/cosmos/ibc-go/v3/testing" ) var ( diff --git a/modules/apps/29-fee/types/keys_test.go b/modules/apps/29-fee/types/keys_test.go index b8329ecb206..fbe2c2bbdde 100644 --- a/modules/apps/29-fee/types/keys_test.go +++ b/modules/apps/29-fee/types/keys_test.go @@ -1,7 +1,7 @@ package types_test import ( - fmt "fmt" + "fmt" "testing" "github.com/stretchr/testify/require" diff --git a/modules/apps/transfer/types/expected_keepers.go b/modules/apps/transfer/types/expected_keepers.go index ee0254526d1..8ae670d27b2 100644 --- a/modules/apps/transfer/types/expected_keepers.go +++ b/modules/apps/transfer/types/expected_keepers.go @@ -38,7 +38,7 @@ type ChannelKeeper interface { // ClientKeeper defines the expected IBC client keeper type ClientKeeper interface { - GetClientConsensusState(ctx sdk.Context, clientID string) (connection exported.ConsensusState, found bool) + GetClientConsensusState(ctx sdk.Context, clientID string) (connection ibcexported.ConsensusState, found bool) } // ConnectionKeeper defines the expected IBC connection keeper diff --git a/testing/simapp/app.go b/testing/simapp/app.go index ff22c59d1a4..5fbda3bb931 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -207,6 +207,7 @@ type SimApp struct { ScopedIBCKeeper capabilitykeeper.ScopedKeeper ScopedTransferKeeper capabilitykeeper.ScopedKeeper ScopedIBCFeeKeeper capabilitykeeper.ScopedKeeper + ScopedFeeMockKeeper capabilitykeeper.ScopedKeeper ScopedICAControllerKeeper capabilitykeeper.ScopedKeeper ScopedICAHostKeeper capabilitykeeper.ScopedKeeper ScopedIBCMockKeeper capabilitykeeper.ScopedKeeper @@ -286,6 +287,7 @@ func NewSimApp( // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // not replicate if you do not need to test core IBC or light clients. scopedIBCMockKeeper := app.CapabilityKeeper.ScopeToModule(ibcmock.ModuleName) + scopedFeeMockKeeper := app.CapabilityKeeper.ScopeToModule(ibcmock.ModuleName + ibcfeetypes.ModuleName) scopedICAMockKeeper := app.CapabilityKeeper.ScopeToModule(ibcmock.ModuleName + icacontrollertypes.SubModuleName) // seal capability keeper after scoping modules @@ -356,8 +358,10 @@ func NewSimApp( app.AccountKeeper, app.BankKeeper, scopedTransferKeeper, ) transferModule := transfer.NewAppModule(app.TransferKeeper) + transferIBCModule := transfer.NewIBCModule(app.TransferKeeper) + // create fee-wrapped transfer module - feeTransferModule := ibcfee.NewIBCModule(app.IBCFeeKeeper, transferModule) + feeTransferModule := ibcfee.NewIBCModule(app.IBCFeeKeeper, transferIBCModule) feeModule := ibcfee.NewAppModule(app.IBCFeeKeeper) @@ -366,7 +370,7 @@ func NewSimApp( // 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, &ibcmock.MockIBCApp{}) + feeMockModule := ibcfee.NewIBCModule(app.IBCFeeKeeper, ibcmock.NewIBCModule(nil, scopedFeeMockKeeper)) mockIBCModule := ibcmock.NewIBCModule(&ibcmock.MockIBCApp{}, scopedIBCMockKeeper)