Embedded game monitor - #151
Open
piersy wants to merge 127 commits into
Open
Conversation
Overhaul the game monitor with a two-tier retry system, progress persistence, operational tooling, and a slimmed-down Docker image. ## Retry system - **Primary retries** with configurable count (`--cost-estimator-retries`, default 1) and linear backoff (`delay * 2 * retry_number`). Previously hardcoded to 3 immediate retries which overloaded the monitor. - **Background retries** for games that exhaust primary retries. These run only when spare execution slots are available (primary always wins), use exponential 4x backoff, and are evicted after `--background-retry-max-age-secs` (default 3.5 days). Games are marked "completed" when entering the background queue so `last_contiguous` can advance past them. - `AttemptKind` enum (`Primary`/`Background`) replaces bare `retries: u32`, enabling distinct scheduling and log-file naming (`-bg-retry<n>`). ## Progress persistence - New `ProgressState` (serialised to `progress.json`) stores `last_contiguous` and the `background_retries` queue. The daemon resumes from persisted progress on restart. - `SequenceTracker` (new `utils/common` crate) tracks out-of-order game completions to compute `last_contiguous` correctly. - Both `progress.json` and `completion_history.json` are now written atomically (write-to-tmp + rename). ## Operational tooling - **`game-monitor run`** / **`game-monitor gen-background-retry`** subcommand split via clap. The daemon moves under `run`; `gen-background-retry` reads an existing `progress.json`, appends new background entries for given game indexes (fetching `game_created_at` from L1), and prints the complete state to stdout. Existing k8s manifests need `command: [game-monitor, run]`. - **`rerun-cost-estimator.sh`** — replays a failed game from the log-file header (command + env). - **`fetch-game-logs.sh`** — tars all log files for given game indexes to stdout for local extraction. ## Docker image - Removes Rust toolchain and full workspace copy from the runtime image. Previously needed for `cargo_metadata::MetadataCommand`; now unnecessary since cost-estimator runs with `--log-only`. - Copies the two operational scripts into the image. ## Other improvements - `gameCount()` calls now use `BlockId::finalized` to avoid node-desync issues. - Log-size kill message fixed: was showing `0.0 MB` for both current and median (dividing bytes-per-block by 1024^2); now shows actual total MB and bytes/block. - `--batch-size` flag caps per-execution chunk size (default 200) instead of using the full block range. - Pending games are scheduled by absolute `executable_at` with retry backoff, instead of immediate retry. - Comprehensive doc comments throughout `game_monitor.rs`.
* Execute Jovian on Sepolia. * Add Succinct v2.1.0 upgrade. * Update opsuccinctfdgconfig.mainnet.json
…i-and-agg-image chore(scripts): replace parallel-cost-estimator with cost-estimator + multi-and-agg image
* Classify cost-estimator failures by log introspection
- Add `FailureType` enum covering the five transient infrastructure
failures observed in the production cost-estimator log corpus: HTTP 503
from the RPC proxy (`-32011`), missing block state (`-32002`), exceeded
proof window (`-32602`), missing trie node (`-32000`), and DNS lookup
error from the op-node hostname. An `Unknown` variant covers everything
else, including I/O errors reading the log.
- Add `detect_failure_type`, which reads up to the last 1 MiB of a failure
log and matches it against substring patterns derived from the corpus,
in descending order of observed frequency. The tail window is sized to
cover the worst-case multi-panic logs (~500 KiB) with margin; the match
uses `str::contains` rather than a regex because the chosen anchors are
invariant across thread ids, request ids, and addresses.
- Include the classification in the existing "cost estimator failed" error
log so every natural exit is annotated inline. The literal `unknown` is
grep-friendly, letting operators surface failure modes that need a new
pattern.
The kill path is intentionally not classified: when the daemon terminates
a process for exceeding its runtime or log-size budget, the log holds no
estimator-side failure to classify, so introspection would mislead.
* Skip cost-estimator spawn when L2 is behind the game's end block
The op-geth L2 RPC can fall behind L1, leaving games visible on L1 whose
end block has not yet been finalised on L2. Running the cost estimator
against that range would either fail outright or produce a misleading
result, so the daemon now reads the L2 finalized head before each spawn
and defers the entry when the L2 has not caught up.
- New `SpawnOutcome::L2Behind` variant, returned by `spawn_game` after
`fetch_game_data` succeeds but before any process is started, when the
L2 finalized block is below `game_data.end_block`. The entry stays in
its queue; the next poll re-checks it.
- Plumb an L2 (op-geth) provider through `SpawnContext`. The provider is
built from a new required `L2_RPC` env var alongside the existing
`L1_RPC`, using the same `ProviderBuilder` pattern.
- Refactor both spawn loops (primary and background) from
`while can_spawn_new { find(...) }` to a snapshot-then-`for` over the
eligible candidates. The previous shape always re-found the same first
matching entry, so `L2Behind` had to `break`; otherwise the loop would
spin on that entry forever, making one L2 RPC call per iteration. The
snapshot Vec walks each candidate exactly once, so `L2Behind` can now
`continue` and let older queue entries (with potentially lower end
blocks) have a turn this poll.
- Emit a `warn!` line with the observed L2 finalized block and the
deferred game's end block so operators can see why the daemon is idle
while the queue has unspawned games.
After a long L2 finalized-head stall, the proposer can prune its cache down to the anchor game and rebuild the backlog as a genesis-rooted chain (parent = u32::MAX, since the contract reverts when a new game's parent is the current anchor). compute_canonical_head pinned the head to the root of that alternative chain instead of following it to the tip, so the canonical head could not advance and game creation stalled until the anchor caught up. Follow each qualifying alternative-chain root (genesis-rooted, or a lower parent index than the anchor head) to its highest-block tip. Extract the selection into ProposerState::select_canonical_head and cover it with unit tests.
In-process replacement for the subprocess-based game monitor: a predictive, finalization-triggered witness pipeline that prebuilds SP1 stdin from PROPOSAL_INTERVAL-predicted ranges, on-disk caching of WitnessData and SP1Stdin, structured per-range/per-call retries, and cgroup-aware memory admission. Targets EigenDA; leaves cost_estimator.rs and the proposer untouched.
…t Sp1Execute variant Every ExecutionError is deterministic over fixed stdin, so all are non-retryable; the per-variant TooMuchMemory special-case only labelled, it never changed control flow. Carry the concrete error in Sp1Execute(#[from] ExecutionError) instead -- non-transient by construction, type preserved for logs, no string round-trip. Drop the vestigial 'too much memory' arm from classify (execute now matches typed).
A game that built stdin for some sub-ranges then hit a fatal error left those blobs orphaned -- the Fatal handler completed the game without scheduling any prune, so only the size-cap GC eventually reclaimed them. execute_game now returns its sub-ranges regardless of outcome, so Fatal carries them like Success does and the handler schedules a prune per range (deterministic split; prune_stdin no-ops the sub-ranges that were never built). Transient still keeps its cache for the retry. WrongType is unaffected: it is returned from fetch_game_data before the block range is fetched and before execute_game runs, so it has no ranges and never built stdin -- nothing to prune.
A finished game (Success or Fatal) is done with its ranges, so there is no reason to keep their stdin blobs around. Remove the --stdin-grace-secs flag, the per-blob eligible_at deadline, and is_prune_eligible; the main-loop sweep now drains the prune list every iteration, reclaiming a completed game's stdin in the same tick. The size-cap GC still bounds the cache and protects background-retry ranges. The only thing given up is a soft, time-bounded cache hit for a near-term rerun (overlapping neighbour games, or #27 restart re-runs of above-watermark games) -- those become rebuilds, which are idempotent.
…re-run them (#27) Completions ahead of the contiguous watermark lived only in SequenceTracker's in-memory pending set; progress.json stored only last_contiguous, so a restart re-discovered and re-executed every out-of-order success. With stdin now pruned immediately on completion, those reruns are full recomputes. SequenceTracker gains restore/contains/pending_indices; ProgressState persists the pending set (serde-default empty for old files); on resume the tracker is rehydrated from last_contiguous + pending, and discovery skips any index already completed. An explicit --start-index still starts clean. This makes immediate stdin pruning fully safe: a completed game is never run again.
Reverses the fatal-prune from the #9 fix: a fatally-failed game's cached stdin is now kept on disk so it can be fetched to iterate on the execution code locally. The size-cap GC reclaims it under space pressure (fatal ranges are unprotected, evicted oldest-first) -- the chaos overlay sets --max-cache-size=40GB. execute_game returns ranges only on success again (a failure needs none), and the Fatal outcome no longer carries ranges or schedules a prune. Success still prunes immediately; Transient still keeps its cache for the retry.
…ence changes Outstanding-work doc: #9 marked RESOLVED (success prunes immediately, fatal retains stdin for debugging, wrongtype vacuous); #27 marked FIXED (pending set persisted, restart skips completed); added the error-classification, immediate-prune, and persistence fixes to the fixed table. Also document why MissingTrieNode is transient in error.rs. (The workspace-root architecture doc was updated in place too; it is not tracked in this repo.)
The readiness buffer and the host's calculate_safe_l1_head +20 are now one shared constant L1_HEAD_BUFFER in utils/host, imported by the readiness gate and the DA hosts, so they cannot drift -- the compile-time tie #10 wanted. Added to the fixed table and updated the item; a flag was deemed unnecessary (must match the host).
A drop_witness failure after stdin was saved made build_range_witness return Transient -- a false 'build failed' that triggered a spurious retry -- and the leaked witness blob was never reclaimed because the retry short-circuits at has_stdin and never re-reached the drop. Now the post-stdin drop is best-effort: a failure is logged and the build returns Ok (stdin is the durable product). The has_stdin short-circuit also best-effort re-drops, so re-requesting the range self-heals a previously-leaked blob; the size-cap GC stays the backstop. Dropping is always safe once stdin exists.
… read (#28) baseline_bytes was read once at startup (~50 MB, pre-warm-up) and frozen, while the real steady-state idle floor is ~2.5-5 GiB. With baseline near-zero, per-kind cost_per_gas folded the fixed floor into the slope, so project() under-projected at low concurrency (over-admit) and over-projected at high. baseline_bytes now lives in CostModel (serde-default, persisted) and is learned as an EWMA of idle RSS: observe() folds the current rss whenever nothing is in flight and no episode is closing that tick (the transitional close tick is skipped). net = rss - baseline yields the true marginal cost, and project = baseline + slope*gas is correct at all concurrency levels. The startup read only seeds it until the first idle sample. An old persisted model loads (baseline defaults to 0, re-seeded); its folded-in costs decay to the true marginal as the baseline rises -- the transition over-projects (conservative), and the 20 GiB margin covers the floor during warm-up regardless. Tested: observe_learns_idle_baseline + baseline round-trip in persist test.
…comments (#13) The Defer re-queue set executable_at = now_instant + poll, but the loop's own sleep(poll) runs between the apply step (where Defer re-queues) and the spawn step, so by spawn time the clock has already advanced past now_instant + poll. The delay was therefore a no-op: a deferred game is eligible at the next spawn either way (one re-check per iteration). Use now_instant so the code matches the behaviour, and drop the now-unused poll parameter from apply_game_result. Also correct two stale '--delay' comments: the flag is --retry-backoff-delay and no longer gates discovery (games are enqueued immediately; readiness gates when each runs). Marks #13 fixed in the notes.
…nwrapping (#11) A panic while holding the cost mutex poisons it; every later admit/observe/persist then unwrap()s the PoisonError and panics too, silently killing the sampler and the prebuild pipeline. Route all four production lock sites through a cost_guard() helper that recovers the guard (lock().unwrap_or_else(|e| e.into_inner())). CostModel is a learned heuristic with no fragile invariant, so proceeding on a possibly half-written value is fine and the count cap + margin still bound admission. Tests keep unwrap(). Also add backlog item #30: a tracing panic hook so the root panic (e.g. in the detached sampler) reaches the structured log stream, not just stderr.
Both concern block_range.rs code the embedded daemon does not use -- the .expect() panics (#14) are in get_validated_block_range/get_rolling_block_range, and the untested safe-head splitter (#18) was removed from the daemon in #3. Only cost_estimator and tests call them. Per the scope rule (fix/test only what the embedded monitor uses), both are out of scope.
…ction (#16) second_build_is_noop_fast_path only checked that a second build_range_witness returned Ok with cached stdin -- it never proved the expensive host.run was skipped. Renamed to second_build_short_circuits_host_run and strengthened: after building the real range, take a range the host cannot build (blocks beyond any chain height). Without cached stdin that build fails (host path exercised); with its stdin pre-seeded it returns Ok, which is only reachable via the has_stdin short-circuit before host.fetch/host.run. Still ENV-gated; skips cleanly in CI.
) The parity test only asserted shape (batch_end, nb_blocks, instruction count > 0). execute_game_matches_serial_reference now splits the window with split_range_basic, execute_range's each sub-range independently, aggregates, and asserts execute_game's concurrent split-and-aggregate equals that reference field-for-field (ExecutionStats gains PartialEq/Eq). Also asserts the split matches split_range_basic and the batch bounds. Execution is deterministic and aggregation is an order-independent sum, so the equality is exact. A cross-tool cost_estimator baseline is out of scope: it is not daemon code, not retrofitted to utils/estimator, and splits differently (not apples-to-apples); its aggregation is the same logic this reuses.
The RPC-gated integration tests used #[tokio::test] (current-thread). Witness generation (host.run) calls kona's block_on, which uses tokio::task::block_in_place -- that panics on a current-thread runtime. The daemon works because #[tokio::main] is multi-threaded. So none of these tests could actually run against a real node; they only ever exercised the skip path. Switch them to #[tokio::test(flavor = "multi_thread")] so they run for real. Found while running #15/#16 against Celo Sepolia.
Without it the client falls back to L2StateNode hints for trie nodes missing from the eth_getProof prefetch, and reth's debug_dbGet (code-only, 33-byte keys) can never serve them. kona's backend then retries the deterministic -32602 forever, wedging witness gen — observed on Sepolia (game 29347, >17k errors/min). debug_executePayload supplies the complete execution witness up front; a failure of the call itself is swallowed and degrades to the previous behaviour. Also records outstanding-work item #31: bound the witness build so a deterministic hint-fetch error fails the build instead of spinning.
Backport of celo-kona succinctlabs#229 onto piersy/game-monitor-improvements (27c9f328): the L2PayloadWitness hint handler now uses OpPayloadAttributes end-to-end — matching what kona-proof serializes and what celo-reth's debug_executePayload deserializes. Without it the hint route failed with 'missing field opPayloadAttributes', closing the preimage channel and failing every witness build once the witness endpoint was enabled.
…he unforked kona bump Propagating deterministic fetch_hint errors in OnlineHostBackend is the right fix (handlers already own transient retry), but kona sits in the celo-org/optimism fork and #150 moves to a kona we don't need to fork, so no fork patch now. The monitor-side timeout fallback is noted but not implemented.
… subsumes #24) Replace the admission poll-race with a Scheduler owning per-type queues drained by build/execute worker pools: a FIFO priority queue of game demands (served in demand order, so speculation and newer games never starve a game already executing) then a LIFO speculative queue (newest range first). Games no longer run heavy work: execute_game demands its missing ranges and assembles the aggregate from a new proof cache (ExecutionStats per range, JSON on the PVC). An un-built demand promotes a witness demand onto the build priority queue and parks until the witness lands, instead of building inline. A partially-failed game's retry now re-runs only the ranges that actually failed. Completed speculative builds feed a speculative execute queue bounded by --max-speculative-lead-windows (default 1, 0 disables): window prediction is operator config, not protocol law, and a wasted speculative execute is the dominant uncancellable cost, so the lead cap bounds the waste when an assumption breaks (interval change, re-anchor, stalled proposer). A demand for the same range bypasses the cap. Demanded failures cross the scheduler as DemandError and feed the normal two-tier game retry; speculative failures are dropped. monitor status gains queued_builds / queued_executes / waiting_witness.
The ~1/s 'admission rss sample' line was temporary INFO instrumentation for characterising the RSS-vs-gas curve. Keep it, but at TRACE so it is silent by default; opt in with an admission=trace RUST_LOG directive.
The old text implied a hard SP1 guest memory cap. The guest is 64-bit (sp1 v6); its bound is the soft, env-overridable MEMORY_LIMIT budget (default 24 GiB). Document what the batch size actually scales: per-unit host/guest memory and the retry/cache/speculation granularity.
… queued_witness/queued_prove Aligns the scheduler queue-depth fields with the existing active_witness / active_prove naming in the monitor status line.
Reword control/data plane, frontier, horizon, feeder, wedge, backstop, happy path, and similar figurative comment language into plain descriptive terms. Rename park_for_witness to wait_for_witness (and 'parked' to 'waiting' throughout) to match the exec_waiting set and the waiting_witness status field; rename the affected tests likewise. Industry-standard terms (watermark, speculative, cold start, starvation, liveness floor, high-water mark) are kept.
Witness generation was called build and proving was called execute. Rename throughout the embedded monitor to the correct terms. - WorkKind::Build/Execute -> Witness/Prove - Estimator::build_range_witness/execute_range -> witness_range/prove_range - executor execute_step/execute_game -> prove_step/prove_game - scheduler queue fields, worker fns, and public methods - admission/registry CostModel and gas/unit fields, log fields - tracing spans work=build/execute -> witness/prove; child span execute -> prove - CLI flags --max-concurrent-builds/-units -> --max-concurrent-witness-tasks/-prove-tasks - tests and docs updated to match External SP1 terminology kept (Sp1Execute, ExecutionError, ExecutionStats, CpuProver.execute/run). Renaming the persisted CostModel JSON keys resets the learned memory model once on next start.
…y it Game tasks are cheap orchestration work (split, demand, wait) — the heavy witness/prove units are bounded by --max-concurrent-witness-tasks / --max-concurrent-prove-tasks and the RSS admission gate. Bump the default from 5 to 20 and rewrite the help to explain it is an orchestration-layer cap that only overlaps the worker caps when a game is a single sub-range (batch_size >= proposal_interval). Also fixes two stray build/execute leftovers in nearby comments.
The original spec and plan described the initial design and build order, which have materially diverged from the shipped code (SafeDB-based splitting, the 'contained' naming, JSON-default logs, the Task-by-Task build order). They are now misleading rather than useful. The architecture doc and the outstanding-work tracker are the current source of truth, alongside the code itself. Reword the outstanding-work header to drop the dangling spec/plan links; the in-body 'the spec'/'the plan' mentions remain as historical prose.
Rewrite the memory-admission (RSS) flag docs for a reader who does not know what RSS is: explain resident set size in plain terms and what each of --rss-source, --rss-margin-mb, --sample-period-ms and --persist-every-ticks does. Reorder the CLI so related flags sit together, under section comments: startup, chain/window, discovery/main loop, retry, concurrency, memory admission (RSS), cache/persistence. Field names are unchanged, so behaviour and callers are unaffected (clap uses declaration order only for --help). Also complete the batch_size -> range_split_count change: replace the per-range-block-size flag with --range-split-count (default 1, mirrors the proposer's range_split_count), derive range_size = proposal_interval / range_split_count (rounded up), and thread it through prove_game and ReadyRangeProvider (internal name range_size) plus the tests.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52d2618c18
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // control, so start from a clean tracker at the requested point. | ||
| let mut tracker = match (args.start_index, persisted.as_ref()) { | ||
| (None, Some(p)) => SequenceTracker::restore(p.last_contiguous, p.pending.iter().copied()), | ||
| _ => SequenceTracker::new(next_game_index.saturating_sub(1)), |
There was a problem hiding this comment.
When there is no persisted progress and next_game_index is zero—such as a fresh factory with its first game or --start-index 0—saturating_sub(1) initializes the tracker with end == 0. Because SequenceTracker::contains treats every index at or below end as completed, discovery permanently skips game 0 instead of queueing it. Use a watermark representation that can express “no games completed” rather than mapping that state to zero.
Useful? React with 👍 / 👎.
Comment on lines
+79
to
+83
| let w = self | ||
| .host | ||
| .run(&args) | ||
| .instrument(tracing::info_span!("host.run")) | ||
| .await |
There was a problem hiding this comment.
When the host backend encounters a deterministic hint-fetch failure, its preimage path can retry indefinitely, but this await has no timeout or cancellation boundary. The call then permanently retains its witness worker and memory-admission reservation, while any game demanding that range waits forever; enough such calls exhaust the worker pool and stop game processing while the daemon remains alive. Bound this operation and surface expiry through the existing retry policy.
Useful? React with 👍 / 👎.
Comment on lines
+155
to
+156
| pub fn stats_path(&self, start: u64, end: u64) -> PathBuf { | ||
| self.cache_dir().join(format!("{start}-{end}-{}-stats.json", self.da_type.as_str())) |
There was a problem hiding this comment.
The persistent stats key includes only the block range and DA type, although instruction counts and SP1 gas also depend on get_range_elf_embedded() and the executor version. After deploying a binary with a changed range program while retaining the cache, a previously speculated range or partially completed background game is accepted as a cache hit and reports the old program's statistics without executing the new ELF. Include a program/vkey or build-version discriminator in this cache namespace.
Useful? React with 👍 / 👎.
Comment on lines
+798
to
+802
| pending_games.push_back(PendingGame { | ||
| executable_at: Instant::now(), | ||
| game_index, | ||
| kind: AttemptKind::Background { attempts: attempts + 1 }, | ||
| }); |
There was a problem hiding this comment.
When a background retry is both due and older than background_retry_max_age_secs, this loop first pushes it into pending_games; the subsequent age-eviction only removes the original background entry and leaves the pending attempt intact. Consequently a game logged as permanently abandoned still starts another potentially expensive witness/prove run, and because that removal is not persisted it can recur after a restart. Filter or evict aged entries before constructing the due queue.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a draft PR for the embedded game monitor
It's a complete re-write of the game monitor that keeps everything in process.
Bear in mind that it has mostly been vibe coded, but seems to work well so far running on the chaos network (pre espresso) and sepolia.
Doing so makes:
The result is a game monitor with better throughput and lower memory usage.
This is currently running on sepolia, and seems to be running well. It will need updating on top of the develop branch if we want to run it against a network with the espresso integration.
See a grafana dashboard for it here - https://clabs.grafana.net/goto/skbn42?orgId=stacks-177453
There is an open PR here - https://github.com/celo-org/infrastructure/pull/3161 to add the config to argocd for the embedded game monitor
What remains is to figure out how to alert nicely from this, it's very much geared to being resilient to failures with multiple levels of retry built in. So far I've not actually witnessed it getting stuck on anything. So my first thought would be to track how far behind it starts lagging and to alert when that goes over some threshold.