Skip to content

Commit 6f31d44

Browse files
dappliondapplion
authored
Remove CGC from data_availability checker (#7033)
- Part of #6767 Validator custody makes the CGC and set of sampling columns dynamic. Right now this information is stored twice: - in the data availability checker - in the network globals If that state becomes dynamic we must make sure it is in sync updating it twice, or guarding it behind a mutex. However, I noted that we don't really have to keep the CGC inside the data availability checker. All consumers can actually read it from the network globals, and we can update `make_available` to read the expected count of data columns from the block.
1 parent 9dce729 commit 6f31d44

21 files changed

+297
-214
lines changed

‎beacon_node/beacon_chain/src/beacon_chain.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3031,6 +3031,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
30313031
pub async fn verify_block_for_gossip(
30323032
self: &Arc<Self>,
30333033
block: Arc<SignedBeaconBlock<T::EthSpec>>,
3034+
custody_columns_count: usize,
30343035
) -> Result<GossipVerifiedBlock<T>, BlockError> {
30353036
let chain = self.clone();
30363037
self.task_executor
@@ -3040,7 +3041,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
30403041
let slot = block.slot();
30413042
let graffiti_string = block.message().body().graffiti().as_utf8_lossy();
30423043

3043-
match GossipVerifiedBlock::new(block, &chain) {
3044+
match GossipVerifiedBlock::new(block, &chain, custody_columns_count) {
30443045
Ok(verified) => {
30453046
let commitments_formatted = verified.block.commitments_formatted();
30463047
debug!(
@@ -7161,10 +7162,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
71617162
block_root: Hash256,
71627163
block_data: AvailableBlockData<T::EthSpec>,
71637164
) -> Result<Option<StoreOp<T::EthSpec>>, String> {
7164-
// TODO(das) we currently store all subnet sampled columns. Tracking issue to exclude non
7165-
// custody columns: https://github.com/sigp/lighthouse/issues/6465
7166-
let _custody_columns_count = self.data_availability_checker.get_sampling_column_count();
7167-
71687165
match block_data {
71697166
AvailableBlockData::NoData => Ok(None),
71707167
AvailableBlockData::Blobs(blobs) => {

‎beacon_node/beacon_chain/src/block_verification.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,7 @@ pub struct GossipVerifiedBlock<T: BeaconChainTypes> {
683683
pub block_root: Hash256,
684684
parent: Option<PreProcessingSnapshot<T::EthSpec>>,
685685
consensus_context: ConsensusContext<T::EthSpec>,
686+
custody_columns_count: usize,
686687
}
687688

688689
/// A wrapper around a `SignedBeaconBlock` that indicates that all signatures (except the deposit
@@ -718,6 +719,7 @@ pub trait IntoGossipVerifiedBlock<T: BeaconChainTypes>: Sized {
718719
fn into_gossip_verified_block(
719720
self,
720721
chain: &BeaconChain<T>,
722+
custody_columns_count: usize,
721723
) -> Result<GossipVerifiedBlock<T>, BlockError>;
722724
fn inner_block(&self) -> Arc<SignedBeaconBlock<T::EthSpec>>;
723725
}
@@ -726,6 +728,7 @@ impl<T: BeaconChainTypes> IntoGossipVerifiedBlock<T> for GossipVerifiedBlock<T>
726728
fn into_gossip_verified_block(
727729
self,
728730
_chain: &BeaconChain<T>,
731+
_custody_columns_count: usize,
729732
) -> Result<GossipVerifiedBlock<T>, BlockError> {
730733
Ok(self)
731734
}
@@ -738,8 +741,9 @@ impl<T: BeaconChainTypes> IntoGossipVerifiedBlock<T> for Arc<SignedBeaconBlock<T
738741
fn into_gossip_verified_block(
739742
self,
740743
chain: &BeaconChain<T>,
744+
custody_columns_count: usize,
741745
) -> Result<GossipVerifiedBlock<T>, BlockError> {
742-
GossipVerifiedBlock::new(self, chain)
746+
GossipVerifiedBlock::new(self, chain, custody_columns_count)
743747
}
744748

745749
fn inner_block(&self) -> Arc<SignedBeaconBlock<T::EthSpec>> {
@@ -808,6 +812,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
808812
pub fn new(
809813
block: Arc<SignedBeaconBlock<T::EthSpec>>,
810814
chain: &BeaconChain<T>,
815+
custody_columns_count: usize,
811816
) -> Result<Self, BlockError> {
812817
// If the block is valid for gossip we don't supply it to the slasher here because
813818
// we assume it will be transformed into a fully verified block. We *do* need to supply
@@ -817,19 +822,22 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
817822
// The `SignedBeaconBlock` and `SignedBeaconBlockHeader` have the same canonical root,
818823
// but it's way quicker to calculate root of the header since the hash of the tree rooted
819824
// at `BeaconBlockBody` is already computed in the header.
820-
Self::new_without_slasher_checks(block, &header, chain).map_err(|e| {
821-
process_block_slash_info::<_, BlockError>(
822-
chain,
823-
BlockSlashInfo::from_early_error_block(header, e),
824-
)
825-
})
825+
Self::new_without_slasher_checks(block, &header, chain, custody_columns_count).map_err(
826+
|e| {
827+
process_block_slash_info::<_, BlockError>(
828+
chain,
829+
BlockSlashInfo::from_early_error_block(header, e),
830+
)
831+
},
832+
)
826833
}
827834

828835
/// As for new, but doesn't pass the block to the slasher.
829836
fn new_without_slasher_checks(
830837
block: Arc<SignedBeaconBlock<T::EthSpec>>,
831838
block_header: &SignedBeaconBlockHeader,
832839
chain: &BeaconChain<T>,
840+
custody_columns_count: usize,
833841
) -> Result<Self, BlockError> {
834842
// Ensure the block is the correct structure for the fork at `block.slot()`.
835843
block
@@ -1036,6 +1044,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
10361044
block_root,
10371045
parent,
10381046
consensus_context,
1047+
custody_columns_count,
10391048
})
10401049
}
10411050

@@ -1183,6 +1192,7 @@ impl<T: BeaconChainTypes> SignatureVerifiedBlock<T> {
11831192
block: MaybeAvailableBlock::AvailabilityPending {
11841193
block_root: from.block_root,
11851194
block,
1195+
custody_columns_count: from.custody_columns_count,
11861196
},
11871197
block_root: from.block_root,
11881198
parent: Some(parent),

‎beacon_node/beacon_chain/src/block_verification_types.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use types::{
3131
pub struct RpcBlock<E: EthSpec> {
3232
block_root: Hash256,
3333
block: RpcBlockInner<E>,
34+
custody_columns_count: usize,
3435
}
3536

3637
impl<E: EthSpec> Debug for RpcBlock<E> {
@@ -44,6 +45,10 @@ impl<E: EthSpec> RpcBlock<E> {
4445
self.block_root
4546
}
4647

48+
pub fn custody_columns_count(&self) -> usize {
49+
self.custody_columns_count
50+
}
51+
4752
pub fn as_block(&self) -> &SignedBeaconBlock<E> {
4853
match &self.block {
4954
RpcBlockInner::Block(block) => block,
@@ -104,6 +109,8 @@ impl<E: EthSpec> RpcBlock<E> {
104109
Self {
105110
block_root,
106111
block: RpcBlockInner::Block(block),
112+
// Block has zero columns
113+
custody_columns_count: 0,
107114
}
108115
}
109116

@@ -145,13 +152,16 @@ impl<E: EthSpec> RpcBlock<E> {
145152
Ok(Self {
146153
block_root,
147154
block: inner,
155+
// Block is before PeerDAS
156+
custody_columns_count: 0,
148157
})
149158
}
150159

151160
pub fn new_with_custody_columns(
152161
block_root: Option<Hash256>,
153162
block: Arc<SignedBeaconBlock<E>>,
154163
custody_columns: Vec<CustodyDataColumn<E>>,
164+
custody_columns_count: usize,
155165
spec: &ChainSpec,
156166
) -> Result<Self, AvailabilityCheckError> {
157167
let block_root = block_root.unwrap_or_else(|| get_block_root(&block));
@@ -172,6 +182,7 @@ impl<E: EthSpec> RpcBlock<E> {
172182
Ok(Self {
173183
block_root,
174184
block: inner,
185+
custody_columns_count,
175186
})
176187
}
177188

@@ -239,10 +250,12 @@ impl<E: EthSpec> ExecutedBlock<E> {
239250
MaybeAvailableBlock::AvailabilityPending {
240251
block_root: _,
241252
block: pending_block,
253+
custody_columns_count,
242254
} => Self::AvailabilityPending(AvailabilityPendingExecutedBlock::new(
243255
pending_block,
244256
import_data,
245257
payload_verification_outcome,
258+
custody_columns_count,
246259
)),
247260
}
248261
}
@@ -308,18 +321,21 @@ pub struct AvailabilityPendingExecutedBlock<E: EthSpec> {
308321
pub block: Arc<SignedBeaconBlock<E>>,
309322
pub import_data: BlockImportData<E>,
310323
pub payload_verification_outcome: PayloadVerificationOutcome,
324+
pub custody_columns_count: usize,
311325
}
312326

313327
impl<E: EthSpec> AvailabilityPendingExecutedBlock<E> {
314328
pub fn new(
315329
block: Arc<SignedBeaconBlock<E>>,
316330
import_data: BlockImportData<E>,
317331
payload_verification_outcome: PayloadVerificationOutcome,
332+
custody_columns_count: usize,
318333
) -> Self {
319334
Self {
320335
block,
321336
import_data,
322337
payload_verification_outcome,
338+
custody_columns_count,
323339
}
324340
}
325341

@@ -439,19 +455,13 @@ impl<E: EthSpec> AsBlock<E> for MaybeAvailableBlock<E> {
439455
fn as_block(&self) -> &SignedBeaconBlock<E> {
440456
match &self {
441457
MaybeAvailableBlock::Available(block) => block.as_block(),
442-
MaybeAvailableBlock::AvailabilityPending {
443-
block_root: _,
444-
block,
445-
} => block,
458+
MaybeAvailableBlock::AvailabilityPending { block, .. } => block,
446459
}
447460
}
448461
fn block_cloned(&self) -> Arc<SignedBeaconBlock<E>> {
449462
match &self {
450463
MaybeAvailableBlock::Available(block) => block.block_cloned(),
451-
MaybeAvailableBlock::AvailabilityPending {
452-
block_root: _,
453-
block,
454-
} => block.clone(),
464+
MaybeAvailableBlock::AvailabilityPending { block, .. } => block.clone(),
455465
}
456466
}
457467
fn canonical_root(&self) -> Hash256 {

‎beacon_node/beacon_chain/src/builder.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -975,14 +975,8 @@ where
975975
validator_monitor: RwLock::new(validator_monitor),
976976
genesis_backfill_slot,
977977
data_availability_checker: Arc::new(
978-
DataAvailabilityChecker::new(
979-
slot_clock,
980-
self.kzg.clone(),
981-
store,
982-
self.import_all_data_columns,
983-
self.spec,
984-
)
985-
.map_err(|e| format!("Error initializing DataAvailabilityChecker: {:?}", e))?,
978+
DataAvailabilityChecker::new(slot_clock, self.kzg.clone(), store, self.spec)
979+
.map_err(|e| format!("Error initializing DataAvailabilityChecker: {:?}", e))?,
986980
),
987981
kzg: self.kzg.clone(),
988982
};

‎beacon_node/beacon_chain/src/data_availability_checker.rs

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -111,21 +111,9 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
111111
slot_clock: T::SlotClock,
112112
kzg: Arc<Kzg>,
113113
store: BeaconStore<T>,
114-
import_all_data_columns: bool,
115114
spec: Arc<ChainSpec>,
116115
) -> Result<Self, AvailabilityCheckError> {
117-
let custody_group_count = spec.custody_group_count(import_all_data_columns);
118-
// This should only panic if the chain spec contains invalid values.
119-
let sampling_size = spec
120-
.sampling_size(custody_group_count)
121-
.expect("should compute node sampling size from valid chain spec");
122-
123-
let inner = DataAvailabilityCheckerInner::new(
124-
OVERFLOW_LRU_CAPACITY,
125-
store,
126-
sampling_size as usize,
127-
spec.clone(),
128-
)?;
116+
let inner = DataAvailabilityCheckerInner::new(OVERFLOW_LRU_CAPACITY, store, spec.clone())?;
129117
Ok(Self {
130118
availability_cache: Arc::new(inner),
131119
slot_clock,
@@ -134,14 +122,6 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
134122
})
135123
}
136124

137-
pub fn get_sampling_column_count(&self) -> usize {
138-
self.availability_cache.sampling_column_count()
139-
}
140-
141-
pub(crate) fn is_supernode(&self) -> bool {
142-
self.get_sampling_column_count() == self.spec.number_of_columns as usize
143-
}
144-
145125
/// Checks if the block root is currenlty in the availability cache awaiting import because
146126
/// of missing components.
147127
pub fn get_execution_valid_block(
@@ -326,6 +306,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
326306
&self,
327307
block: RpcBlock<T::EthSpec>,
328308
) -> Result<MaybeAvailableBlock<T::EthSpec>, AvailabilityCheckError> {
309+
let custody_columns_count = block.custody_columns_count();
329310
let (block_root, block, blobs, data_columns) = block.deconstruct();
330311
if self.blobs_required_for_block(&block) {
331312
return if let Some(blob_list) = blobs {
@@ -339,7 +320,11 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
339320
spec: self.spec.clone(),
340321
}))
341322
} else {
342-
Ok(MaybeAvailableBlock::AvailabilityPending { block_root, block })
323+
Ok(MaybeAvailableBlock::AvailabilityPending {
324+
block_root,
325+
block,
326+
custody_columns_count,
327+
})
343328
};
344329
}
345330
if self.data_columns_required_for_block(&block) {
@@ -364,7 +349,11 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
364349
spec: self.spec.clone(),
365350
}))
366351
} else {
367-
Ok(MaybeAvailableBlock::AvailabilityPending { block_root, block })
352+
Ok(MaybeAvailableBlock::AvailabilityPending {
353+
block_root,
354+
block,
355+
custody_columns_count,
356+
})
368357
};
369358
}
370359

@@ -421,6 +410,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
421410
}
422411

423412
for block in blocks {
413+
let custody_columns_count = block.custody_columns_count();
424414
let (block_root, block, blobs, data_columns) = block.deconstruct();
425415

426416
let maybe_available_block = if self.blobs_required_for_block(&block) {
@@ -433,7 +423,11 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
433423
spec: self.spec.clone(),
434424
})
435425
} else {
436-
MaybeAvailableBlock::AvailabilityPending { block_root, block }
426+
MaybeAvailableBlock::AvailabilityPending {
427+
block_root,
428+
block,
429+
custody_columns_count,
430+
}
437431
}
438432
} else if self.data_columns_required_for_block(&block) {
439433
if let Some(data_columns) = data_columns {
@@ -447,7 +441,11 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
447441
spec: self.spec.clone(),
448442
})
449443
} else {
450-
MaybeAvailableBlock::AvailabilityPending { block_root, block }
444+
MaybeAvailableBlock::AvailabilityPending {
445+
block_root,
446+
block,
447+
custody_columns_count,
448+
}
451449
}
452450
} else {
453451
MaybeAvailableBlock::Available(AvailableBlock {
@@ -804,6 +802,7 @@ pub enum MaybeAvailableBlock<E: EthSpec> {
804802
AvailabilityPending {
805803
block_root: Hash256,
806804
block: Arc<SignedBeaconBlock<E>>,
805+
custody_columns_count: usize,
807806
},
808807
}
809808

‎beacon_node/beacon_chain/src/data_availability_checker/error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ pub enum Error {
1010
blob_commitment: KzgCommitment,
1111
block_commitment: KzgCommitment,
1212
},
13-
Unexpected(&'static str),
13+
Unexpected(String),
1414
SszTypes(ssz_types::Error),
1515
MissingBlobs,
1616
MissingCustodyColumns,

0 commit comments

Comments
 (0)