adr 8 callback packet data impl followups

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

@@ -24,7 +24,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
 	registry.RegisterImplementations((*authtypes.GenesisAccount)(nil), &InterchainAccount{})
 
 	registry.RegisterImplementations(
-		(*exported.CallbackPacketDataI)(nil),
+		(*exported.CallbackPacketData)(nil),
 		&InterchainAccountPacketData{},
 	)
 }

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

@@ -1,16 +1,41 @@
 package types_test
 
 import (
+	"fmt"
+
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
 	"github.com/cosmos/gogoproto/proto"
 
 	"github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
+	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 	"github.com/cosmos/ibc-go/v7/testing/simapp"
 )
 
+func (suite *TypesTestSuite) TestUnmarshalCallbackPacketData() {
+	sourceCallbackAddress := "sourceCallbackAddress"
+
+	packetData := types.InterchainAccountPacketData{
+		Type: types.EXECUTE_TX,
+		Data: []byte("data"),
+		Memo: fmt.Sprintf("{\"callbacks\": {\"src_callback_address\": \"%s\"}}", sourceCallbackAddress),
+	}
+
+	types.RegisterInterfaces(types.ModuleCdc.InterfaceRegistry())
+	data, err := types.ModuleCdc.MarshalInterfaceJSON(&packetData)
+	suite.Require().NoError(err)
+
+	var callbackPacketData exported.CallbackPacketData
+	err = suite.chainA.App.AppCodec().UnmarshalInterfaceJSON(data, &callbackPacketData)
+	suite.Require().NoError(err, "failed to unmarshal into callback packet data interface")
+
+	suite.Require().Equal(sourceCallbackAddress, callbackPacketData.GetSourceCallbackAddress())
+	suite.Require().Equal("", callbackPacketData.GetDestCallbackAddress())
+	suite.Require().Equal(uint64(0), callbackPacketData.UserDefinedGasLimit())
+}
+
 // mockSdkMsg defines a mock struct, used for testing codec error scenarios
 type mockSdkMsg struct{}
 

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

@@ -31,7 +31,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
 	)
 
 	registry.RegisterImplementations(
-		(*exported.CallbackPacketDataI)(nil),
+		(*exported.CallbackPacketData)(nil),
 		&FungibleTokenPacketData{},
 	)