Remove old block processing shim (#1327)

* Remove old block processing shim

* Run rustfmt

* Fix log formatting

* Swap peer ids over to display
This commit is contained in:
Paul Hauner 2020-07-06 16:28:00 +10:00 committed by GitHub
parent 2856f5122d
commit e429c3eefe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 108 additions and 261 deletions

View File

@ -69,10 +69,6 @@ use types::{
PublicKey, RelativeEpoch, SignedBeaconBlock, Slot,
};
mod block_processing_outcome;
pub use block_processing_outcome::BlockProcessingOutcome;
/// Maximum block slot number. Block with slots bigger than this constant will NOT be processed.
const MAXIMUM_BLOCK_SLOT_NUMBER: u64 = 4_294_967_296; // 2^32

View File

@ -1,130 +0,0 @@
use crate::{BeaconChainError, BlockError};
use state_processing::BlockProcessingError;
use types::{Hash256, Slot};
/// This is a legacy object that is being kept around to reduce merge conflicts.
///
/// TODO: As soon as this is merged into master, it should be removed as soon as possible.
#[derive(Debug, PartialEq)]
pub enum BlockProcessingOutcome {
/// Block was valid and imported into the block graph.
Processed {
block_root: Hash256,
},
InvalidSignature,
/// The proposal signature in invalid.
ProposalSignatureInvalid,
/// The `block.proposal_index` is not known.
UnknownValidator(u64),
/// The parent block was unknown.
ParentUnknown(Hash256),
/// The block slot is greater than the present slot.
FutureSlot {
present_slot: Slot,
block_slot: Slot,
},
/// The block state_root does not match the generated state.
StateRootMismatch {
block: Hash256,
local: Hash256,
},
/// The block was a genesis block, these blocks cannot be re-imported.
GenesisBlock,
/// The slot is finalized, no need to import.
WouldRevertFinalizedSlot {
block_slot: Slot,
finalized_slot: Slot,
},
/// Block is already known, no need to re-import.
BlockIsAlreadyKnown,
/// A block for this proposer and slot has already been observed.
RepeatProposal {
proposer: u64,
slot: Slot,
},
/// The block slot exceeds the MAXIMUM_BLOCK_SLOT_NUMBER.
BlockSlotLimitReached,
/// The provided block is from an earlier slot than its parent.
BlockIsNotLaterThanParent {
block_slot: Slot,
state_slot: Slot,
},
/// The `BeaconBlock` has a `proposer_index` that does not match the index we computed locally.
///
/// The block is invalid.
IncorrectBlockProposer {
block: u64,
local_shuffling: u64,
},
/// At least one block in the chain segement did not have it's parent root set to the root of
/// the prior block.
NonLinearParentRoots,
/// The slots of the blocks in the chain segment were not strictly increasing. I.e., a child
/// had lower slot than a parent.
NonLinearSlots,
/// The block could not be applied to the state, it is invalid.
PerBlockProcessingError(BlockProcessingError),
}
impl BlockProcessingOutcome {
pub fn shim(
result: Result<Hash256, BlockError>,
) -> Result<BlockProcessingOutcome, BeaconChainError> {
match result {
Ok(block_root) => Ok(BlockProcessingOutcome::Processed { block_root }),
Err(BlockError::ParentUnknown(root)) => Ok(BlockProcessingOutcome::ParentUnknown(root)),
Err(BlockError::FutureSlot {
present_slot,
block_slot,
}) => Ok(BlockProcessingOutcome::FutureSlot {
present_slot,
block_slot,
}),
Err(BlockError::StateRootMismatch { block, local }) => {
Ok(BlockProcessingOutcome::StateRootMismatch { block, local })
}
Err(BlockError::GenesisBlock) => Ok(BlockProcessingOutcome::GenesisBlock),
Err(BlockError::WouldRevertFinalizedSlot {
block_slot,
finalized_slot,
}) => Ok(BlockProcessingOutcome::WouldRevertFinalizedSlot {
block_slot,
finalized_slot,
}),
Err(BlockError::BlockIsAlreadyKnown) => Ok(BlockProcessingOutcome::BlockIsAlreadyKnown),
Err(BlockError::RepeatProposal { proposer, slot }) => {
Ok(BlockProcessingOutcome::RepeatProposal { proposer, slot })
}
Err(BlockError::BlockSlotLimitReached) => {
Ok(BlockProcessingOutcome::BlockSlotLimitReached)
}
Err(BlockError::ProposalSignatureInvalid) => {
Ok(BlockProcessingOutcome::ProposalSignatureInvalid)
}
Err(BlockError::UnknownValidator(i)) => Ok(BlockProcessingOutcome::UnknownValidator(i)),
Err(BlockError::InvalidSignature) => Ok(BlockProcessingOutcome::InvalidSignature),
Err(BlockError::BlockIsNotLaterThanParent {
block_slot,
state_slot,
}) => Ok(BlockProcessingOutcome::BlockIsNotLaterThanParent {
block_slot,
state_slot,
}),
Err(BlockError::IncorrectBlockProposer {
block,
local_shuffling,
}) => Ok(BlockProcessingOutcome::IncorrectBlockProposer {
block,
local_shuffling,
}),
Err(BlockError::NonLinearParentRoots) => {
Ok(BlockProcessingOutcome::NonLinearParentRoots)
}
Err(BlockError::NonLinearSlots) => Ok(BlockProcessingOutcome::NonLinearSlots),
Err(BlockError::PerBlockProcessingError(e)) => {
Ok(BlockProcessingOutcome::PerBlockProcessingError(e))
}
Err(BlockError::BeaconChainError(e)) => Err(e),
}
}
}

View File

@ -35,7 +35,7 @@ pub use self::beacon_snapshot::BeaconSnapshot;
pub use self::errors::{BeaconChainError, BlockProductionError};
pub use attestation_verification::Error as AttestationError;
pub use beacon_fork_choice_store::{BeaconForkChoiceStore, Error as ForkChoiceStoreError};
pub use block_verification::{BlockError, BlockProcessingOutcome, GossipVerifiedBlock};
pub use block_verification::{BlockError, GossipVerifiedBlock};
pub use eth1_chain::{Eth1Chain, Eth1ChainBackend};
pub use events::EventHandler;
pub use metrics::scrape_for_metrics;

View File

@ -6,8 +6,8 @@ use beacon_chain::{
VerifiedUnaggregatedAttestation,
},
observed_operations::ObservationOutcome,
BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, BlockProcessingOutcome,
ForkChoiceError, GossipVerifiedBlock,
BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, ForkChoiceError,
GossipVerifiedBlock,
};
use eth2_libp2p::rpc::*;
use eth2_libp2p::{NetworkGlobals, PeerId, PeerRequestId, Request, Response};
@ -103,7 +103,7 @@ impl<T: BeaconChainTypes> Processor<T> {
debug!(
self.log,
"Sending Status Request";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"fork_digest" => format!("{:?}", status_message.fork_digest),
"finalized_root" => format!("{:?}", status_message.finalized_root),
"finalized_epoch" => format!("{:?}", status_message.finalized_epoch),
@ -127,7 +127,7 @@ impl<T: BeaconChainTypes> Processor<T> {
debug!(
self.log,
"Received Status Request";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"fork_digest" => format!("{:?}", status.fork_digest),
"finalized_root" => format!("{:?}", status.finalized_root),
"finalized_epoch" => format!("{:?}", status.finalized_epoch),
@ -206,7 +206,7 @@ impl<T: BeaconChainTypes> Processor<T> {
// clock is incorrect.
debug!(
self.log, "Handshake Failure";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"reason" => "different system clocks or genesis time"
);
self.network
@ -226,7 +226,7 @@ impl<T: BeaconChainTypes> Processor<T> {
// Therefore, the node is on a different chain and we should not communicate with them.
debug!(
self.log, "Handshake Failure";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"reason" => "different finalized chain"
);
self.network
@ -248,7 +248,7 @@ impl<T: BeaconChainTypes> Processor<T> {
debug!(
self.log,
"NaivePeer";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"reason" => "lower finalized epoch"
);
} else if self
@ -259,7 +259,7 @@ impl<T: BeaconChainTypes> Processor<T> {
{
debug!(
self.log, "Peer with known chain found";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"remote_head_slot" => remote.head_slot,
"remote_latest_finalized_epoch" => remote.finalized_epoch,
);
@ -274,7 +274,7 @@ impl<T: BeaconChainTypes> Processor<T> {
// head that are worth downloading.
debug!(
self.log, "UsefulPeer";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"local_finalized_epoch" => local.finalized_epoch,
"remote_latest_finalized_epoch" => remote.finalized_epoch,
);
@ -302,7 +302,7 @@ impl<T: BeaconChainTypes> Processor<T> {
debug!(
self.log,
"Peer requested unknown block";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"request_root" => format!("{:}", root),
);
}
@ -310,7 +310,7 @@ impl<T: BeaconChainTypes> Processor<T> {
debug!(
self.log,
"Received BlocksByRoot Request";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"requested" => request.block_roots.len(),
"returned" => send_block_count,
);
@ -422,7 +422,7 @@ impl<T: BeaconChainTypes> Processor<T> {
debug!(
self.log,
"BlocksByRange Response Sent";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"msg" => "Failed to return all requested blocks",
"start_slot" => req.start_slot,
"current_slot" => self.chain.slot().unwrap_or_else(|_| Slot::from(0_u64)).as_u64(),
@ -432,7 +432,7 @@ impl<T: BeaconChainTypes> Processor<T> {
debug!(
self.log,
"Sending BlocksByRange Response";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"start_slot" => req.start_slot,
"current_slot" => self.chain.slot().unwrap_or_else(|_| Slot::from(0_u64)).as_u64(),
"requested" => req.count,
@ -455,7 +455,7 @@ impl<T: BeaconChainTypes> Processor<T> {
trace!(
self.log,
"Received BlocksByRange Response";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
);
if let RequestId::Sync(id) = request_id {
@ -482,7 +482,7 @@ impl<T: BeaconChainTypes> Processor<T> {
trace!(
self.log,
"Received BlocksByRoot Response";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
);
if let RequestId::Sync(id) = request_id {
@ -527,59 +527,55 @@ impl<T: BeaconChainTypes> Processor<T> {
verified_block: GossipVerifiedBlock<T>,
) -> bool {
let block = Box::new(verified_block.block.clone());
match BlockProcessingOutcome::shim(self.chain.process_block(verified_block)) {
Ok(outcome) => match outcome {
BlockProcessingOutcome::Processed { .. } => {
trace!(self.log, "Gossipsub block processed";
"peer_id" => format!("{:?}",peer_id));
// TODO: It would be better if we can run this _after_ we publish the block to
// reduce block propagation latency.
//
// The `MessageHandler` would be the place to put this, however it doesn't seem
// to have a reference to the `BeaconChain`. I will leave this for future
// works.
match self.chain.fork_choice() {
Ok(()) => trace!(
self.log,
"Fork choice success";
"location" => "block gossip"
),
Err(e) => error!(
self.log,
"Fork choice failed";
"error" => format!("{:?}", e),
"location" => "block gossip"
),
}
}
BlockProcessingOutcome::ParentUnknown { .. } => {
// 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" => format!("{:?}",peer_id));
self.send_to_sync(SyncMessage::UnknownBlock(peer_id, block));
}
other => {
warn!(
self.log,
"Invalid gossip beacon block";
"outcome" => format!("{:?}", other),
"block root" => format!("{}", block.canonical_root()),
"block slot" => block.slot()
);
trace!(
self.log,
"Invalid gossip beacon block ssz";
"ssz" => format!("0x{}", hex::encode(block.as_ssz_bytes())),
);
}
},
Err(_) => {
// error is logged during the processing therefore no error is logged here
match self.chain.process_block(verified_block) {
Ok(_block_root) => {
trace!(
self.log,
"Erroneous gossip beacon block ssz";
"Gossipsub block processed";
"peer_id" => peer_id.to_string()
);
// TODO: It would be better if we can run this _after_ we publish the block to
// reduce block propagation latency.
//
// The `MessageHandler` would be the place to put this, however it doesn't seem
// to have a reference to the `BeaconChain`. I will leave this for future
// works.
match self.chain.fork_choice() {
Ok(()) => trace!(
self.log,
"Fork choice success";
"location" => "block gossip"
),
Err(e) => error!(
self.log,
"Fork choice failed";
"error" => format!("{:?}", e),
"location" => "block gossip"
),
}
}
Err(BlockError::ParentUnknown { .. }) => {
// 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.to_string()
);
self.send_to_sync(SyncMessage::UnknownBlock(peer_id, block));
}
other => {
warn!(
self.log,
"Invalid gossip beacon block";
"outcome" => format!("{:?}", other),
"block root" => format!("{}", block.canonical_root()),
"block slot" => block.slot()
);
trace!(
self.log,
"Invalid gossip beacon block ssz";
"ssz" => format!("0x{}", hex::encode(block.as_ssz_bytes())),
);
}
@ -601,7 +597,7 @@ impl<T: BeaconChainTypes> Processor<T> {
self.log,
"Invalid attestation from network";
"block" => format!("{}", beacon_block_root),
"peer_id" => format!("{:?}", peer_id),
"peer_id" => peer_id.to_string(),
"type" => format!("{:?}", attestation_type),
);
@ -707,7 +703,7 @@ impl<T: BeaconChainTypes> Processor<T> {
debug!(
self.log,
"Attestation for unknown block";
"peer_id" => format!("{:?}", peer_id),
"peer_id" => peer_id.to_string(),
"block" => format!("{}", beacon_block_root)
);
// we don't know the block, get the sync manager to handle the block lookup
@ -790,7 +786,7 @@ impl<T: BeaconChainTypes> Processor<T> {
error!(
self.log,
"Unable to validate aggregate";
"peer_id" => format!("{:?}", peer_id),
"peer_id" => peer_id.to_string(),
"error" => format!("{:?}", e),
);
}
@ -837,7 +833,7 @@ impl<T: BeaconChainTypes> Processor<T> {
self.log,
"Attestation invalid for op pool";
"reason" => format!("{:?}", e),
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"beacon_block_root" => format!("{:?}", beacon_block_root)
)
}
@ -887,7 +883,7 @@ impl<T: BeaconChainTypes> Processor<T> {
self.log,
"Attestation invalid for agg pool";
"reason" => format!("{:?}", e),
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"beacon_block_root" => format!("{:?}", beacon_block_root)
)
}
@ -915,7 +911,7 @@ impl<T: BeaconChainTypes> Processor<T> {
self.log,
"Attestation invalid for fork choice";
"reason" => format!("{:?}", e),
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"beacon_block_root" => format!("{:?}", beacon_block_root)
)
}
@ -923,7 +919,7 @@ impl<T: BeaconChainTypes> Processor<T> {
self.log,
"Error applying attestation to fork choice";
"reason" => format!("{:?}", e),
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"beacon_block_root" => format!("{:?}", beacon_block_root)
),
}
@ -947,7 +943,7 @@ impl<T: BeaconChainTypes> Processor<T> {
self.log,
"Dropping exit for already exiting validator";
"validator_index" => validator_index,
"peer" => format!("{:?}", peer_id)
"peer" => peer_id.to_string()
);
None
}
@ -956,7 +952,7 @@ impl<T: BeaconChainTypes> Processor<T> {
self.log,
"Dropping invalid exit";
"validator_index" => validator_index,
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"error" => format!("{:?}", e)
);
None
@ -994,7 +990,7 @@ impl<T: BeaconChainTypes> Processor<T> {
"Dropping proposer slashing";
"reason" => "Already seen a proposer slashing for that validator",
"validator_index" => validator_index,
"peer" => format!("{:?}", peer_id)
"peer" => peer_id.to_string()
);
None
}
@ -1003,7 +999,7 @@ impl<T: BeaconChainTypes> Processor<T> {
self.log,
"Dropping invalid proposer slashing";
"validator_index" => validator_index,
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"error" => format!("{:?}", e)
);
None
@ -1038,7 +1034,7 @@ impl<T: BeaconChainTypes> Processor<T> {
self.log,
"Dropping attester slashing";
"reason" => "Slashings already known for all slashed validators",
"peer" => format!("{:?}", peer_id)
"peer" => peer_id.to_string()
);
None
}
@ -1046,7 +1042,7 @@ impl<T: BeaconChainTypes> Processor<T> {
debug!(
self.log,
"Dropping invalid attester slashing";
"peer" => format!("{:?}", peer_id),
"peer" => peer_id.to_string(),
"error" => format!("{:?}", e)
);
None

View File

@ -39,7 +39,7 @@ use super::peer_sync_info::{PeerSyncInfo, PeerSyncType};
use super::range_sync::{BatchId, ChainId, RangeSync, EPOCHS_PER_BATCH};
use super::RequestId;
use crate::service::NetworkMessage;
use beacon_chain::{BeaconChain, BeaconChainTypes, BlockProcessingOutcome};
use beacon_chain::{BeaconChain, BeaconChainTypes, BlockError};
use eth2_libp2p::rpc::{methods::MAX_REQUEST_BLOCKS, BlocksByRootRequest};
use eth2_libp2p::types::NetworkGlobals;
use eth2_libp2p::PeerId;
@ -396,42 +396,38 @@ impl<T: BeaconChainTypes> SyncManager<T> {
}
// we have the correct block, try and process it
match BlockProcessingOutcome::shim(self.chain.process_block(block.clone())) {
Ok(outcome) => {
match outcome {
BlockProcessingOutcome::Processed { block_root } => {
info!(self.log, "Processed block"; "block" => format!("{}", block_root));
match self.chain.process_block(block.clone()) {
Ok(block_root) => {
info!(self.log, "Processed block"; "block" => format!("{}", block_root));
match self.chain.fork_choice() {
Ok(()) => trace!(
self.log,
"Fork choice success";
"location" => "single block"
),
Err(e) => error!(
self.log,
"Fork choice failed";
"error" => format!("{:?}", e),
"location" => "single block"
),
}
}
BlockProcessingOutcome::ParentUnknown { .. } => {
// We don't know of the blocks parent, begin a parent lookup search
self.add_unknown_block(peer_id, block);
}
BlockProcessingOutcome::BlockIsAlreadyKnown => {
trace!(self.log, "Single block lookup already known");
}
_ => {
warn!(self.log, "Single block lookup failed"; "outcome" => format!("{:?}", outcome));
self.network.downvote_peer(peer_id);
}
match self.chain.fork_choice() {
Ok(()) => trace!(
self.log,
"Fork choice success";
"location" => "single block"
),
Err(e) => error!(
self.log,
"Fork choice failed";
"error" => format!("{:?}", e),
"location" => "single block"
),
}
}
Err(e) => {
Err(BlockError::ParentUnknown { .. }) => {
// We don't know of the blocks parent, begin a parent lookup search
self.add_unknown_block(peer_id, block);
}
Err(BlockError::BlockIsAlreadyKnown) => {
trace!(self.log, "Single block lookup already known");
}
Err(BlockError::BeaconChainError(e)) => {
warn!(self.log, "Unexpected block processing error"; "error" => format!("{:?}", e));
}
outcome => {
warn!(self.log, "Single block lookup failed"; "outcome" => format!("{:?}", outcome));
self.network.downvote_peer(peer_id);
}
}
}
@ -645,16 +641,15 @@ impl<T: BeaconChainTypes> SyncManager<T> {
.downloaded_blocks
.pop()
.expect("There is always at least one block in the queue");
match BlockProcessingOutcome::shim(self.chain.process_block(newest_block.clone())) {
Ok(BlockProcessingOutcome::ParentUnknown { .. }) => {
match self.chain.process_block(newest_block.clone()) {
Err(BlockError::ParentUnknown { .. }) => {
// need to keep looking for parents
// add the block back to the queue and continue the search
parent_request.downloaded_blocks.push(newest_block);
self.request_parent(parent_request);
return;
}
Ok(BlockProcessingOutcome::Processed { .. })
| Ok(BlockProcessingOutcome::BlockIsAlreadyKnown { .. }) => {
Ok(_) | Err(BlockError::BlockIsAlreadyKnown { .. }) => {
spawn_block_processor(
Arc::downgrade(&self.chain),
ProcessId::ParentLookup(parent_request.last_submitted_peer.clone()),
@ -663,7 +658,7 @@ impl<T: BeaconChainTypes> SyncManager<T> {
self.log.clone(),
);
}
Ok(outcome) => {
Err(outcome) => {
// all else we consider the chain a failure and downvote the peer that sent
// us the last block
warn!(
@ -675,16 +670,6 @@ impl<T: BeaconChainTypes> SyncManager<T> {
.downvote_peer(parent_request.last_submitted_peer);
return;
}
Err(e) => {
warn!(
self.log, "Parent chain processing error. Downvoting peer";
"error" => format!("{:?}", e),
"last_peer" => format!("{:?}", parent_request.last_submitted_peer),
);
self.network
.downvote_peer(parent_request.last_submitted_peer);
return;
}
}
}
}