From 0504176d80957c7bde6a46b44fed173188c3a1f7 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 18 Nov 2021 14:18:05 +0100 Subject: [PATCH 1/5] add iterate logic --- modules/apps/29-fee/keeper/escrow.go | 4 ++++ modules/apps/29-fee/keeper/keeper.go | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index cddb3be047a..f552e471a25 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -104,3 +104,7 @@ func (k Keeper) DistributeFeeTimeout(ctx sdk.Context, refundAcc, timeoutRelayer return nil } + +func (k Keeper) RefundAllFees(ctx sdk.Context) { + // TODO +} diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index 3691204d2a6..d18ca8a79e7 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -144,6 +144,21 @@ func (k Keeper) GetFeeInEscrow(ctx sdk.Context, packetId *channeltypes.PacketId) return fee, true } +// IterateFeeInEscrow iterates over all the fees currently escrowed and calls the provided callback +// if the callback returns true, then iteration is stopped. +func (k Keeper) IterateFeeInEscrow(ctx sdk.Context, cb func(identifiedFee types.IdentifiedPacketFee) (stop bool)) { + store := ctx.KVStore(k.storeKey) + iterator := sdk.KVStorePrefixIterator(store, []byte(types.FeeInEscrowPrefix)) + + defer iterator.Close() + for ; iterator.Valid(); iterator.Next() { + identifiedFee := k.MustUnmarshalFee(iterator.Value()) + if cb(identifiedFee) { + break + } + } +} + // Deletes the fee associated with the given packetId func (k Keeper) DeleteFeeInEscrow(ctx sdk.Context, packetId *channeltypes.PacketId) { store := ctx.KVStore(k.storeKey) From 21c86be385e637080211fe9ee827b3bfd741dc9d Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 19 Nov 2021 15:45:15 +0100 Subject: [PATCH 2/5] add closing logic with tests --- modules/apps/29-fee/ibc_module.go | 14 +- modules/apps/29-fee/ibc_module_test.go | 148 ++++++++++++++++++ modules/apps/29-fee/keeper/escrow.go | 37 ++++- modules/apps/29-fee/keeper/escrow_test.go | 50 ++++++ modules/apps/29-fee/keeper/keeper.go | 8 +- modules/apps/29-fee/types/expected_keepers.go | 1 + modules/apps/29-fee/types/keys.go | 7 +- 7 files changed, 256 insertions(+), 9 deletions(-) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index 0c22fb19d8d..2dd3893249c 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -130,8 +130,13 @@ func (im IBCModule) OnChanCloseInit( portID, channelID string, ) error { - // TODO: Unescrow all remaining funds for unprocessed packets + // delete fee enabled on channel + // and refund any remaining fees escrowed on channel im.keeper.DeleteFeeEnabled(ctx, portID, channelID) + err := im.keeper.RefundFeesOnChannel(ctx, portID, channelID) + if err != nil { + panic(err) + } return im.app.OnChanCloseInit(ctx, portID, channelID) } @@ -141,8 +146,13 @@ func (im IBCModule) OnChanCloseConfirm( portID, channelID string, ) error { - // TODO: Unescrow all remaining funds for unprocessed packets + // delete fee enabled on channel + // and refund any remaining fees escrowed on channel im.keeper.DeleteFeeEnabled(ctx, portID, channelID) + err := im.keeper.RefundFeesOnChannel(ctx, portID, channelID) + if err != nil { + panic(err) + } return im.app.OnChanCloseConfirm(ctx, portID, channelID) } diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index 915fdaebf79..77eb7801c6d 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -3,6 +3,7 @@ package fee_test import ( "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" @@ -12,6 +13,13 @@ import ( ibctesting "github.com/cosmos/ibc-go/testing" ) +var ( + validCoins = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}} + validCoins2 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(200)}} + validCoins3 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(300)}} + invalidCoins = sdk.Coins{sdk.Coin{Denom: "invalidDenom", Amount: sdk.NewInt(100)}} +) + // Tests OnChanOpenInit on ChainA func (suite *FeeTestSuite) TestOnChanOpenInit() { testCases := []struct { @@ -300,3 +308,143 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { }) } } + +// Tests OnChanCloseInit on chainA +func (suite *FeeTestSuite) TestOnChanCloseInit() { + testCases := []struct { + name string + setup func(suite *FeeTestSuite) + panics bool + }{ + { + "success", + func(suite *FeeTestSuite) { + packetId := channeltypes.PacketId{ + PortId: suite.path.EndpointA.ChannelConfig.PortID, + ChannelId: suite.path.EndpointA.ChannelID, + Sequence: 1, + } + refundAcc := suite.chainA.SenderAccount.GetAddress() + identifiedFee := types.NewIdentifiedPacketFee(&packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{}) + err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee) + suite.Require().NoError(err) + }, + false, + }, + { + "module account balance insufficient", + func(suite *FeeTestSuite) { + packetId := channeltypes.PacketId{ + PortId: suite.path.EndpointA.ChannelConfig.PortID, + ChannelId: suite.path.EndpointA.ChannelID, + Sequence: 1, + } + refundAcc := suite.chainA.SenderAccount.GetAddress() + identifiedFee := types.NewIdentifiedPacketFee(&packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{}) + err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee) + suite.Require().NoError(err) + + suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, refundAcc, validCoins3) + }, + true, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + suite.coordinator.Setup(suite.path) + + origBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress()) + + tc.setup(suite) + + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + if tc.panics { + suite.Require().Panics(func() { + cbs.OnChanCloseInit(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + }, "did not panic as expected on refund fees") + } else { + cbs.OnChanCloseInit(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + afterBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress()) + suite.Require().Equal(origBal, afterBal, "balances of refund account not equal after all fees refunded") + } + }) + } +} + +// Tests OnChanCloseConfirm on chainA +func (suite *FeeTestSuite) TestOnChanCloseConfirm() { + testCases := []struct { + name string + setup func(suite *FeeTestSuite) + panics bool + }{ + { + "success", + func(suite *FeeTestSuite) { + packetId := channeltypes.PacketId{ + PortId: suite.path.EndpointA.ChannelConfig.PortID, + ChannelId: suite.path.EndpointA.ChannelID, + Sequence: 1, + } + refundAcc := suite.chainA.SenderAccount.GetAddress() + identifiedFee := types.NewIdentifiedPacketFee(&packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{}) + err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee) + suite.Require().NoError(err) + }, + false, + }, + { + "module account balance insufficient", + func(suite *FeeTestSuite) { + packetId := channeltypes.PacketId{ + PortId: suite.path.EndpointA.ChannelConfig.PortID, + ChannelId: suite.path.EndpointA.ChannelID, + Sequence: 1, + } + refundAcc := suite.chainA.SenderAccount.GetAddress() + identifiedFee := types.NewIdentifiedPacketFee(&packetId, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{}) + err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), identifiedFee) + suite.Require().NoError(err) + + suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, refundAcc, validCoins3) + }, + true, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + suite.coordinator.Setup(suite.path) + + origBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress()) + + tc.setup(suite) + + module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort) + suite.Require().NoError(err) + + cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) + suite.Require().True(ok) + + if tc.panics { + suite.Require().Panics(func() { + cbs.OnChanCloseConfirm(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + }, "did not panic as expected on refund fees") + } else { + cbs.OnChanCloseConfirm(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) + afterBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress()) + suite.Require().Equal(origBal, afterBal, "balances of refund account not equal after all fees refunded") + } + }) + } +} diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index 5ffa096c2a9..785a6a88f7d 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -109,6 +109,39 @@ func (k Keeper) DistributeFeeTimeout(ctx sdk.Context, refundAcc, timeoutRelayer return nil } -func (k Keeper) RefundAllFees(ctx sdk.Context) { - // TODO +func (k Keeper) RefundFeesOnChannel(ctx sdk.Context, portID, channelID string) error { + // get module accAddr + feeModuleAccAddr := k.authKeeper.GetModuleAddress(types.ModuleName) + + var refundErr error + + k.IterateChannelFeesInEscrow(ctx, portID, channelID, func(identifiedFee types.IdentifiedPacketFee) (stop bool) { + refundAccAddr, err := sdk.AccAddressFromBech32(identifiedFee.RefundAddress) + if err != nil { + refundErr = err + return true + } + + // refund all fees to refund address + // Use SendCoins rather than the module account send functions since refund address may be a user account or module address. + // if any `SendCoins` call returns an error, we return error and stop iteration + err = k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, refundAccAddr, identifiedFee.Fee.ReceiveFee) + if err != nil { + refundErr = err + return true + } + err = k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, refundAccAddr, identifiedFee.Fee.AckFee) + if err != nil { + refundErr = err + return true + } + err = k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, refundAccAddr, identifiedFee.Fee.TimeoutFee) + if err != nil { + refundErr = err + return true + } + return false + }) + + return refundErr } diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 91cd66a2a14..d846035191c 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -282,3 +282,53 @@ func (suite *KeeperTestSuite) TestDistributeTimeoutFee() { }) } } + +func (suite *KeeperTestSuite) TestRefundFeesOnChannel() { + // setup + refundAcc := suite.chainA.SenderAccount.GetAddress() + + // refundAcc balance before escrow + prevBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), refundAcc) + + ackFee := validCoins + receiveFee := validCoins2 + timeoutFee := validCoins3 + + for i := 0; i < 5; i++ { + packetId := &channeltypes.PacketId{ChannelId: "channel-0", PortId: types.PortKey, Sequence: uint64(i)} + fee := types.Fee{receiveFee, ackFee, timeoutFee} + + identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} + err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) + suite.Require().NoError(err) + } + + // send a packet over a different channel to ensure this fee is not refunded + packetId := &channeltypes.PacketId{ChannelId: "channel-1", PortId: types.PortKey, Sequence: 1} + fee := types.Fee{receiveFee, ackFee, timeoutFee} + + identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} + err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) + suite.Require().NoError(err) + + // check that refunding all fees on channel-0 refunds all fees except for fee on channel-1 + err = suite.chainA.GetSimApp().IBCFeeKeeper.RefundFeesOnChannel(suite.chainA.GetContext(), types.PortKey, "channel-0") + suite.Require().NoError(err, "refund fees returned unexpected error") + + // add fee sent to channel-1 to after balance to recover original balance + afterBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), refundAcc) + suite.Require().Equal(prevBal, afterBal.Add(validCoins...).Add(validCoins2...).Add(validCoins3...), "refund account not back to original balance after refunding all tokens") + + // create escrow and then change module account balance to cause error on refund + packetId = &channeltypes.PacketId{ChannelId: "channel-0", PortId: types.PortKey, Sequence: uint64(6)} + fee = types.Fee{receiveFee, ackFee, timeoutFee} + + identifiedPacketFee = types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}} + err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), &identifiedPacketFee) + suite.Require().NoError(err) + + suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, refundAcc, validCoins3) + + err = suite.chainA.GetSimApp().IBCFeeKeeper.RefundFeesOnChannel(suite.chainA.GetContext(), types.PortKey, "channel-0") + suite.Require().Error(err, "refund fees returned no error with insufficient balance on module account") +} diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index d18ca8a79e7..8bbf1a23312 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -144,11 +144,11 @@ func (k Keeper) GetFeeInEscrow(ctx sdk.Context, packetId *channeltypes.PacketId) return fee, true } -// IterateFeeInEscrow iterates over all the fees currently escrowed and calls the provided callback +// IterateChannelFeesInEscrow iterates over all the fees on the given channel currently escrowed and calls the provided callback // if the callback returns true, then iteration is stopped. -func (k Keeper) IterateFeeInEscrow(ctx sdk.Context, cb func(identifiedFee types.IdentifiedPacketFee) (stop bool)) { +func (k Keeper) IterateChannelFeesInEscrow(ctx sdk.Context, portID, channelID string, cb func(identifiedFee types.IdentifiedPacketFee) (stop bool)) { store := ctx.KVStore(k.storeKey) - iterator := sdk.KVStorePrefixIterator(store, []byte(types.FeeInEscrowPrefix)) + iterator := sdk.KVStorePrefixIterator(store, types.KeyFeeInEscrowChannelPrefix(portID, channelID)) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { @@ -166,7 +166,7 @@ func (k Keeper) DeleteFeeInEscrow(ctx sdk.Context, packetId *channeltypes.Packet store.Delete(key) } -// GetFeeInEscrow returns true if there is a Fee still to be escrowed for a given packet +// HasFeeInEscrow returns true if there is a Fee still to be escrowed for a given packet func (k Keeper) HasFeeInEscrow(ctx sdk.Context, packetId *channeltypes.PacketId) bool { store := ctx.KVStore(k.storeKey) key := types.KeyFeeInEscrow(packetId) diff --git a/modules/apps/29-fee/types/expected_keepers.go b/modules/apps/29-fee/types/expected_keepers.go index 33edb1c01ce..d5aad9621c0 100644 --- a/modules/apps/29-fee/types/expected_keepers.go +++ b/modules/apps/29-fee/types/expected_keepers.go @@ -31,5 +31,6 @@ type PortKeeper interface { type BankKeeper interface { HasBalance(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coin) bool SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error + SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error } diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index 3df1f637a6c..3225cd349a4 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -47,5 +47,10 @@ func KeyRelayerAddress(address string) []byte { // KeyFeeInEscrow returns the key for escrowed fees func KeyFeeInEscrow(packetID *channeltypes.PacketId) []byte { - return []byte(fmt.Sprintf("%s/%s/%s/packet/%d", FeeInEscrowPrefix, packetID.PortId, packetID.ChannelId, packetID.Sequence)) + return []byte(fmt.Sprintf("%s/%d", KeyFeeInEscrowChannelPrefix(packetID.PortId, packetID.ChannelId), packetID.Sequence)) +} + +// KeyFeeInEscrowChannelPrefix returns the key prefix for escrowed fees on the given channel +func KeyFeeInEscrowChannelPrefix(portID, channelID string) []byte { + return []byte(fmt.Sprintf("%s/%s/%s/packet", FeeInEscrowPrefix, portID, channelID)) } From cc0e5ac2e531576d02e22dcfb5f19535cb97bef1 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 19 Nov 2021 15:48:48 +0100 Subject: [PATCH 3/5] add comments for panic --- modules/apps/29-fee/ibc_module.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index 2dd3893249c..2b8a7ec36bb 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -134,6 +134,9 @@ func (im IBCModule) OnChanCloseInit( // and refund any remaining fees escrowed on channel im.keeper.DeleteFeeEnabled(ctx, portID, channelID) err := im.keeper.RefundFeesOnChannel(ctx, portID, channelID) + // error should only be non-nil if there is a bug in the code + // that causes module account to have insufficient funds to refund + // all escrowed fees on the channel. if err != nil { panic(err) } @@ -150,6 +153,9 @@ func (im IBCModule) OnChanCloseConfirm( // and refund any remaining fees escrowed on channel im.keeper.DeleteFeeEnabled(ctx, portID, channelID) err := im.keeper.RefundFeesOnChannel(ctx, portID, channelID) + // error should only be non-nil if there is a bug in the code + // that causes module account to have insufficient funds to refund + // all escrowed fees on the channel. if err != nil { panic(err) } From 4ab4b02ec6bbfdc02c0b4c84ffa98f2ce5fb974d Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 24 Nov 2021 15:39:39 +0100 Subject: [PATCH 4/5] change invariant breaking recovery to disabling middleware rather than panicing --- modules/apps/29-fee/ibc_module.go | 8 +++- modules/apps/29-fee/ibc_module_test.go | 48 ++++++++++++++------- modules/apps/29-fee/keeper/escrow.go | 4 ++ modules/apps/29-fee/keeper/escrow_test.go | 7 ---- modules/apps/29-fee/keeper/keeper.go | 15 +++++++ modules/apps/29-fee/keeper/keeper_test.go | 51 +++++++++++++++++++++++ modules/apps/29-fee/types/errors.go | 1 + 7 files changed, 111 insertions(+), 23 deletions(-) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index 2b8a7ec36bb..1e841e8f3e0 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -137,8 +137,10 @@ func (im IBCModule) OnChanCloseInit( // error should only be non-nil if there is a bug in the code // that causes module account to have insufficient funds to refund // all escrowed fees on the channel. + // Disable all channels to allow for coordinated fix to the issue + // and mitigate damage. if err != nil { - panic(err) + im.keeper.DisableAllChannels(ctx) } return im.app.OnChanCloseInit(ctx, portID, channelID) } @@ -156,8 +158,10 @@ func (im IBCModule) OnChanCloseConfirm( // error should only be non-nil if there is a bug in the code // that causes module account to have insufficient funds to refund // all escrowed fees on the channel. + // Disable all channels to allow for coordinated fix to the issue + // and mitigate damage. if err != nil { - panic(err) + im.keeper.DisableAllChannels(ctx) } return im.app.OnChanCloseConfirm(ctx, portID, channelID) } diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index 77eb7801c6d..9ea65e76a07 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -312,9 +312,9 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() { // Tests OnChanCloseInit on chainA func (suite *FeeTestSuite) TestOnChanCloseInit() { testCases := []struct { - name string - setup func(suite *FeeTestSuite) - panics bool + name string + setup func(suite *FeeTestSuite) + disabled bool }{ { "success", @@ -345,6 +345,9 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() { suite.Require().NoError(err) suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, refundAcc, validCoins3) + + // set fee enabled on different channel + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), "portID7", "channel-7") }, true, }, @@ -366,10 +369,17 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() { cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) suite.Require().True(ok) - if tc.panics { - suite.Require().Panics(func() { - cbs.OnChanCloseInit(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) - }, "did not panic as expected on refund fees") + if tc.disabled { + suite.Require().True( + suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID), + + "fee is not disabled on original channel: %s", suite.path.EndpointA.ChannelID, + ) + suite.Require().True( + suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), "portID7", "channel-7"), + + "fee is not disabled on other channel: %s", "channel-7", + ) } else { cbs.OnChanCloseInit(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) afterBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress()) @@ -382,9 +392,9 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() { // Tests OnChanCloseConfirm on chainA func (suite *FeeTestSuite) TestOnChanCloseConfirm() { testCases := []struct { - name string - setup func(suite *FeeTestSuite) - panics bool + name string + setup func(suite *FeeTestSuite) + disabled bool }{ { "success", @@ -415,6 +425,9 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() { suite.Require().NoError(err) suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, refundAcc, validCoins3) + + // set fee enabled on different channel + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), "portID7", "channel-7") }, true, }, @@ -436,10 +449,17 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() { cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) suite.Require().True(ok) - if tc.panics { - suite.Require().Panics(func() { - cbs.OnChanCloseConfirm(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) - }, "did not panic as expected on refund fees") + if tc.disabled { + suite.Require().True( + suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID), + + "fee is not disabled on original channel: %s", suite.path.EndpointA.ChannelID, + ) + suite.Require().True( + suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), "portID7", "channel-7"), + + "fee is not disabled on other channel: %s", "channel-7", + ) } else { cbs.OnChanCloseConfirm(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID) afterBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress()) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index 785a6a88f7d..a58197da31c 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -13,6 +13,10 @@ import ( // EscrowPacketFee sends the packet fee to the 29-fee module account to hold in escrow func (k Keeper) EscrowPacketFee(ctx sdk.Context, identifiedFee *types.IdentifiedPacketFee) error { + if !k.IsFeeEnabled(ctx, identifiedFee.PacketId.PortId, identifiedFee.PacketId.ChannelId) { + // users may not escrow fees on this channel. Must send packets without a fee message + return sdkerrors.Wrap(types.ErrFeeNotEnabled, "cannot escrow fee for packet") + } // check if the refund account exists refundAcc, err := sdk.AccAddressFromBech32(identifiedFee.RefundAddress) if err != nil { diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index d846035191c..5ceabef59dc 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -9,13 +9,6 @@ import ( channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ) -var ( - validCoins = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}} - validCoins2 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(200)}} - validCoins3 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(300)}} - invalidCoins = sdk.Coins{sdk.Coin{Denom: "invalidDenom", Amount: sdk.NewInt(100)}} -) - func (suite *KeeperTestSuite) TestEscrowPacketFee() { var ( err error diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index 8bbf1a23312..e101072744c 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -174,6 +174,21 @@ func (k Keeper) HasFeeInEscrow(ctx sdk.Context, packetId *channeltypes.PacketId) return store.Has(key) } +// DisableAllChannels will disable the fee module for all channels. +// Only called if the module enters into an invalid state +// e.g. ModuleAccount has insufficient balance to refund users. +// In this case, chain developers should investigate the issue, fix it, +// and then re-enable the fee module in a coordinated upgrade. +func (k Keeper) DisableAllChannels(ctx sdk.Context) { + store := ctx.KVStore(k.storeKey) + iterator := sdk.KVStorePrefixIterator(store, []byte(types.FeeEnabledKeyPrefix)) + + defer iterator.Close() + for ; iterator.Valid(); iterator.Next() { + store.Delete(iterator.Key()) + } +} + // MustMarshalFee attempts to encode a Fee object and returns the // raw encoded bytes. It panics on error. func (k Keeper) MustMarshalFee(fee *types.IdentifiedPacketFee) []byte { diff --git a/modules/apps/29-fee/keeper/keeper_test.go b/modules/apps/29-fee/keeper/keeper_test.go index 476a14698b0..bfa3f9c34c3 100644 --- a/modules/apps/29-fee/keeper/keeper_test.go +++ b/modules/apps/29-fee/keeper/keeper_test.go @@ -1,17 +1,26 @@ package keeper_test import ( + "fmt" "testing" "github.com/stretchr/testify/suite" "github.com/cosmos/cosmos-sdk/baseapp" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/testing" ) +var ( + validCoins = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}} + validCoins2 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(200)}} + validCoins3 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(300)}} + invalidCoins = sdk.Coins{sdk.Coin{Denom: "invalidDenom", Amount: sdk.NewInt(100)}} +) + type KeeperTestSuite struct { suite.Suite @@ -66,3 +75,45 @@ func SetupFeePath(path *ibctesting.Path) error { func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } + +func (suite *KeeperTestSuite) TestFeeInEscrow() { + ackFee := validCoins + receiveFee := validCoins2 + timeoutFee := validCoins3 + fee := types.Fee{ReceiveFee: receiveFee, AckFee: ackFee, TimeoutFee: timeoutFee} + + // set some fees + for i := 1; i < 6; i++ { + packetId := types.NewPacketId(fmt.Sprintf("channel-1"), uint64(i)) + fee := types.NewIdentifiedPacketFee(packetId, fee, suite.chainA.SenderAccount.GetAddress().String(), []string{}) + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeInEscrow(suite.chainA.GetContext(), fee) + } + + // delete 1 fee + packetId := types.NewPacketId("channel-1", 3) + suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeInEscrow(suite.chainA.GetContext(), packetId) + + // iterate over remaining fees + arr := []int64{} + expectedArr := []int64{1, 2, 4, 5} + suite.chainA.GetSimApp().IBCFeeKeeper.IterateChannelFeesInEscrow(suite.chainA.GetContext(), types.PortKey, "channel-1", func(identifiedFee types.IdentifiedPacketFee) (stop bool) { + arr = append(arr, int64(identifiedFee.PacketId.Sequence)) + return false + }) + suite.Require().Equal(expectedArr, arr, "did not retrieve expected fees during iteration") +} + +func (suite *KeeperTestSuite) TestDisableAllChannels() { + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), "port1", "channel1") + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), "port2", "channel2") + suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainA.GetContext(), "port3", "channel3") + + suite.chainA.GetSimApp().IBCFeeKeeper.DisableAllChannels(suite.chainA.GetContext()) + + suite.Require().False(suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), "port1", "channel1"), + "fee is still enabled on channel-1 after DisableAllChannels call") + suite.Require().False(suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), "port2", "channel2"), + "fee is still enabled on channel-2 after DisableAllChannels call") + suite.Require().False(suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), "port3", "channel3"), + "fee is still enabled on channel-3 after DisableAllChannels call") +} diff --git a/modules/apps/29-fee/types/errors.go b/modules/apps/29-fee/types/errors.go index b5e8203ec9f..7891d6e18b8 100644 --- a/modules/apps/29-fee/types/errors.go +++ b/modules/apps/29-fee/types/errors.go @@ -12,4 +12,5 @@ var ( ErrFeeNotFound = sdkerrors.Register(ModuleName, 5, "there is no fee escrowed for the given packetID") ErrRelayersNotNil = sdkerrors.Register(ModuleName, 6, "relayers must be nil. This feature is not supported") ErrCounterpartyAddressEmpty = sdkerrors.Register(ModuleName, 7, "counterparty address must not be empty") + ErrFeeNotEnabled = sdkerrors.Register(ModuleName, 8, "fee module is not enabled for this channel. If this error occurs after channel setup, fee module may not be enabled") ) From 665337863f48fb9c1fef1c6ab646bcc14311ef3e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 24 Nov 2021 17:18:28 +0100 Subject: [PATCH 5/5] docs, tests, minor refactor --- modules/apps/29-fee/ibc_module_test.go | 4 ++-- modules/apps/29-fee/keeper/escrow_test.go | 20 ++++++++++++------- modules/apps/29-fee/keeper/genesis_test.go | 5 +++-- modules/apps/29-fee/keeper/grpc_query_test.go | 11 +++++----- modules/apps/29-fee/keeper/keeper_test.go | 4 ++-- modules/apps/29-fee/keeper/msg_server_test.go | 4 ++-- modules/core/04-channel/types/packet.go | 5 +++++ 7 files changed, 33 insertions(+), 20 deletions(-) diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index 9ea65e76a07..598b3af6b14 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -357,7 +357,7 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() { tc := tc suite.Run(tc.name, func() { suite.SetupTest() - suite.coordinator.Setup(suite.path) + suite.coordinator.Setup(suite.path) // setup channel origBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress()) @@ -437,7 +437,7 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() { tc := tc suite.Run(tc.name, func() { suite.SetupTest() - suite.coordinator.Setup(suite.path) + suite.coordinator.Setup(suite.path) // setup channel origBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress()) diff --git a/modules/apps/29-fee/keeper/escrow_test.go b/modules/apps/29-fee/keeper/escrow_test.go index 1fede1b50c8..8eed26d52a4 100644 --- a/modules/apps/29-fee/keeper/escrow_test.go +++ b/modules/apps/29-fee/keeper/escrow_test.go @@ -17,6 +17,7 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() { ackFee sdk.Coins receiveFee sdk.Coins timeoutFee sdk.Coins + packetId *channeltypes.PacketId ) testCases := []struct { @@ -27,6 +28,11 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() { { "success", func() {}, true, }, + { + "fee not enabled on this channel", func() { + packetId.ChannelId = "disabled_channel" + }, false, + }, { "refundAcc does not exist", func() { // this acc does not exist on chainA @@ -54,15 +60,15 @@ func (suite *KeeperTestSuite) TestEscrowPacketFee() { tc := tc suite.Run(tc.name, func() { - suite.SetupTest() // reset - suite.coordinator.Setup(suite.path) + suite.SetupTest() // reset + suite.coordinator.Setup(suite.path) // setup channel // setup refundAcc = suite.chainA.SenderAccount.GetAddress() ackFee = validCoins receiveFee = validCoins2 timeoutFee = validCoins3 - packetId := &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: uint64(1)} + packetId = &channeltypes.PacketId{ChannelId: suite.path.EndpointA.ChannelID, PortId: transfertypes.PortID, Sequence: uint64(1)} tc.malleate() fee := types.Fee{ackFee, receiveFee, timeoutFee} @@ -129,8 +135,8 @@ func (suite *KeeperTestSuite) TestDistributeFee() { tc := tc suite.Run(tc.name, func() { - suite.SetupTest() // reset - suite.coordinator.Setup(suite.path) + suite.SetupTest() // reset + suite.coordinator.Setup(suite.path) // setup channel // setup refundAcc = suite.chainA.SenderAccount.GetAddress() @@ -220,8 +226,8 @@ func (suite *KeeperTestSuite) TestDistributeTimeoutFee() { tc := tc suite.Run(tc.name, func() { - suite.SetupTest() // reset - suite.coordinator.Setup(suite.path) + suite.SetupTest() // reset + suite.coordinator.Setup(suite.path) // setup channel // setup refundAcc = suite.chainA.SenderAccount.GetAddress() diff --git a/modules/apps/29-fee/keeper/genesis_test.go b/modules/apps/29-fee/keeper/genesis_test.go index 05c16df55d8..3fe9a3db38d 100644 --- a/modules/apps/29-fee/keeper/genesis_test.go +++ b/modules/apps/29-fee/keeper/genesis_test.go @@ -3,6 +3,7 @@ package keeper_test import ( "github.com/cosmos/ibc-go/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/testing" ) @@ -11,7 +12,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() { // build PacketId & Fee refundAcc := suite.chainA.SenderAccount.GetAddress() - packetId := types.NewPacketId( + packetId := channeltypes.NewPacketId( ibctesting.FirstChannelID, transfertypes.PortID, uint64(1), @@ -73,7 +74,7 @@ func (suite *KeeperTestSuite) TestExportGenesis() { // setup & escrow the packet fee refundAcc := suite.chainA.SenderAccount.GetAddress() - packetId := types.NewPacketId( + packetId := channeltypes.NewPacketId( ibctesting.FirstChannelID, transfertypes.PortID, uint64(1), diff --git a/modules/apps/29-fee/keeper/grpc_query_test.go b/modules/apps/29-fee/keeper/grpc_query_test.go index 9cf1824d417..b8b13f36242 100644 --- a/modules/apps/29-fee/keeper/grpc_query_test.go +++ b/modules/apps/29-fee/keeper/grpc_query_test.go @@ -8,6 +8,7 @@ import ( "github.com/cosmos/ibc-go/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/testing" ) @@ -19,8 +20,8 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacketI() { // setup suite.coordinator.Setup(suite.path) // setup channel - validPacketId := types.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 1) - invalidPacketId := types.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 2) + validPacketId := channeltypes.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 1) + invalidPacketId := channeltypes.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 2) identifiedPacketFee := types.NewIdentifiedPacketFee( validPacketId, types.Fee{ @@ -114,9 +115,9 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPackets() { func() { refundAcc := suite.chainA.SenderAccount.GetAddress() - fee1 := types.NewIdentifiedPacketFee(types.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 1), fee, refundAcc.String(), []string(nil)) - fee2 := types.NewIdentifiedPacketFee(types.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 2), fee, refundAcc.String(), []string(nil)) - fee3 := types.NewIdentifiedPacketFee(types.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 3), fee, refundAcc.String(), []string(nil)) + fee1 := types.NewIdentifiedPacketFee(channeltypes.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 1), fee, refundAcc.String(), []string(nil)) + fee2 := types.NewIdentifiedPacketFee(channeltypes.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 2), fee, refundAcc.String(), []string(nil)) + fee3 := types.NewIdentifiedPacketFee(channeltypes.NewPacketId(ibctesting.FirstChannelID, transfertypes.PortID, 3), fee, refundAcc.String(), []string(nil)) expPackets = []*types.IdentifiedPacketFee{} expPackets = append(expPackets, fee1, fee2, fee3) diff --git a/modules/apps/29-fee/keeper/keeper_test.go b/modules/apps/29-fee/keeper/keeper_test.go index 8c73f0b836e..7675bb127e3 100644 --- a/modules/apps/29-fee/keeper/keeper_test.go +++ b/modules/apps/29-fee/keeper/keeper_test.go @@ -64,13 +64,13 @@ func (suite *KeeperTestSuite) TestFeeInEscrow() { // set some fees for i := 1; i < 6; i++ { - packetId := types.NewPacketId(fmt.Sprintf("channel-1"), transfertypes.PortID, uint64(i)) + packetId := channeltypes.NewPacketId(fmt.Sprintf("channel-1"), transfertypes.PortID, uint64(i)) fee := types.NewIdentifiedPacketFee(packetId, fee, suite.chainA.SenderAccount.GetAddress().String(), []string{}) suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeInEscrow(suite.chainA.GetContext(), fee) } // delete 1 fee - packetId := types.NewPacketId("channel-1", transfertypes.PortID, 3) + packetId := channeltypes.NewPacketId("channel-1", transfertypes.PortID, 3) suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeInEscrow(suite.chainA.GetContext(), packetId) // iterate over remaining fees diff --git a/modules/apps/29-fee/keeper/msg_server_test.go b/modules/apps/29-fee/keeper/msg_server_test.go index a2a89e6b17b..ec7f830bd4d 100644 --- a/modules/apps/29-fee/keeper/msg_server_test.go +++ b/modules/apps/29-fee/keeper/msg_server_test.go @@ -65,7 +65,7 @@ func (suite *KeeperTestSuite) TestPayPacketFee() { for _, tc := range testCases { suite.SetupTest() - suite.coordinator.Setup(suite.path) + suite.coordinator.Setup(suite.path) // setup channel refundAcc := suite.chainA.SenderAccount.GetAddress() channelID := suite.path.EndpointA.ChannelID @@ -98,7 +98,7 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() { for _, tc := range testCases { suite.SetupTest() - suite.coordinator.Setup(suite.path) + suite.coordinator.Setup(suite.path) // setup channel ctxA := suite.chainA.GetContext() diff --git a/modules/core/04-channel/types/packet.go b/modules/core/04-channel/types/packet.go index 5e9c56e62d8..34eb9a80c3a 100644 --- a/modules/core/04-channel/types/packet.go +++ b/modules/core/04-channel/types/packet.go @@ -11,6 +11,11 @@ import ( "github.com/cosmos/ibc-go/modules/core/exported" ) +// NewPacketId returns a new instance of PacketId +func NewPacketId(channelId, portId string, seq uint64) *PacketId { + return &PacketId{ChannelId: channelId, PortId: portId, Sequence: seq} +} + // CommitPacket returns the packet commitment bytes. The commitment consists of: // sha256_hash(timeout_timestamp + timeout_height.RevisionNumber + timeout_height.RevisionHeight + sha256_hash(data)) // from a given packet. This results in a fixed length preimage.