Skip to content

Commit 5933901

Browse files
SunnysidedJSunnysidedJ
authored
peerdas-devnet-7: update DataColumnSidecarsByRoot request to use DataColumnsByRootIdentifier (#7399)
Update DataColumnSidecarsByRoot request to use DataColumnsByRootIdentifier #7377 As described in ethereum/consensus-specs#4284
1 parent 92391cd commit 5933901

File tree

19 files changed

+262
-208
lines changed

19 files changed

+262
-208
lines changed

‎beacon_node/beacon_chain/src/beacon_chain.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ use tokio_stream::Stream;
127127
use tracing::{debug, error, info, trace, warn};
128128
use tree_hash::TreeHash;
129129
use types::blob_sidecar::FixedBlobSidecarList;
130-
use types::data_column_sidecar::{ColumnIndex, DataColumnIdentifier};
130+
use types::data_column_sidecar::ColumnIndex;
131131
use types::payload::BlockProductionVersion;
132132
use types::*;
133133

@@ -1106,23 +1106,25 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
11061106
.map_or_else(|| self.get_blobs(block_root), Ok)
11071107
}
11081108

1109-
pub fn get_data_column_checking_all_caches(
1109+
pub fn get_data_columns_checking_all_caches(
11101110
&self,
11111111
block_root: Hash256,
1112-
index: ColumnIndex,
1113-
) -> Result<Option<Arc<DataColumnSidecar<T::EthSpec>>>, Error> {
1114-
if let Some(column) = self
1112+
indices: &[ColumnIndex],
1113+
) -> Result<DataColumnSidecarList<T::EthSpec>, Error> {
1114+
let all_cached_columns_opt = self
11151115
.data_availability_checker
1116-
.get_data_column(&DataColumnIdentifier { block_root, index })?
1117-
{
1118-
return Ok(Some(column));
1119-
}
1116+
.get_data_columns(block_root)
1117+
.or_else(|| self.early_attester_cache.get_data_columns(block_root));
11201118

1121-
if let Some(columns) = self.early_attester_cache.get_data_columns(block_root) {
1122-
return Ok(columns.iter().find(|c| c.index == index).cloned());
1119+
if let Some(mut all_cached_columns) = all_cached_columns_opt {
1120+
all_cached_columns.retain(|col| indices.contains(&col.index));
1121+
Ok(all_cached_columns)
1122+
} else {
1123+
indices
1124+
.iter()
1125+
.filter_map(|index| self.get_data_column(&block_root, index).transpose())
1126+
.collect::<Result<_, _>>()
11231127
}
1124-
1125-
self.get_data_column(&block_root, &index)
11261128
}
11271129

11281130
/// Returns the block at the given root, if any.

‎beacon_node/beacon_chain/src/data_availability_checker.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use task_executor::TaskExecutor;
1717
use tracing::{debug, error, info_span, Instrument};
1818
use types::blob_sidecar::{BlobIdentifier, BlobSidecar, FixedBlobSidecarList};
1919
use types::{
20-
BlobSidecarList, ChainSpec, DataColumnIdentifier, DataColumnSidecar, DataColumnSidecarList,
21-
Epoch, EthSpec, Hash256, RuntimeVariableList, SignedBeaconBlock,
20+
BlobSidecarList, ChainSpec, DataColumnSidecarList, Epoch, EthSpec, Hash256,
21+
RuntimeVariableList, SignedBeaconBlock,
2222
};
2323

2424
mod error;
@@ -163,12 +163,12 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
163163
self.availability_cache.peek_blob(blob_id)
164164
}
165165

166-
/// Get a data column from the availability cache.
167-
pub fn get_data_column(
166+
/// Get data columns for a block from the availability cache.
167+
pub fn get_data_columns(
168168
&self,
169-
data_column_id: &DataColumnIdentifier,
170-
) -> Result<Option<Arc<DataColumnSidecar<T::EthSpec>>>, AvailabilityCheckError> {
171-
self.availability_cache.peek_data_column(data_column_id)
169+
block_root: Hash256,
170+
) -> Option<DataColumnSidecarList<T::EthSpec>> {
171+
self.availability_cache.peek_data_columns(block_root)
172172
}
173173

174174
/// Put a list of blobs received via RPC into the availability cache. This performs KZG

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

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::sync::Arc;
1616
use tracing::debug;
1717
use types::blob_sidecar::BlobIdentifier;
1818
use types::{
19-
BlobSidecar, ChainSpec, ColumnIndex, DataColumnIdentifier, DataColumnSidecar, Epoch, EthSpec,
19+
BlobSidecar, ChainSpec, ColumnIndex, DataColumnSidecar, DataColumnSidecarList, Epoch, EthSpec,
2020
Hash256, RuntimeFixedVector, RuntimeVariableList, SignedBeaconBlock,
2121
};
2222

@@ -404,20 +404,21 @@ impl<T: BeaconChainTypes> DataAvailabilityCheckerInner<T> {
404404
}
405405
}
406406

407-
/// Fetch a data column from the cache without affecting the LRU ordering
408-
pub fn peek_data_column(
407+
/// Fetch data columns of a given `block_root` from the cache without affecting the LRU ordering
408+
pub fn peek_data_columns(
409409
&self,
410-
data_column_id: &DataColumnIdentifier,
411-
) -> Result<Option<Arc<DataColumnSidecar<T::EthSpec>>>, AvailabilityCheckError> {
412-
if let Some(pending_components) = self.critical.read().peek(&data_column_id.block_root) {
413-
Ok(pending_components
414-
.verified_data_columns
415-
.iter()
416-
.find(|data_column| data_column.as_data_column().index == data_column_id.index)
417-
.map(|data_column| data_column.clone_arc()))
418-
} else {
419-
Ok(None)
420-
}
410+
block_root: Hash256,
411+
) -> Option<DataColumnSidecarList<T::EthSpec>> {
412+
self.critical
413+
.read()
414+
.peek(&block_root)
415+
.map(|pending_components| {
416+
pending_components
417+
.verified_data_columns
418+
.iter()
419+
.map(|col| col.clone_arc())
420+
.collect()
421+
})
421422
}
422423

423424
pub fn peek_pending_components<R, F: FnOnce(Option<&PendingComponents<T::EthSpec>>) -> R>(

‎beacon_node/beacon_chain/src/data_column_verification.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::iter;
1616
use std::marker::PhantomData;
1717
use std::sync::Arc;
1818
use tracing::debug;
19-
use types::data_column_sidecar::{ColumnIndex, DataColumnIdentifier};
19+
use types::data_column_sidecar::ColumnIndex;
2020
use types::{
2121
BeaconStateError, ChainSpec, DataColumnSidecar, DataColumnSubnetId, EthSpec, Hash256,
2222
RuntimeVariableList, SignedBeaconBlockHeader, Slot,
@@ -200,13 +200,6 @@ impl<T: BeaconChainTypes, O: ObservationStrategy> GossipVerifiedDataColumn<T, O>
200200
)
201201
}
202202

203-
pub fn id(&self) -> DataColumnIdentifier {
204-
DataColumnIdentifier {
205-
block_root: self.block_root,
206-
index: self.data_column.index(),
207-
}
208-
}
209-
210203
pub fn as_data_column(&self) -> &DataColumnSidecar<T::EthSpec> {
211204
self.data_column.as_data_column()
212205
}
@@ -741,7 +734,7 @@ pub fn observe_gossip_data_column<T: BeaconChainTypes>(
741734
chain: &BeaconChain<T>,
742735
) -> Result<(), GossipDataColumnError> {
743736
// Now the signature is valid, store the proposal so we don't accept another data column sidecar
744-
// with the same `DataColumnIdentifier`. It's important to double-check that the proposer still
737+
// with the same `ColumnIndex`. It's important to double-check that the proposer still
745738
// hasn't been observed so we don't have a race-condition when verifying two blocks
746739
// simultaneously.
747740
//

‎beacon_node/lighthouse_network/src/rpc/codec.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@ use std::marker::PhantomData;
1616
use std::sync::Arc;
1717
use tokio_util::codec::{Decoder, Encoder};
1818
use types::{
19-
BlobSidecar, ChainSpec, DataColumnSidecar, EthSpec, ForkContext, ForkName, Hash256,
20-
LightClientBootstrap, LightClientFinalityUpdate, LightClientOptimisticUpdate,
21-
LightClientUpdate, RuntimeVariableList, SignedBeaconBlock, SignedBeaconBlockAltair,
22-
SignedBeaconBlockBase, SignedBeaconBlockBellatrix, SignedBeaconBlockCapella,
23-
SignedBeaconBlockDeneb, SignedBeaconBlockElectra, SignedBeaconBlockFulu,
19+
BlobSidecar, ChainSpec, DataColumnSidecar, DataColumnsByRootIdentifier, EthSpec, ForkContext,
20+
ForkName, Hash256, LightClientBootstrap, LightClientFinalityUpdate,
21+
LightClientOptimisticUpdate, LightClientUpdate, RuntimeVariableList, SignedBeaconBlock,
22+
SignedBeaconBlockAltair, SignedBeaconBlockBase, SignedBeaconBlockBellatrix,
23+
SignedBeaconBlockCapella, SignedBeaconBlockDeneb, SignedBeaconBlockElectra,
24+
SignedBeaconBlockFulu,
2425
};
2526
use unsigned_varint::codec::Uvi;
2627

@@ -596,10 +597,12 @@ fn handle_rpc_request<E: EthSpec>(
596597
))),
597598
SupportedProtocol::DataColumnsByRootV1 => Ok(Some(RequestType::DataColumnsByRoot(
598599
DataColumnsByRootRequest {
599-
data_column_ids: RuntimeVariableList::from_ssz_bytes(
600-
decoded_buffer,
601-
spec.max_request_data_column_sidecars as usize,
602-
)?,
600+
data_column_ids:
601+
<RuntimeVariableList<DataColumnsByRootIdentifier>>::from_ssz_bytes_with_nested(
602+
decoded_buffer,
603+
spec.max_request_blocks(current_fork),
604+
spec.number_of_columns as usize,
605+
)?,
603606
},
604607
))),
605608
SupportedProtocol::PingV1 => Ok(Some(RequestType::Ping(Ping {
@@ -935,8 +938,8 @@ mod tests {
935938
use crate::types::{EnrAttestationBitfield, EnrSyncCommitteeBitfield};
936939
use types::{
937940
blob_sidecar::BlobIdentifier, data_column_sidecar::Cell, BeaconBlock, BeaconBlockAltair,
938-
BeaconBlockBase, BeaconBlockBellatrix, BeaconBlockHeader, DataColumnIdentifier, EmptyBlock,
939-
Epoch, FixedBytesExtended, FullPayload, KzgCommitment, KzgProof, Signature,
941+
BeaconBlockBase, BeaconBlockBellatrix, BeaconBlockHeader, DataColumnsByRootIdentifier,
942+
EmptyBlock, Epoch, FixedBytesExtended, FullPayload, KzgCommitment, KzgProof, Signature,
940943
SignedBeaconBlockHeader, Slot,
941944
};
942945

@@ -1066,14 +1069,15 @@ mod tests {
10661069
}
10671070
}
10681071

1069-
fn dcbroot_request(spec: &ChainSpec) -> DataColumnsByRootRequest {
1072+
fn dcbroot_request(spec: &ChainSpec, fork_name: ForkName) -> DataColumnsByRootRequest {
1073+
let number_of_columns = spec.number_of_columns as usize;
10701074
DataColumnsByRootRequest {
10711075
data_column_ids: RuntimeVariableList::new(
1072-
vec![DataColumnIdentifier {
1076+
vec![DataColumnsByRootIdentifier {
10731077
block_root: Hash256::zero(),
1074-
index: 0,
1078+
columns: RuntimeVariableList::from_vec(vec![0, 1, 2], number_of_columns),
10751079
}],
1076-
spec.max_request_data_column_sidecars as usize,
1080+
spec.max_request_blocks(fork_name),
10771081
)
10781082
.unwrap(),
10791083
}
@@ -1904,7 +1908,6 @@ mod tests {
19041908
RequestType::MetaData(MetadataRequest::new_v1()),
19051909
RequestType::BlobsByRange(blbrange_request()),
19061910
RequestType::DataColumnsByRange(dcbrange_request()),
1907-
RequestType::DataColumnsByRoot(dcbroot_request(&chain_spec)),
19081911
RequestType::MetaData(MetadataRequest::new_v2()),
19091912
];
19101913
for req in requests.iter() {
@@ -1920,6 +1923,7 @@ mod tests {
19201923
RequestType::BlobsByRoot(blbroot_request(fork_name)),
19211924
RequestType::BlocksByRoot(bbroot_request_v1(fork_name)),
19221925
RequestType::BlocksByRoot(bbroot_request_v2(fork_name)),
1926+
RequestType::DataColumnsByRoot(dcbroot_request(&chain_spec, fork_name)),
19231927
]
19241928
};
19251929
for fork_name in ForkName::list_all() {

‎beacon_node/lighthouse_network/src/rpc/methods.rs

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use serde::Serialize;
66
use ssz::Encode;
77
use ssz_derive::{Decode, Encode};
88
use ssz_types::{typenum::U256, VariableList};
9-
use std::collections::BTreeMap;
109
use std::fmt::Display;
1110
use std::marker::PhantomData;
1211
use std::ops::Deref;
@@ -16,9 +15,10 @@ use superstruct::superstruct;
1615
use types::blob_sidecar::BlobIdentifier;
1716
use types::light_client_update::MAX_REQUEST_LIGHT_CLIENT_UPDATES;
1817
use types::{
19-
blob_sidecar::BlobSidecar, ChainSpec, ColumnIndex, DataColumnIdentifier, DataColumnSidecar,
20-
Epoch, EthSpec, Hash256, LightClientBootstrap, LightClientFinalityUpdate,
21-
LightClientOptimisticUpdate, LightClientUpdate, RuntimeVariableList, SignedBeaconBlock, Slot,
18+
blob_sidecar::BlobSidecar, ChainSpec, ColumnIndex, DataColumnSidecar,
19+
DataColumnsByRootIdentifier, Epoch, EthSpec, Hash256, LightClientBootstrap,
20+
LightClientFinalityUpdate, LightClientOptimisticUpdate, LightClientUpdate, RuntimeVariableList,
21+
SignedBeaconBlock, Slot,
2222
};
2323
use types::{ForkContext, ForkName};
2424

@@ -479,31 +479,20 @@ impl BlobsByRootRequest {
479479
#[derive(Clone, Debug, PartialEq)]
480480
pub struct DataColumnsByRootRequest {
481481
/// The list of beacon block roots and column indices being requested.
482-
pub data_column_ids: RuntimeVariableList<DataColumnIdentifier>,
482+
pub data_column_ids: RuntimeVariableList<DataColumnsByRootIdentifier>,
483483
}
484484

485485
impl DataColumnsByRootRequest {
486-
pub fn new(data_column_ids: Vec<DataColumnIdentifier>, spec: &ChainSpec) -> Self {
487-
let data_column_ids = RuntimeVariableList::from_vec(
488-
data_column_ids,
489-
spec.max_request_data_column_sidecars as usize,
490-
);
486+
pub fn new(
487+
data_column_ids: Vec<DataColumnsByRootIdentifier>,
488+
max_request_blocks: usize,
489+
) -> Self {
490+
let data_column_ids = RuntimeVariableList::from_vec(data_column_ids, max_request_blocks);
491491
Self { data_column_ids }
492492
}
493493

494-
pub fn new_single(block_root: Hash256, index: ColumnIndex, spec: &ChainSpec) -> Self {
495-
Self::new(vec![DataColumnIdentifier { block_root, index }], spec)
496-
}
497-
498-
pub fn group_by_ordered_block_root(&self) -> Vec<(Hash256, Vec<ColumnIndex>)> {
499-
let mut column_indexes_by_block = BTreeMap::<Hash256, Vec<ColumnIndex>>::new();
500-
for request_id in self.data_column_ids.as_slice() {
501-
column_indexes_by_block
502-
.entry(request_id.block_root)
503-
.or_default()
504-
.push(request_id.index);
505-
}
506-
column_indexes_by_block.into_iter().collect()
494+
pub fn max_requested(&self) -> usize {
495+
self.data_column_ids.iter().map(|id| id.columns.len()).sum()
507496
}
508497
}
509498

‎beacon_node/lighthouse_network/src/rpc/protocol.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,7 @@ impl<E: EthSpec> RequestType<E> {
740740
RequestType::BlocksByRoot(req) => req.block_roots().len() as u64,
741741
RequestType::BlobsByRange(req) => req.max_blobs_requested(current_fork, spec),
742742
RequestType::BlobsByRoot(req) => req.blob_ids.len() as u64,
743-
RequestType::DataColumnsByRoot(req) => req.data_column_ids.len() as u64,
743+
RequestType::DataColumnsByRoot(req) => req.max_requested() as u64,
744744
RequestType::DataColumnsByRange(req) => req.max_requested::<E>(),
745745
RequestType::Ping(_) => 1,
746746
RequestType::MetaData(_) => 1,

‎beacon_node/network/src/network_beacon_processor/gossip_methods.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1130,7 +1130,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
11301130
let processing_start_time = Instant::now();
11311131
let block_root = verified_data_column.block_root();
11321132
let data_column_slot = verified_data_column.slot();
1133-
let data_column_index = verified_data_column.id().index;
1133+
let data_column_index = verified_data_column.index();
11341134

11351135
let result = self
11361136
.chain

‎beacon_node/network/src/network_beacon_processor/rpc_methods.rs

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -360,24 +360,25 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
360360
) -> Result<(), (RpcErrorResponse, &'static str)> {
361361
let mut send_data_column_count = 0;
362362

363-
for data_column_id in request.data_column_ids.as_slice() {
364-
match self.chain.get_data_column_checking_all_caches(
365-
data_column_id.block_root,
366-
data_column_id.index,
363+
for data_column_ids_by_root in request.data_column_ids.as_slice() {
364+
match self.chain.get_data_columns_checking_all_caches(
365+
data_column_ids_by_root.block_root,
366+
data_column_ids_by_root.columns.as_slice(),
367367
) {
368-
Ok(Some(data_column)) => {
369-
send_data_column_count += 1;
370-
self.send_response(
371-
peer_id,
372-
inbound_request_id,
373-
Response::DataColumnsByRoot(Some(data_column)),
374-
);
368+
Ok(data_columns) => {
369+
send_data_column_count += data_columns.len();
370+
for data_column in data_columns {
371+
self.send_response(
372+
peer_id,
373+
inbound_request_id,
374+
Response::DataColumnsByRoot(Some(data_column)),
375+
);
376+
}
375377
}
376-
Ok(None) => {} // no-op
377378
Err(e) => {
378379
// TODO(das): lower log level when feature is stabilized
379380
error!(
380-
block_root = ?data_column_id.block_root,
381+
block_root = ?data_column_ids_by_root.block_root,
381382
%peer_id,
382383
error = ?e,
383384
"Error getting data column"
@@ -389,7 +390,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
389390

390391
debug!(
391392
%peer_id,
392-
request = ?request.group_by_ordered_block_root(),
393+
request = ?request.data_column_ids,
393394
returned = send_data_column_count,
394395
"Received DataColumnsByRoot Request"
395396
);

‎beacon_node/network/src/sync/network_context.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -875,7 +875,11 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
875875

876876
self.send_network_msg(NetworkMessage::SendRequest {
877877
peer_id,
878-
request: RequestType::DataColumnsByRoot(request.clone().into_request(&self.chain.spec)),
878+
request: RequestType::DataColumnsByRoot(
879+
request
880+
.clone()
881+
.try_into_request(self.fork_context.current_fork(), &self.chain.spec)?,
882+
),
879883
app_request_id: AppRequestId::Sync(SyncRequestId::DataColumnsByRoot(id)),
880884
})?;
881885

0 commit comments

Comments
 (0)