Fix rpc limits version 2 (#3146)

## Issue Addressed

N/A

## Proposed Changes

https://github.com/sigp/lighthouse/pull/3133 changed the rpc type limits to be fork aware i.e. if our current fork based on wall clock slot is Altair, then we apply only altair rpc type limits. This is a bug because phase0 blocks can still be sent over rpc and phase 0 block minimum size is smaller than altair block minimum size. So a phase0 block with `size < SIGNED_BEACON_BLOCK_ALTAIR_MIN` will return an `InvalidData` error as it doesn't pass the rpc types bound check.

This error can be seen when we try syncing pre-altair blocks with size smaller than `SIGNED_BEACON_BLOCK_ALTAIR_MIN`.

This PR fixes the issue by also accounting for forks earlier than current_fork in the rpc limits calculation in the  `rpc_block_limits_by_fork` function. I decided to hardcode the limits in the function because that seemed simpler than calculating previous forks based on current fork and doing a min across forks. Adding a new fork variant is simple and can the limits can be easily checked in a review. 

Adds unit tests and modifies the syncing simulator to check the syncing from across fork boundaries. 
The syncing simulator's block 1 would always be of phase 0 minimum size (404 bytes) which is smaller than altair min block size (since block 1 contains no attestations).

beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs

@@ -659,9 +659,11 @@ mod tests {
         ForkContext::new::<Spec>(current_slot, Hash256::zero(), &chain_spec)
     }
 
-    fn base_block() -> SignedBeaconBlock<Spec> {
+    /// Smallest sized block across all current forks. Useful for testing
-        let full_block = BeaconBlock::Base(BeaconBlockBase::<Spec>::full(&Spec::default_spec()));
+    /// min length check conditions.
-        SignedBeaconBlock::from_block(full_block, Signature::empty())
+    fn empty_base_block() -> SignedBeaconBlock<Spec> {
+        let empty_block = BeaconBlock::Base(BeaconBlockBase::<Spec>::empty(&Spec::default_spec()));
+        SignedBeaconBlock::from_block(empty_block, Signature::empty())
     }
 
     fn altair_block() -> SignedBeaconBlock<Spec> {
@@ -830,10 +832,12 @@ mod tests {
             encode_then_decode(
                 Protocol::BlocksByRange,
                 Version::V1,
-                RPCCodedResponse::Success(RPCResponse::BlocksByRange(Box::new(base_block()))),
+                RPCCodedResponse::Success(RPCResponse::BlocksByRange(Box::new(empty_base_block()))),
                 ForkName::Base,
             ),
-            Ok(Some(RPCResponse::BlocksByRange(Box::new(base_block()))))
+            Ok(Some(RPCResponse::BlocksByRange(Box::new(
+                empty_base_block()
+            ))))
         );
 
         assert!(
@@ -854,10 +858,12 @@ mod tests {
             encode_then_decode(
                 Protocol::BlocksByRoot,
                 Version::V1,
-                RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(base_block()))),
+                RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(empty_base_block()))),
                 ForkName::Base,
             ),
-            Ok(Some(RPCResponse::BlocksByRoot(Box::new(base_block()))))
+            Ok(Some(RPCResponse::BlocksByRoot(
+                Box::new(empty_base_block())
+            )))
         );
 
         assert!(
@@ -941,10 +947,27 @@ mod tests {
             encode_then_decode(
                 Protocol::BlocksByRange,
                 Version::V2,
-                RPCCodedResponse::Success(RPCResponse::BlocksByRange(Box::new(base_block()))),
+                RPCCodedResponse::Success(RPCResponse::BlocksByRange(Box::new(empty_base_block()))),
                 ForkName::Base,
             ),
-            Ok(Some(RPCResponse::BlocksByRange(Box::new(base_block()))))
+            Ok(Some(RPCResponse::BlocksByRange(Box::new(
+                empty_base_block()
+            ))))
+        );
+
+        // Decode the smallest possible base block when current fork is altair
+        // This is useful for checking that we allow for blocks smaller than
+        // the current_fork's rpc limit
+        assert_eq!(
+            encode_then_decode(
+                Protocol::BlocksByRange,
+                Version::V2,
+                RPCCodedResponse::Success(RPCResponse::BlocksByRange(Box::new(empty_base_block()))),
+                ForkName::Altair,
+            ),
+            Ok(Some(RPCResponse::BlocksByRange(Box::new(
+                empty_base_block()
+            ))))
         );
 
         assert_eq!(
@@ -996,10 +1019,27 @@ mod tests {
             encode_then_decode(
                 Protocol::BlocksByRoot,
                 Version::V2,
-                RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(base_block()))),
+                RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(empty_base_block()))),
                 ForkName::Base,
             ),
-            Ok(Some(RPCResponse::BlocksByRoot(Box::new(base_block())))),
+            Ok(Some(RPCResponse::BlocksByRoot(
+                Box::new(empty_base_block())
+            ))),
+        );
+
+        // Decode the smallest possible base block when current fork is altair
+        // This is useful for checking that we allow for blocks smaller than
+        // the current_fork's rpc limit
+        assert_eq!(
+            encode_then_decode(
+                Protocol::BlocksByRoot,
+                Version::V2,
+                RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(empty_base_block()))),
+                ForkName::Altair,
+            ),
+            Ok(Some(RPCResponse::BlocksByRoot(
+                Box::new(empty_base_block())
+            )))
         );
 
         assert_eq!(
@@ -1073,7 +1113,7 @@ mod tests {
         let mut encoded_bytes = encode(
             Protocol::BlocksByRange,
             Version::V2,
-            RPCCodedResponse::Success(RPCResponse::BlocksByRange(Box::new(base_block()))),
+            RPCCodedResponse::Success(RPCResponse::BlocksByRange(Box::new(empty_base_block()))),
             ForkName::Base,
         )
         .unwrap();
@@ -1094,7 +1134,7 @@ mod tests {
         let mut encoded_bytes = encode(
             Protocol::BlocksByRoot,
             Version::V2,
-            RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(base_block()))),
+            RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(empty_base_block()))),
             ForkName::Base,
         )
         .unwrap();
@@ -1116,7 +1156,7 @@ mod tests {
         let mut encoded_bytes = encode(
             Protocol::BlocksByRange,
             Version::V2,
-            RPCCodedResponse::Success(RPCResponse::BlocksByRange(Box::new(base_block()))),
+            RPCCodedResponse::Success(RPCResponse::BlocksByRange(Box::new(empty_base_block()))),
             ForkName::Altair,
         )
         .unwrap();
@@ -1186,7 +1226,7 @@ mod tests {
         let mut encoded_bytes = encode(
             Protocol::BlocksByRoot,
             Version::V2,
-            RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(base_block()))),
+            RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(empty_base_block()))),
             ForkName::Altair,
         )
         .unwrap();
@@ -1210,7 +1250,7 @@ mod tests {
         let mut encoded_bytes = encode(
             Protocol::BlocksByRoot,
             Version::V2,
-            RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(base_block()))),
+            RPCCodedResponse::Success(RPCResponse::BlocksByRoot(Box::new(empty_base_block()))),
             ForkName::Altair,
         )
         .unwrap();

beacon_node/lighthouse_network/src/rpc/protocol.rs

@@ -118,6 +118,26 @@ pub fn max_rpc_size(fork_context: &ForkContext) -> usize {
     }
 }
 
+/// Returns the rpc limits for beacon_block_by_range and beacon_block_by_root responses.
+///
+/// Note: This function should take care to return the min/max limits accounting for all
+/// previous valid forks when adding a new fork variant.
+pub fn rpc_block_limits_by_fork(current_fork: ForkName) -> RpcLimits {
+    match &current_fork {
+        ForkName::Base => {
+            RpcLimits::new(*SIGNED_BEACON_BLOCK_BASE_MIN, *SIGNED_BEACON_BLOCK_BASE_MAX)
+        }
+        ForkName::Altair => RpcLimits::new(
+            *SIGNED_BEACON_BLOCK_BASE_MIN, // Base block is smaller than altair blocks
+            *SIGNED_BEACON_BLOCK_ALTAIR_MAX, // Altair block is larger than base blocks
+        ),
+        ForkName::Merge => RpcLimits::new(
+            *SIGNED_BEACON_BLOCK_BASE_MIN, // Base block is smaller than altair and merge blocks
+            *SIGNED_BEACON_BLOCK_MERGE_MAX, // Merge block is larger than base and altair blocks
+        ),
+    }
+}
+
 /// Protocol names to be used.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Protocol {
@@ -281,32 +301,8 @@ impl ProtocolId {
                 <StatusMessage as Encode>::ssz_fixed_len(),
             ),
             Protocol::Goodbye => RpcLimits::new(0, 0), // Goodbye request has no response
-            Protocol::BlocksByRange => match fork_context.current_fork() {
+            Protocol::BlocksByRange => rpc_block_limits_by_fork(fork_context.current_fork()),
-                ForkName::Base => {
+            Protocol::BlocksByRoot => rpc_block_limits_by_fork(fork_context.current_fork()),
-                    RpcLimits::new(*SIGNED_BEACON_BLOCK_BASE_MIN, *SIGNED_BEACON_BLOCK_BASE_MAX)
-                }
-                ForkName::Altair => RpcLimits::new(
-                    *SIGNED_BEACON_BLOCK_ALTAIR_MIN,
-                    *SIGNED_BEACON_BLOCK_ALTAIR_MAX,
-                ),
-                ForkName::Merge => RpcLimits::new(
-                    *SIGNED_BEACON_BLOCK_MERGE_MIN,
-                    *SIGNED_BEACON_BLOCK_MERGE_MAX,
-                ),
-            },
-            Protocol::BlocksByRoot => match fork_context.current_fork() {
-                ForkName::Base => {
-                    RpcLimits::new(*SIGNED_BEACON_BLOCK_BASE_MIN, *SIGNED_BEACON_BLOCK_BASE_MAX)
-                }
-                ForkName::Altair => RpcLimits::new(
-                    *SIGNED_BEACON_BLOCK_ALTAIR_MIN,
-                    *SIGNED_BEACON_BLOCK_ALTAIR_MAX,
-                ),
-                ForkName::Merge => RpcLimits::new(
-                    *SIGNED_BEACON_BLOCK_MERGE_MIN,
-                    *SIGNED_BEACON_BLOCK_MERGE_MAX,
-                ),
-            },
 
             Protocol::Ping => RpcLimits::new(
                 <Ping as Encode>::ssz_fixed_len(),

testing/simulator/src/sync_sim.rs

@@ -62,6 +62,9 @@ fn syncing_sim(
     let end_after_checks = true;
     let eth1_block_time = Duration::from_millis(15_000 / speed_up_factor);
 
+    // Set fork epochs to test syncing across fork boundaries
+    spec.altair_fork_epoch = Some(Epoch::new(1));
+    spec.bellatrix_fork_epoch = Some(Epoch::new(2));
     spec.seconds_per_slot /= speed_up_factor;
     spec.seconds_per_slot = max(1, spec.seconds_per_slot);
     spec.eth1_follow_distance = 16;
@@ -86,6 +89,8 @@ fn syncing_sim(
     beacon_config.dummy_eth1_backend = true;
     beacon_config.sync_eth1_chain = true;
 
+    beacon_config.http_api.allow_sync_stalled = true;
+
     beacon_config.network.enr_address = Some(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)));
 
     // Generate the directories and keystores required for the validator clients.