diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 500f718e8a..81f09e0547 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -7267,6 +7267,32 @@ impl BeaconChain { ) } + pub async fn set_unsatisfied_inclusion_list_block(self: &Arc, block_root: Hash256) -> Result<(), Error> { + let chain = self.clone(); + let fork_choice_result = self + .spawn_blocking_handle( + move || { + chain + .canonical_head + .fork_choice_write_lock() + .on_invalid_inclusion_list_payload(block_root) + }, + "invalid_inclusion_list_payload", + ) + .await; + + // Update fork choice. + if let Err(e) = fork_choice_result { + crit!( + self.log, + "Failed to process invalid inclusion list payload"; + "error" => ?e, + ); + } + + Ok(()) + } + pub fn metrics(&self) -> BeaconChainMetrics { BeaconChainMetrics { reqresp_pre_import_cache_len: self.reqresp_pre_import_cache.read().len(), 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 318de20c01..2fc35f1b58 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -141,6 +141,7 @@ pub struct BeaconForkChoiceStore, Cold: ItemStore< proposer_boost_root: Hash256, equivocating_indices: BTreeSet, inclusion_list_equivocators: HashMap<(Slot, Hash256), BTreeSet>, + unsatisfied_inclusion_list_block: Hash256, _phantom: PhantomData, } @@ -191,6 +192,7 @@ where proposer_boost_root: Hash256::zero(), equivocating_indices: BTreeSet::new(), inclusion_list_equivocators: HashMap::new(), + unsatisfied_inclusion_list_block: Hash256::zero(), _phantom: PhantomData, }) } @@ -208,6 +210,7 @@ where unrealized_finalized_checkpoint: self.unrealized_finalized_checkpoint, proposer_boost_root: self.proposer_boost_root, equivocating_indices: self.equivocating_indices.clone(), + unsatisfied_inclusion_list_block: self.unsatisfied_inclusion_list_block, } } @@ -230,6 +233,7 @@ where proposer_boost_root: persisted.proposer_boost_root, equivocating_indices: persisted.equivocating_indices, inclusion_list_equivocators: HashMap::new(), + unsatisfied_inclusion_list_block: persisted.unsatisfied_inclusion_list_block, _phantom: PhantomData, }) } @@ -347,6 +351,14 @@ where fn extend_equivocating_indices(&mut self, indices: impl IntoIterator) { self.equivocating_indices.extend(indices); } + + fn set_unsatisfied_inclusion_list_block(&mut self, block_root: Hash256) { + self.unsatisfied_inclusion_list_block = block_root; + } + + fn unsatisfied_inclusion_list_block(&self) -> Hash256 { + self.unsatisfied_inclusion_list_block + } } pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV17; @@ -363,4 +375,5 @@ pub struct PersistedForkChoiceStore { pub unrealized_finalized_checkpoint: Checkpoint, pub proposer_boost_root: Hash256, pub equivocating_indices: BTreeSet, + pub unsatisfied_inclusion_list_block: Hash256, } diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index e5e49d4472..28c55f97e8 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -200,6 +200,13 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>( // imported to fork choice was the parent. let latest_root = block.parent_root(); + // If the payload is invalid because it didn't satisfy the inclusion lit + // transactions for this slot, update the fork choice store before processing + // the invalid EL payload. + if *validation_error == Some("INVALID_INCLUSION_LIST".to_string()) { + chain.set_unsatisfied_inclusion_list_block(block.tree_hash_root()).await?; + } + chain .process_invalid_execution_payload(&InvalidationOperation::InvalidateMany { head_block_root: latest_root, diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 85704042df..eeba8fc095 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -489,6 +489,7 @@ where store.justified_balances(), store.proposer_boost_root(), store.equivocating_indices(), + store.unsatisfied_inclusion_list_block(), current_slot, spec, )?; @@ -626,6 +627,14 @@ where .map_err(Error::FailedToProcessInvalidExecutionPayload) } + // TODO(focil) add documentation + pub fn on_invalid_inclusion_list_payload( + &mut self, + block_root: Hash256, + ) { + self.fc_store.set_unsatisfied_inclusion_list_block(block_root); + } + /// Add `block` to the fork choice DAG. /// /// - `block_root` is the root of `block. diff --git a/consensus/fork_choice/src/fork_choice_store.rs b/consensus/fork_choice/src/fork_choice_store.rs index 27f3d34dbc..79403ee79b 100644 --- a/consensus/fork_choice/src/fork_choice_store.rs +++ b/consensus/fork_choice/src/fork_choice_store.rs @@ -79,4 +79,10 @@ 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_block`. + fn unsatisfied_inclusion_list_block(&self) -> Hash256; + + /// Sets the `unsatisfied_inclusion_list_block`. + fn set_unsatisfied_inclusion_list_block(&mut self, block_root: Hash256); } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index d99ace05f9..dc13d63678 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -110,6 +110,7 @@ impl ForkChoiceTestDefinition { &justified_balances, Hash256::zero(), &equivocating_indices, + Hash256::zero(), Slot::new(0), &spec, ) @@ -141,6 +142,7 @@ impl ForkChoiceTestDefinition { &justified_balances, proposer_boost_root, &equivocating_indices, + Hash256::zero(), Slot::new(0), &spec, ) @@ -169,6 +171,7 @@ impl ForkChoiceTestDefinition { &justified_balances, Hash256::zero(), &equivocating_indices, + Hash256::zero(), Slot::new(0), &spec, ); diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 38ea141199..9364a92738 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -159,6 +159,7 @@ impl ProtoArray { finalized_checkpoint: Checkpoint, new_justified_balances: &JustifiedBalances, proposer_boost_root: Hash256, + unsatisfied_inclusion_list_block: Hash256, current_slot: Slot, spec: &ChainSpec, ) -> Result<(), Error> { @@ -195,17 +196,20 @@ impl ProtoArray { let execution_status_is_invalid = node.execution_status.is_invalid(); - let mut node_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 { - deltas - .get(node_index) - .copied() - .ok_or(Error::InvalidNodeDelta(node_index))? - }; + // TODO(focil) seems sketchy... + let mut node_delta = + if execution_status_is_invalid || node.root == unsatisfied_inclusion_list_block { + // If the node has an invalid execution payload, or the payload doesn't satisfy + // an inclusion list, reduce its weight to zero. + 0_i64 + .checked_sub(node.weight as i64) + .ok_or(Error::InvalidExecutionDeltaOverflow(node_index))? + } else { + deltas + .get(node_index) + .copied() + .ok_or(Error::InvalidNodeDelta(node_index))? + }; // If we find the node for which the proposer boost was previously applied, decrease // the delta by the previous score amount. diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 88d4660311..85b568abce 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -467,6 +467,7 @@ impl ProtoArrayForkChoice { justified_state_balances: &JustifiedBalances, proposer_boost_root: Hash256, equivocating_indices: &BTreeSet, + unsatisfied_inclusion_list_block: Hash256, current_slot: Slot, spec: &ChainSpec, ) -> Result { @@ -489,6 +490,7 @@ impl ProtoArrayForkChoice { finalized_checkpoint, new_balances, proposer_boost_root, + unsatisfied_inclusion_list_block, current_slot, spec, ) diff --git a/consensus/types/src/beacon_state/inclusion_list_cache.rs b/consensus/types/src/beacon_state/inclusion_list_cache.rs index 182189b9f2..fb4fc675f8 100644 --- a/consensus/types/src/beacon_state/inclusion_list_cache.rs +++ b/consensus/types/src/beacon_state/inclusion_list_cache.rs @@ -46,7 +46,11 @@ impl InclusionListCache { return; } - if inner.inclusion_lists_seen.contains(&inclusion_list.message.validator_index) && !inner.inclusion_lists.contains(&inclusion_list) { + if inner + .inclusion_lists_seen + .contains(&inclusion_list.message.validator_index) + && !inner.inclusion_lists.contains(&inclusion_list) + { inner .inclusion_list_equivocators .insert(inclusion_list.message.validator_index); @@ -54,7 +58,11 @@ impl InclusionListCache { } // Skip inserting into the cache if we've already seen an identical IL - if inner.inclusion_lists_seen.contains(&inclusion_list.message.validator_index) && inner.inclusion_lists.contains(&inclusion_list) { + if inner + .inclusion_lists_seen + .contains(&inclusion_list.message.validator_index) + && inner.inclusion_lists.contains(&inclusion_list) + { return; } @@ -63,7 +71,9 @@ impl InclusionListCache { .inclusion_list_transactions .insert(transaction.clone()); } - inner.inclusion_lists_seen.insert(inclusion_list.message.validator_index); + inner + .inclusion_lists_seen + .insert(inclusion_list.message.validator_index); inner.inclusion_lists.insert(inclusion_list); }