Skip to content

refactor: dead-code sweep in sync / api / storage subspecs - #751

Merged
tcoratger merged 5 commits into
leanEthereum:mainfrom
tcoratger:refactor/sync-api-storage-cleanup
May 21, 2026
Merged

refactor: dead-code sweep in sync / api / storage subspecs#751
tcoratger merged 5 commits into
leanEthereum:mainfrom
tcoratger:refactor/sync-api-storage-cleanup

Conversation

@tcoratger

Copy link
Copy Markdown
Collaborator

Summary

Removes unused code and narrows public surface across sync, api, and storage subspecs. Nine items, no behaviour change for any production caller.

Commits (one per logical change)

  1. refactor(api): drop empty re-exports from endpoints/__init__.py — package only needed a docstring; routes import each endpoint module directly.
  2. refactor(sync): drop unused SyncProgress / get_progress / reset — no production caller; callers query store and peer_manager directly.
  3. refactor(sync): expose PeerManager.peers, drop get_peer/get_all_peers — the two methods were one-line passthroughs to the internal _peers dict. Renaming to a public attribute matches standard Python practice; updated the single production caller (node.py) and tests.
  4. refactor(storage): drop has_block / has_state / commit from Database — none have production callers. Existence checks already work via get_block/get_state returning None; commit() was a single-statement wrapper around _conn.commit() only used in tests. Tests now go through batch_write(), the canonical atomic-write API.
  5. refactor(api): drop /lean/v0/blocks/finalized endpoint — the endpoint and its client helpers (fetch_finalized_block, fetch_finalized_anchor) shipped the anchor SignedBlock alongside the finalized state for checkpoint sync. No production caller: __main__.py uses fetch_finalized_state alone, and Store.create_store no longer needs the anchor SignedBlock. Removes the endpoint module, route entry, client helpers, FINALIZED_BLOCK_ENDPOINT constant, ApiServer.signed_block_getter field, and the matching test fixtures.

Diff size

~790 lines removed across 17 files (≈60 inserted).

Test plan

  • uv run --group lint ruff check — clean
  • uv run --group lint ruff format --check — clean
  • uv run --group lint ty check src/ tests/ — clean
  • uv run --group lint codespell … — clean
  • uv run --group docs mdformat --check docs/ — clean
  • uv lock --check — clean
  • uv run pytest tests/lean_spec/subspecs/{sync,storage,api}/ — 207 passed
  • uv run pytest tests/api/ (conformance) — 29 passed

🤖 Generated with Claude Code

tcoratger and others added 5 commits May 21, 2026 18:02
The endpoints package only needs a docstring; route registration imports
each module directly from routes.py.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SyncProgress, SyncService.get_progress, and SyncService.reset have no
production callers; callers query store and peer_manager directly for
progress information. Removing them shrinks the service surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The two methods were one-line passthroughs to the internal _peers dict.
Renaming to a public attribute lets callers iterate or look up without
going through wrappers, matching standard Python practice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- has_block / has_state: no production caller checks existence without
  reading the payload; get_block / get_state already return None for
  missing entries.
- commit(): a single-statement wrapper around _conn.commit(); only test
  code used it. Tests now go through batch_write(), the canonical atomic
  write API.

Together these narrow the Database Protocol surface to the methods
production callers actually use.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The endpoint, its client helpers (fetch_finalized_block,
fetch_finalized_anchor) and the ApiServer.signed_block_getter plumbing
existed to ship the anchor SignedBlock alongside the finalized state for
checkpoint sync. They have no production callers — __main__.py uses
fetch_finalized_state alone, and Store.create_store no longer needs the
anchor SignedBlock to seed the store.

Removes the endpoint module, the route entry, the client helpers, the
FINALIZED_BLOCK_ENDPOINT constant, the signed_block_getter field on
ApiServer, and the corresponding test fixtures and test classes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcoratger
tcoratger merged commit 720ce03 into leanEthereum:main May 21, 2026
12 of 13 checks passed
tcoratger added a commit that referenced this pull request Jun 17, 2026
…or block (#974)

* feat(api): restore /lean/v0/blocks/finalized for checkpoint-sync anchor block

PR #713 added this endpoint so a checkpoint-syncing peer can fetch the
(state, signed block) anchor pair. PR #751 removed it as dead code
because it has no callers inside this repository. The callers are
external: the hive lean simulator gates every checkpoint-sync scenario
on this endpoint, and client implementations serve it for interop.
Since the removal shipped, all hive checkpoint-sync-based reqresp tests
fail for every client with a permanent 404 from the helper node.

Restores the endpoint and the injectable signed-block source on the API
server, since the fork-choice store only retains unsigned blocks. The
handler and field docstrings now name the external consumers so the
next dead-code sweep has the missing context.

* feat(sync): fetch the finalized block during checkpoint sync

The anchor builder previously rebuilt the anchor block from the header
embedded in the state with an empty body. The anchor root is the hash of
the full block, so whenever the finalized block carried attestations the
rebuilt root diverged from the finalized root the rest of the network
agrees on. This is the gap issue #712 originally described.

Fetch the real signed block from the finalized block endpoint and anchor
the store on it. The block is fetched before the state: it is small, so
a source that cannot serve it fails fast before the multi-megabyte state
download starts. A block that does not pair with the fetched state
raises, since that means the source advanced finalization between the
two requests and a retry is the fix.

This also gives the restored endpoint an in-repo production caller.

* feat(node): wire the finalized signed-block source into the API server

The API exposes /lean/v0/blocks/finalized so checkpoint-syncing peers can
fetch the (state, signed block) anchor pair, but the live node never
supplied a signed-block source, so the endpoint always returned 503.

The store and database retain only unsigned blocks, and the receiving peer
pairs the block with the finalized state without verifying its proof. Wrap
the looked-up block in an empty proof, matching the genesis anchor that no
proposer ever signed.

* docs: simplify doc

Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>

* docs: apply suggestions

Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>

* test(api,sync): address review feedback on finalized-block endpoint

Reviewer feedback from PR #974:

- Document the retryable block/state pairing mismatch in the anchor
  builder's Raises list, so callers know that case is worth a refetch.
- Pin full error messages with equality instead of match=/startswith
  fragments in the checkpoint-sync and anchor tests, matching the
  project's full-message assertion rule.
- Assert the fetched anchor block actually lands in the store, making
  the intent of the keyed-by-fetched-block change explicit.
- Replace the weak deserialize-and-not-None block test with a full
  object equality against the block rebuilt from the finalized state.
- Hoist the "wrap an unsigned block in an empty proof" helper and the
  store-backed signed-block getter into consensus_testing, reused
  across the API, sync, and anchor tests.
- Bind test servers to port 0 and read the OS-assigned port via a new
  ApiServer.bound_port, removing hardcoded ports that could collide
  under parallel runs.

* fix(vulture): whitelist the validator index-position model validator

The index/position invariant validator added in #1147 is invoked by
Pydantic during validation, so vulture cannot see the call and reports
it as dead code. Whitelist it alongside the other model validators.

---------

Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant