From 5c0ccefab1a33f59fbc8ffa804ab641cdef6d3a6 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 27 Jun 2024 06:15:18 +1000 Subject: [PATCH 1/2] Fix csc encoding and decoding (#5997) --- beacon_node/lighthouse_network/src/discovery/enr.rs | 11 +++++------ .../lighthouse_network/src/peer_manager/peerdb.rs | 5 +---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/beacon_node/lighthouse_network/src/discovery/enr.rs b/beacon_node/lighthouse_network/src/discovery/enr.rs index fc8d3809e2..115c36b207 100644 --- a/beacon_node/lighthouse_network/src/discovery/enr.rs +++ b/beacon_node/lighthouse_network/src/discovery/enr.rs @@ -67,8 +67,8 @@ impl Eth2Enr for Enr { /// if the custody value is non-existent in the ENR, then we assume the minimum custody value /// defined in the spec. fn custody_subnet_count(&self, spec: &ChainSpec) -> u64 { - self.get(PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY) - .and_then(|custody_bytes| custody_bytes.try_into().map(u64::from_be_bytes).ok()) + self.get_decodable::(PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY) + .and_then(|r| r.ok()) // If value supplied in ENR is invalid, fallback to `custody_requirement` .filter(|csc| csc <= &spec.data_column_sidecar_subnet_count) .unwrap_or(spec.custody_requirement) @@ -245,8 +245,7 @@ pub fn build_enr( spec.custody_requirement }; - let csc_bytes = custody_subnet_count.to_be_bytes(); - builder.add_value(PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY, &csc_bytes.as_slice()); + builder.add_value(PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY, &custody_subnet_count); builder .build(enr_key) @@ -353,11 +352,11 @@ mod test { let config = NetworkConfig::default(); let spec = E::default_spec(); let (mut enr, enr_key) = build_enr_with_config(config, &spec); - let invalid_subnet_count = 999u64; + let invalid_subnet_count = 99u64; enr.insert( PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY, - &invalid_subnet_count.to_be_bytes().as_slice(), + &invalid_subnet_count, &enr_key, ) .unwrap(); diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs index 66ba098039..d0c6710e8a 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs @@ -686,10 +686,7 @@ impl PeerDB { if supernode { enr.insert( PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY, - &spec - .data_column_sidecar_subnet_count - .to_be_bytes() - .as_slice(), + &spec.data_column_sidecar_subnet_count, &enr_key, ) .expect("u64 can be encoded"); From 7206909fbc8ac82115ce7f7c3bcb1348e5dd0d2a Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 27 Jun 2024 06:15:56 +1000 Subject: [PATCH 2/2] Fix data column rpc request not being sent due to incorrect limits set. (#6000) --- .../src/rpc/codec/ssz_snappy.rs | 58 ++++++++++++++++++- .../lighthouse_network/src/rpc/methods.rs | 20 +++++++ .../lighthouse_network/src/rpc/protocol.rs | 4 +- 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs b/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs index 74751c604b..3a4ddef0bc 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs @@ -801,7 +801,8 @@ mod tests { use crate::types::{EnrAttestationBitfield, EnrSyncCommitteeBitfield}; use types::{ blob_sidecar::BlobIdentifier, BeaconBlock, BeaconBlockAltair, BeaconBlockBase, - BeaconBlockBellatrix, EmptyBlock, Epoch, FullPayload, Signature, Slot, + BeaconBlockBellatrix, DataColumnIdentifier, EmptyBlock, Epoch, FullPayload, Signature, + Slot, }; type Spec = types::MainnetEthSpec; @@ -848,6 +849,10 @@ mod tests { Arc::new(BlobSidecar::empty()) } + fn empty_data_column_sidecar() -> Arc> { + Arc::new(DataColumnSidecar::empty()) + } + /// Bellatrix block with length < max_rpc_size. fn bellatrix_block_small( fork_context: &ForkContext, @@ -909,6 +914,27 @@ mod tests { } } + fn dcbrange_request() -> DataColumnsByRangeRequest { + DataColumnsByRangeRequest { + start_slot: 0, + count: 10, + columns: vec![1, 2, 3], + } + } + + fn dcbroot_request(spec: &ChainSpec) -> DataColumnsByRootRequest { + DataColumnsByRootRequest { + data_column_ids: RuntimeVariableList::new( + vec![DataColumnIdentifier { + block_root: Hash256::zero(), + index: 0, + }], + spec.max_request_data_column_sidecars as usize, + ) + .unwrap(), + } + } + fn bbroot_request_v1(spec: &ChainSpec) -> BlocksByRootRequest { BlocksByRootRequest::new_v1(vec![Hash256::zero()], spec) } @@ -1198,6 +1224,34 @@ mod tests { ), Ok(Some(RPCResponse::BlobsByRoot(empty_blob_sidecar()))), ); + + assert_eq!( + encode_then_decode_response( + SupportedProtocol::DataColumnsByRangeV1, + RPCCodedResponse::Success(RPCResponse::DataColumnsByRange( + empty_data_column_sidecar() + )), + ForkName::Deneb, + &chain_spec + ), + Ok(Some(RPCResponse::DataColumnsByRange( + empty_data_column_sidecar() + ))), + ); + + assert_eq!( + encode_then_decode_response( + SupportedProtocol::DataColumnsByRootV1, + RPCCodedResponse::Success(RPCResponse::DataColumnsByRoot( + empty_data_column_sidecar() + )), + ForkName::Deneb, + &chain_spec + ), + Ok(Some(RPCResponse::DataColumnsByRoot( + empty_data_column_sidecar() + ))), + ); } // Test RPCResponse encoding/decoding for V1 messages @@ -1551,6 +1605,8 @@ mod tests { OutboundRequest::MetaData(MetadataRequest::new_v1()), OutboundRequest::BlobsByRange(blbrange_request()), OutboundRequest::BlobsByRoot(blbroot_request(&chain_spec)), + OutboundRequest::DataColumnsByRange(dcbrange_request()), + OutboundRequest::DataColumnsByRoot(dcbroot_request(&chain_spec)), OutboundRequest::MetaData(MetadataRequest::new_v2()), ]; diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index a892b7f07c..f7a92ac29f 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -312,6 +312,26 @@ impl DataColumnsByRangeRequest { .saturating_mul(E::max_blobs_per_block() as u64) .saturating_mul(self.columns.len() as u64) } + + pub fn ssz_min_len() -> usize { + DataColumnsByRangeRequest { + start_slot: 0, + count: 0, + columns: vec![0], + } + .as_ssz_bytes() + .len() + } + + pub fn ssz_max_len(spec: &ChainSpec) -> usize { + DataColumnsByRangeRequest { + start_slot: 0, + count: 0, + columns: vec![0; spec.number_of_columns], + } + .as_ssz_bytes() + .len() + } } /// Request a number of beacon block roots from a peer. diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index 6dcd1eabd8..65a0e3e88d 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -496,8 +496,8 @@ impl ProtocolId { Protocol::BlobsByRoot => RpcLimits::new(0, spec.max_blobs_by_root_request), Protocol::DataColumnsByRoot => RpcLimits::new(0, spec.max_data_columns_by_root_request), Protocol::DataColumnsByRange => RpcLimits::new( - ::ssz_fixed_len(), - ::ssz_fixed_len(), + DataColumnsByRangeRequest::ssz_min_len(), + DataColumnsByRangeRequest::ssz_max_len(spec), ), Protocol::Ping => RpcLimits::new( ::ssz_fixed_len(),