Fetch blobs from EL prior to block verification #6600

michaelsproul · 2024-11-21T02:08:05Z

Proposed Changes

Optimise fetch_blobs significantly, by fetching blobs from the EL prior to consensus and execution verification of the block.

We had noticed that we weren't getting many hits with fetch blobs, and this was because blobs were almost always arriving on gossip prior to us requesting them. Only a few times an hour would the fetch_blobs logic actually fire.

With this change I'm seeing much more frequent hits, without a substantial increase in publication bandwidth. In the last 30 mins running on mainnet there have been 116 hits, and 156 individual blobs published (out of 395 fetched).

Data here: https://docs.google.com/spreadsheets/d/1ZJIYbOPwNGa_veqUC0ywsOdzFYvh4aqJLMYqFoisA_E/edit?usp=sharing

This does imply that we're publishing around 35% of all blobs! But this will likely come down as more nodes chip in to publishing.

dapplion · 2024-11-21T02:19:13Z

Any security considerations on triggering this logic before validating the block? The most damage a proposer can do is waste bandwidth on a bad proposal. This does not seem like a big issue and can be done anyway regardless of fetch_blobs.

Else the experimental results look great

michaelsproul · 2024-11-21T03:59:05Z

We're doing this after gossip validation of the block, so we know that the proposer's signature is valid and they are a legitimate proposer for the slot.

Unless the proposer slashes themselves, the blob versioned hashes in the block header are the "true" (valid) versioned hashes for this slot. Alternatively the block could be completely invalid (but not slashable), in which case we will reject it upon completion of block processing.

As part of fetch_blobs we run gossip blob validation:

lighthouse/beacon_node/beacon_chain/src/fetch_blobs.rs

Lines 131 to 148 in 6e1945f

    
           // Gossip verify blobs before publishing. This prevents blobs with invalid KZG proofs from 
        
           // the EL making it into the data availability checker. We do not immediately add these 
        
           // blobs to the observed blobs/columns cache because we want to allow blobs/columns to arrive on gossip 
        
           // and be accepted (and propagated) while we are waiting to publish. Just before publishing 
        
           // we will observe the blobs/columns and only proceed with publishing if they are not yet seen. 
        
           let blobs_to_import_and_publish = fixed_blob_sidecar_list 
        
               .iter() 
        
               .filter_map(|opt_blob| { 
        
                   let blob = opt_blob.as_ref()?; 
        
                   match GossipVerifiedBlob::<T, DoNotObserve>::new(blob.clone(), blob.index, &chain) { 
        
                       Ok(verified) => Some(Ok(verified)), 
        
                       // Ignore already seen blobs. 
        
                       Err(GossipBlobError::RepeatBlob { .. }) => None, 
        
                       Err(e) => Some(Err(e)), 
        
                   } 
        
               }) 
        
               .collect::<Result<Vec<_>, _>>() 
        
               .map_err(FetchEngineBlobError::GossipBlob)?;

So if they are malformed (e.g. bad KZG proof), they will be rejected at this point.

TL;DR on the whole I think it's security-equivalent to processing blobs on gossip:

We have a valid signature from the proposer committing to the blobs.
We verify the blobs themselves before importing them.

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

dapplion · 2024-11-22T05:21:20Z

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

+        self.executor.spawn(
+            async move {
+                self_clone
+                    .fetch_engine_blobs_and_publish(block_clone, block_root, publish_blobs)


Since this is running as a task, it's no longer bound by the beacon processor queue. Could someone spam gossip blocks and cause a lot of fetch blobs work?

They would need to be beacon blocks with valid signatures, and this is a linear factor, so it can't really blow up much beyond the number of threads allocated to the beacon processor. E.g. if we have 16 threads in the BP, we might end up with 32 running tasks max, which are mostly I/O bound and should be handled just fine by Tokio.

We do this in a few other places, like when we check the payload with the EL:

lighthouse/beacon_node/beacon_chain/src/block_verification.rs

Lines 1407 to 1416 in 6329042

// Spawn the payload verification future as a new task, but don't wait for it to complete.

// The `payload_verification_future` will be awaited later to ensure verification completed

// successfully.

let payload_verification_handle = chain

.task_executor

.spawn_handle(

payload_verification_future,

"execution_payload_verification",

)

.ok_or(BeaconChainError::RuntimeShutdown)?;

Squashed commit of the following: commit 5f563ef Author: Michael Sproul <michael@sigmaprime.io> Date: Fri Nov 22 12:33:10 2024 +1100 Run fetch blobs in parallel with block import commit 3cfe9df Author: Michael Sproul <michael@sigmaprime.io> Date: Thu Nov 21 10:46:34 2024 +1100 Fetch blobs from EL prior to block verification

michaelsproul · 2024-11-27T12:27:42Z

@mergify queue

mergify · 2024-11-27T12:28:37Z

🛑 The pull request has been removed from the queue `default`

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

michaelsproul · 2024-11-27T21:02:38Z

@mergify requeue

mergify · 2024-11-27T21:02:45Z

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

mergify · 2024-11-27T21:02:46Z

🛑 The pull request has been removed from the queue `default`

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

Squashed commit of the following: commit 5f563ef Author: Michael Sproul <michael@sigmaprime.io> Date: Fri Nov 22 12:33:10 2024 +1100 Run fetch blobs in parallel with block import commit 3cfe9df Author: Michael Sproul <michael@sigmaprime.io> Date: Thu Nov 21 10:46:34 2024 +1100 Fetch blobs from EL prior to block verification

michaelsproul · 2024-11-28T06:10:12Z

@mergify queue

mergify · 2024-11-28T06:10:16Z

🛑 The pull request has been removed from the queue `default`

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

michaelsproul · 2024-11-28T07:11:53Z

@mergify requeue

mergify · 2024-11-28T07:11:57Z

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

mergify · 2024-11-28T07:11:58Z

🛑 The pull request has been removed from the queue `default`

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

michaelsproul · 2024-11-28T07:46:19Z

@mergify requeue

mergify · 2024-11-28T07:46:47Z

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

mergify · 2024-11-28T07:46:48Z

🛑 The pull request has been removed from the queue `default`

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

michaelsproul · 2024-11-28T10:33:30Z

@mergify requeue

mergify · 2024-11-28T10:33:40Z

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

mergify · 2024-11-28T10:33:41Z

🛑 The pull request has been removed from the queue `default`

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

michaelsproul · 2024-11-28T12:34:47Z

@mergify requeue

mergify · 2024-11-28T12:34:57Z

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

mergify · 2024-11-28T12:34:58Z

🛑 The pull request has been removed from the queue `default`

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

michaelsproul · 2024-11-29T02:47:10Z

@mergify requeue

mergify · 2024-11-29T02:47:14Z

❌ This pull request head commit has not been previously disembarked from queue.

michaelsproul · 2024-11-29T02:48:55Z

@mergify queue

mergify · 2024-11-29T02:49:06Z

✅ The pull request has been merged automatically

The pull request has been merged automatically at 1c8161f

michaelsproul · 2024-11-29T03:46:14Z

THANK FUCK.

Fetch blobs from EL prior to block verification

3cfe9df

michaelsproul added ready-for-review The code is ready for review optimization Something to make Lighthouse run more efficiently. labels Nov 21, 2024

michaelsproul added v6.0.0 New major release for hierarchical state diffs waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Nov 21, 2024

michaelsproul commented Nov 21, 2024

View reviewed changes

beacon_node/network/src/network_beacon_processor/gossip_methods.rs Outdated Show resolved Hide resolved

Run fetch blobs in parallel with block import

5f563ef

dapplion reviewed Nov 22, 2024

View reviewed changes

michaelsproul added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Nov 22, 2024

michaelsproul mentioned this pull request Nov 25, 2024

Release v6.0.0 #6605

Merged

dapplion approved these changes Nov 27, 2024

View reviewed changes

michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Nov 27, 2024

mergify bot added a commit that referenced this pull request Nov 27, 2024

Merge of #6600

eac4838

mergify bot mentioned this pull request Nov 27, 2024

merge queue: embarking unstable (720f596) and #6600 together #6620

Closed

5 tasks

mergify bot added a commit that referenced this pull request Nov 27, 2024

Merge of #6600

e11b73f

mergify bot mentioned this pull request Nov 27, 2024

merge queue: embarking unstable (720f596) and #6600 together #6624

Closed

5 tasks

Merge branch 'unstable' into fetch-blobs-earlier

a0a35fe

Merge branch 'unstable' into fetch-blobs-earlier

7b38603

mergify bot merged commit 1c8161f into sigp:unstable Nov 29, 2024
29 checks passed

michaelsproul deleted the fetch-blobs-earlier branch November 29, 2024 03:46

jimmygchen mentioned this pull request Jan 6, 2025

Fix data columns not persisting for PeerDAS due to a getBlobs race condition #6756

Merged

1 task

	// Spawn the payload verification future as a new task, but don't wait for it to complete.
	// The `payload_verification_future` will be awaited later to ensure verification completed
	// successfully.
	let payload_verification_handle = chain
	.task_executor
	.spawn_handle(
	payload_verification_future,
	"execution_payload_verification",
	)
	.ok_or(BeaconChainError::RuntimeShutdown)?;

Fetch blobs from EL prior to block verification #6600

Fetch blobs from EL prior to block verification #6600

Uh oh!

Conversation

michaelsproul commented Nov 21, 2024

Proposed Changes

Uh oh!

dapplion commented Nov 21, 2024

Uh oh!

michaelsproul commented Nov 21, 2024

Uh oh!

Uh oh!

dapplion Nov 22, 2024

Choose a reason for hiding this comment

Uh oh!

michaelsproul Nov 22, 2024

Choose a reason for hiding this comment

Uh oh!

michaelsproul commented Nov 27, 2024

Uh oh!

mergify bot commented Nov 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request has been removed from the queue default

Uh oh!

michaelsproul commented Nov 27, 2024

Uh oh!

mergify bot commented Nov 27, 2024

✅ This pull request will be re-embarked automatically

Uh oh!

mergify bot commented Nov 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request has been removed from the queue default

Uh oh!

michaelsproul commented Nov 28, 2024

Uh oh!

mergify bot commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request has been removed from the queue default

Uh oh!

michaelsproul commented Nov 28, 2024

Uh oh!

mergify bot commented Nov 28, 2024

✅ This pull request will be re-embarked automatically

Uh oh!

mergify bot commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request has been removed from the queue default

Uh oh!

michaelsproul commented Nov 28, 2024

Uh oh!

mergify bot commented Nov 28, 2024

✅ This pull request will be re-embarked automatically

Uh oh!

mergify bot commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request has been removed from the queue default

Uh oh!

michaelsproul commented Nov 28, 2024

Uh oh!

mergify bot commented Nov 28, 2024

✅ This pull request will be re-embarked automatically

Uh oh!

mergify bot commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request has been removed from the queue default

Uh oh!

michaelsproul commented Nov 28, 2024

Uh oh!

mergify bot commented Nov 28, 2024

✅ This pull request will be re-embarked automatically

Uh oh!

mergify bot commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request has been removed from the queue default

Uh oh!

michaelsproul commented Nov 29, 2024

Uh oh!

mergify bot commented Nov 29, 2024

❌ This pull request head commit has not been previously disembarked from queue.

Uh oh!

mergify bot commented Nov 27, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented Nov 27, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented Nov 28, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented Nov 28, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented Nov 28, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented Nov 28, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented Nov 28, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented Nov 29, 2024 •

edited

Loading