feat(api): restore /lean/v0/blocks/finalized for checkpoint-sync anchor block - #974
Conversation
…or 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.
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.
b1b6491 to
5a8e346
Compare
…zed-endpoint # Conflicts: # src/lean_spec/node/api/routes.py # tests/node/api/endpoints/test_blocks.py
…zed-endpoint # Conflicts: # src/lean_spec/node/api/routes.py # tests/node/api/endpoints/conftest.py # tests/node/api/test_server.py
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.
…zed-endpoint # Conflicts: # src/lean_spec/node/sync/checkpoint_sync.py
| def test_ssz_deserializes(self, server_url: str) -> None: | ||
| """Finalized block SSZ bytes deserialize to a valid SignedBlock object.""" | ||
| response = get_finalized_block(server_url) | ||
| signed_block = SignedBlock.decode_bytes(response.content) | ||
| assert signed_block is not None |
There was a problem hiding this comment.
Maybe we can remove this test since in the next one we already assert on signed_block, so no need here to assert that it is not None
| ) | ||
| state = State.decode_bytes(state_response.content) | ||
|
|
||
| assert signed_block.block.state_root == hash_tree_root(state) |
There was a problem hiding this comment.
Here this is probably better to assert on the full signed_block with assert signed_block = ....
tcoratger
left a comment
tcoratger
left a comment
There was a problem hiding this comment.
Thanks for restoring this — and the anchor-root fix (fetching the real signed block instead of rebuilding it from the header with an empty body) is a really nice catch 🙌
I left a few small, totally optional suggestions inline, mostly around the tests. Nothing blocking — the source side looks good to me. The main themes:
- a couple of exception assertions use
match=/startswith, which we usually swap for a full==on the message - the "wrap a block in an empty proof" helper is repeated in a few places, so maybe a small shared helper could simplify the setup
- a couple of hardcoded ports overlap, which can get flaky under parallel runs
Feel free to take or leave any of these! 🙂
| """Tests for the /lean/v0/blocks/finalized endpoint.""" | ||
|
|
||
| @staticmethod | ||
| def _signed_block_getter_for(store: Store) -> Callable[[Bytes32], SignedBlock | None]: |
There was a problem hiding this comment.
Small thought: this little "look up a block, wrap it in an empty proof" helper shows up in a few places now (here, the endpoints conftest, test_checkpoint_sync, test_anchor's _signed_genesis_block, and the getter in node.py). Maybe we could move one version into consensus_testing and reuse it everywhere? Would trim the setup a bit. Totally optional though!
|
|
||
| async def test_returns_503_without_signed_block_source(self, base_store: Store) -> None: | ||
| """Endpoint returns 503 when no signed-block source is configured.""" | ||
| config = ApiServerConfig(port=15075) |
There was a problem hiding this comment.
Tiny heads-up: this port (15075) is also used in tests/node/sync/test_checkpoint_sync.py, and 15071 is reused within this file. Under xdist they could occasionally race for the same bind. If it's easy, port=0 (we already do that in test_node.py) lets the OS pick a free port and sidesteps the collision. No worries if it feels out of scope here.
| await server.start() | ||
|
|
||
| try: | ||
| with pytest.raises(CheckpointSyncError, match="HTTP error 503"): |
There was a problem hiding this comment.
Minor: we usually assert the full message with == rather than a match= fragment, so the rest of the string can't drift unnoticed. Same idea for the startswith(...) one a few lines up (line 273), if its message is stable. Something like:
with pytest.raises(CheckpointSyncError) as exception_info:
await fetch_finalized_block("http://127.0.0.1:15076")
assert str(exception_info.value) == "HTTP error 503: ..."| new_callable=AsyncMock, | ||
| return_value=mismatched_block, | ||
| ), | ||
| pytest.raises(CheckpointSyncError, match="mismatch"), |
There was a problem hiding this comment.
Same small thing as in the checkpoint-sync tests — could we assert the full message here too? e.g. == "anchor block / state mismatch; source advanced finalization between requests, retry". Keeps the exact wording pinned. 🙂
| assert anchor.initial_status.finalized == anchor.store.latest_finalized | ||
| # The anchor is keyed by the fetched block's root, so the store's | ||
| # finalized checkpoint matches the root the network agrees on. | ||
| assert anchor.store.latest_finalized.root == hash_tree_root(signed_block.block) |
There was a problem hiding this comment.
Nice assertion! One small idea: since the heart of this change is that the store is keyed by the fetched block, maybe also assert the block actually landed in the store, e.g. assert hash_tree_root(signed_block.block) in anchor.store.blocks? Just makes the intent extra explicit.
| @@ -96,6 +96,10 @@ async def from_checkpoint( | |||
| CheckpointSyncError: For every failure mode covering transport, | |||
There was a problem hiding this comment.
Small doc nit (feel free to fold into your other docstring tweaks): the method now also raises on the block/state pairing mismatch, so it might be worth adding that to the Raises: list here — it's a retryable case, so handy for callers to know about.
Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
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.
|
@tcoratger all comments were addressed 🫡 |
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.
2 participants
Summary
Restores the
/lean/v0/blocks/finalizedendpoint that #713 introduced and #751 removed as dead code, makes the node's checkpoint-sync client consume it, and wires the live node to serve it.The endpoint's consumers were external, which is why the dead-code sweep missed them:
signed_block_getterparameter onApiServer(lean_spec_client_runner.py) and its readiness probe polls/lean/v0/blocks/finalized(helper.rs). Since refactor: dead-code sweep in sync / api / storage subspecs #751 shipped, the probe 404s forever and all checkpoint-sync-based reqresp tests fail for every client on hive.leanroadmap.org: 17 of 21 tests per client in the latest reqresp run. These failures are easy to miss in the UI, since they happen during setup, before the client-under-test starts, so they disappear from per-client filtered views.Restore the endpoint (server side)
This branch was rebased onto the
node/apirefactor that landed in main (handlers.py/context.py/responses.py, replacing the oldroutes.py+endpoints/modules). The endpoint is integrated into that new structure rather than the original file layout:node/api/handlers.py: afinalized_blockhandler method, alongside the other endpoint handlers.node/api/context.py: an optionalsigned_block_getteronApiContext, with arequire_signed_block_getteraccessor that raises 503 when unset. The fork-choice store retains only unsigned blocks, so the embedding node injects the signed-block source.node/api/server.py: thesigned_block_getterfield onApiServer, threaded into the context, and the route registration. The injection interface matches feat(api,sync): add /lean/v0/blocks/finalized for checkpoint-sync anchor block #713 and hive's feature detection.Consume it during checkpoint sync (client side)
Anchor.from_checkpointpreviously rebuilt the anchor block fromstate.latest_block_headerwith an empty body.create_storekeys the head, the checkpoints, and the block map byhash_tree_root(anchor_block), so whenever the finalized block carried attestations the rebuilt anchor root diverged from the finalized root the rest of the network agrees on. This is the gap #712 originally described.The new flow, replacing the header reconstruction entirely:
/lean/v0/blocks/finalized. The block is fetched first because it is small: a source that cannot serve it (503/404) aborts checkpoint sync before the multi-megabyte state download starts./lean/v0/states/finalizedand run the existing structural and genesis-time checks.block.state_root == hash_tree_root(state). A mismatch raises, since it means the source advanced finalization between the two requests and a retry is the fix.create_store(state, block), now keyed by the network's true finalized root.Wire the live node to serve it
The server accepted a
signed_block_getterbut the node never supplied one, so on a real node the endpoint always returned 503 (only the test/conformance servers wired a source).Node.from_genesisnow injects a source that looks up the block by root in the live store and wraps it in an empty proof.The empty proof is intentional, not a placeholder:
Anchor.from_checkpoint) uses onlysigned_block.blockand never verifies the proof, so an empty one is sufficient for the anchor pair. This mirrors the genesis anchor that no proposer ever signed.This is a deliberate departure from #713, which served a mock proposer signature (in the older
BlockSignaturesenvelope, since replaced by the mergedMultiMessageAggregate) and whose comment said real nodes should retain the actual signed block for the finalized root. With no consumer verifying the proof, the empty proof avoids that retention machinery until a verifier exists.Tests
tests/node/api/endpoints/test_blocks.py: contract tests (200, content type, SSZ round-trip, block state-root matches the finalized state's hash tree root).tests/node/api/test_server.py: 503 without store, 503 without signed-block source, 404 when the source has no block, 200 + anchor-root match.tests/node/test_node.py: the node wires the signed-block source (returns the finalized block in an empty proof, returnsNonefor an unknown root).tests/node/sync/test_checkpoint_sync.py: transport/HTTP/corrupt-SSZ error wrapping for the block fetch, plus a live-server round-trip and the 503 path.tests/node/test_anchor.py: anchor keyed by the fetched block's root, abort on block-fetch failure, abort on state-fetch failure, raise on state/block pairing mismatch.tests/node/api/endpoints/conftest.py: the conformance server wires a signed-block source that wraps the store's anchor block with an empty proof.just checkand the node/api test trees are green.