diff --git a/beacon_node/beacon_chain/src/beacon_block_reward.rs b/beacon_node/beacon_chain/src/beacon_block_reward.rs index 7eaaa3da702..fdd145b4234 100644 --- a/beacon_node/beacon_chain/src/beacon_block_reward.rs +++ b/beacon_node/beacon_chain/src/beacon_block_reward.rs @@ -37,6 +37,27 @@ impl BeaconChain { state.build_committee_cache(RelativeEpoch::Current, &self.spec)?; initialize_epoch_cache(state, &self.spec)?; + // [New in Gloas:EIP7732] Since payload processing is deferred to the next block, the + // state doesn't have the parent's payload availability bit set yet. Set it here (if the + // payload is available) so that the head vote check on attestations matches block processing. + if state.fork_name_unchecked().gloas_enabled() { + let parent_bid = state.latest_execution_payload_bid()?; + let bid_parent_block_hash = block + .body() + .signed_execution_payload_bid()? + .message + .parent_block_hash; + if bid_parent_block_hash == parent_bid.block_hash { + let availability_index = parent_bid + .slot + .as_usize() + .safe_rem(T::EthSpec::slots_per_historical_root())?; + state + .execution_payload_availability_mut()? + .set(availability_index, true)?; + } + } + self.compute_beacon_block_reward_with_cache(block, state) } diff --git a/beacon_node/beacon_chain/src/block_production/gloas.rs b/beacon_node/beacon_chain/src/block_production/gloas.rs index 655db71ea18..fe12c525aef 100644 --- a/beacon_node/beacon_chain/src/block_production/gloas.rs +++ b/beacon_node/beacon_chain/src/block_production/gloas.rs @@ -228,6 +228,7 @@ impl BeaconChain { randao_reveal, graffiti, &parent_execution_requests_ref, + should_build_on_full, ) }, "produce_partial_beacon_block_gloas", @@ -289,6 +290,7 @@ impl BeaconChain { randao_reveal: Signature, graffiti: Graffiti, parent_execution_requests: &ExecutionRequestsGloas, + should_build_on_full: bool, ) -> Result<(PartialBeaconBlock, BeaconState), BlockProductionError> { // It is invalid to try to produce a block using a state from a future slot. @@ -374,6 +376,19 @@ impl BeaconChain { state.build_total_active_balance_cache(&self.spec)?; initialize_epoch_cache(&mut state, &self.spec)?; + // [New in Gloas:EIP7732] Since payload processing is deferred to the next block, the + // state doesn't have the parent's payload availability bit set yet. Set it here (if + // building on the parent's payload) so that attestation packing scores head votes like + // block processing does. + if should_build_on_full { + let parent_slot = state.latest_execution_payload_bid()?.slot; + let availability_index = + parent_slot.as_usize() % T::EthSpec::slots_per_historical_root(); + state + .execution_payload_availability_mut()? + .set(availability_index, true)?; + } + let mut prev_filter_cache = HashMap::new(); let prev_attestation_filter = |att: &CompactAttestationRef| { self.filter_op_pool_attestation(&mut prev_filter_cache, att, &state) diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 195be342846..1e82a5cba69 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -16,6 +16,7 @@ use futures::channel::mpsc::TrySendError; use milhouse::Error as MilhouseError; use operation_pool::OpPoolError; use safe_arith::ArithError; +use ssz::BitfieldError; use ssz_types::Error as SszTypesError; use state_processing::envelope_processing::EnvelopeProcessingError; use state_processing::{ @@ -81,6 +82,7 @@ pub enum BeaconChainError { BlsExecutionChangeValidationError(BlsExecutionChangeValidationError), MissingFinalizedStateRoot(Slot), SszTypesError(SszTypesError), + BitfieldError(BitfieldError), NoProposerForSlot(Slot), CanonicalHeadLockTimeout, AttestationCacheLockTimeout, @@ -269,6 +271,7 @@ easy_from_to!(ProposerSlashingValidationError, BeaconChainError); easy_from_to!(AttesterSlashingValidationError, BeaconChainError); easy_from_to!(BlsExecutionChangeValidationError, BeaconChainError); easy_from_to!(SszTypesError, BeaconChainError); +easy_from_to!(BitfieldError, BeaconChainError); easy_from_to!(OpPoolError, BeaconChainError); easy_from_to!(NaiveAggregationError, BeaconChainError); easy_from_to!(ObservedAttestationsError, BeaconChainError); @@ -298,6 +301,7 @@ pub enum BlockProductionError { EpochCacheError(EpochCacheError), ForkChoiceError(ForkChoiceError), BeaconStateError(BeaconStateError), + BitfieldError(BitfieldError), StateAdvanceError(StateAdvanceError), OpPoolError(OpPoolError), StateSlotTooHigh { @@ -337,6 +341,7 @@ pub enum BlockProductionError { easy_from_to!(BlockProcessingError, BlockProductionError); easy_from_to!(BeaconStateError, BlockProductionError); +easy_from_to!(BitfieldError, BlockProductionError); easy_from_to!(SlotProcessingError, BlockProductionError); easy_from_to!(StateAdvanceError, BlockProductionError); easy_from_to!(ForkChoiceError, BlockProductionError); diff --git a/beacon_node/beacon_chain/src/validator_monitor.rs b/beacon_node/beacon_chain/src/validator_monitor.rs index fb028d0c1f5..ed68eac8540 100644 --- a/beacon_node/beacon_chain/src/validator_monitor.rs +++ b/beacon_node/beacon_chain/src/validator_monitor.rs @@ -728,10 +728,13 @@ impl ValidatorMonitor { let data = unaggregated_attestation.data(); + // Score the attestation as if it were included in the next block. That block + // would build on the canonical block at `data.slot`, which may have been proposed + // in an earlier slot if `data.slot` was skipped. let parent_slot = state - .latest_execution_payload_bid() - .ok() - .map(|bid| bid.slot); + .fork_name_unchecked() + .gloas_enabled() + .then(|| canonical_block_slot(state, data.slot)); // Get the reward indices for the unaggregated attestation or log an error match get_attestation_participation_flag_indices( @@ -2053,6 +2056,22 @@ impl ValidatorMonitor { } } +/// Returns the slot of the canonical block at `slot`, accounting for skipped slots. +fn canonical_block_slot(state: &BeaconState, mut slot: Slot) -> Slot { + let Ok(block_root) = state.get_block_root(slot) else { + return slot; + }; + + while slot > 0 + && state + .get_block_root(slot - 1) + .is_ok_and(|root| root == block_root) + { + slot -= 1; + } + slot +} + fn register_simulated_attestation( data: &AttestationData, head_hit: bool, diff --git a/beacon_node/beacon_chain/tests/attestation_production.rs b/beacon_node/beacon_chain/tests/attestation_production.rs index 48092288da6..6a8b4b4540c 100644 --- a/beacon_node/beacon_chain/tests/attestation_production.rs +++ b/beacon_node/beacon_chain/tests/attestation_production.rs @@ -12,7 +12,7 @@ use bls::{AggregateSignature, Keypair}; use slot_clock::SlotClock; use std::sync::{Arc, LazyLock}; use tree_hash::TreeHash; -use types::{Attestation, EthSpec, MainnetEthSpec, RelativeEpoch, Slot}; +use types::{Attestation, EthSpec, ForkName, MainnetEthSpec, RelativeEpoch, Slot}; pub const VALIDATOR_COUNT: usize = 32; @@ -105,6 +105,153 @@ async fn produces_attestations_from_attestation_simulator_service() { }); } +/// Checks that the attestation simulator reports a head hit for a gloas attestation made on a +/// skipped slot, which votes for the previous block's payload (`data.index == 1`). +#[tokio::test] +async fn gloas_attestation_simulator_head_hit_on_skipped_slot() { + let spec = ForkName::Gloas.make_genesis_spec(MainnetEthSpec::default_spec()); + let harness = BeaconChainHarness::builder(MainnetEthSpec) + .spec(Arc::new(spec)) + .keypairs(KEYPAIRS[..].to_vec()) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + let chain = &harness.chain; + + // Simulated attestations are scored once the chain is `UNAGGREGATED_ATTESTATION_LAG_SLOTS` + // past them, so produce enough blocks after the skipped slot for it to be scored. + let skipped_slot = Slot::new(3); + let last_slot = skipped_slot + UNAGGREGATED_ATTESTATION_LAG_SLOTS as u64 + 2; + for slot in 1..=last_slot.as_u64() { + harness.advance_slot(); + if slot != skipped_slot.as_u64() { + harness + .extend_chain( + 1, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ) + .await; + } + produce_unaggregated_attestation(chain.clone(), chain.slot().unwrap()); + + if slot == skipped_slot.as_u64() { + let validator_monitor = chain.validator_monitor.read(); + let attestation = validator_monitor + .get_unaggregated_attestation(skipped_slot) + .expect("should get unaggregated attestation"); + assert_eq!( + attestation.data().index, + 1, + "the attestation on the skipped slot should vote for the previous block's payload" + ); + } + } + + // Every scored attestation is a head hit, including the one on the skipped slot. + let expected_hits = last_slot.as_u64() - UNAGGREGATED_ATTESTATION_LAG_SLOTS as u64 - 1; + assert!(expected_hits > skipped_slot.as_u64()); + metrics::gather().iter().for_each(|mf| { + if mf.get_name() == metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_HEAD_ATTESTER_HIT_TOTAL + { + assert_eq!( + mf.get_metric()[0].get_counter().get_value() as u64, + expected_hits + ); + } + if mf.get_name() + == metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_HEAD_ATTESTER_MISS_TOTAL + { + assert_eq!(mf.get_metric()[0].get_counter().get_value() as u64, 0); + } + }); +} + +/// Checks that the attestation simulator reports a head hit for a gloas attestation made on a +/// skipped slot when the previous block's payload is unavailable (`data.index == 0`). +#[tokio::test] +async fn gloas_attestation_simulator_head_hit_on_skipped_slot_without_payload() { + let spec = ForkName::Gloas.make_genesis_spec(MainnetEthSpec::default_spec()); + let harness = BeaconChainHarness::builder(MainnetEthSpec) + .spec(Arc::new(spec)) + .keypairs(KEYPAIRS[..].to_vec()) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + let chain = &harness.chain; + + // Build slots 1 and 2 normally, importing their payload envelopes. + harness.advance_slot(); + harness + .extend_chain( + 2, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ) + .await; + + // Import the block at slot 3 without its payload envelope. + harness.advance_slot(); + let payload_unavailable_slot = Slot::new(3); + let (block_contents, _envelope, _new_state) = harness + .make_block_with_envelope(harness.get_current_state(), payload_unavailable_slot) + .await; + let block_root = block_contents.0.canonical_root(); + harness + .process_block(payload_unavailable_slot, block_root, block_contents) + .await + .expect("block should import without envelope"); + assert!( + !chain + .canonical_head + .fork_choice_read_lock() + .get_block(&block_root) + .expect("block should be in fork choice") + .payload_received, + "the head block's payload should be unavailable" + ); + + // Skip slot 4 and simulate an attestation to the payload-unavailable head block. + harness.advance_slot(); + let skipped_slot = Slot::new(4); + produce_unaggregated_attestation(chain.clone(), skipped_slot); + { + let validator_monitor = chain.validator_monitor.read(); + let attestation = validator_monitor + .get_unaggregated_attestation(skipped_slot) + .expect("should get unaggregated attestation"); + assert_eq!( + attestation.data().index, + 0, + "the attestation on the skipped slot should vote that the previous block's payload is unavailable" + ); + } + + // Advance far enough for the simulated attestation to be scored. + for _ in 0..=UNAGGREGATED_ATTESTATION_LAG_SLOTS { + harness.advance_slot(); + harness + .extend_chain( + 1, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ) + .await; + } + + metrics::gather().iter().for_each(|mf| { + if mf.get_name() == metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_HEAD_ATTESTER_HIT_TOTAL + { + assert_eq!(mf.get_metric()[0].get_counter().get_value() as u64, 1); + } + if mf.get_name() + == metrics::VALIDATOR_MONITOR_ATTESTATION_SIMULATOR_HEAD_ATTESTER_MISS_TOTAL + { + assert_eq!(mf.get_metric()[0].get_counter().get_value() as u64, 0); + } + }); +} + /// This test builds a chain that is just long enough to finalize an epoch then it produces an /// attestation at each slot from genesis through to three epochs past the head. /// diff --git a/beacon_node/beacon_chain/tests/rewards.rs b/beacon_node/beacon_chain/tests/rewards.rs index 0c8815995e3..27e52725058 100644 --- a/beacon_node/beacon_chain/tests/rewards.rs +++ b/beacon_node/beacon_chain/tests/rewards.rs @@ -714,6 +714,91 @@ async fn test_rewards_electra() { assert_eq!(expected_balances, balances); } +/// Checks that the computed block reward matches the proposer's actual balance change when the +/// block includes attestations from a skipped slot that vote for an available payload +/// (`data.index == 1`). +#[tokio::test] +async fn test_rewards_gloas_non_same_slot_attestations() { + let spec = ForkName::Gloas.make_genesis_spec(E::default_spec()); + let harness = get_harness(spec.clone()); + + harness.extend_slots(2).await; + + let head = harness.chain.head_snapshot(); + let head_root = head.beacon_block_root; + let head_slot = head.beacon_block.slot(); + + // Skip a slot and attest to the head. These attestations are not same-slot, and the + // head's payload is available, so `data.index == 1`. + harness.advance_slot(); + let attestation_slot = head_slot + 1; + let attestations = harness.make_attestations( + &harness.get_all_validators(), + &head.beacon_state, + head.beacon_state_root(), + head_root.into(), + attestation_slot, + ); + assert!( + attestations + .iter() + .flat_map(|(unaggregated, _)| unaggregated.iter()) + .all(|(attestation, _)| attestation.data().index == 1), + "attestations for the skipped slot should vote for an available payload" + ); + harness.process_attestations(attestations, &head.beacon_state); + + // Produce the next block, which includes those attestations. + harness.advance_slot(); + let block_slot = head_slot + 2; + let ((signed_block, _), mut pre_state) = harness + .make_block_return_pre_state(harness.get_current_state(), block_slot) + .await; + assert!( + signed_block + .message() + .body() + .attestations() + .any(|attestation| attestation.data().index == 1), + "the block should include attestations with data.index == 1" + ); + + let proposer_index = signed_block.message().proposer_index(); + let balance_before = *pre_state.balances().get(proposer_index as usize).unwrap(); + + let block_reward = harness + .chain + .compute_beacon_block_reward(signed_block.message(), &mut pre_state) + .unwrap(); + let proposer_sync_reward: i64 = harness + .chain + .compute_sync_committee_rewards(signed_block.message(), &mut pre_state) + .unwrap() + .iter() + .filter(|reward| reward.validator_index == proposer_index) + .map(|reward| reward.reward) + .sum(); + + harness + .process_block( + block_slot, + signed_block.canonical_root(), + (signed_block.clone(), None), + ) + .await + .unwrap(); + + let balance_after = *harness + .get_current_state() + .balances() + .get(proposer_index as usize) + .unwrap(); + assert_eq!( + balance_after as i64 - balance_before as i64, + block_reward.total as i64 + proposer_sync_reward + ); +} + #[tokio::test] async fn test_rewards_base_subset_only() { let spec = ForkName::Base.make_genesis_spec(E::default_spec()); diff --git a/beacon_node/beacon_chain/tests/tests.rs b/beacon_node/beacon_chain/tests/tests.rs index 3958ce6c6df..4e2eb032f0e 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -6,14 +6,15 @@ use beacon_chain::{ custody_context::NodeCustodyType, test_utils::{ AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType, - OP_POOL_DB_KEY, + MakeAttestationOptions, OP_POOL_DB_KEY, }, }; use bls::Keypair; use operation_pool::PersistedOperationPool; use state_processing::EpochProcessingError; +use state_processing::common::get_attesting_indices_from_state; use state_processing::{per_slot_processing, per_slot_processing::Error as SlotProcessingError}; -use std::sync::LazyLock; +use std::sync::{Arc, LazyLock}; use types::{ BeaconState, BeaconStateError, BlockImportSource, ChainSpec, Checkpoint, DEFAULT_PRE_ELECTRA_WS_PERIOD, EthSpec, ForkName, Hash256, MainnetEthSpec, MinimalEthSpec, @@ -526,6 +527,105 @@ async fn does_not_finalize_without_attestation() { ); } +/// Checks that block production ranks attestations that vote for an available parent payload +/// (`data.index == 1`) above ones that vote for the same block without it, when the op pool +/// holds more attestations than fit in a block. +#[tokio::test] +async fn gloas_packs_attestations_voting_for_available_payload() { + let spec = ForkName::Gloas.make_genesis_spec(E::default_spec()); + let harness = BeaconChainHarness::builder(E::default()) + .spec(Arc::new(spec)) + .keypairs(KEYPAIRS.to_vec()) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + harness.advance_slot(); + harness.extend_slots(1).await; + + let head = harness.chain.head_snapshot(); + let head_root = head.beacon_block_root; + let head_slot = head.beacon_block.slot(); + assert_eq!(head_slot, Slot::new(1)); + let validators = harness.get_all_validators(); + + let insert_attestations = + |attesting_validators: &[usize], slot: Slot, payload_present_override: Option| { + let fork = harness.spec.fork_at_epoch(slot.epoch(E::slots_per_epoch())); + let (attestations, _) = harness.make_attestations_with_opts( + attesting_validators, + &head.beacon_state, + head.beacon_state_root(), + head_root.into(), + slot, + MakeAttestationOptions { + limit: None, + fork, + payload_present_override, + }, + ); + for (attestation, _) in attestations + .into_iter() + .flat_map(|(committee_attestations, _)| committee_attestations) + { + let attesting_indices = + get_attesting_indices_from_state(&head.beacon_state, attestation.to_ref()) + .unwrap(); + harness + .chain + .op_pool + .insert_attestation(attestation, attesting_indices) + .unwrap(); + } + }; + + // Skip the rest of the epoch, attesting to the head at every skipped slot. Together with + // the head's own attestations, this fills a block's attestation limit with attestations + // that earn the target flag, but not the head flag. + let last_skipped_slot = Slot::new(E::slots_per_epoch() - 1); + for slot in (head_slot + 1).as_u64()..=last_skipped_slot.as_u64() { + harness.advance_slot(); + insert_attestations(&validators, Slot::new(slot), None); + } + + // At the next slot, half of the committee votes for the head's payload (`index == 1`) and + // the other half does not (`index == 0`). Both are non same-slot votes for the head. Only + // the first earns the head flag, so it must outrank the target-only attestations, and the + // second must not. + harness.advance_slot(); + let contested_slot = last_skipped_slot + 1; + let mut state = head.beacon_state.clone(); + state + .build_committee_cache(RelativeEpoch::Next, &harness.spec) + .unwrap(); + let committees = state.get_beacon_committees_at_slot(contested_slot).unwrap(); + assert_eq!(committees.len(), 1); + let committee = committees[0].committee; + let (payload_voters, no_payload_voters) = committee.split_at(committee.len() / 2); + insert_attestations(payload_voters, contested_slot, Some(true)); + insert_attestations(no_payload_voters, contested_slot, Some(false)); + + harness.advance_slot(); + let ((signed_block, _), _) = harness + .make_block(harness.get_current_state(), contested_slot + 1) + .await; + let attestations = signed_block + .message() + .body() + .attestations() + .collect::>(); + assert_eq!(attestations.len(), E::max_attestations_electra()); + let contested_slot_indices = attestations + .iter() + .filter(|attestation| attestation.data().slot == contested_slot) + .map(|attestation| attestation.data().index) + .collect::>(); + assert_eq!( + contested_slot_indices, + vec![1], + "the block should pack only the attestation voting for the available payload" + ); +} + #[tokio::test] async fn roundtrip_operation_pool() { let num_blocks_produced = MinimalEthSpec::slots_per_epoch() * 5;