diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 2b90fcb63a..8be199d719 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -7,12 +7,15 @@ use crate::attester_cache::{AttesterCache, AttesterCacheKey}; use crate::beacon_proposer_cache::compute_proposer_duties_from_head; use crate::beacon_proposer_cache::BeaconProposerCache; use crate::blob_cache::BlobCache; -use crate::blob_verification::{AsBlock, AvailabilityPendingBlock, AvailableBlock, BlobError, BlockWrapper, IntoAvailableBlock, Blobs}; +use crate::blob_verification::{ + AsBlock, AvailabilityPendingBlock, AvailableBlock, BlobError, Blobs, BlockWrapper, + IntoAvailableBlock, +}; use crate::block_times_cache::BlockTimesCache; use crate::block_verification::{ check_block_is_finalized_checkpoint_or_descendant, check_block_relevancy, get_block_root, - signature_verify_chain_segment, BlockError, ExecutionPendingBlock, GossipVerifiedBlock, - IntoExecutionPendingBlock, PayloadVerificationOutcome, POS_PANDA_BANNER, ExecutedBlock, + signature_verify_chain_segment, BlockError, ExecutedBlock, ExecutionPendingBlock, + GossipVerifiedBlock, IntoExecutionPendingBlock, PayloadVerificationOutcome, POS_PANDA_BANNER, }; pub use crate::canonical_head::{CanonicalHead, CanonicalHeadRwLock}; use crate::chain_config::ChainConfig; @@ -82,6 +85,7 @@ use slasher::Slasher; use slog::{crit, debug, error, info, trace, warn, Logger}; use slot_clock::SlotClock; use ssz::Encode; +use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; use state_processing::{ common::get_attesting_indices_from_state, per_block_processing, @@ -102,16 +106,14 @@ use std::io::prelude::*; use std::marker::PhantomData; use std::sync::Arc; use std::time::{Duration, Instant}; -use tokio::task::JoinHandle; -use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; use store::iter::{BlockRootsIterator, ParentRootBlockIterator, StateRootsIterator}; use store::{ DatabaseBlock, Error as DBError, HotColdDB, KeyValueStore, KeyValueStoreOp, StoreItem, StoreOp, }; use task_executor::{ShutdownReason, TaskExecutor}; +use tokio::task::JoinHandle; use tree_hash::TreeHash; use types::beacon_state::CloneConfig; -use types::blobs_sidecar::KzgCommitments; use types::consts::eip4844::MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS; use types::consts::merge::INTERVALS_PER_SLOT; use types::*; @@ -2709,7 +2711,6 @@ impl BeaconChain { let slot = unverified_block.block().slot(); - let execution_pending = unverified_block.into_execution_pending_block( block_root, &chain, @@ -2723,9 +2724,13 @@ impl BeaconChain { let chain = self.clone(); - // Check if the executed block has all it's blobs available to qualify as a fully + // Check if the executed block has all it's blobs available to qualify as a fully // available block - let import_block = if let Ok(blobs) = self.gossip_blob_cache.lock().blobs(executed_block.block_root) { + let import_block = if let Ok(blobs) = self + .gossip_blob_cache + .lock() + .blobs(executed_block.block_root) + { self.import_available_block(executed_block, blobs, count_unrealized) } else { return Ok(BlockProcessingResult::AvailabilityPending(executed_block)); @@ -2839,7 +2844,7 @@ impl BeaconChain { confirmed_state_roots, parent_eth1_finalization_data, consensus_context, - payload_verification_outcome + payload_verification_outcome, }) } @@ -2851,7 +2856,7 @@ impl BeaconChain { async fn import_available_block( self: Arc, executed_block: ExecutedBlock, - blobs: Blobs + blobs: Blobs, count_unrealized: CountUnrealized, ) -> Result> { let ExecutedBlock { @@ -2865,14 +2870,13 @@ impl BeaconChain { consensus_context, } = execution_pending_block; - let chain = self.clone(); - + let available_block = AvailableBlock { - block: block, - blobs: blobs + block: block, + blobs: blobs, }; - + let block_hash = self .spawn_blocking_handle( move || { @@ -4911,7 +4915,7 @@ impl BeaconChain { )), )?; - kzg_utils::compute_blob_kzg_proof::(kzg, blob, kzg_commitment.clone()) + kzg_utils::compute_blob_kzg_proof::(kzg, blob, *kzg_commitment) .map_err(BlockProductionError::KzgError) }) .collect::, BlockProductionError>>() @@ -6196,19 +6200,22 @@ impl BeaconChain { .unwrap_or(false)) } - pub async fn check_data_availability(&self, block: Arc>) -> Result, Error> { - let kzg_commitments = block - .message() - .body() - .blob_kzg_commitments() - .map_err(|_| BlobError::KzgCommitmentMissing)?; - let transactions = block - .message() - .body() - .execution_payload_eip4844() - .map(|payload| payload.transactions()) - .map_err(|_| BlobError::TransactionsMissing)? - .ok_or(BlobError::TransactionsMissing)?; + pub async fn check_data_availability( + &self, + block: Arc>, + ) -> Result, Error> { + let kzg_commitments = block + .message() + .body() + .blob_kzg_commitments() + .map_err(|_| BlobError::KzgCommitmentMissing)?; + let transactions = block + .message() + .body() + .execution_payload_eip4844() + .map(|payload| payload.transactions()) + .map_err(|_| BlobError::TransactionsMissing)? + .ok_or(BlobError::TransactionsMissing)?; if verify_kzg_commitments_against_transactions::(transactions, kzg_commitments) .is_err() @@ -6229,7 +6236,7 @@ impl BeaconChain { kzg_commitments, blob_sidecar, ) - .map_err(BlobError::KzgError)? + .map_err(BlobError::KzgError)? { return Err(BlobError::InvalidKzgProof); } diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index bfc0242208..50c58391e9 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -1,8 +1,8 @@ use derivative::Derivative; use slot_clock::SlotClock; +use ssz_types::VariableList; use std::sync::Arc; use tokio::task::JoinHandle; -use ssz_types::VariableList; use crate::beacon_chain::{ BeaconChain, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY, @@ -10,13 +10,13 @@ use crate::beacon_chain::{ }; use crate::BeaconChainError; use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; +use types::blob_sidecar::BlobSidecar; use types::{ BeaconBlockRef, BeaconStateError, BlobsSidecar, EthSpec, Hash256, KzgCommitment, SignedBeaconBlock, SignedBeaconBlockAndBlobsSidecar, SignedBeaconBlockHeader, SignedBlobSidecar, Slot, Transactions, }; use types::{Epoch, ExecPayload}; -use types::blob_sidecar::BlobSidecar; #[derive(Debug)] pub enum BlobError { @@ -252,8 +252,8 @@ pub fn verify_data_availability( blob_sidecar: &BlobsSidecar, kzg_commitments: &[KzgCommitment], transactions: &Transactions, - block_slot: Slot, - block_root: Hash256, + _block_slot: Slot, + _block_root: Hash256, chain: &BeaconChain, ) -> Result<(), BlobError> { if verify_kzg_commitments_against_transactions::(transactions, kzg_commitments) @@ -263,7 +263,7 @@ pub fn verify_data_availability( } // Validatate that the kzg proof is valid against the commitments and blobs - let kzg = chain + let _kzg = chain .kzg .as_ref() .ok_or(BlobError::TrustedSetupNotInitialized)?; @@ -300,8 +300,6 @@ impl BlockWrapper { AvailabilityPendingBlock::new(block, block_root, da_check_required) } BlockWrapper::BlockAndBlobs(block, blobs_sidecar) => { - - AvailabilityPendingBlock::new_with_blobs(block, blobs_sidecar, da_check_required) } } @@ -336,22 +334,23 @@ pub struct AvailableBlock { blobs: Blobs, } -impl AvailableBlock { +impl AvailableBlock { pub fn blobs(&self) -> Option>> { match &self.blobs { Blobs::NotRequired | Blobs::None => None, - Blobs::Available(block_sidecar) => { - Some(block_sidecar.clone()) - } + Blobs::Available(block_sidecar) => Some(block_sidecar.clone()), } } - pub fn deconstruct(self) -> (Arc>, Option>>) { + pub fn deconstruct( + self, + ) -> ( + Arc>, + Option>>, + ) { match self.blobs { Blobs::NotRequired | Blobs::None => (self.block, None), - Blobs::Available(blob_sidecars) => { - (self.block, Some(blob_sidecars)) - } + Blobs::Available(blob_sidecars) => (self.block, Some(blob_sidecars)), } } } @@ -388,12 +387,10 @@ impl AvailabilityPendingBlock { SignedBeaconBlock::Base(_) | SignedBeaconBlock::Altair(_) | SignedBeaconBlock::Capella(_) - | SignedBeaconBlock::Merge(_) => { - Ok(AvailabilityPendingBlock { - block: beacon_block , - data_availability_handle: async{ Ok(Some(Blobs::NotRequired))} - }) - } + | SignedBeaconBlock::Merge(_) => Ok(AvailabilityPendingBlock { + block: beacon_block, + data_availability_handle: async { Ok(Some(Blobs::NotRequired)) }, + }), SignedBeaconBlock::Eip4844(_) => { match da_check_required { DataAvailabilityCheckRequired::Yes => { @@ -408,12 +405,10 @@ impl AvailabilityPendingBlock { }, ))) } - DataAvailabilityCheckRequired::No => { - AvailabilityPendingBlock { - block: beacon_block, - data_availability_handle: async{ Ok(Some(Blobs::NotRequired))} - } - } + DataAvailabilityCheckRequired::No => AvailabilityPendingBlock { + block: beacon_block, + data_availability_handle: async { Ok(Some(Blobs::NotRequired)) }, + }, } } } @@ -444,24 +439,22 @@ impl AvailabilityPendingBlock { | SignedBeaconBlock::Merge(_) => Err(BlobError::InconsistentFork), SignedBeaconBlock::Eip4844(_) => { match da_check_required { - DataAvailabilityCheckRequired::Yes => Ok(AvailableBlock{ - block: beacon_block, - blobs: Blobs::Available(blobs_sidecar), - } - ), + DataAvailabilityCheckRequired::Yes => Ok(AvailableBlock { + block: beacon_block, + blobs: Blobs::Available(blobs_sidecar), + }), DataAvailabilityCheckRequired::No => { // Blobs were not verified so we drop them, we'll instead just pass around // an available `Eip4844` block without blobs. - Ok(AvailableBlock{ - block: beacon_block, - blobs: Blobs::NotRequired + Ok(AvailableBlock { + block: beacon_block, + blobs: Blobs::NotRequired, }) } } } } } - } pub trait IntoBlockWrapper: AsBlock { diff --git a/beacon_node/beacon_chain/src/gossip_blob_cache.rs b/beacon_node/beacon_chain/src/gossip_blob_cache.rs index c76d122a78..fa615a9c2a 100644 --- a/beacon_node/beacon_chain/src/gossip_blob_cache.rs +++ b/beacon_node/beacon_chain/src/gossip_blob_cache.rs @@ -1,19 +1,19 @@ +use crate::blob_verification::{verify_data_availability, AvailabilityPendingBlock}; +use crate::block_verification::{ExecutedBlock, IntoExecutionPendingBlock}; +use crate::kzg_utils::validate_blob; +use crate::{BeaconChainError, BlockError}; +use eth2::reqwest::header::Entry; +use kzg::{Kzg, KzgCommitment}; +use parking_lot::{Mutex, RwLock}; +use ssz_types::VariableList; use std::collections::{BTreeMap, HashMap, HashSet}; use std::future::Future; use std::sync::Arc; -use parking_lot::{Mutex, RwLock}; -use eth2::reqwest::header::Entry; -use kzg::{Kzg, KzgCommitment}; -use ssz_types::VariableList; use types::blob_sidecar::{BlobIdentifier, BlobSidecar}; use types::{EthSpec, Hash256}; -use crate::blob_verification::{AvailabilityPendingBlock, verify_data_availability}; -use crate::block_verification::{ExecutedBlock, IntoExecutionPendingBlock}; -use crate::{BeaconChainError, BlockError}; -use crate::kzg_utils::validate_blob; pub enum BlobCacheError { - DuplicateBlob(Hash256) + DuplicateBlob(Hash256), } /// This cache contains /// - blobs that have been gossip verified @@ -31,14 +31,13 @@ struct GossipBlobCacheInner { executed_block: Option>, } -impl GossipBlobCache { +impl GossipBlobCache { pub fn new(kzg: Kzg) -> Self { Self { rpc_blob_cache: RwLock::new(HashMap::new()), gossip_blob_cache: Mutex::new(HashMap::new()), kzg, } - } /// When we receive a blob check if we've cached it. If it completes a set and we have the @@ -46,29 +45,36 @@ impl GossipBlobCache { /// cached, verify the block and import it. /// /// This should only accept gossip verified blobs, so we should not have to worry about dupes. - pub fn put_blob(&self, blob: Arc>) -> Result<(),BlobCacheError> { - + pub fn put_blob(&self, blob: Arc>) -> Result<(), BlobCacheError> { // TODO(remove clones) - let verified = validate_blob(&self.kzg, blob.blob.clone(), blob.kzg_commitment.clone(), blob.kzg_proof)?; + let verified = validate_blob( + &self.kzg, + blob.blob.clone(), + blob.kzg_commitment.clone(), + blob.kzg_proof, + )?; if verified { let mut blob_cache = self.gossip_blob_cache.lock(); // Gossip cache. - blob_cache.entry(blob.block_root) + blob_cache + .entry(blob.block_root) .and_modify(|mut inner| { // All blobs reaching this cache should be gossip verified and gossip verification // should filter duplicates, as well as validate indices. - inner.verified_blobs.insert(blob.index as usize, blob.clone()); + inner + .verified_blobs + .insert(blob.index as usize, blob.clone()); - if let Some (executed_block) = inner.executed_block.as_ref() { + if let Some(executed_block) = inner.executed_block.as_ref() { // trigger reprocessing ? } }) .or_insert(GossipBlobCacheInner { - verified_blobs: vec![blob.clone()], - executed_block: None - }); + verified_blobs: vec![blob.clone()], + executed_block: None, + }); drop(blob_cache); @@ -80,18 +86,21 @@ impl GossipBlobCache { } pub fn put_block(&self, block: ExecutedBlock) -> () { - let mut guard = self.gossip_blob_cache.lock(); - guard.entry(block.block_root).and_modify(|cache|{ - if cache.verified_blobs == block.block.message_eip4844().blob_kzg_commitments() { - // send to reprocessing queue ? - } else if let Some(dup) = cache.executed_block.insert(block) { + let mut guard = self.gossip_blob_cache.lock(); + guard + .entry(block.block_root) + .and_modify(|cache| { + if cache.verified_blobs == block.block.message_eip4844().blob_kzg_commitments() { + // send to reprocessing queue ? + } else if let Some(dup) = cache.executed_block.insert(block) { // return error } else { // log that we cached it } - }).or_insert(GossipBlobCacheInner { - verified_blobs: vec![], - executed_block: Some(block) - }); + }) + .or_insert(GossipBlobCacheInner { + verified_blobs: vec![], + executed_block: Some(block), + }); } } diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 74b641f352..865539a7b6 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -23,6 +23,7 @@ pub mod events; pub mod execution_payload; pub mod fork_choice_signal; pub mod fork_revert; +pub mod gossip_blob_cache; mod head_tracker; pub mod historical_blocks; pub mod kzg_utils; @@ -51,7 +52,6 @@ pub mod test_utils; mod timeout_rw_lock; pub mod validator_monitor; pub mod validator_pubkey_cache; -pub mod gossip_blob_cache; pub use self::beacon_chain::{ AttestationProcessingOutcome, BeaconChain, BeaconChainTypes, BeaconStore, ChainSegmentResult, diff --git a/beacon_node/execution_layer/src/engine_api/json_structures.rs b/beacon_node/execution_layer/src/engine_api/json_structures.rs index ed9a58f266..658022e119 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -2,7 +2,6 @@ use super::*; use serde::{Deserialize, Serialize}; use strum::EnumString; use superstruct::superstruct; -use types::blobs_sidecar::KzgCommitments; use types::{ Blobs, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadCapella, ExecutionPayloadEip4844, ExecutionPayloadMerge, FixedVector, Transaction, Unsigned, diff --git a/beacon_node/http_api/src/build_block_contents.rs b/beacon_node/http_api/src/build_block_contents.rs index 1908c03ea1..9fbde0ce06 100644 --- a/beacon_node/http_api/src/build_block_contents.rs +++ b/beacon_node/http_api/src/build_block_contents.rs @@ -24,9 +24,9 @@ pub fn build_block_contents>(slot_d, &randao_reveal, None) .await .unwrap() - .data; + .data + .deconstruct() + .0; // Head is now B. assert_eq!( diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 977c737fd0..dae17006bc 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -61,8 +61,8 @@ struct ApiTester { harness: Arc>>, chain: Arc>>, client: BeaconNodeHttpClient, - next_block: SignedBeaconBlock, - reorg_block: SignedBeaconBlock, + next_block: SignedBlockContents, + reorg_block: SignedBlockContents, attestations: Vec>, contribution_and_proofs: Vec>, attester_slashing: AttesterSlashing, @@ -154,11 +154,13 @@ impl ApiTester { let (next_block, _next_state) = harness .make_block(head.beacon_state.clone(), harness.chain.slot().unwrap()) .await; + let next_block = SignedBlockContents::from(next_block); // `make_block` adds random graffiti, so this will produce an alternate block let (reorg_block, _reorg_state) = harness .make_block(head.beacon_state.clone(), harness.chain.slot().unwrap()) .await; + let reorg_block = SignedBlockContents::from(reorg_block); let head_state_root = head.beacon_state_root(); let attestations = harness @@ -288,11 +290,13 @@ impl ApiTester { let (next_block, _next_state) = harness .make_block(head.beacon_state.clone(), harness.chain.slot().unwrap()) .await; + let next_block = SignedBlockContents::from(next_block); // `make_block` adds random graffiti, so this will produce an alternate block let (reorg_block, _reorg_state) = harness .make_block(head.beacon_state.clone(), harness.chain.slot().unwrap()) .await; + let reorg_block = SignedBlockContents::from(reorg_block); let head_state_root = head.beacon_state_root(); let attestations = harness @@ -975,9 +979,9 @@ impl ApiTester { } pub async fn test_post_beacon_blocks_valid(mut self) -> Self { - let next_block = &self.next_block; + let next_block = self.next_block.clone(); - self.client.post_beacon_blocks(next_block).await.unwrap(); + self.client.post_beacon_blocks(&next_block).await.unwrap(); assert!( self.network_rx.network_recv.recv().await.is_some(), @@ -988,10 +992,14 @@ impl ApiTester { } pub async fn test_post_beacon_blocks_invalid(mut self) -> Self { - let mut next_block = self.next_block.clone(); + let mut next_block = self.next_block.clone().deconstruct().0; *next_block.message_mut().proposer_index_mut() += 1; - assert!(self.client.post_beacon_blocks(&next_block).await.is_err()); + assert!(self + .client + .post_beacon_blocks(&SignedBlockContents::from(next_block)) + .await + .is_err()); assert!( self.network_rx.network_recv.recv().await.is_some(), @@ -2065,11 +2073,17 @@ impl ApiTester { .get_validator_blocks::>(slot, &randao_reveal, None) .await .unwrap() - .data; + .data + .deconstruct() + .0; let signed_block = block.sign(&sk, &fork, genesis_validators_root, &self.chain.spec); + let signed_block_contents = SignedBlockContents::from(signed_block.clone()); - self.client.post_beacon_blocks(&signed_block).await.unwrap(); + self.client + .post_beacon_blocks(&signed_block_contents) + .await + .unwrap(); assert_eq!(self.chain.head_beacon_block().as_ref(), &signed_block); @@ -2093,7 +2107,9 @@ impl ApiTester { ) .await .unwrap() - .data; + .data + .deconstruct() + .0; assert_eq!(block.slot(), slot); self.chain.slot_clock.set_slot(slot.as_u64() + 1); } @@ -3760,12 +3776,12 @@ impl ApiTester { // Submit the next block, which is on an epoch boundary, so this will produce a finalized // checkpoint event, head event, and block event - let block_root = self.next_block.canonical_root(); + let block_root = self.next_block.signed_block().canonical_root(); // current_duty_dependent_root = block root because this is the first slot of the epoch let current_duty_dependent_root = self.chain.head_beacon_block_root(); let current_slot = self.chain.slot().unwrap(); - let next_slot = self.next_block.slot(); + let next_slot = self.next_block.signed_block().slot(); let finalization_distance = E::slots_per_epoch() * 2; let expected_block = EventKind::Block(SseBlock { @@ -3777,7 +3793,7 @@ impl ApiTester { let expected_head = EventKind::Head(SseHead { block: block_root, slot: next_slot, - state: self.next_block.state_root(), + state: self.next_block.signed_block().state_root(), current_duty_dependent_root, previous_duty_dependent_root: self .chain @@ -3826,13 +3842,17 @@ impl ApiTester { .unwrap(); let expected_reorg = EventKind::ChainReorg(SseChainReorg { - slot: self.next_block.slot(), + slot: self.next_block.signed_block().slot(), depth: 1, - old_head_block: self.next_block.canonical_root(), - old_head_state: self.next_block.state_root(), - new_head_block: self.reorg_block.canonical_root(), - new_head_state: self.reorg_block.state_root(), - epoch: self.next_block.slot().epoch(E::slots_per_epoch()), + old_head_block: self.next_block.signed_block().canonical_root(), + old_head_state: self.next_block.signed_block().state_root(), + new_head_block: self.reorg_block.signed_block().canonical_root(), + new_head_state: self.reorg_block.signed_block().state_root(), + epoch: self + .next_block + .signed_block() + .slot() + .epoch(E::slots_per_epoch()), execution_optimistic: false, }); @@ -3894,8 +3914,8 @@ impl ApiTester { .await .unwrap(); - let block_root = self.next_block.canonical_root(); - let next_slot = self.next_block.slot(); + let block_root = self.next_block.signed_block().canonical_root(); + let next_slot = self.next_block.signed_block().slot(); let expected_block = EventKind::Block(SseBlock { block: block_root, @@ -3906,7 +3926,7 @@ impl ApiTester { let expected_head = EventKind::Head(SseHead { block: block_root, slot: next_slot, - state: self.next_block.state_root(), + state: self.next_block.signed_block().state_root(), current_duty_dependent_root: self.chain.genesis_block_root, previous_duty_dependent_root: self.chain.genesis_block_root, epoch_transition: false, diff --git a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs index 4d7c949dc0..0b64725544 100644 --- a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs @@ -652,12 +652,12 @@ impl Worker { #[allow(clippy::too_many_arguments)] pub async fn process_gossip_blob( self, - message_id: MessageId, + _message_id: MessageId, peer_id: PeerId, peer_client: Client, blob_index: u64, signed_blob: Arc>, - seen_duration: Duration, + _seen_duration: Duration, ) { // TODO: gossip verification crit!(self.log, "UNIMPLEMENTED gossip blob verification"; diff --git a/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs b/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs index 4480f37130..78b9de303f 100644 --- a/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs @@ -252,7 +252,7 @@ impl Worker { block_parent_root: block.parent_root, proposer_index: block.proposer_index, blob, - kzg_commitment: block.body.blob_kzg_commitments[known_index].clone(), // TODO: needs to be stored in a more logical way so that this won't panic. + kzg_commitment: block.body.blob_kzg_commitments[known_index], // TODO: needs to be stored in a more logical way so that this won't panic. kzg_proof: kzg_aggregated_proof // TODO: yeah }; self.send_response( @@ -843,7 +843,7 @@ impl Worker { beacon_block_root, beacon_block_slot, blobs: blob_bundle, - kzg_aggregated_proof, + kzg_aggregated_proof: _, }: types::BlobsSidecar<_> = blobs; for (blob_index, blob) in blob_bundle.into_iter().enumerate() { diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index 438317d1cd..67db9a7a32 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/beacon_node/network/src/sync/block_sidecar_coupling.rs @@ -32,7 +32,7 @@ impl BlocksAndBlobsRequestInfo { pub fn into_responses(self) -> Result>, &'static str> { let BlocksAndBlobsRequestInfo { accumulated_blocks, - mut accumulated_sidecars, + accumulated_sidecars, .. } = self; diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 353d3e896e..768b95273e 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -875,8 +875,8 @@ impl SyncManager { fn rpc_blobs_received( &mut self, request_id: RequestId, - peer_id: PeerId, - maybe_blob: Option::EthSpec>>>, + _peer_id: PeerId, + _maybe_blob: Option::EthSpec>>>, _seen_timestamp: Duration, ) { match request_id { @@ -892,7 +892,7 @@ impl SyncManager { RequestId::RangeBlocks { .. } => { unreachable!("Only-blocks range requests don't receive sidecars") } - RequestId::RangeBlobs { id } => { + RequestId::RangeBlobs { id: _ } => { unimplemented!("Adjust range"); } } diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index d328639120..db64d74c2a 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -5,6 +5,7 @@ use crate::Error as ServerError; use lighthouse_network::{ConnectionDirection, Enr, Multiaddr, PeerConnectionStatus}; use mime::{Mime, APPLICATION, JSON, OCTET_STREAM, STAR}; use serde::{Deserialize, Serialize}; +use ssz_derive::Encode; use std::cmp::Reverse; use std::convert::TryFrom; use std::fmt; @@ -1322,3 +1323,60 @@ impl> Into> } } } + +pub type BlockContentsTuple = ( + SignedBeaconBlock, + Option, ::MaxBlobsPerBlock>>, +); + +/// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobSidecars`]. +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(untagged)] +#[serde(bound = "T: EthSpec")] +pub enum SignedBlockContents = FullPayload> { + BlockAndBlobSidecars(SignedBeaconBlockAndBlobSidecars), + Block(SignedBeaconBlock), +} + +impl> SignedBlockContents { + pub fn signed_block(&self) -> &SignedBeaconBlock { + match self { + SignedBlockContents::BlockAndBlobSidecars(block_and_sidecars) => { + &block_and_sidecars.signed_block + } + SignedBlockContents::Block(block) => block, + } + } + + pub fn deconstruct(self) -> BlockContentsTuple { + match self { + SignedBlockContents::BlockAndBlobSidecars(block_and_sidecars) => ( + block_and_sidecars.signed_block, + Some(block_and_sidecars.signed_blob_sidecars), + ), + SignedBlockContents::Block(block) => (block, None), + } + } +} + +impl> From> + for SignedBlockContents +{ + fn from(block: SignedBeaconBlock) -> Self { + match block { + SignedBeaconBlock::Base(_) + | SignedBeaconBlock::Altair(_) + | SignedBeaconBlock::Merge(_) + | SignedBeaconBlock::Capella(_) => SignedBlockContents::Block(block), + //TODO: error handling, this should be try from + SignedBeaconBlock::Eip4844(_block) => todo!(), + } + } +} + +#[derive(Debug, Clone, Serialize, Deserialize, Encode)] +#[serde(bound = "T: EthSpec")] +pub struct SignedBeaconBlockAndBlobSidecars> { + pub signed_block: SignedBeaconBlock, + pub signed_blob_sidecars: VariableList, ::MaxBlobsPerBlock>, +} diff --git a/consensus/types/src/beacon_block_body.rs b/consensus/types/src/beacon_block_body.rs index ace5e0f081..e49f633459 100644 --- a/consensus/types/src/beacon_block_body.rs +++ b/consensus/types/src/beacon_block_body.rs @@ -9,6 +9,8 @@ use superstruct::superstruct; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; +pub type KzgCommitments = VariableList::MaxBlobsPerBlock>; + /// The body of a `BeaconChain` block, containing operations. /// /// This *superstruct* abstracts over the hard-fork. diff --git a/consensus/types/src/blobs_sidecar.rs b/consensus/types/src/blobs_sidecar.rs deleted file mode 100644 index e2560fb30b..0000000000 --- a/consensus/types/src/blobs_sidecar.rs +++ /dev/null @@ -1,62 +0,0 @@ -use crate::test_utils::TestRandom; -use crate::{Blob, EthSpec, Hash256, KzgCommitment, SignedRoot, Slot}; -use derivative::Derivative; -use kzg::KzgProof; -use serde_derive::{Deserialize, Serialize}; -use ssz::Encode; -use ssz_derive::{Decode, Encode}; -use ssz_types::VariableList; -use test_random_derive::TestRandom; -use tree_hash_derive::TreeHash; - -pub type KzgCommitments = VariableList::MaxBlobsPerBlock>; -pub type Blobs = VariableList, ::MaxBlobsPerBlock>; - -#[derive( - Debug, - Clone, - Serialize, - Deserialize, - Encode, - Decode, - TreeHash, - Default, - TestRandom, - Derivative, - arbitrary::Arbitrary, -)] -#[serde(bound = "T: EthSpec")] -#[arbitrary(bound = "T: EthSpec")] -#[derivative(PartialEq, Hash(bound = "T: EthSpec"))] -pub struct BlobsSidecar { - pub beacon_block_root: Hash256, - pub beacon_block_slot: Slot, - #[serde(with = "ssz_types::serde_utils::list_of_hex_fixed_vec")] - pub blobs: Blobs, - pub kzg_aggregated_proof: KzgProof, -} - -impl SignedRoot for BlobsSidecar {} - -impl BlobsSidecar { - pub fn empty() -> Self { - Self::default() - } - - pub fn empty_from_parts(beacon_block_root: Hash256, beacon_block_slot: Slot) -> Self { - Self { - beacon_block_root, - beacon_block_slot, - blobs: VariableList::empty(), - kzg_aggregated_proof: KzgProof::empty(), - } - } - - #[allow(clippy::integer_arithmetic)] - pub fn max_size() -> usize { - // Fixed part - Self::empty().as_ssz_bytes().len() - // Max size of variable length `blobs` field - + (T::max_blobs_per_block() * as Encode>::ssz_fixed_len()) - } -} diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 14f06bb51d..592b6cc758 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -99,10 +99,8 @@ pub mod sqlite; pub mod beacon_block_and_blob_sidecars; pub mod blob_sidecar; -pub mod blobs_sidecar; pub mod signed_blob; pub mod signed_block_and_blobs; -pub mod signed_block_contents; pub mod transaction; use ethereum_types::{H160, H256}; @@ -125,7 +123,6 @@ pub use crate::beacon_block_header::BeaconBlockHeader; pub use crate::beacon_committee::{BeaconCommittee, OwnedBeaconCommittee}; pub use crate::beacon_state::{BeaconTreeHashCache, Error as BeaconStateError, *}; pub use crate::blob_sidecar::{BlobSidecar, BlobSidecarList}; -pub use crate::blobs_sidecar::{Blobs, BlobsSidecar}; pub use crate::bls_to_execution_change::BlsToExecutionChange; pub use crate::chain_spec::{ChainSpec, Config, Domain}; pub use crate::checkpoint::Checkpoint; @@ -185,10 +182,8 @@ pub use crate::signed_beacon_block::{ pub use crate::signed_beacon_block_header::SignedBeaconBlockHeader; pub use crate::signed_blob::*; pub use crate::signed_block_and_blobs::{ - SignedBeaconBlockAndBlobSidecars, SignedBeaconBlockAndBlobsSidecar, - SignedBeaconBlockAndBlobsSidecarDecode, + SignedBeaconBlockAndBlobsSidecar, SignedBeaconBlockAndBlobsSidecarDecode, }; -pub use crate::signed_block_contents::SignedBlockContents; pub use crate::signed_bls_to_execution_change::SignedBlsToExecutionChange; pub use crate::signed_contribution_and_proof::SignedContributionAndProof; pub use crate::signed_voluntary_exit::SignedVoluntaryExit; diff --git a/consensus/types/src/signed_beacon_block.rs b/consensus/types/src/signed_beacon_block.rs index ae59690bf2..52800f0782 100644 --- a/consensus/types/src/signed_beacon_block.rs +++ b/consensus/types/src/signed_beacon_block.rs @@ -250,28 +250,6 @@ impl> SignedBeaconBlock pub fn canonical_root(&self) -> Hash256 { self.message().tree_hash_root() } - - /// Reconstructs an empty `BlobsSidecar`, using the given block root if provided, else calculates it. - /// If this block has kzg commitments, an error will be returned. If this block is from prior to the - /// Eip4844 fork, this will error. - pub fn reconstruct_empty_blobs( - &self, - block_root_opt: Option, - ) -> Result, BlobReconstructionError> { - let kzg_commitments = self - .message() - .body() - .blob_kzg_commitments() - .map_err(|_| BlobReconstructionError::InconsistentFork)?; - if kzg_commitments.is_empty() { - Ok(BlobsSidecar::empty_from_parts( - block_root_opt.unwrap_or_else(|| self.canonical_root()), - self.slot(), - )) - } else { - Err(BlobReconstructionError::UnavailableBlobs) - } - } } // We can convert pre-Bellatrix blocks without payloads into blocks with payloads. diff --git a/consensus/types/src/signed_block_and_blobs.rs b/consensus/types/src/signed_block_and_blobs.rs index c6d154ef0f..9bfdfc4576 100644 --- a/consensus/types/src/signed_block_and_blobs.rs +++ b/consensus/types/src/signed_block_and_blobs.rs @@ -1,12 +1,12 @@ use crate::{ - AbstractExecPayload, BlobsSidecar, EthSpec, SignedBeaconBlock, SignedBeaconBlockEip4844, + AbstractExecPayload, EthSpec, SignedBeaconBlock, SignedBeaconBlockEip4844, SignedBlobSidecar, }; +use crate::{BlobsSidecar, EthSpec, SignedBeaconBlock, SignedBeaconBlockEip4844}; use derivative::Derivative; use serde_derive::{Deserialize, Serialize}; use ssz::{Decode, DecodeError}; use ssz_derive::{Decode, Encode}; -use ssz_types::VariableList; use std::sync::Arc; use tree_hash_derive::TreeHash; @@ -37,11 +37,3 @@ impl SignedBeaconBlockAndBlobsSidecar { }) } } - -#[derive(Debug, Clone, Serialize, Deserialize, Encode, TreeHash, Derivative)] -#[derivative(PartialEq, Hash(bound = "T: EthSpec"))] -#[serde(bound = "T: EthSpec")] -pub struct SignedBeaconBlockAndBlobSidecars> { - pub signed_block: SignedBeaconBlock, - pub signed_blob_sidecars: VariableList, ::MaxBlobsPerBlock>, -} diff --git a/consensus/types/src/signed_block_contents.rs b/consensus/types/src/signed_block_contents.rs deleted file mode 100644 index bce6233338..0000000000 --- a/consensus/types/src/signed_block_contents.rs +++ /dev/null @@ -1,49 +0,0 @@ -use crate::signed_block_and_blobs::SignedBeaconBlockAndBlobSidecars; -use crate::{AbstractExecPayload, EthSpec, FullPayload, SignedBeaconBlock, SignedBlobSidecar}; -use derivative::Derivative; -use serde_derive::{Deserialize, Serialize}; -use ssz_types::VariableList; - -/// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobSidecars`]. -#[derive(Clone, Debug, Derivative, Serialize, Deserialize)] -#[derivative(PartialEq, Hash(bound = "T: EthSpec"))] -#[serde(untagged)] -#[serde(bound = "T: EthSpec")] -pub enum SignedBlockContents = FullPayload> { - BlockAndBlobSidecars(SignedBeaconBlockAndBlobSidecars), - Block(SignedBeaconBlock), -} - -impl> SignedBlockContents { - pub fn signed_block(&self) -> &SignedBeaconBlock { - match self { - SignedBlockContents::BlockAndBlobSidecars(block_and_sidecars) => { - &block_and_sidecars.signed_block - } - SignedBlockContents::Block(block) => block, - } - } - - pub fn deconstruct( - self, - ) -> ( - SignedBeaconBlock, - Option, ::MaxBlobsPerBlock>>, - ) { - match self { - SignedBlockContents::BlockAndBlobSidecars(block_and_sidecars) => ( - block_and_sidecars.signed_block, - Some(block_and_sidecars.signed_blob_sidecars), - ), - SignedBlockContents::Block(block) => (block, None), - } - } -} - -impl> From> - for SignedBlockContents -{ - fn from(block: SignedBeaconBlock) -> Self { - SignedBlockContents::Block(block) - } -} diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs index eb40dee9b3..0eb9a07c39 100644 --- a/validator_client/src/block_service.rs +++ b/validator_client/src/block_service.rs @@ -7,6 +7,7 @@ use crate::{ }; use crate::{http_metrics::metrics, validator_store::ValidatorStore}; use environment::RuntimeContext; +use eth2::types::SignedBlockContents; use slog::{crit, debug, error, info, trace, warn}; use slot_clock::SlotClock; use std::ops::Deref; @@ -16,7 +17,7 @@ use tokio::sync::mpsc; use tokio::time::sleep; use types::{ AbstractExecPayload, BeaconBlock, BlindedPayload, BlockType, EthSpec, FullPayload, Graffiti, - PublicKeyBytes, SignedBlockContents, Slot, + PublicKeyBytes, Slot, }; #[derive(Debug)] @@ -388,7 +389,6 @@ impl BlockService { )) })? .data - .into() } }; @@ -455,7 +455,7 @@ impl BlockService { ); beacon_node // TODO: need to be adjusted for blobs - .post_beacon_blinded_blocks(&signed_block_contents.signed_block()) + .post_beacon_blinded_blocks(signed_block_contents.signed_block()) .await .map_err(|e| { BlockError::Irrecoverable(format!(