Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Add more logging for invalid payloads #3515

Closed
wants to merge 3 commits into from
Closed

[Merged by Bors] - Add more logging for invalid payloads #3515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c62f5be
Add more logs for invalid payloads
paulhauner Aug 26, 2022
2b88c0e
Modify existing log
paulhauner Aug 26, 2022
d9a0785
Empty commit for old mate Github actions
paulhauner Aug 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions beacon_node/beacon_chain/src/beacon_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3598,9 +3598,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
) -> Result<(), Error> {
debug!(
self.log,
"Invalid execution payload in block";
"latest_valid_ancestor" => ?op.latest_valid_ancestor(),
"block_root" => ?op.block_root(),
"Processing payload invalidation";
"op" => ?op,
);

// Update the execution status in fork choice.
Expand Down Expand Up @@ -4080,8 +4079,18 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
Ok(())
}
PayloadStatus::Invalid {
latest_valid_hash, ..
latest_valid_hash,
ref validation_error,
} => {
debug!(
self.log,
"Invalid execution payload";
"validation_error" => ?validation_error,
"latest_valid_hash" => ?latest_valid_hash,
"head_hash" => ?head_hash,
"head_block_root" => ?head_block_root,
"method" => "fcU",
);
warn!(
self.log,
"Fork choice update invalidated payload";
Expand Down Expand Up @@ -4112,7 +4121,17 @@ impl<T: BeaconChainTypes> BeaconChain<T> {

Err(BeaconChainError::ExecutionForkChoiceUpdateInvalid { status })
}
PayloadStatus::InvalidBlockHash { .. } => {
PayloadStatus::InvalidBlockHash {
ref validation_error,
} => {
debug!(
self.log,
"Invalid execution payload block hash";
"validation_error" => ?validation_error,
"head_hash" => ?head_hash,
"head_block_root" => ?head_block_root,
"method" => "fcU",
);
warn!(
self.log,
"Fork choice update invalidated payload";
Expand Down
33 changes: 31 additions & 2 deletions beacon_node/beacon_chain/src/execution_payload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use state_processing::per_block_processing::{
};
use std::sync::Arc;
use tokio::task::JoinHandle;
use tree_hash::TreeHash;
use types::*;

pub type PreparePayloadResult<Payload> = Result<Payload, BlockProductionError>;
Expand Down Expand Up @@ -112,8 +113,22 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>(
Ok(PayloadVerificationStatus::Optimistic)
}
PayloadStatus::Invalid {
latest_valid_hash, ..
latest_valid_hash,
ref validation_error,
} => {
debug!(
chain.log,
"Invalid execution payload";
"validation_error" => ?validation_error,
"latest_valid_hash" => ?latest_valid_hash,
"execution_block_hash" => ?execution_payload.execution_payload.block_hash,
"root" => ?block.tree_hash_root(),
"graffiti" => block.body().graffiti().as_utf8_lossy(),
"proposer_index" => block.proposer_index(),
"slot" => block.slot(),
"method" => "new_payload",
);

// latest_valid_hash == 0 implies that this was the terminal block
// Hence, we don't need to run `BeaconChain::process_invalid_execution_payload`.
if latest_valid_hash == ExecutionBlockHash::zero() {
Expand All @@ -132,7 +147,21 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>(

Err(ExecutionPayloadError::RejectedByExecutionEngine { status }.into())
}
PayloadStatus::InvalidBlockHash { .. } => {
PayloadStatus::InvalidBlockHash {
ref validation_error,
} => {
debug!(
chain.log,
"Invalid execution payload block hash";
"validation_error" => ?validation_error,
"execution_block_hash" => ?execution_payload.execution_payload.block_hash,
"root" => ?block.tree_hash_root(),
"graffiti" => block.body().graffiti().as_utf8_lossy(),
"proposer_index" => block.proposer_index(),
"slot" => block.slot(),
"method" => "new_payload",
);

// Returning an error here should be sufficient to invalidate the block. We have no
// information to indicate its parent is invalid, so no need to run
// `BeaconChain::process_invalid_execution_payload`.
Expand Down
2 changes: 1 addition & 1 deletion consensus/proto_array/src/proto_array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ four_byte_option_impl!(four_byte_option_usize, usize);
four_byte_option_impl!(four_byte_option_checkpoint, Checkpoint);

/// Defines an operation which may invalidate the `execution_status` of some nodes.
#[derive(Clone)]
#[derive(Clone, Debug)]
pub enum InvalidationOperation {
/// Invalidate only `block_root` and it's descendants. Don't invalidate any ancestors.
InvalidateOne { block_root: Hash256 },
Expand Down