feat(cketh): sweeper funding observability and the prepaid-gas gate - #11094
feat(cketh): sweeper funding observability and the prepaid-gas gate#11094mbjorkqvist wants to merge 8 commits into
Conversation
Makes the backing invariant visible to an operator and gives S2 (DEFI-2926) the check it must perform before submitting a sweep. `check_prepaid_sweep_gas` answers "may sweeping spend this much gas?" and fails *closed* in three distinct ways — never observed, stale observation, or insufficient balance. Withholding a sweep costs only delay; allowing one against ETH no burn has covered under-backs ckETH and is not recoverable. So a large but stale balance authorises nothing, and even a zero-wei request is refused when nothing has ever been read. To make that gate consultable without an outcall, the funding task caches the balance it reads, and does so even when no funding is due — that is what keeps the observation fresh enough to be trusted. The cache is deliberately not event-sourced: it is an observation of the chain, and a stale one should be refused rather than replayed. Metrics export burned and spent as counters, so the invariant can be alerted on directly (burned must never fall below spent), plus the surplus and the prepaid balance. The one to alert on is `sweeper_gas_balance_age_seconds`, which reports +Inf when never read: a growing age means the funding task is failing and sweeping is about to stall, before any sweep is actually refused. The dashboard gains a sweeper-funding section. It renders the observation as a timestamp rather than an age because `DashboardTemplate::from_state` must stay callable outside a canister — `ic_cdk::api::time()` traps in unit tests — and the age belongs in metrics, which is where alerting wants it. An unread balance renders "never observed" rather than 0: "no gas" and "never looked" are different operationally. Note the gate has no production caller yet; S2 is what will consult it, exactly as `burn_from_own_subaccount` waited for the funding task. The tests exercise it directly in the meantime, including a clock-skew case where a future-dated observation must not wrap into an enormous age. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds observability and a fail-closed prepaid-gas gate for ckETH sweeper funding.
Changes:
- Caches sweeper balance observations and validates gas availability and freshness.
- Exposes funding accounting, balances, and age metrics.
- Adds dashboard reporting and tests for funding state and gate behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
templates/dashboard.html |
Adds sweeper-funding dashboard section. |
src/sweeper.rs |
Caches observed sweeper balances. |
src/state/tests.rs |
Updates state fixture. |
src/state/sweeper_funding/tests.rs |
Tests prepaid-gas gate behavior. |
src/state/sweeper_funding.rs |
Defines observations and fail-closed gate. |
src/state.rs |
Stores the volatile balance observation. |
src/main.rs |
Exports six sweeper-funding metrics. |
src/lifecycle/init.rs |
Initializes the observation cache. |
src/dashboard/tests.rs |
Tests sweeper dashboard output. |
src/dashboard.rs |
Builds sweeper dashboard data. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…into mathias/DEFI-2933-observability
…into mathias/DEFI-2933-observability
…into mathias/DEFI-2933-observability
| if observed.balance < required { | ||
| return Err(PrepaidGasUnavailable::Insufficient { | ||
| available: observed.balance, | ||
| required, | ||
| }); | ||
| } | ||
| Ok(observed.balance) |
There was a problem hiding this comment.
The observation about the function's semantics is correct — it is a per-request predicate against a snapshot, and two 6-wei sweeps would both pass against a 10-wei observation. I have made that explicit in the doc comment (cd36d1a), since the previous wording invited exactly this reading.
I don't think the conclusion follows, though, on two counts.
The backing invariant does not rest on this check. record_finalized_funding adds the whole transferred amount to cumulative_transferred as soon as the funding transaction finalizes, and cumulative_spent() = transferred + fees. Every wei at the sweeper address is therefore already covered by a burn that preceded the ETH moving, so drawing that balance down — even over-eagerly — cannot make cumulative spend outrun cumulative burn. What over-authorising costs is a sweep the sweeper cannot pay for: wasted signatures, nonces and outcalls, not under-backed ckETH.
There is no caller yet. Nothing in production calls check_prepaid_sweep_gas; this PR adds the observation cache and the predicate, while sweeping itself is [S2] (DEFI-2926), still in the backlog. The function takes Option<ObservedSweeperBalance> by value, so it cannot reserve anything by construction — reserving across several sweeps is the caller's job.
So rather than restructure a function whose caller does not exist, I have recorded the requirement on DEFI-2926: the sweeping task must subtract what it has already committed since the last observation, either by tracking outstanding reservations in state and including them in required or by reconciling each against its receipt. Flagging it here was useful — daily observations mean many sweeps can fall between two of them, so this would have been easy to get wrong.
…into mathias/DEFI-2933-observability
The wording invited reading it as the gate that keeps spending covered by burns, which it is not, and which a caller could rely on wrongly. It is a precondition on one snapshot for one sweep: it neither reserves nor deducts, so two 6-wei sweeps both pass against a 10-wei observation. The backing invariant does not rest on it. Funding counts the whole transfer as spent once it finalizes, so every wei at the sweeper address is already covered by a burn that preceded it and drawing it down cannot make spend outrun burn. Over-authorising costs a sweep the sweeper cannot pay for, not a violation. Reserving across several sweeps belongs to the sweeping task, which is the caller this function does not yet have.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (1)
rs/ethereum/cketh/minter/src/sweeper.rs:62
- The cache-write behavior is not asserted by the new tests: the gate and dashboard tests inject
ObservedSweeperBalancedirectly, whiletests/sweeper_funding.rsonly checks that funding lands. Removing this block—or accidentally placing it after theNotDuereturn—would therefore leave tests green but keep the gate atNeverObservedon the common no-funding path. Add task-level or integration coverage showing that every successful balance read refreshes the value and timestamp even when funding is not due, and that a failed read does not fabricate an observation.
// Cached before deciding anything: sweeping consults this far more often than it changes, and
// recording it even when no funding is due is what keeps the observation fresh.
let observed_at_nanos = ic_cdk::api::time();
mutate_state(|s| {
s.last_observed_sweeper_balance = Some(ObservedSweeperBalance {
balance: sweeper_balance,
observed_at_nanos,
});
});
The observation cache had no integration coverage in this PR: the gate and dashboard tests inject an observation directly, so removing the write, or moving it after the not-due return, would have left them green while the gate stayed at NeverObserved. Asserts both directions through the metrics. The install-time read reports a balance above the low-water mark, so the successful case is also the not-due path: the observation has to be refreshed even though nothing is funded. A read the minter cannot decode must leave the gauge at NaN and the age at +Inf, since a fabricated observation would authorise sweeping against gas nobody has seen. The first attempt was vacuous — it passed with the read succeeding too. The write happens in the balance outcall's callback, which needs rounds to run, so both cases read NaN right after construction and the assertion was measuring "queried too early". Both setups are now ticked equally, which also keeps the negative case as generous as the positive one, and the test fails if the failing read is made to succeed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
rs/ethereum/cketh/minter/src/state.rs:126
- The safety-critical upgrade behavior is not covered: existing tests exercise initial
Noneand successful/failed reads, but none starts withSome(...), performs replay/upgrade, and verifies the observation is absent afterward. Please add that test so this volatile cache cannot later become event-sourced or upgrade-persistent unnoticed; assert the post-upgrade metrics/gate remains fail-closed until a new balance callback completes.
/// The sweeper address' ETH balance as last read on chain, i.e. the prepaid sweep gas.
/// Volatile cache refreshed by the funding task, deliberately not event-sourced.
pub last_observed_sweeper_balance: Option<ObservedSweeperBalance>,
rs/ethereum/cketh/minter/src/main.rs:1135
- This gauge is not always the credit available to offset the next funding. Once a funding burn is accepted,
burned_not_yet_spent()includes that new burn, whilein_flight_funding().amountis explicitly earmarked andplan_fundingrefuses to reuse it. During that interval the metric can overstate usable credit by the entire funding amount. Either subtract the in-flight earmark when exporting the “outstanding credit” metric, or describe this as gross unspent burn and expose the offsettable credit separately.
w.encode_gauge(
"cketh_minter_sweeper_funding_burned_not_yet_spent",
s.sweeper_funding.burned_not_yet_spent().as_f64(),
"ckETH burned for sweeping but not yet spent, i.e. the credit that offsets \
the next funding.",
Nothing asserted the observation is dropped across an upgrade, so event-sourcing or otherwise persisting this cache would have gone unnoticed. It authorises spending by being fresh, so one taken before an upgrade must not authorise a sweep after it. The gate staying closed afterwards comes for free: this harness leaves the post-upgrade balance read unanswered, so the assertion holds until a read actually completes rather than merely being made too early. Answering that read makes the test fail, which is what makes it worth having. Also corrects what the unspent-burn gauge claims. A burn is recorded when its funding is accepted, so while that funding is in flight the value includes an amount `plan_funding` refuses to reuse — it is how far burn runs ahead of spend, not the credit the next funding may offset against. The number is left alone, being the margin R14 is monitored on; the earmark is already exposed separately.
Part of DEFI-2933 (sweeper fee funding), sixth of a seven-PR stack. Targets #11086.
Why
Two things are missing once funding works: an operator cannot see whether ckETH is still fully backed, and sweeping has no way to ask whether it may spend gas at all. This adds both.
The gate
check_prepaid_sweep_gasanswers "may sweeping spend this much?" and fails closed in three distinct ways — never observed, stale observation, insufficient balance. Withholding a sweep costs only delay; allowing one against ETH no burn has covered under-backs ckETH and is not recoverable. So a large but stale balance authorises nothing, and even a zero-wei request is refused when nothing has ever been read.The cached observation is deliberately not event-sourced: it is a reading of the chain, and after an upgrade it is simply absent, which the gate treats as "refuse to spend". An observation from before an upgrade should not authorise spending after it.
The staleness limit is two days against a 24-hour funding interval, which leaves exactly one tick of slack: a single missed refresh is tolerated, two consecutive misses stop sweeping.
To make the gate consultable without an outcall, the funding task caches the balance it reads — and does so even when no funding is due, since that is what keeps the observation fresh enough to be trusted.
Note the gate has no production caller yet; S2 (DEFI-2926) is what will consult it, exactly as the fee-subaccount burn waited for the funding task. Tests exercise it directly in the meantime.
Observability
Burned and spent are exported as counters, so the invariant can be alerted on directly — burned must never fall below spent. Alongside them: the outstanding credit, the prepaid balance, and two ages.
The age to alert on is
sweeper_gas_balance_age_seconds, which reports+Infwhen never read: a growing age means the funding task is failing and sweeping is about to stall, and it fires before any sweep is actually refused.sweeper_in_flight_funding_age_secondsneeds its own alert because the balance-age gauge cannot reveal a wedged funding — the task refreshes the observation before consulting the guard, so that age resets every tick regardless.Proposed alert conditions for all six metrics are recorded on DEFI-2965, which collects observability for this pipeline rather than alerting on each piece separately.
The dashboard gains a sweeper-funding section. It renders the observation as a timestamp rather than an age because
DashboardTemplate::from_statemust stay callable outside a canister —ic_cdk::api::time()traps in unit tests — and the age belongs in metrics anyway. An unread balance renders "never observed" rather than 0: "no gas" and "never looked" are different operationally.Stack
Merge in order; each PR targets the one above it.