Skip to content

fix(sae): don't report a gas-time gap after restart - #6008

Open
JonathanOppenheimer wants to merge 1 commit into
masterfrom
JonathanOppenheimer/fix-sae-gas-time-gap-metric
Open

JonathanOppenheimer wants to merge 1 commit into
masterfrom
JonathanOppenheimer/fix-sae-gas-time-gap-metric

Conversation

@JonathanOppenheimer

Copy link
Copy Markdown
Contributor

Why this should be merged

The wall-clock execution time isn't persisted, so a node that restarts seeds its metrics from a block with the zero [time.Time]. Subtracting that maxes out the duration, and gas_time_wall_time_gap_seconds reports ~9.2e9 seconds until the first block executes. On Helicon launch every mainnet node did this on the last restart, which msesses up the the scaling of the dashboard that uses it.

Note that we should still set NaN, otherwise ti would appear perfectly healthy w/ 0.

How this works

How this was tested

We don't test metrics

Need to be documented in RELEASES.md?

No

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

Fixes SAE gas-time gap metrics after restart by treating missing wall-clock timestamps as unknown.

Changes:

  • Detects zero wall-clock execution timestamps.
  • Reports NaN when timing data is unavailable.
  • Preserves normal gap calculations.
File Summary
vms/​saevm/​saexec/​metrics.go Handles missing wall-clock timestamps during metric initialization and execution.

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

@JonathanOppenheimer JonathanOppenheimer self-assigned this Sep 22, 2026
@JonathanOppenheimer JonathanOppenheimer added the evm Related to EVM functionality label Sep 22, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

evm Related to EVM functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants