feat(nns): tolerate XRC failures when updating maturity modulation#10210
feat(nns): tolerate XRC failures when updating maturity modulation#10210jasonz-dfinity wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the NNS governance timer task that backfills ICP/XDR rates (via XRC) and computes maturity modulation so that transient/multi-day XRC gaps no longer stall maturity modulation updates.
Changes:
- Switch
compute_average_icp_xdr_rateto Last Observation Carried Forward (LOCF) so averages remain usable across missing days. - Add an in-memory
attempted_through_daycursor so repeated failures advance to later missing days instead of retrying the same day every tick. - Replace size-based buffer eviction with timestamp-based eviction (
evict_stale_rates) that retains one pre-window “seed” entry for LOCF.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| rs/nns/governance/src/timer_tasks/update_icp_xdr_rate_related_data.rs | Implements LOCF averaging, cursor-based retry progression, and timestamp-anchored eviction to keep maturity modulation progressing despite XRC gaps. |
| rs/nns/governance/src/timer_tasks/update_icp_xdr_rate_related_data_tests.rs | Adds/updates unit + async tests for LOCF behavior, eviction rules, and cursor advancement/reset across ticks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
| fn test_compute_average_icp_xdr_rate_locf_fills_middle_gap() { | ||
| // 7-day window ending at day 100. Days 94-99 present (50_000), day 100 missing. |
| let current_day = 1_000_u64; | ||
| let oldest_kept_day = current_day - (MAX_RATES_BUFFER_SIZE as u64 - 1); // 636 | ||
| let mut rates: Vec<SampledPrice> = (0..=635) | ||
| .map(|d| SampledPrice { | ||
| timestamp_seconds: d * ONE_DAY_SECONDS, | ||
| xdr_permyriad_per_icp: 50_000, | ||
| }) | ||
| .collect(); | ||
| // Plus one entry at the boundary inside the window. |
Why
XRC sometimes fails to return a rate for the current day, possibly for
several consecutive days — most recently between 2026-05-07 and 2026-05-10,
when no rate was returned for four consecutive days even though XRC had
succeeded on every one of the preceding 365 days. The previous
implementation got stuck retrying the same failing day every 60s and
refused to update maturity modulation until the full 365-day window was
filled, so a single persistent gap stalled the whole feature.
NNS1-4319: https://dfinity.atlassian.net/browse/NNS1-4319
What
compute_average_icp_xdr_ratenow uses Last Observation Carried Forward:for each day in the window, fall back to the most recent prior day's rate
if that day is missing. The fallback is compute-only — nothing is written
back to the price history.
attempted_through_daycursor on the timer task. Failedfetches advance the cursor so the next tick moves on instead of looping
on the failing day; missing days are retried on subsequent midnight runs
until they fall out of the lookback window.
update_rates_bufferno longer evicts by size. A newevict_stale_ratesruns at the top of each tick: it drops entries before the lookback
window, but keeps the single most-recent pre-window entry as an LOCF
seed so leading gaps in the window can still be filled.
Testing
compute_average_icp_xdr_ratecovering middle gaps,trailing gaps, multi-day walk-back, no-prior-rate skip, and full windows.
evict_stale_ratescovering seed retention,sparse-within-window preservation, and lone-seed cases.
execute()async tests chaining ticks to verify cursor advancement onfailure (failed day stays absent in the buffer — fallback is not
persisted) and cursor reset after a completed round.