Sidecar inclusion proof (#4900)

* Refactor BlobSidecar to new type

* Fix some compile errors

* Gossip verification compiles

* Fix http api types take 1

* Fix another round of compile errors

* Beacon node crate compiles

* EF tests compile

* Remove all blob signing from VC

* fmt

* Tests compile

* Fix some tests

* Fix more http tests

* get compiling

* Fix gossip conditions and tests

* Add basic proof generation and verification

* remove unnecessary ssz decode

* add back build_sidecar

* remove default at fork for blobs

* fix beacon chain tests

* get relase tests compiling

* fix lints

* fix existing spec tests

* add new ef tests

* fix gossip duplicate rule

* lints

* add back sidecar signature check in gossip

* add finalized descendant check to blob sidecar gossip

* fix error conversion

* fix release tests

* sidecar inclusion self review cleanup

* Add proof verification and computation metrics

* Remove accidentally committed file

* Unify some block and blob errors; add slashing conditions for sidecars

* Address review comment

* Clean up re-org tests (#4957)

* Address more review comments

* Add Comments & Eliminate Unnecessary Clones

* update names

* Update beacon_node/beacon_chain/src/metrics.rs

Co-authored-by: Jimmy Chen <jchen.tc@gmail.com>

* Update beacon_node/network/src/network_beacon_processor/tests.rs

Co-authored-by: Jimmy Chen <jchen.tc@gmail.com>

* pr feedback

* fix test compile

* Sidecar Inclusion proof small refactor and updates (#4967)

* Update some comments, variables and small cosmetic fixes.

* Couple blobs and proofs into a tuple in `PayloadAndBlobs` for simplicity and safety.

* Update function comment.

* Update testing/ef_tests/src/cases/merkle_proof_validity.rs

Co-authored-by: Jimmy Chen <jchen.tc@gmail.com>

* Rename the block and blob wrapper types used in the beacon API interfaces.

* make sure gossip invalid blobs are passed to the slasher (#4970)

* Add blob headers to slasher before adding to DA checker

* Replace Vec with HashSet in BlockQueue

* fmt

* Rename gindex -> index

* Simplify gossip condition

---------

Co-authored-by: realbigsean <seananderson33@gmail.com>
Co-authored-by: realbigsean <sean@sigmaprime.io>
Co-authored-by: Michael Sproul <michael@sigmaprime.io>
Co-authored-by: Mark Mackey <mark@sigmaprime.io>
Co-authored-by: Jimmy Chen <jchen.tc@gmail.com>

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

@@ -33,10 +33,11 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH};
 use store::hot_cold_store::HotColdDBError;
 use tokio::sync::mpsc;
 use types::{
-    Attestation, AttesterSlashing, EthSpec, Hash256, IndexedAttestation, LightClientFinalityUpdate,
-    LightClientOptimisticUpdate, ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock,
-    SignedBlobSidecar, SignedBlsToExecutionChange, SignedContributionAndProof, SignedVoluntaryExit,
-    Slot, SubnetId, SyncCommitteeMessage, SyncSubnetId,
+    Attestation, AttesterSlashing, BlobSidecar, EthSpec, Hash256, IndexedAttestation,
+    LightClientFinalityUpdate, LightClientOptimisticUpdate, ProposerSlashing,
+    SignedAggregateAndProof, SignedBeaconBlock, SignedBlsToExecutionChange,
+    SignedContributionAndProof, SignedVoluntaryExit, Slot, SubnetId, SyncCommitteeMessage,
+    SyncSubnetId,
 };
 
 use beacon_processor::{
@@ -607,20 +608,20 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
         peer_id: PeerId,
         _peer_client: Client,
         blob_index: u64,
-        signed_blob: SignedBlobSidecar<T::EthSpec>,
+        blob_sidecar: Arc<BlobSidecar<T::EthSpec>>,
         seen_duration: Duration,
     ) {
-        let slot = signed_blob.message.slot;
-        let root = signed_blob.message.block_root;
-        let index = signed_blob.message.index;
-        let commitment = signed_blob.message.kzg_commitment;
+        let slot = blob_sidecar.slot();
+        let root = blob_sidecar.block_root();
+        let index = blob_sidecar.index;
+        let commitment = blob_sidecar.kzg_commitment;
         let delay = get_slot_delay_ms(seen_duration, slot, &self.chain.slot_clock);
         // Log metrics to track delay from other nodes on the network.
         metrics::observe_duration(&metrics::BEACON_BLOB_GOSSIP_SLOT_START_DELAY_TIME, delay);
         metrics::set_gauge(&metrics::BEACON_BLOB_LAST_DELAY, delay.as_millis() as i64);
         match self
             .chain
-            .verify_blob_sidecar_for_gossip(signed_blob, blob_index)
+            .verify_blob_sidecar_for_gossip(blob_sidecar, blob_index)
         {
             Ok(gossip_verified_blob) => {
                 metrics::inc_counter(&metrics::BEACON_PROCESSOR_GOSSIP_BLOB_VERIFIED_TOTAL);
@@ -631,7 +632,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                         self.log,
                         "Gossip blob arrived late";
                         "block_root" => ?gossip_verified_blob.block_root(),
-                        "proposer_index" => gossip_verified_blob.proposer_index(),
+                        "proposer_index" => gossip_verified_blob.block_proposer_index(),
                         "slot" => gossip_verified_blob.slot(),
                         "delay" => ?delay,
                         "commitment" => %gossip_verified_blob.kzg_commitment(),
@@ -670,17 +671,30 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                             self.log,
                             "Unknown parent hash for blob";
                             "action" => "requesting parent",
-                            "block_root" => %blob.block_root,
-                            "parent_root" => %blob.block_parent_root,
+                            "block_root" => %blob.block_root(),
+                            "parent_root" => %blob.block_parent_root(),
                             "commitment" => %commitment,
                         );
                         self.send_sync_message(SyncMessage::UnknownParentBlob(peer_id, blob));
                     }
-                    GossipBlobError::ProposerSignatureInvalid
+                    GossipBlobError::KzgNotInitialized
+                    | GossipBlobError::PubkeyCacheTimeout
+                    | GossipBlobError::BeaconChainError(_) => {
+                        crit!(
+                            self.log,
+                            "Internal error when verifying blob sidecar";
+                            "error" => ?err,
+                        )
+                    }
+                    GossipBlobError::ProposalSignatureInvalid
                     | GossipBlobError::UnknownValidator(_)
                     | GossipBlobError::ProposerIndexMismatch { .. }
                     | GossipBlobError::BlobIsNotLaterThanParent { .. }
-                    | GossipBlobError::InvalidSubnet { .. } => {
+                    | GossipBlobError::InvalidSubnet { .. }
+                    | GossipBlobError::InvalidInclusionProof
+                    | GossipBlobError::KzgError(_)
+                    | GossipBlobError::InclusionProof(_)
+                    | GossipBlobError::NotFinalizedDescendant { .. } => {
                         warn!(
                             self.log,
                             "Could not verify blob sidecar for gossip. Rejecting the blob sidecar";
@@ -703,7 +717,6 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                         );
                     }
                     GossipBlobError::FutureSlot { .. }
-                    | GossipBlobError::BeaconChainError(_)
                     | GossipBlobError::RepeatBlob { .. }
                     | GossipBlobError::PastFinalizedSlot { .. } => {
                         warn!(

beacon_node/network/src/network_beacon_processor/mod.rs

@@ -212,7 +212,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
         peer_id: PeerId,
         peer_client: Client,
         blob_index: u64,
-        blob: SignedBlobSidecar<T::EthSpec>,
+        blob_sidecar: Arc<BlobSidecar<T::EthSpec>>,
         seen_timestamp: Duration,
     ) -> Result<(), Error<T::EthSpec>> {
         let processor = self.clone();
@@ -223,7 +223,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                     peer_id,
                     peer_client,
                     blob_index,
-                    blob,
+                    blob_sidecar,
                     seen_timestamp,
                 )
                 .await
@@ -231,7 +231,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
 
         self.try_send(BeaconWorkEvent {
             drop_during_sync: false,
-            work: Work::GossipSignedBlobSidecar(Box::pin(process_fn)),
+            work: Work::GossipBlobSidecar(Box::pin(process_fn)),
         })
     }
 

beacon_node/network/src/network_beacon_processor/sync_methods.rs

@@ -292,7 +292,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
     ) {
         let Some(slot) = blobs
             .iter()
-            .find_map(|blob| blob.as_ref().map(|blob| blob.slot))
+            .find_map(|blob| blob.as_ref().map(|blob| blob.slot()))
         else {
             return;
         };

beacon_node/network/src/network_beacon_processor/tests.rs

@@ -33,8 +33,8 @@ use std::time::Duration;
 use tokio::sync::mpsc;
 use types::blob_sidecar::FixedBlobSidecarList;
 use types::{
-    Attestation, AttesterSlashing, Epoch, Hash256, MainnetEthSpec, ProposerSlashing,
-    SignedAggregateAndProof, SignedBeaconBlock, SignedBlobSidecarList, SignedVoluntaryExit, Slot,
+    Attestation, AttesterSlashing, BlobSidecar, BlobSidecarList, Epoch, Hash256, MainnetEthSpec,
+    ProposerSlashing, SignedAggregateAndProof, SignedBeaconBlock, SignedVoluntaryExit, Slot,
     SubnetId,
 };
 
@@ -55,7 +55,7 @@ const STANDARD_TIMEOUT: Duration = Duration::from_secs(10);
 struct TestRig {
     chain: Arc<BeaconChain<T>>,
     next_block: Arc<SignedBeaconBlock<E>>,
-    next_blobs: Option<SignedBlobSidecarList<E>>,
+    next_blobs: Option<BlobSidecarList<E>>,
     attestations: Vec<(Attestation<E>, SubnetId)>,
     next_block_attestations: Vec<(Attestation<E>, SubnetId)>,
     next_block_aggregate_attestations: Vec<SignedAggregateAndProof<E>>,
@@ -186,8 +186,10 @@ impl TestRig {
 
         let log = harness.logger().clone();
 
-        let mut beacon_processor_config = BeaconProcessorConfig::default();
-        beacon_processor_config.enable_backfill_rate_limiting = enable_backfill_rate_limiting;
+        let beacon_processor_config = BeaconProcessorConfig {
+            enable_backfill_rate_limiting,
+            ..Default::default()
+        };
         let BeaconProcessorChannels {
             beacon_processor_tx,
             beacon_processor_rx,
@@ -243,12 +245,17 @@ impl TestRig {
             chain.spec.maximum_gossip_clock_disparity(),
         );
 
-        assert!(!beacon_processor.is_err());
-
+        assert!(beacon_processor.is_ok());
+        let block = next_block_tuple.0;
+        let blob_sidecars = if let Some((kzg_proofs, blobs)) = next_block_tuple.1 {
+            Some(BlobSidecar::build_sidecars(blobs, &block, kzg_proofs).unwrap())
+        } else {
+            None
+        };
         Self {
             chain,
-            next_block: Arc::new(next_block_tuple.0),
-            next_blobs: next_block_tuple.1,
+            next_block: Arc::new(block),
+            next_blobs: blob_sidecars,
             attestations,
             next_block_attestations,
             next_block_aggregate_attestations,
@@ -293,7 +300,7 @@ impl TestRig {
                     junk_message_id(),
                     junk_peer_id(),
                     Client::default(),
-                    blob.message.index,
+                    blob.index,
                     blob.clone(),
                     Duration::from_secs(0),
                 )
@@ -306,7 +313,7 @@ impl TestRig {
         self.network_beacon_processor
             .send_rpc_beacon_block(
                 block_root,
-                RpcBlock::new_without_blobs(Some(block_root), self.next_block.clone().into()),
+                RpcBlock::new_without_blobs(Some(block_root), self.next_block.clone()),
                 std::time::Duration::default(),
                 BlockProcessType::ParentLookup {
                     chain_hash: Hash256::random(),
@@ -320,7 +327,7 @@ impl TestRig {
         self.network_beacon_processor
             .send_rpc_beacon_block(
                 block_root,
-                RpcBlock::new_without_blobs(Some(block_root), self.next_block.clone().into()),
+                RpcBlock::new_without_blobs(Some(block_root), self.next_block.clone()),
                 std::time::Duration::default(),
                 BlockProcessType::SingleBlock { id: 1 },
             )
@@ -328,12 +335,7 @@ impl TestRig {
     }
     pub fn enqueue_single_lookup_rpc_blobs(&self) {
         if let Some(blobs) = self.next_blobs.clone() {
-            let blobs = FixedBlobSidecarList::from(
-                blobs
-                    .into_iter()
-                    .map(|b| Some(b.message))
-                    .collect::<Vec<_>>(),
-            );
+            let blobs = FixedBlobSidecarList::from(blobs.into_iter().map(Some).collect::<Vec<_>>());
             self.network_beacon_processor
                 .send_rpc_blobs(
                     self.next_block.canonical_root(),

beacon_node/network/src/router.rs

@@ -302,14 +302,14 @@ impl<T: BeaconChainTypes> Router<T> {
                 ),
             ),
             PubsubMessage::BlobSidecar(data) => {
-                let (blob_index, signed_blob) = *data;
+                let (blob_index, blob_sidecar) = *data;
                 self.handle_beacon_processor_send_result(
                     self.network_beacon_processor.send_gossip_blob_sidecar(
                         message_id,
                         peer_id,
                         self.network_globals.client(&peer_id),
                         blob_index,
-                        signed_blob,
+                        blob_sidecar,
                         timestamp_now(),
                     ),
                 )

beacon_node/network/src/sync/block_lookups/common.rs

@@ -428,7 +428,7 @@ impl<L: Lookup, T: BeaconChainTypes> RequestState<L, T> for BlobRequestState<L,
         verified_response
             .into_iter()
             .filter_map(|blob| blob.as_ref())
-            .map(|blob| blob.block_parent_root)
+            .map(|blob| blob.block_parent_root())
             .next()
     }
 

beacon_node/network/src/sync/block_lookups/tests.rs

@@ -213,10 +213,8 @@ impl TestRig {
     ) -> (SignedBeaconBlock<E>, Vec<BlobSidecar<E>>) {
         let (mut block, mut blobs) = self.rand_block_and_blobs(fork_name, num_blobs);
         *block.message_mut().parent_root_mut() = parent_root;
-        let block_root = block.canonical_root();
         blobs.iter_mut().for_each(|blob| {
-            blob.block_parent_root = parent_root;
-            blob.block_root = block_root;
+            blob.signed_block_header = block.signed_block_header();
         });
         (block, blobs)
     }
@@ -1293,7 +1291,7 @@ mod deneb_only {
 
                         let child_blob = blobs.first().cloned().unwrap();
                         let parent_root = block_root;
-                        let child_root = child_blob.block_root;
+                        let child_root = child_blob.block_root();
                         block_root = child_root;
 
                         let mut blobs = FixedBlobSidecarList::default();

beacon_node/network/src/sync/block_sidecar_coupling.rs

@@ -46,7 +46,7 @@ impl<T: EthSpec> BlocksAndBlobsRequestInfo<T> {
             while {
                 let pair_next_blob = blob_iter
                     .peek()
-                    .map(|sidecar| sidecar.slot == block.slot())
+                    .map(|sidecar| sidecar.slot() == block.slot())
                     .unwrap_or(false);
                 pair_next_blob
             } {

beacon_node/network/src/sync/manager.rs

@@ -637,9 +637,9 @@ impl<T: BeaconChainTypes> SyncManager<T> {
                 );
             }
             SyncMessage::UnknownParentBlob(peer_id, blob) => {
-                let blob_slot = blob.slot;
-                let block_root = blob.block_root;
-                let parent_root = blob.block_parent_root;
+                let blob_slot = blob.slot();
+                let block_root = blob.block_root();
+                let parent_root = blob.block_parent_root();
                 let blob_index = blob.index;
                 if blob_index >= T::EthSpec::max_blobs_per_block() as u64 {
                     warn!(self.log, "Peer sent blob with invalid index"; "index" => blob_index, "peer_id" => %peer_id);