Skip to content

Commit 8d9356a

Browse files
committed
address process PR comment
1 parent 819c6e9 commit 8d9356a

File tree

5 files changed

+87
-38
lines changed

5 files changed

+87
-38
lines changed

contracts/FlowCallbackScheduler.cdc

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,14 @@ access(all) contract FlowCallbackScheduler {
233233
self.description = handlerRef.description
234234
}
235235

236+
access(all) view fun getHandlerType(): Type? {
237+
return self.handler.getType()
238+
}
239+
240+
access(all) view fun getHandlerAddress(): Address {
241+
return self.handler.address
242+
}
243+
236244
/// setStatus updates the status of the callback.
237245
/// It panics if the callback status is already finalized.
238246
access(contract) fun setStatus(newStatus: Status) {
@@ -1224,19 +1232,21 @@ access(all) contract FlowCallbackScheduler {
12241232
}
12251233

12261234
for callback in pendingCallbacks {
1227-
let callbackHandler = callback.handler.borrow()
1228-
?? panic("Invalid callback handler: Could not borrow a reference to the callback handler")
1229-
1230-
emit PendingExecution(
1231-
id: callback.id,
1232-
priority: callback.priority.rawValue,
1233-
executionEffort: callback.executionEffort,
1234-
fees: callback.fees,
1235-
callbackOwner: callback.handler.address,
1236-
callbackHandlerTypeIdentifier: callbackHandler.getType().identifier,
1237-
callbackName: callbackHandler.name,
1238-
callbackDescription: callbackHandler.description
1239-
)
1235+
// Only emit the pending execution event if the callback handler capability is borrowable
1236+
// This is to prevent a situation where the callback handler is not available
1237+
// In that case, the callback is no longer valid because it cannot be executed
1238+
if let callbackHandler = callback.handler.borrow() {
1239+
emit PendingExecution(
1240+
id: callback.id,
1241+
priority: callback.priority.rawValue,
1242+
executionEffort: callback.executionEffort,
1243+
fees: callback.fees,
1244+
callbackOwner: callback.handler.address,
1245+
callbackHandlerTypeIdentifier: callbackHandler.getType().identifier,
1246+
callbackName: callbackHandler.name,
1247+
callbackDescription: callbackHandler.description
1248+
)
1249+
}
12401250

12411251
// after pending execution event is emitted we set the callback as executed because we
12421252
// must rely on execution node to actually execute it. Execution of the callback is

lib/go/contracts/internal/assets/assets.go

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/callbackScheduler_events_test.cdc

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ access(all) fun testCallbackScheduleAnotherCallback() {
332332
}
333333

334334
let currentTime = getTimestamp()
335-
var timeInFuture = currentTime + futureDelta*10.0
335+
var timeInFuture = currentTime + futureDelta*5.0
336336

337337
// Schedule a medium callback
338338
scheduleCallback(
@@ -353,7 +353,7 @@ access(all) fun testCallbackScheduleAnotherCallback() {
353353
Test.assertEqual(callbackData!.executionEffort, mediumEffort)
354354
Test.assertEqual(callbackData!.status.rawValue, statusScheduled)
355355

356-
Test.moveTime(by: Fix64(futureDelta*11.0))
356+
Test.moveTime(by: Fix64(futureDelta*6.0))
357357

358358
processCallbacks()
359359

@@ -367,4 +367,52 @@ access(all) fun testCallbackScheduleAnotherCallback() {
367367
var status = getStatus(id: 2)
368368
Test.assertEqual(statusScheduled, status!)
369369

370+
}
371+
372+
373+
access(all) fun testCallbackDestroyHandler() {
374+
375+
if startingHeight < getCurrentBlockHeight() {
376+
Test.reset(to: startingHeight)
377+
}
378+
379+
let currentTime = getTimestamp()
380+
var timeInFuture = currentTime + futureDelta*10.0
381+
382+
// Schedule a medium callback
383+
scheduleCallback(
384+
timestamp: timeInFuture,
385+
fee: feeAmount,
386+
effort: mediumEffort,
387+
priority: mediumPriority,
388+
data: testData,
389+
testName: "Destroy Handler: Scheduled",
390+
failWithErr: nil
391+
)
392+
393+
// Destroy the handler for the callback
394+
let executeCallbackCode = Test.readFile("./transactions/destroy_handler.cdc")
395+
let executeTx = Test.Transaction(
396+
code: executeCallbackCode,
397+
authorizers: [serviceAccount.address],
398+
signers: [serviceAccount],
399+
arguments: []
400+
)
401+
var result = Test.executeTransaction(executeTx)
402+
Test.expect(result, Test.beSucceeded())
403+
404+
Test.moveTime(by: Fix64(futureDelta*11.0))
405+
406+
processCallbacks()
407+
408+
// The callback with the handler should not have emitted an event because the handler was destroyed
409+
let pendingExecutionEvents = Test.eventsOfType(Type<FlowCallbackScheduler.PendingExecution>())
410+
Test.assertEqual(pendingExecutionEvents.length, 0)
411+
412+
executeCallback(
413+
id: 1,
414+
testName: "Destroy Handler: Execute",
415+
failWithErr: "Invalid callback handler: Could not borrow a reference to the callback handler"
416+
)
417+
370418
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import "TestFlowCallbackHandler"
2+
import "FlowCallbackScheduler"
3+
4+
transaction {
5+
prepare(account: auth(BorrowValue, LoadValue) &Account) {
6+
let handler <- account.storage.load<@TestFlowCallbackHandler.Handler>(from: TestFlowCallbackHandler.HandlerStoragePath)
7+
?? panic("Could not load TestFlowCallbackHandler")
8+
destroy handler
9+
}
10+
}
11+

tests/transactions/test_pending_queue.cdc

Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

Comments
 (0)