From 86258225281ebf88a8cc0520a05c85950bb3c785 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Tue, 2 Apr 2024 20:11:56 +0200 Subject: [PATCH 1/9] feat!: wire ibcfee module --- app/keepers/keepers.go | 16 ++++++++++++++++ app/keepers/keys.go | 2 ++ app/modules.go | 8 ++++++++ app/upgrades/v16/constants.go | 2 ++ 4 files changed, 28 insertions(+) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 5f36b78ba2d..7b3824227d5 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -22,6 +22,9 @@ import ( icahost "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/host" icahostkeeper "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/host/keeper" icahosttypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/host/types" + ibcfee "github.com/cosmos/ibc-go/v7/modules/apps/29-fee" + ibcfeekeeper "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/keeper" + ibcfeetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" "github.com/cosmos/ibc-go/v7/modules/apps/transfer" ibctransferkeeper "github.com/cosmos/ibc-go/v7/modules/apps/transfer/keeper" ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" @@ -114,6 +117,7 @@ type AppKeepers struct { // Modules ICAModule ica.AppModule + IBCFeeKeeper ibcfeekeeper.Keeper TransferModule transfer.AppModule PFMRouterModule pfmrouter.AppModule RateLimitModule ratelimit.AppModule @@ -419,9 +423,17 @@ func NewAppKeeper( appKeepers.BankKeeper, appKeepers.ScopedTransferKeeper, ) + // Must be called on PFMRouter AFTER TransferKeeper initialized appKeepers.PFMRouterKeeper.SetTransferKeeper(appKeepers.TransferKeeper) + appKeepers.IBCFeeKeeper = ibcfeekeeper.NewKeeper( + appCodec, appKeepers.keys[ibcfeetypes.StoreKey], + appKeepers.IBCKeeper.ChannelKeeper, // may be replaced with IBC middleware + appKeepers.IBCKeeper.ChannelKeeper, + &appKeepers.IBCKeeper.PortKeeper, appKeepers.AccountKeeper, appKeepers.BankKeeper, + ) + // Middleware Stacks appKeepers.ICAModule = ica.NewAppModule(&appKeepers.ICAControllerKeeper, &appKeepers.ICAHostKeeper) appKeepers.TransferModule = transfer.NewAppModule(appKeepers.TransferKeeper) @@ -430,6 +442,7 @@ func NewAppKeeper( // Create Transfer Stack (from bottom to top of stack) // - core IBC + // - ibcfee // - ratelimit // - pfm // - transfer @@ -443,12 +456,15 @@ func NewAppKeeper( pfmrouterkeeper.DefaultRefundTransferPacketTimeoutTimestamp, ) transferStack = ratelimit.NewIBCMiddleware(appKeepers.RatelimitKeeper, transferStack) + transferStack = ibcfee.NewIBCMiddleware(transferStack, appKeepers.IBCFeeKeeper) // Create ICAHost Stack var icaHostStack porttypes.IBCModule = icahost.NewIBCModule(appKeepers.ICAHostKeeper) + icaHostStack = ibcfee.NewIBCMiddleware(icaHostStack, appKeepers.IBCFeeKeeper) // Create Interchain Accounts Controller Stack var icaControllerStack porttypes.IBCModule = icacontroller.NewIBCMiddleware(nil, appKeepers.ICAControllerKeeper) + icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, appKeepers.IBCFeeKeeper) // Create IBC Router & seal ibcRouter := porttypes.NewRouter(). diff --git a/app/keepers/keys.go b/app/keepers/keys.go index 2c8ce911d76..66a28544ae3 100644 --- a/app/keepers/keys.go +++ b/app/keepers/keys.go @@ -6,6 +6,7 @@ import ( routertypes "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7/packetforward/types" icacontrollertypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types" icahosttypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/host/types" + ibcfeetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported" providertypes "github.com/cosmos/interchain-security/v4/x/ccv/provider/types" @@ -46,6 +47,7 @@ func (appKeepers *AppKeepers) GenerateKeys() { upgradetypes.StoreKey, evidencetypes.StoreKey, ibctransfertypes.StoreKey, + ibcfeetypes.StoreKey, icahosttypes.StoreKey, icacontrollertypes.StoreKey, capabilitytypes.StoreKey, diff --git a/app/modules.go b/app/modules.go index 754405865fa..40df403100c 100644 --- a/app/modules.go +++ b/app/modules.go @@ -8,6 +8,8 @@ import ( pfmroutertypes "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7/packetforward/types" ica "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts" icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types" + ibcfee "github.com/cosmos/ibc-go/v7/modules/apps/29-fee" + ibcfeetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" "github.com/cosmos/ibc-go/v7/modules/apps/transfer" ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" ibc "github.com/cosmos/ibc-go/v7/modules/core" @@ -74,6 +76,7 @@ var maccPerms = map[string][]string{ govtypes.ModuleName: {authtypes.Burner}, // liquiditytypes.ModuleName: {authtypes.Minter, authtypes.Burner}, ibctransfertypes.ModuleName: {authtypes.Minter, authtypes.Burner}, + ibcfeetypes.ModuleName: nil, providertypes.ConsumerRewardsPool: nil, } @@ -107,6 +110,7 @@ var ModuleBasics = module.NewBasicManager( authzmodule.AppModuleBasic{}, ibc.AppModuleBasic{}, ibctm.AppModuleBasic{}, + ibcfee.AppModuleBasic{}, upgrade.AppModuleBasic{}, evidence.AppModuleBasic{}, transfer.AppModuleBasic{}, @@ -152,6 +156,7 @@ func appModules( sdkparams.NewAppModule(app.ParamsKeeper), globalfee.NewAppModule(app.GetSubspace(globalfee.ModuleName)), consensus.NewAppModule(appCodec, app.ConsensusParamsKeeper), + ibcfee.NewAppModule(app.IBCFeeKeeper), app.TransferModule, app.ICAModule, app.PFMRouterModule, @@ -221,6 +226,7 @@ func orderBeginBlockers() []string { icatypes.ModuleName, pfmroutertypes.ModuleName, ratelimittypes.ModuleName, + ibcfeetypes.ModuleName, genutiltypes.ModuleName, authz.ModuleName, feegrant.ModuleName, @@ -252,6 +258,7 @@ func orderEndBlockers() []string { pfmroutertypes.ModuleName, ratelimittypes.ModuleName, capabilitytypes.ModuleName, + ibcfeetypes.ModuleName, authtypes.ModuleName, banktypes.ModuleName, distrtypes.ModuleName, @@ -294,6 +301,7 @@ func orderInitBlockers() []string { ibctransfertypes.ModuleName, ibcexported.ModuleName, icatypes.ModuleName, + ibcfeetypes.ModuleName, evidencetypes.ModuleName, authz.ModuleName, feegrant.ModuleName, diff --git a/app/upgrades/v16/constants.go b/app/upgrades/v16/constants.go index 2e47cf6256c..010e0ca86a1 100644 --- a/app/upgrades/v16/constants.go +++ b/app/upgrades/v16/constants.go @@ -4,6 +4,7 @@ import ( ratelimittypes "github.com/Stride-Labs/ibc-rate-limiting/ratelimit/types" icacontrollertypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types" + ibcfeetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" store "github.com/cosmos/cosmos-sdk/store/types" @@ -22,6 +23,7 @@ var Upgrade = upgrades.Upgrade{ Added: []string{ ratelimittypes.ModuleName, icacontrollertypes.SubModuleName, + ibcfeetypes.ModuleName, }, }, } From 673399a76489d29392050d52cd9c07b00036312f Mon Sep 17 00:00:00 2001 From: MSalopek Date: Tue, 2 Apr 2024 20:18:38 +0200 Subject: [PATCH 2/9] docs: add changelogs --- .changelog/unreleased/dependencies/3038-ibc-fee.md | 3 +++ .changelog/unreleased/features/3038-ibc-fee.md | 3 +++ .changelog/unreleased/state-breaking/3038-ibc-fee.md | 3 +++ 3 files changed, 9 insertions(+) create mode 100644 .changelog/unreleased/dependencies/3038-ibc-fee.md create mode 100644 .changelog/unreleased/features/3038-ibc-fee.md create mode 100644 .changelog/unreleased/state-breaking/3038-ibc-fee.md diff --git a/.changelog/unreleased/dependencies/3038-ibc-fee.md b/.changelog/unreleased/dependencies/3038-ibc-fee.md new file mode 100644 index 00000000000..937edb2f361 --- /dev/null +++ b/.changelog/unreleased/dependencies/3038-ibc-fee.md @@ -0,0 +1,3 @@ +- Add the [IBC Fee Module](https://ibc.cosmos.network/v7/middleware/ics29-fee/overview) + [v7.3.2](https://github.com/cosmos/ibc-go/releases/tag/v7.3.2). + ([\#3038](https://github.com/cosmos/gaia/pull/3038)) \ No newline at end of file diff --git a/.changelog/unreleased/features/3038-ibc-fee.md b/.changelog/unreleased/features/3038-ibc-fee.md new file mode 100644 index 00000000000..937edb2f361 --- /dev/null +++ b/.changelog/unreleased/features/3038-ibc-fee.md @@ -0,0 +1,3 @@ +- Add the [IBC Fee Module](https://ibc.cosmos.network/v7/middleware/ics29-fee/overview) + [v7.3.2](https://github.com/cosmos/ibc-go/releases/tag/v7.3.2). + ([\#3038](https://github.com/cosmos/gaia/pull/3038)) \ No newline at end of file diff --git a/.changelog/unreleased/state-breaking/3038-ibc-fee.md b/.changelog/unreleased/state-breaking/3038-ibc-fee.md new file mode 100644 index 00000000000..937edb2f361 --- /dev/null +++ b/.changelog/unreleased/state-breaking/3038-ibc-fee.md @@ -0,0 +1,3 @@ +- Add the [IBC Fee Module](https://ibc.cosmos.network/v7/middleware/ics29-fee/overview) + [v7.3.2](https://github.com/cosmos/ibc-go/releases/tag/v7.3.2). + ([\#3038](https://github.com/cosmos/gaia/pull/3038)) \ No newline at end of file From 44a6f622e07ffca00825bf178aeea0ecafcc8a25 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Tue, 2 Apr 2024 22:15:43 +0200 Subject: [PATCH 3/9] chore: update e2e ICA test; correctly wire IBCFee with ICA and PFM --- app/keepers/keepers.go | 20 ++++++++++---------- tests/e2e/e2e_ica_test.go | 1 + tests/e2e/e2e_test.go | 26 +++++++++++++------------- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 7b3824227d5..f23653a2cf2 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -362,12 +362,19 @@ func NewAppKeeper( // If evidence needs to be handled for the app, set routes in router here and seal appKeepers.EvidenceKeeper = *evidenceKeeper + appKeepers.IBCFeeKeeper = ibcfeekeeper.NewKeeper( + appCodec, appKeepers.keys[ibcfeetypes.StoreKey], + appKeepers.IBCKeeper.ChannelKeeper, // may be replaced with IBC middleware + appKeepers.IBCKeeper.ChannelKeeper, + &appKeepers.IBCKeeper.PortKeeper, appKeepers.AccountKeeper, appKeepers.BankKeeper, + ) + // ICA Host keeper appKeepers.ICAHostKeeper = icahostkeeper.NewKeeper( appCodec, appKeepers.keys[icahosttypes.StoreKey], appKeepers.GetSubspace(icahosttypes.SubModuleName), - appKeepers.IBCKeeper.ChannelKeeper, // ICS4Wrapper + appKeepers.IBCFeeKeeper, // ICS4Wrapper appKeepers.IBCKeeper.ChannelKeeper, &appKeepers.IBCKeeper.PortKeeper, appKeepers.AccountKeeper, @@ -393,7 +400,7 @@ func NewAppKeeper( appCodec, appKeepers.keys[icacontrollertypes.StoreKey], appKeepers.GetSubspace(icacontrollertypes.SubModuleName), - appKeepers.IBCKeeper.ChannelKeeper, // ICS4Wrapper + appKeepers.IBCFeeKeeper, // ICS4Wrapper appKeepers.IBCKeeper.ChannelKeeper, &appKeepers.IBCKeeper.PortKeeper, appKeepers.ScopedICAControllerKeeper, @@ -427,13 +434,6 @@ func NewAppKeeper( // Must be called on PFMRouter AFTER TransferKeeper initialized appKeepers.PFMRouterKeeper.SetTransferKeeper(appKeepers.TransferKeeper) - appKeepers.IBCFeeKeeper = ibcfeekeeper.NewKeeper( - appCodec, appKeepers.keys[ibcfeetypes.StoreKey], - appKeepers.IBCKeeper.ChannelKeeper, // may be replaced with IBC middleware - appKeepers.IBCKeeper.ChannelKeeper, - &appKeepers.IBCKeeper.PortKeeper, appKeepers.AccountKeeper, appKeepers.BankKeeper, - ) - // Middleware Stacks appKeepers.ICAModule = ica.NewAppModule(&appKeepers.ICAControllerKeeper, &appKeepers.ICAHostKeeper) appKeepers.TransferModule = transfer.NewAppModule(appKeepers.TransferKeeper) @@ -442,9 +442,9 @@ func NewAppKeeper( // Create Transfer Stack (from bottom to top of stack) // - core IBC - // - ibcfee // - ratelimit // - pfm + // - ibcfee // - transfer var transferStack porttypes.IBCModule transferStack = transfer.NewIBCModule(appKeepers.TransferKeeper) diff --git a/tests/e2e/e2e_ica_test.go b/tests/e2e/e2e_ica_test.go index 1e39736f042..5dc451ddcfe 100644 --- a/tests/e2e/e2e_ica_test.go +++ b/tests/e2e/e2e_ica_test.go @@ -147,6 +147,7 @@ func (s *IntegrationTestSuite) registerICAAccount(c *chain, valIdx int, sender, fmt.Sprintf("--from=%s", sender), fmt.Sprintf("--%s=%s", flags.FlagFees, fees), fmt.Sprintf("--%s=%s", flags.FlagChainID, c.id), + "--gas=250000", // default 200_000 is not enough; gas fees increased after adding IBC fee middleware "--keyring-backend=test", "--broadcast-mode=sync", "--output=json", diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 9c9d71d3947..ffb6e9dc888 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -3,20 +3,20 @@ package e2e import "fmt" var ( - runBankTest = true - runBypassMinFeeTest = true - runEncodeTest = true - runEvidenceTest = true - runFeeGrantTest = true - runGlobalFeesTest = true - runGovTest = true + runBankTest = false + runBypassMinFeeTest = false + runEncodeTest = false + runEvidenceTest = false + runFeeGrantTest = false + runGlobalFeesTest = false + runGovTest = false runIBCTest = true - runSlashingTest = true - runStakingAndDistributionTest = true - runVestingTest = true - runRestInterfacesTest = true - runLsmTest = true - runRateLimitTest = true + runSlashingTest = false + runStakingAndDistributionTest = false + runVestingTest = false + runRestInterfacesTest = false + runLsmTest = false + runRateLimitTest = false ) func (s *IntegrationTestSuite) TestRestInterfaces() { From 7734d57ff7a316833b2e01c5ebb1704af401b357 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Wed, 3 Apr 2024 13:40:23 +0200 Subject: [PATCH 4/9] uncomment tests --- tests/e2e/e2e_test.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index ffb6e9dc888..9c9d71d3947 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -3,20 +3,20 @@ package e2e import "fmt" var ( - runBankTest = false - runBypassMinFeeTest = false - runEncodeTest = false - runEvidenceTest = false - runFeeGrantTest = false - runGlobalFeesTest = false - runGovTest = false + runBankTest = true + runBypassMinFeeTest = true + runEncodeTest = true + runEvidenceTest = true + runFeeGrantTest = true + runGlobalFeesTest = true + runGovTest = true runIBCTest = true - runSlashingTest = false - runStakingAndDistributionTest = false - runVestingTest = false - runRestInterfacesTest = false - runLsmTest = false - runRateLimitTest = false + runSlashingTest = true + runStakingAndDistributionTest = true + runVestingTest = true + runRestInterfacesTest = true + runLsmTest = true + runRateLimitTest = true ) func (s *IntegrationTestSuite) TestRestInterfaces() { From 56a7c0f400524665f46397d0e41c46b5d21cacf7 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Thu, 4 Apr 2024 12:13:24 +0200 Subject: [PATCH 5/9] tests: add IBC fee integration tests --- tests/integration/ibcfee_test.go | 334 ++++++++++++++++++ .../interchain_security_test.go | 2 +- 2 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 tests/integration/ibcfee_test.go rename tests/{ics => integration}/interchain_security_test.go (98%) diff --git a/tests/integration/ibcfee_test.go b/tests/integration/ibcfee_test.go new file mode 100644 index 00000000000..d5f9b8bb8d0 --- /dev/null +++ b/tests/integration/ibcfee_test.go @@ -0,0 +1,334 @@ +package integration + +import ( + "testing" + + "github.com/cosmos/gogoproto/proto" + "github.com/stretchr/testify/suite" + + sdk "github.com/cosmos/cosmos-sdk/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + + icahosttypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/host/types" + icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types" + "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" + ibcfeetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" + transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" + clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" + channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" + ibctesting "github.com/cosmos/ibc-go/v7/testing" + + gaiaApp "github.com/cosmos/gaia/v16/app" +) + +// These integration tests were modified to work with the GaiaApp +// Sources: +// * Transfer tests: https://github.com/cosmos/ibc-go/blob/v7.3.2/modules/apps/29-fee/transfer_test.go#L13 +// * ICA tests: https://github.com/cosmos/ibc-go/blob/v7.3.2/modules/apps/29-fee/ica_test.go#L94 +var ( + + // transfer + IBC fee test variables + defaultRecvFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}} + defaultAckFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(200)}} + defaultTimeoutFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(300)}} + smallAmount = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(50)}} + + // ICA + IBC fee test variables + defaultOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" // defaultOwnerAddress defines a reusable bech32 address for testing purposes + defaultPortID, _ = icatypes.NewControllerPortID(defaultOwnerAddress) // defaultPortID defines a reusable port identifier for testing purposes + defaultICAVersion = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) // defaultICAVersion defines a reusable interchainaccounts version string for testing purposes +) + +type IBCFeeTestSuite struct { + suite.Suite + coordinator *ibctesting.Coordinator + + chainA *ibctesting.TestChain + chainB *ibctesting.TestChain + chainC *ibctesting.TestChain + + path *ibctesting.Path + pathAToC *ibctesting.Path +} + +func TestFeeMarketTestSuite(t *testing.T) { + ibcfeeSuite := &IBCFeeTestSuite{} + suite.Run(t, ibcfeeSuite) +} + +func (suite *IBCFeeTestSuite) SetupTest() { + ibctesting.DefaultTestingAppInit = GaiaAppIniter + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 3) + suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1)) + suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2)) + suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(3)) + + chain, ok := suite.coordinator.Chains[ibctesting.GetChainID(1)] + suite.Require().True(ok, "chain not found") + + _, ok = chain.App.(*gaiaApp.GaiaApp) + suite.Require().True(ok, "expected App to be GaiaApp") + + path := ibctesting.NewPath(suite.chainA, suite.chainB) + mockFeeVersion := string(ibcfeetypes.ModuleCdc.MustMarshalJSON( + &ibcfeetypes.Metadata{ + FeeVersion: ibcfeetypes.Version, + AppVersion: "test-version", + }, + )) + path.EndpointA.ChannelConfig.Version = mockFeeVersion + path.EndpointB.ChannelConfig.Version = mockFeeVersion + path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort + path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort + suite.path = path + + path = ibctesting.NewPath(suite.chainA, suite.chainC) + path.EndpointA.ChannelConfig.Version = mockFeeVersion + path.EndpointB.ChannelConfig.Version = mockFeeVersion + path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort + path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort + suite.pathAToC = path +} + +// Integration test to ensure ics29 works with ics20 +// Source: https://github.com/cosmos/ibc-go/blob/v7.3.2/modules/apps/29-fee/transfer_test.go#L13 +func (suite *IBCFeeTestSuite) TestFeeTransfer() { + path := ibctesting.NewPath(suite.chainA, suite.chainB) + feeTransferVersion := string(ibcfeetypes.ModuleCdc.MustMarshalJSON(&ibcfeetypes.Metadata{FeeVersion: ibcfeetypes.Version, AppVersion: 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.coordinator.Setup(path) + + // set up coin & ics20 packet + coin := ibctesting.TestCoin + fee := types.Fee{ + RecvFee: defaultRecvFee, + AckFee: defaultAckFee, + TimeoutFee: defaultTimeoutFee, + } + + msgs := []sdk.Msg{ + ibcfeetypes.NewMsgPayPacketFee(fee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil), + transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""), + } + res, err := suite.chainA.SendMsgs(msgs...) + suite.Require().NoError(err) // message committed + + // after incentivizing the packets + originalChainASenderAccountBalance := sdk.NewCoins(getApp(suite.chainA).BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)) + + packet, err := ibctesting.ParsePacketFromEvents(res.GetEvents()) + suite.Require().NoError(err) + + // register counterparty address on chainB + // relayerAddress is address of sender account on chainB, but we will use it on chainA + // to differentiate from the chainA.SenderAccount for checking successful relay payouts + relayerAddress := suite.chainB.SenderAccount.GetAddress() + + msgRegister := types.NewMsgRegisterCounterpartyPayee(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, suite.chainB.SenderAccount.GetAddress().String(), relayerAddress.String()) + _, err = suite.chainB.SendMsgs(msgRegister) + suite.Require().NoError(err) // message committed + + // relay packet + err = path.RelayPacket(packet) + suite.Require().NoError(err) // relay committed + + // ensure relayers got paid + // relayer for forward relay: chainB.SenderAccount + // relayer for reverse relay: chainA.SenderAccount + + // check forward relay balance + suite.Require().Equal( + fee.RecvFee, + sdk.NewCoins(getApp(suite.chainA).BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainB.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)), + ) + + suite.Require().Equal( + fee.AckFee.Add(fee.TimeoutFee...), // ack fee paid, timeout fee refunded + sdk.NewCoins(getApp(suite.chainA).BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)).Sub(originalChainASenderAccountBalance[0])) +} + +// TestFeeInterchainAccounts Integration test to ensure ics29 works with ics27 +// Source: https://github.com/cosmos/ibc-go/blob/v7.3.2/modules/apps/29-fee/ica_test.go#L94 +func (suite *IBCFeeTestSuite) TestFeeInterchainAccounts() { + path := NewIncentivizedICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupPath(path, defaultOwnerAddress) + suite.Require().NoError(err) + + // assert the newly established channel is fee enabled on both ends + suite.Require().True(getApp(suite.chainA).IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) + suite.Require().True(getApp(suite.chainB).IBCFeeKeeper.IsFeeEnabled(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)) + + // register counterparty address on destination chainB as chainA.SenderAccounts[1] for recv fee distribution + getApp(suite.chainB).IBCFeeKeeper.SetCounterpartyPayeeAddress(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccounts[1].SenderAccount.GetAddress().String(), path.EndpointB.ChannelID) + + // escrow a packet fee for the next send sequence + expectedFee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + msgPayPacketFee := types.NewMsgPayPacketFee(expectedFee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil) + + // fetch the account balance before fees are escrowed and assert the difference below + preEscrowBalance := getApp(suite.chainA).BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom) + + res, err := suite.chainA.SendMsgs(msgPayPacketFee) + suite.Require().NotNil(res) + suite.Require().NoError(err) + + postEscrowBalance := getApp(suite.chainA).BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom) + suite.Require().Equal(postEscrowBalance.AddAmount(expectedFee.Total().AmountOf(sdk.DefaultBondDenom)), preEscrowBalance) + + packetID := channeltypes.NewPacketID(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, 1) + packetFees, found := getApp(suite.chainA).IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID) + suite.Require().True(found) + suite.Require().Equal(expectedFee, packetFees.PacketFees[0].Fee) + + interchainAccountAddr, found := getApp(suite.chainB).ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID) + suite.Require().True(found) + + // fund the interchain account on chainB + coins := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100000))) + msgBankSend := &banktypes.MsgSend{ + FromAddress: suite.chainB.SenderAccount.GetAddress().String(), + ToAddress: interchainAccountAddr, + Amount: coins, + } + + res, err = suite.chainB.SendMsgs(msgBankSend) + suite.Require().NotEmpty(res) + suite.Require().NoError(err) + + // prepare a simple stakingtypes.MsgDelegate to be used as the interchain account msg executed on chainB + validatorAddr := (sdk.ValAddress)(suite.chainB.Vals.Validators[0].Address) + msgDelegate := &stakingtypes.MsgDelegate{ + DelegatorAddress: interchainAccountAddr, + ValidatorAddress: validatorAddr.String(), + Amount: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(5000)), + } + + data, err := icatypes.SerializeCosmosTx(getApp(suite.chainA).AppCodec(), []proto.Message{msgDelegate}) + suite.Require().NoError(err) + + icaPacketData := icatypes.InterchainAccountPacketData{ + Type: icatypes.EXECUTE_TX, + Data: data, + } + + // ensure chainB is allowed to execute stakingtypes.MsgDelegate + params := icahosttypes.NewParams(true, []string{sdk.MsgTypeURL(msgDelegate)}) + getApp(suite.chainB).ICAHostKeeper.SetParams(suite.chainB.GetContext(), params) + + // build the interchain accounts packet + packet := buildInterchainAccountsPacket(path, icaPacketData.GetBytes()) + + // write packet commitment to state on chainA and commit state + commitment := channeltypes.CommitPacket(getApp(suite.chainA).AppCodec(), packet) + getApp(suite.chainA).IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, 1, commitment) + suite.chainA.NextBlock() + + err = path.RelayPacket(packet) + suite.Require().NoError(err) + + // ensure escrowed fees are cleaned up + packetFees, found = getApp(suite.chainA).IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID) + suite.Require().False(found) + suite.Require().Empty(packetFees) + + // assert the value of the account balance after fee distribution + // NOTE: the balance after fee distribution should be equal to the pre-escrow balance minus the recv fee + // as chainA.SenderAccount is used as the msg signer and refund address for msgPayPacketFee above as well as the relyer account for acknowledgements in path.RelayPacket() + postDistBalance := getApp(suite.chainA).BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom) + suite.Require().Equal(preEscrowBalance.SubAmount(defaultRecvFee.AmountOf(sdk.DefaultBondDenom)), postDistBalance) +} + +func buildInterchainAccountsPacket(path *ibctesting.Path, data []byte) channeltypes.Packet { + packet := channeltypes.NewPacket( + data, + 1, + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + path.EndpointB.ChannelConfig.PortID, + path.EndpointB.ChannelID, + clienttypes.NewHeight(1, 100), + 0, + ) + + return packet +} + +// NewIncentivizedICAPath creates and returns a new ibctesting path configured for a fee enabled interchain accounts channel +func NewIncentivizedICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { + path := ibctesting.NewPath(chainA, chainB) + + feeMetadata := types.Metadata{ + FeeVersion: types.Version, + AppVersion: defaultICAVersion, + } + + feeICAVersion := string(types.ModuleCdc.MustMarshalJSON(&feeMetadata)) + + path.SetChannelOrdered() + path.EndpointA.ChannelConfig.Version = feeICAVersion + path.EndpointB.ChannelConfig.Version = feeICAVersion + path.EndpointA.ChannelConfig.PortID = defaultPortID + path.EndpointB.ChannelConfig.PortID = icatypes.HostPortID + + return path +} + +// SetupPath performs the InterchainAccounts channel creation handshake using an ibctesting path +func SetupPath(path *ibctesting.Path, owner string) error { + if err := RegisterInterchainAccount(path.EndpointA, owner); err != nil { + return err + } + + if err := path.EndpointB.ChanOpenTry(); err != nil { + return err + } + + if err := path.EndpointA.ChanOpenAck(); err != nil { + return err + } + + if err := path.EndpointB.ChanOpenConfirm(); err != nil { + return err + } + + return nil +} + +// RegisterInterchainAccount invokes the the InterchainAccounts entrypoint, routes a new MsgChannelOpenInit to the appropriate handler, +// commits state changes and updates the testing endpoint accordingly +func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) error { + portID, err := icatypes.NewControllerPortID(owner) + if err != nil { + return err + } + + channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) + + if err := getApp(endpoint.Chain).ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil { + return err + } + + // commit state changes for proof verification + endpoint.Chain.NextBlock() + + // update port/channel ids + endpoint.ChannelID = channeltypes.FormatChannelIdentifier(channelSequence) + endpoint.ChannelConfig.PortID = portID + + return nil +} + +func getApp(chain *ibctesting.TestChain) *gaiaApp.GaiaApp { + app, ok := chain.App.(*gaiaApp.GaiaApp) + if !ok { + panic("expected App to be GaiaApp") + } + return app +} diff --git a/tests/ics/interchain_security_test.go b/tests/integration/interchain_security_test.go similarity index 98% rename from tests/ics/interchain_security_test.go rename to tests/integration/interchain_security_test.go index 1631a6de550..7d4eeb56cee 100644 --- a/tests/ics/interchain_security_test.go +++ b/tests/integration/interchain_security_test.go @@ -1,4 +1,4 @@ -package ics +package integration import ( "encoding/json" From 2cb521bc74269bb1ecec5ec9b26ac297a6044df8 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Thu, 4 Apr 2024 12:16:30 +0200 Subject: [PATCH 6/9] chore: appease linter --- tests/integration/ibcfee_test.go | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/tests/integration/ibcfee_test.go b/tests/integration/ibcfee_test.go index d5f9b8bb8d0..82fb7f4aa52 100644 --- a/tests/integration/ibcfee_test.go +++ b/tests/integration/ibcfee_test.go @@ -3,22 +3,21 @@ package integration import ( "testing" - "github.com/cosmos/gogoproto/proto" "github.com/stretchr/testify/suite" - sdk "github.com/cosmos/cosmos-sdk/types" - banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - + "github.com/cosmos/gogoproto/proto" icahosttypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types" - "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" ibcfeetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/v7/testing" + sdk "github.com/cosmos/cosmos-sdk/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + gaiaApp "github.com/cosmos/gaia/v16/app" ) @@ -32,7 +31,6 @@ var ( defaultRecvFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}} defaultAckFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(200)}} defaultTimeoutFee = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(300)}} - smallAmount = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(50)}} // ICA + IBC fee test variables defaultOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" // defaultOwnerAddress defines a reusable bech32 address for testing purposes @@ -105,7 +103,7 @@ func (suite *IBCFeeTestSuite) TestFeeTransfer() { // set up coin & ics20 packet coin := ibctesting.TestCoin - fee := types.Fee{ + fee := ibcfeetypes.Fee{ RecvFee: defaultRecvFee, AckFee: defaultAckFee, TimeoutFee: defaultTimeoutFee, @@ -129,7 +127,7 @@ func (suite *IBCFeeTestSuite) TestFeeTransfer() { // to differentiate from the chainA.SenderAccount for checking successful relay payouts relayerAddress := suite.chainB.SenderAccount.GetAddress() - msgRegister := types.NewMsgRegisterCounterpartyPayee(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, suite.chainB.SenderAccount.GetAddress().String(), relayerAddress.String()) + msgRegister := ibcfeetypes.NewMsgRegisterCounterpartyPayee(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, suite.chainB.SenderAccount.GetAddress().String(), relayerAddress.String()) _, err = suite.chainB.SendMsgs(msgRegister) suite.Require().NoError(err) // message committed @@ -169,8 +167,8 @@ func (suite *IBCFeeTestSuite) TestFeeInterchainAccounts() { getApp(suite.chainB).IBCFeeKeeper.SetCounterpartyPayeeAddress(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccounts[1].SenderAccount.GetAddress().String(), path.EndpointB.ChannelID) // escrow a packet fee for the next send sequence - expectedFee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) - msgPayPacketFee := types.NewMsgPayPacketFee(expectedFee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil) + expectedFee := ibcfeetypes.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + msgPayPacketFee := ibcfeetypes.NewMsgPayPacketFee(expectedFee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil) // fetch the account balance before fees are escrowed and assert the difference below preEscrowBalance := getApp(suite.chainA).BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom) @@ -264,12 +262,12 @@ func buildInterchainAccountsPacket(path *ibctesting.Path, data []byte) channelty func NewIncentivizedICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { path := ibctesting.NewPath(chainA, chainB) - feeMetadata := types.Metadata{ - FeeVersion: types.Version, + feeMetadata := ibcfeetypes.Metadata{ + FeeVersion: ibcfeetypes.Version, AppVersion: defaultICAVersion, } - feeICAVersion := string(types.ModuleCdc.MustMarshalJSON(&feeMetadata)) + feeICAVersion := string(ibcfeetypes.ModuleCdc.MustMarshalJSON(&feeMetadata)) path.SetChannelOrdered() path.EndpointA.ChannelConfig.Version = feeICAVersion @@ -294,11 +292,7 @@ func SetupPath(path *ibctesting.Path, owner string) error { return err } - if err := path.EndpointB.ChanOpenConfirm(); err != nil { - return err - } - - return nil + return path.EndpointB.ChanOpenConfirm() } // RegisterInterchainAccount invokes the the InterchainAccounts entrypoint, routes a new MsgChannelOpenInit to the appropriate handler, From 17dd094f52ad29cb88764ee10dbd8becf1aadc79 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Tue, 9 Apr 2024 18:00:39 +0200 Subject: [PATCH 7/9] nit: rm printline from test --- tests/integration/interchain_security_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration/interchain_security_test.go b/tests/integration/interchain_security_test.go index 8b51b14eaac..1db5ff92e7b 100644 --- a/tests/integration/interchain_security_test.go +++ b/tests/integration/interchain_security_test.go @@ -2,7 +2,6 @@ package integration import ( "encoding/json" - "fmt" "testing" "github.com/stretchr/testify/require" @@ -103,7 +102,6 @@ func TestICSEpochs(t *testing.T) { // Bond some tokens on provider to change validator powers delegateFn(provCtx) - fmt.Println(app.StakingKeeper.GetLastTotalPower(provCtx)) // VSCPacket should only be created at the end of the current epoch require.Empty(t, getVSCPacketsFn()) From 246ad288375d6b6e4e2fe068a533074dea7e3700 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Wed, 10 Apr 2024 17:14:19 +0200 Subject: [PATCH 8/9] apply comment suggestion --- app/keepers/keepers.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index f23653a2cf2..d42d9099dba 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -392,7 +392,7 @@ func NewAppKeeper( govAuthority, // authority appKeepers.BankKeeper, appKeepers.IBCKeeper.ChannelKeeper, // ChannelKeeper - appKeepers.IBCKeeper.ChannelKeeper, // ICS4Wrapper + appKeepers.IBCFeeKeeper, // ICS4Wrapper ) // ICA Controller keeper @@ -442,10 +442,14 @@ func NewAppKeeper( // Create Transfer Stack (from bottom to top of stack) // - core IBC + // - ibcfee // - ratelimit // - pfm - // - ibcfee // - transfer + // + // This is how transfer stack will work in the end: + // * RecvPacket -> IBC core -> Fee -> RateLimit -> PFM -> Transfer (AddRoute) + // * SentPacket -> Transfer -> PFM -> RateLimit -> Fee -> IBC core (ICS4Wrapper) var transferStack porttypes.IBCModule transferStack = transfer.NewIBCModule(appKeepers.TransferKeeper) transferStack = pfmrouter.NewIBCMiddleware( From 77afec92aab8e1f2a1dd965bb0ccfb2753a5410b Mon Sep 17 00:00:00 2001 From: MSalopek Date: Fri, 12 Apr 2024 14:22:49 +0200 Subject: [PATCH 9/9] Update app/keepers/keepers.go Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> --- app/keepers/keepers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index d42d9099dba..6e2acac5547 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -449,7 +449,7 @@ func NewAppKeeper( // // This is how transfer stack will work in the end: // * RecvPacket -> IBC core -> Fee -> RateLimit -> PFM -> Transfer (AddRoute) - // * SentPacket -> Transfer -> PFM -> RateLimit -> Fee -> IBC core (ICS4Wrapper) + // * SendPacket -> Transfer -> PFM -> RateLimit -> Fee -> IBC core (ICS4Wrapper) var transferStack porttypes.IBCModule transferStack = transfer.NewIBCModule(appKeepers.TransferKeeper) transferStack = pfmrouter.NewIBCMiddleware(