From a0a6526313c9df3ea5e0c17e0c71fed91ef99f06 Mon Sep 17 00:00:00 2001 From: srdtrk <59252793+srdtrk@users.noreply.github.com> Date: Tue, 1 Aug 2023 18:18:45 +0200 Subject: [PATCH] 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 --- .../27-interchain-accounts/types/packet.go | 27 +++++++ .../types/packet_test.go | 69 +++++++++++++++++ modules/apps/transfer/types/packet.go | 26 +++++++ modules/apps/transfer/types/packet_test.go | 75 +++++++++++++++++++ modules/core/exported/channel.go | 33 -------- modules/core/exported/packet.go | 45 +++++++++++ 6 files changed, 242 insertions(+), 33 deletions(-) create mode 100644 modules/core/exported/packet.go diff --git a/modules/apps/27-interchain-accounts/types/packet.go b/modules/apps/27-interchain-accounts/types/packet.go index 213497f6ac6..0173169be9a 100644 --- a/modules/apps/27-interchain-accounts/types/packet.go +++ b/modules/apps/27-interchain-accounts/types/packet.go @@ -1,14 +1,19 @@ package types import ( + "encoding/json" "time" errorsmod "cosmossdk.io/errors" codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" + + 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 @@ -64,3 +69,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 index 329e5a837f4..7a18923202d 100644 --- 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 valu", + 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/packet.go b/modules/apps/transfer/types/packet.go index 66f3e5730e1..86b49c469d1 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -1,6 +1,7 @@ package types import ( + "encoding/json" "strings" "time" @@ -10,8 +11,11 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ibcerrors "github.com/cosmos/ibc-go/v7/modules/core/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 @@ -64,3 +68,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 index 6c877d4fcb0..47d6738a654 100644 --- 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" @@ -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 index ac52a3f34ba..a6ec511880b 100644 --- a/modules/core/exported/channel.go +++ b/modules/core/exported/channel.go @@ -17,36 +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. -// The Acknowledgement interface is used by core IBC to ensure partial state changes are not committed -// when packet receives have not properly succeeded (typically resulting in an error acknowledgement being returned). -// The interface also allows core IBC to obtain the acknowledgement bytes whose encoding is determined by each IBC application or middleware. -// Each custom acknowledgement type must implement this interface. -type Acknowledgement interface { - // Success determines if the IBC application state should be persisted when handling `RecvPacket`. - // During `OnRecvPacket` IBC application callback execution, all state changes are held in a cache store and committed if: - // - the acknowledgement.Success() returns true - // - a nil acknowledgement is returned (asynchronous acknowledgements) - // - // Note 1: IBC application callback events are always persisted so long as `RecvPacket` succeeds without error. - // - // Note 2: The return value should account for the success of the underlying IBC application or middleware. Thus the `acknowledgement.Success` is representative of the entire IBC stack's success when receiving a packet. The individual success of each acknowledgement associated with an IBC application or middleware must be determined by obtaining the actual acknowledgement type after decoding the acknowledgement bytes. - // - // See https://github.com/cosmos/ibc-go/blob/v7.0.0/docs/ibc/apps.md for further explanations. - Success() bool - Acknowledgement() []byte -} diff --git a/modules/core/exported/packet.go b/modules/core/exported/packet.go new file mode 100644 index 00000000000..15427df3c5b --- /dev/null +++ b/modules/core/exported/packet.go @@ -0,0 +1,45 @@ +package exported + +// 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. +// The Acknowledgement interface is used by core IBC to ensure partial state changes are not committed +// when packet receives have not properly succeeded (typically resulting in an error acknowledgement being returned). +// The interface also allows core IBC to obtain the acknowledgement bytes whose encoding is determined by each IBC application or middleware. +// Each custom acknowledgement type must implement this interface. +type Acknowledgement interface { + // Success determines if the IBC application state should be persisted when handling `RecvPacket`. + // During `OnRecvPacket` IBC application callback execution, all state changes are held in a cache store and committed if: + // - the acknowledgement.Success() returns true + // - a nil acknowledgement is returned (asynchronous acknowledgements) + // + // Note 1: IBC application callback events are always persisted so long as `RecvPacket` succeeds without error. + // + // Note 2: The return value should account for the success of the underlying IBC application or middleware. Thus the `acknowledgement.Success` is representative of the entire IBC stack's success when receiving a packet. The individual success of each acknowledgement associated with an IBC application or middleware must be determined by obtaining the actual acknowledgement type after decoding the acknowledgement bytes. + // + // See https://github.com/cosmos/ibc-go/blob/v7.0.0/docs/ibc/apps.md for further explanations. + Success() bool + Acknowledgement() []byte +} + +// PacketDataProvider defines an optional interfaces for retrieving custom packet data stored on behalf of another application. +// An existing problem in the IBC middleware design is the inability for a middleware to define its own packet data type and insert packet sender provided information. +// A short term solution was introduced into several application's packet data to utilize a memo field to carry this information on behalf of another application. +// This interfaces standardizes that behaviour. Upon realization of the ability for middleware's to define their own packet data types, this interface will be deprecated and removed with time. +type PacketDataProvider interface { + // GetCustomPacketData returns the packet data held on behalf of another application. + // The name the information is stored under should be provided as the key. + // If no custom packet data exists for the key, nil should be returned. + GetCustomPacketData(key string) interface{} +}