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.

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -5001,7 +5001,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             bls_to_execution_changes,
         } = partial_beacon_block;
 
-        let (inner_block, blobs_opt, proofs_opt, execution_payload_value) = match &state {
+        let (inner_block, maybe_blobs_and_proofs, execution_payload_value) = match &state {
             BeaconState::Base(_) => (
                 BeaconBlock::Base(BeaconBlockBase {
                     slot,
@@ -5021,7 +5021,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     },
                 }),
                 None,
-                None,
                 Uint256::zero(),
             ),
             BeaconState::Altair(_) => (
@@ -5045,7 +5044,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     },
                 }),
                 None,
-                None,
                 Uint256::zero(),
             ),
             BeaconState::Merge(_) => {
@@ -5076,7 +5074,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                         },
                     }),
                     None,
-                    None,
                     execution_payload_value,
                 )
             }
@@ -5110,12 +5107,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                         },
                     }),
                     None,
-                    None,
                     execution_payload_value,
                 )
             }
             BeaconState::Deneb(_) => {
-                let (payload, kzg_commitments, blobs, proofs, execution_payload_value) =
+                let (payload, kzg_commitments, maybe_blobs_and_proofs, execution_payload_value) =
                     block_contents
                         .ok_or(BlockProductionError::MissingExecutionPayload)?
                         .deconstruct();
@@ -5145,8 +5141,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                                 .ok_or(BlockProductionError::InvalidPayloadFork)?,
                         },
                     }),
-                    blobs,
-                    proofs,
+                    maybe_blobs_and_proofs,
                     execution_payload_value,
                 )
             }
@@ -5205,8 +5200,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
         let blobs_verification_timer =
             metrics::start_timer(&metrics::BLOCK_PRODUCTION_BLOBS_VERIFICATION_TIMES);
-        let blob_item = match (blobs_opt, proofs_opt) {
-            (Some(blobs), Some(proofs)) => {
+        let blob_items = match maybe_blobs_and_proofs {
+            Some((blobs, proofs)) => {
                 let expected_kzg_commitments =
                     block.body().blob_kzg_commitments().map_err(|_| {
                         BlockProductionError::InvalidBlockVariant(
@@ -5239,7 +5234,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
                 Some((kzg_proofs.into(), blobs))
             }
-            _ => None,
+            None => None,
         };
 
         drop(blobs_verification_timer);
@@ -5257,7 +5252,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         Ok(BeaconBlockResponse {
             block,
             state,
-            blob_items: blob_item,
+            blob_items,
             execution_payload_value: Some(execution_payload_value),
             consensus_block_value: Some(consensus_block_value),
         })

beacon_node/beacon_chain/src/block_verification.rs

@@ -616,7 +616,7 @@ pub fn signature_verify_chain_segment<T: BeaconChainTypes>(
         .collect::<Vec<_>>();
 
     // verify signatures
-    let pubkey_cache = get_validator_pubkey_cache(chain).map_err(BlockError::from)?;
+    let pubkey_cache = get_validator_pubkey_cache(chain)?;
     let mut signature_verifier = get_signature_verifier(&state, &pubkey_cache, &chain.spec);
     for svb in &mut signature_verified_blocks {
         signature_verifier
@@ -923,7 +923,7 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
         };
 
         let signature_is_valid = {
-            let pubkey_cache = get_validator_pubkey_cache(chain).map_err(BlockError::from)?;
+            let pubkey_cache = get_validator_pubkey_cache(chain)?;
             let pubkey = pubkey_cache
                 .get(block.message().proposer_index() as usize)
                 .ok_or_else(|| BlockError::UnknownValidator(block.message().proposer_index()))?;
@@ -1036,7 +1036,7 @@ impl<T: BeaconChainTypes> SignatureVerifiedBlock<T> {
             &chain.spec,
         )?;
 
-        let pubkey_cache = get_validator_pubkey_cache(chain).map_err(BlockError::from)?;
+        let pubkey_cache = get_validator_pubkey_cache(chain)?;
 
         let mut signature_verifier = get_signature_verifier(&state, &pubkey_cache, &chain.spec);
 
@@ -1087,7 +1087,7 @@ impl<T: BeaconChainTypes> SignatureVerifiedBlock<T> {
             &chain.spec,
         )?;
 
-        let pubkey_cache = get_validator_pubkey_cache(chain).map_err(BlockError::from)?;
+        let pubkey_cache = get_validator_pubkey_cache(chain)?;
 
         let mut signature_verifier = get_signature_verifier(&state, &pubkey_cache, &chain.spec);
 
@@ -1918,7 +1918,7 @@ fn load_parent<T: BeaconChainTypes, B: AsBlock<T::EthSpec>>(
     result
 }
 
-/// This trait is used to unify `BlockError` and `BlobError`.
+/// This trait is used to unify `BlockError` and `GossipBlobError`.
 pub trait BlockBlobError: From<BeaconStateError> + From<BeaconChainError> + Debug {
     fn not_later_than_parent_error(block_slot: Slot, state_slot: Slot) -> Self;
     fn unknown_validator_error(validator_index: u64) -> Self;
@@ -2055,8 +2055,7 @@ fn verify_header_signature<T: BeaconChainTypes, Err: BlockBlobError>(
     chain: &BeaconChain<T>,
     header: &SignedBeaconBlockHeader,
 ) -> Result<(), Err> {
-    let proposer_pubkey = get_validator_pubkey_cache(chain)
-        .map_err(BeaconChainError::from)?
+    let proposer_pubkey = get_validator_pubkey_cache(chain)?
         .get(header.message.proposer_index as usize)
         .cloned()
         .ok_or(Err::unknown_validator_error(header.message.proposer_index))?;

beacon_node/beacon_chain/src/observed_blob_sidecars.rs

@@ -45,7 +45,7 @@ impl<E: EthSpec> Default for ObservedBlobSidecars<E> {
 }
 
 impl<T: EthSpec> ObservedBlobSidecars<T> {
-    /// Observe the `blob_sidecar` at (`blob_sidecar.block_root, blob_sidecar.slot`).
+    /// Observe the `blob_sidecar` at (`blob_sidecar.block_proposer_index, blob_sidecar.slot`).
     /// This will update `self` so future calls to it indicate that this `blob_sidecar` is known.
     ///
     /// The supplied `blob_sidecar` **MUST** have completed proposer signature verification.
@@ -138,7 +138,7 @@ mod tests {
         assert_eq!(
             cache.items.len(),
             1,
-            "only one (slot, root) tuple should be present"
+            "only one (validator_index, slot) tuple should be present"
         );
         assert_eq!(
             cache

beacon_node/execution_layer/src/lib.rs

@@ -105,8 +105,7 @@ impl<E: EthSpec> TryFrom<BuilderBid<E>> for ProvenancedPayload<BlockProposalCont
                 payload: ExecutionPayloadHeader::Deneb(builder_bid.header).into(),
                 block_value: builder_bid.value,
                 kzg_commitments: builder_bid.blob_kzg_commitments,
-                blobs: None,
-                proofs: None,
+                blobs_and_proofs: None,
             },
         };
         Ok(ProvenancedPayload::Builder(
@@ -168,8 +167,8 @@ pub enum BlockProposalContents<T: EthSpec, Payload: AbstractExecPayload<T>> {
         payload: Payload,
         block_value: Uint256,
         kzg_commitments: KzgCommitments<T>,
-        blobs: Option<BlobsList<T>>,
-        proofs: Option<KzgProofs<T>>,
+        /// `None` for blinded `PayloadAndBlobs`.
+        blobs_and_proofs: Option<(BlobsList<T>, KzgProofs<T>)>,
     },
 }
 
@@ -201,8 +200,7 @@ impl<E: EthSpec, Payload: AbstractExecPayload<E>> TryFrom<GetPayloadResponse<E>>
                 payload: execution_payload.into(),
                 block_value,
                 kzg_commitments: bundle.commitments,
-                blobs: Some(bundle.blobs),
-                proofs: Some(bundle.proofs),
+                blobs_and_proofs: Some((bundle.blobs, bundle.proofs)),
             }),
             None => Ok(Self::Payload {
                 payload: execution_payload.into(),
@@ -230,22 +228,25 @@ impl<T: EthSpec, Payload: AbstractExecPayload<T>> BlockProposalContents<T, Paylo
     ) -> (
         Payload,
         Option<KzgCommitments<T>>,
-        Option<BlobsList<T>>,
-        Option<KzgProofs<T>>,
+        Option<(BlobsList<T>, KzgProofs<T>)>,
         Uint256,
     ) {
         match self {
             Self::Payload {
                 payload,
                 block_value,
-            } => (payload, None, None, None, block_value),
+            } => (payload, None, None, block_value),
             Self::PayloadAndBlobs {
                 payload,
                 block_value,
                 kzg_commitments,
-                blobs,
-                proofs,
-            } => (payload, Some(kzg_commitments), blobs, proofs, block_value),
+                blobs_and_proofs,
+            } => (
+                payload,
+                Some(kzg_commitments),
+                blobs_and_proofs,
+                block_value,
+            ),
         }
     }
 

common/eth2/src/types.rs

@@ -1705,7 +1705,7 @@ impl<T: EthSpec> SignedBlockContents<T> {
     }
 }
 
-/// Converting from a Blinded
+/// Converting from a `SignedBlindedBeaconBlock` into a full `SignedBlockContents`.
 pub fn into_full_block_and_blobs<T: EthSpec>(
     blinded_block: SignedBlindedBeaconBlock<T>,
     maybe_full_payload_contents: Option<FullPayloadContents<T>>,