Tree-sync friendly lookup sync tests

[dapplion]

Issue Addressed

• Step 0 of the tree-sync roadmap Tree sync #7678

Current lookup sync tests are written in an explicit way that assume how the internals of lookup sync work. For example the test would do:

• Emit unknown block parent message
• Expect block request for X
• Respond with successful block request
• Expect block processing request for X
• Response with successful processing request
• etc..

This is unnecessarily verbose. And it will requires a complete re-write when something changes in the internals of lookup sync (has happened a few times, mostly for deneb and fulu).

What we really want to assert is:

• WHEN: we receive an unknown block parent message
• THEN: Lookup sync can sync that block
• ASSERT: Without penalizing peers, without unnecessary retries

Proposed Changes

Keep all existing tests and add new cases but written in the new style described above. The logic to serve and respond to request is in this function fn simulate https://github.com/dapplion/lighthouse/blob/2288a3aeb11164bb1960dc803f41696c984c69ff/beacon_node/network/src/sync/tests/lookups.rs#L301

• It controls peer behavior based on a CompleteStrategy where you can set for example "respond to BlocksByRoot requests with empty"
• It actually runs beacon processor messages running their clousures. Now sync tests actually import blocks, increasing the test coverage to the interaction of sync and the da_checker.
• To achieve the above the tests create real blocks with the test harness. To make the tests as fast as before, I disabled crypto with TestConfig

Along the way I found a couple bugs, which I documented on the diff.

Review guide

Look at lighthouse/beacon_node/network/src/sync/tests/lookups.rs directly (no diff).

Other changes are very minor and should not affect production paths

[dapplion]

Bug found on testing, lookups may be stuck given this sequence of events

[dapplion]

Minor bug, we need to remove the peer from the sync states (e.g. self.block_lookups) then inject the disconnect events. Otherwise we may send requests to peers that are already disconnected. I don't think there's risk of sync getting stuck if libp2p rejects sending messages to disconnected peers, but deserves a fix anyway.

[mergify]

This pull request has merge conflicts. Could you please resolve them @dapplion? 🙏

[dapplion]

@jimmygchen Thanks for the review! I've addressed all your feedback:

Already fixed:

• Documentation, naming improvements (max_seen_peers → seen_peers, with_process_result)
• Removed unused code (disable_fetch_blobs, external_harness field, random queue selection)
• Fixed panic message and typos

Just fixed (8d29e73):

• Renamed import_block_before_process → with_import_block_before_process for builder pattern consistency
• Fixed incomplete test documentation

All 12 review threads resolved. The 2 remaining threads are my own bug notes.

[jimmygchen]

Double check if we need these changes in this file.
Should we feature gate this?

[jimmygchen]

Potentialy delete this function in a future PR?

[jimmygchen]

Add documentation for naming conventions, e.g.

• .expect_ for mocking / setting up component behaviour
• .assert_ for making assertion on behaviour

[jimmygchen]

do we need to add the same check to all other places where is_valid_deposit_signature is called? or would it be cleaner to use feature so we don't have to pass VerifySignatures around and remember to add this check whereever we verify signature?

• lighthouse/consensus/state_processing/src/per_block_processing/process_operations.rs

  Line 889 in 765dcae

  if is_valid_deposit_signature(&deposit_data, spec).is_ok() {

• lighthouse/consensus/state_processing/src/per_epoch_processing/single_pass.rs

  Line 396 in 605ef8e

  } else if is_valid_deposit_signature(&deposit_data, spec).is_ok() {

[mergify]

Some required checks have failed. Could you please take a look @dapplion? 🙏

[dapplion]

The network-tests CI failure is a pre-existing flaky test unrelated to this PR's changes:

data_column_reconstruction_at_deadline
  left: ["column_reconstruction", "gossip_data_column_sidecar", "worker_freed"]
 right: ["gossip_data_column_sidecar", "worker_freed", "nothing_to_do"]

The test has a latent race: on the last loop iteration, enqueuing the final gossip column triggers reconstruction nearly simultaneously. The assert_event_journal_completes in the loop expects to drain gossip_data_column_sidecar first, but column_reconstruction can fire before it. All merge queue runs on unstable pass — the race just surfaces under different CI timing/load. Worth hardening the event ordering assertion in a follow-up.

[jimmygchen]

The network-tests CI failure is a pre-existing flaky test unrelated to this PR's changes:

data_column_reconstruction_at_deadline
  left: ["column_reconstruction", "gossip_data_column_sidecar", "worker_freed"]
 right: ["gossip_data_column_sidecar", "worker_freed", "nothing_to_do"]

The test has a latent race: on the last loop iteration, enqueuing the final gossip column triggers reconstruction nearly simultaneously. The assert_event_journal_completes in the loop expects to drain gossip_data_column_sidecar first, but column_reconstruction can fire before it. All merge queue runs on unstable pass — the race just surfaces under different CI timing/load. Worth hardening the event ordering assertion in a follow-up.

are you @dapplion

[jimmygchen]

Having this condition in the same test feels a bit confusing, as we're testing different outcomes and in the case of fake_crypto scenario, it doesn't match what the test name suggests,

I prefer something like:

  #[tokio::test]
  #[cfg(feature = "fake_crypto")]
  async fn block_signature_not_checked_with_fake_crypto() { ... }

  #[tokio::test]
  #[cfg(not(feature = "fake_crypto"))]
  async fn invalid_block_signature_fails_processing() { ... }

but yeah this is a minor style issue

[jimmygchen]

Hey @dapplion

Nice work! This is a great improvement to the existing tests, which is becoming hard to maintain and will break once we change how sync works.

It looks pretty solid now and we can keep iterating over the test interfaces as we add more tests.

[mergify]

Merge Queue Status

Rule: default

---

• ✅ Entered queue — 2026-02-13 03:52 UTC
• ✅ Checks passed · on draft merge queue: embarking unstable (c59e4a0) and #8592 together #8812
• ✅ Merged — 2026-02-13 04:24 UTC · at 415e89edc872fdd841ef59febb9b3461396b77c5

This pull request spent 32 minutes 9 seconds in the queue, including 29 minutes 48 seconds running CI.

Required conditions to merge

• check-success=local-testnet-success
• check-success=test-suite-success