From 7ca30574774a2c80e7913c7219e213ee09ce5ee4 Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Tue, 11 Jun 2024 13:18:07 +0200 Subject: [PATCH 1/5] feat(transfer): add forwarding info validation to token packet --- modules/apps/transfer/types/errors.go | 1 + .../apps/transfer/types/forwarding_info.go | 30 +++ .../transfer/types/forwarding_info_test.go | 206 ++++++++++++++++++ modules/apps/transfer/types/packet.go | 12 +- modules/apps/transfer/types/packet_test.go | 123 +++++++++++ 5 files changed, 371 insertions(+), 1 deletion(-) create mode 100644 modules/apps/transfer/types/forwarding_info.go create mode 100644 modules/apps/transfer/types/forwarding_info_test.go diff --git a/modules/apps/transfer/types/errors.go b/modules/apps/transfer/types/errors.go index 2a7d90c0fbe..13f87f17a77 100644 --- a/modules/apps/transfer/types/errors.go +++ b/modules/apps/transfer/types/errors.go @@ -16,4 +16,5 @@ var ( ErrMaxTransferChannels = errorsmod.Register(ModuleName, 9, "max transfer channels") ErrInvalidAuthorization = errorsmod.Register(ModuleName, 10, "invalid transfer authorization") ErrInvalidMemo = errorsmod.Register(ModuleName, 11, "invalid memo") + ErrInvalidForwardingInfo = errorsmod.Register(ModuleName, 12, "invalid forwarding info") ) diff --git a/modules/apps/transfer/types/forwarding_info.go b/modules/apps/transfer/types/forwarding_info.go new file mode 100644 index 00000000000..ade4945cc65 --- /dev/null +++ b/modules/apps/transfer/types/forwarding_info.go @@ -0,0 +1,30 @@ +package types + +import ( + errorsmod "cosmossdk.io/errors" + + host "github.com/cosmos/ibc-go/v8/modules/core/24-host" +) + +const MaximumNumberOfForwardingHops = 64 + +func (fi ForwardingInfo) Validate() error { + if len(fi.Hops) > MaximumNumberOfForwardingHops { + return errorsmod.Wrapf(ErrInvalidForwardingInfo, "number of hops in forwarding path cannot exceed %d", MaximumNumberOfForwardingHops) + } + + for _, hop := range fi.Hops { + if err := host.PortIdentifierValidator(hop.PortId); err != nil { + return errorsmod.Wrapf(err, "invalid source port ID %s", hop.PortId) + } + if err := host.ChannelIdentifierValidator(hop.ChannelId); err != nil { + return errorsmod.Wrapf(err, "invalid source channel ID %s", hop.ChannelId) + } + } + + if len(fi.Memo) > MaximumMemoLength { + return errorsmod.Wrapf(ErrInvalidMemo, "memo length cannot exceed %d", MaximumMemoLength) + } + + return nil +} diff --git a/modules/apps/transfer/types/forwarding_info_test.go b/modules/apps/transfer/types/forwarding_info_test.go new file mode 100644 index 00000000000..c31f8fc8e87 --- /dev/null +++ b/modules/apps/transfer/types/forwarding_info_test.go @@ -0,0 +1,206 @@ +package types_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + host "github.com/cosmos/ibc-go/v8/modules/core/24-host" + ibctesting "github.com/cosmos/ibc-go/v8/testing" +) + +func TestForwardingInfo_Validate(t *testing.T) { + tests := []struct { + name string + forwardingInfo types.ForwardingInfo + expError error + }{ + { + "valid msg no hops", + types.ForwardingInfo{ + Hops: []*types.Hop{}, + Memo: "", + }, + nil, + }, + { + "valid msg nil hops", + types.ForwardingInfo{ + Hops: nil, + Memo: "", + }, + nil, + }, + { + "valid msg with hops", + types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: "transfer", + ChannelId: "channel-0", + }, + }, + Memo: "", + }, + nil, + }, + { + "valid msg with memo", + types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: "transfer", + ChannelId: "channel-0", + }, + { + PortId: "transfer", + ChannelId: "channel-1", + }, + }, + Memo: testMemo1, + }, + nil, + }, + { + "valid msg with max hops", + types.ForwardingInfo{ + Hops: generateHops(types.MaximumNumberOfForwardingHops), + Memo: "", + }, + nil, + }, + { + "valid msg with max memo length", + types.ForwardingInfo{ + Hops: []*types.Hop{}, + Memo: ibctesting.GenerateString(types.MaximumMemoLength), + }, + nil, + }, + { + "invalid msg with too many hops", + types.ForwardingInfo{ + Hops: generateHops(types.MaximumNumberOfForwardingHops + 1), + Memo: "", + }, + types.ErrInvalidForwardingInfo, + }, + { + "invalid msg with too long memo", + types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: "transfer", + ChannelId: "channel-0", + }, + }, + Memo: ibctesting.GenerateString(types.MaximumMemoLength + 1), + }, + types.ErrInvalidMemo, + }, + { + "invalid msg with too short hop port ID", + types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: invalidShortPort, + ChannelId: "channel-0", + }, + }, + Memo: "", + }, + host.ErrInvalidID, + }, + { + "invalid msg with too long hop port ID", + types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: invalidLongPort, + ChannelId: "channel-0", + }, + }, + Memo: "", + }, + host.ErrInvalidID, + }, + { + "invalid msg with non-alpha hop port ID", + types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: invalidPort, + ChannelId: "channel-0", + }, + }, + Memo: "", + }, + host.ErrInvalidID, + }, + { + "invalid msg with too long hop channel ID", + types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: "transfer", + ChannelId: invalidLongChannel, + }, + }, + Memo: "", + }, + host.ErrInvalidID, + }, + { + "invalid msg with too short hop channel ID", + types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: "transfer", + ChannelId: invalidShortChannel, + }, + }, + Memo: "", + }, + host.ErrInvalidID, + }, + { + "invalid msg with non-alpha hop channel ID", + types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: "transfer", + ChannelId: invalidChannel, + }, + }, + Memo: "", + }, + host.ErrInvalidID, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tc := tc + + err := tc.forwardingInfo.Validate() + + expPass := tc.expError == nil + if expPass { + require.NoError(t, err) + } else { + require.ErrorIs(t, err, tc.expError) + } + }) + } +} + +func generateHops(n int) []*types.Hop { + hops := make([]*types.Hop, n) + for i := 0; i < n; i++ { + hops[i] = &types.Hop{ + PortId: "transfer", + ChannelId: "channel-0", + } + } + return hops +} diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go index fcdb431695e..bb187627fdf 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -140,7 +140,17 @@ func (ftpd FungibleTokenPacketDataV2) ValidateBasic() error { return errorsmod.Wrapf(ErrInvalidMemo, "memo must not exceed %d bytes", MaximumMemoLength) } - // We cannot have non-empty memo and non-empty forwarding path hops at the same time. + if ftpd.ForwardingPath != nil { + if err := ftpd.ForwardingPath.Validate(); err != nil { + return err + } + + // We cannot have non-empty memo and non-empty forwarding path hops at the same time. + if len(ftpd.ForwardingPath.Hops) > 0 && ftpd.Memo != "" { + return errorsmod.Wrapf(ErrInvalidMemo, "memo must be empty if forwarding path hops is not empty: %s, %s", ftpd.Memo, ftpd.ForwardingPath.Hops) + } + } + if ftpd.ForwardingPath != nil && len(ftpd.ForwardingPath.Hops) > 0 && ftpd.Memo != "" { return errorsmod.Wrapf(ErrInvalidMemo, "memo must be empty if forwarding path hops is not empty: %s, %s", ftpd.Memo, ftpd.ForwardingPath.Hops) } diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index 9997751b69e..7726ca67f2a 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v8/testing" ) @@ -239,12 +240,43 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { PortId: "transfer", ChannelId: "channel-1", }, + { + PortId: "transfer", + ChannelId: "channel-0", + }, }, Memo: "", }, ), nil, }, + { + "success: valid packet with forwarding path hops with memo", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: types.Denom{ + Base: denom, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, + }, + Amount: amount, + }, + }, + sender, + receiver, + "", + &types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: "transfer", + ChannelId: "channel-1", + }, + }, + Memo: "memo", + }, + ), + nil, + }, { "failure: invalid denom", types.NewFungibleTokenPacketDataV2( @@ -411,6 +443,97 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { ), types.ErrInvalidMemo, }, + { + "failure: invalid forwarding path port", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: types.NewDenom(denom, types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")), + Amount: amount, + }, + }, + sender, + receiver, + "", + &types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: invalidPort, + ChannelId: "channel-1", + }, + }, + Memo: "", + }, + ), + host.ErrInvalidID, + }, + { + "failure: invalid forwarding path channel", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: types.NewDenom(denom, types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")), + Amount: amount, + }, + }, + sender, + receiver, + "", + &types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: "transfer", + ChannelId: invalidChannel, + }, + }, + Memo: "", + }, + ), + host.ErrInvalidID, + }, + { + "failure: invalid forwarding path too many hops", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: types.NewDenom(denom, types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")), + Amount: amount, + }, + }, + sender, + receiver, + "", + &types.ForwardingInfo{ + Hops: generateHops(types.MaximumNumberOfForwardingHops + 1), + Memo: "", + }, + ), + types.ErrInvalidForwardingInfo, + }, + { + "failure: invalid forwarding path too long memo", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: types.NewDenom(denom, types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")), + Amount: amount, + }, + }, + sender, + receiver, + "", + &types.ForwardingInfo{ + Hops: []*types.Hop{ + { + PortId: "transfer", + ChannelId: "channel-1", + }, + }, + Memo: ibctesting.GenerateString(types.MaximumMemoLength + 1), + }, + ), + types.ErrInvalidMemo, + }, } for _, tc := range testCases { From 721cec4ce729c08bbeb76a47a3752d30e3619abc Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Tue, 11 Jun 2024 15:27:51 +0200 Subject: [PATCH 2/5] Added NewForwardingInfo constructor --- .../apps/transfer/types/forwarding_info.go | 7 + .../transfer/types/forwarding_info_test.go | 153 ++++++------------ modules/apps/transfer/types/packet_test.go | 77 ++------- 3 files changed, 73 insertions(+), 164 deletions(-) diff --git a/modules/apps/transfer/types/forwarding_info.go b/modules/apps/transfer/types/forwarding_info.go index ade4945cc65..6caa5ad9893 100644 --- a/modules/apps/transfer/types/forwarding_info.go +++ b/modules/apps/transfer/types/forwarding_info.go @@ -8,6 +8,13 @@ import ( const MaximumNumberOfForwardingHops = 64 +func NewForwardingInfo(memo string, hops ...*Hop) *ForwardingInfo { + return &ForwardingInfo{ + Memo: memo, + Hops: hops, + } +} + func (fi ForwardingInfo) Validate() error { if len(fi.Hops) > MaximumNumberOfForwardingHops { return errorsmod.Wrapf(ErrInvalidForwardingInfo, "number of hops in forwarding path cannot exceed %d", MaximumNumberOfForwardingHops) diff --git a/modules/apps/transfer/types/forwarding_info_test.go b/modules/apps/transfer/types/forwarding_info_test.go index c31f8fc8e87..28615aec425 100644 --- a/modules/apps/transfer/types/forwarding_info_test.go +++ b/modules/apps/transfer/types/forwarding_info_test.go @@ -10,171 +10,116 @@ import ( ibctesting "github.com/cosmos/ibc-go/v8/testing" ) +var validHop = &types.Hop{ + PortId: "transfer", + ChannelId: "channel-0", +} + func TestForwardingInfo_Validate(t *testing.T) { tests := []struct { name string - forwardingInfo types.ForwardingInfo + forwardingInfo *types.ForwardingInfo expError error }{ { "valid msg no hops", - types.ForwardingInfo{ - Hops: []*types.Hop{}, - Memo: "", - }, - nil, - }, - { - "valid msg nil hops", - types.ForwardingInfo{ - Hops: nil, - Memo: "", - }, + types.NewForwardingInfo(""), nil, }, { "valid msg with hops", - types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: "channel-0", - }, - }, - Memo: "", - }, + types.NewForwardingInfo("", validHop), nil, }, { "valid msg with memo", - types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: "channel-0", - }, - { - PortId: "transfer", - ChannelId: "channel-1", - }, - }, - Memo: testMemo1, - }, + types.NewForwardingInfo(testMemo1, validHop, validHop), nil, }, { "valid msg with max hops", - types.ForwardingInfo{ - Hops: generateHops(types.MaximumNumberOfForwardingHops), - Memo: "", - }, + types.NewForwardingInfo("", generateHops(types.MaximumNumberOfForwardingHops)...), nil, }, { "valid msg with max memo length", - types.ForwardingInfo{ - Hops: []*types.Hop{}, - Memo: ibctesting.GenerateString(types.MaximumMemoLength), - }, + types.NewForwardingInfo(ibctesting.GenerateString(types.MaximumMemoLength), validHop), nil, }, { "invalid msg with too many hops", - types.ForwardingInfo{ - Hops: generateHops(types.MaximumNumberOfForwardingHops + 1), - Memo: "", - }, + types.NewForwardingInfo("", generateHops(types.MaximumNumberOfForwardingHops+1)...), types.ErrInvalidForwardingInfo, }, { "invalid msg with too long memo", - types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: "channel-0", - }, - }, - Memo: ibctesting.GenerateString(types.MaximumMemoLength + 1), - }, + types.NewForwardingInfo(ibctesting.GenerateString(types.MaximumMemoLength+1), validHop), types.ErrInvalidMemo, }, { "invalid msg with too short hop port ID", - types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: invalidShortPort, - ChannelId: "channel-0", - }, + types.NewForwardingInfo( + "", + &types.Hop{ + PortId: invalidShortPort, + ChannelId: "channel-0", }, - Memo: "", - }, + ), host.ErrInvalidID, }, { "invalid msg with too long hop port ID", - types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: invalidLongPort, - ChannelId: "channel-0", - }, + types.NewForwardingInfo( + "", + &types.Hop{ + PortId: invalidLongPort, + ChannelId: "channel-0", }, - Memo: "", - }, + ), host.ErrInvalidID, }, { "invalid msg with non-alpha hop port ID", - types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: invalidPort, - ChannelId: "channel-0", - }, + types.NewForwardingInfo( + "", + &types.Hop{ + PortId: invalidPort, + ChannelId: "channel-0", }, - Memo: "", - }, + ), host.ErrInvalidID, }, { "invalid msg with too long hop channel ID", - types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: invalidLongChannel, - }, + types.NewForwardingInfo( + "", + &types.Hop{ + PortId: "transfer", + ChannelId: invalidLongChannel, }, - Memo: "", - }, + ), host.ErrInvalidID, }, { "invalid msg with too short hop channel ID", - types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: invalidShortChannel, - }, + types.NewForwardingInfo( + "", + &types.Hop{ + PortId: "transfer", + ChannelId: invalidShortChannel, }, - Memo: "", - }, + ), host.ErrInvalidID, }, { "invalid msg with non-alpha hop channel ID", - types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: invalidChannel, - }, + types.NewForwardingInfo( + "", + &types.Hop{ + PortId: "transfer", + ChannelId: invalidChannel, }, - Memo: "", - }, + ), host.ErrInvalidID, }, } diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index 7726ca67f2a..46e2c618670 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -234,19 +234,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { sender, receiver, "", - &types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: "channel-1", - }, - { - PortId: "transfer", - ChannelId: "channel-0", - }, - }, - Memo: "", - }, + types.NewForwardingInfo("", validHop, validHop), ), nil, }, @@ -265,15 +253,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { sender, receiver, "", - &types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: "channel-1", - }, - }, - Memo: "memo", - }, + types.NewForwardingInfo("memo", validHop), ), nil, }, @@ -431,15 +411,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { sender, receiver, "memo", - &types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: "channel-1", - }, - }, - Memo: "", - }, + types.NewForwardingInfo("", validHop), ), types.ErrInvalidMemo, }, @@ -455,15 +427,13 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { sender, receiver, "", - &types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: invalidPort, - ChannelId: "channel-1", - }, + types.NewForwardingInfo( + "", + &types.Hop{ + PortId: invalidPort, + ChannelId: "channel-1", }, - Memo: "", - }, + ), ), host.ErrInvalidID, }, @@ -479,15 +449,13 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { sender, receiver, "", - &types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: invalidChannel, - }, + types.NewForwardingInfo( + "", + &types.Hop{ + PortId: "transfer", + ChannelId: invalidChannel, }, - Memo: "", - }, + ), ), host.ErrInvalidID, }, @@ -503,10 +471,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { sender, receiver, "", - &types.ForwardingInfo{ - Hops: generateHops(types.MaximumNumberOfForwardingHops + 1), - Memo: "", - }, + types.NewForwardingInfo("", generateHops(types.MaximumNumberOfForwardingHops+1)...), ), types.ErrInvalidForwardingInfo, }, @@ -522,15 +487,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { sender, receiver, "", - &types.ForwardingInfo{ - Hops: []*types.Hop{ - { - PortId: "transfer", - ChannelId: "channel-1", - }, - }, - Memo: ibctesting.GenerateString(types.MaximumMemoLength + 1), - }, + types.NewForwardingInfo(ibctesting.GenerateString(types.MaximumMemoLength+1), validHop), ), types.ErrInvalidMemo, }, From 4e268761f12767446aa996bc0ea45f4c3d6b1ea8 Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Tue, 11 Jun 2024 15:54:10 +0200 Subject: [PATCH 3/5] Removed redundant check --- modules/apps/transfer/types/packet.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go index bb187627fdf..e4f5179bd78 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -151,10 +151,6 @@ func (ftpd FungibleTokenPacketDataV2) ValidateBasic() error { } } - if ftpd.ForwardingPath != nil && len(ftpd.ForwardingPath.Hops) > 0 && ftpd.Memo != "" { - return errorsmod.Wrapf(ErrInvalidMemo, "memo must be empty if forwarding path hops is not empty: %s, %s", ftpd.Memo, ftpd.ForwardingPath.Hops) - } - return nil } From 67342352bfcbd3577933b93b5234e0d28fd4c0e5 Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Wed, 12 Jun 2024 10:33:59 +0200 Subject: [PATCH 4/5] Clean up tests per cr comments --- .../apps/transfer/types/forwarding_info.go | 4 +- .../transfer/types/forwarding_info_test.go | 48 +++++++++---------- modules/apps/transfer/types/packet.go | 2 +- modules/apps/transfer/types/packet_test.go | 4 +- 4 files changed, 30 insertions(+), 28 deletions(-) diff --git a/modules/apps/transfer/types/forwarding_info.go b/modules/apps/transfer/types/forwarding_info.go index 6caa5ad9893..c85aff54e93 100644 --- a/modules/apps/transfer/types/forwarding_info.go +++ b/modules/apps/transfer/types/forwarding_info.go @@ -8,6 +8,7 @@ import ( const MaximumNumberOfForwardingHops = 64 +// NewForwardingInfo creates a new ForwardingInfo instance given a memo and a variable number of hops. func NewForwardingInfo(memo string, hops ...*Hop) *ForwardingInfo { return &ForwardingInfo{ Memo: memo, @@ -15,7 +16,8 @@ func NewForwardingInfo(memo string, hops ...*Hop) *ForwardingInfo { } } -func (fi ForwardingInfo) Validate() error { +// ValidateBasic performs a basic validation of the ForwardingInfo fields. +func (fi ForwardingInfo) ValidateBasic() error { if len(fi.Hops) > MaximumNumberOfForwardingHops { return errorsmod.Wrapf(ErrInvalidForwardingInfo, "number of hops in forwarding path cannot exceed %d", MaximumNumberOfForwardingHops) } diff --git a/modules/apps/transfer/types/forwarding_info_test.go b/modules/apps/transfer/types/forwarding_info_test.go index 28615aec425..3f42561fcda 100644 --- a/modules/apps/transfer/types/forwarding_info_test.go +++ b/modules/apps/transfer/types/forwarding_info_test.go @@ -11,8 +11,8 @@ import ( ) var validHop = &types.Hop{ - PortId: "transfer", - ChannelId: "channel-0", + PortId: types.PortID, + ChannelId: ibctesting.FirstChannelID, } func TestForwardingInfo_Validate(t *testing.T) { @@ -22,101 +22,101 @@ func TestForwardingInfo_Validate(t *testing.T) { expError error }{ { - "valid msg no hops", + "valid forwarding info with no hops", types.NewForwardingInfo(""), nil, }, { - "valid msg with hops", + "valid forwarding info with hops", types.NewForwardingInfo("", validHop), nil, }, { - "valid msg with memo", + "valid forwarding info with memo", types.NewForwardingInfo(testMemo1, validHop, validHop), nil, }, { - "valid msg with max hops", + "valid forwarding info with max hops", types.NewForwardingInfo("", generateHops(types.MaximumNumberOfForwardingHops)...), nil, }, { - "valid msg with max memo length", + "valid forwarding info with max memo length", types.NewForwardingInfo(ibctesting.GenerateString(types.MaximumMemoLength), validHop), nil, }, { - "invalid msg with too many hops", + "invalid forwarding info with too many hops", types.NewForwardingInfo("", generateHops(types.MaximumNumberOfForwardingHops+1)...), types.ErrInvalidForwardingInfo, }, { - "invalid msg with too long memo", + "invalid forwarding info with too long memo", types.NewForwardingInfo(ibctesting.GenerateString(types.MaximumMemoLength+1), validHop), types.ErrInvalidMemo, }, { - "invalid msg with too short hop port ID", + "invalid forwarding info with too short hop port ID", types.NewForwardingInfo( "", &types.Hop{ PortId: invalidShortPort, - ChannelId: "channel-0", + ChannelId: ibctesting.FirstChannelID, }, ), host.ErrInvalidID, }, { - "invalid msg with too long hop port ID", + "invalid forwarding info with too long hop port ID", types.NewForwardingInfo( "", &types.Hop{ PortId: invalidLongPort, - ChannelId: "channel-0", + ChannelId: ibctesting.FirstChannelID, }, ), host.ErrInvalidID, }, { - "invalid msg with non-alpha hop port ID", + "invalid forwarding info with non-alpha hop port ID", types.NewForwardingInfo( "", &types.Hop{ PortId: invalidPort, - ChannelId: "channel-0", + ChannelId: ibctesting.FirstChannelID, }, ), host.ErrInvalidID, }, { - "invalid msg with too long hop channel ID", + "invalid forwarding info with too long hop channel ID", types.NewForwardingInfo( "", &types.Hop{ - PortId: "transfer", + PortId: types.PortID, ChannelId: invalidLongChannel, }, ), host.ErrInvalidID, }, { - "invalid msg with too short hop channel ID", + "invalid forwarding info with too short hop channel ID", types.NewForwardingInfo( "", &types.Hop{ - PortId: "transfer", + PortId: types.PortID, ChannelId: invalidShortChannel, }, ), host.ErrInvalidID, }, { - "invalid msg with non-alpha hop channel ID", + "invalid forwarding info with non-alpha hop channel ID", types.NewForwardingInfo( "", &types.Hop{ - PortId: "transfer", + PortId: types.PortID, ChannelId: invalidChannel, }, ), @@ -127,7 +127,7 @@ func TestForwardingInfo_Validate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tc := tc - err := tc.forwardingInfo.Validate() + err := tc.forwardingInfo.ValidateBasic() expPass := tc.expError == nil if expPass { @@ -143,8 +143,8 @@ func generateHops(n int) []*types.Hop { hops := make([]*types.Hop, n) for i := 0; i < n; i++ { hops[i] = &types.Hop{ - PortId: "transfer", - ChannelId: "channel-0", + PortId: types.PortID, + ChannelId: ibctesting.FirstChannelID, } } return hops diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go index e4f5179bd78..5fb2a29a26d 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -141,7 +141,7 @@ func (ftpd FungibleTokenPacketDataV2) ValidateBasic() error { } if ftpd.ForwardingPath != nil { - if err := ftpd.ForwardingPath.Validate(); err != nil { + if err := ftpd.ForwardingPath.ValidateBasic(); err != nil { return err } diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index 46e2c618670..67c4927838f 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -416,7 +416,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { types.ErrInvalidMemo, }, { - "failure: invalid forwarding path port", + "failure: invalid forwarding path port ID", types.NewFungibleTokenPacketDataV2( []types.Token{ { @@ -438,7 +438,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { host.ErrInvalidID, }, { - "failure: invalid forwarding path channel", + "failure: invalid forwarding path channel ID", types.NewFungibleTokenPacketDataV2( []types.Token{ { From 4d0951693388f335941ee068887d57ae1e2085bc Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Wed, 12 Jun 2024 11:30:30 +0200 Subject: [PATCH 5/5] Back to Validate and use NewDenom --- modules/apps/transfer/types/forwarding_info.go | 4 ++-- .../apps/transfer/types/forwarding_info_test.go | 2 +- modules/apps/transfer/types/packet.go | 2 +- modules/apps/transfer/types/packet_test.go | 15 +++------------ 4 files changed, 7 insertions(+), 16 deletions(-) diff --git a/modules/apps/transfer/types/forwarding_info.go b/modules/apps/transfer/types/forwarding_info.go index c85aff54e93..4235068ff87 100644 --- a/modules/apps/transfer/types/forwarding_info.go +++ b/modules/apps/transfer/types/forwarding_info.go @@ -16,8 +16,8 @@ func NewForwardingInfo(memo string, hops ...*Hop) *ForwardingInfo { } } -// ValidateBasic performs a basic validation of the ForwardingInfo fields. -func (fi ForwardingInfo) ValidateBasic() error { +// Validate performs a basic validation of the ForwardingInfo fields. +func (fi ForwardingInfo) Validate() error { if len(fi.Hops) > MaximumNumberOfForwardingHops { return errorsmod.Wrapf(ErrInvalidForwardingInfo, "number of hops in forwarding path cannot exceed %d", MaximumNumberOfForwardingHops) } diff --git a/modules/apps/transfer/types/forwarding_info_test.go b/modules/apps/transfer/types/forwarding_info_test.go index 3f42561fcda..f82b4df95a1 100644 --- a/modules/apps/transfer/types/forwarding_info_test.go +++ b/modules/apps/transfer/types/forwarding_info_test.go @@ -127,7 +127,7 @@ func TestForwardingInfo_Validate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tc := tc - err := tc.forwardingInfo.ValidateBasic() + err := tc.forwardingInfo.Validate() expPass := tc.expError == nil if expPass { diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go index 5fb2a29a26d..e4f5179bd78 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -141,7 +141,7 @@ func (ftpd FungibleTokenPacketDataV2) ValidateBasic() error { } if ftpd.ForwardingPath != nil { - if err := ftpd.ForwardingPath.ValidateBasic(); err != nil { + if err := ftpd.ForwardingPath.Validate(); err != nil { return err } diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index 67c4927838f..40e55a3a020 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -224,10 +224,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { types.NewFungibleTokenPacketDataV2( []types.Token{ { - Denom: types.Denom{ - Base: denom, - Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, - }, + Denom: types.NewDenom(denom, types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")), Amount: amount, }, }, @@ -243,10 +240,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { types.NewFungibleTokenPacketDataV2( []types.Token{ { - Denom: types.Denom{ - Base: denom, - Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, - }, + Denom: types.NewDenom(denom, types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")), Amount: amount, }, }, @@ -401,10 +395,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { types.NewFungibleTokenPacketDataV2( []types.Token{ { - Denom: types.Denom{ - Base: denom, - Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, - }, + Denom: types.NewDenom(denom, types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")), Amount: amount, }, },