Skip to content

fix(control-panel): make cycle monitoring independent of monitored canisters - #651

Draft
MRmarioruci wants to merge 2 commits into
mainfrom
fix/bound-self-reported-station-cycles
Draft

fix(control-panel): make cycle monitoring independent of monitored canisters#651
MRmarioruci wants to merge 2 commits into
mainfrom
fix/bound-self-reported-station-cycles

Conversation

@MRmarioruci

@MRmarioruci MRmarioruci commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Change

The control panel is not a controller of the stations it funds — the station resets its own controllers to [upgrader, NNS root] during init (services/system.rs:359-363) — so it reads each balance by calling that station's http_request and parsing a Prometheus metric. Both the timing and the value of that response therefore come from the monitored canister.

The monitoring round is coupled to both. canfund awaits every registered canister inside one join_all while holding the process lock (canfund-0.8.3/src/manager/mod.rs:208-217, :411), so the round's completion depends on every canister replying; and the reported value feeds a consumption-rate calculation whose multiplication is not saturating (manager/record.rs:49-60) while the workspace builds with overflow-checks = true.

Two layers around the fetcher remove both dependencies:

  • BoundedCyclesFetcher rejects readings above u128::MAX / 1_000_000_000, the point past which that multiplication can leave u128. The bound is derived, not chosen: at or below it the product fits for any pair of accepted readings.
  • CachedCyclesFetcher answers from a local cache and never awaits a canister inside the round; the call happens in a spawned task that updates the cache when it completes. The round takes and releases the lock regardless of how any canister behaves.

Supporting details:

  • A refresh is not started while one is already in flight for the same canister, so an unresponsive canister costs one open call context rather than one per round.
  • Readings older than three days stop being used. The round is daily, so this tolerates one missed refresh.
  • Rejections and stale entries surface as MetricsHttpRequestFailed, which canfund already records per canister via set_funding_failure without aborting the round.
  • Adds async-trait to the control panel (already a workspace dependency).

Tests

Eight cases in services::canister::bounded_cycles_fetcher_tests, covering the bound (plausible value, exactly at the bound, u128::MAX rejected, and a property test so the constant cannot drift out of correctness) and the cache (no reading yet, fresh reading served, expiry boundary, no second refresh while one is in flight).

cargo test -p control-panel --lib passes (215). Clippy and cargo fmt clean.

Trade-offs worth reviewing

Cold start. With no cached reading a canister is skipped for one round and funded from the next. Stations are funded at deploy time, so this is a day's delay on an already-funded canister.

Funding acts on data up to a day old. It already did, since the previous round's reading informed the same decision, but it is now explicit rather than incidental.

An unresponsive canister holds one open call context until it replies or is stopped, and its own monitoring slot degrades rather than the fleet's.

Note on upstream

canfund 0.8.5 changes neither behaviour: set_cycles is byte-identical and its fetcher uses Call::unbounded_wait. It also requires ic-cdk ^0.19 against this repo's 0.17.1, so adopting it is a two-major-version toolchain migration rather than a dependency bump. Handling this here avoids that.

MRmarioruci and others added 2 commits August 10, 2026 16:04
Station balances are read from each station's own /metrics, so the value
is chosen by whoever controls that station's code. canfund derives a
consumption rate from consecutive readings as
`(previous - current) * 1_000_000_000 / elapsed`, and that multiplication
is not saturating. With overflow-checks enabled a crafted reading traps
inside the shared monitoring round, and because the poisoned reading is
committed by an earlier sweep while the trap rolls back only the later
message, every subsequent sweep traps at the same point and no station is
topped up again.

Wrap the metrics fetcher so readings above the point where that
arithmetic can overflow are reported as a fetch failure instead. canfund
already records fetch failures per canister without aborting the round,
so one misbehaving station no longer stops the rest being funded.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A station that accepts the metrics call and never replies stalled the
whole sweep. canfund awaits every registered canister inside one
join_all while holding the process lock, so one unanswered call meant
the round never returned and the lock was never released. Every later
round then found the lock held and no station was ever topped up again.

The lock cannot be released early and the call cannot be given a
deadline: the repo is on ic-cdk 0.17, which has no bounded-wait call
API. So stop reading balances inside the round at all. The fetcher now
answers immediately from a local cache and refreshes it in a spawned
task, which means the round no longer waits on any station. A canister
that never replies only starves its own cache entry and is skipped,
while every other canister is funded as normal.

A refresh is not started while one is already in flight for the same
canister, so an unresponsive station costs one open call context rather
than one per round. Readings older than three days stop being used, so
a station that goes quiet is skipped rather than funded indefinitely on
a stale figure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@MRmarioruci MRmarioruci changed the title fix(control-panel): bound self-reported station cycles balances fix(control-panel): stop one station stalling or trapping the shared cycle monitor Aug 11, 2026
@MRmarioruci MRmarioruci changed the title fix(control-panel): stop one station stalling or trapping the shared cycle monitor fix(control-panel): make cycle monitoring independent of monitored canisters Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant