Shift subnet backbone structure (attnets revamp) (#4304)

AgeManning · AgeManning · commit fdea8f2b27c2 · 2023-05-30T06:15:56.000Z
This PR address the following spec change: ethereum/consensus-specs#3312 Instead of subscribing to a long-lived subnet for every attached validator to a beacon node, all beacon nodes will subscribe to `SUBNETS_PER_NODE` long-lived subnets. This is currently set to 2 for mainnet. This PR does not include any scoring or advanced discovery mechanisms. A future PR will improve discovery and we can implement scoring after the next hard fork when we expect all client teams and all implementations to respect this spec change. This will be a significant change in the subnet network structure for consensus clients and we will likely have to monitor and tweak our peer management logic.
diff --git a/beacon_node/network/Cargo.toml b/beacon_node/network/Cargo.toml
@@ -46,8 +46,4 @@ derivative = "2.2.0"
 delay_map = "0.3.0"
 ethereum-types = { version = "0.14.1", optional = true }
 operation_pool =  { path = "../operation_pool" }
-execution_layer =  { path = "../execution_layer" }
-
-[features]
-deterministic_long_lived_attnets = [ "ethereum-types" ]
-# default = ["deterministic_long_lived_attnets"]
+execution_layer =  { path = "../execution_layer" }
diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs
@@ -317,8 +317,7 @@ impl<T: BeaconChainTypes> NetworkService<T> {
         // attestation subnet service
         let attestation_service = AttestationService::new(
             beacon_chain.clone(),
-            #[cfg(feature = "deterministic_long_lived_attnets")]
-            network_globals.local_enr().node_id().raw().into(),
+            network_globals.local_enr().node_id(),
             config,
             &network_log,
         );
diff --git a/beacon_node/network/src/subnet_service/attestation_subnets.rs b/beacon_node/network/src/subnet_service/attestation_subnets.rs
diff --git a/beacon_node/network/src/subnet_service/tests/mod.rs b/beacon_node/network/src/subnet_service/tests/mod.rs
diff --git a/common/eth2_network_config/built_in_network_configs/gnosis/config.yaml b/common/eth2_network_config/built_in_network_configs/gnosis/config.yaml
@@ -86,3 +86,7 @@ PROPOSER_SCORE_BOOST: 40
 DEPOSIT_CHAIN_ID: 100
 DEPOSIT_NETWORK_ID: 100
 DEPOSIT_CONTRACT_ADDRESS: 0x0B98057eA310F4d31F2a452B414647007d1645d9
+
+# Network
+# ---------------------------------------------------------------
+SUBNETS_PER_NODE: 4
diff --git a/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml b/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml
@@ -86,3 +86,7 @@ PROPOSER_SCORE_BOOST: 40
 DEPOSIT_CHAIN_ID: 1
 DEPOSIT_NETWORK_ID: 1
 DEPOSIT_CONTRACT_ADDRESS: 0x00000000219ab540356cBB839Cbe05303d7705Fa
+
+# Network
+# ---------------------------------------------------------------
+SUBNETS_PER_NODE: 2
diff --git a/common/eth2_network_config/built_in_network_configs/prater/config.yaml b/common/eth2_network_config/built_in_network_configs/prater/config.yaml
@@ -86,3 +86,7 @@ DEPOSIT_CHAIN_ID: 5
 DEPOSIT_NETWORK_ID: 5
 # Prater test deposit contract on Goerli Testnet
 DEPOSIT_CONTRACT_ADDRESS: 0xff50ed3d0ec03aC01D4C79aAd74928BFF48a7b2b
+
+# Network
+# ---------------------------------------------------------------
+SUBNETS_PER_NODE: 2
diff --git a/common/eth2_network_config/built_in_network_configs/sepolia/config.yaml b/common/eth2_network_config/built_in_network_configs/sepolia/config.yaml
@@ -74,3 +74,7 @@ PROPOSER_SCORE_BOOST: 40
 DEPOSIT_CHAIN_ID: 11155111
 DEPOSIT_NETWORK_ID: 11155111
 DEPOSIT_CONTRACT_ADDRESS: 0x7f02C3E3c98b133055B8B348B2Ac625669Ed295D
+
+# Network
+# ---------------------------------------------------------------
+SUBNETS_PER_NODE: 2
diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs
@@ -168,11 +168,9 @@ pub struct ChainSpec {
     pub maximum_gossip_clock_disparity_millis: u64,
     pub target_aggregators_per_committee: u64,
     pub attestation_subnet_count: u64,
-    pub random_subnets_per_validator: u64,
-    pub epochs_per_random_subnet_subscription: u64,
     pub subnets_per_node: u8,
     pub epochs_per_subnet_subscription: u64,
-    attestation_subnet_extra_bits: u8,
+    pub attestation_subnet_extra_bits: u8,
 
     /*
      * Application params
@@ -455,17 +453,7 @@ impl ChainSpec {
 
     #[allow(clippy::integer_arithmetic)]
     pub const fn attestation_subnet_prefix_bits(&self) -> u32 {
-        // maybe use log2 when stable https://github.com/rust-lang/rust/issues/70887
-
-        // NOTE: this line is here simply to guarantee that if self.attestation_subnet_count type
-        // is changed, a compiler warning will be raised. This code depends on the type being u64.
-        let attestation_subnet_count: u64 = self.attestation_subnet_count;
-        let attestation_subnet_count_bits = if attestation_subnet_count == 0 {
-            0
-        } else {
-            63 - attestation_subnet_count.leading_zeros()
-        };
-
+        let attestation_subnet_count_bits = self.attestation_subnet_count.ilog2();
         self.attestation_subnet_extra_bits as u32 + attestation_subnet_count_bits
     }
 
@@ -625,13 +613,11 @@ impl ChainSpec {
             network_id: 1, // mainnet network id
             attestation_propagation_slot_range: 32,
             attestation_subnet_count: 64,
-            random_subnets_per_validator: 1,
-            subnets_per_node: 1,
+            subnets_per_node: 2,
             maximum_gossip_clock_disparity_millis: 500,
             target_aggregators_per_committee: 16,
-            epochs_per_random_subnet_subscription: 256,
             epochs_per_subnet_subscription: 256,
-            attestation_subnet_extra_bits: 6,
+            attestation_subnet_extra_bits: 0,
 
             /*
              * Application specific
@@ -852,13 +838,11 @@ impl ChainSpec {
             network_id: 100, // Gnosis Chain network id
             attestation_propagation_slot_range: 32,
             attestation_subnet_count: 64,
-            random_subnets_per_validator: 1,
-            subnets_per_node: 1,
+            subnets_per_node: 4, // Make this larger than usual to avoid network damage
             maximum_gossip_clock_disparity_millis: 500,
             target_aggregators_per_committee: 16,
-            epochs_per_random_subnet_subscription: 256,
             epochs_per_subnet_subscription: 256,
-            attestation_subnet_extra_bits: 6,
+            attestation_subnet_extra_bits: 0,
 
             /*
              * Application specific
@@ -946,6 +930,9 @@ pub struct Config {
     shard_committee_period: u64,
     #[serde(with = "serde_utils::quoted_u64")]
     eth1_follow_distance: u64,
+    #[serde(default = "default_subnets_per_node")]
+    #[serde(with = "serde_utils::quoted_u8")]
+    subnets_per_node: u8,
 
     #[serde(with = "serde_utils::quoted_u64")]
     inactivity_score_bias: u64,
@@ -1002,6 +989,10 @@ fn default_safe_slots_to_import_optimistically() -> u64 {
     128u64
 }
 
+fn default_subnets_per_node() -> u8 {
+    2u8
+}
+
 impl Default for Config {
     fn default() -> Self {
         let chain_spec = MainnetEthSpec::default_spec();
@@ -1084,6 +1075,7 @@ impl Config {
             min_validator_withdrawability_delay: spec.min_validator_withdrawability_delay,
             shard_committee_period: spec.shard_committee_period,
             eth1_follow_distance: spec.eth1_follow_distance,
+            subnets_per_node: spec.subnets_per_node,
 
             inactivity_score_bias: spec.inactivity_score_bias,
             inactivity_score_recovery_rate: spec.inactivity_score_recovery_rate,
@@ -1130,6 +1122,7 @@ impl Config {
             min_validator_withdrawability_delay,
             shard_committee_period,
             eth1_follow_distance,
+            subnets_per_node,
             inactivity_score_bias,
             inactivity_score_recovery_rate,
             ejection_balance,
@@ -1162,6 +1155,7 @@ impl Config {
             min_validator_withdrawability_delay,
             shard_committee_period,
             eth1_follow_distance,
+            subnets_per_node,
             inactivity_score_bias,
             inactivity_score_recovery_rate,
             ejection_balance,
diff --git a/consensus/types/src/config_and_preset.rs b/consensus/types/src/config_and_preset.rs
@@ -86,10 +86,6 @@ pub fn get_extra_fields(spec: &ChainSpec) -> HashMap<String, Value> {
         "domain_application_mask".to_uppercase()=> u32_hex(spec.domain_application_mask),
         "target_aggregators_per_committee".to_uppercase() =>
             spec.target_aggregators_per_committee.to_string().into(),
-        "random_subnets_per_validator".to_uppercase() =>
-            spec.random_subnets_per_validator.to_string().into(),
-        "epochs_per_random_subnet_subscription".to_uppercase() =>
-            spec.epochs_per_random_subnet_subscription.to_string().into(),
         "domain_contribution_and_proof".to_uppercase() =>
             u32_hex(spec.domain_contribution_and_proof),
         "domain_sync_committee".to_uppercase() => u32_hex(spec.domain_sync_committee),
diff --git a/consensus/types/src/subnet_id.rs b/consensus/types/src/subnet_id.rs
@@ -80,15 +80,26 @@ impl SubnetId {
         epoch: Epoch,
         spec: &ChainSpec,
     ) -> Result<(impl Iterator<Item = SubnetId>, Epoch), &'static str> {
+        // Simplify the variable name
+        let subscription_duration = spec.epochs_per_subnet_subscription;
+
         let node_id_prefix =
             (node_id >> (256 - spec.attestation_subnet_prefix_bits() as usize)).as_usize();
 
-        let subscription_event_idx = epoch.as_u64() / spec.epochs_per_subnet_subscription;
+        // NOTE: The as_u64() panics if the number is larger than u64::max_value(). This cannot be
+        // true as spec.epochs_per_subnet_subscription is a u64.
+        let node_offset = (node_id % ethereum_types::U256::from(subscription_duration)).as_u64();
+
+        // Calculate at which epoch this node needs to re-evaluate
+        let valid_until_epoch = epoch.as_u64()
+            + subscription_duration
+                .saturating_sub((epoch.as_u64() + node_offset) % subscription_duration);
+
+        let subscription_event_idx = (epoch.as_u64() + node_offset) / subscription_duration;
         let permutation_seed =
             ethereum_hashing::hash(&int_to_bytes::int_to_bytes8(subscription_event_idx));
 
         let num_subnets = 1 << spec.attestation_subnet_prefix_bits();
-
         let permutated_prefix = compute_shuffled_index(
             node_id_prefix,
             num_subnets,
@@ -107,7 +118,6 @@ impl SubnetId {
         let subnet_set_generator = (0..subnets_per_node).map(move |idx| {
             SubnetId::new((permutated_prefix + idx as u64) % attestation_subnet_count)
         });
-        let valid_until_epoch = (subscription_event_idx + 1) * spec.epochs_per_subnet_subscription;
         Ok((subnet_set_generator, valid_until_epoch.into()))
     }
 }
@@ -149,3 +159,80 @@ impl AsRef<str> for SubnetId {
         subnet_id_to_string(self.0)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// A set of tests compared to the python specification
+    #[test]
+    fn compute_subnets_for_epoch_unit_test() {
+        // Randomized variables used generated with the python specification
+        let node_ids = [
+            "0",
+            "88752428858350697756262172400162263450541348766581994718383409852729519486397",
+            "18732750322395381632951253735273868184515463718109267674920115648614659369468",
+            "27726842142488109545414954493849224833670205008410190955613662332153332462900",
+            "39755236029158558527862903296867805548949739810920318269566095185775868999998",
+            "31899136003441886988955119620035330314647133604576220223892254902004850516297",
+            "58579998103852084482416614330746509727562027284701078483890722833654510444626",
+            "28248042035542126088870192155378394518950310811868093527036637864276176517397",
+            "60930578857433095740782970114409273483106482059893286066493409689627770333527",
+            "103822458477361691467064888613019442068586830412598673713899771287914656699997",
+        ]
+        .into_iter()
+        .map(|v| ethereum_types::U256::from_dec_str(v).unwrap())
+        .collect::<Vec<_>>();
+
+        let epochs = [
+            54321u64, 1017090249, 1827566880, 846255942, 766597383, 1204990115, 1616209495,
+            1774367616, 1484598751, 3525502229,
+        ]
+        .into_iter()
+        .map(Epoch::from)
+        .collect::<Vec<_>>();
+
+        // Test mainnet
+        let spec = ChainSpec::mainnet();
+
+        // Calculated by hand
+        let expected_valid_time: Vec<u64> = [
+            54528, 1017090371, 1827567108, 846256076, 766597570, 1204990135, 1616209582,
+            1774367723, 1484598953, 3525502371,
+        ]
+        .into();
+
+        // Calculated from pyspec
+        let expected_subnets = vec![
+            vec![4u64, 5u64],
+            vec![61, 62],
+            vec![23, 24],
+            vec![38, 39],
+            vec![53, 54],
+            vec![39, 40],
+            vec![48, 49],
+            vec![39, 40],
+            vec![34, 35],
+            vec![37, 38],
+        ];
+
+        for x in 0..node_ids.len() {
+            println!("Test: {}", x);
+            println!(
+                "NodeId: {}\n Epoch: {}\n, expected_update_time: {}\n, expected_subnets: {:?}",
+                node_ids[x], epochs[x], expected_valid_time[x], expected_subnets[x]
+            );
+
+            let (computed_subnets, valid_time) = SubnetId::compute_subnets_for_epoch::<
+                crate::MainnetEthSpec,
+            >(node_ids[x], epochs[x], &spec)
+            .unwrap();
+
+            assert_eq!(Epoch::from(expected_valid_time[x]), valid_time);
+            assert_eq!(
+                expected_subnets[x],
+                computed_subnets.map(SubnetId::into).collect::<Vec<u64>>()
+            );
+        }
+    }
+}
