Skip to content

Commit d0d0422

Browse files
tcoratgerclaude
andauthored
refactor(sync): drop _SyncStoreView adapter, make SyncService implement StoreView (leanEthereum#730)
_SyncStoreView was a dataclass wrapping a lambda wrapping a getter on SyncService.store. Three layers of indirection to read two fields. SyncService can satisfy the StoreView protocol structurally with two short methods, removing the wrapper without growing the call surface. Also drops the finalized_slot method from the StoreView protocol. It was defined on the protocol, implemented on _SyncStoreView and on FakeStoreView, and never called from backfill_sync. Pure protocol bloat. Two dead set-but-not-read writes in tests removed too. The StoreView protocol itself stays. It breaks a real circular dependency (BackfillSync is imported by SyncService, so the field type cannot reference SyncService directly), keeps the test seam (FakeStoreView is 5 lines, no real Store construction needed), and documents BackfillSync's exact dependency on forkchoice state in 8 lines. Net: -28 lines, zero behavior change. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 75dfe92 commit d0d0422

3 files changed

Lines changed: 15 additions & 50 deletions

File tree

src/lean_spec/subspecs/sync/backfill_sync.py

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,24 +55,12 @@
5555

5656

5757
class StoreView(Protocol):
58-
"""
59-
Read-only view of forkchoice state used by backfill.
60-
61-
Used to skip blocks already in the Store and to find the highest known
62-
canonical slot for gap detection.
63-
64-
Decouples backfill from the concrete Store class.
65-
Lets tests supply a tiny in-memory implementation.
66-
"""
58+
"""Read-only view of forkchoice state used by backfill."""
6759

6860
def has_root(self, root: Bytes32) -> bool:
6961
"""Return True if the block root is present in the Store."""
7062
...
7163

72-
def finalized_slot(self) -> Slot:
73-
"""Return the slot of the latest finalized checkpoint."""
74-
...
75-
7664
def head_slot(self) -> Slot:
7765
"""Return the slot of the current canonical head."""
7866
...

src/lean_spec/subspecs/sync/service.py

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -66,30 +66,6 @@
6666
logger = logging.getLogger(__name__)
6767

6868

69-
@dataclass(slots=True)
70-
class _SyncStoreView:
71-
"""StoreView adapter delegating to the live SyncService.store reference.
72-
73-
Wraps a getter so updates to ``SyncService.store`` (assigned after each
74-
block is processed) are observed by backfill without re-wiring.
75-
"""
76-
77-
_get_store: Callable[[], Store]
78-
79-
def has_root(self, root: Bytes32) -> bool:
80-
"""Return True if the block root is present in the Store."""
81-
return root in self._get_store().blocks
82-
83-
def finalized_slot(self) -> Slot:
84-
"""Return the slot of the latest finalized checkpoint."""
85-
return self._get_store().latest_finalized.slot
86-
87-
def head_slot(self) -> Slot:
88-
"""Return the slot of the current canonical head."""
89-
store = self._get_store()
90-
return store.blocks[store.head].slot
91-
92-
9369
def _ancestor_set(blocks: dict[Bytes32, Block], head: Bytes32) -> set[Bytes32]:
9470
"""Walk parent links from head and collect every reachable block root."""
9571
seen: set[Bytes32] = set()
@@ -303,14 +279,14 @@ def _init_components(self) -> None:
303279
"""
304280
# BackfillSync handles fetching missing parent blocks from peers.
305281
#
306-
# It needs network access to request blocks and the cache to store them.
307-
# The store view is a thin adapter that always reads the current
308-
# store reference, since we replace `self.store` after each block.
282+
# SyncService implements the StoreView protocol directly.
283+
# Backfill reads `self.store` through us, so the live reference is
284+
# always observed even as we reassign it after each block.
309285
self._backfill = BackfillSync(
310286
peer_manager=self.peer_manager,
311287
block_cache=self.block_cache,
312288
network=self.network,
313-
store_view=_SyncStoreView(_get_store=lambda: self.store),
289+
store_view=self,
314290
)
315291

316292
# HeadSync processes incoming gossip blocks and coordinates backfill.
@@ -406,6 +382,14 @@ def state(self) -> SyncState:
406382
"""Current sync state."""
407383
return self._state
408384

385+
def has_root(self, root: Bytes32) -> bool:
386+
"""Return True if the block root is present in the current store."""
387+
return root in self.store.blocks
388+
389+
def head_slot(self) -> Slot:
390+
"""Return the slot of the current canonical head."""
391+
return self.store.blocks[self.store.head].slot
392+
409393
def get_progress(self) -> SyncProgress:
410394
"""
411395
Get current sync progress.

tests/lean_spec/subspecs/sync/test_backfill_sync.py

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,11 @@ class FakeStoreView:
2828
"""In-memory StoreView used to drive backfill tests.
2929
3030
Concrete implementation. Avoids MagicMock so tests fail loudly when
31-
fields drift. Tests mutate `known_roots`, `head`, and `finalized` directly.
31+
fields drift. Tests mutate `known_roots` and `head` directly.
3232
"""
3333

3434
known_roots: set[Bytes32] = field(default_factory=set)
3535
head: Slot = field(default_factory=lambda: Slot(0))
36-
finalized: Slot = field(default_factory=lambda: Slot(0))
3736

3837
def has_root(self, root: Bytes32) -> bool:
3938
"""Return True if the root has been registered with this view."""
@@ -43,10 +42,6 @@ def head_slot(self) -> Slot:
4342
"""Return the head slot stored on this view."""
4443
return self.head
4544

46-
def finalized_slot(self) -> Slot:
47-
"""Return the finalized slot stored on this view."""
48-
return self.finalized
49-
5045

5146
@pytest.fixture
5247
def network() -> MockNetworkRequester:
@@ -376,7 +371,6 @@ async def test_store_awareness_skips_known_parents(
376371
parent_root = Bytes32(b"\x01" * 32)
377372
store_view.known_roots.add(parent_root)
378373
store_view.head = Slot(10)
379-
store_view.finalized = Slot(10)
380374

381375
# Child is received above the head.
382376
child = make_signed_block(
@@ -406,9 +400,8 @@ async def test_range_sync_triggered_by_gap_above_head(
406400
Floor is the head slot, not the finalized slot: slots above finalized
407401
but at or below head are already canonical for us and are not refetched.
408402
"""
409-
# Store head is at slot 49, finalized is older at slot 10.
403+
# Store head is at slot 49.
410404
store_view.head = Slot(49)
411-
store_view.finalized = Slot(10)
412405
store_view.known_roots.add(Bytes32.zero())
413406

414407
# Pre-fill the parent in the network at slot 50.

0 commit comments

Comments
 (0)