Merge pull request #3832 from divagant-martian/4844-backfill-fixes

Remove sync validation no longer valid + Fix bakcfill batch sizes

beacon_node/network/src/sync/backfill_sync/mod.rs

@@ -33,7 +33,7 @@ use types::{Epoch, EthSpec};
 /// we will negatively report peers with poor bandwidth. This can be set arbitrarily high, in which
 /// case the responder will fill the response up to the max request size, assuming they have the
 /// bandwidth to do so.
-pub const BACKFILL_EPOCHS_PER_BATCH: u64 = 2;
+pub const BACKFILL_EPOCHS_PER_BATCH: u64 = 1;
 
 /// The maximum number of batches to queue before requesting more.
 const BACKFILL_BATCH_BUFFER_SIZE: u8 = 20;

beacon_node/network/src/sync/network_context.rs

@@ -531,25 +531,21 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
         id
     }
 
+    /// Check whether a batch for this epoch (and only this epoch) should request just blocks or
+    /// blocks and blobs.
     pub fn batch_type(&self, epoch: types::Epoch) -> ExpectedBatchTy {
-        // Keep tests only for blocks.
+        const _: () = assert!(
+            super::backfill_sync::BACKFILL_EPOCHS_PER_BATCH == 1
+                && super::range_sync::EPOCHS_PER_BATCH == 1,
+            "To deal with alignment with 4844 boundaries, batches need to be of just one epoch"
+        );
         #[cfg(test)]
         {
+            // Keep tests only for blocks.
             return ExpectedBatchTy::OnlyBlock;
         }
         #[cfg(not(test))]
         {
-            use super::range_sync::EPOCHS_PER_BATCH;
-            assert_eq!(
-                EPOCHS_PER_BATCH, 1,
-                "If this is not one, everything will fail horribly"
-            );
-
-            // Here we need access to the beacon chain, check the fork boundary, the current epoch, the
-            // blob period to serve and check with that if the batch is a blob batch or not.
-            // NOTE: This would carelessly assume batch sizes are always 1 epoch, to avoid needing to
-            // align with the batch boundary.
-
             if let Some(data_availability_boundary) = self.chain.data_availability_boundary() {
                 if epoch >= data_availability_boundary {
                     ExpectedBatchTy::OnlyBlockBlobs

beacon_node/network/src/sync/range_sync/batch.rs

@@ -5,6 +5,7 @@ use std::collections::HashSet;
 use std::hash::{Hash, Hasher};
 use std::ops::Sub;
 use std::sync::Arc;
+use strum::Display;
 use types::signed_block_and_blobs::BlockWrapper;
 use types::{Epoch, EthSpec, SignedBeaconBlock, SignedBeaconBlockAndBlobsSidecar, Slot};
 
@@ -40,7 +41,8 @@ impl<T: EthSpec> BatchTy<T> {
 pub struct MixedBlockTyErr;
 
 /// Type of expected batch.
-#[derive(Debug, Clone)]
+#[derive(Debug, Copy, Clone, Display)]
+#[strum(serialize_all = "snake_case")]
 pub enum ExpectedBatchTy {
     OnlyBlockBlobs,
     OnlyBlock,
@@ -247,7 +249,7 @@ impl<T: EthSpec, B: BatchConfig> BatchInfo<T, B> {
                 start_slot: self.start_slot.into(),
                 count: self.end_slot.sub(self.start_slot).into(),
             },
-            self.batch_type.clone(),
+            self.batch_type,
         )
     }
 
@@ -557,6 +559,7 @@ impl<T: EthSpec, B: BatchConfig> slog::KV for BatchInfo<T, B> {
         serializer.emit_usize("processed", self.failed_processing_attempts.len())?;
         serializer.emit_u8("processed_no_penalty", self.non_faulty_processing_attempts)?;
         serializer.emit_arguments("state", &format_args!("{:?}", self.state))?;
+        serializer.emit_arguments("batch_ty", &format_args!("{}", self.batch_type));
         slog::Result::Ok(())
     }
 }