cerc-io/lighthouse
16
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Remove INVALID_TERMINAL_BLOCK (#3385)

Browse Source 
## Issue Addressed

Resolves #3379 

## Proposed Changes

Remove instances of `InvalidTerminalBlock` in lighthouse and use 
`Invalid {latest_valid_hash: "0x0000000000000000000000000000000000000000000000000000000000000000"}` 
to represent that status.
This commit is contained in:
Pawan Dhananjay 2022-08-10 07:52:58 +00:00
parent 2de26b20f8
commit c25934956b
7 changed files with 58 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
beacon_node/beacon_chain/src/beacon_chain.rs
View File

@ -4084,21 +4084,32 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
"Fork choice update invalidated payload";
"status" => ?status
);
// The execution engine has stated that all blocks between the
// `head_execution_block_hash` and `latest_valid_hash` are invalid.
self.process_invalid_execution_payload(
&InvalidationOperation::InvalidateMany {
head_block_root,
always_invalidate_head: true,
latest_valid_ancestor: latest_valid_hash,
},
)
.await?;
// This implies that the terminal block was invalid. We are being explicit in
// invalidating only the head block in this case.
if latest_valid_hash == ExecutionBlockHash::zero() {
self.process_invalid_execution_payload(
&InvalidationOperation::InvalidateOne {
block_root: head_block_root,
},
)
.await?;
} else {
// The execution engine has stated that all blocks between the
// `head_execution_block_hash` and `latest_valid_hash` are invalid.
self.process_invalid_execution_payload(
&InvalidationOperation::InvalidateMany {
head_block_root,
always_invalidate_head: true,
latest_valid_ancestor: latest_valid_hash,
},
)
.await?;
}
Err(BeaconChainError::ExecutionForkChoiceUpdateInvalid { status })
}
PayloadStatus::InvalidTerminalBlock { .. }
| PayloadStatus::InvalidBlockHash { .. } => {
PayloadStatus::InvalidBlockHash { .. } => {
warn!(
self.log,
"Fork choice update invalidated payload";

7
beacon_node/beacon_chain/src/execution_payload.rs
View File

@ -114,6 +114,11 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>(
PayloadStatus::Invalid {
latest_valid_hash, ..
} => {
// 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() {
return Err(ExecutionPayloadError::RejectedByExecutionEngine { status }.into());
}
// This block has not yet been applied to fork choice, so the latest block that was
// imported to fork choice was the parent.
let latest_root = block.parent_root();
@ -127,7 +132,7 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>(
Err(ExecutionPayloadError::RejectedByExecutionEngine { status }.into())
}
PayloadStatus::InvalidTerminalBlock { .. } | PayloadStatus::InvalidBlockHash { .. } => {
PayloadStatus::InvalidBlockHash { .. } => {
// 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`.

52
beacon_node/beacon_chain/tests/payload_invalidation.rs
View File

@ -40,7 +40,6 @@ enum Payload {
},
Syncing,
InvalidBlockHash,
InvalidTerminalBlock,
}
struct InvalidPayloadRig {
@ -231,16 +230,20 @@ impl InvalidPayloadRig {
Payload::Invalid { latest_valid_hash } => {
let latest_valid_hash = latest_valid_hash
.unwrap_or_else(|| self.block_hash(block.message().parent_root()));
mock_execution_layer
.server
.all_payloads_invalid_on_new_payload(latest_valid_hash)
if latest_valid_hash == ExecutionBlockHash::zero() {
mock_execution_layer
.server
.all_payloads_invalid_terminal_block_on_new_payload()
} else {
mock_execution_layer
.server
.all_payloads_invalid_on_new_payload(latest_valid_hash)
}
}
Payload::InvalidBlockHash => mock_execution_layer
.server
.all_payloads_invalid_block_hash_on_new_payload(),
Payload::InvalidTerminalBlock => mock_execution_layer
.server
.all_payloads_invalid_terminal_block_on_new_payload(),
};
let set_forkchoice_updated = |payload: Payload| match payload {
Payload::Valid => mock_execution_layer
@ -252,16 +255,20 @@ impl InvalidPayloadRig {
Payload::Invalid { latest_valid_hash } => {
let latest_valid_hash = latest_valid_hash
.unwrap_or_else(|| self.block_hash(block.message().parent_root()));
mock_execution_layer
.server
.all_payloads_invalid_on_forkchoice_updated(latest_valid_hash)
if latest_valid_hash == ExecutionBlockHash::zero() {
mock_execution_layer
.server
.all_payloads_invalid_terminal_block_on_forkchoice_updated()
} else {
mock_execution_layer
.server
.all_payloads_invalid_on_forkchoice_updated(latest_valid_hash)
}
}
Payload::InvalidBlockHash => mock_execution_layer
.server
.all_payloads_invalid_block_hash_on_forkchoice_updated(),
Payload::InvalidTerminalBlock => mock_execution_layer
.server
.all_payloads_invalid_terminal_block_on_forkchoice_updated(),
};
match (new_payload_response, forkchoice_response) {
@ -294,9 +301,7 @@ impl InvalidPayloadRig {
match forkchoice_response {
Payload::Syncing => assert!(execution_status.is_strictly_optimistic()),
Payload::Valid => assert!(execution_status.is_valid_and_post_bellatrix()),
Payload::Invalid { .. }
| Payload::InvalidBlockHash
| Payload::InvalidTerminalBlock => unreachable!(),
Payload::Invalid { .. } | Payload::InvalidBlockHash => unreachable!(),
}
assert_eq!(
@ -310,14 +315,8 @@ impl InvalidPayloadRig {
"block from db must match block imported"
);
}
(
Payload::Invalid { .. } | Payload::InvalidBlockHash | Payload::InvalidTerminalBlock,
_,
)
| (
_,
Payload::Invalid { .. } | Payload::InvalidBlockHash | Payload::InvalidTerminalBlock,
) => {
(Payload::Invalid { .. } | Payload::InvalidBlockHash, _)
| (_, Payload::Invalid { .. } | Payload::InvalidBlockHash) => {
set_new_payload(new_payload_response);
set_forkchoice_updated(forkchoice_response);
@ -473,7 +472,10 @@ async fn immediate_forkchoice_update_payload_invalid_block_hash() {
#[tokio::test]
async fn immediate_forkchoice_update_payload_invalid_terminal_block() {
immediate_forkchoice_update_invalid_test(|_| Payload::InvalidTerminalBlock).await
immediate_forkchoice_update_invalid_test(|_| Payload::Invalid {
latest_valid_hash: Some(ExecutionBlockHash::zero()),
})
.await
}
/// Ensure the client tries to exit when the justified checkpoint is invalidated.

1
beacon_node/execution_layer/src/engine_api.rs
View File

@ -78,7 +78,6 @@ pub enum PayloadStatusV1Status {
Syncing,
Accepted,
InvalidBlockHash,
InvalidTerminalBlock,
}
#[derive(Clone, Debug, PartialEq)]

7
beacon_node/execution_layer/src/engine_api/json_structures.rs
View File

@ -319,7 +319,6 @@ pub enum JsonPayloadStatusV1Status {
Syncing,
Accepted,
InvalidBlockHash,
InvalidTerminalBlock,
}
#[derive(Debug, PartialEq, Serialize, Deserialize)]
@ -338,9 +337,6 @@ impl From<PayloadStatusV1Status> for JsonPayloadStatusV1Status {
PayloadStatusV1Status::Syncing => JsonPayloadStatusV1Status::Syncing,
PayloadStatusV1Status::Accepted => JsonPayloadStatusV1Status::Accepted,
PayloadStatusV1Status::InvalidBlockHash => JsonPayloadStatusV1Status::InvalidBlockHash,
PayloadStatusV1Status::InvalidTerminalBlock => {
JsonPayloadStatusV1Status::InvalidTerminalBlock
}
}
}
}
@ -352,9 +348,6 @@ impl From<JsonPayloadStatusV1Status> for PayloadStatusV1Status {
JsonPayloadStatusV1Status::Syncing => PayloadStatusV1Status::Syncing,
JsonPayloadStatusV1Status::Accepted => PayloadStatusV1Status::Accepted,
JsonPayloadStatusV1Status::InvalidBlockHash => PayloadStatusV1Status::InvalidBlockHash,
JsonPayloadStatusV1Status::InvalidTerminalBlock => {
PayloadStatusV1Status::InvalidTerminalBlock
}
}
}
}

19
beacon_node/execution_layer/src/payload_status.rs
View File

@ -18,9 +18,6 @@ pub enum PayloadStatus {
InvalidBlockHash {
validation_error: Option<String>,
},
InvalidTerminalBlock {
validation_error: Option<String>,
},
}
/// Processes the response from the execution engine.
@ -90,22 +87,6 @@ pub fn process_payload_status(
validation_error: response.validation_error.clone(),
})
}
PayloadStatusV1Status::InvalidTerminalBlock => {
// In the interests of being liberal with what we accept, only raise a
// warning here.
if response.latest_valid_hash.is_some() {
warn!(
log,
"Malformed response from execution engine";
"msg" => "expected a null latest_valid_hash",
"status" => ?response.status
)
}
Ok(PayloadStatus::InvalidTerminalBlock {
validation_error: response.validation_error.clone(),
})
}
PayloadStatusV1Status::Syncing => {
// In the interests of being liberal with what we accept, only raise a
// warning here.

4
beacon_node/execution_layer/src/test_utils/mod.rs
View File

@ -245,8 +245,8 @@ impl<T: EthSpec> MockServer<T> {
fn invalid_terminal_block_status() -> PayloadStatusV1 {
PayloadStatusV1 {
status: PayloadStatusV1Status::InvalidTerminalBlock,
latest_valid_hash: None,
status: PayloadStatusV1Status::Invalid,
latest_valid_hash: Some(ExecutionBlockHash::zero()),
validation_error: Some("static response".into()),
}
}