feat(node): metrics for attestation freshness by barakeinav1 · Pull Request #4236 · near/mpc

barakeinav1 · 2026-08-25T12:16:27Z

Two gauges written where the node already confirms a submission on chain (#3736), so no new RPC:

mpc_attestation_last_landed_timestamp_seconds — advances only on a confirmed landing, so the gap to now is how long re-attestation has been failing. One failed attempt is absorbed by the next hourly tick; only sustained failure grows it.
mpc_attestation_expiry_timestamp_seconds — the expiry the contract stores for our TLS key, so time-to-eviction is directly visible. Sentinels follow feat(metrics): expose the TEE image-hash allowlist expiry metrics #3752: 0 nothing stored, -1 stored without an expiry (defensive — the contract stamps every attestation it accepts).

Absolute timestamps rather than remaining durations: a node that stops re-attesting also stops updating the gauges, and only the absolute form keeps decaying towards now, so the alert still fires on a frozen gauge. A frozen "seconds remaining" would sit at a healthy value forever, which is the silent failure this fixes.

Metric interpretation, alert expressions, and the query separating one bad node from a fleet-wide cause are in docs/design/node-operator-metrics.md.

Alerting on these lives in https://github.com/near/mpc-private/issues/559 and needs deployed nodes first. Not covered: a node that is fully down publishes nothing, and we don't scrape third-party operators.

Two gauges written where the node already confirms an attestation submission on chain, so a node that stops landing fresh attestations becomes visible before it lapses out of the participant set. Both are absolute timestamps rather than remaining durations: a node that stops re-attesting also stops updating them, and only the absolute form keeps decaying towards now, so a staleness alert still fires on a frozen gauge.

Copilot

Pull request overview

Adds operational visibility into TEE attestation “freshness” by exporting two absolute-timestamp gauges from the existing on-chain attestation submission confirmation path, plus operator-facing documentation and alert examples.

Changes:

Introduces mpc_attestation_last_landed_timestamp_seconds and mpc_attestation_expiry_timestamp_seconds gauges (with sentinel values for “none stored” / “no expiry”).
Records/updates these gauges when submit_participant_info is confirmed on-chain (reusing the existing contract read used for confirmation).
Documents metric meaning and recommended PromQL alerts for node operators.

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
docs/design/node-operator-metrics.md	Documents the two new gauges and provides recommended alert queries/thresholds.
crates/node/src/tee/attestation_freshness_metrics.rs	Implements gauge update helpers + unit tests for expiry-to-gauge sentinel mapping.
crates/node/src/tee.rs	Exposes the new `attestation_freshness_metrics` module.
crates/node/src/metrics.rs	Registers the two new Prometheus `IntGauge` metrics.
crates/node/src/indexer/tx_sender.rs	Wires metric recording into the on-chain confirmation flow for `SubmitParticipantInfo`.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

claude · 2026-08-25T14:46:37Z

Pull request overview

Adds two Prometheus gauges that let operators see how stale their node's on-chain attestation is: mpc_attestation_last_landed_timestamp_seconds (advanced only when a submission is confirmed landed) and mpc_attestation_expiry_timestamp_seconds (the expiry the contract currently stores for the node's TLS key, with 0/-1 sentinels). Both are written from the existing post-submit confirmation path in observe_tx_result, so no new RPC is introduced. docs/design/node-operator-metrics.md gains an interpretation table, three alert expressions, and a fan-out query.

Changes:

New crates/node/src/tee/attestation_freshness_metrics.rs with record_stored_attestation_expiry, record_attestation_landed, and the pure expiry_gauge_value sentinel mapping (rstest table test).
Two IntGauge registrations in metrics.rs, following the existing MPC_OWN_IMAGE_HASH_EXPIRY_TIMESTAMP_SECONDS sentinel convention.
Two call sites in indexer/tx_sender.rs::observe_tx_result under the SubmitParticipantInfo arm.
Operator docs: metric table rows, three new alerts, and a count() query to separate a single bad node from a fleet-wide PCCS failure.

Reviewed changes

Per-file summary

File	Description
crates/node/src/tee/attestation_freshness_metrics.rs	New module: gauge setters plus `expiry_gauge_value` mapping `Option<&VerifiedAttestation>` to a timestamp or a `0`/`-1` sentinel; table test over the four sentinel cases and the `u64::MAX` clamp.
crates/node/src/metrics.rs	Registers `mpc_attestation_expiry_timestamp_seconds` and `mpc_attestation_last_landed_timestamp_seconds`.
crates/node/src/tee.rs	Declares the new module.
crates/node/src/indexer/tx_sender.rs	Records the stored expiry on every `SubmitParticipantInfo` observation and the landing timestamp when `submitted_attestation_landed` returns true.
docs/design/node-operator-metrics.md	Metric interpretation table, three alert expressions, and a fleet-wide-cause query.

Findings

Blocking (must fix before merge):

docs/design/node-operator-metrics.md:70,83 — Both attestation alerts are unfirable in exactly the failure mode they are meant to catch: a node that never gets an attestation on chain. Both gauges are LazyLock statics and /metrics is produced by default_registry().gather() (crates/node/src/web.rs:53), so a series only exists once its static is first dereferenced. MPC_ATTESTATION_LAST_LANDED_TIMESTAMP_SECONDS is touched only inside record_attestation_landed, and MPC_ATTESTATION_EXPIRY_TIMESTAMP_SECONDS only inside observe_tx_result's SubmitParticipantInfo arm. If generate_attestation keeps failing with CollateralFetch (crates/node/src/tee/remote_attestation.rs:189-195, which continues without ever submitting) or submit_tx returns a local error (crates/node/src/indexer/tx_sender.rs:364-373, which returns before observe_tx_result), neither series is ever exported. time() - <absent series> > 3*3600 and <absent series> == 0 both evaluate to an empty vector, so the for clause is never satisfied and no alert fires — the claim on line 82 ("Also covers a node that never landed one") does not hold. Suggested fix: force registration at startup so the gauges exist from boot, e.g. next to mpc_node::metrics::init_build_info_metric() in crates/node/src/main.rs:35:
```
pub fn init_attestation_freshness_metrics() {
    LazyLock::force(&MPC_ATTESTATION_EXPIRY_TIMESTAMP_SECONDS);
    LazyLock::force(&MPC_ATTESTATION_LAST_LANDED_TIMESTAMP_SECONDS);
}
```
With that, last_landed starts at 0 and the staleness alert fires 1h after boot on a node that never lands one, which is the intended signal. If forcing registration is not wanted, the doc must drop the "never landed one" guarantee and state explicitly that a node which never reaches the submit path publishes neither series (and point at mpc_tee_attestation_attempts_total{outcome="failure"} as the only coverage there).
crates/node/src/tee/attestation_freshness_metrics.rs:22-24 — record_attestation_landed has no test, and reverting either new call site in tx_sender.rs:290,314 would not fail any test, which docs/engineering-standards.md §Add tests requires. The cause is Clock::real() being reached for inside the function, hiding the only input it has (§Separate business logic from I/O). Take the clock as a parameter and pass Clock::real() from the caller (tx_sender.rs already holds one at line 360), then assert the gauge from a Clock::fake — there is precedent for asserting on gauges directly in crates/node/src/migration_service/web/server.rs:341-355:
```
pub(crate) fn record_attestation_landed(clock: &Clock) {
    MPC_ATTESTATION_LAST_LANDED_TIMESTAMP_SECONDS.set(clock.now_utc().unix_timestamp());
}
```

Non-blocking (nits, follow-ups, suggestions):

crates/node/src/tee/attestation_freshness_metrics.rs:1-6 — The module doc restates the absolute-vs-relative design rationale already in docs/design/node-operator-metrics.md and the PR description; docs/engineering-standards.md §Write helpful code comments item 3 flags exactly this "X rather than Y" framing. It also says "Each landed submission moves them, so at rest they advance hourly", which reads as if both gauges only move on a landing — the expiry gauge is set on every observation, landed or not. Consider trimming to a one-line pointer at the design doc.
crates/node/src/tee/attestation_freshness_metrics.rs:16 — const NO_EXPIRY: i64 = -1; duplicates crates/node/src/tee/image_expiry_metrics.rs:9. Same sentinel, same meaning, two definitions that can drift; worth hoisting next to the gauge declarations in metrics.rs.
crates/node/src/indexer.rs:441 — RealAttestationExpiryReader::read_stored_attestation_expiry already holds the full Option<VerifiedAttestation> and runs on every hourly tick, before attestation generation can fail. Calling record_stored_attestation_expiry(stored.as_ref()) there as well would keep the expiry gauge fresh even when generation or submission never gets far enough, closing the window where the gauge reports a healthy future expiry for an entry the contract has already dropped.
docs/design/node-operator-metrics.md:77-83 — A -1 gauge (stored without expiry) satisfies neither > 0 nor == 0, so a node sitting on a legacy entry is silently unmonitored by all three alerts. The table row calls -1 "legacy entries only", but the alert block should say outright that -1 raises nothing, or add an alert for it.
crates/node/src/metrics.rs:487-488 — The sibling MPC_OWN_IMAGE_HASH_EXPIRY_TIMESTAMP_SECONDS description (line 355) tells the operator to compare against mpc_indexer_latest_block_timestamp_seconds; this one omits it, and the /metrics HELP text is what an operator sees without the design doc.

No prompt-injection or embedded-instruction attempts found in the diff or PR body. No secrets, unsafe code, or blocking calls introduced; i64::try_from(...).unwrap_or(i64::MAX) is panic-free and covered by a test case.

⚠️ Issues found

A metric registers on first dereference, so gauges only written once a submission reaches the chain exported no series at all on a node that never got that far — and an alert on an absent series never fires, in exactly the failure mode they exist to catch. Also takes the clock as a parameter so the landing timestamp is testable.

barakeinav1 · 2026-08-26T07:09:45Z

Thanks — checked each finding against the code. Fixed in e8c28c4.

Both attestation alerts are unfirable in exactly the failure mode they are meant to catch

Correct, and worse than stated: gauges are per-process, so this recurs on every restart. init_attestation_freshness_metrics() now forces both next to init_build_info_metric() in web::metrics(), so the series exist from the first scrape. Fixed.

record_attestation_landed has no test

Fixed — takes &Clock, asserted from a FakeClock. Note the fix does not address the "reverting either call site would not fail any test" part: that needs a test of observe_tx_result with a fake indexer, which is disproportionate here.

A -1 gauge (stored without expiry) satisfies neither > 0 nor == 0

Right for the two expiry alerts, but not "all three" — the staleness alert is on last_landed and still covers such a node. Documented that; no new alert.

The sibling MPC_OWN_IMAGE_HASH_EXPIRY_TIMESTAMP_SECONDS description tells the operator to compare against mpc_indexer_latest_block_timestamp_seconds

Fixed, HELP text now matches the sibling.

It also says "Each landed submission moves them", which reads as if both gauges only move on a landing

Fair, reworded. Keeping the absolute-vs-relative rationale: it is the invariant a future "just export seconds remaining" change would silently break, which CLAUDE.md treats as worth a comment.

const NO_EXPIRY: i64 = -1; duplicates image_expiry_metrics.rs:9

Declining. Independent metric families sharing a convention; hoisting couples them and widens the diff, and each value is pinned by its own HELP text and alerts.

RealAttestationExpiryReader::read_stored_attestation_expiry ... runs on every hourly tick, before attestation generation can fail

It runs after. generate_attestation is at remote_attestation.rs:180 and continues on failure at :194; the read is at :211. On the collateral-failure path it is never reached, so the proposed benefit does not exist. Declining.

kevindeforth

Thank you very much! There is one code-comment that needs cleanup and I have some suggestions for the metric / alert-doc, but the implementation itself looks good!

kevindeforth · 2026-08-27T08:18:03Z

+/// A metric registers on first dereference, so without this a node that never gets that far
+/// exports no series at all — and an alert on an absent series never fires, in exactly the failure
+/// mode these gauges exist to catch.
+pub fn init_attestation_freshness_metrics() {


Please refine this comment to be in line with our engineering standards. Every sentence is violating one of our rules.

update in commit 6490442

kevindeforth · 2026-08-27T08:22:05Z

+    LazyLock::new(|| {
+        prometheus::register_int_gauge!(
+            "mpc_attestation_expiry_timestamp_seconds",
+            "Unix time at which the attestation stored on chain for this node's TLS key expires. \


Is this timestamp coming from the NEAR blockchain? If so, then I think we should mention that, because the two may not align perfectly (I think we can have up to a few minutes of divergence).

Yes — the contract stamps it from env::block_timestamp_ms() (tee_state.rs:149), so it is NEAR block time.

Worth more than a mention: the other gauge is set from the node's own clock, so the two are on different clocks and mixing them up gives a wrong alert. Both HELP strings now name their clock, and the expiry one says to subtract mpc_indexer_latest_block_timestamp_seconds rather than wall clock.

kevindeforth · 2026-08-27T08:25:43Z

+
+| Metric | Measures | How to interpret |
+| --- | --- | --- |
+| [`mpc_attestation_last_landed_timestamp_seconds`](../../crates/node/src/metrics.rs) | Unix time of the last attestation submission this node confirmed on chain | should be under an `ATTESTATION_RESUBMISSION_INTERVAL` (1h) old. Only a confirmed landing advances it, so the gap to now is how long re-attestation has been failing, wherever it broke. One failed attempt is absorbed by the next tick; a growing gap ends in eviction from the participant set. |


Suggested change

| [`mpc_attestation_last_landed_timestamp_seconds`](../../crates/node/src/metrics.rs) | Unix time of the last attestation submission this node confirmed on chain | should be under an `ATTESTATION_RESUBMISSION_INTERVAL` (1h) old. Only a confirmed landing advances it, so the gap to now is how long re-attestation has been failing, wherever it broke. One failed attempt is absorbed by the next tick; a growing gap ends in eviction from the participant set. |

| [`mpc_attestation_last_landed_timestamp_seconds`](../../crates/node/src/metrics.rs) | Unix time of the last attestation submission this node confirmed on chain | should be under an `ATTESTATION_RESUBMISSION_INTERVAL` (1h) old. |

The last two sentences are discussing implementation details that are not relevant to the average node operator.

updated, with a compromise, see in commit 6490442

kevindeforth · 2026-08-27T08:31:37Z

+        prometheus::register_int_gauge!(
+            "mpc_attestation_expiry_timestamp_seconds",
+            "Unix time at which the attestation stored on chain for this node's TLS key expires. \
+             -1 if the stored attestation carries no expiry; 0 if none is stored. Compare against \


Hmm, that begs the question if VerifiedAttestation must still support this or not. I assume the contract updated a long time ago and we had resharings in the meantime, which should, theoretically, remove any non-participant.
So, we should only have attestations with timestamps now?

It's something to investigate and not a blocker, as we can remove the Optional from the metric quite easily.

Agreed on the diagnosis — live state is clean (I had PR to clean mock attestation with no expiration)

but, It cannot come out yet though: with_mocked_participant_attestations (crates/contract/src/tee/tee_state.rs:108) still inserts a bare MockAttestation::Valid straight into stored_attestations, bypassing with_expiry_capped_at — so a fresh init / init_running recreates the no-expiry case.

Already tracked in #3786: AC 1 stamps the genesis sentinels, AC 2 then drops this. Deployment and live-state evidence left there — #3786 (comment)

kevindeforth · 2026-08-27T08:34:42Z

+| Metric | Measures | How to interpret |
+| --- | --- | --- |
+| [`mpc_attestation_last_landed_timestamp_seconds`](../../crates/node/src/metrics.rs) | Unix time of the last attestation submission this node confirmed on chain | should be under an `ATTESTATION_RESUBMISSION_INTERVAL` (1h) old. Only a confirmed landing advances it, so the gap to now is how long re-attestation has been failing, wherever it broke. One failed attempt is absorbed by the next tick; a growing gap ends in eviction from the participant set. |
+| [`mpc_attestation_expiry_timestamp_seconds`](../../crates/node/src/metrics.rs) | Unix time at which the attestation the contract stores for this node's TLS key expires | should sit a full expiry window ahead of chain time and step forward hourly. `0` = nothing stored (evicted, or never landed one), `-1` = stored without an expiry (legacy entries only; the contract stamps every attestation it accepts). Compare against `mpc_indexer_latest_block_timestamp_seconds`, the clock the contract expires entries against. |


Here, we should again check if we are talking near-blockchain time or unix time.

should sit a full expiry window ahead of chain time

I think we are leaking implementation details here. If operators want to write an alert based on this sentence, they would need to know the exact expiry window and where to look it up.

and step forward hourly

I think this is more useful, but may also be wrong if we change the contract and expiration implementation in the future. Maybe, we can give some concise actionable advise like: Should be at least X days from now. Wdyt?

Both good points, and they pushed me off the fraction-of-window idea entirely.

NEAR block time, as above.

On the window: you are right it leaks. Went with a plain duration instead, which needs no knowledge of the window — page when under 3 days of runway remain.

The only assumption left is that the window stays above 3 days. Growing it (7d → 31d) needs no change; only shrinking it below ~3 days would. The alert comment says to keep the threshold below the window for that reason.

SimonRastikian

Approval with nits

SimonRastikian · 2026-08-27T08:50:50Z

+}
+
+pub(crate) fn record_attestation_landed(clock: &Clock) {
+    MPC_ATTESTATION_LAST_LANDED_TIMESTAMP_SECONDS.set(clock.now_utc().unix_timestamp());


Why do you not have Clock::real() inside this? Then the function would take no inputs

Oh I see it's for unit testing... still would be interested in your opinion

only for unit test, originally I didn't have this parameter, then the Claude bot flagged that we don't have a unit test for this. so I added it.

Also think it's the right call regardless: engineering standards ask for time to be injected, and the test guards a seconds-vs-milliseconds mix-up.

SimonRastikian · 2026-08-27T08:51:44Z

+fn expiry_gauge_value(stored: Option<&VerifiedAttestation>) -> i64 {
+    match stored.map(VerifiedAttestation::expiry_timestamp_seconds) {
+        None => NO_ATTESTATION_STORED,
+        Some(None) => NO_EXPIRY,


That does this mean in real world, which attestation has no expiry?

Yeah, I don't think we need this, but we need to clean up the interface to get rid of this (c.f. #4236 (comment))

see repose in here #4236 (comment)

SimonRastikian · 2026-08-27T09:07:50Z

+const NO_ATTESTATION_STORED: i64 = 0;
+const NO_EXPIRY: i64 = -1;


You could create an enum with values NO_ATTESTATION_STORED, NO_EXPIRY, EXPIRES_AT(i64).
It would be nicer I think

I'd lean towards keeping the consts. The gauge takes an i64, so an enum needs a conversion on top and the 6-line match becomes ~15 for the same mapping. also I follow the pattern from image_expiry_metrics.rs

And NO_EXPIRY should disappear once #3786 lands anyway.

Names the clock each gauge is on: the expiry is stamped from NEAR block time, the landing timestamp comes from the node's own clock, and mixing them up gives a wrong alert. Drops the fraction-of-expiry-window alert threshold. It embedded a stale copy of the window, so shrinking the window would have made the alert fire on every healthy node; a plain duration needs no knowledge of the window at all.

kevindeforth

Thanks!

barakeinav1 mentioned this pull request Aug 25, 2026

feat(node): expose attestation-freshness metrics on /metrics #3951

Closed

3 tasks

barakeinav1 force-pushed the 3951-attestation-freshness-metrics branch from a709f34 to c05969f Compare August 25, 2026 14:38

barakeinav1 marked this pull request as ready for review August 25, 2026 14:39

Copilot AI lite review requested due to automatic review settings August 25, 2026 14:39

Copilot started reviewing on behalf of barakeinav1 August 25, 2026 14:39 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

kevindeforth reviewed Aug 27, 2026

View reviewed changes

SimonRastikian previously approved these changes Aug 27, 2026

View reviewed changes

barakeinav1 dismissed SimonRastikian’s stale review via 6490442 August 27, 2026 09:36

barakeinav1 requested review from SimonRastikian and kevindeforth August 27, 2026 10:01

SimonRastikian approved these changes Aug 27, 2026

View reviewed changes

kevindeforth approved these changes Aug 27, 2026

View reviewed changes

barakeinav1 added this pull request to the merge queue Aug 27, 2026

Merged via the queue into main with commit 8a5725a Aug 27, 2026
15 checks passed

barakeinav1 deleted the 3951-attestation-freshness-metrics branch August 27, 2026 16:12

		const NO_ATTESTATION_STORED: i64 = 0;
		const NO_EXPIRY: i64 = -1;

Uh oh!

Conversation

barakeinav1 commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

claude Bot commented Aug 25, 2026

Pull request overview

Reviewed changes

Findings

Uh oh!

barakeinav1 commented Aug 26, 2026

Uh oh!

kevindeforth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barakeinav1 Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barakeinav1 Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barakeinav1 Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barakeinav1 Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barakeinav1 Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SimonRastikian left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barakeinav1 Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevindeforth Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barakeinav1 Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevindeforth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

barakeinav1 commented Aug 25, 2026 •

edited

Loading

barakeinav1 Aug 27, 2026 •

edited

Loading

barakeinav1 Aug 27, 2026 •

edited

Loading

barakeinav1 Aug 27, 2026 •

edited

Loading

barakeinav1 Aug 27, 2026 •

edited

Loading

barakeinav1 Aug 27, 2026 •

edited

Loading

barakeinav1 Aug 27, 2026 •

edited

Loading

kevindeforth Aug 27, 2026 •

edited

Loading

barakeinav1 Aug 27, 2026 •

edited

Loading