From 4406eff43a6f7d9f2e45a80625b57f353a2a011c Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Mon, 18 May 2026 17:35:44 +0200 Subject: [PATCH] refactor(sync): drop _SyncStoreView adapter, make SyncService implement StoreView _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) --- src/lean_spec/subspecs/sync/backfill_sync.py | 14 +------ src/lean_spec/subspecs/sync/service.py | 40 ++++++------------- .../subspecs/sync/test_backfill_sync.py | 11 +---- 3 files changed, 15 insertions(+), 50 deletions(-) diff --git a/src/lean_spec/subspecs/sync/backfill_sync.py b/src/lean_spec/subspecs/sync/backfill_sync.py index ed7c53d7b..c2a95ef2c 100644 --- a/src/lean_spec/subspecs/sync/backfill_sync.py +++ b/src/lean_spec/subspecs/sync/backfill_sync.py @@ -55,24 +55,12 @@ class StoreView(Protocol): - """ - Read-only view of forkchoice state used by backfill. - - Used to skip blocks already in the Store and to find the highest known - canonical slot for gap detection. - - Decouples backfill from the concrete Store class. - Lets tests supply a tiny in-memory implementation. - """ + """Read-only view of forkchoice state used by backfill.""" def has_root(self, root: Bytes32) -> bool: """Return True if the block root is present in the Store.""" ... - def finalized_slot(self) -> Slot: - """Return the slot of the latest finalized checkpoint.""" - ... - def head_slot(self) -> Slot: """Return the slot of the current canonical head.""" ... diff --git a/src/lean_spec/subspecs/sync/service.py b/src/lean_spec/subspecs/sync/service.py index 60428b0a2..b9dafc2bf 100644 --- a/src/lean_spec/subspecs/sync/service.py +++ b/src/lean_spec/subspecs/sync/service.py @@ -66,30 +66,6 @@ logger = logging.getLogger(__name__) -@dataclass(slots=True) -class _SyncStoreView: - """StoreView adapter delegating to the live SyncService.store reference. - - Wraps a getter so updates to ``SyncService.store`` (assigned after each - block is processed) are observed by backfill without re-wiring. - """ - - _get_store: Callable[[], Store] - - def has_root(self, root: Bytes32) -> bool: - """Return True if the block root is present in the Store.""" - return root in self._get_store().blocks - - def finalized_slot(self) -> Slot: - """Return the slot of the latest finalized checkpoint.""" - return self._get_store().latest_finalized.slot - - def head_slot(self) -> Slot: - """Return the slot of the current canonical head.""" - store = self._get_store() - return store.blocks[store.head].slot - - def _ancestor_set(blocks: dict[Bytes32, Block], head: Bytes32) -> set[Bytes32]: """Walk parent links from head and collect every reachable block root.""" seen: set[Bytes32] = set() @@ -303,14 +279,14 @@ def _init_components(self) -> None: """ # BackfillSync handles fetching missing parent blocks from peers. # - # It needs network access to request blocks and the cache to store them. - # The store view is a thin adapter that always reads the current - # store reference, since we replace `self.store` after each block. + # SyncService implements the StoreView protocol directly. + # Backfill reads `self.store` through us, so the live reference is + # always observed even as we reassign it after each block. self._backfill = BackfillSync( peer_manager=self.peer_manager, block_cache=self.block_cache, network=self.network, - store_view=_SyncStoreView(_get_store=lambda: self.store), + store_view=self, ) # HeadSync processes incoming gossip blocks and coordinates backfill. @@ -406,6 +382,14 @@ def state(self) -> SyncState: """Current sync state.""" return self._state + def has_root(self, root: Bytes32) -> bool: + """Return True if the block root is present in the current store.""" + return root in self.store.blocks + + def head_slot(self) -> Slot: + """Return the slot of the current canonical head.""" + return self.store.blocks[self.store.head].slot + def get_progress(self) -> SyncProgress: """ Get current sync progress. diff --git a/tests/lean_spec/subspecs/sync/test_backfill_sync.py b/tests/lean_spec/subspecs/sync/test_backfill_sync.py index 968606a5a..671bee094 100644 --- a/tests/lean_spec/subspecs/sync/test_backfill_sync.py +++ b/tests/lean_spec/subspecs/sync/test_backfill_sync.py @@ -28,12 +28,11 @@ class FakeStoreView: """In-memory StoreView used to drive backfill tests. Concrete implementation. Avoids MagicMock so tests fail loudly when - fields drift. Tests mutate `known_roots`, `head`, and `finalized` directly. + fields drift. Tests mutate `known_roots` and `head` directly. """ known_roots: set[Bytes32] = field(default_factory=set) head: Slot = field(default_factory=lambda: Slot(0)) - finalized: Slot = field(default_factory=lambda: Slot(0)) def has_root(self, root: Bytes32) -> bool: """Return True if the root has been registered with this view.""" @@ -43,10 +42,6 @@ def head_slot(self) -> Slot: """Return the head slot stored on this view.""" return self.head - def finalized_slot(self) -> Slot: - """Return the finalized slot stored on this view.""" - return self.finalized - @pytest.fixture def network() -> MockNetworkRequester: @@ -376,7 +371,6 @@ async def test_store_awareness_skips_known_parents( parent_root = Bytes32(b"\x01" * 32) store_view.known_roots.add(parent_root) store_view.head = Slot(10) - store_view.finalized = Slot(10) # Child is received above the head. child = make_signed_block( @@ -406,9 +400,8 @@ async def test_range_sync_triggered_by_gap_above_head( Floor is the head slot, not the finalized slot: slots above finalized but at or below head are already canonical for us and are not refetched. """ - # Store head is at slot 49, finalized is older at slot 10. + # Store head is at slot 49. store_view.head = Slot(49) - store_view.finalized = Slot(10) store_view.known_roots.add(Bytes32.zero()) # Pre-fill the parent in the network at slot 50.