diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 7032a0f27f3..f2d150d72bf 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -599,6 +599,16 @@ pub fn validate_blob_sidecar_for_gossip( }); } + chain + .observed_slashable + .write() + .observe_slashable( + blob_sidecar.slot(), + blob_sidecar.block_proposer_index(), + block_root, + ) + .map_err(|e| GossipBlobError::BeaconChainError(e.into()))?; + // Now the signature is valid, store the proposal so we don't accept another blob sidecar // with the same `BlobIdentifier`. // It's important to double-check that the proposer still hasn't been observed so we don't @@ -623,16 +633,6 @@ pub fn validate_blob_sidecar_for_gossip( }); } - chain - .observed_slashable - .write() - .observe_slashable( - blob_sidecar.slot(), - blob_sidecar.block_proposer_index(), - block_root, - ) - .map_err(|e| GossipBlobError::BeaconChainError(e.into()))?; - // Kzg verification for gossip blob sidecar let kzg = chain .kzg diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 832a005ef52..1b37139d2b7 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -945,6 +945,11 @@ impl GossipVerifiedBlock { return Err(BlockError::ProposalSignatureInvalid); } + chain + .observed_slashable + .write() + .observe_slashable(block.slot(), block.message().proposer_index(), block_root) + .map_err(|e| BlockError::BeaconChainError(e.into()))?; // Now the signature is valid, store the proposal so we don't accept another from this // validator and slot. // @@ -959,12 +964,6 @@ impl GossipVerifiedBlock { return Err(BlockError::BlockIsAlreadyKnown); }; - chain - .observed_slashable - .write() - .observe_slashable(block.slot(), block.message().proposer_index(), block_root) - .map_err(|e| BlockError::BeaconChainError(e.into()))?; - if block.message().proposer_index() != expected_proposer as u64 { return Err(BlockError::IncorrectBlockProposer { block: block.message().proposer_index(), diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 1247bb61405..e8d783c182c 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -19,9 +19,9 @@ use std::time::Duration; use tokio::sync::mpsc::UnboundedSender; use tree_hash::TreeHash; use types::{ - AbstractExecPayload, BeaconBlockRef, BlobSidecarList, EthSpec, ExecPayload, ExecutionBlockHash, - ForkName, FullPayload, FullPayloadMerge, Hash256, SignedBeaconBlock, SignedBlindedBeaconBlock, - VariableList, + AbstractExecPayload, BeaconBlockRef, BlobSidecar, BlobSidecarList, EthSpec, ExecPayload, + ExecutionBlockHash, ForkName, FullPayload, FullPayloadMerge, Hash256, SignedBeaconBlock, + SignedBlindedBeaconBlock, VariableList, }; use warp::http::StatusCode; use warp::{reply::Response, Rejection, Reply}; @@ -117,6 +117,22 @@ pub async fn publish_block { + if let Err(e) = check_slashable( + &chain_clone, + &None, + block_root.unwrap_or(block.canonical_root()), + &block, + &log_clone, + ) { + warn!( + log, + "Not publishing block - not gossip verified"; + "slot" => slot, + "error" => ?e + ); + return Err(warp_utils::reject::custom_bad_request(e.to_string())); + } + // Allow the status code for duplicate blocks to be overridden based on config. return Ok(warp::reply::with_status( warp::reply::json(&ErrorMessage { @@ -175,46 +191,20 @@ pub async fn publish_block { - let slashable_cache = chain_clone.observed_slashable.read(); - if let Some(blobs) = blobs_opt.as_ref() { - blobs.iter().try_for_each(|blob| { - if slashable_cache - .is_slashable(blob.slot(), blob.block_proposer_index(), blob.block_root()) - .map_err(|e| BlockError::BeaconChainError(e.into()))? - { - warn!( - log_clone, - "Not publishing equivocating blob"; - "slot" => block_clone.slot() - ); - return Err(BlockError::Slashable); - } - Ok(()) - })?; - }; - if slashable_cache - .is_slashable( - block_clone.slot(), - block_clone.message().proposer_index(), - block_root, - ) - .map_err(|e| BlockError::BeaconChainError(e.into()))? - { - warn!( - log_clone, - "Not publishing equivocating block"; - "slot" => block_clone.slot() - ); - Err(BlockError::Slashable) - } else { - publish_block( - block_clone, - blobs_opt, - sender_clone, - log_clone, - seen_timestamp, - ) - } + check_slashable( + &chain_clone, + &blobs_opt, + block_root, + &block_clone, + &log_clone, + )?; + publish_block( + block_clone, + blobs_opt, + sender_clone, + log_clone, + seen_timestamp, + ) } }; @@ -471,3 +461,46 @@ fn late_block_logging>( ) } } + +/// Check if any of the blobs or the block are slashable. Returns `BlockError::Slashable` if so. +fn check_slashable( + chain_clone: &BeaconChain, + blobs_opt: &Option>, + block_root: Hash256, + block_clone: &SignedBeaconBlock>, + log_clone: &Logger, +) -> Result<(), BlockError> { + let slashable_cache = chain_clone.observed_slashable.read(); + if let Some(blobs) = blobs_opt.as_ref() { + blobs.iter().try_for_each(|blob| { + if slashable_cache + .is_slashable(blob.slot(), blob.block_proposer_index(), blob.block_root()) + .map_err(|e| BlockError::BeaconChainError(e.into()))? + { + warn!( + log_clone, + "Not publishing equivocating blob"; + "slot" => block_clone.slot() + ); + return Err(BlockError::Slashable); + } + Ok(()) + })?; + }; + if slashable_cache + .is_slashable( + block_clone.slot(), + block_clone.message().proposer_index(), + block_root, + ) + .map_err(|e| BlockError::BeaconChainError(e.into()))? + { + warn!( + log_clone, + "Not publishing equivocating block"; + "slot" => block_clone.slot() + ); + return Err(BlockError::Slashable); + } + Ok(()) +} diff --git a/beacon_node/http_api/tests/broadcast_validation_tests.rs b/beacon_node/http_api/tests/broadcast_validation_tests.rs index 7961b32c578..eb39bdd1153 100644 --- a/beacon_node/http_api/tests/broadcast_validation_tests.rs +++ b/beacon_node/http_api/tests/broadcast_validation_tests.rs @@ -1226,9 +1226,13 @@ pub async fn blinded_equivocation_gossip() { ); } -/// This test checks that a block that is valid from both a gossip and consensus perspective but that equivocates **late** is rejected when using `broadcast_validation=consensus_and_equivocation`. +/// This test checks that a block that is valid from both a gossip and +/// consensus perspective but that equivocates **late** is rejected when using +/// `broadcast_validation=consensus_and_equivocation`. /// -/// This test is unique in that we can't actually test the HTTP API directly, but instead have to hook into the `publish_blocks` code manually. This is in order to handle the late equivocation case. +/// This test is unique in that we can't actually test the HTTP API directly, +/// but instead have to hook into the `publish_blocks` code manually. This is +/// in order to handle the late equivocation case. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] pub async fn blinded_equivocation_consensus_late_equivocation() { /* this test targets gossip-level validation */