From 2c4413454a16ae5a6615c9e4524c00495b32de75 Mon Sep 17 00:00:00 2001
From: ethDreamer <mark@sigmaprime.io>
Date: Fri, 29 Oct 2021 00:05:27 +0000
Subject: [PATCH] Fixed Gossip Topics on Fork Boundary (#2619)

## Issue Addressed

The [p2p-interface section of the `altair` spec](https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/p2p-interface.md#transitioning-the-gossip) says you should subscribe to the topics for a fork "In advance of the fork" and unsubscribe from old topics `2 Epochs` after the new fork is activated. We've chosen to subscribe to new fork topics `2 slots` before the fork is initiated.

This function is supposed to return the required fork digests at any given time but as it was currently written, it doesn't return the fork digest for a previous fork if you've switched to the current fork less than 2 epoch's ago. Also this function required modification for every new fork we add.

## Proposed Changes

Make this function fork-agnostic and correctly handle the previous fork topic digests when you've only just switched to the new fork.
---
 beacon_node/network/src/service.rs | 50 +++++++++++++++++-------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs
index 49c15c8da..6eeb17123 100644
--- a/beacon_node/network/src/service.rs
+++ b/beacon_node/network/src/service.rs
@@ -25,8 +25,8 @@ use task_executor::ShutdownReason;
 use tokio::sync::mpsc;
 use tokio::time::Sleep;
 use types::{
-    ChainSpec, EthSpec, ForkContext, ForkName, RelativeEpoch, Slot, SubnetId,
-    SyncCommitteeSubscription, SyncSubnetId, Unsigned, ValidatorSubscription,
+    ChainSpec, EthSpec, ForkContext, RelativeEpoch, Slot, SubnetId, SyncCommitteeSubscription,
+    SyncSubnetId, Unsigned, ValidatorSubscription,
 };
 
 mod tests;
@@ -281,28 +281,34 @@ impl<T: BeaconChainTypes> NetworkService<T> {
     pub fn required_gossip_fork_digests(&self) -> Vec<[u8; 4]> {
         let fork_context = &self.fork_context;
         let spec = &self.beacon_chain.spec;
-        match fork_context.current_fork() {
-            ForkName::Base => {
-                // If we are SUBSCRIBE_DELAY_SLOTS before the fork slot, subscribe only to Base,
-                // else subscribe to Base and Altair.
-                let current_slot = self.beacon_chain.slot().unwrap_or(spec.genesis_slot);
-                match spec.next_fork_epoch::<T::EthSpec>(current_slot) {
-                    Some((_, fork_epoch)) => {
-                        if current_slot.saturating_add(Slot::new(SUBSCRIBE_DELAY_SLOTS))
-                            >= fork_epoch.start_slot(T::EthSpec::slots_per_epoch())
-                        {
-                            fork_context.all_fork_digests()
-                        } else {
-                            vec![fork_context.genesis_context_bytes()]
-                        }
-                    }
-                    None => vec![fork_context.genesis_context_bytes()],
-                }
+        let current_slot = self.beacon_chain.slot().unwrap_or(spec.genesis_slot);
+        let current_fork = fork_context.current_fork();
+
+        let mut result = vec![fork_context
+            .to_context_bytes(current_fork)
+            .unwrap_or_else(|| {
+                panic!(
+                    "{} fork bytes should exist as it's initialized in ForkContext",
+                    current_fork
+                )
+            })];
+
+        if let Some((next_fork, fork_epoch)) = spec.next_fork_epoch::<T::EthSpec>(current_slot) {
+            if current_slot.saturating_add(Slot::new(SUBSCRIBE_DELAY_SLOTS))
+                >= fork_epoch.start_slot(T::EthSpec::slots_per_epoch())
+            {
+                let next_fork_context_bytes =
+                    fork_context.to_context_bytes(next_fork).unwrap_or_else(|| {
+                        panic!(
+                            "context bytes should exist as spec.next_fork_epoch({}) returned Some({})",
+                            current_slot, next_fork
+                        )
+                    });
+                result.push(next_fork_context_bytes);
             }
-            ForkName::Altair => vec![fork_context
-                .to_context_bytes(ForkName::Altair)
-                .expect("Altair fork bytes should exist as it's initialized in ForkContext")],
         }
+
+        result
     }
 }