Refuse to sign sync committee messages when head is optimistic (#3191)
## Issue Addressed Resolves #3151 ## Proposed Changes When fetching duties for sync committee contributions, check the value of `execution_optimistic` of the head block from the BN and refuse to sign any sync committee messages `if execution_optimistic == true`. ## Additional Info - Is backwards compatible with older BNs - Finding a way to add test coverage for this would be prudent. Open to suggestions.
This commit is contained in:
parent
d316305411
commit
44fae52cd7
@ -1380,10 +1380,41 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
|
|||||||
pub fn get_aggregated_sync_committee_contribution(
|
pub fn get_aggregated_sync_committee_contribution(
|
||||||
&self,
|
&self,
|
||||||
sync_contribution_data: &SyncContributionData,
|
sync_contribution_data: &SyncContributionData,
|
||||||
) -> Option<SyncCommitteeContribution<T::EthSpec>> {
|
) -> Result<Option<SyncCommitteeContribution<T::EthSpec>>, Error> {
|
||||||
self.naive_sync_aggregation_pool
|
if let Some(contribution) = self
|
||||||
|
.naive_sync_aggregation_pool
|
||||||
.read()
|
.read()
|
||||||
.get(sync_contribution_data)
|
.get(sync_contribution_data)
|
||||||
|
{
|
||||||
|
self.filter_optimistic_sync_committee_contribution(contribution)
|
||||||
|
.map(Option::Some)
|
||||||
|
} else {
|
||||||
|
Ok(None)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn filter_optimistic_sync_committee_contribution(
|
||||||
|
&self,
|
||||||
|
contribution: SyncCommitteeContribution<T::EthSpec>,
|
||||||
|
) -> Result<SyncCommitteeContribution<T::EthSpec>, Error> {
|
||||||
|
let beacon_block_root = contribution.beacon_block_root;
|
||||||
|
match self
|
||||||
|
.canonical_head
|
||||||
|
.fork_choice_read_lock()
|
||||||
|
.get_block_execution_status(&beacon_block_root)
|
||||||
|
{
|
||||||
|
// The contribution references a block that is not in fork choice, it must be
|
||||||
|
// pre-finalization.
|
||||||
|
None => Err(Error::SyncContributionDataReferencesFinalizedBlock { beacon_block_root }),
|
||||||
|
// The contribution references a fully valid `beacon_block_root`.
|
||||||
|
Some(execution_status) if execution_status.is_valid_or_irrelevant() => Ok(contribution),
|
||||||
|
// The contribution references a block that has not been verified by an EL (i.e. it
|
||||||
|
// is optimistic or invalid). Don't return the block, return an error instead.
|
||||||
|
Some(execution_status) => Err(Error::HeadBlockNotFullyVerified {
|
||||||
|
beacon_block_root,
|
||||||
|
execution_status,
|
||||||
|
}),
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Produce an unaggregated `Attestation` that is valid for the given `slot` and `index`.
|
/// Produce an unaggregated `Attestation` that is valid for the given `slot` and `index`.
|
||||||
|
@ -184,6 +184,9 @@ pub enum BeaconChainError {
|
|||||||
CannotAttestToFinalizedBlock {
|
CannotAttestToFinalizedBlock {
|
||||||
beacon_block_root: Hash256,
|
beacon_block_root: Hash256,
|
||||||
},
|
},
|
||||||
|
SyncContributionDataReferencesFinalizedBlock {
|
||||||
|
beacon_block_root: Hash256,
|
||||||
|
},
|
||||||
RuntimeShutdown,
|
RuntimeShutdown,
|
||||||
TokioJoin(tokio::task::JoinError),
|
TokioJoin(tokio::task::JoinError),
|
||||||
ProcessInvalidExecutionPayload(JoinError),
|
ProcessInvalidExecutionPayload(JoinError),
|
||||||
|
@ -2324,6 +2324,12 @@ pub fn serve<T: BeaconChainTypes>(
|
|||||||
blocking_json_task(move || {
|
blocking_json_task(move || {
|
||||||
chain
|
chain
|
||||||
.get_aggregated_sync_committee_contribution(&sync_committee_data)
|
.get_aggregated_sync_committee_contribution(&sync_committee_data)
|
||||||
|
.map_err(|e| {
|
||||||
|
warp_utils::reject::custom_bad_request(format!(
|
||||||
|
"unable to fetch sync contribution: {:?}",
|
||||||
|
e
|
||||||
|
))
|
||||||
|
})?
|
||||||
.map(api_types::GenericResponse::from)
|
.map(api_types::GenericResponse::from)
|
||||||
.ok_or_else(|| {
|
.ok_or_else(|| {
|
||||||
warp_utils::reject::custom_not_found(
|
warp_utils::reject::custom_not_found(
|
||||||
|
@ -4,7 +4,7 @@ use environment::RuntimeContext;
|
|||||||
use eth2::types::BlockId;
|
use eth2::types::BlockId;
|
||||||
use futures::future::join_all;
|
use futures::future::join_all;
|
||||||
use futures::future::FutureExt;
|
use futures::future::FutureExt;
|
||||||
use slog::{crit, debug, error, info, trace};
|
use slog::{crit, debug, error, info, trace, warn};
|
||||||
use slot_clock::SlotClock;
|
use slot_clock::SlotClock;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::ops::Deref;
|
use std::ops::Deref;
|
||||||
@ -174,17 +174,39 @@ impl<T: SlotClock + 'static, E: EthSpec> SyncCommitteeService<T, E> {
|
|||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fetch block root for `SyncCommitteeContribution`.
|
// Fetch `block_root` and `execution_optimistic` for `SyncCommitteeContribution`.
|
||||||
let block_root = self
|
let response = self
|
||||||
.beacon_nodes
|
.beacon_nodes
|
||||||
.first_success(RequireSynced::Yes, |beacon_node| async move {
|
.first_success(RequireSynced::Yes, |beacon_node| async move {
|
||||||
beacon_node.get_beacon_blocks_root(BlockId::Head).await
|
beacon_node.get_beacon_blocks_root(BlockId::Head).await
|
||||||
})
|
})
|
||||||
.await
|
.await
|
||||||
.map_err(|e| e.to_string())?
|
.map_err(|e| e.to_string())?
|
||||||
.ok_or_else(|| format!("No block root found for slot {}", slot))?
|
.ok_or_else(|| format!("No block root found for slot {}", slot))?;
|
||||||
.data
|
|
||||||
.root;
|
let block_root = response.data.root;
|
||||||
|
if let Some(execution_optimistic) = response.execution_optimistic {
|
||||||
|
if execution_optimistic {
|
||||||
|
warn!(
|
||||||
|
log,
|
||||||
|
"Refusing to sign sync committee messages for optimistic head block";
|
||||||
|
"slot" => slot,
|
||||||
|
);
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
} else if let Some(bellatrix_fork_epoch) = self.duties_service.spec.bellatrix_fork_epoch {
|
||||||
|
// If the slot is post Bellatrix, do not sign messages when we cannot verify the
|
||||||
|
// optimistic status of the head block.
|
||||||
|
if slot.epoch(E::slots_per_epoch()) > bellatrix_fork_epoch {
|
||||||
|
warn!(
|
||||||
|
log,
|
||||||
|
"Refusing to sign sync committee messages for a head block with an unknown \
|
||||||
|
optimistic status";
|
||||||
|
"slot" => slot,
|
||||||
|
);
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Spawn one task to publish all of the sync committee signatures.
|
// Spawn one task to publish all of the sync committee signatures.
|
||||||
let validator_duties = slot_duties.duties;
|
let validator_duties = slot_duties.duties;
|
||||||
|
Loading…
Reference in New Issue
Block a user