From 71d7480c923f4227453e8a80f51be01ae7ee845e Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 22 Jun 2022 16:56:29 +0200 Subject: [PATCH 1/4] refactor: writing test case for module account incentivizing packet (#1397) * refactor: using SendCoins & writing test case for module account incentivizing packet * updating ModuleAccountAddrs helper fn and adding additional test for refunding to module acc * chore: changelog --- CHANGELOG.md | 1 + modules/apps/29-fee/keeper/escrow_test.go | 50 ++++++++++++++++--- modules/apps/29-fee/keeper/msg_server_test.go | 13 ++++- testing/simapp/app.go | 8 +++ 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e33c591d130..cb6e080f5f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (modules/core/04-channel) [\#1232](https://github.com/cosmos/ibc-go/pull/1232) Updating params on `NewPacketId` and moving to bottom of file. * (app/29-fee) [\#1305](https://github.com/cosmos/ibc-go/pull/1305) Change version string for fee module to `ics29-1` * (app/29-fee) [\#1341](https://github.com/cosmos/ibc-go/pull/1341) Check if the fee module is locked and if the fee module is enabled before refunding all fees +* (testing/simapp) [\#1397](https://github.com/cosmos/ibc-go/pull/1397) Adding mock module to maccperms and adding check to ensure mock module is not a blocked account address. ### Features diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 7ccf87aa91b..0ab829877ea 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -5,6 +5,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" + "github.com/cosmos/ibc-go/v3/testing/mock" "github.com/tendermint/tendermint/crypto/secp256k1" ) @@ -18,6 +19,7 @@ func (suite *KeeperTestSuite) TestDistributeFee() { refundAccBal sdk.Coin packetFee types.PacketFee packetFees []types.PacketFee + fee types.Fee ) testCases := []struct { @@ -27,7 +29,10 @@ func (suite *KeeperTestSuite) TestDistributeFee() { }{ { "success", - func() {}, + func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + }, func() { // check if fees has been deleted packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) @@ -56,8 +61,30 @@ func (suite *KeeperTestSuite) TestDistributeFee() { suite.Require().Equal(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0)), balance) }, }, + { + "success: refund account is module account", + func() { + refundAcc = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(mock.ModuleName) + + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + + // fund mock account + err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), mock.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...)) + suite.Require().NoError(err) + }, + func() { + // check if the refund acc has been refunded the timeoutFee + expectedRefundAccBal := defaultTimeoutFee[0].Add(defaultTimeoutFee[0]) + balance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom) + suite.Require().Equal(expectedRefundAccBal, balance) + }, + }, { "escrow account out of balance, fee module becomes locked - no distribution", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + // pass in an extra packet fee packetFees = append(packetFees, packetFee) }, @@ -76,6 +103,9 @@ func (suite *KeeperTestSuite) TestDistributeFee() { { "invalid forward address", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + forwardRelayer = "invalid address" }, func() { @@ -88,6 +118,9 @@ func (suite *KeeperTestSuite) TestDistributeFee() { { "invalid forward address: blocked address", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + forwardRelayer = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress().String() }, func() { @@ -100,6 +133,9 @@ func (suite *KeeperTestSuite) TestDistributeFee() { { "invalid receiver address: ack fee returned to sender", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + reverseRelayer = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress() }, func() { @@ -112,6 +148,9 @@ func (suite *KeeperTestSuite) TestDistributeFee() { { "invalid refund address: no-op, timeout fee remains in escrow", func() { + packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) + packetFees = []types.PacketFee{packetFee, packetFee} + packetFees[0].RefundAddress = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress().String() packetFees[1].RefundAddress = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress().String() }, @@ -137,18 +176,15 @@ func (suite *KeeperTestSuite) TestDistributeFee() { refundAcc = suite.chainA.SenderAccount.GetAddress() packetID := channeltypes.NewPacketId(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1) - fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + fee = types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) - // escrow the packet fees & store the fees in state - packetFee = types.NewPacketFee(fee, refundAcc.String(), []string{}) - packetFees = []types.PacketFee{packetFee, packetFee} + tc.malleate() + // escrow the packet fees & store the fees in state suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees(packetFees)) err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, packetFee.Fee.Total().Add(packetFee.Fee.Total()...)) suite.Require().NoError(err) - tc.malleate() - // fetch the account balances before fee distribution (forward, reverse, refund) forwardAccAddress, _ := sdk.AccAddressFromBech32(forwardRelayer) forwardRelayerBal = suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), forwardAccAddress, sdk.DefaultBondDenom) diff --git a/modules/apps/29-fee/keeper/msg_server_test.go b/modules/apps/29-fee/keeper/msg_server_test.go index 9a05f294c9d..cdcbe574d1c 100644 --- a/modules/apps/29-fee/keeper/msg_server_test.go +++ b/modules/apps/29-fee/keeper/msg_server_test.go @@ -3,6 +3,7 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + disttypes "github.com/cosmos/cosmos-sdk/x/distribution/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" @@ -152,6 +153,7 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { expEscrowBalance sdk.Coins expFeesInEscrow []types.PacketFee msg *types.MsgPayPacketFee + fee types.Fee ) testCases := []struct { @@ -182,6 +184,15 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { }, true, }, + { + "refund account is module account", + func() { + msg.Signer = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(disttypes.ModuleName).String() + expPacketFee := types.NewPacketFee(fee, msg.Signer, nil) + expFeesInEscrow = []types.PacketFee{expPacketFee} + }, + true, + }, { "fee module is locked", func() { @@ -241,7 +252,7 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { suite.SetupTest() suite.coordinator.Setup(suite.path) // setup channel - fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) + fee = types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) msg = types.NewMsgPayPacketFee( fee, suite.path.EndpointA.ChannelConfig.PortID, diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 94d5640aa17..cb6c1cdccb3 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -43,6 +43,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/capability" capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" + "github.com/cosmos/ibc-go/v3/testing/mock" simappparams "github.com/cosmos/ibc-go/v3/testing/simapp/params" "github.com/cosmos/cosmos-sdk/x/crisis" @@ -164,6 +165,7 @@ var ( ibctransfertypes.ModuleName: {authtypes.Minter, authtypes.Burner}, ibcfeetypes.ModuleName: nil, icatypes.ModuleName: nil, + mock.ModuleName: nil, } ) @@ -654,6 +656,12 @@ func (app *SimApp) LoadHeight(height int64) error { func (app *SimApp) ModuleAccountAddrs() map[string]bool { modAccAddrs := make(map[string]bool) for acc := range maccPerms { + // do not add mock module to blocked addresses + // this is only used for testing + if acc == mock.ModuleName { + continue + } + modAccAddrs[authtypes.NewModuleAddress(acc).String()] = true } From dbd0f774803fc08a545002da1e7474e357a63c86 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 23 Jun 2022 11:46:20 +0200 Subject: [PATCH 2/4] chore: ics29 fee payee event emission (#1568) * adding event emission for registered payees and counterparty payees in ics29 * updating naming of event emission funcs --- modules/apps/29-fee/keeper/events.go | 32 ++++++++++++++++++++++++ modules/apps/29-fee/keeper/msg_server.go | 4 +++ modules/apps/29-fee/types/events.go | 14 ++++++++--- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/modules/apps/29-fee/keeper/events.go b/modules/apps/29-fee/keeper/events.go index d60866e21d2..36207dbc5db 100644 --- a/modules/apps/29-fee/keeper/events.go +++ b/modules/apps/29-fee/keeper/events.go @@ -39,3 +39,35 @@ func EmitIncentivizedPacketEvent(ctx sdk.Context, packetID channeltypes.PacketId ), ) } + +// EmitRegisterPayeeEvent emits an event containing information of a registered payee for a relayer on a particular channel +func EmitRegisterPayeeEvent(ctx sdk.Context, relayer, payee, channelID string) { + ctx.EventManager().EmitEvents(sdk.Events{ + sdk.NewEvent( + types.EventTypeRegisterPayee, + sdk.NewAttribute(types.AttributeKeyRelayer, relayer), + sdk.NewAttribute(types.AttributeKeyPayee, payee), + sdk.NewAttribute(types.AttributeKeyChannelID, channelID), + ), + sdk.NewEvent( + sdk.EventTypeMessage, + sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), + ), + }) +} + +// EmitRegisterCounterpartyPayeeEvent emits an event containing information of a registered counterparty payee for a relayer on a particular channel +func EmitRegisterCounterpartyPayeeEvent(ctx sdk.Context, relayer, counterpartyPayee, channelID string) { + ctx.EventManager().EmitEvents(sdk.Events{ + sdk.NewEvent( + types.EventTypeRegisterCounterpartyPayee, + sdk.NewAttribute(types.AttributeKeyRelayer, relayer), + sdk.NewAttribute(types.AttributeKeyCounterpartyPayee, counterpartyPayee), + sdk.NewAttribute(types.AttributeKeyChannelID, channelID), + ), + sdk.NewEvent( + sdk.EventTypeMessage, + sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), + ), + }) +} diff --git a/modules/apps/29-fee/keeper/msg_server.go b/modules/apps/29-fee/keeper/msg_server.go index fe7af407d2f..1acb153594f 100644 --- a/modules/apps/29-fee/keeper/msg_server.go +++ b/modules/apps/29-fee/keeper/msg_server.go @@ -33,6 +33,8 @@ func (k Keeper) RegisterPayee(goCtx context.Context, msg *types.MsgRegisterPayee k.Logger(ctx).Info("registering payee address for relayer", "relayer", msg.Relayer, "payee", msg.Payee, "channel", msg.ChannelId) + EmitRegisterPayeeEvent(ctx, msg.Relayer, msg.Payee, msg.ChannelId) + return &types.MsgRegisterPayeeResponse{}, nil } @@ -57,6 +59,8 @@ func (k Keeper) RegisterCounterpartyPayee(goCtx context.Context, msg *types.MsgR k.Logger(ctx).Info("registering counterparty payee for relayer", "relayer", msg.Relayer, "counterparty payee", msg.CounterpartyPayee, "channel", msg.ChannelId) + EmitRegisterCounterpartyPayeeEvent(ctx, msg.Relayer, msg.CounterpartyPayee, msg.ChannelId) + return &types.MsgRegisterCounterpartyPayeeResponse{}, nil } diff --git a/modules/apps/29-fee/types/events.go b/modules/apps/29-fee/types/events.go index cac882d98d3..cffca5dabdd 100644 --- a/modules/apps/29-fee/types/events.go +++ b/modules/apps/29-fee/types/events.go @@ -2,9 +2,15 @@ package types // 29-fee events const ( - EventTypeIncentivizedPacket = "incentivized_ibc_packet" + EventTypeIncentivizedPacket = "incentivized_ibc_packet" + EventTypeRegisterPayee = "register_payee" + EventTypeRegisterCounterpartyPayee = "register_counterparty_payee" - AttributeKeyRecvFee = "recv_fee" - AttributeKeyAckFee = "ack_fee" - AttributeKeyTimeoutFee = "timeout_fee" + AttributeKeyRecvFee = "recv_fee" + AttributeKeyAckFee = "ack_fee" + AttributeKeyTimeoutFee = "timeout_fee" + AttributeKeyChannelID = "channel_id" + AttributeKeyRelayer = "relayer" + AttributeKeyPayee = "payee" + AttributeKeyCounterpartyPayee = "counterparty_payee" ) From 8ffa912d32ff6adbffc5c87aac2f1a26f54ea42b Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 23 Jun 2022 16:31:01 +0200 Subject: [PATCH 3/4] chore: allow ics27 to select and return default JSON encoded metadata (#1550) * adding distribution logic with new registered payees * updating ack test cases * removing commented out test * adding additional testcase to TestOnAcknowledgementPacket * cleaning up distribution logic and adapting tests * updating timeout test cases * updating timeout logic and adding application callback failure test cases * Apply suggestions from code review Co-authored-by: Charly * WIP handling empty version string for ics27 * updating testcases * adding test with auth module modifying channel version * adding inline comments re. ignored channel version from auth module * updating with deduplication suggestion Co-authored-by: Cian Hatton Co-authored-by: Charly --- .../controller/ibc_middleware.go | 3 +- .../controller/ibc_middleware_test.go | 26 ++++++++--------- .../controller/keeper/handshake.go | 29 ++++++++++++------- .../controller/keeper/handshake_test.go | 22 ++++++++++++-- .../27-interchain-accounts/types/metadata.go | 16 +++++++--- 5 files changed, 65 insertions(+), 31 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index b0b6bd6767c..b7c65874bfa 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -50,7 +50,8 @@ func (im IBCMiddleware) OnChanOpenInit( return "", types.ErrControllerSubModuleDisabled } - if err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil { + version, err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version) + if err != nil { return "", err } 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 1c3660c54b5..7656c665333 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -130,6 +130,18 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { { "success", func() {}, true, }, + { + "ICA auth module modification of channel version is ignored", func() { + // NOTE: explicitly modify the channel version via the auth module callback, + // ensuring the expected JSON encoded metadata is not modified upon return + suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string, + portID, channelID string, chanCap *capabilitytypes.Capability, + counterparty channeltypes.Counterparty, version string, + ) (string, error) { + return "invalid-version", nil + } + }, true, + }, { "controller submodule disabled", func() { suite.chainA.GetSimApp().ICAControllerKeeper.SetParams(suite.chainA.GetContext(), types.NewParams(false)) @@ -200,19 +212,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { ) if tc.expPass { - expMetadata := icatypes.NewMetadata( - icatypes.Version, - path.EndpointA.ConnectionID, - path.EndpointB.ConnectionID, - "", - icatypes.EncodingProtobuf, - icatypes.TxTypeSDKMultiMsg, - ) - - expBytes, err := icatypes.ModuleCdc.MarshalJSON(&expMetadata) - suite.Require().NoError(err) - - suite.Require().Equal(version, string(expBytes)) + suite.Require().Equal(icatypes.NewDefaultMetadataString(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID), version) suite.Require().NoError(err) } else { suite.Require().Error(err) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index 6b84fb554ca..c9f72ae3380 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go @@ -28,26 +28,35 @@ func (k Keeper) OnChanOpenInit( chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, -) error { +) (string, error) { if order != channeltypes.ORDERED { - return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, order) + return "", sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, order) } if !strings.HasPrefix(portID, icatypes.PortPrefix) { - return sdkerrors.Wrapf(icatypes.ErrInvalidControllerPort, "expected %s{owner-account-address}, got %s", icatypes.PortPrefix, portID) + return "", sdkerrors.Wrapf(icatypes.ErrInvalidControllerPort, "expected %s{owner-account-address}, got %s", icatypes.PortPrefix, portID) } if counterparty.PortId != icatypes.PortID { - return sdkerrors.Wrapf(icatypes.ErrInvalidHostPort, "expected %s, got %s", icatypes.PortID, counterparty.PortId) + return "", sdkerrors.Wrapf(icatypes.ErrInvalidHostPort, "expected %s, got %s", icatypes.PortID, counterparty.PortId) } var metadata icatypes.Metadata - if err := icatypes.ModuleCdc.UnmarshalJSON([]byte(version), &metadata); err != nil { - return sdkerrors.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") + if strings.TrimSpace(version) == "" { + connection, err := k.channelKeeper.GetConnection(ctx, connectionHops[0]) + if err != nil { + return "", err + } + + metadata = icatypes.NewDefaultMetadata(connectionHops[0], connection.GetCounterparty().GetConnectionID()) + } else { + if err := icatypes.ModuleCdc.UnmarshalJSON([]byte(version), &metadata); err != nil { + return "", sdkerrors.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") + } } if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil { - return err + return "", err } activeChannelID, found := k.GetActiveChannelID(ctx, connectionHops[0], portID) @@ -58,15 +67,15 @@ func (k Keeper) OnChanOpenInit( } if channel.State == channeltypes.OPEN { - return sdkerrors.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s is already OPEN", activeChannelID, portID) + return "", sdkerrors.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s is already OPEN", activeChannelID, portID) } if !icatypes.IsPreviousMetadataEqual(channel.Version, metadata) { - return sdkerrors.Wrap(icatypes.ErrInvalidVersion, "previous active channel metadata does not match provided version") + return "", sdkerrors.Wrap(icatypes.ErrInvalidVersion, "previous active channel metadata does not match provided version") } } - return nil + return string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)), nil } // OnChanOpenAck sets the active channel for the interchain account/owner pair diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index f2104e2032d..b0cb9c3a125 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -30,7 +30,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { true, }, { - "success - previous active channel closed", + "success: previous active channel closed", func() { suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) @@ -47,6 +47,13 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { }, true, }, + { + "success: empty channel version returns default metadata JSON string", + func() { + channel.Version = "" + }, + true, + }, { "invalid metadata - previous metadata is different", func() { @@ -138,6 +145,14 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { }, false, }, + { + "connection not found with default empty channel version", + func() { + channel.ConnectionHops = []string{"connection-10"} + channel.Version = "" + }, + false, + }, { "invalid controller connection ID", func() { @@ -214,7 +229,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { path.EndpointA.ChannelConfig.PortID = portID // default values - metadata = icatypes.NewMetadata(icatypes.Version, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID, "", icatypes.EncodingProtobuf, icatypes.TxTypeSDKMultiMsg) + metadata = icatypes.NewMetadata(icatypes.Version, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID, "", icatypes.EncodingProtobuf, icatypes.TxTypeSDKMultiMsg) versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) suite.Require().NoError(err) @@ -232,12 +247,13 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { tc.malleate() // malleate mutates test data - err = suite.chainA.GetSimApp().ICAControllerKeeper.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), + version, err := suite.chainA.GetSimApp().ICAControllerKeeper.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.Version, ) if tc.expPass { suite.Require().NoError(err) + suite.Require().Equal(string(versionBytes), version) } else { suite.Require().Error(err) } diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go index 15f27314d47..dd14c4788ec 100644 --- a/modules/apps/27-interchain-accounts/types/metadata.go +++ b/modules/apps/27-interchain-accounts/types/metadata.go @@ -27,17 +27,25 @@ func NewMetadata(version, controllerConnectionID, hostConnectionID, accAddress, } } -// NewDefaultMetadataString creates and returns a new JSON encoded version string containing the default ICS27 Metadata values +// NewDefaultMetadata creates and returns a new ICS27 Metadata instance containing the default ICS27 Metadata values // with the provided controller and host connection identifiers -func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) string { +func NewDefaultMetadata(connectionConnectionID, hostConnectionID string) Metadata { metadata := Metadata{ - Version: Version, - ControllerConnectionId: controllerConnectionID, + ControllerConnectionId: connectionConnectionID, HostConnectionId: hostConnectionID, Encoding: EncodingProtobuf, TxType: TxTypeSDKMultiMsg, + Version: Version, } + return metadata +} + +// 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 := NewDefaultMetadata(controllerConnectionID, hostConnectionID) + return string(ModuleCdc.MustMarshalJSON(&metadata)) } From 5467300423eeadae68d0ac41db20c4b8c5315970 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 23 Jun 2022 16:34:00 +0200 Subject: [PATCH 4/4] docs: ics29 middleware usage for app devs (#1567) * adding integration doc for app wiring configuration * adjusting wording * updating grammar * updating vuepress config * adding explicit comment regarding intended audience of cosmos sdk devs * adding explicit reference to app.go in transfer and ica examples * fixing indentation in vuepress config.js * fixing more indentation --- docs/.vuepress/config.js | 25 ++++--- docs/middleware/ics29-fee/integration.md | 85 ++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 10 deletions(-) create mode 100644 docs/middleware/ics29-fee/integration.md diff --git a/docs/.vuepress/config.js b/docs/.vuepress/config.js index aaf668ffc97..663a40a093b 100644 --- a/docs/.vuepress/config.js +++ b/docs/.vuepress/config.js @@ -145,9 +145,9 @@ module.exports = { path: "/apps", children: [ { - title: "Overview", - directory: false, - path: "/apps/interchain-accounts/overview.html" + title: "Overview", + directory: false, + path: "/apps/interchain-accounts/overview.html" }, { title: "Authentication Modules", @@ -160,9 +160,9 @@ module.exports = { path: "/apps/interchain-accounts/active-channels.html" }, { - title: "Integration", - directory: false, - path: "/apps/interchain-accounts/integration.html" + title: "Integration", + directory: false, + path: "/apps/interchain-accounts/integration.html" }, { title: "Parameters", @@ -187,11 +187,16 @@ module.exports = { path: "/middleware", children: [ { - title: "Overview", - directory: false, - path: "/middleware/ics29-fee/overview.html" + title: "Overview", + directory: false, + path: "/middleware/ics29-fee/overview.html" }, - ] + { + title: "Integration", + directory: false, + path: "/middleware/ics29-fee/integration.html" + }, + ] }, ] }, diff --git a/docs/middleware/ics29-fee/integration.md b/docs/middleware/ics29-fee/integration.md new file mode 100644 index 00000000000..f1d0158e702 --- /dev/null +++ b/docs/middleware/ics29-fee/integration.md @@ -0,0 +1,85 @@ + + +# Integration + +Learn how to configure the Fee Middleware module with IBC applications. The following document is intended for developers building on top of the Cosmos SDK and only applies for Cosmos SDK chains. {synopsis} + +## Pre-requisite Readings + +* [IBC middleware development](../../ibc/middleware/develop.md) {prereq} +* [IBC middleware integration](../../ibc/middleware/integration.md) {prereq} + +The Fee Middleware module, as the name suggests, plays the role of an IBC middleware and as such must be configured by chain developers to route and handle IBC messages correctly. +For Cosmos SDK chains this setup is done via the `app/app.go` file, where modules are constructed and configured in order to bootstrap the blockchain application. + +## Configuring an application stack with Fee Middleware + +As mentioned in [IBC middleware development](../../ibc/middleware/develop.md) an application stack may be composed of many or no middlewares that nest a base application. +These layers form the complete set of application logic that enable developers to build composable and flexible IBC application stacks. +For example, an application stack may be just a single base application like `transfer`, however, the same application stack composed with `29-fee` will nest the `transfer` base application +by wrapping it with the Fee Middleware module. + + +### Transfer + +See below for an example of how to create an application stack using `transfer` and `29-fee`. +The following `transferStack` is configured in `app/app.go` and added to the IBC `Router`. +The in-line comments describe the execution flow of packets between the application stack and IBC core. + +```go +// Create Transfer Stack +// SendPacket, since it is originating from the application to core IBC: +// transferKeeper.SendPacket -> fee.SendPacket -> channel.SendPacket + +// RecvPacket, message that originates from core IBC and goes down to app, the flow is the other way +// channel.RecvPacket -> fee.OnRecvPacket -> transfer.OnRecvPacket + +// transfer stack contains (from top to bottom): +// - IBC Fee Middleware +// - Transfer + +// create IBC module from bottom to top of stack +var transferStack porttypes.IBCModule +transferStack = transfer.NewIBCModule(app.TransferKeeper) +transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper) + +// Add transfer stack to IBC Router +ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack) +``` + +### Interchain Accounts + +See below for an example of how to create an application stack using `27-interchain-accounts` and `29-fee`. +The following `icaControllerStack` and `icaHostStack` are configured in `app/app.go` and added to the IBC `Router` with the associated authentication module. +The in-line comments describe the execution flow of packets between the application stack and IBC core. + +```go +// Create Interchain Accounts Stack +// SendPacket, since it is originating from the application to core IBC: +// 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) + +// 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 authentication module, controller and host to IBC router +ibcRouter. + // the ICA Controller middleware needs to be explicitly added to the IBC Router because the + // ICA controller module owns the port capability for ICA. The ICA authentication module + // owns the channel capability. + AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack) + AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). + AddRoute(icahosttypes.SubModuleName, icaHostStack). +``` \ No newline at end of file