From 1430b561c37adb44d5705005de6bf633deb8c16d Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Thu, 6 Oct 2022 21:16:57 -0500 Subject: [PATCH] Add more gossip verification conditions --- .../beacon_chain/src/blob_verification.rs | 96 ++++++++++++------- consensus/types/src/signed_blobs_sidecar.rs | 38 +++++++- 2 files changed, 100 insertions(+), 34 deletions(-) diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index dd2d1badc..be9b0effc 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -1,13 +1,13 @@ use derivative::Derivative; use slot_clock::SlotClock; -use crate::beacon_chain::{BeaconChain, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY}; -use crate::{BeaconChainError}; -use bls::PublicKey; -use types::{ - consts::eip4844::BLS_MODULUS, BeaconStateError, Hash256, - SignedBlobsSidecar, Slot, +use crate::beacon_chain::{ + BeaconChain, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY, + VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT, }; +use crate::BeaconChainError; +use bls::PublicKey; +use types::{consts::eip4844::BLS_MODULUS, BeaconStateError, Hash256, SignedBlobsSidecar, Slot}; pub enum BlobError { /// The blob sidecar is from a slot that is later than the current slot (with respect to the @@ -69,6 +69,13 @@ pub enum BlobError { /// is valid or not. UnknownHeadBlock { beacon_block_root: Hash256 }, + /// The proposal_index corresponding to blob.beacon_block_root is not known. + /// + /// ## Peer scoring + /// + /// The block is invalid and the peer is faulty. + UnknownValidator(u64), + /// There was an error whilst processing the sync contribution. It is not known if it is valid or invalid. /// /// ## Peer scoring @@ -103,17 +110,17 @@ impl<'a, T: BeaconChainTypes> VerifiedBlobsSidecar<'a, T> { blob_sidecar: &'a SignedBlobsSidecar, chain: &BeaconChain, ) -> Result { - let blob_slot = blob_sidecar.message.beacon_block_slot; - let _blob_root = blob_sidecar.message.beacon_block_root; + let block_slot = blob_sidecar.message.beacon_block_slot; + let block_root = blob_sidecar.message.beacon_block_root; // Do not gossip or process blobs from future or past slots. let latest_permissible_slot = chain .slot_clock .now_with_future_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY) .ok_or(BeaconChainError::UnableToReadSlot)?; - if blob_slot > latest_permissible_slot { + if block_slot > latest_permissible_slot { return Err(BlobError::FutureSlot { message_slot: latest_permissible_slot, - latest_permissible_slot: blob_slot, + latest_permissible_slot: block_slot, }); } @@ -124,10 +131,10 @@ impl<'a, T: BeaconChainTypes> VerifiedBlobsSidecar<'a, T> { .slot_clock .now_with_past_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY) .ok_or(BeaconChainError::UnableToReadSlot)?; - if blob_slot > earliest_permissible_slot { + if block_slot > earliest_permissible_slot { return Err(BlobError::PastSlot { message_slot: earliest_permissible_slot, - earliest_permissible_slot: blob_slot, + earliest_permissible_slot: block_slot, }); } @@ -140,34 +147,59 @@ impl<'a, T: BeaconChainTypes> VerifiedBlobsSidecar<'a, T> { } // Verify that the KZG proof is a valid G1 point + // TODO(pawan): KZG commitment can also be point at infinity, use a different check + // (bls.KeyValidate) if PublicKey::deserialize(&blob_sidecar.message.kzg_aggregate_proof.0).is_err() { return Err(BlobError::InvalidKZGCommitment); } - // TODO: Verify proposer signature + let proposer_shuffling_root = chain + .canonical_head + .cached_head() + .snapshot + .beacon_state + .proposer_shuffling_decision_root(block_root)?; - // // let state = /* Get a valid state */ - // let proposer_index = state.get_beacon_proposer_index(blob_slot, &chain.spec)? as u64; - // let signature_is_valid = { - // let pubkey_cache = get_validator_pubkey_cache(chain)?; - // let pubkey = pubkey_cache - // .get(proposer_index as usize) - // .ok_or_else(|| BlobError::UnknownValidator(proposer_index)?; - // blob.verify_signature( - // Some(block_root), - // pubkey, - // &fork, - // chain.genesis_validators_root, - // &chain.spec, - // ) - // }; + let (proposer_index, fork) = match chain + .beacon_proposer_cache + .lock() + .get_slot::(proposer_shuffling_root, block_slot) + { + Some(proposer) => (proposer.index, proposer.fork), + None => { + let state = &chain.canonical_head.cached_head().snapshot.beacon_state; + ( + state.get_beacon_proposer_index(block_slot, &chain.spec)?, + state.fork(), + ) + } + }; + let signature_is_valid = { + let pubkey_cache = chain + .validator_pubkey_cache + .try_read_for(VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT) + .ok_or(BeaconChainError::ValidatorPubkeyCacheLockTimeout) + .map_err(BlobError::BeaconChainError)?; - // if !signature_is_valid { - // return Err(BlobError::ProposalSignatureInvalid); - // } + let pubkey = pubkey_cache + .get(proposer_index as usize) + .ok_or_else(|| BlobError::UnknownValidator(proposer_index as u64))?; - // TODO: Check that we have not already received a sidecar with a valid signature for this slot. + blob_sidecar.verify_signature( + None, + pubkey, + &fork, + chain.genesis_validators_root, + &chain.spec, + ) + }; + if !signature_is_valid { + return Err(BlobError::ProposalSignatureInvalid); + } + + // TODO(pawan): Check that we have not already received a sidecar with a valid signature for this slot. + // TODO(pawan): check if block hash is already known Ok(Self { blob_sidecar }) } } diff --git a/consensus/types/src/signed_blobs_sidecar.rs b/consensus/types/src/signed_blobs_sidecar.rs index 74a779219..677b95bd3 100644 --- a/consensus/types/src/signed_blobs_sidecar.rs +++ b/consensus/types/src/signed_blobs_sidecar.rs @@ -1,7 +1,9 @@ -use crate::{BlobsSidecar, EthSpec}; +use crate::{ + signing_data::SignedRoot, BlobsSidecar, ChainSpec, Domain, EthSpec, Fork, Hash256, PublicKey, + SigningData, +}; use bls::Signature; use serde_derive::{Deserialize, Serialize}; -use ssz::Encode; use ssz_derive::{Decode, Encode}; use tree_hash::TreeHash; use tree_hash_derive::TreeHash; @@ -21,4 +23,36 @@ impl SignedBlobsSidecar { signature, } } + + /// Verify `self.signature`. + /// + /// If the root of `blob_sidecar.message` is already known it can be passed in via `object_root_opt`. + /// Otherwise, it will be computed locally. + pub fn verify_signature( + &self, + object_root_opt: Option, + pubkey: &PublicKey, + fork: &Fork, + genesis_validators_root: Hash256, + spec: &ChainSpec, + ) -> bool { + let domain = spec.get_domain( + self.message.beacon_block_slot.epoch(T::slots_per_epoch()), + Domain::BlobsSideCar, + fork, + genesis_validators_root, + ); + + let message = if let Some(object_root) = object_root_opt { + SigningData { + object_root, + domain, + } + .tree_hash_root() + } else { + self.message.signing_root(domain) + }; + + self.signature.verify(pubkey, message) + } }