Merge branch 'capella-bugfixes' of https://github.com/michaelsproul/lighthouse into blob-sync-kzg

This commit is contained in:
realbigsean 2022-11-23 11:27:41 -05:00
commit abc933faa8
No known key found for this signature in database
GPG Key ID: B372B64D866BF8CC
5 changed files with 88 additions and 53 deletions

View File

@ -4157,37 +4157,29 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
return Ok(());
}
#[cfg(feature = "withdrawals")]
let head_state = &self.canonical_head.cached_head().snapshot.beacon_state;
#[cfg(feature = "withdrawals")]
let withdrawals = match self.spec.fork_name_at_epoch(prepare_epoch) {
ForkName::Base | ForkName::Altair | ForkName::Merge => {
None
},
ForkName::Capella | ForkName::Eip4844 => match &head_state {
&BeaconState::Capella(_) | &BeaconState::Eip4844(_) => {
// The head_state is already BeaconState::Capella or later
ForkName::Base | ForkName::Altair | ForkName::Merge => None,
ForkName::Capella | ForkName::Eip4844 => {
// We must use the advanced state because balances can change at epoch boundaries
// and balances affect withdrawals.
// FIXME(mark)
// Might implement caching here in the future..
Some(get_expected_withdrawals(head_state, &self.spec))
}
&BeaconState::Base(_) | &BeaconState::Altair(_) | &BeaconState::Merge(_) => {
// We are the Capella transition block proposer, need advanced state
let mut prepare_state = self
let prepare_state = self
.state_at_slot(prepare_slot, StateSkipConfig::WithoutStateRoots)
.or_else(|e| {
error!(self.log, "Capella Transition Proposer"; "Error Advancing State: " => ?e);
error!(self.log, "State advance for withdrawals failed"; "error" => ?e);
Err(e)
})?;
// FIXME(mark)
// Might implement caching here in the future..
Some(get_expected_withdrawals(&prepare_state, &self.spec))
}
},
}.transpose().or_else(|e| {
error!(self.log, "Error preparing beacon proposer"; "while calculating expected withdrawals" => ?e);
}
.transpose()
.or_else(|e| {
error!(self.log, "Error preparing beacon proposer"; "error" => ?e);
Err(e)
}).map(|withdrawals_opt| withdrawals_opt.map(|w| w.into()))
})
.map(|withdrawals_opt| withdrawals_opt.map(|w| w.into()))
.map_err(Error::PrepareProposerFailed)?;
let payload_attributes = PayloadAttributes::V2(PayloadAttributesV2 {

View File

@ -310,7 +310,7 @@ pub fn validate_execution_payload_for_gossip<T: BeaconChainTypes>(
}
};
if is_merge_transition_complete || !execution_payload.is_default() {
if is_merge_transition_complete || !execution_payload.is_default_with_empty_roots() {
let expected_timestamp = chain
.slot_clock
.start_of(block.slot())

View File

@ -431,9 +431,11 @@ pub fn process_execution_payload<'payload, T: EthSpec, Payload: AbstractExecPayl
/// repeaetedly write code to treat these errors as false.
/// https://github.com/ethereum/consensus-specs/blob/dev/specs/bellatrix/beacon-chain.md#is_merge_transition_complete
pub fn is_merge_transition_complete<T: EthSpec>(state: &BeaconState<T>) -> bool {
// We must check defaultness against the payload header with 0x0 roots, as that's what's meant
// by `ExecutionPayloadHeader()` in the spec.
state
.latest_execution_payload_header()
.map(|header| !header.is_default())
.map(|header| !header.is_default_with_zero_roots())
.unwrap_or(false)
}
/// https://github.com/ethereum/consensus-specs/blob/dev/specs/bellatrix/beacon-chain.md#is_merge_transition_block
@ -441,8 +443,12 @@ pub fn is_merge_transition_block<T: EthSpec, Payload: AbstractExecPayload<T>>(
state: &BeaconState<T>,
body: BeaconBlockBodyRef<T, Payload>,
) -> bool {
// For execution payloads in blocks (which may be headers) we must check defaultness against
// the payload with `transactions_root` equal to the tree hash of the empty list.
body.execution_payload()
.map(|payload| !is_merge_transition_complete(state) && !payload.is_default())
.map(|payload| {
!is_merge_transition_complete(state) && !payload.is_default_with_empty_roots()
})
.unwrap_or(false)
}
/// https://github.com/ethereum/consensus-specs/blob/dev/specs/bellatrix/beacon-chain.md#is_execution_enabled

View File

@ -103,9 +103,9 @@ impl<T: EthSpec> ExecutionPayloadHeader<T> {
}
impl<'a, T: EthSpec> ExecutionPayloadHeaderRef<'a, T> {
pub fn is_default(self) -> bool {
pub fn is_default_with_zero_roots(self) -> bool {
map_execution_payload_header_ref!(&'a _, self, |inner, cons| {
let _ = cons(inner);
cons(inner);
*inner == Default::default()
})
}

View File

@ -40,8 +40,11 @@ pub trait ExecPayload<T: EthSpec>: Debug + Clone + PartialEq + Hash + TreeHash +
#[cfg(feature = "withdrawals")]
fn withdrawals_root(&self) -> Result<Hash256, Error>;
/// Is this a default payload? (pre-merge)
fn is_default(&self) -> bool;
/// Is this a default payload with 0x0 roots for transactions and withdrawals?
fn is_default_with_zero_roots(&self) -> bool;
/// Is this a default payload with the hash of the empty list for transactions and withdrawals?
fn is_default_with_empty_roots(&self) -> bool;
}
/// `ExecPayload` functionality the requires ownership.
@ -241,12 +244,17 @@ impl<T: EthSpec> ExecPayload<T> for FullPayload<T> {
}
}
fn is_default<'a>(&'a self) -> bool {
fn is_default_with_zero_roots<'a>(&'a self) -> bool {
map_full_payload_ref!(&'a _, self.to_ref(), move |payload, cons| {
cons(payload);
payload.execution_payload == <_>::default()
})
}
fn is_default_with_empty_roots<'a>(&'a self) -> bool {
// For full payloads the empty/zero distinction does not exist.
self.is_default_with_zero_roots()
}
}
impl<T: EthSpec> FullPayload<T> {
@ -338,13 +346,17 @@ impl<'b, T: EthSpec> ExecPayload<T> for FullPayloadRef<'b, T> {
}
}
// TODO: can this function be optimized?
fn is_default<'a>(&'a self) -> bool {
fn is_default_with_zero_roots<'a>(&'a self) -> bool {
map_full_payload_ref!(&'a _, self, move |payload, cons| {
cons(payload);
payload.execution_payload == <_>::default()
})
}
fn is_default_with_empty_roots(&self) -> bool {
// For full payloads the empty/zero distinction does not exist.
self.is_default_with_zero_roots()
}
}
impl<T: EthSpec> AbstractExecPayload<T> for FullPayload<T> {
@ -505,11 +517,16 @@ impl<T: EthSpec> ExecPayload<T> for BlindedPayload<T> {
}
}
fn is_default<'a>(&'a self) -> bool {
map_blinded_payload_ref!(&'a _, self.to_ref(), move |payload, cons| {
cons(payload);
payload.execution_payload_header == <_>::default()
})
fn is_default_with_zero_roots<'a>(&'a self) -> bool {
self.to_ref().is_default_with_zero_roots()
}
// For blinded payloads we must check "defaultness" against the default `ExecutionPayload`
// which has been blinded into an `ExecutionPayloadHeader`, NOT against the default
// `ExecutionPayloadHeader` which has a zeroed out `transactions_root`. The transactions root
// should be the root of the empty list.
fn is_default_with_empty_roots(&self) -> bool {
self.to_ref().is_default_with_empty_roots()
}
}
@ -591,24 +608,38 @@ impl<'b, T: EthSpec> ExecPayload<T> for BlindedPayloadRef<'b, T> {
}
}
// TODO: can this function be optimized?
fn is_default<'a>(&'a self) -> bool {
map_blinded_payload_ref!(&'a _, self, move |payload, cons| {
fn is_default_with_zero_roots<'a>(&'a self) -> bool {
map_blinded_payload_ref!(&'b _, self, move |payload, cons| {
cons(payload);
payload.execution_payload_header == <_>::default()
})
}
fn is_default_with_empty_roots<'a>(&'a self) -> bool {
map_blinded_payload_ref!(&'b _, self, move |payload, cons| {
cons(payload);
payload.is_default_with_empty_roots()
})
}
}
macro_rules! impl_exec_payload_common {
($wrapper_type:ident, $wrapped_type_full:ident, $wrapped_header_type:ident, $wrapped_field:ident, $fork_variant:ident, $block_type_variant:ident, $f:block, $g:block) => {
($wrapper_type:ident,
$wrapped_type:ident,
$wrapped_type_full:ident,
$wrapped_type_header:ident,
$wrapped_field:ident,
$fork_variant:ident,
$block_type_variant:ident,
$f:block,
$g:block) => {
impl<T: EthSpec> ExecPayload<T> for $wrapper_type<T> {
fn block_type() -> BlockType {
BlockType::$block_type_variant
}
fn to_execution_payload_header(&self) -> ExecutionPayloadHeader<T> {
ExecutionPayloadHeader::$fork_variant($wrapped_header_type::from(
ExecutionPayloadHeader::$fork_variant($wrapped_type_header::from(
self.$wrapped_field.clone(),
))
}
@ -641,8 +672,12 @@ macro_rules! impl_exec_payload_common {
self.$wrapped_field.gas_limit
}
fn is_default(&self) -> bool {
self.$wrapped_field == $wrapped_type_full::default()
fn is_default_with_zero_roots(&self) -> bool {
self.$wrapped_field == $wrapped_type::default()
}
fn is_default_with_empty_roots(&self) -> bool {
self.$wrapped_field == $wrapped_type::from($wrapped_type_full::default())
}
fn transactions(&self) -> Option<&Transactions<T>> {
@ -657,8 +692,8 @@ macro_rules! impl_exec_payload_common {
}
}
impl<T: EthSpec> From<$wrapped_type_full<T>> for $wrapper_type<T> {
fn from($wrapped_field: $wrapped_type_full<T>) -> Self {
impl<T: EthSpec> From<$wrapped_type<T>> for $wrapper_type<T> {
fn from($wrapped_field: $wrapped_type<T>) -> Self {
Self { $wrapped_field }
}
}
@ -672,6 +707,7 @@ macro_rules! impl_exec_payload_for_fork {
impl_exec_payload_common!(
$wrapper_type_header,
$wrapped_type_header,
$wrapped_type_full,
$wrapped_type_header,
execution_payload_header,
$fork_variant,
@ -741,6 +777,7 @@ macro_rules! impl_exec_payload_for_fork {
impl_exec_payload_common!(
$wrapper_type_full,
$wrapped_type_full,
$wrapped_type_full,
$wrapped_type_header,
execution_payload,
$fork_variant,