Penalize peer if missing blobs

[dapplion]

Issue Addressed

During range sync, if a peer replies with empty blobs we do not downscore that peer. We should.

Proposed Changes

Downscore peer if we can't match block and data when constructing an RpcBlock

The PRs below also move these two variants into not being internal

• Strict match of errors in range sync #6521
• Stricter match of BlockError in lookup sync #6321

[jimmygchen]

The category change makes sense but it doesn't look like we use this to determine peer penalty.

Related issue:
#4546

I think we'll need to handle BlockError::AvailabilityCheck in handle_failed_chain_segment:

lighthouse/beacon_node/network/src/network_beacon_processor/sync_methods.rs

Line 711 in ee7fca3

fn handle_failed_chain_segment(&self, error: BlockError) -> Result<(), ChainSegmentFailed> {

[jxs]

it seems we use Jimmy, category() is called on:

lighthouse/beacon_node/network/src/sync/block_lookups/mod.rs

Lines 580 to 605 in 2e0eb6d

	BlockError::AvailabilityCheck(e)
	if e.category() == AvailabilityCheckErrorCategory::Internal =>
	{
	// There errors indicate internal problems and should not downscore the peer
	warn!(self.log, "Internal availability check failure"; "block_root" => ?block_root, "error" => ?e);
	// Here we choose *not* to call `on_processing_failure` because this could result in a bad
	// lookup state transition. This error invalidates both blob and block requests, and we don't know the
	// state of both requests. Blobs may have already successfullly processed for example.
	// We opt to drop the lookup instead.
	Action::Drop
	}
	other => {
	debug!(self.log, "Invalid lookup component"; "block_root" => ?block_root, "component" => ?R::response_type(), "error" => ?other);
	let peer_id = request_state.on_processing_failure()?;
	cx.report_peer(
	peer_id,
	PeerAction::MidToleranceError,
	match R::response_type() {
	ResponseType::Block => "lookup_block_processing_failure",
	ResponseType::Blob => "lookup_blobs_processing_failure",
	},
	);
	Action::Retry

with Lion's change Error::MissingBlobs and Error::MissingCustodyColumns are now reported as ErrorCategory::Malicious instead of ErrorCategory::Internal which makes it so that those nodes get reported via report_peer

[jimmygchen]

This is now covered in #7352, closing.