Skip to content

Commit 94a1446

Browse files
authored
Fix unexpected blob error and duplicate import in fetch blobs (#7541)
Getting this error on a non-PeerDAS network: ``` May 29 13:30:13.484 ERROR Error fetching or processing blobs from EL error: BlobProcessingError(AvailabilityCheck(Unexpected("empty blobs"))), block_root: 0x98aa3927056d453614fefbc79eb1f9865666d1f119d0e8aa9e6f4d02aa9395d9 ``` It appears we're passing an empty `Vec` to DA checker, because all blobs were already seen on gossip and filtered out, this causes a `AvailabilityCheckError::Unexpected("empty blobs")`. I've added equivalent unit tests for `getBlobsV1` to cover all the scenarios we test in `getBlobsV2`. This would have caught the bug if I had added it earlier. It also caught another bug which could trigger duplicate block import. Thanks Santito for reporting this! 🙏
1 parent 886ceb7 commit 94a1446

File tree

3 files changed

+536
-237
lines changed

3 files changed

+536
-237
lines changed

beacon_node/beacon_chain/src/fetch_blobs/mod.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ use types::{
4242
/// Result from engine get blobs to be passed onto `DataAvailabilityChecker` and published to the
4343
/// gossip network. The blobs / data columns have not been marked as observed yet, as they may not
4444
/// be published immediately.
45+
#[derive(Debug)]
4546
pub enum EngineGetBlobsOutput<T: BeaconChainTypes> {
4647
Blobs(Vec<GossipVerifiedBlob<T, DoNotObserve>>),
4748
/// A filtered list of custody data columns to be imported into the `DataAvailabilityChecker`.
@@ -163,9 +164,22 @@ async fn fetch_and_process_blobs_v1<T: BeaconChainTypes>(
163164
inc_counter(&metrics::BLOBS_FROM_EL_MISS_TOTAL);
164165
return Ok(None);
165166
} else {
167+
debug!(
168+
num_expected_blobs,
169+
num_fetched_blobs, "Received blobs from the EL"
170+
);
166171
inc_counter(&metrics::BLOBS_FROM_EL_HIT_TOTAL);
167172
}
168173

174+
if chain_adapter.fork_choice_contains_block(&block_root) {
175+
// Avoid computing sidecars if the block has already been imported.
176+
debug!(
177+
info = "block has already been imported",
178+
"Ignoring EL blobs response"
179+
);
180+
return Ok(None);
181+
}
182+
169183
let (signed_block_header, kzg_commitments_proof) = block
170184
.signed_block_header_and_kzg_commitments_proof()
171185
.map_err(FetchEngineBlobError::BeaconStateError)?;
@@ -197,13 +211,13 @@ async fn fetch_and_process_blobs_v1<T: BeaconChainTypes>(
197211
.collect::<Result<Vec<_>, _>>()
198212
.map_err(FetchEngineBlobError::GossipBlob)?;
199213

200-
if !blobs_to_import_and_publish.is_empty() {
201-
publish_fn(EngineGetBlobsOutput::Blobs(
202-
blobs_to_import_and_publish.clone(),
203-
));
214+
if blobs_to_import_and_publish.is_empty() {
215+
return Ok(None);
204216
}
205217

206-
debug!(num_fetched_blobs, "Processing engine blobs");
218+
publish_fn(EngineGetBlobsOutput::Blobs(
219+
blobs_to_import_and_publish.clone(),
220+
));
207221

208222
let availability_processing_status = chain_adapter
209223
.process_engine_blobs(

0 commit comments

Comments
 (0)