refactor: single-round timers with rescheduling by lpahlavi · Pull Request #97 · dfinity/cksol

lpahlavi · 2026-04-01T08:06:48Z

Summary

Refactors all timer-based processing functions to a single-round, immediately-rescheduling pattern:

Each timer invocation processes one round of up to MAX_CONCURRENT_RPC_CALLS batches, then immediately reschedules itself with Duration::ZERO if work remains
Rescheduling uses a scopeguard that fires on drop (including panics); defused when all work fits in the current round
The more_to_process flag is computed from a snapshot of the queue before processing begins, so items added concurrently during execution don't trigger a spurious reschedule

Stack

Based on: refactor: split monitor timer into finalize and resubmit #120 (split monitor timer)

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR refactors timer-based processing functions to execute one round of concurrent work per invocation instead of looping through multiple rounds. After each round, timers reschedule themselves immediately with Duration::ZERO if there's more work, or with the normal delay if idle. This architectural change allows the IC scheduler to interleave other canister work between timer invocations, avoiding long-running timer callbacks.

Changes:

Timer-based functions (consolidate_deposits, process_pending_withdrawals, monitor_submitted_transactions) now execute a single round of work per invocation
Each function reschedules itself based on whether there's remaining work
Replaced set_timer_interval with set_timer in main timer setup
Removed MAX_WITHDRAWAL_ROUNDS constant and associated multi-round tests

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
minter/src/consolidate/mod.rs	Refactored to process one consolidation round per invocation with automatic rescheduling
minter/src/monitor/mod.rs	Refactored to process one round of transaction monitoring per invocation with extracted inner function
minter/src/withdraw_sol/mod.rs	Refactored to process one withdrawal round per invocation; removed multi-round tests
minter/src/withdraw_sol/tests.rs	Updated test calls to pass cloned runtime; removed multi-round test cases
minter/src/main.rs	Changed timer setup to use `set_timer` instead of `set_timer_interval`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Process one round of concurrent RPC calls per timer invocation, then reschedule immediately (via set_timer(0)) if work remains, instead of batching all work in a single async task. This avoids holding the timer guard across multiple async rounds and simplifies cancellation. Also simplify tests to use event helpers (events::expire_transaction, events::accept_withdrawal) instead of mocking finalize_transactions as setup, and remove the test-only MAX_SIGNATURES_PER_STATUS_CHECK override (using the real value of 256 with a usize-indexed signature helper instead). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Replace set_timer(delay, future) with set_timer(delay, fn) in CanisterRuntime trait (automatically clones self) - Remove RESCHEDULE_DELAY constant; reschedule timers immediately (Duration::ZERO) - Change signature/deposit_id/account helpers to accept usize with consistent le_bytes encoding - Merge per-timer capacity + reschedule tests into single successive-rounds tests - Drop runtime_ref clone pattern; pass runtime.clone() directly to timer functions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Use the same num_remaining pattern as resubmit_transactions instead of scopeguard + checked_count tracking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Also fix signature() doc comment and merge MAX_CONCURRENT_RPC_CALLS import into the crate:: block in withdraw/mod.rs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Add scopeguard + more_to_process pattern to process_pending_withdrawals - Add comments above ScopeGuard::into_inner defuse calls in all timers - Use deposit_id(i) helper in submit_consolidation_transaction_with_signature - Merge use crate:: statements in withdraw/mod.rs - Merge use cksol_minter:: statements in main.rs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…th_signature Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…on_with_signature Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…_with_signature Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…_until_all_transactions_resubmitted`

maciejdfinity · 2026-04-14T15:18:17Z

+    futures::future::join_all(batches.into_iter().map(async |funds| {
+        match submit_consolidation_transaction(&runtime, funds, slot, recent_blockhash).await {
+            Ok(sig) => log!(Priority::Info, "Submitted consolidation transaction {sig}"),
+            Err(e) => log!(Priority::Info, "Deposit consolidation failed: {e}"),


This code was not changed by this PR, but maybe we should log errors with error priority?

maciejdfinity · 2026-04-14T16:23:18Z

+// Transactions that are in-flight (Processed/Confirmed) or whose status
+// check failed are implicitly excluded from the below sets.


Suggested change

// Transactions that are in-flight (Processed/Confirmed) or whose status

// check failed are implicitly excluded from the below sets.

/// Transactions that are in-flight (Processed/Confirmed) or whose status

/// check failed are implicitly excluded from the below sets.

Copilot AI review requested due to automatic review settings April 1, 2026 08:06

Copilot started reviewing on behalf of lpahlavi April 1, 2026 08:07 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 1, 2026 09:40

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Comment thread minter/src/main.rs

Copilot AI review requested due to automatic review settings April 2, 2026 20:19

Copilot started reviewing on behalf of lpahlavi April 2, 2026 20:20 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

lpahlavi force-pushed the lpahlavi/single-round-timers branch from 2ca221e to 899184c Compare April 7, 2026 08:27

Copilot AI review requested due to automatic review settings April 7, 2026 08:58

lpahlavi force-pushed the lpahlavi/single-round-timers branch from 899184c to 2b725d4 Compare April 7, 2026 08:58

Copilot started reviewing on behalf of lpahlavi April 7, 2026 08:59 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Comment thread minter/src/withdraw/mod.rs Outdated

Comment thread minter/src/consolidate/mod.rs

Comment thread minter/src/monitor/mod.rs Outdated

Comment thread minter/src/consolidate/mod.rs

lpahlavi force-pushed the lpahlavi/single-round-timers branch from 2b725d4 to 5b63fde Compare April 9, 2026 13:13

Copilot AI review requested due to automatic review settings April 9, 2026 13:22

lpahlavi force-pushed the lpahlavi/single-round-timers branch from 5b63fde to 909192f Compare April 9, 2026 13:22

Copilot started reviewing on behalf of lpahlavi April 9, 2026 13:23 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Comment thread minter/src/withdraw/mod.rs

Comment thread minter/src/consolidate/mod.rs

lpahlavi force-pushed the lpahlavi/single-round-timers branch from 909192f to b6a0ee5 Compare April 9, 2026 13:26

Copilot AI review requested due to automatic review settings April 9, 2026 13:30

lpahlavi force-pushed the lpahlavi/single-round-timers branch from b6a0ee5 to 2dafe84 Compare April 9, 2026 13:30

Copilot started reviewing on behalf of lpahlavi April 9, 2026 13:30 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Comment thread minter/src/withdraw/mod.rs

Comment thread minter/src/consolidate/mod.rs

Copilot AI review requested due to automatic review settings April 9, 2026 13:56

Copilot started reviewing on behalf of lpahlavi April 9, 2026 13:56 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 9, 2026 14:11

lpahlavi force-pushed the lpahlavi/single-round-timers branch from 515cfcc to cd0cdae Compare April 9, 2026 14:11

Copilot started reviewing on behalf of lpahlavi April 9, 2026 14:12 View session

lpahlavi force-pushed the lpahlavi/single-round-timers branch from cd0cdae to df69dca Compare April 9, 2026 14:16

lpahlavi force-pushed the lpahlavi/single-round-timers branch 2 times, most recently from c05d5a8 to 320ef52 Compare April 14, 2026 10:55

Base automatically changed from lpahlavi/split-monitor-timer to main April 14, 2026 11:54

lpahlavi force-pushed the lpahlavi/single-round-timers branch from 320ef52 to f0c874a Compare April 14, 2026 11:56

lpahlavi force-pushed the lpahlavi/single-round-timers branch from f0c874a to 8613a52 Compare April 14, 2026 12:30

lpahlavi changed the title ~~refactor: single-round timers with immediate rescheduling~~ refactor: single-round timers with rescheduling Apr 14, 2026

lpahlavi and others added 9 commits April 14, 2026 14:50

refactor: rename runtime2 to runtime in merged timer tests

3bbf04a

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test: add queue size assertions to merged timer reschedule tests

eb397ca

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor: simplify finalize_transactions rescheduling

e7d8ccb

Use the same num_remaining pattern as resubmit_transactions instead of scopeguard + checked_count tracking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor: use scopeguard for both monitor timer rescheduling

ab4a0b7

Also fix signature() doc comment and merge MAX_CONCURRENT_RPC_CALLS import into the crate:: block in withdraw/mod.rs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor: extract sig variable in submit_consolidation_transaction_wi…

27173fb

…th_signature Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor: use signature(i) directly in submit_consolidation_transacti…

e4c31c2

…on_with_signature Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor: rename sig to signature in submit_consolidation_transaction…

c40f18c

…_with_signature Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

lpahlavi requested review from THLO and maciejdfinity April 14, 2026 13:25

lpahlavi marked this pull request as ready for review April 14, 2026 13:29

lpahlavi requested a review from a team as a code owner April 14, 2026 13:29

lpahlavi mentioned this pull request Apr 14, 2026

fix: handle resubmission replay for canisters without ExpiredTransaction #123

Closed

THLO reviewed Apr 14, 2026

View reviewed changes

Comment thread minter/src/monitor/tests.rs Outdated

Use MAX_CONCURRENT_RPC_CALLS + 1 transactions in `should_reschedule…

4860f81

…_until_all_transactions_resubmitted`

lpahlavi requested a review from THLO April 14, 2026 15:10

THLO approved these changes Apr 14, 2026

View reviewed changes

maciejdfinity reviewed Apr 14, 2026

View reviewed changes

maciejdfinity approved these changes Apr 14, 2026

View reviewed changes

lpahlavi merged commit 4214b01 into main Apr 14, 2026
10 checks passed

lpahlavi deleted the lpahlavi/single-round-timers branch April 14, 2026 17:00

		// Transactions that are in-flight (Processed/Confirmed) or whose status
		// check failed are implicitly excluded from the below sets.

Conversation

lpahlavi commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Stack

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

maciejdfinity Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

maciejdfinity Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lpahlavi commented Apr 1, 2026 •

edited

Loading