From d0fe2ce03b9b033424f55aa9904f23d8f046a251 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Thu, 12 Oct 2023 15:46:47 -0400 Subject: [PATCH] add runtime variable list type --- .../src/rpc/codec/ssz_snappy.rs | 11 ++--- .../lighthouse_network/src/rpc/methods.rs | 13 +++--- .../network/src/sync/block_lookups/common.rs | 21 +++++----- .../network/src/sync/block_lookups/mod.rs | 3 ++ .../src/sync/block_lookups/parent_lookup.rs | 3 ++ .../sync/block_lookups/single_block_lookup.rs | 1 + consensus/types/src/fork_context.rs | 2 + consensus/types/src/lib.rs | 2 + consensus/types/src/runtime_var_list.rs | 40 +++++++++++++++++++ 9 files changed, 76 insertions(+), 20 deletions(-) create mode 100644 consensus/types/src/runtime_var_list.rs 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 2bcaec147bd..1a6045e7528 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs @@ -15,9 +15,9 @@ use std::io::{Read, Write}; use std::marker::PhantomData; use std::sync::Arc; use tokio_util::codec::{Decoder, Encoder}; -use types::{light_client_bootstrap::LightClientBootstrap, BlobSidecar}; +use types::{light_client_bootstrap::LightClientBootstrap, BlobSidecar, ChainSpec}; use types::{ - EthSpec, ForkContext, ForkName, Hash256, SignedBeaconBlock, SignedBeaconBlockAltair, + EthSpec, ForkContext, ForkName, Hash256, RuntimeVariableList, SignedBeaconBlock, SignedBeaconBlockAltair, SignedBeaconBlockBase, SignedBeaconBlockCapella, SignedBeaconBlockDeneb, SignedBeaconBlockMerge, }; @@ -163,7 +163,7 @@ impl Decoder for SSZSnappyInboundCodec { let n = reader.get_ref().get_ref().position(); self.len = None; let _read_bytes = src.split_to(n as usize); - handle_rpc_request(self.protocol.versioned_protocol, &decoded_buffer) + handle_rpc_request(self.protocol.versioned_protocol, &decoded_buffer, &self.fork_context) } Err(e) => handle_error(e, reader.get_ref().get_ref().position(), max_compressed_len), } @@ -455,6 +455,7 @@ fn handle_length( fn handle_rpc_request( versioned_protocol: SupportedProtocol, decoded_buffer: &[u8], + fork_context: &ForkContext ) -> Result>, RPCError> { match versioned_protocol { SupportedProtocol::StatusV1 => Ok(Some(InboundRequest::Status( @@ -471,12 +472,12 @@ fn handle_rpc_request( ))), SupportedProtocol::BlocksByRootV2 => Ok(Some(InboundRequest::BlocksByRoot( BlocksByRootRequest::V2(BlocksByRootRequestV2 { - block_roots: VariableList::from_ssz_bytes(decoded_buffer)?, + block_roots: RuntimeVariableList::from_ssz_bytes(decoded_buffer, 1)?, }), ))), SupportedProtocol::BlocksByRootV1 => Ok(Some(InboundRequest::BlocksByRoot( BlocksByRootRequest::V1(BlocksByRootRequestV1 { - block_roots: VariableList::from_ssz_bytes(decoded_buffer)?, + block_roots: RuntimeVariableList::from_ssz_bytes(decoded_buffer, 1)?, }), ))), SupportedProtocol::BlobsByRangeV1 => Ok(Some(InboundRequest::BlobsByRange( diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index 438c2c75447..e8ffe7f8564 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -3,7 +3,7 @@ use crate::types::{EnrAttestationBitfield, EnrSyncCommitteeBitfield}; use regex::bytes::Regex; use serde::Serialize; -use ssz::Encode; +use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use ssz_types::{ typenum::{U1024, U128, U256, U768}, @@ -18,7 +18,7 @@ use types::blob_sidecar::BlobIdentifier; use types::consts::deneb::MAX_BLOBS_PER_BLOCK; use types::{ blob_sidecar::BlobSidecar, light_client_bootstrap::LightClientBootstrap, Epoch, EthSpec, - Hash256, SignedBeaconBlock, Slot, + Hash256, SignedBeaconBlock, Slot, RuntimeVariableList }; /// Maximum number of blocks in a single request. @@ -343,23 +343,24 @@ impl OldBlocksByRangeRequest { } } + /// Request a number of beacon block bodies from a peer. #[superstruct( variants(V1, V2), - variant_attributes(derive(Encode, Decode, Clone, Debug, PartialEq)) + variant_attributes(derive(Clone, Debug, PartialEq)) )] #[derive(Clone, Debug, PartialEq)] pub struct BlocksByRootRequest { /// The list of beacon block bodies being requested. - pub block_roots: VariableList, + pub block_roots: RuntimeVariableList, } impl BlocksByRootRequest { - pub fn new(block_roots: VariableList) -> Self { + pub fn new(block_roots: RuntimeVariableList) -> Self { Self::V2(BlocksByRootRequestV2 { block_roots }) } - pub fn new_v1(block_roots: VariableList) -> Self { + pub fn new_v1(block_roots: RuntimeVariableList) -> Self { Self::V1(BlocksByRootRequestV1 { block_roots }) } } diff --git a/beacon_node/network/src/sync/block_lookups/common.rs b/beacon_node/network/src/sync/block_lookups/common.rs index 7f141edb5b1..b238192f137 100644 --- a/beacon_node/network/src/sync/block_lookups/common.rs +++ b/beacon_node/network/src/sync/block_lookups/common.rs @@ -20,7 +20,7 @@ use std::ops::IndexMut; use std::sync::Arc; use std::time::Duration; use types::blob_sidecar::{BlobIdentifier, FixedBlobSidecarList}; -use types::{BlobSidecar, EthSpec, Hash256, SignedBeaconBlock}; +use types::{BlobSidecar, ChainSpec, EthSpec, Hash256, RuntimeVariableList ,SignedBeaconBlock}; #[derive(Debug, Copy, Clone)] pub enum ResponseType { @@ -89,11 +89,11 @@ pub trait RequestState { /* Request building methods */ /// Construct a new request. - fn build_request(&mut self) -> Result<(PeerShouldHave, Self::RequestType), LookupRequestError> { + fn build_request(&mut self, spec: &ChainSpec) -> Result<(PeerShouldHave, Self::RequestType), LookupRequestError> { // Verify and construct request. self.too_many_attempts()?; let peer = self.get_peer()?; - let request = self.new_request(); + let request = self.new_request(spec)?; Ok((peer, request)) } @@ -110,7 +110,7 @@ pub trait RequestState { } // Construct request. - let (peer_id, request) = self.build_request()?; + let (peer_id, request) = self.build_request(&cx.chain.spec)?; // Update request state. self.get_state_mut().state = State::Downloading { peer_id }; @@ -164,7 +164,7 @@ pub trait RequestState { } /// Initialize `Self::RequestType`. - fn new_request(&self) -> Self::RequestType; + fn new_request(&self, spec: &ChainSpec) -> Result; /// Send the request to the network service. fn make_request( @@ -272,8 +272,11 @@ impl RequestState for BlockRequestState type VerifiedResponseType = Arc>; type ReconstructedResponseType = RpcBlock; - fn new_request(&self) -> BlocksByRootRequest { - BlocksByRootRequest::new(VariableList::from(vec![self.requested_block_root])) + fn new_request(&self, spec: &ChainSpec) -> Result { + Ok(BlocksByRootRequest::new(RuntimeVariableList::new(vec![self.requested_block_root], 1) + .map_err(|_e|{ + LookupRequestError::SszError("invalid request length") + })?)) } fn make_request( @@ -376,10 +379,10 @@ impl RequestState for BlobRequestState; type ReconstructedResponseType = FixedBlobSidecarList; - fn new_request(&self) -> BlobsByRootRequest { + fn new_request(&self, spec: &ChainSpec) -> Result { let blob_id_vec: Vec = self.requested_ids.clone().into(); let blob_ids = VariableList::from(blob_id_vec); - BlobsByRootRequest { blob_ids } + Ok(BlobsByRootRequest { blob_ids }) } fn make_request( diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index fd9d0d6e0e4..974f617fd56 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -642,6 +642,9 @@ impl BlockLookups { RequestError::SendFailed(_) => { // Probably shutting down, nothing to do here. Drop the request } + RequestError::SszError(e) => { + warn!(self.log, "Invalid parent request constructed"; "reason" => %e); + } RequestError::ChainTooLong => { self.failed_chains.insert(parent_lookup.chain_hash()); // This indicates faulty peers. diff --git a/beacon_node/network/src/sync/block_lookups/parent_lookup.rs b/beacon_node/network/src/sync/block_lookups/parent_lookup.rs index 93bd2f57c09..1baae1c20f3 100644 --- a/beacon_node/network/src/sync/block_lookups/parent_lookup.rs +++ b/beacon_node/network/src/sync/block_lookups/parent_lookup.rs @@ -55,6 +55,7 @@ pub enum RequestError { cannot_process: bool, }, NoPeers, + SszError(&'static str) } impl ParentLookup { @@ -262,6 +263,7 @@ impl From for RequestError { } E::NoPeers => RequestError::NoPeers, E::SendFailed(msg) => RequestError::SendFailed(msg), + E::SszError(msg) => RequestError::SszError(msg), } } } @@ -289,6 +291,7 @@ impl RequestError { } RequestError::TooManyAttempts { cannot_process: _ } => "too_many_downloading_attempts", RequestError::NoPeers => "no_peers", + RequestError::SszError(e) => e, } } } diff --git a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs index e0f7d880949..2a2b2391cc5 100644 --- a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs +++ b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs @@ -50,6 +50,7 @@ pub enum LookupRequestError { }, NoPeers, SendFailed(&'static str), + SszError(&'static str), } pub struct SingleBlockLookup { diff --git a/consensus/types/src/fork_context.rs b/consensus/types/src/fork_context.rs index 23163f0eecb..cf4218a6319 100644 --- a/consensus/types/src/fork_context.rs +++ b/consensus/types/src/fork_context.rs @@ -9,6 +9,7 @@ pub struct ForkContext { current_fork: RwLock, fork_to_digest: HashMap, digest_to_fork: HashMap<[u8; 4], ForkName>, + spec: ChainSpec, } impl ForkContext { @@ -73,6 +74,7 @@ impl ForkContext { current_fork: RwLock::new(spec.fork_name_at_slot::(current_slot)), fork_to_digest, digest_to_fork, + spec: spec.clone(), } } diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 26541a188c6..a7b5ba77876 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -101,6 +101,7 @@ pub mod sqlite; pub mod blob_sidecar; pub mod sidecar; pub mod signed_blob; +pub mod runtime_var_list; use ethereum_types::{H160, H256}; @@ -168,6 +169,7 @@ pub use crate::preset::{AltairPreset, BasePreset, BellatrixPreset, CapellaPreset pub use crate::proposer_preparation_data::ProposerPreparationData; pub use crate::proposer_slashing::ProposerSlashing; pub use crate::relative_epoch::{Error as RelativeEpochError, RelativeEpoch}; +pub use crate::runtime_var_list::RuntimeVariableList; pub use crate::selection_proof::SelectionProof; pub use crate::shuffling_id::AttestationShufflingId; pub use crate::signed_aggregate_and_proof::SignedAggregateAndProof; diff --git a/consensus/types/src/runtime_var_list.rs b/consensus/types/src/runtime_var_list.rs new file mode 100644 index 00000000000..87d0fc20be4 --- /dev/null +++ b/consensus/types/src/runtime_var_list.rs @@ -0,0 +1,40 @@ +use ssz::{Decode, Encode}; +use ssz_derive::Encode; + +#[derive(Debug, Clone, PartialEq, Encode)] +#[ssz(struct_behaviour = "transparent")] +pub struct RuntimeVariableList { + vec: Vec, + #[ssz(skip_serializing, skip_deserializing)] + max_len: usize, +} + +impl RuntimeVariableList { + pub fn new(vec: Vec, max_len: usize) -> Result { + if vec.len() <= max_len { + Ok(Self { vec, max_len }) + } else { + Err(ssz_types::Error::OutOfBounds { + i: vec.len(), + len: max_len, + }) + } + } + + pub fn to_vec(&self)-> Vec { + self.vec.clone() + } + + pub fn len(&self) -> usize { + self.vec.len() + } + + pub fn from_ssz_bytes(bytes: &[u8], max_len: usize) -> Result { + let vec = if bytes.is_empty() { + vec![] + } else { + ssz::decode_list_of_variable_length_items(bytes, Some(max_len))? + }; + Ok(Self { vec, max_len }) + } +} \ No newline at end of file