From f530072b52adc9c1d3678c383b6c63a0183603ff Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 10:36:52 +0200 Subject: [PATCH 01/12] adding interchain accounts integration tests for ics29 fee --- modules/apps/29-fee/ica_test.go | 191 ++++++++++++++++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 modules/apps/29-fee/ica_test.go diff --git a/modules/apps/29-fee/ica_test.go b/modules/apps/29-fee/ica_test.go new file mode 100644 index 00000000000..e6022356cc7 --- /dev/null +++ b/modules/apps/29-fee/ica_test.go @@ -0,0 +1,191 @@ +package fee_test + +import ( + 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/v3/modules/apps/27-interchain-accounts/host/types" + icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types" + "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/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/v3/testing" +) + +var ( + // DefaultOwnerAddress defines a reusable bech32 address for testing purposes + DefaultOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" + + // DefaultPortID defines a resuable port identifier for testing purposes + DefaultPortID, _ = icatypes.NewControllerPortID(DefaultOwnerAddress) + + // DefaultICAVersion defines a resuable interchainaccounts version string for testing purposes + DefaultICAVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{ + Version: icatypes.Version, + ControllerConnectionId: ibctesting.FirstConnectionID, + HostConnectionId: ibctesting.FirstConnectionID, + Encoding: icatypes.EncodingProtobuf, + TxType: icatypes.TxTypeSDKMultiMsg, + })) +) + +func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { + path := ibctesting.NewPath(chainA, chainB) + + feeICAVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: DefaultICAVersion})) + + path.SetChannelOrdered() + path.EndpointA.ChannelConfig.Version = feeICAVersion + path.EndpointB.ChannelConfig.Version = feeICAVersion + path.EndpointA.ChannelConfig.PortID = DefaultPortID + path.EndpointB.ChannelConfig.PortID = icatypes.PortID + + return path +} + +// SetupICAPath performs the InterchainAccounts channel creation handshake using an ibctesting path +func SetupICAPath(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, 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 := endpoint.Chain.GetSimApp().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 +} + +// TestFeeInterchainAccounts Integration test to ensure ics29 works with ics27 +func (suite *FeeTestSuite) TestFeeInterchainAccounts() { + path := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICAPath(path, DefaultOwnerAddress) + suite.Require().NoError(err) + + // assert the newly established channel is fee enabled on both ends + suite.Require().True(suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) + suite.Require().True(suite.chainB.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, 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 := suite.chainA.GetSimApp().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 := suite.chainA.GetSimApp().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 := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID) + suite.Require().True(found) + suite.Require().Equal(expectedFee, packetFees.PacketFees[0].Fee) + + interchainAccountAddr, found := suite.chainB.GetSimApp().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(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{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)}) + suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params) + + // build the interchain accounts packet + packet := buildInterchainAccountsPacket(path, icaPacketData.GetBytes(), 1) + + // write packet commitment to state on chainA and commit state + commitment := channeltypes.CommitPacket(suite.chainA.GetSimApp().AppCodec(), packet) + suite.chainA.GetSimApp().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 = suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID) + suite.Require().False(found) + suite.Require().Empty(packetFees) + + // assert the value of the account balance after fee distribution + postDistBalance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom) + suite.Require().Equal(preEscrowBalance, postDistBalance) +} + +func buildInterchainAccountsPacket(path *ibctesting.Path, data []byte, seq uint64) channeltypes.Packet { + packet := channeltypes.NewPacket( + data, + 1, + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + path.EndpointB.ChannelConfig.PortID, + path.EndpointB.ChannelID, + clienttypes.NewHeight(0, 100), + 0, + ) + + return packet +} From dd1bb08e1204d97e07b758075fe12ff6635f5e08 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 10:37:17 +0200 Subject: [PATCH 02/12] updating simapp to include fee middleware in ica stack --- testing/simapp/app.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index f707ba6087b..a368e8ae1c9 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -366,7 +366,7 @@ func NewSimApp( // ICA Controller keeper app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName), - app.IBCKeeper.ChannelKeeper, // may be replaced with middleware such as ics29 fee + app.IBCFeeKeeper, // use ics29 fee as ics4Wrapper in middleware stack app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, scopedICAControllerKeeper, app.MsgServiceRouter(), ) @@ -424,15 +424,21 @@ func NewSimApp( // Create Interchain Accounts Stack // SendPacket, since it is originating from the application to core IBC: - // icaAuthModuleKeeper.SendTx -> icaControllerKeeper.SendPacket -> channel.SendPacket + // icaAuthModuleKeeper.SendTx -> icaController.SendPacket -> fee.SendPacket -> channel.SendPacket // initialize ICA module with mock module as the authentication module on the controller side var icaControllerStack porttypes.IBCModule icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) app.ICAAuthModule = icaControllerStack.(ibcmock.IBCModule) icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper) + icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper) - icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper) + // RecvPacket, message that originates from core IBC and goes down to app, the flow is: + // channel.RecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket + + var icaHostStack porttypes.IBCModule + icaHostStack = icahost.NewIBCModule(app.ICAHostKeeper) + icaHostStack = ibcfee.NewIBCMiddleware(icaHostStack, app.IBCFeeKeeper) // Add host, controller & ica auth modules to IBC router ibcRouter. @@ -440,7 +446,7 @@ func NewSimApp( // ICA controller module owns the port capability for ICA. The ICA authentication module // owns the channel capability. AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). - AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). + AddRoute(icahosttypes.SubModuleName, icaHostStack). AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack) // Create Mock IBC Fee module stack for testing From 42c4a40155aab7552b3907cf0f42ceef2286242e Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 11:21:27 +0200 Subject: [PATCH 03/12] updating simapp to include ics29 fee in ica stacks --- testing/simapp/app.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index f707ba6087b..a368e8ae1c9 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -366,7 +366,7 @@ func NewSimApp( // ICA Controller keeper app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName), - app.IBCKeeper.ChannelKeeper, // may be replaced with middleware such as ics29 fee + app.IBCFeeKeeper, // use ics29 fee as ics4Wrapper in middleware stack app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, scopedICAControllerKeeper, app.MsgServiceRouter(), ) @@ -424,15 +424,21 @@ func NewSimApp( // Create Interchain Accounts Stack // SendPacket, since it is originating from the application to core IBC: - // icaAuthModuleKeeper.SendTx -> icaControllerKeeper.SendPacket -> channel.SendPacket + // icaAuthModuleKeeper.SendTx -> icaController.SendPacket -> fee.SendPacket -> channel.SendPacket // initialize ICA module with mock module as the authentication module on the controller side var icaControllerStack porttypes.IBCModule icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) app.ICAAuthModule = icaControllerStack.(ibcmock.IBCModule) icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper) + icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper) - icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper) + // RecvPacket, message that originates from core IBC and goes down to app, the flow is: + // channel.RecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket + + var icaHostStack porttypes.IBCModule + icaHostStack = icahost.NewIBCModule(app.ICAHostKeeper) + icaHostStack = ibcfee.NewIBCMiddleware(icaHostStack, app.IBCFeeKeeper) // Add host, controller & ica auth modules to IBC router ibcRouter. @@ -440,7 +446,7 @@ func NewSimApp( // ICA controller module owns the port capability for ICA. The ICA authentication module // owns the channel capability. AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). - AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). + AddRoute(icahosttypes.SubModuleName, icaHostStack). AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack) // Create Mock IBC Fee module stack for testing From 9f97a2089b34bf3d87866551bdbabea259cd1708 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 11:22:25 +0200 Subject: [PATCH 04/12] updating RegisterInterchainAccount to pass through version arg and support fee middleware functionality --- .../controller/keeper/account.go | 36 +++++-------------- 1 file changed, 9 insertions(+), 27 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/account.go b/modules/apps/27-interchain-accounts/controller/keeper/account.go index 03eeef69f1f..9e2d8ce5199 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/account.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/account.go @@ -9,12 +9,14 @@ import ( host "github.com/cosmos/ibc-go/v3/modules/core/24-host" ) -// RegisterInterchainAccount is the entry point to registering an interchain account. -// It generates a new port identifier using the owner address. It will bind to the -// port identifier and call 04-channel 'ChanOpenInit'. An error is returned if the port -// identifier is already in use. Gaining access to interchain accounts whose channels -// have closed cannot be done with this function. A regular MsgChanOpenInit must be used. -func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner string) error { +// RegisterInterchainAccount is the entry point to registering an interchain account: +// - It generates a new port identifier using the provided owner string, binds to the port identifier and clais the associated capability. +// - Callers are expected to provide the appropriate application version string. +// - For example, this could be an ICS27 encoded metadata type or an ICS29 encoded metadata type with a nested application version. +// - A new MsgChannelOpenInit is routed through the MsgServiceRouter, executing the OnOpenChanInit callback stack as configured. +// - An error is returned if the port identifier is already in use. Gaining access to interchain accounts whose channels +// have closed cannot be done with this function. A regular MsgChannelOpenInit must be used. +func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner, version string) error { portID, err := icatypes.NewControllerPortID(owner) if err != nil { return err @@ -36,27 +38,7 @@ func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner s } } - connectionEnd, err := k.channelKeeper.GetConnection(ctx, connectionID) - if err != nil { - return err - } - - // NOTE: An empty string is provided for accAddress, to be fulfilled upon OnChanOpenTry handshake step - metadata := icatypes.NewMetadata( - icatypes.Version, - connectionID, - connectionEnd.GetCounterparty().GetConnectionID(), - "", - icatypes.EncodingProtobuf, - icatypes.TxTypeSDKMultiMsg, - ) - - versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) - if err != nil { - return err - } - - msg := channeltypes.NewMsgChannelOpenInit(portID, string(versionBytes), channeltypes.ORDERED, []string{connectionID}, icatypes.PortID, icatypes.ModuleName) + msg := channeltypes.NewMsgChannelOpenInit(portID, version, channeltypes.ORDERED, []string{connectionID}, icatypes.PortID, icatypes.ModuleName) handler := k.msgRouter.Handler(msg) res, err := handler(ctx, msg) From 42f0095c4ac08f19ac04c3371461daab2bdb3ad6 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 11:27:22 +0200 Subject: [PATCH 05/12] updating tests to support additional version arg in RegisterInterchainAccount --- .../controller/ibc_middleware_test.go | 10 +++--- .../controller/keeper/account_test.go | 32 +++++++++++++++---- .../controller/keeper/keeper_test.go | 2 +- .../host/ibc_module_test.go | 2 +- .../host/keeper/keeper_test.go | 2 +- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index dd7849efca9..f30248a0950 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -9,9 +9,9 @@ import ( "github.com/stretchr/testify/suite" "github.com/tendermint/tendermint/crypto" - icacontroller "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller" "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/types" icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types" + fee "github.com/cosmos/ibc-go/v3/modules/apps/29-fee" clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v3/modules/core/24-host" @@ -83,7 +83,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil { return err } @@ -730,9 +730,9 @@ func (suite *InterchainAccountsTestSuite) TestGetAppVersion() { cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) suite.Require().True(ok) - controllerModule := cbs.(icacontroller.IBCMiddleware) - - appVersion, found := controllerModule.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + controllerStack := cbs.(fee.IBCMiddleware) + appVersion, found := controllerStack.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) + suite.Require().Equal(path.EndpointA.ChannelConfig.Version, appVersion) } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/account_test.go b/modules/apps/27-interchain-accounts/controller/keeper/account_test.go index 9387baa3936..0f8c89c51a8 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/account_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/account_test.go @@ -72,7 +72,7 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount() { tc.malleate() // malleate mutates test data - err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), path.EndpointA.ConnectionID, owner) + err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), path.EndpointA.ConnectionID, owner, TestVersion) if tc.expPass { suite.Require().NoError(err) @@ -88,15 +88,33 @@ func (suite *KeeperTestSuite) TestRegisterSameOwnerMultipleConnections() { owner := TestOwnerAddress - path := NewICAPath(suite.chainA, suite.chainB) - suite.coordinator.SetupConnections(path) + pathAToB := NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(pathAToB) - path2 := NewICAPath(suite.chainA, suite.chainC) - suite.coordinator.SetupConnections(path2) + pathAToC := NewICAPath(suite.chainA, suite.chainC) + suite.coordinator.SetupConnections(pathAToC) - err := suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), path.EndpointA.ConnectionID, owner) + // build ICS27 metadata with connection identifiers for path A->B + metadata := &icatypes.Metadata{ + Version: icatypes.Version, + ControllerConnectionId: pathAToB.EndpointA.ConnectionID, + HostConnectionId: pathAToB.EndpointB.ConnectionID, + Encoding: icatypes.EncodingProtobuf, + TxType: icatypes.TxTypeSDKMultiMsg, + } + + err := suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToB.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata))) suite.Require().NoError(err) - err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), path2.EndpointA.ConnectionID, owner) + // build ICS27 metadata with connection identifiers for path A->C + metadata = &icatypes.Metadata{ + Version: icatypes.Version, + ControllerConnectionId: pathAToC.EndpointA.ConnectionID, + HostConnectionId: pathAToC.EndpointB.ConnectionID, + Encoding: icatypes.EncodingProtobuf, + TxType: icatypes.TxTypeSDKMultiMsg, + } + + err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToC.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata))) suite.Require().NoError(err) } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go index a0772f1e648..76020e17c87 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go @@ -95,7 +95,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil { return err } diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 3fa45384104..b2e9ad2266e 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -86,7 +86,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil { return err } diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go index 609fb06884d..98f9c06bf25 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go @@ -95,7 +95,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil { return err } From a243ac0445add28947037039766e5f9093d4cda3 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 12:05:05 +0200 Subject: [PATCH 06/12] adding migration docs for ICS27 fee middleware support --- docs/migrations/v3-to-v4.md | 60 ++++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/docs/migrations/v3-to-v4.md b/docs/migrations/v3-to-v4.md index 5f684581ab2..4afd6bcd7d3 100644 --- a/docs/migrations/v3-to-v4.md +++ b/docs/migrations/v3-to-v4.md @@ -18,7 +18,7 @@ No genesis or in-place migrations required when upgrading from v1 or v2 of ibc-g ## Chains -### IS04 - Channel +### ICS04 - Channel The `WriteAcknowledgement` API now takes the `exported.Acknowledgement` type instead of passing in the acknowledgement byte array directly. This is an API breaking change and as such IBC application developers will have to update any calls to `WriteAcknowledgement`. @@ -26,6 +26,64 @@ This is an API breaking change and as such IBC application developers will have The `OnChanOpenInit` application callback has been modified. The return signature now includes the application version as detailed in the latest IBC [spec changes](https://github.com/cosmos/ibc/pull/629). +### ICS27 - Interchain Accounts + +The `RegisterInterchainAccount` API has been modified to include an additional `version` argument. This change has been made in order to support ICS29 fee middelware, for relayer incentivization of ICS27 packets. +Consumers of the `RegisterInterchainAccount` are now expected to build the appropriate JSON encoded version string themselves and pass it accordingly. +This should be constructed within the interchain accounts authentication module which leverages the APIs exposed via the interchain accounts `controllerKeeper`. + +The following code snippet illustrates how to construct an appropriate interchain accounts Metadata and encode it as a JSON bytestring: + +```go +icaMetadata := icatypes.Metadata{ + Version: icatypes.Version, + ControllerConnectionId: controllerConnectionID, + HostConnectionId: hostConnectionID, + Encoding: icatypes.EncodingProtobuf, + TxType: icatypes.TxTypeSDKMultiMsg, +} + +appVersion, err := icatypes.ModuleCdc.MarshalJSON(&icaMetadata) +if err != nil { + retutn err +} + +if err := k.icaControllerKeeper.RegisterInterchainAccount(ctx, msg.ConnectionId, msg.Owner, string(appVersion)); err != nil { + return err +} +``` + +Similarly, if the application stack is configured to route through ICS29 fee middleware and a fee enabled channel is desired, construct the appropriate ICS29 Metadata type: + +```go +icaMetadata := icatypes.Metadata{ + Version: icatypes.Version, + ControllerConnectionId: controllerConnectionID, + HostConnectionId: hostConnectionID, + Encoding: icatypes.EncodingProtobuf, + TxType: icatypes.TxTypeSDKMultiMsg, +} + +appVersion, err := icatypes.ModuleCdc.MarshalJSON(&icaMetadata) +if err != nil { + return err +} + +feeMetadata := feetypes.Metadata{ + AppVersion: string(appVersion), + FeeVersion: feetypes.Version, +} + +feeEnabledVersion, err := feetypes.ModuleCdc.MarshalJSON(&feeMetadata) +if err != nil { + return err +} + +if err := k.icaControllerKeeper.RegisterInterchainAccount(ctx, msg.ConnectionId, msg.Owner, string(feeEnabledVersion)); err != nil { + return err +} +``` + ## Relayers When using the `DenomTrace` gRPC, the full IBC denomination with the `ibc/` prefix may now be passed in. From 2ea8a6912ae2c2f7377eb31bc5539dff799c1d05 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 12:13:53 +0200 Subject: [PATCH 07/12] remove unnecessary spacing --- .../27-interchain-accounts/controller/ibc_middleware_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 19bd303664d..1c3660c54b5 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -717,6 +717,5 @@ func (suite *InterchainAccountsTestSuite) TestGetAppVersion() { controllerStack := cbs.(fee.IBCMiddleware) appVersion, found := controllerStack.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) - suite.Require().Equal(path.EndpointA.ChannelConfig.Version, appVersion) } From 6bf4bf6736a5b71daf92b07be94ceff0a677eb30 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 12:15:25 +0200 Subject: [PATCH 08/12] fixing typo in godoc --- .../apps/27-interchain-accounts/controller/keeper/account.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/account.go b/modules/apps/27-interchain-accounts/controller/keeper/account.go index 9e2d8ce5199..2cb5e023e22 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/account.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/account.go @@ -10,7 +10,7 @@ import ( ) // RegisterInterchainAccount is the entry point to registering an interchain account: -// - It generates a new port identifier using the provided owner string, binds to the port identifier and clais the associated capability. +// - It generates a new port identifier using the provided owner string, binds to the port identifier and claims the associated capability. // - Callers are expected to provide the appropriate application version string. // - For example, this could be an ICS27 encoded metadata type or an ICS29 encoded metadata type with a nested application version. // - A new MsgChannelOpenInit is routed through the MsgServiceRouter, executing the OnOpenChanInit callback stack as configured. From 2daa515c87d84dce69acecac1afb23d762b278a7 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 12:25:53 +0200 Subject: [PATCH 09/12] adding changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 465c738b2ee..b40379b2b22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (channel) [\#1283](https://github.com/cosmos/ibc-go/pull/1283) The `OnChanOpenInit` application callback now returns a version string in line with the latest [spec changes](https://github.com/cosmos/ibc/pull/629). * (modules/29-fee)[\#1338](https://github.com/cosmos/ibc-go/pull/1338) Renaming `Result` field in `IncentivizedAcknowledgement` to `AppAcknowledgement`. * (modules/29-fee)[\#1343](https://github.com/cosmos/ibc-go/pull/1343) Renaming `KeyForwardRelayerAddress` to `KeyRelayerAddressForAsyncAck`, and `ParseKeyForwardRelayerAddress` to `ParseKeyRelayerAddressForAsyncAck`. +* (apps/27-interchain-accounts)[\#1432](https://github.com/cosmos/ibc-go/pull/1432) Updating `RegisterInterchainAccount` to include an additional `version` argument, supporting ICS29 fee middleware functionality in ICS27 interchain accounts. ### State Machine Breaking From 7af6de30826229d688be825bb742d6d6deda3e1d Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 25 May 2022 12:49:44 +0200 Subject: [PATCH 10/12] adding godoc for NewICAPath in fee test suite --- modules/apps/29-fee/ica_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/apps/29-fee/ica_test.go b/modules/apps/29-fee/ica_test.go index e6022356cc7..394c3b1736a 100644 --- a/modules/apps/29-fee/ica_test.go +++ b/modules/apps/29-fee/ica_test.go @@ -30,6 +30,7 @@ var ( })) ) +// NewICAPath creates and returns a new ibctesting path configured for a fee enabled interchain accounts channel func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { path := ibctesting.NewPath(chainA, chainB) From 2f56126e79e5faac3ca968430483841f6a25e803 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 3 Jun 2022 11:22:24 +0200 Subject: [PATCH 11/12] adding helper for default metadata version string to ics27 --- .../apps/27-interchain-accounts/types/metadata.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go index 3a7eae51cdf..15f27314d47 100644 --- a/modules/apps/27-interchain-accounts/types/metadata.go +++ b/modules/apps/27-interchain-accounts/types/metadata.go @@ -27,6 +27,20 @@ func NewMetadata(version, controllerConnectionID, hostConnectionID, accAddress, } } +// NewDefaultMetadataString creates and returns a new JSON encoded version string containing the default ICS27 Metadata values +// with the provided controller and host connection identifiers +func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) string { + metadata := Metadata{ + Version: Version, + ControllerConnectionId: controllerConnectionID, + HostConnectionId: hostConnectionID, + Encoding: EncodingProtobuf, + TxType: TxTypeSDKMultiMsg, + } + + return string(ModuleCdc.MustMarshalJSON(&metadata)) +} + // IsPreviousMetadataEqual compares a metadata to a previous version string set in a channel struct. // It ensures all fields are equal except the Address string func IsPreviousMetadataEqual(previousVersion string, metadata Metadata) bool { From de3648b9c9d206321f95a751b9688ebed01a985d Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 3 Jun 2022 11:23:08 +0200 Subject: [PATCH 12/12] unexported vars, register counterparty address for recv fee distribution, add comments and adjust assertions --- modules/apps/29-fee/ica_test.go | 53 ++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/modules/apps/29-fee/ica_test.go b/modules/apps/29-fee/ica_test.go index 394c3b1736a..88b3d088412 100644 --- a/modules/apps/29-fee/ica_test.go +++ b/modules/apps/29-fee/ica_test.go @@ -14,39 +14,38 @@ import ( ) var ( - // DefaultOwnerAddress defines a reusable bech32 address for testing purposes - DefaultOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" - - // DefaultPortID defines a resuable port identifier for testing purposes - DefaultPortID, _ = icatypes.NewControllerPortID(DefaultOwnerAddress) - - // DefaultICAVersion defines a resuable interchainaccounts version string for testing purposes - DefaultICAVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{ - Version: icatypes.Version, - ControllerConnectionId: ibctesting.FirstConnectionID, - HostConnectionId: ibctesting.FirstConnectionID, - Encoding: icatypes.EncodingProtobuf, - TxType: icatypes.TxTypeSDKMultiMsg, - })) + // defaultOwnerAddress defines a reusable bech32 address for testing purposes + defaultOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" + + // defaultPortID defines a resuable port identifier for testing purposes + defaultPortID, _ = icatypes.NewControllerPortID(defaultOwnerAddress) + + // defaultICAVersion defines a resuable interchainaccounts version string for testing purposes + defaultICAVersion = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) ) -// NewICAPath creates and returns a new ibctesting path configured for a fee enabled interchain accounts channel -func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { +// 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) - feeICAVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: DefaultICAVersion})) + 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.EndpointA.ChannelConfig.PortID = defaultPortID path.EndpointB.ChannelConfig.PortID = icatypes.PortID return path } -// SetupICAPath performs the InterchainAccounts channel creation handshake using an ibctesting path -func SetupICAPath(path *ibctesting.Path, owner string) error { +// 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 } @@ -66,7 +65,8 @@ func SetupICAPath(path *ibctesting.Path, owner string) error { return nil } -// RegisterInterchainAccount invokes the the InterchainAccounts entrypoint, commits state changes and updates the testing endpoint accordingly +// 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 { @@ -91,16 +91,19 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro // TestFeeInterchainAccounts Integration test to ensure ics29 works with ics27 func (suite *FeeTestSuite) TestFeeInterchainAccounts() { - path := NewICAPath(suite.chainA, suite.chainB) + path := NewIncentivizedICAPath(suite.chainA, suite.chainB) suite.coordinator.SetupConnections(path) - err := SetupICAPath(path, DefaultOwnerAddress) + err := SetupPath(path, defaultOwnerAddress) suite.Require().NoError(err) // assert the newly established channel is fee enabled on both ends suite.Require().True(suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) suite.Require().True(suite.chainB.GetSimApp().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 + suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(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) @@ -172,8 +175,10 @@ func (suite *FeeTestSuite) TestFeeInterchainAccounts() { 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 := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom) - suite.Require().Equal(preEscrowBalance, postDistBalance) + suite.Require().Equal(preEscrowBalance.SubAmount(defaultRecvFee.AmountOf(sdk.DefaultBondDenom)), postDistBalance) } func buildInterchainAccountsPacket(path *ibctesting.Path, data []byte, seq uint64) channeltypes.Packet {