fix(watchdog): address PR review on metrics and devnet stack#25
Conversation
Resolve chain id at init (persist from eth_chainId) so tick exit never blocks on RPC. Return nil,err from resolve_path; drop dead timestamp; clear stale status.prom in divergence drill. Observe Anvil + sequencer in devnet-for-watchdog; fix doc label order.
GCdePaula
left a comment
GCdePaula
left a comment
There was a problem hiding this comment.
Great PR, addresses everything I commented!
A few remarks.
The fix updated README.md and operator-deployment.md but missed getting-started.md, which still describes the old tick-time behavior:
docs/watchdog/getting-started.md:178 — the env table row:
CARTESI_WATCHDOG_BLOCKCHAIN_ID | init / tick | Chain id label for status.prom; also read at tick from env or eth_chainId when unset
The "read at tick from ... eth_chainId when unset" is no longer true, tick never queries eth_chainId now.
docs/watchdog/getting-started.md:199 — the troubleshooting row:
chain="unknown" … | Export CARTESI_WATCHDOG_BLOCKCHAIN_ID before init, or rely on tick-time eth_chainId from the L1 RPC
Should say init-time, and note that auto-detect needs CARTESI_WATCHDOG_BLOCKCHAIN_HTTP_ENDPOINT present at init.
This second point is a real behavioral nuance worth a sentence: README.md:171 still frames the L1 endpoint as "intentionally read at tick time, not persisted." Auto-detection now also reads it at init — so an operator who exports the endpoint only for tick (per that framing) will silently get chain="unknown". Setting BLOCKCHAIN_ID explicitly remains the reliable path; just worth making the docs consistent.
| end | ||
|
|
||
| local rpc_factory = deps and deps.rpc_factory or nil | ||
| if type(deps) == "table" and type(deps.rpc) == "table" and type(deps.rpc.get_chain_id) == "function" then |
There was a problem hiding this comment.
resolve_init_blockchain_id has both a deps.rpc branch and a deps.rpc_factory path, but production init uses default_machine_deps (no .rpc) and the tests use rpc_factory, so the deps.rpc branch is never exercised.
Harmless, slightly redundant, could drop it, or keep as defensive.
| eprintln!(); | ||
| eprintln!("=== sequencer exited unexpectedly ({status}) ==="); | ||
| eprintln!("=== {label} exited unexpectedly ({status}) ==="); | ||
| eprintln!("Sequencer log: {}", log_path.display()); |
There was a problem hiding this comment.
report_child_exit always prints "Sequencer log:" and tails the sequencer log even when Anvil is the child that died. The "Other logs under: .../devnet-stack-*" hint still points at the anvil log, so it's just a mild UX gap, for an Anvil exit the tailed lines will be the sequencer's (probably just connection errors).
Drop unused deps.rpc init path; tail the dying child's log (Anvil vs sequencer). Correct getting-started/README/operator docs: chain id is resolved at init, not via tick-time eth_chainId.
678a00c to
9d637a6
Compare
2 participants
Resolve chain id at init (persist from eth_chainId) so tick exit never blocks on RPC. Return nil,err from resolve_path; drop dead timestamp; clear stale status.prom in divergence drill. Observe Anvil + sequencer in devnet-for-watchdog; fix doc label order.