Add early check for RPC block relevancy (#2289)

## Issue Addressed

NA

## Proposed Changes

When observing `jemallocator` heap profiles and Grafana, it became clear that Lighthouse is spending significant RAM/CPU on processing blocks from the RPC. On investigation, it seems that we are loading the parent of the block *before* we check to see if the block is already known. This is a big waste of resources.

This PR adds an additional `check_block_relevancy` call as the first thing we do when we try to process a `SignedBeaconBlock` via the RPC (or other similar methods). Ultimately, `check_block_relevancy` will be called again later in the block processing flow. It's a very light function and I don't think trying to optimize it out is worth the risk of a bad block slipping through. 

Also adds a `New RPC block received` info log when we process a new RPC block. This seems like interesting and infrequent info.

## Additional Info

NA
This commit is contained in:
Paul Hauner 2021-06-02 01:07:27 +00:00
parent bf4e02e2cc
commit d34f922c1d
2 changed files with 19 additions and 5 deletions

View File

@ -690,6 +690,7 @@ impl<T: BeaconChainTypes> SignatureVerifiedBlock<T> {
/// Returns an error if the block is invalid, or if the block was unable to be verified. /// Returns an error if the block is invalid, or if the block was unable to be verified.
pub fn new( pub fn new(
block: SignedBeaconBlock<T::EthSpec>, block: SignedBeaconBlock<T::EthSpec>,
block_root: Hash256,
chain: &BeaconChain<T>, chain: &BeaconChain<T>,
) -> Result<Self, BlockError<T::EthSpec>> { ) -> Result<Self, BlockError<T::EthSpec>> {
let (mut parent, block) = load_parent(block, chain)?; let (mut parent, block) = load_parent(block, chain)?;
@ -697,8 +698,6 @@ impl<T: BeaconChainTypes> SignatureVerifiedBlock<T> {
// Reject any block that exceeds our limit on skipped slots. // Reject any block that exceeds our limit on skipped slots.
check_block_skip_slots(chain, parent.beacon_block.slot(), &block.message)?; check_block_skip_slots(chain, parent.beacon_block.slot(), &block.message)?;
let block_root = get_block_root(&block);
let state = cheap_state_advance_to_obtain_committees( let state = cheap_state_advance_to_obtain_committees(
&mut parent.pre_state, &mut parent.pre_state,
parent.beacon_state_root, parent.beacon_state_root,
@ -726,10 +725,11 @@ impl<T: BeaconChainTypes> SignatureVerifiedBlock<T> {
/// As for `new` above but producing `BlockSlashInfo`. /// As for `new` above but producing `BlockSlashInfo`.
pub fn check_slashable( pub fn check_slashable(
block: SignedBeaconBlock<T::EthSpec>, block: SignedBeaconBlock<T::EthSpec>,
block_root: Hash256,
chain: &BeaconChain<T>, chain: &BeaconChain<T>,
) -> Result<Self, BlockSlashInfo<BlockError<T::EthSpec>>> { ) -> Result<Self, BlockSlashInfo<BlockError<T::EthSpec>>> {
let header = block.signed_block_header(); let header = block.signed_block_header();
Self::new(block, chain).map_err(|e| BlockSlashInfo::from_early_error(header, e)) Self::new(block, block_root, chain).map_err(|e| BlockSlashInfo::from_early_error(header, e))
} }
/// Finishes signature verification on the provided `GossipVerifedBlock`. Does not re-verify /// Finishes signature verification on the provided `GossipVerifedBlock`. Does not re-verify
@ -814,7 +814,11 @@ impl<T: BeaconChainTypes> IntoFullyVerifiedBlock<T> for SignedBeaconBlock<T::Eth
self, self,
chain: &BeaconChain<T>, chain: &BeaconChain<T>,
) -> Result<FullyVerifiedBlock<T>, BlockSlashInfo<BlockError<T::EthSpec>>> { ) -> Result<FullyVerifiedBlock<T>, BlockSlashInfo<BlockError<T::EthSpec>>> {
SignatureVerifiedBlock::check_slashable(self, chain)? // Perform an early check to prevent wasting time on irrelevant blocks.
let block_root = check_block_relevancy(&self, None, chain)
.map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?;
SignatureVerifiedBlock::check_slashable(self, block_root, chain)?
.into_fully_verified_block_slashable(chain) .into_fully_verified_block_slashable(chain)
} }

View File

@ -6,7 +6,7 @@ use crate::sync::manager::SyncMessage;
use crate::sync::{BatchProcessResult, ChainId}; use crate::sync::{BatchProcessResult, ChainId};
use beacon_chain::{BeaconChainTypes, BlockError, ChainSegmentResult}; use beacon_chain::{BeaconChainTypes, BlockError, ChainSegmentResult};
use eth2_libp2p::PeerId; use eth2_libp2p::PeerId;
use slog::{crit, debug, error, trace, warn}; use slog::{crit, debug, error, info, trace, warn};
use types::{Epoch, Hash256, SignedBeaconBlock}; use types::{Epoch, Hash256, SignedBeaconBlock};
/// Id associated to a block processing request, either a batch or a single block. /// Id associated to a block processing request, either a batch or a single block.
@ -28,10 +28,20 @@ impl<T: BeaconChainTypes> Worker<T> {
block: SignedBeaconBlock<T::EthSpec>, block: SignedBeaconBlock<T::EthSpec>,
result_tx: BlockResultSender<T::EthSpec>, result_tx: BlockResultSender<T::EthSpec>,
) { ) {
let slot = block.slot();
let block_result = self.chain.process_block(block); let block_result = self.chain.process_block(block);
metrics::inc_counter(&metrics::BEACON_PROCESSOR_RPC_BLOCK_IMPORTED_TOTAL); metrics::inc_counter(&metrics::BEACON_PROCESSOR_RPC_BLOCK_IMPORTED_TOTAL);
if let Ok(root) = &block_result {
info!(
self.log,
"New RPC block received";
"slot" => slot,
"hash" => %root
);
}
if result_tx.send(block_result).is_err() { if result_tx.send(block_result).is_err() {
crit!(self.log, "Failed return sync block result"); crit!(self.log, "Failed return sync block result");
} }