diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index c2ccad7d8c..a6b2f63f21 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1569,6 +1569,7 @@ where beacon_block_root: Hash256, mut state: Cow>, state_root: Hash256, + payload_present_override: Option, ) -> Result, BeaconChainError> { assert_eq!( state.get_latest_block_root(state_root), @@ -1603,12 +1604,17 @@ where *state.get_block_root(target_slot)? }; - let payload_present = state.fork_name_unchecked().gloas_enabled() - && state.latest_block_header().slot != slot - && self - .chain - .canonical_head - .block_has_canonical_payload(&beacon_block_root, &self.spec)?; + let payload_present = match payload_present_override { + Some(payload_present) => payload_present, + None => { + state.fork_name_unchecked().gloas_enabled() + && state.latest_block_header().slot != slot + && self + .chain + .canonical_head + .block_has_canonical_payload(&beacon_block_root, &self.spec)? + } + }; Ok(Attestation::empty_for_signing( index, @@ -1647,7 +1653,11 @@ where state_root, head_block_root, attestation_slot, - MakeAttestationOptions { limit: None, fork }, + MakeAttestationOptions { + limit: None, + fork, + payload_present_override: None, + }, ) .0 } @@ -1674,7 +1684,11 @@ where state_root, head_block_root, attestation_slot, - MakeAttestationOptions { limit: None, fork }, + MakeAttestationOptions { + limit: None, + fork, + payload_present_override: None, + }, ) .0 } @@ -1688,7 +1702,7 @@ where attestation_slot: Slot, opts: MakeAttestationOptions, ) -> (Vec, Vec) { - let MakeAttestationOptions { limit, fork } = opts; + let MakeAttestationOptions { limit, fork, .. } = opts; let committee_count = state.get_committee_count_at_slot(state.slot()).unwrap(); let num_attesters = AtomicUsize::new(0); @@ -1781,7 +1795,11 @@ where attestation_slot: Slot, opts: MakeAttestationOptions, ) -> (Vec>, Vec) { - let MakeAttestationOptions { limit, fork } = opts; + let MakeAttestationOptions { + limit, + fork, + payload_present_override, + } = opts; let committee_count = state.get_committee_count_at_slot(state.slot()).unwrap(); let num_attesters = AtomicUsize::new(0); @@ -1814,6 +1832,7 @@ where head_block_root.into(), Cow::Borrowed(state), state_root, + payload_present_override, ) .unwrap(); @@ -2016,7 +2035,11 @@ where state_root, block_hash, slot, - MakeAttestationOptions { limit, fork }, + MakeAttestationOptions { + limit, + fork, + payload_present_override: None, + }, ) } @@ -3761,6 +3784,8 @@ pub struct MakeAttestationOptions { pub limit: Option, /// Fork to use for signing attestations. pub fork: Fork, + /// Override post-Gloas regular attestation payload-present encoding. + pub payload_present_override: Option, } pub enum NumBlobs { diff --git a/beacon_node/beacon_chain/tests/attestation_verification.rs b/beacon_node/beacon_chain/tests/attestation_verification.rs index da7f380e36..03b8ae58ac 100644 --- a/beacon_node/beacon_chain/tests/attestation_verification.rs +++ b/beacon_node/beacon_chain/tests/attestation_verification.rs @@ -1636,6 +1636,7 @@ async fn attestation_verification_use_head_state_fork() { MakeAttestationOptions { fork: capella_fork, limit: None, + payload_present_override: None, }, ) .0 @@ -1667,6 +1668,7 @@ async fn attestation_verification_use_head_state_fork() { MakeAttestationOptions { fork: bellatrix_fork, limit: None, + payload_present_override: None, }, ) .0 @@ -1741,6 +1743,7 @@ async fn aggregated_attestation_verification_use_head_state_fork() { MakeAttestationOptions { fork: capella_fork, limit: None, + payload_present_override: None, }, ) .0 @@ -1768,6 +1771,7 @@ async fn aggregated_attestation_verification_use_head_state_fork() { MakeAttestationOptions { fork: bellatrix_fork, limit: None, + payload_present_override: None, }, ) .0 diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 533ef61219..f84c77db32 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -8,7 +8,8 @@ use beacon_chain::{ WhenSlotSkipped, custody_context::NodeCustodyType, test_utils::{ - AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType, test_spec, + AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType, + MakeAttestationOptions, test_spec, }, }; use beacon_chain::{ @@ -17,6 +18,7 @@ use beacon_chain::{ }; use bls::{AggregateSignature, Keypair, Signature}; use fixed_bytes::FixedBytesExtended; +use fork_choice::PayloadStatus; use logging::create_test_tracing_subscriber; use slasher::{Config as SlasherConfig, Slasher}; use state_processing::{ @@ -1926,6 +1928,152 @@ async fn add_altair_block_to_base_chain() { )); } +// This is a regression test for the bogus `InvalidBestNode` error which was reachable in Gloas +// networks. Previously Lighthouse would return an `InvalidBestNode` error from `get_head` in +// contradiction to the spec, which states that the justified root should be returned when no leaf +// node is viable. +// +// The chain construction in this test is contrived but not impossible: the justified block's full +// branch is what contained the evidence to justify it, but the empty branch is more weighty and +// wins out. +#[tokio::test] +async fn gloas_get_head_can_return_justified_empty_payload_branch() { + let spec = test_spec::(); + if !spec.fork_name_at_epoch(Epoch::new(0)).gloas_enabled() { + return; + } + + let harness = BeaconChainHarness::builder(MainnetEthSpec) + .spec(spec.clone().into()) + .chain_config(ChainConfig { + archive: true, + ..ChainConfig::default() + }) + .keypairs(KEYPAIRS[0..VALIDATOR_COUNT].to_vec()) + .node_custody_type(NodeCustodyType::Supernode) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + + harness + .extend_slots(E::slots_per_epoch() as usize * 3) + .await; + + let justified_checkpoint = harness.justified_checkpoint(); + assert_ne!(justified_checkpoint.epoch, Epoch::new(0)); + let justified_root = justified_checkpoint.root; + let justified_block = harness + .chain + .get_blinded_block(&justified_root) + .unwrap() + .unwrap(); + let justified_slot = justified_block.message().slot(); + let justified_state_root = justified_block.message().state_root(); + + harness.advance_slot(); + harness + .extend_chain( + E::slots_per_epoch() as usize * 2, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::SomeValidators(vec![]), + ) + .await; + + let current_slot = harness.get_current_slot(); + let current_epoch = current_slot.epoch(E::slots_per_epoch()); + assert_eq!( + harness + .chain + .canonical_head + .cached_head() + .head_payload_status(), + PayloadStatus::Full + ); + + { + let fork_choice = harness.chain.canonical_head.fork_choice_read_lock(); + assert!(fork_choice.is_payload_received(&justified_root)); + let justified_node = fork_choice.get_block(&justified_root).unwrap(); + let voting_source = justified_node + .unrealized_justified_checkpoint + .unwrap_or(justified_node.justified_checkpoint); + assert!( + voting_source.epoch + 2 < current_epoch, + "the justified node's own voting source must be stale" + ); + } + + let mut attestation_state = harness + .chain + .get_state(&justified_state_root, Some(justified_slot), true) + .unwrap() + .unwrap(); + assert!( + attestation_state + .validators() + .iter() + .all(|validator| !validator.slashed), + "reproducer must not rely on slashed validators" + ); + + let all_validators = harness.get_all_validators(); + let mut validators_with_empty_vote = vec![false; VALIDATOR_COUNT]; + let attestation_start_slot = (current_epoch - 1).start_slot(E::slots_per_epoch()); + let attestation_slot = current_slot - 1; + assert_eq!( + attestation_start_slot + E::slots_per_epoch() - 1, + attestation_slot + ); + + // With 32 Mainnet validators, each slot only covers that slot's committee. Use the previous + // epoch so every validator gets a latest vote while every attestation remains gossip-current. + for slot in (attestation_start_slot.as_u64()..current_slot.as_u64()).map(Slot::new) { + while attestation_state.slot() < slot { + per_slot_processing(&mut attestation_state, None, &spec).unwrap(); + } + attestation_state.build_caches(&spec).unwrap(); + let attestation_state_root = attestation_state.update_tree_hash_cache().unwrap(); + assert_eq!( + attestation_state.get_latest_block_root(attestation_state_root), + justified_root + ); + + let fork = spec.fork_at_epoch(slot.epoch(E::slots_per_epoch())); + let (attestations, attesters) = harness.make_attestations_with_opts( + &all_validators, + &attestation_state, + attestation_state_root, + justified_root.into(), + slot, + MakeAttestationOptions { + limit: None, + fork, + payload_present_override: Some(false), + }, + ); + + for validator_index in attesters { + validators_with_empty_vote[validator_index] = true; + } + harness.process_attestations(attestations, &attestation_state); + } + + assert!( + validators_with_empty_vote.iter().all(|attested| *attested), + "all validators should have a latest regular attestation to the justified root" + ); + + let (head_root, payload_status) = harness + .chain + .canonical_head + .fork_choice_write_lock() + .get_head(current_slot, &spec) + .expect("fork choice should return the justified root on the empty payload branch"); + + assert_eq!(head_root, justified_root); + assert_eq!(payload_status, PayloadStatus::Empty); +} + // This is a regression test for this bug: // https://github.com/sigp/lighthouse/issues/4332#issuecomment-1565092279 #[tokio::test]