Skip to content

Commit 7d579b9

Browse files
feat: add field in callbacks data for custom calldata (#8353)
Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
1 parent a5ad744 commit 7d579b9

File tree

7 files changed

+289
-33
lines changed

7 files changed

+289
-33
lines changed

modules/apps/callbacks/ibc_middleware_test.go

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ func (s *CallbacksTestSuite) TestWithICS4Wrapper() {
9494

9595
func (s *CallbacksTestSuite) TestSendPacket() {
9696
var packetData transfertypes.FungibleTokenPacketData
97+
var callbackExecuted bool
9798

9899
testCases := []struct {
99100
name string
@@ -127,7 +128,7 @@ func (s *CallbacksTestSuite) TestSendPacket() {
127128
},
128129
"none", // improperly formatted callback data should result in no callback execution
129130
false,
130-
types.ErrCallbackAddressNotFound,
131+
types.ErrInvalidCallbackData,
131132
},
132133
{
133134
"failure: ics4Wrapper SendPacket call fails",
@@ -165,6 +166,46 @@ func (s *CallbacksTestSuite) TestSendPacket() {
165166
false,
166167
errorsmod.Wrapf(types.ErrCallbackOutOfGas, "ibc %s callback out of gas", types.CallbackTypeSendPacket),
167168
},
169+
{
170+
"failure: callback address invalid",
171+
func() {
172+
packetData.Memo = fmt.Sprintf(`{"src_callback": {"address":%d}}`, 50)
173+
callbackExecuted = false // callback should not be executed
174+
},
175+
types.CallbackTypeSendPacket,
176+
false,
177+
types.ErrInvalidCallbackData,
178+
},
179+
{
180+
"failure: callback gas limit invalid",
181+
func() {
182+
packetData.Memo = fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":%d}}`, simapp.SuccessContract, 50)
183+
callbackExecuted = false // callback should not be executed
184+
},
185+
types.CallbackTypeSendPacket,
186+
false,
187+
types.ErrInvalidCallbackData,
188+
},
189+
{
190+
"failure: callback calldata invalid",
191+
func() {
192+
packetData.Memo = fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":"%d", "calldata":%d}}`, simapp.SuccessContract, 50, 50)
193+
callbackExecuted = false // callback should not be executed
194+
},
195+
types.CallbackTypeSendPacket,
196+
false,
197+
types.ErrInvalidCallbackData,
198+
},
199+
{
200+
"failure: callback calldata hex invalid",
201+
func() {
202+
packetData.Memo = fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":"%d", "calldata":"%s"}}`, simapp.SuccessContract, 50, "calldata")
203+
callbackExecuted = false // callback should not be executed
204+
},
205+
types.CallbackTypeSendPacket,
206+
false,
207+
types.ErrInvalidCallbackData,
208+
},
168209
}
169210

170211
for _, tc := range testCases {
@@ -180,6 +221,7 @@ func (s *CallbacksTestSuite) TestSendPacket() {
180221
ibctesting.TestAccAddress,
181222
fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, simapp.SuccessContract),
182223
)
224+
callbackExecuted = true
183225

184226
tc.malleate()
185227

@@ -214,11 +256,13 @@ func (s *CallbacksTestSuite) TestSendPacket() {
214256

215257
default:
216258
sendPacket()
217-
s.Require().ErrorIs(err, tc.expValue.(error))
259+
s.Require().ErrorIs(tc.expValue.(error), err)
218260
s.Require().Equal(uint64(0), seq)
219261
}
220262

221-
s.AssertHasExecutedExpectedCallback(tc.callbackType, expPass)
263+
if callbackExecuted {
264+
s.AssertHasExecutedExpectedCallback(tc.callbackType, expPass)
265+
}
222266
})
223267
}
224268
}
@@ -279,7 +323,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
279323
packet.Data = packetData.GetBytes()
280324
},
281325
noExecution,
282-
types.ErrCallbackAddressNotFound,
326+
types.ErrInvalidCallbackData,
283327
},
284328
{
285329
"failure: callback execution reach out of gas, but sufficient gas provided by relayer",
@@ -449,7 +493,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() {
449493
packet.Data = packetData.GetBytes()
450494
},
451495
noExecution,
452-
types.ErrCallbackAddressNotFound,
496+
types.ErrInvalidCallbackData,
453497
},
454498
{
455499
"failure: callback execution reach out of gas, but sufficient gas provided by relayer",
@@ -624,7 +668,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() {
624668
packet.Data = packetData.GetBytes()
625669
},
626670
noExecution,
627-
channeltypes.NewErrorAcknowledgement(types.ErrCallbackAddressNotFound),
671+
channeltypes.NewErrorAcknowledgement(types.ErrInvalidCallbackData),
628672
},
629673
{
630674
"failure: callback execution reach out of gas, but sufficient gas provided by relayer",
@@ -782,7 +826,7 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() {
782826
packet.Data = packetData.GetBytes()
783827
},
784828
"none", // improperly formatted callback data should result in no callback execution
785-
types.ErrCallbackAddressNotFound,
829+
types.ErrInvalidCallbackData,
786830
},
787831
{
788832
"failure: ics4Wrapper WriteAcknowledgement call fails",

modules/apps/callbacks/types/callbacks.go

Lines changed: 62 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package types
22

33
import (
4+
"encoding/hex"
45
"strconv"
56
"strings"
67

78
sdk "github.com/cosmos/cosmos-sdk/types"
89

10+
errorsmod "cosmossdk.io/errors"
11+
912
channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types"
1013
porttypes "github.com/cosmos/ibc-go/v10/modules/core/05-port/types"
1114
ibcexported "github.com/cosmos/ibc-go/v10/modules/core/exported"
@@ -59,6 +62,9 @@ type CallbackData struct {
5962
CommitGasLimit uint64
6063
// ApplicationVersion is the base application version.
6164
ApplicationVersion string
65+
// Calldata is the calldata to be passed to the callback actor.
66+
// This may be empty but if it is not empty, it should be the calldata sent to the callback actor.
67+
Calldata []byte
6268
}
6369

6470
// GetSourceCallbackData parses the packet data and returns the source callback data.
@@ -114,9 +120,9 @@ func GetCallbackData(
114120
}
115121

116122
// get the callback address from the callback data
117-
callbackAddress := getCallbackAddress(callbackData)
118-
if strings.TrimSpace(callbackAddress) == "" {
119-
return CallbackData{}, true, ErrCallbackAddressNotFound
123+
callbackAddress, err := getCallbackAddress(callbackData)
124+
if err != nil || strings.TrimSpace(callbackAddress) == "" {
125+
return CallbackData{}, true, ErrInvalidCallbackData
120126
}
121127

122128
// retrieve packet sender from packet data if possible and if needed
@@ -129,20 +135,32 @@ func GetCallbackData(
129135
}
130136

131137
// get the gas limit from the callback data
132-
executionGasLimit, commitGasLimit := computeExecAndCommitGasLimit(callbackData, remainingGas, maxGas)
138+
executionGasLimit, commitGasLimit, err := computeExecAndCommitGasLimit(callbackData, remainingGas, maxGas)
139+
if err != nil {
140+
return CallbackData{}, true, err
141+
}
142+
143+
callData, err := getCalldata(callbackData)
144+
if err != nil {
145+
return CallbackData{}, true, err
146+
}
133147

134148
return CallbackData{
135149
CallbackAddress: callbackAddress,
136150
ExecutionGasLimit: executionGasLimit,
137151
SenderAddress: packetSender,
138152
CommitGasLimit: commitGasLimit,
139153
ApplicationVersion: version,
154+
Calldata: callData,
140155
}, true, nil
141156
}
142157

143-
func computeExecAndCommitGasLimit(callbackData map[string]any, remainingGas, maxGas uint64) (uint64, uint64) {
158+
func computeExecAndCommitGasLimit(callbackData map[string]any, remainingGas, maxGas uint64) (uint64, uint64, error) {
144159
// get the gas limit from the callback data
145-
commitGasLimit := getUserDefinedGasLimit(callbackData)
160+
commitGasLimit, err := getUserDefinedGasLimit(callbackData)
161+
if err != nil {
162+
return 0, 0, err
163+
}
146164

147165
// ensure user defined gas limit does not exceed the max gas limit
148166
if commitGasLimit == 0 || commitGasLimit > maxGas {
@@ -153,7 +171,7 @@ func computeExecAndCommitGasLimit(callbackData map[string]any, remainingGas, max
153171
// in this case, the callback execution may be retried upon failure
154172
executionGasLimit := min(remainingGas, commitGasLimit)
155173

156-
return executionGasLimit, commitGasLimit
174+
return executionGasLimit, commitGasLimit, nil
157175
}
158176

159177
// getUserDefinedGasLimit returns the custom gas limit provided for callbacks if it is
@@ -164,19 +182,26 @@ func computeExecAndCommitGasLimit(callbackData map[string]any, remainingGas, max
164182
// { "{callbackKey}": { ... , "gas_limit": {stringForCallback} }
165183
//
166184
// Note: the user defined gas limit must be set as a string and not a json number.
167-
func getUserDefinedGasLimit(callbackData map[string]any) uint64 {
185+
func getUserDefinedGasLimit(callbackData map[string]any) (uint64, error) {
168186
// the gas limit must be specified as a string and not a json number
169-
gasLimit, ok := callbackData[UserDefinedGasLimitKey].(string)
187+
gasLimit, ok := callbackData[UserDefinedGasLimitKey]
170188
if !ok {
171-
return 0
189+
return 0, nil
190+
}
191+
gasLimitStr, ok := gasLimit.(string)
192+
if !ok {
193+
return 0, errorsmod.Wrapf(ErrInvalidCallbackData, "gas limit [%v] must be a string", gasLimit)
194+
}
195+
if gasLimitStr == "" {
196+
return 0, nil
172197
}
173198

174-
userGas, err := strconv.ParseUint(gasLimit, 10, 64)
199+
userGas, err := strconv.ParseUint(gasLimitStr, 10, 64)
175200
if err != nil {
176-
return 0
201+
return 0, errorsmod.Wrapf(ErrInvalidCallbackData, "gas limit must be a valid uint64: %s", err)
177202
}
178203

179-
return userGas
204+
return userGas, nil
180205
}
181206

182207
// getCallbackAddress returns the callback address if it is specified in the callback data.
@@ -188,13 +213,34 @@ func getUserDefinedGasLimit(callbackData map[string]any) uint64 {
188213
//
189214
// ADR-8 middleware should callback on the returned address if it is a PacketActor
190215
// (i.e. smart contract that accepts IBC callbacks).
191-
func getCallbackAddress(callbackData map[string]any) string {
216+
func getCallbackAddress(callbackData map[string]any) (string, error) {
192217
callbackAddress, ok := callbackData[CallbackAddressKey].(string)
193218
if !ok {
194-
return ""
219+
return "", errorsmod.Wrapf(ErrInvalidCallbackData, "callback address must be a string")
195220
}
196221

197-
return callbackAddress
222+
return callbackAddress, nil
223+
}
224+
225+
// getCalldata returns the calldata if it is specified in the callback data.
226+
func getCalldata(callbackData map[string]any) ([]byte, error) {
227+
calldataAny, ok := callbackData[CalldataKey]
228+
if !ok {
229+
return nil, nil
230+
}
231+
calldataStr, ok := calldataAny.(string)
232+
if !ok {
233+
return nil, errorsmod.Wrapf(ErrInvalidCallbackData, "calldata must be a string")
234+
}
235+
if calldataStr == "" {
236+
return nil, nil
237+
}
238+
239+
calldata, err := hex.DecodeString(calldataStr)
240+
if err != nil {
241+
return nil, errorsmod.Wrapf(ErrInvalidCallbackData, "calldata must be a valid hex string: %s", err)
242+
}
243+
return calldata, nil
198244
}
199245

200246
// AllowRetry returns true if the callback execution gas limit is less than the commit gas limit.

0 commit comments

Comments
 (0)