From f9ff5d65486d171da54cb82a5b93319b6a9f3911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 24 Feb 2022 12:35:54 +0100 Subject: [PATCH 1/2] chore: add ParseKeyFeesInEscrow helper function --- modules/apps/29-fee/keeper/keeper.go | 11 +++----- modules/apps/29-fee/types/keys.go | 22 +++++++++++++++ modules/apps/29-fee/types/keys_test.go | 39 ++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index 48ff7c935b0..5b2e7810447 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -335,18 +335,15 @@ func (k Keeper) GetAllIdentifiedPacketFees(ctx sdk.Context) []types.IdentifiedPa var identifiedFees []types.IdentifiedPacketFees for ; iterator.Valid(); iterator.Next() { - keySplit := strings.Split(string(iterator.Key()), "/") - - feesInEscrow := k.MustUnmarshalFees(iterator.Value()) - - channelID, portID := keySplit[2], keySplit[1] - seq, err := strconv.ParseUint(keySplit[3], 10, 64) + packetID, err := types.ParseKeyFeesInEscrow(string(iterator.Key())) if err != nil { panic(err) } + feesInEscrow := k.MustUnmarshalFees(iterator.Value()) + identifiedFee := types.IdentifiedPacketFees{ - PacketId: channeltypes.NewPacketId(channelID, portID, seq), + PacketId: packetID, PacketFees: feesInEscrow.PacketFees, } diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index d65d8efcbef..8f23a4e7688 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -2,6 +2,10 @@ package types import ( "fmt" + "strconv" + "strings" + + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" ) @@ -67,6 +71,24 @@ func KeyFeesInEscrow(packetID channeltypes.PacketId) []byte { return []byte(fmt.Sprintf("%s/%d", KeyFeesInEscrowChannelPrefix(packetID.PortId, packetID.ChannelId), packetID.Sequence)) } +// ParseKeyFeesInEscrow parses the key used to store fees in escrow and returns the packet id +func ParseKeyFeesInEscrow(key string) (channeltypes.PacketId, error) { + keySplit := strings.Split(key, "/") + if len(keySplit) != 4 { + return channeltypes.PacketId{}, sdkerrors.Wrapf( + sdkerrors.ErrLogic, "key provided is incorrect: the key split has incorrect length, expected %d, got %d", 4, len(keySplit), + ) + } + + seq, err := strconv.ParseUint(keySplit[3], 10, 64) + if err != nil { + return channeltypes.PacketId{}, err + } + + packetID := channeltypes.NewPacketId(keySplit[2], keySplit[1], seq) + return packetID, nil +} + // 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)) diff --git a/modules/apps/29-fee/types/keys_test.go b/modules/apps/29-fee/types/keys_test.go index a49532b753b..ac60255955b 100644 --- a/modules/apps/29-fee/types/keys_test.go +++ b/modules/apps/29-fee/types/keys_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/require" "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" + channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" + ibctesting "github.com/cosmos/ibc-go/v3/testing" ) func TestKeyCounterpartyRelayer(t *testing.T) { @@ -18,3 +20,40 @@ func TestKeyCounterpartyRelayer(t *testing.T) { key := types.KeyCounterpartyRelayer(relayerAddress, channelID) require.Equal(t, string(key), fmt.Sprintf("%s/%s/%s", types.CounterpartyRelayerAddressKeyPrefix, relayerAddress, channelID)) } + +func TestParseKeyFeesInEscrow(t *testing.T) { + validPacketID := channeltypes.NewPacketId(ibctesting.FirstChannelID, ibctesting.MockFeePort, 1) + + testCases := []struct { + name string + key string + expPass bool + }{ + { + "success", + string(types.KeyFeesInEscrow(validPacketID)), + true, + }, + { + "incorrect key - key split has incorrect length", + string(types.FeeEnabledKey(validPacketID.PortId, validPacketID.ChannelId)), + false, + }, + { + "incorrect key - sequence cannot be parsed", + fmt.Sprintf("%s/%s", types.KeyFeesInEscrowChannelPrefix(validPacketID.PortId, validPacketID.ChannelId), "sequence"), + false, + }, + } + + for _, tc := range testCases { + packetId, err := types.ParseKeyFeesInEscrow(tc.key) + + if tc.expPass { + require.NoError(t, err) + require.Equal(t, validPacketID, packetId) + } else { + require.Error(t, err) + } + } +} From dbd18184706eb4c5c3925c3f56c7c04114b4a8a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Feb 2022 13:07:36 +0100 Subject: [PATCH 2/2] feat: add ParseKeyFeeEnabled function and rename FeeEnabledKey to KeyFeeEnabled --- modules/apps/29-fee/keeper/keeper.go | 16 +++++---- modules/apps/29-fee/types/keys.go | 24 ++++++++++++-- modules/apps/29-fee/types/keys_test.go | 45 ++++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index 5b2e7810447..9b7583ea055 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -81,20 +81,20 @@ func (k Keeper) GetFeeModuleAddress() sdk.AccAddress { // identified by channel and port identifiers. func (k Keeper) SetFeeEnabled(ctx sdk.Context, portID, channelID string) { store := ctx.KVStore(k.storeKey) - store.Set(types.FeeEnabledKey(portID, channelID), []byte{1}) + store.Set(types.KeyFeeEnabled(portID, channelID), []byte{1}) } // DeleteFeeEnabled deletes the fee enabled flag for a given portID and channelID func (k Keeper) DeleteFeeEnabled(ctx sdk.Context, portID, channelID string) { store := ctx.KVStore(k.storeKey) - store.Delete(types.FeeEnabledKey(portID, channelID)) + store.Delete(types.KeyFeeEnabled(portID, channelID)) } // IsFeeEnabled returns whether fee handling logic should be run for the given port. It will check the // fee enabled flag for the given port and channel identifiers func (k Keeper) IsFeeEnabled(ctx sdk.Context, portID, channelID string) bool { store := ctx.KVStore(k.storeKey) - return store.Get(types.FeeEnabledKey(portID, channelID)) != nil + return store.Get(types.KeyFeeEnabled(portID, channelID)) != nil } // GetAllFeeEnabledChannels returns a list of all ics29 enabled channels containing portID & channelID that are stored in state @@ -105,11 +105,13 @@ func (k Keeper) GetAllFeeEnabledChannels(ctx sdk.Context) []types.FeeEnabledChan var enabledChArr []types.FeeEnabledChannel for ; iterator.Valid(); iterator.Next() { - keySplit := strings.Split(string(iterator.Key()), "/") - + portID, channelID, err := types.ParseKeyFeeEnabled(string(iterator.Key())) + if err != nil { + panic(err) + } ch := types.FeeEnabledChannel{ - PortId: keySplit[1], - ChannelId: keySplit[2], + PortId: portID, + ChannelId: channelID, } enabledChArr = append(enabledChArr, ch) diff --git a/modules/apps/29-fee/types/keys.go b/modules/apps/29-fee/types/keys.go index 8f23a4e7688..f39cd9b5d44 100644 --- a/modules/apps/29-fee/types/keys.go +++ b/modules/apps/29-fee/types/keys.go @@ -45,12 +45,32 @@ const ( AttributeKeyTimeoutFee = "timeout_fee" ) -// FeeEnabledKey returns the key that stores a flag to determine if fee logic should +// KeyFeeEnabled returns the key that stores a flag to determine if fee logic should // be enabled for the given port and channel identifiers. -func FeeEnabledKey(portID, channelID string) []byte { +func KeyFeeEnabled(portID, channelID string) []byte { return []byte(fmt.Sprintf("%s/%s/%s", FeeEnabledKeyPrefix, portID, channelID)) } +// ParseKeyFeeEnabled parses the key used to indicate if the fee logic should be +// enabled for the given port and channel identifiers. +func ParseKeyFeeEnabled(key string) (portID, channelID string, err error) { + keySplit := strings.Split(key, "/") + if len(keySplit) != 3 { + return "", "", sdkerrors.Wrapf( + sdkerrors.ErrLogic, "key provided is incorrect: the key split has incorrect length, expected %d, got %d", 3, len(keySplit), + ) + } + + if keySplit[0] != FeeEnabledKeyPrefix { + return "", "", sdkerrors.Wrapf(sdkerrors.ErrLogic, "key prefix is incorrect: expected %s, got %s", FeeEnabledKeyPrefix, keySplit[0]) + } + + portID = keySplit[1] + channelID = keySplit[2] + + return portID, channelID, nil +} + // KeyCounterpartyRelayer returns the key for relayer address -> counteryparty address mapping func KeyCounterpartyRelayer(address, channelID string) []byte { return []byte(fmt.Sprintf("%s/%s/%s", CounterpartyRelayerAddressKeyPrefix, address, channelID)) diff --git a/modules/apps/29-fee/types/keys_test.go b/modules/apps/29-fee/types/keys_test.go index ac60255955b..f196f5de56b 100644 --- a/modules/apps/29-fee/types/keys_test.go +++ b/modules/apps/29-fee/types/keys_test.go @@ -11,6 +11,10 @@ import ( ibctesting "github.com/cosmos/ibc-go/v3/testing" ) +var ( + validPacketID = channeltypes.NewPacketId(ibctesting.FirstChannelID, ibctesting.MockFeePort, 1) +) + func TestKeyCounterpartyRelayer(t *testing.T) { var ( relayerAddress = "relayer_address" @@ -21,8 +25,45 @@ func TestKeyCounterpartyRelayer(t *testing.T) { require.Equal(t, string(key), fmt.Sprintf("%s/%s/%s", types.CounterpartyRelayerAddressKeyPrefix, relayerAddress, channelID)) } +func TestParseKeyFeeEnabled(t *testing.T) { + testCases := []struct { + name string + key string + expPass bool + }{ + { + "success", + string(types.KeyFeeEnabled(ibctesting.MockPort, ibctesting.FirstChannelID)), + true, + }, + { + "incorrect key - key split has incorrect length", + string(types.KeyFeesInEscrow(validPacketID)), + false, + }, + { + "incorrect key - key split has incorrect length", + fmt.Sprintf("%s/%s/%s", "fee", ibctesting.MockPort, ibctesting.FirstChannelID), + false, + }, + } + + for _, tc := range testCases { + portID, channelID, err := types.ParseKeyFeeEnabled(tc.key) + + if tc.expPass { + require.NoError(t, err) + require.Equal(t, ibctesting.MockPort, portID) + require.Equal(t, ibctesting.FirstChannelID, channelID) + } else { + require.Error(t, err) + require.Empty(t, portID) + require.Empty(t, channelID) + } + } +} + func TestParseKeyFeesInEscrow(t *testing.T) { - validPacketID := channeltypes.NewPacketId(ibctesting.FirstChannelID, ibctesting.MockFeePort, 1) testCases := []struct { name string @@ -36,7 +77,7 @@ func TestParseKeyFeesInEscrow(t *testing.T) { }, { "incorrect key - key split has incorrect length", - string(types.FeeEnabledKey(validPacketID.PortId, validPacketID.ChannelId)), + string(types.KeyFeeEnabled(validPacketID.PortId, validPacketID.ChannelId)), false, }, {