Skip to content

Commit

Permalink
Merge branch 'feat/ics20-v2-path-forwarding' into serdar/6558-refacto…
Browse files Browse the repository at this point in the history
…r-revert-forward
  • Loading branch information
srdtrk committed Jun 13, 2024
2 parents cbad515 + ba7c4a8 commit d3a60fc
Show file tree
Hide file tree
Showing 10 changed files with 380 additions and 92 deletions.
10 changes: 5 additions & 5 deletions modules/apps/transfer/keeper/relay_forwarding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (suite *KeeperTestSuite) TestPathForwarding() {
sender := suite.chainA.SenderAccounts[0].SenderAccount
receiver := suite.chainA.SenderAccounts[1].SenderAccount
forwardingPath := types.ForwardingInfo{
Hops: []*types.Hop{
Hops: []types.Hop{
{
PortId: path2.EndpointA.ChannelConfig.PortID,
ChannelId: path2.EndpointA.ChannelID,
Expand Down Expand Up @@ -91,7 +91,7 @@ func (suite *KeeperTestSuite) TestEscrowsAreSetAfterForwarding() {
sender := suite.chainA.SenderAccounts[0].SenderAccount
receiver := suite.chainA.SenderAccounts[1].SenderAccount
forwardingPath := types.ForwardingInfo{
Hops: []*types.Hop{
Hops: []types.Hop{
{
PortId: path2.EndpointB.ChannelConfig.PortID,
ChannelId: path2.EndpointB.ChannelID,
Expand Down Expand Up @@ -174,7 +174,7 @@ func (suite *KeeperTestSuite) TestHappyPathForwarding() {
sender := suite.chainA.SenderAccounts[0].SenderAccount
receiver := suite.chainA.SenderAccounts[1].SenderAccount
forwardingPath := types.ForwardingInfo{
Hops: []*types.Hop{
Hops: []types.Hop{
{
PortId: path2.EndpointB.ChannelConfig.PortID,
ChannelId: path2.EndpointB.ChannelID,
Expand Down Expand Up @@ -282,7 +282,7 @@ func (suite *KeeperTestSuite) TestSimplifiedHappyPathForwarding() {
sender := suite.chainA.SenderAccounts[0].SenderAccount
receiver := suite.chainA.SenderAccounts[1].SenderAccount
forwardingPath := types.ForwardingInfo{
Hops: []*types.Hop{
Hops: []types.Hop{
{
PortId: path2.EndpointB.ChannelConfig.PortID,
ChannelId: path2.EndpointB.ChannelID,
Expand Down Expand Up @@ -476,7 +476,7 @@ func (suite *KeeperTestSuite) TestAcknowledgementFailureScenario5Forwarding() {
receiver = suite.chainA.SenderAccounts[0].SenderAccount // Receiver is the A chain account

forwardingPath := types.ForwardingInfo{
Hops: []*types.Hop{
Hops: []types.Hop{
{
PortId: path1.EndpointB.ChannelConfig.PortID,
ChannelId: path1.EndpointB.ChannelID,
Expand Down
1 change: 1 addition & 0 deletions modules/apps/transfer/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
39 changes: 39 additions & 0 deletions modules/apps/transfer/types/forwarding_info.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package types

import (
errorsmod "cosmossdk.io/errors"

host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
)

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,
Hops: hops,
}
}

// 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)
}

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
}
151 changes: 151 additions & 0 deletions modules/apps/transfer/types/forwarding_info_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
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"
)

var validHop = types.Hop{
PortId: types.PortID,
ChannelId: ibctesting.FirstChannelID,
}

func TestForwardingInfo_Validate(t *testing.T) {
tests := []struct {
name string
forwardingInfo *types.ForwardingInfo
expError error
}{
{
"valid forwarding info with no hops",
types.NewForwardingInfo(""),
nil,
},
{
"valid forwarding info with hops",
types.NewForwardingInfo("", validHop),
nil,
},
{
"valid forwarding info with memo",
types.NewForwardingInfo(testMemo1, validHop, validHop),
nil,
},
{
"valid forwarding info with max hops",
types.NewForwardingInfo("", generateHops(types.MaximumNumberOfForwardingHops)...),
nil,
},
{
"valid forwarding info with max memo length",
types.NewForwardingInfo(ibctesting.GenerateString(types.MaximumMemoLength), validHop),
nil,
},
{
"invalid forwarding info with too many hops",
types.NewForwardingInfo("", generateHops(types.MaximumNumberOfForwardingHops+1)...),
types.ErrInvalidForwardingInfo,
},
{
"invalid forwarding info with too long memo",
types.NewForwardingInfo(ibctesting.GenerateString(types.MaximumMemoLength+1), validHop),
types.ErrInvalidMemo,
},
{
"invalid forwarding info with too short hop port ID",
types.NewForwardingInfo(
"",
types.Hop{
PortId: invalidShortPort,
ChannelId: ibctesting.FirstChannelID,
},
),
host.ErrInvalidID,
},
{
"invalid forwarding info with too long hop port ID",
types.NewForwardingInfo(
"",
types.Hop{
PortId: invalidLongPort,
ChannelId: ibctesting.FirstChannelID,
},
),
host.ErrInvalidID,
},
{
"invalid forwarding info with non-alpha hop port ID",
types.NewForwardingInfo(
"",
types.Hop{
PortId: invalidPort,
ChannelId: ibctesting.FirstChannelID,
},
),
host.ErrInvalidID,
},
{
"invalid forwarding info with too long hop channel ID",
types.NewForwardingInfo(
"",
types.Hop{
PortId: types.PortID,
ChannelId: invalidLongChannel,
},
),
host.ErrInvalidID,
},
{
"invalid forwarding info with too short hop channel ID",
types.NewForwardingInfo(
"",
types.Hop{
PortId: types.PortID,
ChannelId: invalidShortChannel,
},
),
host.ErrInvalidID,
},
{
"invalid forwarding info with non-alpha hop channel ID",
types.NewForwardingInfo(
"",
types.Hop{
PortId: types.PortID,
ChannelId: invalidChannel,
},
),
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: types.PortID,
ChannelId: ibctesting.FirstChannelID,
}
}
return hops
}
11 changes: 11 additions & 0 deletions modules/apps/transfer/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,17 @@ func (msg MsgTransfer) ValidateBasic() error {
return errorsmod.Wrapf(ErrInvalidMemo, "memo must not exceed %d bytes", MaximumMemoLength)
}

if msg.ForwardingPath != nil {
if err := msg.ForwardingPath.Validate(); err != nil {
return err
}

// We cannot have non-empty memo and non-empty forwarding path hops at the same time.
if len(msg.ForwardingPath.Hops) > 0 && msg.Memo != "" {
return errorsmod.Wrapf(ErrInvalidMemo, "memo must be empty if forwarding path hops is not empty: %s, %s", msg.Memo, msg.ForwardingPath.Hops)
}
}

for _, coin := range msg.GetCoins() {
if err := validateIBCCoin(coin); err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidCoins, "%s: %s", err.Error(), coin.String())
Expand Down
77 changes: 41 additions & 36 deletions modules/apps/transfer/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,24 @@ func TestMsgTransferValidation(t *testing.T) {
{"multidenom: zero coins", types.NewMsgTransfer(validPort, validChannel, zeroCoins, sender, receiver, timeoutHeight, 0, "", nil), ibcerrors.ErrInvalidCoins},
{"multidenom: too many coins", types.NewMsgTransfer(validPort, validChannel, make([]sdk.Coin, types.MaximumTokensLength+1), sender, receiver, timeoutHeight, 0, "", nil), ibcerrors.ErrInvalidCoins},
{"multidenom: both token and tokens are set", &types.MsgTransfer{validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, "", coins, nil}, ibcerrors.ErrInvalidCoins},
{"memo must be empty if forwarding path hops is not empty", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "memo", types.NewForwardingInfo("", validHop)), types.ErrInvalidMemo},
{"invalid forwarding info port", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "", types.NewForwardingInfo("", types.Hop{PortId: invalidPort, ChannelId: validChannel})), host.ErrInvalidID},
{"invalid forwarding info channel", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "", types.NewForwardingInfo("", types.Hop{PortId: validPort, ChannelId: invalidChannel})), host.ErrInvalidID},
{"invalid forwarding info too many hops", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "", types.NewForwardingInfo("", generateHops(types.MaximumNumberOfForwardingHops+1)...)), types.ErrInvalidForwardingInfo},
{"invalid forwarding info too long memo", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "", types.NewForwardingInfo(ibctesting.GenerateString(types.MaximumMemoLength+1), validHop)), types.ErrInvalidMemo},
}

for _, tc := range testCases {
tc := tc

err := tc.msg.ValidateBasic()

expPass := tc.expError == nil
if expPass {
require.NoError(t, err)
} else {
require.ErrorIs(t, err, tc.expError)
}
t.Run(tc.name, func(t *testing.T) {
err := tc.msg.ValidateBasic()

expPass := tc.expError == nil
if expPass {
require.NoError(t, err)
} else {
require.ErrorIs(t, err, tc.expError)
}
})
}
}

Expand Down Expand Up @@ -119,16 +124,16 @@ func TestMsgUpdateParamsValidateBasic(t *testing.T) {
}

for _, tc := range testCases {
tc := tc

err := tc.msg.ValidateBasic()

expPass := tc.expError == nil
if expPass {
require.NoError(t, err)
} else {
require.ErrorIs(t, err, tc.expError)
}
t.Run(tc.name, func(t *testing.T) {
err := tc.msg.ValidateBasic()

expPass := tc.expError == nil
if expPass {
require.NoError(t, err)
} else {
require.ErrorIs(t, err, tc.expError)
}
})
}
}

Expand All @@ -144,21 +149,21 @@ func TestMsgUpdateParamsGetSigners(t *testing.T) {
}

for _, tc := range testCases {
tc := tc

msg := types.MsgUpdateParams{
Signer: tc.address.String(),
Params: types.DefaultParams(),
}

encodingCfg := moduletestutil.MakeTestEncodingConfig(transfer.AppModuleBasic{})
signers, _, err := encodingCfg.Codec.GetMsgV1Signers(&msg)

if tc.expPass {
require.NoError(t, err)
require.Equal(t, tc.address.Bytes(), signers[0])
} else {
require.Error(t, err)
}
t.Run(tc.name, func(t *testing.T) {
msg := types.MsgUpdateParams{
Signer: tc.address.String(),
Params: types.DefaultParams(),
}

encodingCfg := moduletestutil.MakeTestEncodingConfig(transfer.AppModuleBasic{})
signers, _, err := encodingCfg.Codec.GetMsgV1Signers(&msg)

if tc.expPass {
require.NoError(t, err)
require.Equal(t, tc.address.Bytes(), signers[0])
} else {
require.Error(t, err)
}
})
}
}
12 changes: 9 additions & 3 deletions modules/apps/transfer/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,15 @@ 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 && 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 {
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)
}
}

return nil
Expand Down
Loading

0 comments on commit d3a60fc

Please sign in to comment.