feat(core, apps): 'PacketDataProvider' interface added and implemente…

…d (backport #4199) (#4218) * feat(core, apps): 'PacketDataProvider' interface added and implemented (#4199) * refactor(core/exported): moved packet interfaces to packet.go * feat(core/exported): 'PacketDataProvider' interface added * feat(transfer): PacketDataProvider implemented * feat(ica): implemented PacketDataProvider * style(transfer_test, ica_test): improved test name * docs(core.adr8): updated godocs * style(ica_test): changed a variable name * docs(core.adr8): added missing '.' * imp(transfer): removed type assertion on jsonKey * fix(transfer_test): removed unused test case parameter * docs(transfer): updated godocs * imp(ica): removed type assertion from 'GetCustomPacketData' * imp(transfer_test): improved tests without type assertion * imp(ica_test): improved tests without type assertion * style(transfer_test): changed test case parameter name (cherry picked from commit a0a6526) # Conflicts: # modules/apps/27-interchain-accounts/types/packet.go # modules/apps/transfer/types/packet.go # modules/core/exported/channel.go * fix: fixed backport conflicts * style: ran gofumpt * style(ica_test): typo fixed --------- Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com> Co-authored-by: srdtrk <srdtrk@hotmail.com>
cosmos · Aug 2, 2023 · 0ba9f75 · 0ba9f75
1 parent 8802471
commit 0ba9f75
Show file tree

Hide file tree

Showing 7 changed files with 273 additions and 50 deletions.
diff --git a/modules/apps/27-interchain-accounts/types/packet.go b/modules/apps/27-interchain-accounts/types/packet.go
@@ -1,13 +1,18 @@
 package types
 
 import (
+	"encoding/json"
 	"time"
 
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
+	ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
+var _ ibcexported.PacketDataProvider = (*InterchainAccountPacketData)(nil)
+
 // MaxMemoCharLength defines the maximum length for the InterchainAccountPacketData memo field
 const MaxMemoCharLength = 256
 
@@ -63,3 +68,25 @@ func (ct CosmosTx) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error {
 
 	return nil
 }
+
+// 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 (iapd InterchainAccountPacketData) GetCustomPacketData(key string) interface{} {
+	if len(iapd.Memo) == 0 {
+		return nil
+	}
+
+	jsonObject := make(map[string]interface{})
+	err := json.Unmarshal([]byte(iapd.Memo), &jsonObject)
+	if err != nil {
+		return nil
+	}
+
+	memoData, found := jsonObject[key]
+	if !found {
+		return nil
+	}
+
+	return memoData
+}
diff --git a/modules/apps/27-interchain-accounts/types/packet_test.go b/modules/apps/27-interchain-accounts/types/packet_test.go
@@ -1,7 +1,10 @@
 package types_test
 
 import (
+	"fmt"
+
 	"github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
+	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 )
 
 var largeMemo = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum"
@@ -82,3 +85,69 @@ func (suite *TypesTestSuite) TestValidateBasic() {
 		})
 	}
 }
+
+func (suite *TypesTestSuite) TestPacketDataProvider() {
+	expCallbackAddr := ibctesting.TestAccAddress
+
+	testCases := []struct {
+		name          string
+		packetData    types.InterchainAccountPacketData
+		expCustomData interface{}
+	}{
+		{
+			"success: src_callback key in memo",
+			types.InterchainAccountPacketData{
+				Type: types.EXECUTE_TX,
+				Data: []byte("data"),
+				Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, expCallbackAddr),
+			},
+			map[string]interface{}{
+				"address": expCallbackAddr,
+			},
+		},
+		{
+			"success: src_callback key in memo with additional fields",
+			types.InterchainAccountPacketData{
+				Type: types.EXECUTE_TX,
+				Data: []byte("data"),
+				Memo: fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "200000"}}`, expCallbackAddr),
+			},
+			map[string]interface{}{
+				"address":   expCallbackAddr,
+				"gas_limit": "200000",
+			},
+		},
+		{
+			"success: src_callback has string value",
+			types.InterchainAccountPacketData{
+				Type: types.EXECUTE_TX,
+				Data: []byte("data"),
+				Memo: `{"src_callback": "string"}`,
+			},
+			"string",
+		},
+		{
+			"failure: empty memo",
+			types.InterchainAccountPacketData{
+				Type: types.EXECUTE_TX,
+				Data: []byte("data"),
+				Memo: "",
+			},
+			nil,
+		},
+		{
+			"failure: non-json memo",
+			types.InterchainAccountPacketData{
+				Type: types.EXECUTE_TX,
+				Data: []byte("data"),
+				Memo: "invalid",
+			},
+			nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		customData := tc.packetData.GetCustomPacketData("src_callback")
+		suite.Require().Equal(tc.expCustomData, customData)
+	}
+}
diff --git a/modules/apps/transfer/types/msgs_test.go b/modules/apps/transfer/types/msgs_test.go
@@ -27,8 +27,8 @@ const (
 )
 
 var (
-	addr1     = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
-	addr2     = sdk.AccAddress("testaddr2").String()
+	sender    = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
+	receiver  = sdk.AccAddress("testreceiver").String()
 	emptyAddr string
 
 	coin             = sdk.NewCoin("atom", sdk.NewInt(100))
@@ -42,14 +42,14 @@ var (
 
 // TestMsgTransferRoute tests Route for MsgTransfer
 func TestMsgTransferRoute(t *testing.T) {
-	msg := types.NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, "")
+	msg := types.NewMsgTransfer(validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, "")
 
 	require.Equal(t, types.RouterKey, msg.Route())
 }
 
 func TestMsgTransferGetSignBytes(t *testing.T) {
-	msg := types.NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, "")
-	expected := fmt.Sprintf(`{"type":"cosmos-sdk/MsgTransfer","value":{"receiver":"%s","sender":"%s","source_channel":"testchannel","source_port":"testportid","timeout_height":{"revision_height":"10"},"token":{"amount":"100","denom":"atom"}}}`, addr2, addr1)
+	msg := types.NewMsgTransfer(validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, "")
+	expected := fmt.Sprintf(`{"type":"cosmos-sdk/MsgTransfer","value":{"receiver":"%s","sender":"%s","source_channel":"testchannel","source_port":"testportid","timeout_height":{"revision_height":"10"},"token":{"amount":"100","denom":"atom"}}}`, receiver, sender)
 	require.NotPanics(t, func() {
 		res := msg.GetSignBytes()
 		require.Equal(t, expected, string(res))
@@ -63,20 +63,20 @@ func TestMsgTransferValidation(t *testing.T) {
 		msg     *types.MsgTransfer
 		expPass bool
 	}{
-		{"valid msg with base denom", types.NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), true},
-		{"valid msg with trace hash", types.NewMsgTransfer(validPort, validChannel, ibcCoin, addr1, addr2, timeoutHeight, 0, ""), true},
-		{"invalid ibc denom", types.NewMsgTransfer(validPort, validChannel, invalidIBCCoin, addr1, addr2, timeoutHeight, 0, ""), false},
-		{"too short port id", types.NewMsgTransfer(invalidShortPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false},
-		{"too long port id", types.NewMsgTransfer(invalidLongPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false},
-		{"port id contains non-alpha", types.NewMsgTransfer(invalidPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false},
-		{"too short channel id", types.NewMsgTransfer(validPort, invalidShortChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false},
-		{"too long channel id", types.NewMsgTransfer(validPort, invalidLongChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false},
-		{"channel id contains non-alpha", types.NewMsgTransfer(validPort, invalidChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false},
-		{"invalid denom", types.NewMsgTransfer(validPort, validChannel, invalidDenomCoin, addr1, addr2, timeoutHeight, 0, ""), false},
-		{"zero coin", types.NewMsgTransfer(validPort, validChannel, zeroCoin, addr1, addr2, timeoutHeight, 0, ""), false},
-		{"missing sender address", types.NewMsgTransfer(validPort, validChannel, coin, emptyAddr, addr2, timeoutHeight, 0, ""), false},
-		{"missing recipient address", types.NewMsgTransfer(validPort, validChannel, coin, addr1, "", timeoutHeight, 0, ""), false},
-		{"empty coin", types.NewMsgTransfer(validPort, validChannel, sdk.Coin{}, addr1, addr2, timeoutHeight, 0, ""), false},
+		{"valid msg with base denom", types.NewMsgTransfer(validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), true},
+		{"valid msg with trace hash", types.NewMsgTransfer(validPort, validChannel, ibcCoin, sender, receiver, timeoutHeight, 0, ""), true},
+		{"invalid ibc denom", types.NewMsgTransfer(validPort, validChannel, invalidIBCCoin, sender, receiver, timeoutHeight, 0, ""), false},
+		{"too short port id", types.NewMsgTransfer(invalidShortPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
+		{"too long port id", types.NewMsgTransfer(invalidLongPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
+		{"port id contains non-alpha", types.NewMsgTransfer(invalidPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
+		{"too short channel id", types.NewMsgTransfer(validPort, invalidShortChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
+		{"too long channel id", types.NewMsgTransfer(validPort, invalidLongChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
+		{"channel id contains non-alpha", types.NewMsgTransfer(validPort, invalidChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
+		{"invalid denom", types.NewMsgTransfer(validPort, validChannel, invalidDenomCoin, sender, receiver, timeoutHeight, 0, ""), false},
+		{"zero coin", types.NewMsgTransfer(validPort, validChannel, zeroCoin, sender, receiver, timeoutHeight, 0, ""), false},
+		{"missing sender address", types.NewMsgTransfer(validPort, validChannel, coin, emptyAddr, receiver, timeoutHeight, 0, ""), false},
+		{"missing recipient address", types.NewMsgTransfer(validPort, validChannel, coin, sender, "", timeoutHeight, 0, ""), false},
+		{"empty coin", types.NewMsgTransfer(validPort, validChannel, sdk.Coin{}, sender, receiver, timeoutHeight, 0, ""), false},
 	}
 
 	for i, tc := range testCases {
@@ -93,7 +93,7 @@ func TestMsgTransferValidation(t *testing.T) {
 func TestMsgTransferGetSigners(t *testing.T) {
 	addr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
 
-	msg := types.NewMsgTransfer(validPort, validChannel, coin, addr.String(), addr2, timeoutHeight, 0, "")
+	msg := types.NewMsgTransfer(validPort, validChannel, coin, addr.String(), receiver, timeoutHeight, 0, "")
 	res := msg.GetSigners()
 
 	require.Equal(t, []sdk.AccAddress{addr}, res)

diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go
@@ -1,13 +1,18 @@
 package types
 
 import (
+	"encoding/json"
 	"strings"
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
+	ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
+var _ ibcexported.PacketDataProvider = (*FungibleTokenPacketData)(nil)
+
 var (
 	// DefaultRelativePacketTimeoutHeight is the default packet timeout height (in blocks) relative
 	// to the current block height of the counterparty chain provided by the client state. The
@@ -60,3 +65,25 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error {
 func (ftpd FungibleTokenPacketData) GetBytes() []byte {
 	return sdk.MustSortJSON(mustProtoMarshalJSON(&ftpd))
 }
+
+// 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
+}
diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go
@@ -1,6 +1,7 @@
 package types_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -22,16 +23,16 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) {
 		packetData types.FungibleTokenPacketData
 		expPass    bool
 	}{
-		{"valid packet", types.NewFungibleTokenPacketData(denom, amount, addr1, addr2, ""), true},
-		{"valid packet with memo", types.NewFungibleTokenPacketData(denom, amount, addr1, addr2, "memo"), true},
-		{"valid packet with large amount", types.NewFungibleTokenPacketData(denom, largeAmount, addr1, addr2, ""), true},
-		{"invalid denom", types.NewFungibleTokenPacketData("", amount, addr1, addr2, ""), false},
-		{"invalid empty amount", types.NewFungibleTokenPacketData(denom, "", addr1, addr2, ""), false},
-		{"invalid zero amount", types.NewFungibleTokenPacketData(denom, "0", addr1, addr2, ""), false},
-		{"invalid negative amount", types.NewFungibleTokenPacketData(denom, "-1", addr1, addr2, ""), false},
-		{"invalid large amount", types.NewFungibleTokenPacketData(denom, invalidLargeAmount, addr1, addr2, ""), false},
-		{"missing sender address", types.NewFungibleTokenPacketData(denom, amount, emptyAddr, addr2, ""), false},
-		{"missing recipient address", types.NewFungibleTokenPacketData(denom, amount, addr1, emptyAddr, ""), false},
+		{"valid packet", types.NewFungibleTokenPacketData(denom, amount, sender, receiver, ""), true},
+		{"valid packet with memo", types.NewFungibleTokenPacketData(denom, amount, sender, receiver, "memo"), true},
+		{"valid packet with large amount", types.NewFungibleTokenPacketData(denom, largeAmount, sender, receiver, ""), true},
+		{"invalid denom", types.NewFungibleTokenPacketData("", amount, sender, receiver, ""), false},
+		{"invalid empty amount", types.NewFungibleTokenPacketData(denom, "", sender, receiver, ""), false},
+		{"invalid zero amount", types.NewFungibleTokenPacketData(denom, "0", sender, receiver, ""), false},
+		{"invalid negative amount", types.NewFungibleTokenPacketData(denom, "-1", sender, receiver, ""), false},
+		{"invalid large amount", types.NewFungibleTokenPacketData(denom, invalidLargeAmount, sender, receiver, ""), false},
+		{"missing sender address", types.NewFungibleTokenPacketData(denom, amount, emptyAddr, receiver, ""), false},
+		{"missing recipient address", types.NewFungibleTokenPacketData(denom, amount, sender, emptyAddr, ""), false},
 	}
 
 	for i, tc := range testCases {
@@ -43,3 +44,77 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) {
 		}
 	}
 }
+
+func (suite *TypesTestSuite) TestPacketDataProvider() {
+	testCases := []struct {
+		name          string
+		packetData    types.FungibleTokenPacketData
+		expCustomData interface{}
+	}{
+		{
+			"success: src_callback key in memo",
+			types.FungibleTokenPacketData{
+				Denom:    denom,
+				Amount:   amount,
+				Sender:   sender,
+				Receiver: receiver,
+				Memo:     fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, receiver),
+			},
+			map[string]interface{}{
+				"address": receiver,
+			},
+		},
+		{
+			"success: src_callback key in memo with additional fields",
+			types.FungibleTokenPacketData{
+				Denom:    denom,
+				Amount:   amount,
+				Sender:   sender,
+				Receiver: receiver,
+				Memo:     fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "200000"}}`, receiver),
+			},
+			map[string]interface{}{
+				"address":   receiver,
+				"gas_limit": "200000",
+			},
+		},
+		{
+			"success: src_callback has string value",
+			types.FungibleTokenPacketData{
+				Denom:    denom,
+				Amount:   amount,
+				Sender:   sender,
+				Receiver: receiver,
+				Memo:     `{"src_callback": "string"}`,
+			},
+			"string",
+		},
+		{
+			"failure: empty memo",
+			types.FungibleTokenPacketData{
+				Denom:    denom,
+				Amount:   amount,
+				Sender:   sender,
+				Receiver: receiver,
+				Memo:     "",
+			},
+			nil,
+		},
+		{
+			"failure: non-json memo",
+			types.FungibleTokenPacketData{
+				Denom:    denom,
+				Amount:   amount,
+				Sender:   sender,
+				Receiver: receiver,
+				Memo:     "invalid",
+			},
+			nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		customData := tc.packetData.GetCustomPacketData("src_callback")
+		suite.Require().Equal(tc.expCustomData, customData)
+	}
+}
diff --git a/modules/core/exported/channel.go b/modules/core/exported/channel.go
@@ -17,23 +17,3 @@ type CounterpartyChannelI interface {
 	GetChannelID() string
 	ValidateBasic() error
 }
-
-// PacketI defines the standard interface for IBC packets
-type PacketI interface {
-	GetSequence() uint64
-	GetTimeoutHeight() Height
-	GetTimeoutTimestamp() uint64
-	GetSourcePort() string
-	GetSourceChannel() string
-	GetDestPort() string
-	GetDestChannel() string
-	GetData() []byte
-	ValidateBasic() error
-}
-
-// Acknowledgement defines the interface used to return
-// acknowledgements in the OnRecvPacket callback.
-type Acknowledgement interface {
-	Success() bool
-	Acknowledgement() []byte
-}