diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 12258a03dc..ea17e20f02 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -175,9 +175,11 @@ pub enum InvalidAttestation { /// The attestation is attesting to a state that is later than itself. (Viz., attesting to the /// future). AttestsToFutureBlock { block: Slot, attestation: Slot }, - /// A same-slot attestation has a non-zero index, indicating a payload attestation during the - /// same slot as the block. Payload attestations must only arrive in subsequent slots. - PayloadAttestationDuringSameSlot { slot: Slot }, + /// A same-slot attestation has a non-zero index, which is invalid post-GLOAS. + InvalidSameSlotAttestationIndex { slot: Slot }, + /// A payload attestation votes payload_present for a block in the current slot, which is + /// invalid because the payload cannot be known yet. + PayloadPresentDuringSameSlot { slot: Slot }, /// A gossip payload attestation must be for the current slot. PayloadAttestationNotCurrentSlot { attestation_slot: Slot, @@ -269,7 +271,7 @@ impl<'a, E: EthSpec> From> for QueuedAttestation { /// Used for queuing payload attestations (PTC votes) from the current slot. /// Payload attestations have different dequeue timing than regular attestations: -/// non-block payload attestations need an extra slot of delay (slot + 1 < current_slot). +/// gossiped payload attestations need an extra slot of delay (slot + 1 < current_slot). #[derive(Clone, PartialEq, Encode, Decode)] pub struct QueuedPayloadAttestation { slot: Slot, @@ -420,7 +422,8 @@ where } else if let Ok(signed_bid) = anchor_block.message().body().signed_execution_payload_bid() { - // Gloas: hashes come from the execution payload bid. + // Gloas: execution status is irrelevant post-Gloas; payload validation + // is decoupled from beacon blocks. ( ExecutionStatus::irrelevant(), Some(signed_bid.message.parent_block_hash), @@ -990,6 +993,12 @@ where Ok(()) } + pub fn on_execution_payload(&mut self, block_root: Hash256) -> Result<(), Error> { + self.proto_array + .on_execution_payload(block_root) + .map_err(Error::FailedToProcessValidExecutionPayload) + } + /// Update checkpoints in store if necessary fn update_checkpoints( &mut self, @@ -1126,15 +1135,15 @@ where }); } - // Post-GLOAS: same-slot attestations with index != 0 indicate a payload-present vote. - // These must go through `on_payload_attestation`, not `on_attestation`. + // Post-GLOAS: same-slot attestations must have index == 0. Attestations with + // index != 0 during the same slot as the block are invalid. if spec .fork_name_at_slot::(indexed_attestation.data().slot) .gloas_enabled() && indexed_attestation.data().slot == block.slot && indexed_attestation.data().index != 0 { - return Err(InvalidAttestation::PayloadAttestationDuringSameSlot { slot: block.slot }); + return Err(InvalidAttestation::InvalidSameSlotAttestationIndex { slot: block.slot }); } Ok(()) @@ -1175,10 +1184,14 @@ where }); } - if self.fc_store.get_current_slot() == block.slot + // A payload attestation voting payload_present for a block in the current slot is + // invalid: the payload cannot be known yet. This only applies to gossip attestations; + // payload attestations from blocks have already been validated by the block producer. + if !is_from_block + && self.fc_store.get_current_slot() == block.slot && indexed_payload_attestation.data.payload_present { - return Err(InvalidAttestation::PayloadAttestationDuringSameSlot { slot: block.slot }); + return Err(InvalidAttestation::PayloadPresentDuringSameSlot { slot: block.slot }); } Ok(()) @@ -1270,7 +1283,7 @@ where let processing_slot = self.fc_store.get_current_slot(); // Payload attestations from blocks can be applied in the next slot (S+1 for data.slot=S), - // while non-block payload attestations are delayed one extra slot. + // while gossiped payload attestations are delayed one extra slot. let should_process_now = if is_from_block { attestation.data.slot < processing_slot } else { @@ -1720,9 +1733,7 @@ where /// be instantiated again later. pub fn to_persisted(&self) -> PersistedForkChoice { PersistedForkChoice { - proto_array: self - .proto_array() - .as_ssz_container(), + proto_array: self.proto_array().as_ssz_container(), queued_attestations: self.queued_attestations().to_vec(), queued_payload_attestations: self.queued_payload_attestations.clone(), } diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index 68ec79d113..6ec1c8aeba 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -993,7 +993,7 @@ async fn invalid_attestation_payload_during_same_slot() { |result| { assert_invalid_attestation!( result, - InvalidAttestation::PayloadAttestationDuringSameSlot { slot } + InvalidAttestation::InvalidSameSlotAttestationIndex { slot } if slot == Slot::new(1) ) }, diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 4ff91638f8..8451e2dc80 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -28,6 +28,7 @@ pub enum Operation { finalized_checkpoint: Checkpoint, justified_state_balances: Vec, expected_head: Hash256, + current_slot: Slot, }, ProposerBoostFindHead { justified_checkpoint: Checkpoint, @@ -147,6 +148,7 @@ impl ForkChoiceTestDefinition { finalized_checkpoint, justified_state_balances, expected_head, + current_slot, } => { let justified_balances = JustifiedBalances::from_effective_balances(justified_state_balances) @@ -158,7 +160,7 @@ impl ForkChoiceTestDefinition { &justified_balances, Hash256::zero(), &equivocating_indices, - Slot::new(0), + current_slot, &spec, ) .unwrap_or_else(|e| { diff --git a/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs b/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs index 318407f598..59e80dbe66 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs @@ -16,6 +16,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), }); // Add a block with a hash of 2. @@ -55,6 +56,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -95,6 +97,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Add a vote to block 1 @@ -124,6 +127,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -166,6 +170,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -222,6 +227,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -272,6 +278,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -321,6 +328,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Invalidation of 3 should have removed upstream weight. @@ -374,6 +382,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { }, justified_state_balances: balances, expected_head: get_root(1), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -427,6 +436,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), }); // Add a block with a hash of 2. @@ -466,6 +476,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -506,6 +517,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Add a vote to block 1 @@ -535,6 +547,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -577,6 +590,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -633,6 +647,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -696,6 +711,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(3), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -745,6 +761,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { }, justified_state_balances: balances, expected_head: get_root(2), + current_slot: Slot::new(0), }); // Invalidation of 3 should have removed upstream weight. @@ -800,6 +817,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), }); // Add a block with a hash of 2. @@ -839,6 +857,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -879,6 +898,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Add a vote to block 1 @@ -908,6 +928,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { @@ -950,6 +971,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), }); ops.push(Operation::AssertWeight { diff --git a/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs b/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs index 88665a22ad..34a4372e27 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs @@ -10,6 +10,7 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), }); // Build the following tree (stick? lol). @@ -63,6 +64,7 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(3), + current_slot: Slot::new(0), }); // Ensure that with justified epoch 1 we find 3 @@ -83,6 +85,7 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(3), + current_slot: Slot::new(0), }); // Ensure that with justified epoch 2 we find 3 @@ -99,6 +102,7 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(1), justified_state_balances: balances, expected_head: get_root(3), + current_slot: Slot::new(0), }); // END OF TESTS @@ -123,6 +127,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), }); // Build the following tree. @@ -269,6 +274,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Same as above, but with justified epoch 2. ops.push(Operation::FindHead { @@ -279,6 +285,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Same as above, but with justified epoch 3. // @@ -293,6 +300,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Add a vote to 1. @@ -332,6 +340,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Save as above but justified epoch 2. ops.push(Operation::FindHead { @@ -342,6 +351,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Save as above but justified epoch 3. // @@ -356,6 +366,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Add a vote to 2. @@ -395,6 +406,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { @@ -405,6 +417,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Same as above but justified epoch 3. // @@ -419,6 +432,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Ensure that if we start at 1 we find 9 (just: 0, fin: 0). @@ -442,6 +456,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { @@ -452,6 +467,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Same as above but justified epoch 3. // @@ -466,6 +482,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Ensure that if we start at 2 we find 10 (just: 0, fin: 0). @@ -486,6 +503,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { @@ -496,6 +514,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Same as above but justified epoch 3. // @@ -510,6 +529,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: balances, expected_head: get_root(10), + current_slot: Slot::new(0), }); // END OF TESTS diff --git a/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs b/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs index 7579b01636..01f804c9aa 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs @@ -71,6 +71,7 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1], expected_head: get_root(3), + current_slot: Slot::new(0), }); ops.push(Operation::SetPayloadTiebreak { @@ -83,6 +84,7 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1], expected_head: get_root(4), + current_slot: Slot::new(0), }); ForkChoiceTestDefinition { @@ -130,6 +132,7 @@ pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1, 1], expected_head: get_root(1), + current_slot: Slot::new(0), }); ops.push(Operation::AssertPayloadWeights { block_root: get_root(1), @@ -154,6 +157,7 @@ pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1, 1], expected_head: get_root(1), + current_slot: Slot::new(0), }); ops.push(Operation::AssertPayloadWeights { block_root: get_root(1), @@ -187,6 +191,7 @@ pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1, 1, 1], expected_head: get_root(5), + current_slot: Slot::new(0), }); ops.push(Operation::AssertPayloadWeights { block_root: get_root(5), @@ -261,6 +266,7 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1], expected_head: get_root(3), + current_slot: Slot::new(0), }); // Validator 0 votes Empty branch -> head flips to 4. @@ -276,6 +282,7 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1], expected_head: get_root(4), + current_slot: Slot::new(0), }); // Latest-message update back to Full branch -> head returns to 3. @@ -291,6 +298,7 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1], expected_head: get_root(3), + current_slot: Slot::new(0), }); ops.push(Operation::AssertPayloadWeights { block_root: get_root(3), @@ -362,6 +370,7 @@ pub fn get_gloas_weight_priority_over_payload_preference_test_definition() finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1], expected_head: get_root(3), + current_slot: Slot::new(0), }); // Add two Empty votes to make the Empty branch strictly heavier. @@ -384,6 +393,7 @@ pub fn get_gloas_weight_priority_over_payload_preference_test_definition() finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1, 1], expected_head: get_root(4), + current_slot: Slot::new(0), }); ForkChoiceTestDefinition { @@ -452,6 +462,143 @@ pub fn get_gloas_parent_empty_when_child_points_to_grandparent_test_definition() } } +/// Test interleaving of blocks, regular attestations, and late-arriving PTC votes. +/// +/// Exercises the spec's `get_weight` rule: FULL/EMPTY virtual nodes at `current_slot - 1` +/// have weight 0, so payload preference is determined solely by the tiebreaker. +/// +/// genesis → block 1 (Full) → block 3 +/// → block 2 (Empty) → block 4 +/// +/// Timeline: +/// 1. Blocks 1 (Full) and 2 (Empty) arrive at slot 1 +/// 2. Regular attestations arrive (equal weight per branch) +/// 3. Child blocks 3 and 4 arrive at slot 2 +/// 4. PTC votes arrive for genesis (2 Full), making genesis prefer Full by weight +/// 5. At current_slot=1 (genesis is current-1), PTC weights are ignored → tiebreaker decides +/// 6. At current_slot=100 (genesis is old), PTC weights apply → Full branch wins +pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDefinition { + let mut ops = vec![]; + + // Step 1: Two competing blocks at slot 1. + ops.push(Operation::ProcessBlock { + slot: Slot::new(1), + root: get_root(1), + parent_root: get_root(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(0)), + execution_payload_block_hash: Some(get_hash(1)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(1), + root: get_root(2), + parent_root: get_root(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(99)), + execution_payload_block_hash: Some(get_hash(2)), + }); + + // Step 2: Regular attestations arrive, one per branch (equal CL weight). + ops.push(Operation::ProcessAttestation { + validator_index: 0, + block_root: get_root(1), + attestation_slot: Slot::new(1), + }); + ops.push(Operation::ProcessAttestation { + validator_index: 1, + block_root: get_root(2), + attestation_slot: Slot::new(1), + }); + + // Step 3: Child blocks at slot 2. + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(3), + parent_root: get_root(1), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(1)), + execution_payload_block_hash: Some(get_hash(3)), + }); + ops.push(Operation::ProcessBlock { + slot: Slot::new(2), + root: get_root(4), + parent_root: get_root(2), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + execution_payload_parent_hash: Some(get_hash(100)), + execution_payload_block_hash: Some(get_hash(4)), + }); + + // Step 4: PTC votes arrive for genesis, 2 Full votes from fresh validators. + // Vals 0 and 1 can't be reused because they already have votes at slot 1. + // Vals 2 and 3 target genesis; CL weight on genesis doesn't affect branch comparison. + ops.push(Operation::ProcessPayloadAttestation { + validator_index: 2, + block_root: get_root(0), + attestation_slot: Slot::new(1), + payload_present: true, + blob_data_available: false, + }); + ops.push(Operation::ProcessPayloadAttestation { + validator_index: 3, + block_root: get_root(0), + attestation_slot: Slot::new(1), + payload_present: true, + blob_data_available: false, + }); + + // Set tiebreaker to Empty on genesis. + ops.push(Operation::SetPayloadTiebreak { + block_root: get_root(0), + is_timely: false, + is_data_available: false, + }); + + // Step 5: At current_slot=1, genesis (slot 0) is at current_slot-1. + // Per spec, FULL/EMPTY weights are zeroed → tiebreaker decides. + // Tiebreaker is Empty → Empty branch (block 4) wins. + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1, 1, 1, 1], + expected_head: get_root(4), + current_slot: Slot::new(1), + }); + + // Step 6: At current_slot=100, genesis (slot 0) is no longer at current_slot-1. + // FULL/EMPTY weights now apply. Genesis has Full > Empty → prefers Full. + // Full branch (block 3) wins despite Empty tiebreaker. + ops.push(Operation::FindHead { + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + justified_state_balances: vec![1, 1, 1, 1], + expected_head: get_root(3), + current_slot: Slot::new(100), + }); + + // Verify the PTC weights are recorded on genesis. + // full = 2 (PTC votes) + 1 (back-propagated from Full child block 1) = 3 + // empty = 0 (PTC votes) + 1 (back-propagated from Empty child block 2) = 1 + ops.push(Operation::AssertPayloadWeights { + block_root: get_root(0), + expected_full_weight: 3, + expected_empty_weight: 1, + }); + + ForkChoiceTestDefinition { + finalized_block_slot: Slot::new(0), + justified_checkpoint: get_checkpoint(0), + finalized_checkpoint: get_checkpoint(0), + operations: ops, + execution_payload_parent_hash: Some(get_hash(42)), + execution_payload_block_hash: Some(get_hash(0)), + spec: Some(gloas_spec()), + } +} + #[cfg(test)] mod tests { use super::*; @@ -485,4 +632,10 @@ mod tests { let test = get_gloas_parent_empty_when_child_points_to_grandparent_test_definition(); test.run(); } + + #[test] + fn interleaved_attestations() { + let test = get_gloas_interleaved_attestations_test_definition(); + test.run(); + } } diff --git a/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs b/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs index 61e4c1270c..71d4c035ae 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs @@ -18,6 +18,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: Hash256::zero(), + current_slot: Slot::new(0), }, // Add block 2 // @@ -55,6 +56,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }, // Add block 1 // @@ -92,6 +94,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }, // Add block 3 // @@ -133,6 +136,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }, // Add block 4 // @@ -174,6 +178,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(4), + current_slot: Slot::new(0), }, // Add block 5 with a justified epoch of 2 // @@ -216,6 +221,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(5), + current_slot: Slot::new(0), }, // Ensure there is no error when starting from a block that has the // wrong justified epoch. @@ -242,6 +248,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(5), + current_slot: Slot::new(0), }, // Set the justified epoch to 2 and the start block to 5 and ensure 5 is the head. // @@ -260,6 +267,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(5), + current_slot: Slot::new(0), }, // Add block 6 // @@ -303,6 +311,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances, expected_head: get_root(6), + current_slot: Slot::new(0), }, ]; diff --git a/consensus/proto_array/src/fork_choice_test_definition/votes.rs b/consensus/proto_array/src/fork_choice_test_definition/votes.rs index ad45d073c2..3ba21db48a 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/votes.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/votes.rs @@ -16,6 +16,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(0), + current_slot: Slot::new(0), }); // Add a block with a hash of 2. @@ -55,6 +56,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -95,6 +97,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Add a vote to block 1 @@ -124,6 +127,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(1), + current_slot: Slot::new(0), }); // Add a vote to block 2 @@ -153,6 +157,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Add block 3. @@ -196,6 +201,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Move validator #0 vote from 1 to 3 @@ -229,6 +235,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(2), + current_slot: Slot::new(0), }); // Move validator #1 vote from 2 to 1 (this is an equivocation, but fork choice doesn't @@ -263,6 +270,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(3), + current_slot: Slot::new(0), }); // Add block 4. @@ -310,6 +318,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(4), + current_slot: Slot::new(0), }); // Add block 5, which has a justified epoch of 2. @@ -361,6 +370,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(4), + current_slot: Slot::new(0), }); // Add block 6, which has a justified epoch of 0. @@ -505,6 +515,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(6), + current_slot: Slot::new(0), }); // Change fork-choice justified epoch to 1, and the start block to 5 and ensure that 9 is @@ -538,6 +549,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Change fork-choice justified epoch to 1, and the start block to 5 and ensure that 9 is @@ -616,6 +628,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Introduce 2 more validators into the system @@ -677,6 +690,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Set the balances of the last two validators to zero @@ -702,6 +716,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Set the balances of the last two validators back to 1 @@ -727,6 +742,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(10), + current_slot: Slot::new(0), }); // Remove the last two validators @@ -753,6 +769,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Ensure that pruning below the prune threshold does not prune. @@ -774,6 +791,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Ensure that pruning above the prune threshold does prune. @@ -812,6 +830,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances.clone(), expected_head: get_root(9), + current_slot: Slot::new(0), }); // Add block 11 @@ -863,6 +882,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { }, justified_state_balances: balances, expected_head: get_root(11), + current_slot: Slot::new(0), }); ForkChoiceTestDefinition { diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index f062fef418..d0806b9e31 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -293,35 +293,38 @@ impl ProtoArray { false }; - let node_deltas = deltas + let node_delta = deltas .get(node_index) .copied() .ok_or(Error::InvalidNodeDelta(node_index))?; - let mut node_delta = if execution_status_is_invalid { + let mut delta = if execution_status_is_invalid { // If the node has an invalid execution payload, reduce its weight to zero. 0_i64 .checked_sub(node.weight() as i64) .ok_or(Error::InvalidExecutionDeltaOverflow(node_index))? } else { - node_deltas.delta + node_delta.delta }; let (node_empty_delta, node_full_delta) = if node.as_v29().is_ok() { - (node_deltas.empty_delta, node_deltas.full_delta) + (node_delta.empty_delta, node_delta.full_delta) } else { (0, 0) }; // If we find the node for which the proposer boost was previously applied, decrease // the delta by the previous score amount. + // TODO(gloas): implement `should_apply_proposer_boost` from the Gloas spec. + // The spec conditionally applies proposer boost based on parent weakness and + // early equivocations. Currently boost is applied unconditionally. if self.previous_proposer_boost.root != Hash256::zero() && self.previous_proposer_boost.root == node.root() // Invalid nodes will always have a weight of zero so there's no need to subtract // the proposer boost delta. && !execution_status_is_invalid { - node_delta = node_delta + delta = delta .checked_sub(self.previous_proposer_boost.score as i64) .ok_or(Error::DeltaOverflow(node_index))?; } @@ -329,6 +332,10 @@ impl ProtoArray { // the delta by the new score amount (unless the block has an invalid execution status). // // https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#get_latest_attesting_balance + // + // TODO(gloas): proposer boost should also be subtracted from `empty_delta` per spec, + // since the spec creates a virtual vote with `payload_present=False` for the proposer + // boost, biasing toward Empty for non-current-slot payload decisions. if let Some(proposer_score_boost) = spec.proposer_score_boost && proposer_boost_root != Hash256::zero() && proposer_boost_root == node.root() @@ -338,7 +345,7 @@ impl ProtoArray { proposer_score = calculate_committee_fraction::(new_justified_balances, proposer_score_boost) .ok_or(Error::ProposerBoostOverflow(node_index))?; - node_delta = node_delta + delta = delta .checked_add(proposer_score as i64) .ok_or(Error::DeltaOverflow(node_index))?; } @@ -347,7 +354,7 @@ impl ProtoArray { if execution_status_is_invalid { *node.weight_mut() = 0; } else { - *node.weight_mut() = apply_delta(node.weight(), node_delta, node_index)?; + *node.weight_mut() = apply_delta(node.weight(), delta, node_index)?; } // Apply post-Gloas score deltas. @@ -356,7 +363,7 @@ impl ProtoArray { apply_delta(node.empty_payload_weight, node_empty_delta, node_index)?; node.full_payload_weight = apply_delta(node.full_payload_weight, node_full_delta, node_index)?; - if let Some(payload_tiebreaker) = node_deltas.payload_tiebreaker { + if let Some(payload_tiebreaker) = node_delta.payload_tiebreaker { node.payload_tiebreak = payload_tiebreaker; } } @@ -370,7 +377,7 @@ impl ProtoArray { // Back-propagate the node's delta to its parent. parent_delta.delta = parent_delta .delta - .checked_add(node_delta) + .checked_add(delta) .ok_or(Error::DeltaOverflow(parent_index))?; // Per spec's `is_supporting_vote`: a vote for descendant B supports @@ -381,13 +388,13 @@ impl ProtoArray { Ok(PayloadStatus::Full) => { parent_delta.full_delta = parent_delta .full_delta - .checked_add(node_delta) + .checked_add(delta) .ok_or(Error::DeltaOverflow(parent_index))?; } Ok(PayloadStatus::Empty) => { parent_delta.empty_delta = parent_delta .empty_delta - .checked_add(node_delta) + .checked_add(delta) .ok_or(Error::DeltaOverflow(parent_index))?; } // Pending or V17 nodes: no payload propagation. @@ -488,6 +495,8 @@ impl ProtoArray { { // Get the parent's execution block hash, handling both V17 and V29 nodes. // V17 parents occur during the Gloas fork transition. + // TODO(gloas): the spec's `get_parent_payload_status` assumes all blocks are + // post-Gloas with bids. Revisit once the spec clarifies fork-transition behavior. let parent_el_block_hash = match parent_node { ProtoNode::V29(v29) => Some(v29.execution_payload_block_hash), ProtoNode::V17(v17) => v17.execution_status.block_hash(), @@ -501,6 +510,9 @@ impl ProtoArray { PayloadStatus::Empty } } else { + // Parent is missing (genesis or pruned due to finalization). Default to Full + // since this path should only be hit at Gloas genesis, and extending the payload + // chain is the safe default. PayloadStatus::Full }; @@ -528,15 +540,16 @@ impl ProtoArray { }; // If the parent has an invalid execution status, return an error before adding the - // block to `self`. This applies when the parent is a V17 node with execution tracking. + // block to `self`. This applies only when the parent is a V17 node with execution tracking. if let Some(parent_index) = node.parent() { let parent = self .nodes .get(parent_index) .ok_or(Error::InvalidNodeIndex(parent_index))?; - if let Ok(status) = parent.execution_status() - && status.is_invalid() + // Execution status tracking only exists on V17 (pre-Gloas) nodes. + if let Ok(v17) = parent.as_v17() + && v17.execution_status.is_invalid() { return Err(Error::ParentExecutionStatusIsInvalid { block_root: block.root, @@ -565,6 +578,29 @@ impl ProtoArray { Ok(()) } + /// Process an excution payload for a Gloas block. + /// + /// this function assumes the + pub fn on_valid_execution_payload(&mut self, block_root: Hash256) -> Result<(), Error> { + let index = *self + .indices + .get(&block_root) + .ok_or(Error::NodeUnknown(block_root))?; + let node = self + .nodes + .get_mut(index) + .ok_or(Error::InvalidNodeIndex(index))?; + let v29 = node + .as_v29_mut() + .map_err(|_| Error::InvalidNodeVariant { block_root })?; + v29.payload_tiebreak = PayloadTiebreak { + is_timely: true, + is_data_available: true, + }; + + Ok(()) + } + /// Updates the `block_root` and all ancestors to have validated execution payloads. /// /// Returns an error if: @@ -871,8 +907,9 @@ impl ProtoArray { // practically possible to set a new justified root if we are unable to find a new head. // // This scenario is *unsupported*. It represents a serious consensus failure. - if let Ok(execution_status) = justified_node.execution_status() - && execution_status.is_invalid() + // Execution status tracking only exists on V17 (pre-Gloas) nodes. + if let Ok(v17) = justified_node.as_v17() + && v17.execution_status.is_invalid() { return Err(Error::InvalidJustifiedCheckpointExecutionStatus { justified_root: *justified_root, @@ -1025,66 +1062,72 @@ impl ProtoArray { ); let no_change = (parent.best_child(), parent.best_descendant()); - let (new_best_child, new_best_descendant) = - if let Some(best_child_index) = parent.best_child() { - if best_child_index == child_index && !child_leads_to_viable_head { - // If the child is already the best-child of the parent but it's not viable for - // the head, remove it. - change_to_none - } else if best_child_index == child_index { - // If the child is the best-child already, set it again to ensure that the - // best-descendant of the parent is updated. - change_to_child - } else { - let best_child = self - .nodes - .get(best_child_index) - .ok_or(Error::InvalidBestDescendant(best_child_index))?; - - let best_child_leads_to_viable_head = self.node_leads_to_viable_head::( - best_child, - current_slot, - best_justified_checkpoint, - best_finalized_checkpoint, - )?; - - if child_leads_to_viable_head && !best_child_leads_to_viable_head { - // The child leads to a viable head, but the current best-child doesn't. - change_to_child - } else if !child_leads_to_viable_head && best_child_leads_to_viable_head { - // The best child leads to a viable head, but the child doesn't. - no_change - } else if child.weight() > best_child.weight() { - // Weight is the primary ordering criterion. - change_to_child - } else if child.weight() < best_child.weight() { - no_change - } else { - // Equal weights: for V29 parents, prefer the child whose - // parent_payload_status matches the parent's payload preference. - let child_matches = child_matches_parent_payload_preference(parent, child); - let best_child_matches = - child_matches_parent_payload_preference(parent, best_child); - - if child_matches && !best_child_matches { - change_to_child - } else if !child_matches && best_child_matches { - no_change - } else if *child.root() >= *best_child.root() { - // Final tie-breaker of equal weights by root. - change_to_child - } else { - no_change - } - } - } - } else if child_leads_to_viable_head { - // There is no current best-child and the child is viable. + let (new_best_child, new_best_descendant) = if let Some(best_child_index) = + parent.best_child() + { + if best_child_index == child_index && !child_leads_to_viable_head { + // If the child is already the best-child of the parent but it's not viable for + // the head, remove it. + change_to_none + } else if best_child_index == child_index { + // If the child is the best-child already, set it again to ensure that the + // best-descendant of the parent is updated. change_to_child } else { - // There is no current best-child but the child is not viable. - no_change - }; + let best_child = self + .nodes + .get(best_child_index) + .ok_or(Error::InvalidBestDescendant(best_child_index))?; + + let best_child_leads_to_viable_head = self.node_leads_to_viable_head::( + best_child, + current_slot, + best_justified_checkpoint, + best_finalized_checkpoint, + )?; + + if child_leads_to_viable_head && !best_child_leads_to_viable_head { + // The child leads to a viable head, but the current best-child doesn't. + change_to_child + } else if !child_leads_to_viable_head && best_child_leads_to_viable_head { + // The best child leads to a viable head, but the child doesn't. + no_change + } else if child.weight() > best_child.weight() { + // Weight is the primary ordering criterion. + change_to_child + } else if child.weight() < best_child.weight() { + no_change + } else { + // Equal weights: for V29 parents, prefer the child whose + // parent_payload_status matches the parent's payload preference + // (full vs empty). This corresponds to the spec's + // `get_payload_status_tiebreaker` ordering in `get_head`. + let child_matches = + child_matches_parent_payload_preference(parent, child, current_slot); + let best_child_matches = + child_matches_parent_payload_preference(parent, best_child, current_slot); + + if child_matches && !best_child_matches { + // Child extends the preferred payload chain, best_child doesn't. + change_to_child + } else if !child_matches && best_child_matches { + // Best child extends the preferred payload chain, child doesn't. + no_change + } else if *child.root() >= *best_child.root() { + // Final tie-breaker: both match or both don't, break by root. + change_to_child + } else { + no_change + } + } + } + } else if child_leads_to_viable_head { + // There is no current best-child and the child is viable. + change_to_child + } else { + // There is no current best-child but the child is not viable. + no_change + }; let parent = self .nodes @@ -1338,16 +1381,35 @@ impl ProtoArray { /// When equal, the tiebreaker uses the parent's `payload_tiebreak`: prefer Full if the block /// was timely and data is available; otherwise prefer Empty. /// For V17 parents (or mixed), always returns `true` (no payload preference). -fn child_matches_parent_payload_preference(parent: &ProtoNode, child: &ProtoNode) -> bool { +/// +/// TODO(gloas): the spec's `should_extend_payload` has additional conditions beyond the +/// tiebreaker: it also checks proposer_boost_root (empty, different parent, or extends full). +/// See: https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md#new-should_extend_payload +/// +/// TODO(gloas): the spec's `should_extend_payload` has additional conditions beyond the +/// tiebreaker: it also checks proposer_boost_root (empty, different parent, or extends full). +/// See: https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md#new-should_extend_payload +fn child_matches_parent_payload_preference( + parent: &ProtoNode, + child: &ProtoNode, + current_slot: Slot, +) -> bool { let (Ok(parent_v29), Ok(child_v29)) = (parent.as_v29(), child.as_v29()) else { return true; }; - let prefers_full = if parent_v29.full_payload_weight > parent_v29.empty_payload_weight { + // Per spec `get_weight`: FULL/EMPTY virtual nodes at `current_slot - 1` have weight 0. + // The PTC is still voting, so payload preference is determined solely by the tiebreaker. + let use_tiebreaker_only = parent.slot() + 1 == current_slot; + let prefers_full = if !use_tiebreaker_only + && parent_v29.full_payload_weight > parent_v29.empty_payload_weight + { true - } else if parent_v29.empty_payload_weight > parent_v29.full_payload_weight { + } else if !use_tiebreaker_only + && parent_v29.empty_payload_weight > parent_v29.full_payload_weight + { false } else { - // Equal weights: tiebreaker per spec + // Equal weights (or current-slot parent): tiebreaker per spec. parent_v29.payload_tiebreak.is_timely && parent_v29.payload_tiebreak.is_data_available }; if prefers_full { diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 15062367bf..66f3627483 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -498,6 +498,11 @@ impl ProtoArrayForkChoice { }) } + pub fn on_execution_payload(&mut self, block_root: Hash256) -> Result<(), String> { + self.proto_array + .on_valid_execution_payload(block_root) + .map_err(|e| format!("Failed to process execution payload: {:?}", e)) + } /// See `ProtoArray::propagate_execution_payload_validation` for documentation. pub fn process_execution_payload_validation( &mut self, @@ -718,7 +723,7 @@ impl ProtoArrayForkChoice { let parent_slot = parent_node.slot(); let head_slot = head_node.slot(); - let re_org_block_slot = head_slot + 1; + let re_org_block_slot = head_slot.saturating_add(1_u64); // Check finalization distance. let proposal_epoch = re_org_block_slot.epoch(E::slots_per_epoch()); @@ -1035,17 +1040,12 @@ impl ProtoArrayForkChoice { self.proto_array.iter_block_roots(block_root) } - pub fn as_ssz_container( - &self, - ) -> SszContainer { + pub fn as_ssz_container(&self) -> SszContainer { SszContainer::from_proto_array(self) } - pub fn as_bytes( - &self, - ) -> Vec { - self.as_ssz_container() - .as_ssz_bytes() + pub fn as_bytes(&self) -> Vec { + self.as_ssz_container().as_ssz_bytes() } pub fn from_bytes(bytes: &[u8], balances: JustifiedBalances) -> Result { @@ -1321,8 +1321,8 @@ mod test_compute_deltas { next_epoch_shuffling_id: junk_shuffling_id.clone(), justified_checkpoint: genesis_checkpoint, finalized_checkpoint: genesis_checkpoint, - execution_status, unrealized_justified_checkpoint: Some(genesis_checkpoint), + execution_status, unrealized_finalized_checkpoint: Some(genesis_checkpoint), execution_payload_parent_hash: None, execution_payload_block_hash: None, diff --git a/consensus/proto_array/src/ssz_container.rs b/consensus/proto_array/src/ssz_container.rs index 02c3e33345..664dfe3ceb 100644 --- a/consensus/proto_array/src/ssz_container.rs +++ b/consensus/proto_array/src/ssz_container.rs @@ -55,9 +55,7 @@ pub struct SszContainerV29 { } impl SszContainerV29 { - pub fn from_proto_array( - from: &ProtoArrayForkChoice, - ) -> Self { + pub fn from_proto_array(from: &ProtoArrayForkChoice) -> Self { let proto_array = &from.proto_array; Self {