Add field in callbacks data for custom calldata (backport #8353)

modules/apps/callbacks/ibc_middleware_test.go

@@ -94,6 +94,7 @@ func (s *CallbacksTestSuite) TestWithICS4Wrapper() {
 
 func (s *CallbacksTestSuite) TestSendPacket() {
 	var packetData transfertypes.FungibleTokenPacketData
+	var callbackExecuted bool
 
 	testCases := []struct {
 		name         string
@@ -127,7 +128,7 @@ func (s *CallbacksTestSuite) TestSendPacket() {
 			},
 			"none", // improperly formatted callback data should result in no callback execution
 			false,
-			types.ErrCallbackAddressNotFound,
+			types.ErrInvalidCallbackData,
 		},
 		{
 			"failure: ics4Wrapper SendPacket call fails",
@@ -165,6 +166,46 @@ func (s *CallbacksTestSuite) TestSendPacket() {
 			false,
 			errorsmod.Wrapf(types.ErrCallbackOutOfGas, "ibc %s callback out of gas", types.CallbackTypeSendPacket),
 		},
+		{
+			"failure: callback address invalid",
+			func() {
+				packetData.Memo = fmt.Sprintf(`{"src_callback": {"address":%d}}`, 50)
+				callbackExecuted = false // callback should not be executed
+			},
+			types.CallbackTypeSendPacket,
+			false,
+			types.ErrInvalidCallbackData,
+		},
+		{
+			"failure: callback gas limit invalid",
+			func() {
+				packetData.Memo = fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":%d}}`, simapp.SuccessContract, 50)
+				callbackExecuted = false // callback should not be executed
+			},
+			types.CallbackTypeSendPacket,
+			false,
+			types.ErrInvalidCallbackData,
+		},
+		{
+			"failure: callback calldata invalid",
+			func() {
+				packetData.Memo = fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":"%d", "calldata":%d}}`, simapp.SuccessContract, 50, 50)
+				callbackExecuted = false // callback should not be executed
+			},
+			types.CallbackTypeSendPacket,
+			false,
+			types.ErrInvalidCallbackData,
+		},
+		{
+			"failure: callback calldata hex invalid",
+			func() {
+				packetData.Memo = fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":"%d", "calldata":"%s"}}`, simapp.SuccessContract, 50, "calldata")
+				callbackExecuted = false // callback should not be executed
+			},
+			types.CallbackTypeSendPacket,
+			false,
+			types.ErrInvalidCallbackData,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -180,6 +221,7 @@ func (s *CallbacksTestSuite) TestSendPacket() {
 				ibctesting.TestAccAddress,
 				fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, simapp.SuccessContract),
 			)
+			callbackExecuted = true
 
 			tc.malleate()
 
@@ -214,11 +256,13 @@ func (s *CallbacksTestSuite) TestSendPacket() {
 
 			default:
 				sendPacket()
-				s.Require().ErrorIs(err, tc.expValue.(error))
+				s.Require().ErrorIs(tc.expValue.(error), err)
 				s.Require().Equal(uint64(0), seq)
 			}
 
-			s.AssertHasExecutedExpectedCallback(tc.callbackType, expPass)
+			if callbackExecuted {
+				s.AssertHasExecutedExpectedCallback(tc.callbackType, expPass)
+			}
 		})
 	}
 }
@@ -279,7 +323,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
 				packet.Data = packetData.GetBytes()
 			},
 			noExecution,
-			types.ErrCallbackAddressNotFound,
+			types.ErrInvalidCallbackData,
 		},
 		{
 			"failure: callback execution reach out of gas, but sufficient gas provided by relayer",
@@ -449,7 +493,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() {
 				packet.Data = packetData.GetBytes()
 			},
 			noExecution,
-			types.ErrCallbackAddressNotFound,
+			types.ErrInvalidCallbackData,
 		},
 		{
 			"failure: callback execution reach out of gas, but sufficient gas provided by relayer",
@@ -624,7 +668,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() {
 				packet.Data = packetData.GetBytes()
 			},
 			noExecution,
-			channeltypes.NewErrorAcknowledgement(types.ErrCallbackAddressNotFound),
+			channeltypes.NewErrorAcknowledgement(types.ErrInvalidCallbackData),
 		},
 		{
 			"failure: callback execution reach out of gas, but sufficient gas provided by relayer",
@@ -782,7 +826,7 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() {
 				packet.Data = packetData.GetBytes()
 			},
 			"none", // improperly formatted callback data should result in no callback execution
-			types.ErrCallbackAddressNotFound,
+			types.ErrInvalidCallbackData,
 		},
 		{
 			"failure: ics4Wrapper WriteAcknowledgement call fails",

modules/apps/callbacks/types/callbacks.go

@@ -1,11 +1,14 @@
 package types
 
 import (
+	"encoding/hex"
 	"strconv"
 	"strings"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
+	errorsmod "cosmossdk.io/errors"
+
 	channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v10/modules/core/05-port/types"
 	"github.com/cosmos/ibc-go/v10/modules/core/api"
@@ -74,6 +77,9 @@
 	CommitGasLimit uint64
 	// ApplicationVersion is the base application version.
 	ApplicationVersion string
+	// Calldata is the calldata to be passed to the callback actor.
+	// This may be empty but if it is not empty, it should be the calldata sent to the callback actor.
+	Calldata []byte
 }
 
 // GetSourceCallbackData parses the packet data and returns the source callback data.
@@ -129,9 +135,9 @@
 	}
 
 	// get the callback address from the callback data
-	callbackAddress := getCallbackAddress(callbackData)
-	if strings.TrimSpace(callbackAddress) == "" {
-		return CallbackData{}, true, ErrCallbackAddressNotFound
+	callbackAddress, err := getCallbackAddress(callbackData)
+	if err != nil || strings.TrimSpace(callbackAddress) == "" {
+		return CallbackData{}, true, ErrInvalidCallbackData
 	}
 
 	// retrieve packet sender from packet data if possible and if needed
@@ -144,20 +150,32 @@
 	}
 
 	// get the gas limit from the callback data
-	executionGasLimit, commitGasLimit := computeExecAndCommitGasLimit(callbackData, remainingGas, maxGas)
+	executionGasLimit, commitGasLimit, err := computeExecAndCommitGasLimit(callbackData, remainingGas, maxGas)
+	if err != nil {
+		return CallbackData{}, true, err
+	}
+
+	callData, err := getCalldata(callbackData)
+	if err != nil {
+		return CallbackData{}, true, err
+	}
 
 	return CallbackData{
 		CallbackAddress:    callbackAddress,
 		ExecutionGasLimit:  executionGasLimit,
 		SenderAddress:      packetSender,
 		CommitGasLimit:     commitGasLimit,
 		ApplicationVersion: version,
+		Calldata:           callData,
 	}, true, nil
 }
 
-func computeExecAndCommitGasLimit(callbackData map[string]any, remainingGas, maxGas uint64) (uint64, uint64) {
+func computeExecAndCommitGasLimit(callbackData map[string]any, remainingGas, maxGas uint64) (uint64, uint64, error) {
 	// get the gas limit from the callback data
-	commitGasLimit := getUserDefinedGasLimit(callbackData)
+	commitGasLimit, err := getUserDefinedGasLimit(callbackData)
+	if err != nil {
+		return 0, 0, err
+	}
 
 	// ensure user defined gas limit does not exceed the max gas limit
 	if commitGasLimit == 0 || commitGasLimit > maxGas {
@@ -168,7 +186,7 @@
 	// in this case, the callback execution may be retried upon failure
 	executionGasLimit := min(remainingGas, commitGasLimit)
 
-	return executionGasLimit, commitGasLimit
+	return executionGasLimit, commitGasLimit, nil
 }
 
 // getUserDefinedGasLimit returns the custom gas limit provided for callbacks if it is
@@ -179,19 +197,26 @@
 // { "{callbackKey}": { ... , "gas_limit": {stringForCallback} }
 //
 // Note: the user defined gas limit must be set as a string and not a json number.
-func getUserDefinedGasLimit(callbackData map[string]any) uint64 {
+func getUserDefinedGasLimit(callbackData map[string]any) (uint64, error) {
 	// the gas limit must be specified as a string and not a json number
-	gasLimit, ok := callbackData[UserDefinedGasLimitKey].(string)
+	gasLimit, ok := callbackData[UserDefinedGasLimitKey]
 	if !ok {
-		return 0
+		return 0, nil
+	}
+	gasLimitStr, ok := gasLimit.(string)
+	if !ok {
+		return 0, errorsmod.Wrapf(ErrInvalidCallbackData, "gas limit [%v] must be a string", gasLimit)
+	}
+	if gasLimitStr == "" {
+		return 0, nil
 	}
 
-	userGas, err := strconv.ParseUint(gasLimit, 10, 64)
+	userGas, err := strconv.ParseUint(gasLimitStr, 10, 64)
 	if err != nil {
-		return 0
+		return 0, errorsmod.Wrapf(ErrInvalidCallbackData, "gas limit must be a valid uint64: %s", err)
 	}
 
-	return userGas
+	return userGas, nil
 }
 
 // getCallbackAddress returns the callback address if it is specified in the callback data.
@@ -203,13 +228,34 @@
 //
 // ADR-8 middleware should callback on the returned address if it is a PacketActor
 // (i.e. smart contract that accepts IBC callbacks).
-func getCallbackAddress(callbackData map[string]any) string {
+func getCallbackAddress(callbackData map[string]any) (string, error) {
 	callbackAddress, ok := callbackData[CallbackAddressKey].(string)
 	if !ok {
-		return ""
+		return "", errorsmod.Wrapf(ErrInvalidCallbackData, "callback address must be a string")
 	}
 
-	return callbackAddress
+	return callbackAddress, nil
+}
+
+// getCalldata returns the calldata if it is specified in the callback data.
+func getCalldata(callbackData map[string]any) ([]byte, error) {
+	calldataAny, ok := callbackData[CalldataKey]
+	if !ok {
+		return nil, nil
+	}
+	calldataStr, ok := calldataAny.(string)
+	if !ok {
+		return nil, errorsmod.Wrapf(ErrInvalidCallbackData, "calldata must be a string")
+	}
+	if calldataStr == "" {
+		return nil, nil
+	}
+
+	calldata, err := hex.DecodeString(calldataStr)
+	if err != nil {
+		return nil, errorsmod.Wrapf(ErrInvalidCallbackData, "calldata must be a valid hex string: %s", err)
+	}
+	return calldata, nil
 }
 
 // AllowRetry returns true if the callback execution gas limit is less than the commit gas limit.