Deprecate blob lookup sync#9383
Merged
Merged
Conversation
Lookup sync no longer downloads or processes blobs. Block lookups and data-column (custody) lookups are unchanged, and range sync and blob serving (BlobsByRoot responses) are left fully intact. Deneb/electra blocks with blobs now resolve their blob data from the EL rather than via lookup sync; a block lookup for such a block completes on the block alone (ComponentRequests::NotNeeded). Tests that previously supplied blobs through the removed lookup path now make blobs available via the data availability checker, simulating EL provision. Removed: lookup blob request/response/processing path (blob_lookup_request, on_single_blob_response, send_blobs_for_processing, BlobRequestState, BlockProcessType::SingleBlob, SyncRequestId::SingleBlob, send_rpc_blobs / generate_rpc_blobs_process_fn / process_rpc_blobs, blobs_by_root.rs request module).
Blobs are no longer part of the consensus-layer data-availability check for Deneb/Electra; they are sourced from the execution layer (and still stored, served, and range-synced). `make_available` returns `NoData` for pre-PeerDAS blocks instead of waiting for blob sidecars, and `AvailableBlock` construction treats blobs as optional (only custody columns gate availability post-PeerDAS). Tests: removed the blob-EL-simulation scaffolding only needed while blocks waited for blobs; `empty_blobs_into_responses` asserts the block is available without blobs (pinned pre-PeerDAS); deleted `block_in_da_checker_skips_download` (premise gone). electra + fulu network tests pass; make lint-full clean.
Revert non-essential comment changes and drop the obsolete `empty_blobs_into_responses` coupling test (and its now-unused `blobs_id` helper) — it asserted the removed "missing blobs => coupling error" behavior.
dapplion
commented
Jun 1, 2026
Member
|
In theory this doesn't break (Even though we will probably get rid of that flag eventually). |
The deneb on_block fork_choice vectors invalid_data_unavailable, invalid_wrong_blobs_length and invalid_wrong_proofs_length expect the block to be rejected purely because its blob sidecars are unavailable or have a mismatched length. Lighthouse no longer enforces blob data-availability at the consensus layer for pre-PeerDAS (Deneb/Electra) blocks (blobs are sourced from the EL), so these blocks now import. The provided blobs still pass KZG verification, so distinguish this case (blob_success && block_imported) from genuinely-invalid vectors like invalid_incorrect_proof, which still fail.
Collaborator
Author
No, backfill uses range sync which are different codepaths |
Update comments to reflect changes in blob data availability enforcement.
Collaborator
Author
|
eserilev
reviewed
Jun 1, 2026
Member
|
This doesn't interfere with backfill sync |
…u) and empty_blobs_into_responses Restore two tests dropped during the blob-lookup deprecation: - block_in_da_checker_skips_download: migrate to new_after_fulu(). The premise (block stays in the da_checker as MissingComponents until its data arrives) no longer holds pre-PeerDAS, where blocks import immediately without blobs, but does hold post-Fulu where custody columns gate availability. Restores the insert_block_to_da_chain_and_assert_missing_componens helper unchanged. - empty_blobs_into_responses: restore the coupling test (plus blobs_id helper and BlobsByRangeRequestId import) asserting that a range request with blocks but no blobs now succeeds, since blobs are no longer required for availability.
Per review feedback, the blob_da_only_invalidity logic is equivalent to simply asserting that a block the spec marks valid must import. valid=false vectors that expect rejection purely due to unavailable/mismatched blobs now import (blobs are sourced from the EL), and we no longer treat that as a failure.
Per review, revert the diagnostic NotNeeded reason for the no-lookup-sync blob branch back to "outside da window".
eserilev
reviewed
Jun 1, 2026
eserilev
reviewed
Jun 1, 2026
| // no longer required for availability (they are sourced from the execution layer, except | ||
| // for historical sync and archive), so `valid=false` vectors that expect rejection purely | ||
| // due to unavailable/mismatched blobs now import, and we don't treat that as a failure. | ||
| if valid && !success { |
Member
There was a problem hiding this comment.
i think its okay to ignore the failure cases, they are all blob da related
eserilev
approved these changes
Jun 1, 2026
Member
eserilev
left a comment
eserilev
left a comment
There was a problem hiding this comment.
just one wrong comment, but other than that LGTM
…mments - fork_choice: keep the original DidntFail message verbatim; only the condition changes (assert valid blocks import, ignore blob-DA failure cases). - coupling test: blobs are not sourced from the EL; drop that claim.
Merge Queue Status
This pull request spent 28 minutes 28 seconds in the queue, including 27 minutes 34 seconds running CI. Required conditions to merge
|
This was referenced Jun 1, 2026
dapplion
added a commit
to dapplion/lighthouse
that referenced
this pull request
Jun 1, 2026
…ixes Reconciles unstable's sigp#9383 (Deprecate blob lookup sync) with this PR's rewritten lookup architecture by removing blob lookup from the new arch: Deneb/Electra block lookups complete on the block alone (the merged da_checker makes them available without blobs), and DataDownload::Blobs, blob_lookup_request, SyncRequestId::SingleBlob, BlockProcessType::SingleBlob, the process_rpc_blobs lookup cluster, and blob lookup tests are removed. Range-sync blobs and blob serving are kept.
dapplion
added a commit
to dapplion/lighthouse
that referenced
this pull request
Jun 1, 2026
dapplion
added a commit
to dapplion/lighthouse
that referenced
this pull request
Jun 1, 2026
Conflict resolutions: - network_beacon_processor/mod.rs: keep BlockProcessingResult re-export; drop unused FixedBlobSidecarList import (removed upstream). - sync/block_lookups/mod.rs: keep PR's classified BlockProcessingResult handling (Imported/ParentUnknown/Error); discard old BlockError match. - sync/tests/lookups.rs: drop the two new blob crypto-failure tests; blob lookup sync was deprecated/removed upstream (sigp#9383), deleting the corrupt_last_blob_* helpers they relied on. Column equivalents remain. - sync_methods.rs: drop BlockError::BlobNotRequired arm (variant removed upstream) from the exhaustive penalty match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue Addressed
Lookup sync is only for unfinalized blocks, which will never contains blobs in any network we support.