adr 8 callback packet data impl followups

modules/apps/27-interchain-accounts/types/codec.go

@@ -7,7 +7,6 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/cosmos/gogoproto/proto"
-	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
 // ModuleCdc references the global interchain accounts module codec. Note, the codec
@@ -22,11 +21,6 @@ var ModuleCdc = codec.NewProtoCodec(codectypes.NewInterfaceRegistry())
 func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
 	registry.RegisterImplementations((*authtypes.AccountI)(nil), &InterchainAccount{})
 	registry.RegisterImplementations((*authtypes.GenesisAccount)(nil), &InterchainAccount{})
-
-	registry.RegisterImplementations(
-		(*exported.CallbackPacketDataI)(nil),
-		&InterchainAccountPacketData{},
-	)
 }
 
 // SerializeCosmosTx serializes a slice of sdk.Msg's using the CosmosTx type. The sdk.Msg's are

modules/apps/27-interchain-accounts/types/packet.go

@@ -26,7 +26,7 @@ var (
 	DefaultRelativePacketTimeoutTimestamp = uint64((time.Duration(10) * time.Minute).Nanoseconds())
 )
 
-var _ exported.CallbackPacketDataI = (*InterchainAccountPacketData)(nil)
+var _ exported.CallbackPacketData = (*InterchainAccountPacketData)(nil)
 
 // ValidateBasic performs basic validation of the interchain account packet data.
 // The memo may be empty.
@@ -55,7 +55,7 @@ func (iapd InterchainAccountPacketData) GetBytes() []byte {
 
 ADR-8 CallbackPacketData implementation
 
-InterchainAccountPacketData implements CallbackPacketDataI interface. This will allow middlewares targetting specific VMs
+InterchainAccountPacketData implements CallbackPacketDataI interface. This will allow middlewares targeting specific VMs
 to retrieve the desired callback addresses for the ICA packet on the source and destination chains.
 
 The Memo is used to set the desired callback addresses.
@@ -66,27 +66,32 @@ The Memo format is defined like so:
 {
 	// ... other memo fields we don't care about
 	"callbacks": {
-		"src_callback_address": {contractAddrOnSrcChain},
+		"src_callback_address": {contractAddrOnSourceChain},
 		"dest_callback_address": {contractAddrOnDestChain},
-		"src_callback_msg": {jsonObjectForSrcChainCallback}, // optional field
-		"dest_callback_msg": {jsonObjectForDestChainCallback}, // optional field
-	}
 
+		// optional fields
+		"src_callback_msg": {jsonObjectForSourceChainCallback},
+		"dest_callback_msg": {jsonObjectForDestChainCallback},
+	}
 }
 ```
 
 */
 
-// GetSrcCallbackAddress returns the callback address on the source chain.
-// ADR-8 middleware should callback on the sender address on the source chain
-// if the sender address is an IBC Actor (i.e. smart contract that accepts IBC callbacks)
-func (iapd InterchainAccountPacketData) GetSrcCallbackAddress() string {
+// GetSourceCallbackAddress returns the source callback address provided in the packet data memo.
+// If no callback address is specified, an empty string is returned.
+//
+// The memo is expected to specify the callback address in the following format:
+// { "callbacks": { "src_callback_address": {contractAddrOnSourceChain}}
+//
+// ADR-8 middleware should callback on the returned address if it is a PacketActor
+// (i.e. smart contract that accepts IBC callbacks).
+func (iapd InterchainAccountPacketData) GetSourceCallbackAddress() string {
 	if len(iapd.Memo) == 0 {
 		return ""
 	}
 
 	jsonObject := make(map[string]interface{})
-	// the jsonObject must be a valid JSON object
 	err := json.Unmarshal([]byte(iapd.Memo), &jsonObject)
 	if err != nil {
 		return ""
@@ -101,17 +106,19 @@ func (iapd InterchainAccountPacketData) GetSrcCallbackAddress() string {
 	if !ok {
 		return ""
 	}
+
 	return callbackAddr
 }
 
-// GetDestCallbackAddress returns the callback address on the destination chain.
-// ADR-8 middleware should callback on the receiver address on the destination chain
-// if the receiver address is an IBC Actor (i.e. smart contract that accepts IBC callbacks)
+// GetDestCallbackAddress returns an empty string. Destination callback addresses
+// are not supported for ICS 27 since this feature is natively supported by
+// interchain accounts host submodule transaction execution.
 func (iapd InterchainAccountPacketData) GetDestCallbackAddress() string {
 	return ""
 }
 
-// UserDefinedGasLimit no-ops on this method to use relayer passed in gas
+// UserDefinedGasLimit returns 0 (no-op). The gas limit of the executing
+// transaction will be used.
 func (fptd InterchainAccountPacketData) UserDefinedGasLimit() uint64 {
 	return 0
 }

modules/apps/27-interchain-accounts/types/packet_test.go

@@ -1,6 +1,8 @@
 package types_test
 
 import (
+	"fmt"
+
 	"github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
 )
 
@@ -83,11 +85,13 @@ func (suite *TypesTestSuite) TestValidateBasic() {
 	}
 }
 
-func (suite *TypesTestSuite) TestGetCallbackAddresses() {
+func (suite *TypesTestSuite) TestGetSourceCallbackAddress() {
+	expSrcCbAddr := "srcCbAddr"
+
 	testCases := []struct {
-		name               string
-		packetData         types.InterchainAccountPacketData
-		expSrcCallbackAddr string
+		name       string
+		packetData types.InterchainAccountPacketData
+		expPass    bool
 	}{
 		{
 			"memo is empty",
@@ -96,7 +100,7 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
 				Data: []byte("data"),
 				Memo: "",
 			},
-			"",
+			false,
 		},
 		{
 			"memo is not json string",
@@ -105,7 +109,7 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
 				Data: []byte("data"),
 				Memo: "memo",
 			},
-			"",
+			false,
 		},
 		{
 			"memo is does not have callbacks in json struct",
@@ -114,7 +118,7 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
 				Data: []byte("data"),
 				Memo: "{\"Key\": 10}",
 			},
-			"",
+			false,
 		},
 		{
 			"memo has callbacks in json struct but does not have src_callback_address key",
@@ -123,7 +127,7 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
 				Data: []byte("data"),
 				Memo: "{\"callbacks\": {\"Key\": 10}}",
 			},
-			"",
+			false,
 		},
 		{
 			"memo has callbacks in json struct but does not have string value for src_callback_address key",
@@ -132,21 +136,71 @@ func (suite *TypesTestSuite) TestGetCallbackAddresses() {
 				Data: []byte("data"),
 				Memo: "{\"callbacks\": {\"src_callback_address\": 10}}",
 			},
-			"",
+			false,
 		},
 		{
-			"memo has callbacks in json struct but does not have string value for src_callback_address key",
+			"memo has callbacks in json struct and properly formatted src_callback_address",
+			types.InterchainAccountPacketData{
+				Type: types.EXECUTE_TX,
+				Data: []byte("data"),
+				Memo: fmt.Sprintf("{\"callbacks\": {\"src_callback_address\": \"%s\"}}", expSrcCbAddr),
+			},
+			true,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			srcCbAddr := tc.packetData.GetSourceCallbackAddress()
+
+			if tc.expPass {
+				suite.Require().Equal(expSrcCbAddr, srcCbAddr)
+			} else {
+				suite.Require().Equal("", srcCbAddr)
+			}
+		})
+	}
+}
+
+func (suite *TypesTestSuite) TestGetDestCallbackAddress() {
+	testCases := []struct {
+		name       string
+		packetData types.InterchainAccountPacketData
+	}{
+		{
+			"memo is empty",
 			types.InterchainAccountPacketData{
 				Type: types.EXECUTE_TX,
 				Data: []byte("data"),
-				Memo: "{\"callbacks\": {\"src_callback_address\": \"testAddress\"}}",
+				Memo: "",
+			},
+		},
+		{
+			"memo has dest callback address specified in json struct",
+			types.InterchainAccountPacketData{
+				Type: types.EXECUTE_TX,
+				Data: []byte("data"),
+				Memo: "{\"callbacks\": {\"dest_callback_address\": \"testAddress\"}}",
 			},
-			"testAddress",
 		},
 	}
 
 	for _, tc := range testCases {
-		srcCbAddr := tc.packetData.GetSrcCallbackAddress()
-		suite.Require().Equal(tc.expSrcCallbackAddr, srcCbAddr, "%s testcase does not have expected src_callback_address", tc.name)
+		tc := tc
+		suite.Run(tc.name, func() {
+			destCbAddr := tc.packetData.GetDestCallbackAddress()
+			suite.Require().Equal("", destCbAddr)
+		})
+	}
+}
+
+func (suite *TypesTestSuite) TestUserDefinedGasLimit() {
+	packetData := types.InterchainAccountPacketData{
+		Type: types.EXECUTE_TX,
+		Data: []byte("data"),
+		Memo: "{\"callbacks\": {\"user_defined_gas_limit\": 100}}",
 	}
+
+	suite.Require().Equal(uint64(0), packetData.UserDefinedGasLimit(), "user defined gas limit does not return 0")
 }

modules/apps/transfer/types/codec.go

@@ -3,15 +3,13 @@ package types
 import (
 	"bytes"
 
-	"github.com/cosmos/cosmos-sdk/x/authz"
-	"github.com/cosmos/gogoproto/jsonpb"
-	"github.com/cosmos/gogoproto/proto"
-	"github.com/cosmos/ibc-go/v7/modules/core/exported"
-
 	"github.com/cosmos/cosmos-sdk/codec"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/msgservice"
+	"github.com/cosmos/cosmos-sdk/x/authz"
+	"github.com/cosmos/gogoproto/jsonpb"
+	"github.com/cosmos/gogoproto/proto"
 )
 
 // RegisterLegacyAminoCodec registers the necessary x/ibc transfer interfaces and concrete types
@@ -30,11 +28,6 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
 		&TransferAuthorization{},
 	)
 
-	registry.RegisterImplementations(
-		(*exported.CallbackPacketDataI)(nil),
-		&FungibleTokenPacketData{},
-	)
-
 	msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
 }
 

modules/apps/transfer/types/packet.go

@@ -25,7 +25,7 @@ var (
 	DefaultRelativePacketTimeoutTimestamp = uint64((time.Duration(10) * time.Minute).Nanoseconds())
 )
 
-var _ exported.CallbackPacketDataI = (*FungibleTokenPacketData)(nil)
+var _ exported.CallbackPacketData = (*FungibleTokenPacketData)(nil)
 
 // NewFungibleTokenPacketData contructs a new FungibleTokenPacketData instance
 func NewFungibleTokenPacketData(
@@ -67,11 +67,11 @@ func (ftpd FungibleTokenPacketData) GetBytes() []byte {
 	return sdk.MustSortJSON(mustProtoMarshalJSON(&ftpd))
 }
 
-/**
+/*
 
 ADR-8 CallbackPacketData implementation
 
-FungibleTokenPacketData implements CallbackPacketDataI interface. This will allow middlewares targetting specific VMs
+FungibleTokenPacketData implements CallbackPacketDataI interface. This will allow middlewares targeting specific VMs
 to retrieve the desired callback addresses for the ICS20 packet on the source and destination chains.
 
 The Memo is used to ensure that the callback is desired by the user. This allows a user to send an ICS20 packet
@@ -84,32 +84,38 @@ The Memo format is defined like so:
 {
 	// ... other memo fields we don't care about
 	"callbacks": {
-		"src_callback_address": {contractAddrOnSrcChain},
+		"src_callback_address": {contractAddrOnSourceChain},
 		"dest_callback_address": {contractAddrOnDestChain},
-		"src_callback_msg": {jsonObjectForSrcChainCallback},
+
+		// optional fields
+		"src_callback_msg": {jsonObjectForSourceChainCallback},
 		"dest_callback_msg": {jsonObjectForDestChainCallback},
 	}
-
 }
 ```
 
 For transfer, we will enforce that the src_callback_address is the same as sender and dest_callback_address is the same as receiver.
 However, we may remove this restriction at a later date if it proves useful.
 
-**/
-
-// ADR-8 middleware should callback on the sender address on the source chain
-// if the sender address is an IBC Actor (i.e. smart contract that accepts IBC callbacks)
-// The desired callback address must be confirmed in the memo under the "callbacks" key.
-// This ensures that the callback is explicitly desired by the user and not just called
-// automatically.
-func (ftpd FungibleTokenPacketData) GetSrcCallbackAddress() string {
+*/
+
+// GetSourceCallbackAddress returns the sender address if it is also specified in
+// the packet data memo. The desired callback address must be confirmed in the
+// memo under the "callbacks" key. This ensures that the callback is explicitly
+// desired by the user and not called automatically. If no callback address is
+// specified, an empty string is returned.
+//
+// The memo is expected to contain the source callback address in the following format:
+// { "callbacks": { "src_callback_address": {contractAddrOnSourceChain}}
+//
+// ADR-8 middleware should callback on the returned address if it is a PacketActor
+// (i.e. smart contract that accepts IBC callbacks).
+func (ftpd FungibleTokenPacketData) GetSourceCallbackAddress() string {
 	if len(ftpd.Memo) == 0 {
 		return ""
 	}
 
 	jsonObject := make(map[string]interface{})
-	// the jsonObject must be a valid JSON object
 	err := json.Unmarshal([]byte(ftpd.Memo), &jsonObject)
 	if err != nil {
 		return ""
@@ -123,21 +129,27 @@ func (ftpd FungibleTokenPacketData) GetSrcCallbackAddress() string {
 	if callbackData["src_callback_address"] == ftpd.Sender {
 		return ftpd.Sender
 	}
+
 	return ""
 }
 
-// ADR-8 middleware should callback on the receiver address on the destination chain
-// if the receiver address is an IBC Actor (i.e. smart contract that accepts IBC callbacks)
-// The desired callback address must be confirmed in the memo under the "callbacks" key.
-// This ensures that the callback is explicitly desired by the user and not just called
-// automatically.
+// GetDestCallbackAddress returns the receiving address if it is also specified in
+// the packet data memo. The desired callback address must be confirmed in the
+// memo under the "callbacks" key. This ensures that the callback is explicitly
+// desired by the user and not called automatically. If no callback address is
+// specified, an empty string is returned.
+//
+// The memo is expected to contain the destination callback address in the following format:
+// { "callbacks": { "dest_callback_address": {contractAddrOnDestChain}}
+//
+// ADR-8 middleware should callback on the returned address if it is a PacketActor
+// (i.e. smart contract that accepts IBC callbacks).
 func (ftpd FungibleTokenPacketData) GetDestCallbackAddress() string {
 	if len(ftpd.Memo) == 0 {
 		return ""
 	}
 
 	jsonObject := make(map[string]interface{})
-	// the jsonObject must be a valid JSON object
 	err := json.Unmarshal([]byte(ftpd.Memo), &jsonObject)
 	if err != nil {
 		return ""
@@ -151,10 +163,12 @@ func (ftpd FungibleTokenPacketData) GetDestCallbackAddress() string {
 	if callbackData["dest_callback_address"] == ftpd.Receiver {
 		return ftpd.Receiver
 	}
+
 	return ""
 }
 
-// no-op on this method to use relayer passed in gas
+// UserDefinedGasLimit returns 0 (no-op). The gas limit of the executing
+// transaction will be used.
 func (fptd FungibleTokenPacketData) UserDefinedGasLimit() uint64 {
 	return 0
 }