diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 27ede361b8..34d9f980bd 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -7765,10 +7765,10 @@ impl BeaconChain { ) } - pub async fn set_unsatisfied_inclusion_list_block( + pub async fn record_payload_inclusion_list_satisfaction( self: &Arc, - slot: Slot, block_root: Hash256, + satisfied: bool, ) -> Result<(), Error> { let chain = self.clone(); let fork_choice_result = self @@ -7777,9 +7777,9 @@ impl BeaconChain { chain .canonical_head .fork_choice_write_lock() - .on_invalid_inclusion_list_payload(slot, block_root) + .record_payload_inclusion_list_satisfaction(block_root, satisfied) }, - "invalid_inclusion_list_payload", + "record_payload_inclusion_list_satisfaction", ) .await; @@ -7787,7 +7787,7 @@ impl BeaconChain { if let Err(e) = fork_choice_result { crit!( error = ?e, - "Failed to process invalid inclusion list payload" + "Failed to record payload inclusion list satisfaction" ); } diff --git a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs index e5832dd88f..af28375e7a 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -146,7 +146,7 @@ pub struct BeaconForkChoiceStore, Cold: ItemStore< proposer_boost_root: Hash256, equivocating_indices: BTreeSet, inclusion_list_equivocators: HashMap<(Slot, Hash256), BTreeSet>, - unsatisfied_inclusion_list_blocks: HashMap, + payload_inclusion_list_satisfaction: HashMap, _phantom: PhantomData, } @@ -215,7 +215,7 @@ where proposer_boost_root: Hash256::zero(), equivocating_indices: BTreeSet::new(), inclusion_list_equivocators: HashMap::new(), - unsatisfied_inclusion_list_blocks: HashMap::new(), + payload_inclusion_list_satisfaction: HashMap::new(), _phantom: PhantomData, }) } @@ -265,7 +265,7 @@ where proposer_boost_root: persisted.proposer_boost_root, equivocating_indices: persisted.equivocating_indices, inclusion_list_equivocators: HashMap::new(), - unsatisfied_inclusion_list_blocks: HashMap::new(), + payload_inclusion_list_satisfaction: HashMap::new(), _phantom: PhantomData, }) } @@ -385,22 +385,29 @@ where self.equivocating_indices.extend(indices); } - fn set_unsatisfied_inclusion_list_block(&mut self, slot: Slot, block_root: Hash256) { + fn record_payload_inclusion_list_satisfaction( + &mut self, + block_root: Hash256, + satisfied: bool, + ) { info!( - ?slot, %block_root, - "Set unsatisfied inclusion list block" + satisfied, + "Record payload inclusion list satisfaction" ); - self.unsatisfied_inclusion_list_blocks - .insert(slot, block_root); + self.payload_inclusion_list_satisfaction + .insert(block_root, satisfied); } - fn unsatisfied_inclusion_list_block(&self, slot: Slot) -> Option<&Hash256> { - self.unsatisfied_inclusion_list_blocks.get(&slot) + fn is_payload_inclusion_list_satisfied(&self, block_root: &Hash256) -> bool { + self.payload_inclusion_list_satisfaction + .get(block_root) + .copied() + .unwrap_or(false) } - fn unsatisfied_inclusion_list_blocks(&self) -> &HashMap { - &self.unsatisfied_inclusion_list_blocks + fn payload_inclusion_list_satisfaction(&self) -> &HashMap { + &self.payload_inclusion_list_satisfaction } } diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 44d06af283..c731d30811 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -232,9 +232,9 @@ pub async fn notify_new_payload( "Unsatisfied inclusion list" ); chain - .set_unsatisfied_inclusion_list_block( - block.slot(), + .record_payload_inclusion_list_satisfaction( block.tree_hash_root(), + false, ) .await?; } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 4c4b0f84c7..802d7842b3 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -452,7 +452,7 @@ where *fc_store.finalized_checkpoint(), current_epoch_shuffling_id, next_epoch_shuffling_id, - fc_store.unsatisfied_inclusion_list_blocks().clone(), + fc_store.payload_inclusion_list_satisfaction().clone(), execution_status, execution_payload_parent_hash, execution_payload_block_hash, @@ -724,10 +724,20 @@ where .map_err(Error::FailedToProcessInvalidExecutionPayload) } - // TODO(focil) add documentation - pub fn on_invalid_inclusion_list_payload(&mut self, slot: Slot, block_root: Hash256) { + /// Record whether a block root satisfies the inclusion list. + pub fn record_payload_inclusion_list_satisfaction( + &mut self, + block_root: Hash256, + satisfied: bool, + ) { self.fc_store - .set_unsatisfied_inclusion_list_block(slot, block_root); + .record_payload_inclusion_list_satisfaction(block_root, satisfied); + } + + /// Returns `true` if the block root has been recorded as satisfying the inclusion list. + pub fn is_payload_inclusion_list_satisfied(&self, block_root: &Hash256) -> bool { + self.fc_store + .is_payload_inclusion_list_satisfied(block_root) } /// Add `block` to the fork choice DAG. diff --git a/consensus/fork_choice/src/fork_choice_store.rs b/consensus/fork_choice/src/fork_choice_store.rs index 7bbfb2e4de..60db0ad12b 100644 --- a/consensus/fork_choice/src/fork_choice_store.rs +++ b/consensus/fork_choice/src/fork_choice_store.rs @@ -90,12 +90,16 @@ pub trait ForkChoiceStore: Sized { /// Adds to the set of equivocating indices. fn extend_equivocating_indices(&mut self, indices: impl IntoIterator); - /// Returns the `unsatisfied_inclusion_list_blocks` mapping. - fn unsatisfied_inclusion_list_blocks(&self) -> &HashMap; + /// Returns the `payload_inclusion_list_satisfaction` mapping. + fn payload_inclusion_list_satisfaction(&self) -> &HashMap; - /// Returns the `unsatisfied_inclusion_list_block` for the given slot. - fn unsatisfied_inclusion_list_block(&self, slot: Slot) -> Option<&Hash256>; + /// Returns `true` if the block root is in the map AND the value is `true`. + fn is_payload_inclusion_list_satisfied(&self, block_root: &Hash256) -> bool; - /// Sets the `unsatisfied_inclusion_list_block`. - fn set_unsatisfied_inclusion_list_block(&mut self, slot: Slot, block_root: Hash256); + /// Records whether a block root satisfies the inclusion list. + fn record_payload_inclusion_list_satisfaction( + &mut self, + block_root: Hash256, + satisfied: bool, + ); } diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 78a0c0490b..5284b0e8aa 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -360,7 +360,7 @@ pub struct ProtoArray { pub prune_threshold: usize, pub nodes: Vec, pub indices: HashMap, - pub unsatisfied_inclusion_list_blocks: HashMap, + pub payload_inclusion_list_satisfaction: HashMap, pub previous_proposer_boost: ProposerBoost, } @@ -1510,6 +1510,16 @@ impl ProtoArray { proto_node: &ProtoNode, proposer_boost_root: Hash256, ) -> Result { + // If the block's inclusion list satisfaction has been recorded as false, + // do not extend the payload. + if self + .payload_inclusion_list_satisfaction + .get(&fc_node.root) + == Some(&false) + { + return Ok(false); + } + // Per spec: `proposer_root == Root()` is one of the `or` conditions that // makes `should_extend_payload` return True. if proposer_boost_root.is_zero() { @@ -1616,12 +1626,10 @@ impl ProtoArray { return false; } - // TODO(focil) unwrap_or - if node.root() - == *self - .unsatisfied_inclusion_list_blocks - .get(¤t_slot) - .unwrap_or(&Hash256::zero()) + if self + .payload_inclusion_list_satisfaction + .get(&node.root()) + == Some(&false) { info!( ?current_slot, diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 4345115b17..9ab9f8460d 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -505,7 +505,7 @@ impl ProtoArrayForkChoice { finalized_checkpoint: Checkpoint, current_epoch_shuffling_id: AttestationShufflingId, next_epoch_shuffling_id: AttestationShufflingId, - unsatisfied_inclusion_list_blocks: HashMap, + payload_inclusion_list_satisfaction: HashMap, execution_status: ExecutionStatus, execution_payload_parent_hash: Option, execution_payload_block_hash: Option, @@ -516,7 +516,7 @@ impl ProtoArrayForkChoice { prune_threshold: DEFAULT_PRUNE_THRESHOLD, nodes: Vec::with_capacity(1), indices: HashMap::with_capacity(1), - unsatisfied_inclusion_list_blocks, + payload_inclusion_list_satisfaction, previous_proposer_boost: ProposerBoost::default(), }; diff --git a/consensus/proto_array/src/ssz_container.rs b/consensus/proto_array/src/ssz_container.rs index 6cc3becec2..4e4c06cc61 100644 --- a/consensus/proto_array/src/ssz_container.rs +++ b/consensus/proto_array/src/ssz_container.rs @@ -8,7 +8,7 @@ use ssz::{Encode, four_byte_option_impl}; use ssz_derive::{Decode, Encode}; use std::collections::HashMap; use superstruct::superstruct; -use types::{Checkpoint, Hash256, Slot}; +use types::{Checkpoint, Hash256}; // Define a "legacy" implementation of `Option` which uses four bytes for encoding the union // selector. @@ -40,7 +40,7 @@ pub struct SszContainer { pub indices: Vec<(Hash256, usize)>, #[superstruct(only(V28))] pub previous_proposer_boost: ProposerBoost, - pub unsatisfied_inclusion_list_blocks: Vec<(Slot, Hash256)>, + pub payload_inclusion_list_satisfaction: Vec<(Hash256, bool)>, } impl SszContainerV29 { @@ -52,8 +52,8 @@ impl SszContainerV29 { prune_threshold: proto_array.prune_threshold, nodes: proto_array.nodes.clone(), indices: proto_array.indices.iter().map(|(k, v)| (*k, *v)).collect(), - unsatisfied_inclusion_list_blocks: proto_array - .unsatisfied_inclusion_list_blocks + payload_inclusion_list_satisfaction: proto_array + .payload_inclusion_list_satisfaction .iter() .map(|(k, v)| (*k, *v)) .collect(), @@ -70,8 +70,8 @@ impl TryFrom<(SszContainerV29, JustifiedBalances)> for ProtoArrayForkChoice { nodes: from.nodes, indices: from.indices.into_iter().collect::>(), previous_proposer_boost: ProposerBoost::default(), - unsatisfied_inclusion_list_blocks: from - .unsatisfied_inclusion_list_blocks + payload_inclusion_list_satisfaction: from + .payload_inclusion_list_satisfaction .into_iter() .collect::>(), }; @@ -102,7 +102,7 @@ impl From for SszContainerV29 { }) .collect(), indices: v28.indices, - unsatisfied_inclusion_list_blocks: vec![], + payload_inclusion_list_satisfaction: vec![], } } } @@ -128,7 +128,7 @@ impl From for SszContainerV28 { indices: v29.indices, // Proposer boost is not tracked in V29 (computed on-the-fly), so reset it. previous_proposer_boost: ProposerBoost::default(), - unsatisfied_inclusion_list_blocks: v29.unsatisfied_inclusion_list_blocks, + payload_inclusion_list_satisfaction: v29.payload_inclusion_list_satisfaction, } } }