From 5c5afafc0dd7f499c8855634db53f8ae664f159d Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 25 Sep 2023 08:05:31 +0300 Subject: [PATCH] Update Deneb to 1.4.0-beta.2 (devnet-9) (#4735) * Add MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT * Update tests to 1.4.0-beta.2 * Implement equivocation check for proposer boost * Use hotfix tests and fix minimal config * Start updating fork choice tests for Deneb * Finish implementing fork choice blob handling --------- Co-authored-by: Michael Sproul --- .../beacon_chain/src/blob_verification.rs | 6 + book/src/api-vc-endpoints.md | 1 + .../chiado/config.yaml | 2 + .../gnosis/config.yaml | 2 + .../holesky/config.yaml | 2 + .../mainnet/config.yaml | 2 + .../prater/config.yaml | 2 + .../sepolia/config.yaml | 2 + consensus/fork_choice/src/fork_choice.rs | 3 +- .../per_epoch_processing/registry_updates.rs | 4 +- consensus/types/src/beacon_state.rs | 18 +++ consensus/types/src/chain_spec.rs | 11 ++ .../environment/tests/testnet_dir/config.yaml | 2 + testing/ef_tests/Makefile | 2 +- testing/ef_tests/src/cases/fork_choice.rs | 138 ++++++++++++++---- 15 files changed, 164 insertions(+), 33 deletions(-) diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index c9388026ba..5f575baf8d 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -157,6 +157,12 @@ impl GossipVerifiedBlob { let blob_index = blob.message.index; validate_blob_sidecar_for_gossip(blob, blob_index, chain) } + /// Construct a `GossipVerifiedBlob` that is assumed to be valid. + /// + /// This should ONLY be used for testing. + pub fn __assumed_valid(blob: SignedBlobSidecar) -> Self { + Self { blob } + } pub fn id(&self) -> BlobIdentifier { self.blob.message.id() } diff --git a/book/src/api-vc-endpoints.md b/book/src/api-vc-endpoints.md index ee0cfd2001..f41625ad88 100644 --- a/book/src/api-vc-endpoints.md +++ b/book/src/api-vc-endpoints.md @@ -243,6 +243,7 @@ Example Response Body "INACTIVITY_SCORE_RECOVERY_RATE": "16", "EJECTION_BALANCE": "16000000000", "MIN_PER_EPOCH_CHURN_LIMIT": "4", + "MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT": "8", "CHURN_LIMIT_QUOTIENT": "65536", "PROPOSER_SCORE_BOOST": "40", "DEPOSIT_CHAIN_ID": "5", diff --git a/common/eth2_network_config/built_in_network_configs/chiado/config.yaml b/common/eth2_network_config/built_in_network_configs/chiado/config.yaml index 47b285a654..fcebaa9bd8 100644 --- a/common/eth2_network_config/built_in_network_configs/chiado/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/chiado/config.yaml @@ -29,6 +29,8 @@ TARGET_COMMITTEE_SIZE: 128 MAX_VALIDATORS_PER_COMMITTEE: 2048 # 2**2 (= 4) MIN_PER_EPOCH_CHURN_LIMIT: 4 +# 2**3 (= 8) +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # 2**12 (= 4096) CHURN_LIMIT_QUOTIENT: 4096 # See issue 563 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 index d8d6a7b969..940fad3615 100644 --- a/common/eth2_network_config/built_in_network_configs/gnosis/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/gnosis/config.yaml @@ -74,6 +74,8 @@ INACTIVITY_SCORE_RECOVERY_RATE: 16 EJECTION_BALANCE: 16000000000 # 2**2 (= 4) MIN_PER_EPOCH_CHURN_LIMIT: 4 +# 2**3 (= 8) +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # 2**12 (= 4096) CHURN_LIMIT_QUOTIENT: 4096 diff --git a/common/eth2_network_config/built_in_network_configs/holesky/config.yaml b/common/eth2_network_config/built_in_network_configs/holesky/config.yaml index a6bfd87ade..40edf9b617 100644 --- a/common/eth2_network_config/built_in_network_configs/holesky/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/holesky/config.yaml @@ -61,6 +61,8 @@ INACTIVITY_SCORE_RECOVERY_RATE: 16 EJECTION_BALANCE: 28000000000 # 2**2 (= 4) MIN_PER_EPOCH_CHURN_LIMIT: 4 +# 2**3 (= 8) +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # 2**16 (= 65,536) CHURN_LIMIT_QUOTIENT: 65536 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 index 0517eb6d82..ed96df2913 100644 --- a/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml @@ -74,6 +74,8 @@ INACTIVITY_SCORE_RECOVERY_RATE: 16 EJECTION_BALANCE: 16000000000 # 2**2 (= 4) MIN_PER_EPOCH_CHURN_LIMIT: 4 +# 2**3 (= 8) +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # 2**16 (= 65,536) CHURN_LIMIT_QUOTIENT: 65536 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 index a0dd85fec0..d82a2c09b8 100644 --- a/common/eth2_network_config/built_in_network_configs/prater/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/prater/config.yaml @@ -70,6 +70,8 @@ INACTIVITY_SCORE_RECOVERY_RATE: 16 EJECTION_BALANCE: 16000000000 # 2**2 (= 4) MIN_PER_EPOCH_CHURN_LIMIT: 4 +# 2**3 (= 8) +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # 2**16 (= 65,536) CHURN_LIMIT_QUOTIENT: 65536 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 index a04d9fa9c9..b3dbb8a115 100644 --- a/common/eth2_network_config/built_in_network_configs/sepolia/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/sepolia/config.yaml @@ -64,6 +64,8 @@ INACTIVITY_SCORE_RECOVERY_RATE: 16 EJECTION_BALANCE: 16000000000 # 2**2 (= 4) MIN_PER_EPOCH_CHURN_LIMIT: 4 +# 2**3 (= 8) +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # 2**16 (= 65,536) CHURN_LIMIT_QUOTIENT: 65536 diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 5e99728845..bdd74c1a2a 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -723,7 +723,8 @@ where // Add proposer score boost if the block is timely. let is_before_attesting_interval = block_delay < Duration::from_secs(spec.seconds_per_slot / INTERVALS_PER_SLOT); - if current_slot == block.slot() && is_before_attesting_interval { + let is_first_block = self.fc_store.proposer_boost_root().is_zero(); + if current_slot == block.slot() && is_before_attesting_interval && is_first_block { self.fc_store.set_proposer_boost_root(block_root); } diff --git a/consensus/state_processing/src/per_epoch_processing/registry_updates.rs b/consensus/state_processing/src/per_epoch_processing/registry_updates.rs index 4fd2d68586..833be41387 100644 --- a/consensus/state_processing/src/per_epoch_processing/registry_updates.rs +++ b/consensus/state_processing/src/per_epoch_processing/registry_updates.rs @@ -50,9 +50,9 @@ pub fn process_registry_updates( .collect_vec(); // Dequeue validators for activation up to churn limit - let churn_limit = state.get_churn_limit(spec)? as usize; + let activation_churn_limit = state.get_activation_churn_limit(spec)? as usize; let delayed_activation_epoch = state.compute_activation_exit_epoch(current_epoch, spec)?; - for index in activation_queue.into_iter().take(churn_limit) { + for index in activation_queue.into_iter().take(activation_churn_limit) { state.get_validator_mut(index)?.activation_epoch = delayed_activation_epoch; } diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 4b187a7517..f0ba15ca89 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -1322,6 +1322,24 @@ impl BeaconState { )) } + /// Return the activation churn limit for the current epoch (number of validators who can enter per epoch). + /// + /// Uses the epoch cache, and will error if it isn't initialized. + /// + /// Spec v1.4.0 + pub fn get_activation_churn_limit(&self, spec: &ChainSpec) -> Result { + Ok(match self { + BeaconState::Base(_) + | BeaconState::Altair(_) + | BeaconState::Merge(_) + | BeaconState::Capella(_) => self.get_churn_limit(spec)?, + BeaconState::Deneb(_) => std::cmp::min( + spec.max_per_epoch_activation_churn_limit, + self.get_churn_limit(spec)?, + ), + }) + } + /// Returns the `slot`, `index`, `committee_position` and `committee_len` for which a validator must produce an /// attestation. /// diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index d94113df7e..0a1dc5c916 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -51,6 +51,7 @@ pub struct ChainSpec { pub max_committees_per_slot: usize, pub target_committee_size: usize, pub min_per_epoch_churn_limit: u64, + pub max_per_epoch_activation_churn_limit: u64, pub churn_limit_quotient: u64, pub shuffle_round_count: u8, pub min_genesis_active_validator_count: u64, @@ -510,6 +511,7 @@ impl ChainSpec { max_committees_per_slot: 64, target_committee_size: 128, min_per_epoch_churn_limit: 4, + max_per_epoch_activation_churn_limit: 8, churn_limit_quotient: 65_536, shuffle_round_count: 90, min_genesis_active_validator_count: 16_384, @@ -686,6 +688,8 @@ impl ChainSpec { config_name: None, max_committees_per_slot: 4, target_committee_size: 4, + min_per_epoch_churn_limit: 2, + max_per_epoch_activation_churn_limit: 4, churn_limit_quotient: 32, shuffle_round_count: 10, min_genesis_active_validator_count: 64, @@ -750,6 +754,7 @@ impl ChainSpec { max_committees_per_slot: 64, target_committee_size: 128, min_per_epoch_churn_limit: 4, + max_per_epoch_activation_churn_limit: 8, churn_limit_quotient: 4_096, shuffle_round_count: 90, min_genesis_active_validator_count: 4_096, @@ -1015,6 +1020,8 @@ pub struct Config { #[serde(with = "serde_utils::quoted_u64")] min_per_epoch_churn_limit: u64, #[serde(with = "serde_utils::quoted_u64")] + max_per_epoch_activation_churn_limit: u64, + #[serde(with = "serde_utils::quoted_u64")] churn_limit_quotient: u64, #[serde(skip_serializing_if = "Option::is_none")] @@ -1227,6 +1234,7 @@ impl Config { ejection_balance: spec.ejection_balance, churn_limit_quotient: spec.churn_limit_quotient, min_per_epoch_churn_limit: spec.min_per_epoch_churn_limit, + max_per_epoch_activation_churn_limit: spec.max_per_epoch_activation_churn_limit, proposer_score_boost: spec.proposer_score_boost.map(|value| MaybeQuoted { value }), @@ -1284,6 +1292,7 @@ impl Config { inactivity_score_recovery_rate, ejection_balance, min_per_epoch_churn_limit, + max_per_epoch_activation_churn_limit, churn_limit_quotient, proposer_score_boost, deposit_chain_id, @@ -1328,6 +1337,7 @@ impl Config { inactivity_score_recovery_rate, ejection_balance, min_per_epoch_churn_limit, + max_per_epoch_activation_churn_limit, churn_limit_quotient, proposer_score_boost: proposer_score_boost.map(|q| q.value), deposit_chain_id, @@ -1583,6 +1593,7 @@ mod yaml_tests { INACTIVITY_SCORE_RECOVERY_RATE: 16 EJECTION_BALANCE: 16000000000 MIN_PER_EPOCH_CHURN_LIMIT: 4 + MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 CHURN_LIMIT_QUOTIENT: 65536 PROPOSER_SCORE_BOOST: 40 DEPOSIT_CHAIN_ID: 1 diff --git a/lighthouse/environment/tests/testnet_dir/config.yaml b/lighthouse/environment/tests/testnet_dir/config.yaml index b98145163c..dbb6819cd8 100644 --- a/lighthouse/environment/tests/testnet_dir/config.yaml +++ b/lighthouse/environment/tests/testnet_dir/config.yaml @@ -67,6 +67,8 @@ INACTIVITY_SCORE_RECOVERY_RATE: 16 EJECTION_BALANCE: 16000000000 # 2**2 (= 4) MIN_PER_EPOCH_CHURN_LIMIT: 4 +# 2**3 (= 8) +MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT: 8 # 2**16 (= 65,536) CHURN_LIMIT_QUOTIENT: 65536 diff --git a/testing/ef_tests/Makefile b/testing/ef_tests/Makefile index a1a71e0b2a..0347fb340a 100644 --- a/testing/ef_tests/Makefile +++ b/testing/ef_tests/Makefile @@ -1,4 +1,4 @@ -TESTS_TAG := v1.4.0-beta.1 +TESTS_TAG := v1.4.0-beta.2-hotfix TESTS = general minimal mainnet TARBALLS = $(patsubst %,%-$(TESTS_TAG).tar.gz,$(TESTS)) diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index f3afb5f7c5..fea728535b 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -6,8 +6,9 @@ use beacon_chain::{ attestation_verification::{ obtain_indexed_attestation_and_committees_per_slot, VerifiedAttestation, }, + blob_verification::GossipVerifiedBlob, test_utils::{BeaconChainHarness, EphemeralHarnessType}, - BeaconChainTypes, CachedHead, ChainConfig, NotifyExecutionLayer, + AvailabilityProcessingStatus, BeaconChainTypes, CachedHead, ChainConfig, NotifyExecutionLayer, }; use execution_layer::{json_structures::JsonPayloadStatusV1Status, PayloadStatusV1}; use serde::Deserialize; @@ -17,9 +18,9 @@ use std::future::Future; use std::sync::Arc; use std::time::Duration; use types::{ - Attestation, AttesterSlashing, BeaconBlock, BeaconState, Checkpoint, EthSpec, - ExecutionBlockHash, ForkName, Hash256, IndexedAttestation, ProgressiveBalancesMode, - SignedBeaconBlock, Slot, Uint256, + Attestation, AttesterSlashing, BeaconBlock, BeaconState, BlobSidecar, BlobsList, Checkpoint, + EthSpec, ExecutionBlockHash, ForkName, Hash256, IndexedAttestation, KzgProof, + ProgressiveBalancesMode, Signature, SignedBeaconBlock, SignedBlobSidecar, Slot, Uint256, }; #[derive(Default, Debug, PartialEq, Clone, Deserialize, Decode)] @@ -71,25 +72,27 @@ impl From for PayloadStatusV1 { #[derive(Debug, Clone, Deserialize)] #[serde(untagged, deny_unknown_fields)] -pub enum Step { +pub enum Step { Tick { tick: u64, }, ValidBlock { - block: B, + block: TBlock, }, MaybeValidBlock { - block: B, + block: TBlock, + blobs: Option, + proofs: Option>, valid: bool, }, Attestation { - attestation: A, + attestation: TAttestation, }, AttesterSlashing { - attester_slashing: AS, + attester_slashing: TAttesterSlashing, }, PowBlock { - pow_block: P, + pow_block: TPowBlock, }, OnPayloadInfo { block_hash: ExecutionBlockHash, @@ -113,7 +116,9 @@ pub struct ForkChoiceTest { pub anchor_state: BeaconState, pub anchor_block: BeaconBlock, #[allow(clippy::type_complexity)] - pub steps: Vec, Attestation, AttesterSlashing, PowBlock>>, + pub steps: Vec< + Step, BlobsList, Attestation, AttesterSlashing, PowBlock>, + >, } impl LoadCase for ForkChoiceTest { @@ -126,7 +131,7 @@ impl LoadCase for ForkChoiceTest { .expect("path must be valid OsStr") .to_string(); let spec = &testing_spec::(fork_name); - let steps: Vec> = + let steps: Vec> = yaml_decode_file(&path.join("steps.yaml"))?; // Resolve the object names in `steps.yaml` into actual decoded block/attestation objects. let steps = steps @@ -139,11 +144,25 @@ impl LoadCase for ForkChoiceTest { }) .map(|block| Step::ValidBlock { block }) } - Step::MaybeValidBlock { block, valid } => { - ssz_decode_file_with(&path.join(format!("{}.ssz_snappy", block)), |bytes| { - SignedBeaconBlock::from_ssz_bytes(bytes, spec) + Step::MaybeValidBlock { + block, + blobs, + proofs, + valid, + } => { + let block = + ssz_decode_file_with(&path.join(format!("{block}.ssz_snappy")), |bytes| { + SignedBeaconBlock::from_ssz_bytes(bytes, spec) + })?; + let blobs = blobs + .map(|blobs| ssz_decode_file(&path.join(format!("{blobs}.ssz_snappy")))) + .transpose()?; + Ok(Step::MaybeValidBlock { + block, + blobs, + proofs, + valid, }) - .map(|block| Step::MaybeValidBlock { block, valid }) } Step::Attestation { attestation } => { ssz_decode_file(&path.join(format!("{}.ssz_snappy", attestation))) @@ -204,10 +223,15 @@ impl Case for ForkChoiceTest { for step in &self.steps { match step { Step::Tick { tick } => tester.set_tick(*tick), - Step::ValidBlock { block } => tester.process_block(block.clone(), true)?, - Step::MaybeValidBlock { block, valid } => { - tester.process_block(block.clone(), *valid)? + Step::ValidBlock { block } => { + tester.process_block(block.clone(), None, None, true)? } + Step::MaybeValidBlock { + block, + blobs, + proofs, + valid, + } => tester.process_block(block.clone(), blobs.clone(), proofs.clone(), *valid)?, Step::Attestation { attestation } => tester.process_attestation(attestation)?, Step::AttesterSlashing { attester_slashing } => { tester.process_attester_slashing(attester_slashing) @@ -380,16 +404,72 @@ impl Tester { .unwrap(); } - pub fn process_block(&self, block: SignedBeaconBlock, valid: bool) -> Result<(), Error> { + pub fn process_block( + &self, + block: SignedBeaconBlock, + blobs: Option>, + kzg_proofs: Option>, + valid: bool, + ) -> Result<(), Error> { let block_root = block.canonical_root(); + + // Convert blobs and kzg_proofs into sidecars, then plumb them into the availability tracker + if let Some(blobs) = blobs.clone() { + let proofs = kzg_proofs.unwrap(); + let commitments = block + .message() + .body() + .blob_kzg_commitments() + .unwrap() + .clone(); + + // Zipping will stop when any of the zipped lists runs out, which is what we want. Some + // of the tests don't provide enough proofs/blobs, and should fail the availability + // check. + for (i, ((blob, kzg_proof), kzg_commitment)) in blobs + .into_iter() + .zip(proofs) + .zip(commitments.into_iter()) + .enumerate() + { + let signed_sidecar = SignedBlobSidecar { + message: Arc::new(BlobSidecar { + block_root, + index: i as u64, + slot: block.slot(), + block_parent_root: block.parent_root(), + proposer_index: block.message().proposer_index(), + blob, + kzg_commitment, + kzg_proof, + }), + signature: Signature::empty(), + _phantom: Default::default(), + }; + let result = self.block_on_dangerous( + self.harness + .chain + .check_gossip_blob_availability_and_import( + GossipVerifiedBlob::__assumed_valid(signed_sidecar), + ), + )?; + if valid { + assert!(result.is_ok()); + } + } + }; + let block = Arc::new(block); - let result = self.block_on_dangerous(self.harness.chain.process_block( - block_root, - block.clone(), - NotifyExecutionLayer::Yes, - || Ok(()), - ))?; - if result.is_ok() != valid { + let result: Result, _> = self + .block_on_dangerous(self.harness.chain.process_block( + block_root, + block.clone(), + NotifyExecutionLayer::Yes, + || Ok(()), + ))? + .map(|avail: AvailabilityProcessingStatus| avail.try_into()); + let success = result.as_ref().map_or(false, |inner| inner.is_ok()); + if success != valid { return Err(Error::DidntFail(format!( "block with root {} was valid={} whilst test expects valid={}. result: {:?}", block_root, @@ -401,8 +481,8 @@ impl Tester { // Apply invalid blocks directly against the fork choice `on_block` function. This ensures // that the block is being rejected by `on_block`, not just some upstream block processing - // function. - if !valid { + // function. When blobs exist, we don't do this. + if !valid && blobs.is_none() { // A missing parent block whilst `valid == false` means the test should pass. if let Some(parent_block) = self .harness