gnosis/lighthouse
1
0
Fork 0
mirror of https://github.com/sigp/lighthouse.git synced 2026-03-07 02:31:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix wrong custody column count for lookup blocks (#7281)

Browse Source 
Fixes
- https://github.com/sigp/lighthouse/issues/7278


  Don't assume 0 columns for `RpcBlockInner::Block`
This commit is contained in:
Lion - dapplion
2025-04-11 19:00:57 -03:00
committed by GitHub
parent 70f8ab9a6f
commit be68dd24d0
13 changed files with 98 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
beacon_node/beacon_chain/src/block_verification.rs
View File

@@ -1267,40 +1267,6 @@ impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for SignatureVerifiedBloc
}
}
impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for Arc<SignedBeaconBlock<T::EthSpec>> {
/// Verifies the `SignedBeaconBlock` by first transforming it into a `SignatureVerifiedBlock`
/// and then using that implementation of `IntoExecutionPendingBlock` to complete verification.
fn into_execution_pending_block_slashable(
self,
block_root: Hash256,
chain: &Arc<BeaconChain<T>>,
notify_execution_layer: NotifyExecutionLayer,
) -> Result<ExecutionPendingBlock<T>, BlockSlashInfo<BlockError>> {
// Perform an early check to prevent wasting time on irrelevant blocks.
let block_root = check_block_relevancy(&self, block_root, chain)
.map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?;
let maybe_available = chain
.data_availability_checker
.verify_kzg_for_rpc_block(RpcBlock::new_without_blobs(Some(block_root), self.clone()))
.map_err(|e| {
BlockSlashInfo::SignatureNotChecked(
self.signed_block_header(),
BlockError::AvailabilityCheck(e),
)
})?;
SignatureVerifiedBlock::check_slashable(maybe_available, block_root, chain)?
.into_execution_pending_block_slashable(block_root, chain, notify_execution_layer)
}
fn block(&self) -> &SignedBeaconBlock<T::EthSpec> {
self
}
fn block_cloned(&self) -> Arc<SignedBeaconBlock<T::EthSpec>> {
self.clone()
}
}
impl<T: BeaconChainTypes> IntoExecutionPendingBlock<T> for RpcBlock<T::EthSpec> {
/// Verifies the `SignedBeaconBlock` by first transforming it into a `SignatureVerifiedBlock`
/// and then using that implementation of `IntoExecutionPendingBlock` to complete verification.

4
beacon_node/beacon_chain/src/block_verification_types.rs
View File

@@ -103,14 +103,14 @@ impl<E: EthSpec> RpcBlock<E> {
pub fn new_without_blobs(
block_root: Option<Hash256>,
block: Arc<SignedBeaconBlock<E>>,
custody_columns_count: usize,
) -> Self {
let block_root = block_root.unwrap_or_else(|| get_block_root(&block));
Self {
block_root,
block: RpcBlockInner::Block(block),
// Block has zero columns
custody_columns_count: 0,
custody_columns_count,
}
}

4
beacon_node/beacon_chain/src/test_utils.rs
View File

@@ -2366,7 +2366,7 @@ where
.blob_kzg_commitments()
.is_ok_and(|c| !c.is_empty());
if !has_blobs {
return RpcBlock::new_without_blobs(Some(block_root), block);
return RpcBlock::new_without_blobs(Some(block_root), block, 0);
}
// Blobs are stored as data columns from Fulu (PeerDAS)
@@ -2417,7 +2417,7 @@ where
&self.spec,
)?
} else {
RpcBlock::new_without_blobs(Some(block_root), block)
RpcBlock::new_without_blobs(Some(block_root), block, 0)
}
} else {
let blobs = blob_items

37
beacon_node/beacon_chain/tests/block_verification.rs
View File

@@ -147,7 +147,7 @@ fn build_rpc_block(
RpcBlock::new_with_custody_columns(None, block, columns.clone(), columns.len(), spec)
.unwrap()
}
None => RpcBlock::new_without_blobs(None, block),
None => RpcBlock::new_without_blobs(None, block, 0),
}
}
@@ -370,6 +370,7 @@ async fn chain_segment_non_linear_parent_roots() {
blocks[3] = RpcBlock::new_without_blobs(
None,
Arc::new(SignedBeaconBlock::from_block(block, signature)),
harness.sampling_column_count,
);
assert!(
@@ -407,6 +408,7 @@ async fn chain_segment_non_linear_slots() {
blocks[3] = RpcBlock::new_without_blobs(
None,
Arc::new(SignedBeaconBlock::from_block(block, signature)),
harness.sampling_column_count,
);
assert!(
@@ -434,6 +436,7 @@ async fn chain_segment_non_linear_slots() {
blocks[3] = RpcBlock::new_without_blobs(
None,
Arc::new(SignedBeaconBlock::from_block(block, signature)),
harness.sampling_column_count,
);
assert!(
@@ -575,11 +578,16 @@ async fn invalid_signature_gossip_block() {
.into_block_error()
.expect("should import all blocks prior to the one being tested");
let signed_block = SignedBeaconBlock::from_block(block, junk_signature());
let rpc_block = RpcBlock::new_without_blobs(
None,
Arc::new(signed_block),
harness.sampling_column_count,
);
let process_res = harness
.chain
.process_block(
signed_block.canonical_root(),
Arc::new(signed_block),
rpc_block.block_root(),
rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),
@@ -1541,12 +1549,13 @@ async fn add_base_block_to_altair_chain() {
));
// Ensure that it would be impossible to import via `BeaconChain::process_block`.
let base_rpc_block = RpcBlock::new_without_blobs(None, Arc::new(base_block.clone()), 0);
assert!(matches!(
harness
.chain
.process_block(
base_block.canonical_root(),
Arc::new(base_block.clone()),
base_rpc_block.block_root(),
base_rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),
@@ -1564,7 +1573,7 @@ async fn add_base_block_to_altair_chain() {
harness
.chain
.process_chain_segment(
vec![RpcBlock::new_without_blobs(None, Arc::new(base_block))],
vec![RpcBlock::new_without_blobs(None, Arc::new(base_block), 0)],
NotifyExecutionLayer::Yes,
)
.await,
@@ -1677,12 +1686,13 @@ async fn add_altair_block_to_base_chain() {
));
// Ensure that it would be impossible to import via `BeaconChain::process_block`.
let altair_rpc_block = RpcBlock::new_without_blobs(None, Arc::new(altair_block.clone()), 0);
assert!(matches!(
harness
.chain
.process_block(
altair_block.canonical_root(),
Arc::new(altair_block.clone()),
altair_rpc_block.block_root(),
altair_rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),
@@ -1700,7 +1710,7 @@ async fn add_altair_block_to_base_chain() {
harness
.chain
.process_chain_segment(
vec![RpcBlock::new_without_blobs(None, Arc::new(altair_block))],
vec![RpcBlock::new_without_blobs(None, Arc::new(altair_block), 0)],
NotifyExecutionLayer::Yes
)
.await,
@@ -1761,11 +1771,16 @@ async fn import_duplicate_block_unrealized_justification() {
// Create two verified variants of the block, representing the same block being processed in
// parallel.
let notify_execution_layer = NotifyExecutionLayer::Yes;
let verified_block1 = block
let rpc_block = RpcBlock::new_without_blobs(
Some(block_root),
block.clone(),
harness.sampling_column_count,
);
let verified_block1 = rpc_block
.clone()
.into_execution_pending_block(block_root, chain, notify_execution_layer)
.unwrap();
let verified_block2 = block
let verified_block2 = rpc_block
.into_execution_pending_block(block_root, chain, notify_execution_layer)
.unwrap();

23
beacon_node/beacon_chain/tests/payload_invalidation.rs
View File

@@ -1,5 +1,6 @@
#![cfg(not(debug_assertions))]
use beacon_chain::block_verification_types::RpcBlock;
use beacon_chain::{
canonical_head::{CachedHead, CanonicalHead},
test_utils::{BeaconChainHarness, EphemeralHarnessType},
@@ -687,12 +688,14 @@ async fn invalidates_all_descendants() {
assert_eq!(fork_parent_state.slot(), fork_parent_slot);
let ((fork_block, _), _fork_post_state) =
rig.harness.make_block(fork_parent_state, fork_slot).await;
let fork_rpc_block =
RpcBlock::new_without_blobs(None, fork_block.clone(), rig.harness.sampling_column_count);
let fork_block_root = rig
.harness
.chain
.process_block(
fork_block.canonical_root(),
fork_block,
fork_rpc_block.block_root(),
fork_rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),
@@ -788,12 +791,14 @@ async fn switches_heads() {
let ((fork_block, _), _fork_post_state) =
rig.harness.make_block(fork_parent_state, fork_slot).await;
let fork_parent_root = fork_block.parent_root();
let fork_rpc_block =
RpcBlock::new_without_blobs(None, fork_block.clone(), rig.harness.sampling_column_count);
let fork_block_root = rig
.harness
.chain
.process_block(
fork_block.canonical_root(),
fork_block,
fork_rpc_block.block_root(),
fork_rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),
@@ -1057,8 +1062,10 @@ async fn invalid_parent() {
));
// Ensure the block built atop an invalid payload is invalid for import.
let rpc_block =
RpcBlock::new_without_blobs(None, block.clone(), rig.harness.sampling_column_count);
assert!(matches!(
rig.harness.chain.process_block(block.canonical_root(), block.clone(), NotifyExecutionLayer::Yes, BlockImportSource::Lookup,
rig.harness.chain.process_block(rpc_block.block_root(), rpc_block, NotifyExecutionLayer::Yes, BlockImportSource::Lookup,
|| Ok(()),
).await,
Err(BlockError::ParentExecutionPayloadInvalid { parent_root: invalid_root })
@@ -1380,11 +1387,13 @@ async fn recover_from_invalid_head_by_importing_blocks() {
} = InvalidHeadSetup::new().await;
// Import the fork block, it should become the head.
let fork_rpc_block =
RpcBlock::new_without_blobs(None, fork_block.clone(), rig.harness.sampling_column_count);
rig.harness
.chain
.process_block(
fork_block.canonical_root(),
fork_block.clone(),
fork_rpc_block.block_root(),
fork_rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),

19
beacon_node/beacon_chain/tests/store_tests.rs
View File

@@ -1,6 +1,7 @@
#![cfg(not(debug_assertions))]
use beacon_chain::attestation_verification::Error as AttnError;
use beacon_chain::block_verification_types::RpcBlock;
use beacon_chain::builder::BeaconChainBuilder;
use beacon_chain::data_availability_checker::AvailableBlock;
use beacon_chain::schema_change::migrate_schema;
@@ -2643,12 +2644,17 @@ async fn process_blocks_and_attestations_for_unaligned_checkpoint() {
assert_eq!(split.block_root, valid_fork_block.parent_root());
assert_ne!(split.state_root, unadvanced_split_state_root);
let invalid_fork_rpc_block = RpcBlock::new_without_blobs(
None,
invalid_fork_block.clone(),
harness.sampling_column_count,
);
// Applying the invalid block should fail.
let err = harness
.chain
.process_block(
invalid_fork_block.canonical_root(),
invalid_fork_block.clone(),
invalid_fork_rpc_block.block_root(),
invalid_fork_rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),
@@ -2658,11 +2664,16 @@ async fn process_blocks_and_attestations_for_unaligned_checkpoint() {
assert!(matches!(err, BlockError::WouldRevertFinalizedSlot { .. }));
// Applying the valid block should succeed, but it should not become head.
let valid_fork_rpc_block = RpcBlock::new_without_blobs(
None,
valid_fork_block.clone(),
harness.sampling_column_count,
);
harness
.chain
.process_block(
valid_fork_block.canonical_root(),
valid_fork_block.clone(),
valid_fork_rpc_block.block_root(),
valid_fork_rpc_block,
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),

8
beacon_node/http_api/src/publish_blocks.rs
View File

@@ -2,7 +2,7 @@ use crate::metrics;
use std::future::Future;
use beacon_chain::blob_verification::{GossipBlobError, GossipVerifiedBlob};
use beacon_chain::block_verification_types::AsBlock;
use beacon_chain::block_verification_types::{AsBlock, RpcBlock};
use beacon_chain::data_column_verification::{GossipDataColumnError, GossipVerifiedDataColumn};
use beacon_chain::validator_monitor::{get_block_delay_ms, timestamp_now};
use beacon_chain::{
@@ -302,7 +302,11 @@ pub async fn publish_block<T: BeaconChainTypes, B: IntoGossipVerifiedBlock<T>>(
);
let import_result = Box::pin(chain.process_block(
block_root,
block.clone(),
RpcBlock::new_without_blobs(
Some(block_root),
block.clone(),
network_globals.custody_columns_count() as usize,
),
NotifyExecutionLayer::Yes,
BlockImportSource::HttpApi,
publish_fn,

18
beacon_node/network/src/network_beacon_processor/tests.rs
View File

@@ -323,12 +323,22 @@ impl TestRig {
}
}
pub fn custody_columns_count(&self) -> usize {
self.network_beacon_processor
.network_globals
.custody_columns_count() as usize
}
pub fn enqueue_rpc_block(&self) {
let block_root = self.next_block.canonical_root();
self.network_beacon_processor
.send_rpc_beacon_block(
block_root,
RpcBlock::new_without_blobs(Some(block_root), self.next_block.clone()),
RpcBlock::new_without_blobs(
Some(block_root),
self.next_block.clone(),
self.custody_columns_count(),
),
std::time::Duration::default(),
BlockProcessType::SingleBlock { id: 0 },
)
@@ -340,7 +350,11 @@ impl TestRig {
self.network_beacon_processor
.send_rpc_beacon_block(
block_root,
RpcBlock::new_without_blobs(Some(block_root), self.next_block.clone()),
RpcBlock::new_without_blobs(
Some(block_root),
self.next_block.clone(),
self.custody_columns_count(),
),
std::time::Duration::default(),
BlockProcessType::SingleBlock { id: 1 },
)

10
beacon_node/network/src/sync/block_lookups/common.rs
View File

@@ -6,7 +6,6 @@ use crate::sync::block_lookups::{
};
use crate::sync::manager::BlockProcessType;
use crate::sync::network_context::{LookupRequestResult, SyncNetworkContext};
use beacon_chain::block_verification_types::RpcBlock;
use beacon_chain::BeaconChainTypes;
use lighthouse_network::service::api_types::Id;
use parking_lot::RwLock;
@@ -97,13 +96,8 @@ impl<T: BeaconChainTypes> RequestState<T> for BlockRequestState<T::EthSpec> {
seen_timestamp,
..
} = download_result;
cx.send_block_for_processing(
id,
block_root,
RpcBlock::new_without_blobs(Some(block_root), value),
seen_timestamp,
)
.map_err(LookupRequestError::SendFailedProcessor)
cx.send_block_for_processing(id, block_root, value, seen_timestamp)
.map_err(LookupRequestError::SendFailedProcessor)
}
fn response_type() -> ResponseType {

3
beacon_node/network/src/sync/block_sidecar_coupling.rs
View File

@@ -266,7 +266,8 @@ impl<E: EthSpec> RangeBlockComponentsRequest<E> {
)
.map_err(|e| format!("{e:?}"))?
} else {
RpcBlock::new_without_blobs(Some(block_root), block)
// Block has no data, expects zero columns
RpcBlock::new_without_blobs(Some(block_root), block, 0)
});
}

8
beacon_node/network/src/sync/network_context.rs
View File

@@ -1308,7 +1308,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
&self,
id: Id,
block_root: Hash256,
block: RpcBlock<T::EthSpec>,
block: Arc<SignedBeaconBlock<T::EthSpec>>,
seen_timestamp: Duration,
) -> Result<(), SendErrorProcessor> {
let span = span!(
@@ -1322,6 +1322,12 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
.beacon_processor_if_enabled()
.ok_or(SendErrorProcessor::ProcessorNotAvailable)?;
let block = RpcBlock::new_without_blobs(
Some(block_root),
block,
self.network_globals().custody_columns_count() as usize,
);
debug!(block = ?block_root, id, "Sending block for processing");
// Lookup sync event safety: If `beacon_processor.send_rpc_beacon_block` returns Ok() sync
// must receive a single `SyncMessage::BlockComponentProcessed` with this process type

3
beacon_node/network/src/sync/tests/range.rs
View File

@@ -459,7 +459,8 @@ fn build_rpc_block(
)
.unwrap()
}
None => RpcBlock::new_without_blobs(None, block),
// Block has no data, expects zero columns
None => RpcBlock::new_without_blobs(None, block, 0),
}
}

3
testing/ef_tests/src/cases/fork_choice.rs
View File

@@ -3,6 +3,7 @@ use crate::decode::{ssz_decode_file, ssz_decode_file_with, ssz_decode_state, yam
use ::fork_choice::{PayloadVerificationStatus, ProposerHeadError};
use beacon_chain::beacon_proposer_cache::compute_proposer_duties_from_head;
use beacon_chain::blob_verification::GossipBlobError;
use beacon_chain::block_verification_types::RpcBlock;
use beacon_chain::chain_config::{
DisallowedReOrgOffsets, DEFAULT_RE_ORG_HEAD_THRESHOLD,
DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION, DEFAULT_RE_ORG_PARENT_THRESHOLD,
@@ -519,7 +520,7 @@ impl<E: EthSpec> Tester<E> {
let result: Result<Result<Hash256, ()>, _> = self
.block_on_dangerous(self.harness.chain.process_block(
block_root,
block.clone(),
RpcBlock::new_without_blobs(Some(block_root), block.clone(), 0),
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),