From 0958ce610fe047e7e7d70bbe5b965d0e05b8842f Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 21 Mar 2023 00:46:31 +0530 Subject: [PATCH] lint --- beacon_node/beacon_chain/src/beacon_chain.rs | 16 +-- .../beacon_chain/src/blob_verification.rs | 14 +- .../beacon_chain/src/block_verification.rs | 9 +- beacon_node/beacon_chain/src/builder.rs | 1 - .../beacon_chain/src/gossip_blob_cache.rs | 55 ++------ beacon_node/client/src/builder.rs | 3 - beacon_node/http_api/src/block_id.rs | 4 +- .../network/src/beacon_processor/mod.rs | 7 +- .../beacon_processor/worker/gossip_methods.rs | 120 +----------------- .../beacon_processor/worker/rpc_methods.rs | 8 +- .../beacon_processor/worker/sync_methods.rs | 2 +- consensus/types/src/blob_sidecar.rs | 1 - 12 files changed, 44 insertions(+), 196 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 6d569d0f85..542c797d1f 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -8,14 +8,13 @@ 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::{ - self, AsBlock, AvailableBlock, BlobError, BlockWrapper, GossipVerifiedBlob, IntoAvailableBlock, - VerifiedBlobs, + self, AsBlock, AvailableBlock, BlobError, BlockWrapper, GossipVerifiedBlob, }; 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, ExecutedBlock, ExecutionPendingBlock, - GossipVerifiedBlock, IntoExecutionPendingBlock, PayloadVerificationOutcome, POS_PANDA_BANNER, + GossipVerifiedBlock, IntoExecutionPendingBlock, POS_PANDA_BANNER, }; pub use crate::canonical_head::{CanonicalHead, CanonicalHeadRwLock}; use crate::chain_config::ChainConfig; @@ -78,7 +77,6 @@ use futures::channel::mpsc::Sender; use itertools::process_results; use itertools::Itertools; use kzg::Kzg; -use oneshot_broadcast::Receiver; use operation_pool::{AttestationRef, OperationPool, PersistedOperationPool, ReceivedPreCapella}; use parking_lot::{Mutex, RwLock}; use proto_array::{CountUnrealizedFull, DoNotReOrg, ProposerHeadError}; @@ -87,7 +85,6 @@ 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, @@ -103,7 +100,6 @@ use std::borrow::Cow; use std::cmp::Ordering; use std::collections::HashMap; use std::collections::HashSet; -use std::future::Future; use std::io::prelude::*; use std::marker::PhantomData; use std::sync::Arc; @@ -113,7 +109,6 @@ 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_block_body::KzgCommitments; use types::beacon_state::CloneConfig; @@ -990,7 +985,7 @@ impl BeaconChain { pub async fn get_block_and_blobs_checking_early_attester_cache( &self, - block_root: &Hash256, + _block_root: &Hash256, ) -> Result, Error> { //TODO(sean) use the rpc blobs cache and revert this to the current block cache logic Ok(Some(())) @@ -2711,7 +2706,7 @@ impl BeaconChain { // TODO(log required errors) let executed_block = self .clone() - .into_executed_block(execution_pending, count_unrealized) + .into_executed_block(execution_pending) .await .map_err(|e| self.handle_block_error(e))?; @@ -2733,7 +2728,6 @@ impl BeaconChain { async fn into_executed_block( self: Arc, execution_pending_block: ExecutionPendingBlock, - count_unrealized: CountUnrealized, ) -> Result, BlockError> { let ExecutionPendingBlock { block, @@ -2843,7 +2837,7 @@ impl BeaconChain { confirmed_state_roots, consensus_context, payload_verification_outcome, - } = block; + } = *block; let available_block = match block { BlockWrapper::Available(block) => block, diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 9736e6c400..64f66365a8 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -1,5 +1,4 @@ use slot_clock::SlotClock; -use state_processing::ConsensusContext; use std::sync::Arc; use crate::beacon_chain::{ @@ -7,7 +6,6 @@ use crate::beacon_chain::{ VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT, }; use crate::gossip_blob_cache::AvailabilityCheckError; -use crate::snapshot_cache::PreProcessingSnapshot; use crate::BeaconChainError; use derivative::Derivative; use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; @@ -221,7 +219,7 @@ pub fn validate_blob_sidecar_for_gossip( .map_err(BlobError::BeaconChainError)?; let pubkey = pubkey_cache - .get(proposer_index as usize) + .get(proposer_index) .ok_or_else(|| BlobError::UnknownValidator(proposer_index as u64))?; signed_blob_sidecar.verify_signature( @@ -265,7 +263,7 @@ pub fn validate_blob_sidecar_for_gossip( } pub fn verify_data_availability( - blob_sidecar: &BlobSidecarList, + _blob_sidecar: &BlobSidecarList, kzg_commitments: &[KzgCommitment], transactions: &Transactions, _block_slot: Slot, @@ -316,8 +314,8 @@ pub trait IntoAvailableBlock { impl IntoAvailableBlock for BlockWrapper { fn into_available_block( self, - block_root: Hash256, - chain: &BeaconChain, + _block_root: Hash256, + _chain: &BeaconChain, ) -> Result, BlobError> { todo!() } @@ -436,7 +434,7 @@ impl AsBlock for BlockWrapper { fn as_block(&self) -> &SignedBeaconBlock { match &self { BlockWrapper::Available(block) => &block.block, - BlockWrapper::AvailabilityPending(block) => &block, + BlockWrapper::AvailabilityPending(block) => block, } } fn block_cloned(&self) -> Arc> { @@ -493,7 +491,7 @@ impl AsBlock for &BlockWrapper { fn as_block(&self) -> &SignedBeaconBlock { match &self { BlockWrapper::Available(block) => &block.block, - BlockWrapper::AvailabilityPending(block) => &block, + BlockWrapper::AvailabilityPending(block) => block, } } fn block_cloned(&self) -> Arc> { diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 3c3cb8c527..91ee0dae65 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -949,7 +949,7 @@ impl GossipVerifiedBlock { .set_kzg_commitments_consistent(true); Ok(Self { - block: block, + block, block_root, parent, consensus_context, @@ -1149,8 +1149,6 @@ impl IntoExecutionPendingBlock for Arc IntoExecutionPendingBlock for BlockWrapper { pub enum Availability { PendingBlobs(Vec), PendingBlock(Hash256), - Available(ExecutedBlock), + Available(Box>), } struct GossipBlobCache { @@ -71,13 +66,8 @@ impl DataAvailabilityChecker { blob: Arc>, ) -> Result, AvailabilityCheckError> { let verified = if let Some(kzg) = self.kzg.as_ref() { - validate_blob::( - kzg, - blob.blob.clone(), - blob.kzg_commitment.clone(), - blob.kzg_proof, - ) - .map_err(|e| AvailabilityCheckError::Kzg(e))? + validate_blob::(kzg, blob.blob.clone(), blob.kzg_commitment, blob.kzg_proof) + .map_err(AvailabilityCheckError::Kzg)? } else { false // error wrong fork @@ -91,7 +81,7 @@ impl DataAvailabilityChecker { // Gossip cache. blob_cache .entry(blob.block_root) - .and_modify(|mut inner| { + .and_modify(|inner| { // All blobs reaching this cache should be gossip verified and gossip verification // should filter duplicates, as well as validate indices. inner @@ -145,7 +135,7 @@ impl DataAvailabilityChecker { let block_clone = executed_block.block.clone(); let availability = match block_clone { - BlockWrapper::Available(available_block) => Availability::Available(executed_block), + BlockWrapper::Available(_) => Availability::Available(Box::new(executed_block)), BlockWrapper::AvailabilityPending(block) => { if let Ok(kzg_commitments) = block.message().body().blob_kzg_commitments() { // first check if the blockwrapper contains blobs, if so, use those @@ -193,7 +183,7 @@ impl DataAvailabilityChecker { consensus_context, payload_verification_outcome, }; - Availability::Available(available_executed) + Availability::Available(Box::new(available_executed)) } else { let mut missing_blobs = Vec::with_capacity(kzg_commitments.len()); for i in 0..kzg_commitments.len() { @@ -207,7 +197,7 @@ impl DataAvailabilityChecker { //TODO(sean) add a check that missing blobs > 0 - let _ = cache.executed_block.insert(executed_block.clone()); + let _ = cache.executed_block.insert(executed_block); // log that we cached the block? Availability::PendingBlobs(missing_blobs) } @@ -230,27 +220,10 @@ impl DataAvailabilityChecker { } } } else { - Availability::Available(executed_block) + Availability::Available(Box::new(executed_block)) } } }; Ok(availability) } - - /// Adds the blob to the cache. Returns true if adding the blob completes - /// all the required blob sidecars for a given block root. - /// - /// Note: we can only know this if we know `block.kzg_commitments.len()` - pub fn put_blob_temp( - &self, - blob: Arc>, - ) -> Result { - unimplemented!() - } - - /// Returns all blobs associated with a given block root otherwise returns - /// a UnavailableBlobs error. - pub fn blobs(&self, block_root: Hash256) -> Result, AvailabilityCheckError> { - unimplemented!() - } } diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 41e08bfdca..e80b6fd18c 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -862,9 +862,6 @@ where .ok_or("beacon_chain requires a runtime context")? .clone(); - let (tx, mut rx) = - tokio::sync::mpsc::channel::>(1000); //TODO(sean) capacity - let chain = self .beacon_chain_builder .ok_or("beacon_chain requires a beacon_chain_builder")? diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index d53bd793e8..fed2921de3 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -218,11 +218,11 @@ impl BlockId { chain: &BeaconChain, ) -> Result>, warp::Rejection> { let root = self.root(chain)?.0; - let Some(data_availability_boundary) = chain.data_availability_boundary() else { + let Some(_data_availability_boundary) = chain.data_availability_boundary() else { return Err(warp_utils::reject::custom_not_found("Eip4844 fork disabled".into())); }; match chain.get_blobs(&root) { - Ok(Some(blob)) => todo!(), // Jimmy's PR will fix this, + Ok(Some(_blob)) => todo!(), // Jimmy's PR will fix this, Ok(None) => Err(warp_utils::reject::custom_not_found(format!( "Blob with block root {} is not in the store", root diff --git a/beacon_node/network/src/beacon_processor/mod.rs b/beacon_node/network/src/beacon_processor/mod.rs index 240bf24285..f58f281351 100644 --- a/beacon_node/network/src/beacon_processor/mod.rs +++ b/beacon_node/network/src/beacon_processor/mod.rs @@ -459,7 +459,7 @@ impl WorkEvent { peer_id, peer_client, blob_index, - signed_blob, + signed_blob: Box::new(signed_blob), seen_timestamp, }, } @@ -864,7 +864,7 @@ pub enum Work { peer_id: PeerId, peer_client: Client, blob_index: u64, - signed_blob: SignedBlobSidecar, + signed_blob: Box>, seen_timestamp: Duration, }, DelayedImportBlock { @@ -1759,8 +1759,7 @@ impl BeaconProcessor { peer_id, peer_client, blob_index, - signed_blob, - work_reprocessing_tx, + *signed_blob, seen_timestamp, ) .await 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 43146e3538..f65691295c 100644 --- a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs @@ -2,7 +2,6 @@ use crate::{metrics, service::NetworkMessage, sync::SyncMessage}; use beacon_chain::blob_verification::{AsBlock, BlockWrapper, GossipVerifiedBlob}; use beacon_chain::store::Error; -use beacon_chain::ExecutedBlock; use beacon_chain::{ attestation_verification::{self, Error as AttnError, VerifiedAttestation}, light_client_finality_update_verification::Error as LightClientFinalityUpdateError, @@ -654,10 +653,9 @@ impl Worker { self, _message_id: MessageId, peer_id: PeerId, - peer_client: Client, + _peer_client: Client, blob_index: u64, signed_blob: SignedBlobSidecar, - reprocess_tx: mpsc::Sender>, _seen_duration: Duration, ) { match self @@ -665,13 +663,8 @@ impl Worker { .verify_blob_sidecar_for_gossip(signed_blob, blob_index) { Ok(gossip_verified_blob) => { - self.process_gossip_verified_blob( - peer_id, - gossip_verified_blob, - reprocess_tx, - _seen_duration, - ) - .await + self.process_gossip_verified_blob(peer_id, gossip_verified_blob, _seen_duration) + .await } Err(_) => { // TODO(pawan): handle all blob errors for peer scoring @@ -684,7 +677,6 @@ impl Worker { self, peer_id: PeerId, verified_blob: GossipVerifiedBlob, - reprocess_tx: mpsc::Sender>, // This value is not used presently, but it might come in handy for debugging. _seen_duration: Duration, ) { @@ -694,7 +686,7 @@ impl Worker { .process_blob(verified_blob.to_blob(), CountUnrealized::True) .await { - Ok(AvailabilityProcessingStatus::Imported(hash)) => { + Ok(AvailabilityProcessingStatus::Imported(_hash)) => { todo!() // add to metrics // logging @@ -707,7 +699,7 @@ impl Worker { Ok(AvailabilityProcessingStatus::PendingBlock(block_hash)) => { self.send_sync_message(SyncMessage::UnknownBlockHash(peer_id, block_hash)); } - Err(e) => { + Err(_err) => { // handle errors todo!() } @@ -847,7 +839,7 @@ impl Worker { verified_block } - Err(BlockError::AvailabilityCheck(e)) => { + Err(BlockError::AvailabilityCheck(_err)) => { todo!() } Err(BlockError::ParentUnknown(block)) => { @@ -1008,104 +1000,6 @@ impl Worker { } } - /// Process the beacon block that has already passed gossip verification. - /// - /// Raises a log if there are errors. - pub async fn process_execution_verified_block( - self, - peer_id: PeerId, - executed_block: ExecutedBlock, - reprocess_tx: mpsc::Sender>, - // This value is not used presently, but it might come in handy for debugging. - seen_duration: Duration, - ) { - let block_root = executed_block.block_root; - let block = executed_block.block.block_cloned(); - - match self - .chain - .check_availability_and_maybe_import( - |chain| { - chain - .data_availability_checker - .check_block_availability(executed_block) - }, - CountUnrealized::True, - ) - .await - { - Ok(AvailabilityProcessingStatus::Imported(block_root)) => { - metrics::inc_counter(&metrics::BEACON_PROCESSOR_GOSSIP_BLOCK_IMPORTED_TOTAL); - - if reprocess_tx - .try_send(ReprocessQueueMessage::BlockImported { - block_root, - parent_root: block.message().parent_root(), - }) - .is_err() - { - error!( - self.log, - "Failed to inform block import"; - "source" => "gossip", - "block_root" => ?block_root, - ) - }; - - debug!( - self.log, - "Gossipsub block processed"; - "block" => ?block_root, - "peer_id" => %peer_id - ); - - self.chain.recompute_head_at_current_slot().await; - } - Ok(AvailabilityProcessingStatus::PendingBlobs(_)) - | Ok(AvailabilityProcessingStatus::PendingBlock(_)) - | Err(BlockError::AvailabilityCheck(_)) => { - // TODO(need to do something different if it's unavailble again) - unimplemented!() - } - Err(BlockError::ParentUnknown(block)) => { - // Inform the sync manager to find parents for this block - // This should not occur. It should be checked by `should_forward_block` - error!( - self.log, - "Block with unknown parent attempted to be processed"; - "peer_id" => %peer_id - ); - self.send_sync_message(SyncMessage::UnknownBlock(peer_id, block, block_root)); - } - Err(ref e @ BlockError::ExecutionPayloadError(ref epe)) if !epe.penalize_peer() => { - debug!( - self.log, - "Failed to verify execution payload"; - "error" => %e - ); - } - other => { - debug!( - self.log, - "Invalid gossip beacon block"; - "outcome" => ?other, - "block root" => ?block_root, - "block slot" => block.slot() - ); - self.gossip_penalize_peer( - peer_id, - PeerAction::MidToleranceError, - "bad_gossip_block_ssz", - ); - trace!( - self.log, - "Invalid gossip beacon block ssz"; - "ssz" => format_args!("0x{}", hex::encode(block.as_ssz_bytes())), - ); - } - }; - } - /// Process the beacon block that has already passed gossip verification. /// /// Raises a log if there are errors. @@ -1115,7 +1009,7 @@ impl Worker { verified_block: GossipVerifiedBlock, reprocess_tx: mpsc::Sender>, // This value is not used presently, but it might come in handy for debugging. - seen_duration: Duration, + _seen_duration: Duration, ) { let block = verified_block.block.block_cloned(); let block_root = verified_block.block_root; 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 a42b56794a..5d4a8e3b05 100644 --- a/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/rpc_methods.rs @@ -225,9 +225,9 @@ impl Worker { executor.spawn( async move { let requested_blobs = request.blob_ids.len(); - let mut send_block_count = 0; + let send_block_count = 0; let mut send_response = true; - for BlobIdentifier{ block_root: root, index } in request.blob_ids.into_iter() { + for BlobIdentifier{ block_root: root, index: _index } in request.blob_ids.into_iter() { match self .chain .get_block_and_blobs_checking_early_attester_cache(&root) @@ -833,12 +833,12 @@ impl Worker { // remove all skip slots let block_roots = block_roots.into_iter().flatten().collect::>(); - let mut blobs_sent = 0; + let blobs_sent = 0; let mut send_response = true; for root in block_roots { match self.chain.get_blobs(&root) { - Ok(Some(blobs)) => { + Ok(Some(_blobs)) => { todo!(); // // TODO: more GROSS code ahead. Reader beware // let types::BlobsSidecar { diff --git a/beacon_node/network/src/beacon_processor/worker/sync_methods.rs b/beacon_node/network/src/beacon_processor/worker/sync_methods.rs index bb4a39d286..542bd9af34 100644 --- a/beacon_node/network/src/beacon_processor/worker/sync_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/sync_methods.rs @@ -7,7 +7,7 @@ use crate::beacon_processor::DuplicateCache; use crate::metrics; use crate::sync::manager::{BlockProcessType, SyncMessage}; use crate::sync::{BatchProcessResult, ChainId}; -use beacon_chain::blob_verification::{AsBlock, BlockWrapper, IntoAvailableBlock}; +use beacon_chain::blob_verification::{AsBlock, BlockWrapper}; use beacon_chain::{AvailabilityProcessingStatus, CountUnrealized}; use beacon_chain::{ BeaconChainError, BeaconChainTypes, BlockError, ChainSegmentResult, HistoricalBlockError, diff --git a/consensus/types/src/blob_sidecar.rs b/consensus/types/src/blob_sidecar.rs index fbd0aebb12..b71c0c91c7 100644 --- a/consensus/types/src/blob_sidecar.rs +++ b/consensus/types/src/blob_sidecar.rs @@ -1,6 +1,5 @@ use crate::test_utils::TestRandom; use crate::{Blob, EthSpec, Hash256, SignedRoot, Slot}; -use bls::Signature; use derivative::Derivative; use kzg::{KzgCommitment, KzgProof}; use serde_derive::{Deserialize, Serialize};