From 6efd95496b636b734dc7d94fd12e773ea31ff41d Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 28 Mar 2022 07:14:13 +0000 Subject: [PATCH] Optionally skip RANDAO verification during block production (#3116) ## Proposed Changes Allow Lighthouse to speculatively create blocks via the `/eth/v1/validators/blocks` endpoint by optionally skipping the RANDAO verification that we introduced in #2740. When `verify_randao=false` is passed as a query parameter the `randao_reveal` is not required to be present, and if present will only be lightly checked (must be a valid BLS sig). If `verify_randao` is omitted it defaults to true and Lighthouse behaves exactly as it did previously, hence this PR is backwards-compatible. I'd like to get this change into `unstable` pretty soon as I've got 3 projects building on top of it: - [`blockdreamer`](https://github.com/michaelsproul/blockdreamer), which mocks block production every slot in order to fingerprint clients - analysis of Lighthouse's block packing _optimality_, which uses `blockdreamer` to extract interesting instances of the attestation packing problem - analysis of Lighthouse's block packing _performance_ (as in speed) on the `tree-states` branch ## Additional Info Having tested `blockdreamer` with Prysm, Nimbus and Teku I noticed that none of them verify the randao signature on `/eth/v1/validator/blocks`. I plan to open a PR to the `beacon-APIs` repo anyway so that this parameter can be standardised in case the other clients add RANDAO verification by default in future. --- beacon_node/beacon_chain/src/beacon_chain.rs | 30 ++++- beacon_node/beacon_chain/src/lib.rs | 4 +- beacon_node/beacon_chain/src/test_utils.rs | 20 +++- beacon_node/http_api/src/lib.rs | 42 +++++-- beacon_node/http_api/tests/tests.rs | 114 +++++++++++++++++++ common/eth2/src/lib.rs | 23 +++- common/eth2/src/types.rs | 8 +- 7 files changed, 223 insertions(+), 18 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 443b44ffa..7bc47815d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -147,6 +147,12 @@ pub enum ChainSegmentResult { }, } +/// Configure the signature verification of produced blocks. +pub enum ProduceBlockVerification { + VerifyRandao, + NoVerification, +} + /// The accepted clock drift for nodes gossiping blocks and attestations. See: /// /// https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/p2p-interface.md#configuration @@ -2891,6 +2897,22 @@ impl BeaconChain { randao_reveal: Signature, slot: Slot, validator_graffiti: Option, + ) -> Result, BlockProductionError> { + self.produce_block_with_verification( + randao_reveal, + slot, + validator_graffiti, + ProduceBlockVerification::VerifyRandao, + ) + } + + /// Same as `produce_block` but allowing for configuration of RANDAO-verification. + pub fn produce_block_with_verification( + &self, + randao_reveal: Signature, + slot: Slot, + validator_graffiti: Option, + verification: ProduceBlockVerification, ) -> Result, BlockProductionError> { metrics::inc_counter(&metrics::BLOCK_PRODUCTION_REQUESTS); let _complete_timer = metrics::start_timer(&metrics::BLOCK_PRODUCTION_TIMES); @@ -2948,6 +2970,7 @@ impl BeaconChain { slot, randao_reveal, validator_graffiti, + verification, ) } @@ -2970,6 +2993,7 @@ impl BeaconChain { produce_at_slot: Slot, randao_reveal: Signature, validator_graffiti: Option, + verification: ProduceBlockVerification, ) -> Result, BlockProductionError> { let eth1_chain = self .eth1_chain @@ -3160,11 +3184,15 @@ impl BeaconChain { } let process_timer = metrics::start_timer(&metrics::BLOCK_PRODUCTION_PROCESS_TIMES); + let signature_strategy = match verification { + ProduceBlockVerification::VerifyRandao => BlockSignatureStrategy::VerifyRandao, + ProduceBlockVerification::NoVerification => BlockSignatureStrategy::NoVerification, + }; per_block_processing( &mut state, &block, None, - BlockSignatureStrategy::VerifyRandao, + signature_strategy, VerifyBlockRoot::True, &self.spec, )?; diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 89a341210..65908547f 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -41,8 +41,8 @@ mod validator_pubkey_cache; pub use self::beacon_chain::{ AttestationProcessingOutcome, BeaconChain, BeaconChainTypes, BeaconStore, ChainSegmentResult, - ForkChoiceError, HeadInfo, HeadSafetyStatus, StateSkipConfig, WhenSlotSkipped, - INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON, MAXIMUM_GOSSIP_CLOCK_DISPARITY, + ForkChoiceError, HeadInfo, HeadSafetyStatus, ProduceBlockVerification, StateSkipConfig, + WhenSlotSkipped, INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON, MAXIMUM_GOSSIP_CLOCK_DISPARITY, }; pub use self::beacon_snapshot::BeaconSnapshot; pub use self::chain_config::ChainConfig; diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index e474b9a52..6684aeec8 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2,7 +2,7 @@ pub use crate::persisted_beacon_chain::PersistedBeaconChain; pub use crate::{ beacon_chain::{BEACON_CHAIN_DB_KEY, ETH1_CACHE_DB_KEY, FORK_CHOICE_DB_KEY, OP_POOL_DB_KEY}, migrate::MigratorConfig, - BeaconChainError, + BeaconChainError, ProduceBlockVerification, }; use crate::{ builder::{BeaconChainBuilder, Witness}, @@ -604,7 +604,14 @@ where let (block, state) = self .chain - .produce_block_on_state(state, None, slot, randao_reveal, Some(graffiti)) + .produce_block_on_state( + state, + None, + slot, + randao_reveal, + Some(graffiti), + ProduceBlockVerification::VerifyRandao, + ) .unwrap(); let signed_block = block.sign( @@ -658,7 +665,14 @@ where let (block, state) = self .chain - .produce_block_on_state(state, None, slot, randao_reveal, Some(graffiti)) + .produce_block_on_state( + state, + None, + slot, + randao_reveal, + Some(graffiti), + ProduceBlockVerification::VerifyRandao, + ) .unwrap(); let signed_block = block.sign( diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index c754c5431..ada4af3d2 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -23,7 +23,7 @@ use beacon_chain::{ observed_operations::ObservationOutcome, validator_monitor::{get_block_delay_ms, timestamp_now}, AttestationError as AttnError, BeaconChain, BeaconChainError, BeaconChainTypes, - HeadSafetyStatus, WhenSlotSkipped, + HeadSafetyStatus, ProduceBlockVerification, WhenSlotSkipped, }; use block_id::BlockId; use eth2::types::{self as api_types, EndpointVersion, ValidatorId}; @@ -46,7 +46,7 @@ use tokio::sync::mpsc::UnboundedSender; use tokio_stream::{wrappers::BroadcastStream, StreamExt}; use types::{ Attestation, AttesterSlashing, BeaconStateError, CommitteeCache, ConfigAndPreset, Epoch, - EthSpec, ForkName, ProposerPreparationData, ProposerSlashing, RelativeEpoch, + EthSpec, ForkName, ProposerPreparationData, ProposerSlashing, RelativeEpoch, Signature, SignedAggregateAndProof, SignedBeaconBlock, SignedContributionAndProof, SignedVoluntaryExit, Slot, SyncCommitteeMessage, SyncContributionData, }; @@ -1872,15 +1872,39 @@ pub fn serve( query: api_types::ValidatorBlocksQuery, chain: Arc>| { blocking_json_task(move || { - let randao_reveal = (&query.randao_reveal).try_into().map_err(|e| { - warp_utils::reject::custom_bad_request(format!( - "randao reveal is not valid BLS signature: {:?}", - e - )) - })?; + let randao_reveal = query.randao_reveal.as_ref().map_or_else( + || { + if query.verify_randao { + Err(warp_utils::reject::custom_bad_request( + "randao_reveal is mandatory unless verify_randao=false".into(), + )) + } else { + Ok(Signature::empty()) + } + }, + |sig_bytes| { + sig_bytes.try_into().map_err(|e| { + warp_utils::reject::custom_bad_request(format!( + "randao reveal is not a valid BLS signature: {:?}", + e + )) + }) + }, + )?; + + let randao_verification = if query.verify_randao { + ProduceBlockVerification::VerifyRandao + } else { + ProduceBlockVerification::NoVerification + }; let (block, _) = chain - .produce_block(randao_reveal, slot, query.graffiti.map(Into::into)) + .produce_block_with_verification( + randao_reveal, + slot, + query.graffiti.map(Into::into), + randao_verification, + ) .map_err(warp_utils::reject::block_production_error)?; let fork_name = block .to_ref() diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 2957a68c0..f3a1ccbf0 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1919,6 +1919,104 @@ impl ApiTester { self } + pub async fn test_block_production_no_verify_randao(self) -> Self { + for _ in 0..E::slots_per_epoch() { + let slot = self.chain.slot().unwrap(); + + let block = self + .client + .get_validator_blocks_with_verify_randao::(slot, None, None, Some(false)) + .await + .unwrap() + .data; + assert_eq!(block.slot(), slot); + self.chain.slot_clock.set_slot(slot.as_u64() + 1); + } + + self + } + + pub async fn test_block_production_verify_randao_invalid(self) -> Self { + let fork = self.chain.head_info().unwrap().fork; + let genesis_validators_root = self.chain.genesis_validators_root; + + for _ in 0..E::slots_per_epoch() { + let slot = self.chain.slot().unwrap(); + let epoch = self.chain.epoch().unwrap(); + + let proposer_pubkey_bytes = self + .client + .get_validator_duties_proposer(epoch) + .await + .unwrap() + .data + .into_iter() + .find(|duty| duty.slot == slot) + .map(|duty| duty.pubkey) + .unwrap(); + let proposer_pubkey = (&proposer_pubkey_bytes).try_into().unwrap(); + + let sk = self + .validator_keypairs + .iter() + .find(|kp| kp.pk == proposer_pubkey) + .map(|kp| kp.sk.clone()) + .unwrap(); + + let bad_randao_reveal = { + let domain = self.chain.spec.get_domain( + epoch, + Domain::Randao, + &fork, + genesis_validators_root, + ); + let message = (epoch + 1).signing_root(domain); + sk.sign(message).into() + }; + + // Check failure with no `verify_randao` passed. + self.client + .get_validator_blocks::(slot, &bad_randao_reveal, None) + .await + .unwrap_err(); + + // Check failure with `verify_randao=true`. + self.client + .get_validator_blocks_with_verify_randao::( + slot, + Some(&bad_randao_reveal), + None, + Some(true), + ) + .await + .unwrap_err(); + + // Check failure with no randao reveal provided. + self.client + .get_validator_blocks_with_verify_randao::(slot, None, None, None) + .await + .unwrap_err(); + + // Check success with `verify_randao=false`. + let block = self + .client + .get_validator_blocks_with_verify_randao::( + slot, + Some(&bad_randao_reveal), + None, + Some(false), + ) + .await + .unwrap() + .data; + + assert_eq!(block.slot(), slot); + self.chain.slot_clock.set_slot(slot.as_u64() + 1); + } + + self + } + pub async fn test_get_validator_attestation_data(self) -> Self { let mut state = self.chain.head_beacon_state().unwrap(); let slot = state.slot(); @@ -2770,6 +2868,22 @@ async fn block_production_with_skip_slots() { .await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn block_production_no_verify_randao() { + ApiTester::new() + .await + .test_block_production_no_verify_randao() + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn block_production_verify_randao_invalid() { + ApiTester::new() + .await + .test_block_production_verify_randao_invalid() + .await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn get_validator_attestation_data() { ApiTester::new() diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index e57faa4fe..856097cfe 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1155,6 +1155,18 @@ impl BeaconNodeHttpClient { slot: Slot, randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, + ) -> Result>, Error> { + self.get_validator_blocks_with_verify_randao(slot, Some(randao_reveal), graffiti, None) + .await + } + + /// `GET v2/validator/blocks/{slot}` + pub async fn get_validator_blocks_with_verify_randao( + &self, + slot: Slot, + randao_reveal: Option<&SignatureBytes>, + graffiti: Option<&Graffiti>, + verify_randao: Option, ) -> Result>, Error> { let mut path = self.eth_path(V2)?; @@ -1164,14 +1176,21 @@ impl BeaconNodeHttpClient { .push("blocks") .push(&slot.to_string()); - path.query_pairs_mut() - .append_pair("randao_reveal", &randao_reveal.to_string()); + if let Some(randao_reveal) = randao_reveal { + path.query_pairs_mut() + .append_pair("randao_reveal", &randao_reveal.to_string()); + } if let Some(graffiti) = graffiti { path.query_pairs_mut() .append_pair("graffiti", &graffiti.to_string()); } + if let Some(verify_randao) = verify_randao { + path.query_pairs_mut() + .append_pair("verify_randao", &verify_randao.to_string()); + } + self.get(path).await } diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 78567ad83..8cd3a1d67 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -627,8 +627,14 @@ pub struct ProposerData { #[derive(Clone, Serialize, Deserialize)] pub struct ValidatorBlocksQuery { - pub randao_reveal: SignatureBytes, + pub randao_reveal: Option, pub graffiti: Option, + #[serde(default = "default_verify_randao")] + pub verify_randao: bool, +} + +fn default_verify_randao() -> bool { + true } #[derive(Clone, Serialize, Deserialize)]