docs: note that equal API and metrics ports merge routers - #560
Merged
Conversation
CLAUDE.md described the RPC crate as always binding two Axum servers. `crates/net/rpc/src/lib.rs` merges the API and metrics/debug routers onto a single listener when --api-port and --metrics-port match, so pointing both flags at one port is supported rather than a misconfiguration. Split out of #554, which only touched it incidentally.
🤖 Kimi Code ReviewThis is a documentation-only change to Minor suggestion: Line 291 mentions "merges all three routers" but the preceding sentence only explicitly mentions two categories ("API router" and "metrics/debug routers"). If there are indeed three distinct routers (e.g., API, metrics, debug as separate routers), consider enumerating them explicitly for clarity: When the two ports differ it binds two independent Axum servers (API vs. metrics/debug);
when they are equal it merges the API, metrics, and debug routers onto a single listener...If "metrics/debug routers" refers to a single combined router, "all three" should be "both" to avoid confusion. Verdict: Documentation is accurate and the change is low-risk. Ensure the implementation in Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewReview: PR 560 — docs(claude-md): note that equal API and metrics ports merge routersThis is a docs-only, 4-line change to
The updated wording accurately reflects this behavior, including the default ports (5052/5054) and the "equal ports is supported, not a misconfiguration" nuance. No code changes, no behavioral impact, and the pointer to Nothing to flag — the change is correct and appropriately scoped. Automated review by Claude (Anthropic) · sonnet · custom prompt |
Contributor
Greptile SummaryThe PR corrects the HTTP server documentation to distinguish between separate API and metrics listeners and the supported equal-port configuration.
Confidence Score: 5/5The documentation-only PR appears safe to merge. The revised defaults and equal-versus-different port behavior match the CLI configuration and RPC server implementation, leaving no actionable issue.
|
| Filename | Overview |
|---|---|
| CLAUDE.md | Accurately documents RPC listener behavior and port defaults without changing runtime behavior. |
Reviews (1): Last reviewed commit: "docs(claude-md): note that equal API and..." | Re-trigger Greptile
🤖 Codex Code ReviewNo findings.
This PR is documentation-only, so the consensus-critical areas you listed are unaffected. Residual risk is limited to documentation drift, and I did not find any drift relative to the current code. Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
MegaRedHand
changed the title
3 tasks
pablodeymo
approved these changes
Aug 3, 2026
MegaRedHand
added a commit
that referenced
this pull request
Aug 3, 2026
## 🗒️ Description / Motivation
Restarting a node without `--checkpoint-sync-url` destroyed its chain.
`fetch_initial_state` gated the on-disk state lookup on that flag:
```rust
if checkpoint_urls.is_empty() {
info!("No checkpoint sync URL provided, initializing from genesis state");
let genesis_state = State::from_genesis(genesis.genesis_time, validators);
return Ok(Store::from_anchor_state(backend, genesis_state));
};
// ... only past this point was Store::from_db_state tried
```
So a redeploy against a populated RocksDB wrote a slot-0 genesis anchor
over a perfectly current chain, and operators had to pass a checkpoint
URL purely as a *fallback trigger* even when the DB was fresh and the
URL was never fetched.
This makes on-disk state authoritative: `--checkpoint-sync-url` becomes
a fallback for when there is nothing resumable, not a precondition for
reading what is there.
## What Changed
**`bin/ethlambda/src/main.rs`** — `fetch_initial_state` tries
`Store::from_db_state` *before* the empty-URL early return:
```
gap = current_slot − store.head_slot()
gap ≤ MAX_RESUMABLE_DB_STATE_AGE → resume from DB info!
gap > MAX, no checkpoint URLs → resume from DB warn! (new)
gap > MAX, checkpoint URLs set → fall through to checkpoint sync
no resumable DB, checkpoint URLs → checkpoint sync
no resumable DB, no URLs → genesis
```
Also removes the `info!(url_count, "Starting checkpoint sync")` that was
emitted *before* the DB was consulted. It fired on every successful
resume, so grepping a boot log for `"Starting checkpoint sync"`
false-positived on nodes that never synced. Each outcome now logs
exactly one line, at the point the decision is made:
| Boot log line | Outcome |
| --- | --- |
| `Resuming from existing DB head_slot=… current_slot=… gap=…` | Resumed
from disk, nothing downloaded |
| `DB is stale; resuming anyway head_slot=… current_slot=… gap=…` |
Resumed past the window, no URL to prefer |
| `DB is stale; checkpoint sync head_slot=… current_slot=… gap=…` | Past
the window, a URL took over |
| `Starting checkpoint sync checkpoint_urls=[…]` | Downloading a
checkpoint |
| `No checkpoint sync URL provided, initializing from genesis state` |
Started from genesis |
**`bin/ethlambda/src/cli.rs`** — `--checkpoint-sync-url` help text no
longer claims it "skips genesis initialization"; it is documented as a
fallback.
**`docs/checkpoint_sync.md`** — new *Restarts and Existing State*
section: precedence table, the resume window and why it is measured
against the head rather than the finalized checkpoint, the P2P-catch-up
caveat, and why an all-URLs-fail abort is intentional.
## Correctness / Behavior Guarantees
| DB state (matching `GENESIS_TIME`) | `--checkpoint-sync-url` | before
| after |
| --- | --- | --- | --- |
| absent | omitted | genesis | genesis |
| absent | set | checkpoint sync | checkpoint sync |
| fresh (head-lag ≤ 450) | omitted | **genesis, resets to slot 0** |
**resume** |
| fresh | set | resume | resume |
| stale (head-lag > 450) | omitted | **genesis, resets to slot 0** |
**resume + warning** |
| stale | set | checkpoint sync | checkpoint sync |
- `MAX_RESUMABLE_DB_STATE_AGE` keeps its value and its meaning in the
URL-present case; it now only decides *whether a checkpoint is
preferable to what we already have*, never whether the DB is readable.
- Staleness is still measured against the head (`current_slot -
head_slot`), so a node whose head is current resumes during a finality
stall.
- **Stale DB + no URL resumes rather than refusing to boot.** No
checkpoint URL was configured, so there is no anchor to switch to and
the node runs against the data directory it was given. The warning
exists because range sync may not close a gap this large: peers prune
block signatures past `SIGNATURE_PRUNING_RANGE` (~1 day), so beyond that
horizon they cannot serve the missing history and the node needs a
checkpoint URL. Refusing to start instead would break unattended
restarts after a routine 31-minute outage.
- **Stale DB + URLs set + every URL failing still aborts.** Deliberate,
and documented as such: configuring the flag asks for a specific anchor,
so an unreachable source is a misconfiguration to surface at boot rather
than paper over by starting a node that is hours behind. Omitting the
flag is how you ask for "resume whatever is on disk"; that path never
aborts.
- **No new flag.** Omitting `--checkpoint-sync-url` no longer means
"start from genesis" when a DB exists; to deliberately start over,
remove the data directory. That is already the documented idiom for a
clean checkpoint sync, and an `--ignore-existing-db` flag would only
reintroduce the write-genesis-over-live-data footgun behind a flag.
- **Unchanged / out of scope:** a `GENESIS_TIME` mismatch still degrades
silently (`from_db_state` logs `"Persisted DB has a different
genesis_time; treating as empty"`), so with no URL the node writes
genesis over a foreign-network DB. Pre-existing behavior, addressed
separately in #556;
`initializes_from_genesis_when_db_genesis_time_differs` pins it here as
a known hazard rather than a desired invariant.
## Tests Added / Run
Six unit tests in `bin/ethlambda/src/main.rs` driving
`fetch_initial_state` against `InMemoryBackend`:
| Test | Gap | Asserts |
| --- | --- | --- |
| `initializes_from_genesis_when_db_is_empty` | — | head slot 0 |
| `resumes_from_fresh_db_without_checkpoint_url` | `MAX / 2` | head slot
is the seeded slot, not 0 |
| `resumes_from_stale_db_without_checkpoint_url` | `MAX + 100` | resumes
despite `gap > MAX_RESUMABLE_DB_STATE_AGE` |
| `resumes_from_fresh_db_with_checkpoint_url` | `= MAX` | resume wins
over a URL; nothing is dialed |
| `falls_through_to_checkpoint_sync_when_db_is_stale` | `MAX + 1` | past
the window the URL takes over, and an unreachable one aborts |
| `initializes_from_genesis_when_db_genesis_time_differs` | — | head
slot 0 (DB treated as empty) |
The seeded anchor sits above slot 0 because a genesis re-init also
yields head slot 0; that is what makes "resumed" distinguishable from
"started over". Staleness is induced purely by choosing `genesis_time`
(`current_slot` derives from the wall clock against it), so no clock
injection.
The two no-URL resume tests cannot pin the threshold on their own: both
no-URL branches return the same store, so inverting the comparison
leaves them green. The pair that can are the two with a URL set, where
the outcomes differ. Verified by mutation:
| Mutation | Result |
| --- | --- |
| `gap <= MAX` → `gap > MAX` | both URL tests fail, the four others pass
|
| `gap <= MAX` → `gap < MAX` | the `= MAX` boundary test fails |
Those two use `#[tokio::test(start_paused = true)]` so the checkpoint
retry backoff (5 attempts × 5s) costs no wall clock; the connection
refusal against `http://127.0.0.1:1` is immediate. That needs tokio's
`test-util` feature as a **dev**-dependency, so it never reaches the
shipped binary.
Commands run:
```
cargo test -p ethlambda --profile release-fast --bin ethlambda # 33 passed
make fmt && make lint && make test # all clean (550 passed, 7 pre-existing ignored)
```
Local multi-client devnet verification is in progress; I'll post the
boot logs showing a keep-DB restart with no `--checkpoint-sync-url` as a
comment.
## Related Issues / PRs
- Related to #505 (head-lag resume gate, which this builds on)
- #556 rejects a data directory from another network, covering the
`GENESIS_TIME`-mismatch hazard this PR only pins
- #560 carries the unrelated `CLAUDE.md` RPC-port note that was
originally in this branch
- #559 tracks a pre-existing bug this PR makes easier to hit: the duty
sync gate reports Synced while a node backfills from a stale resume, so
it attests and proposes on an old head. Not addressed here
- Logging a `Store::from_db_state` read error instead of discarding it
(the `Err` arm of `if let Ok(Some(_))`, unreachable today) is left to a
follow-up PR
## ✅ Verification Checklist
- [x] Ran `make fmt` — clean
- [x] Ran `make lint` (clippy with `-D warnings`) — clean
- [x] Ran `make test` (`cargo test --workspace --profile release-fast`)
— all passing
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
2 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.
🗒️ Description / Motivation
CLAUDE.mddescribed the RPC crate as unconditionally running two independent Axum servers:That is only true when the two ports differ.
crates/net/rpc/src/lib.rsmerges the API and metrics/debug routers onto a single listener when--api-portand--metrics-portare equal, so pointing both flags at one port is a supported configuration rather than a misconfiguration.What Changed
CLAUDE.md— the "HTTP Servers (API + Metrics)" paragraph now states both cases: two servers when the ports differ, one merged listener when they match. The pointer todocs/rpc.mdfor the full reference is unchanged.Docs only; no code, no behavior change.
Related Issues / PRs
✅ Verification Checklist
fmt/lint/testare unaffectedcrates/net/rpc/src/lib.rs