diff --git a/modules/apps/transfer/internal/denom/denom.go b/modules/apps/transfer/internal/denom/denom.go index 972528cd0c0..e6b9d7b7680 100644 --- a/modules/apps/transfer/internal/denom/denom.go +++ b/modules/apps/transfer/internal/denom/denom.go @@ -1,9 +1,13 @@ package denom import ( + "fmt" "strings" + errorsmod "cosmossdk.io/errors" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" + host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ) // ExtractPathAndBaseFromFullDenom returns the trace path and the base denom from @@ -37,3 +41,21 @@ func ExtractPathAndBaseFromFullDenom(fullDenomItems []string) ([]string, string) return pathSlice, baseDenom } + +// ValidateTraceIdentifiers validates the correctness of the trace associated with a particular base denom. +func ValidateTraceIdentifiers(identifiers []string) error { + if len(identifiers) == 0 || len(identifiers)%2 != 0 { + return fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: %s", identifiers) + } + + // validate correctness of port and channel identifiers + for i := 0; i < len(identifiers); i += 2 { + if err := host.PortIdentifierValidator(identifiers[i]); err != nil { + return errorsmod.Wrapf(err, "invalid port ID at position %d", i) + } + if err := host.ChannelIdentifierValidator(identifiers[i+1]); err != nil { + return errorsmod.Wrapf(err, "invalid channel ID at position %d", i) + } + } + return nil +} diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index 00b1d3fd6f4..ab4e7bbce72 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -15,7 +15,6 @@ import ( cmttypes "github.com/cometbft/cometbft/types" denominternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" - host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ) // ParseDenomTrace parses a string with the ibc prefix (denom trace) and the base denomination @@ -82,23 +81,6 @@ func (dt DenomTrace) IsNativeDenom() bool { return dt.Path == "" } -func validateTraceIdentifiers(identifiers []string) error { - if len(identifiers) == 0 || len(identifiers)%2 != 0 { - return fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: %s", identifiers) - } - - // validate correctness of port and channel identifiers - for i := 0; i < len(identifiers); i += 2 { - if err := host.PortIdentifierValidator(identifiers[i]); err != nil { - return errorsmod.Wrapf(err, "invalid port ID at position %d", i) - } - if err := host.ChannelIdentifierValidator(identifiers[i+1]); err != nil { - return errorsmod.Wrapf(err, "invalid channel ID at position %d", i) - } - } - return nil -} - // Validate performs a basic validation of the DenomTrace fields. func (dt DenomTrace) Validate() error { // empty trace is accepted when token lives on the original chain @@ -112,7 +94,7 @@ func (dt DenomTrace) Validate() error { // NOTE: no base denomination validation identifiers := strings.Split(dt.Path, "/") - return validateTraceIdentifiers(identifiers) + return denominternal.ValidateTraceIdentifiers(identifiers) } // Traces defines a wrapper type for a slice of DenomTrace. @@ -178,7 +160,7 @@ func ValidatePrefixedDenom(denom string) error { } identifiers := strings.Split(path, "/") - return validateTraceIdentifiers(identifiers) + return denominternal.ValidateTraceIdentifiers(identifiers) } // ValidateIBCDenom validates that the given denomination is either: diff --git a/modules/apps/transfer/types/v3/packet.go b/modules/apps/transfer/types/v3/packet.go index 44494bfdefe..510a1024cac 100644 --- a/modules/apps/transfer/types/v3/packet.go +++ b/modules/apps/transfer/types/v3/packet.go @@ -1,6 +1,24 @@ package v3 -// NewFungibleTokenPacketData constructs a new FungibleTokenPacketData instance +import ( + "encoding/json" + "errors" + "strings" + + errorsmod "cosmossdk.io/errors" + sdkmath "cosmossdk.io/math" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" + ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported" +) + +var ( + _ ibcexported.PacketData = (*FungibleTokenPacketData)(nil) + _ ibcexported.PacketDataProvider = (*FungibleTokenPacketData)(nil) +) + +// NewFungibleTokenPacketData constructs a new NewFungibleTokenPacketData instance func NewFungibleTokenPacketData( tokens []*Token, sender, receiver string, @@ -13,3 +31,84 @@ func NewFungibleTokenPacketData( Memo: memo, } } + +// ValidateBasic is used for validating the token transfer. +// NOTE: The addresses formats are not validated as the sender and recipient can have different +// formats defined by their corresponding chains that are not known to IBC. +func (ftpd FungibleTokenPacketData) ValidateBasic() error { + if strings.TrimSpace(ftpd.Sender) == "" { + return errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "sender address cannot be blank") + } + + if strings.TrimSpace(ftpd.Receiver) == "" { + return errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "receiver address cannot be blank") + } + + if len(ftpd.Tokens) == 0 { + return errorsmod.Wrap(types.ErrInvalidAmount, "tokens cannot be empty") + } + + for _, token := range ftpd.Tokens { + amount, ok := sdkmath.NewIntFromString(token.Amount) + if !ok { + return errorsmod.Wrapf(types.ErrInvalidAmount, "unable to parse transfer amount (%s) into math.Int", token.Amount) + } + + if !amount.IsPositive() { + return errorsmod.Wrapf(types.ErrInvalidAmount, "amount must be strictly positive: got %d", amount) + } + + if err := token.Validate(); err != nil { + return err + } + } + + if len(ftpd.Memo) > types.MaximumMemoLength { + return errorsmod.Wrapf(types.ErrInvalidMemo, "memo must not exceed %d bytes", types.MaximumMemoLength) + } + + return nil +} + +// GetBytes is a helper for serialising +func (ftpd FungibleTokenPacketData) GetBytes() []byte { + bz, err := json.Marshal(&ftpd) + if err != nil { + panic(errors.New("cannot marshal v3 FungibleTokenPacketData into bytes")) + } + + return bz +} + +// GetCustomPacketData interprets the memo field of the packet data as a JSON object +// and returns the value associated with the given key. +// If the key is missing or the memo is not properly formatted, then nil is returned. +func (ftpd FungibleTokenPacketData) GetCustomPacketData(key string) interface{} { + if len(ftpd.Memo) == 0 { + return nil + } + + jsonObject := make(map[string]interface{}) + err := json.Unmarshal([]byte(ftpd.Memo), &jsonObject) + if err != nil { + return nil + } + + memoData, found := jsonObject[key] + if !found { + return nil + } + + return memoData +} + +// GetPacketSender returns the sender address embedded in the packet data. +// +// NOTE: +// - The sender address is set by the module which requested the packet to be sent, +// and this module may not have validated the sender address by a signature check. +// - The sender address must only be used by modules on the sending chain. +// - sourcePortID is not used in this implementation. +func (ftpd FungibleTokenPacketData) GetPacketSender(sourcePortID string) string { + return ftpd.Sender +} diff --git a/modules/apps/transfer/types/v3/packet_test.go b/modules/apps/transfer/types/v3/packet_test.go new file mode 100644 index 00000000000..63435d8589e --- /dev/null +++ b/modules/apps/transfer/types/v3/packet_test.go @@ -0,0 +1,436 @@ +package v3 + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/cometbft/cometbft/crypto/secp256k1" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" +) + +const ( + denom = "atom/pool" + amount = "1000" + largeAmount = "18446744073709551616" // one greater than largest uint64 (^uint64(0)) + invalidLargeAmount = "115792089237316195423570985008687907853269984665640564039457584007913129639936" // 2^256 +) + +var ( + sender = secp256k1.GenPrivKey().PubKey().Address().String() + receiver = sdk.AccAddress("testaddr2").String() +) + +// TestFungibleTokenPacketDataValidateBasic tests ValidateBasic for FungibleTokenPacketData +func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketData + expErr error + }{ + { + "success: valid packet", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + nil, + }, + { + "success: valid packet with memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "memo", + ), + nil, + }, + { + "success: valid packet with large amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: largeAmount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "memo", + ), + nil, + }, + { + "failure: invalid denom", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "", + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidDenomForTransfer, + }, + { + "failure: invalid empty amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: "", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "failure: invalid empty token array", + NewFungibleTokenPacketData( + []*Token{}, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "failure: invalid zero amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: "0", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "failure: invalid negative amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: "-100", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "failure: invalid large amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: invalidLargeAmount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "memo", + ), + types.ErrInvalidAmount, + }, + { + "failure: missing sender address", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + "", + receiver, + "memo", + ), + ibcerrors.ErrInvalidAddress, + }, + { + "failure: missing recipient address", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + "", + "", + ), + ibcerrors.ErrInvalidAddress, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.packetData.ValidateBasic() + + expPass := tc.expErr == nil + if expPass { + require.NoError(t, err, tc.name) + } else { + require.ErrorContains(t, err, tc.expErr.Error(), tc.name) + } + }) + } +} + +func TestGetPacketSender(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketData + expSender string + }{ + { + "non-empty sender field", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + sender, + }, + { + "empty sender field", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + "", + receiver, + "abc", + ), + "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expSender, tc.packetData.GetPacketSender(types.PortID)) + }) + } +} + +func TestPacketDataProvider(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketData + expCustomData interface{} + }{ + { + "success: src_callback key in memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, receiver)), + + map[string]interface{}{ + "address": receiver, + }, + }, + { + "success: src_callback key in memo with additional fields", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "200000"}}`, receiver)), + map[string]interface{}{ + "address": receiver, + "gas_limit": "200000", + }, + }, + { + "success: src_callback has string value", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + `{"src_callback": "string"}`), + "string", + }, + { + "failure: src_callback key not found memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + fmt.Sprintf(`{"dest_callback": {"address": "%s", "min_gas": "200000"}}`, receiver)), + nil, + }, + { + "failure: empty memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + ""), + nil, + }, + { + "failure: non-json memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "invalid"), + nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + customData := tc.packetData.GetCustomPacketData("src_callback") + require.Equal(t, tc.expCustomData, customData) + }) + } +} + +func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketData + expMemo bool + }{ + { + "empty memo field, resulting marshalled bytes should not contain the memo field", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + false, + }, + { + "non-empty memo field, resulting marshalled bytes should contain the memo field", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "abc", + ), + true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + bz, err := json.Marshal(tc.packetData) + if tc.expMemo { + require.NoError(t, err, tc.name) + // check that the memo field is present in the marshalled bytes + require.Contains(t, string(bz), "memo") + } else { + require.NoError(t, err, tc.name) + // check that the memo field is not present in the marshalled bytes + require.NotContains(t, string(bz), "memo") + } + }) + } +} diff --git a/modules/apps/transfer/types/v3/token.go b/modules/apps/transfer/types/v3/token.go new file mode 100644 index 00000000000..21a658723c8 --- /dev/null +++ b/modules/apps/transfer/types/v3/token.go @@ -0,0 +1,36 @@ +package v3 + +import ( + "strings" + + errorsmod "cosmossdk.io/errors" + + sdk "github.com/cosmos/cosmos-sdk/types" + + denominternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" +) + +// ValidateToken validates a token denomination and trace identifiers. +func (t Token) Validate() error { + if err := sdk.ValidateDenom(t.Denom); err != nil { + return errorsmod.Wrap(types.ErrInvalidDenomForTransfer, err.Error()) + } + + trace := strings.Join(t.Trace, "/") + identifiers := strings.Split(trace, "/") + + return denominternal.ValidateTraceIdentifiers(identifiers) +} + +// GetFullDenomPath returns the full denomination according to the ICS20 specification: +// tracePath + "/" + baseDenom +// If there exists no trace then the base denomination is returned. +func (t Token) GetFullDenomPath() string { + trace := strings.Join(t.Trace, "/") + if len(trace) == 0 { + return t.Denom + } + + return strings.Join(append(t.Trace, t.Denom), "/") +} diff --git a/modules/apps/transfer/types/v3/token_test.go b/modules/apps/transfer/types/v3/token_test.go new file mode 100644 index 00000000000..13654cfcc1d --- /dev/null +++ b/modules/apps/transfer/types/v3/token_test.go @@ -0,0 +1,140 @@ +package v3 + +import ( + fmt "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" +) + +func TestGetFullDenomPath(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketData + expPath string + }{ + { + "denom path with trace", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + "transfer/channel-0/transfer/channel-1/atom/pool", + }, + { + "nil trace", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{}, + }, + }, + sender, + receiver, + "", + ), + denom, + }, + { + "empty string trace", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{""}, + }, + }, + sender, + receiver, + "", + ), + denom, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + path := tc.packetData.Tokens[0].GetFullDenomPath() + require.Equal(t, tc.expPath, path) + }) + } +} + +func TestValidate(t *testing.T) { + testCases := []struct { + name string + token Token + expError error + }{ + { + "success: multiple port channel pair denom", + Token{ + Denom: "atom", + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + nil, + }, + { + "success: one port channel pair denom", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{"transfer/channel-1"}, + }, + nil, + }, + { + "success: non transfer port trace", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + }, + nil, + }, + { + "failure: empty denom", + Token{ + Denom: "", + Amount: amount, + Trace: nil, + }, + types.ErrInvalidDenomForTransfer, + }, + { + "failure: invalid identifier in trace", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{"transfer/channel-1", "randomport"}, + }, + fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: [transfer channel-1 randomport]"), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.token.Validate() + expPass := tc.expError == nil + if expPass { + require.NoError(t, err, tc.name) + } else { + require.ErrorContains(t, err, tc.expError.Error(), tc.name) + } + }) + } +}