Skip to content

Commit

Permalink
Fix failing test
Browse files Browse the repository at this point in the history
  • Loading branch information
paulhauner committed Dec 2, 2021
1 parent a79f220 commit d5fae8f
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 20 deletions.
3 changes: 2 additions & 1 deletion beacon_node/beacon_chain/src/execution_payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,11 @@ pub fn execute_payload<T: BeaconChainTypes>(
Ok(status) => match status {
ExecutePayloadResponseStatus::Valid { .. } => Ok(PayloadVerificationStatus::Verified),
ExecutePayloadResponseStatus::Invalid { latest_valid_hash } => {
let head_block_root = block.parent_root();
match chain
.fork_choice
.write()
.on_invalid_execution_payload(block_root, latest_valid_hash)
.on_invalid_execution_payload(head_block_root, latest_valid_hash)
{
Ok(()) => warn!(
chain.log,
Expand Down
12 changes: 11 additions & 1 deletion beacon_node/beacon_chain/tests/payload_invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,17 @@ impl InvalidPayloadRig {
self.valid_blocks.insert(block_root);
}
Payload::Invalid => {
mock_execution_layer.server.all_payloads_invalid();
let parent = self
.harness
.chain
.get_block(&block.message().parent_root())
.unwrap()
.unwrap();
let parent_payload = parent.message().body().execution_payload().unwrap();
mock_execution_layer
.server
.all_payloads_invalid(parent_payload.block_hash);

match self.harness.process_block(slot, block.clone()) {
Err(BlockError::ExecutionPayloadError(
ExecutionPayloadError::RejectedByExecutionEngine,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
}

pub fn get_payload(&mut self, id: &PayloadId) -> Option<ExecutionPayload<T>> {
self.payload_ids.remove(id)
self.payload_ids.get(id).cloned()
}

pub fn execute_payload(&mut self, payload: ExecutionPayload<T>) -> ExecutePayloadResponse {
Expand Down
5 changes: 4 additions & 1 deletion beacon_node/execution_layer/src/test_utils/handle_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ pub async fn handle_rpc<T: EthSpec>(
message: None,
}
}
FixedPayloadResponse::Invalid => unimplemented!(),
FixedPayloadResponse::Invalid { latest_valid_hash } => ExecutePayloadResponse {
status: ExecutePayloadResponseStatus::Invalid { latest_valid_hash },
message: None,
},
};

let (status, latest_valid_hash) = match response.status {
Expand Down
7 changes: 4 additions & 3 deletions beacon_node/execution_layer/src/test_utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ mod mock_execution_layer;
pub enum FixedPayloadResponse {
None,
Valid,
Invalid,
Invalid { latest_valid_hash: Hash256 },
}

pub struct MockServer<T: EthSpec> {
Expand Down Expand Up @@ -125,8 +125,9 @@ impl<T: EthSpec> MockServer<T> {
*self.ctx.fixed_payload_response.lock() = FixedPayloadResponse::Valid;
}

pub fn all_payloads_invalid(&self) {
*self.ctx.fixed_payload_response.lock() = FixedPayloadResponse::Invalid;
pub fn all_payloads_invalid(&self, latest_valid_hash: Hash256) {
*self.ctx.fixed_payload_response.lock() =
FixedPayloadResponse::Invalid { latest_valid_hash };
}

pub fn full_payload_verification(&self) {
Expand Down
4 changes: 2 additions & 2 deletions consensus/fork_choice/src/fork_choice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,11 +447,11 @@ where

pub fn on_invalid_execution_payload(
&mut self,
invalid_root: Hash256,
head_block_root: Hash256,
latest_valid_ancestor_root: Hash256,
) -> Result<(), Error<T::Error>> {
self.proto_array
.process_execution_payload_invalidation(invalid_root, latest_valid_ancestor_root)
.process_execution_payload_invalidation(head_block_root, latest_valid_ancestor_root)
.map_err(Error::FailedToProcessInvalidExecutionPayload)
}

Expand Down
25 changes: 16 additions & 9 deletions consensus/proto_array/src/proto_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,14 +297,14 @@ impl ProtoArray {

pub fn propagate_execution_payload_invalidation(
&mut self,
invalid_root: Hash256,
latest_valid_ancestor_root: Hash256,
head_block_root: Hash256,
latest_valid_ancestor_hash: Hash256,
) -> Result<(), Error> {
let mut invalidated_indices: HashSet<usize> = <_>::default();
let mut index = *self
.indices
.get(&invalid_root)
.ok_or(Error::NodeUnknown(invalid_root))?;
.get(&head_block_root)
.ok_or(Error::NodeUnknown(head_block_root))?;
let first_potential_descendant = index + 1;

// Collect all *ancestors* which were declared invalid since they reside between the
Expand All @@ -315,11 +315,18 @@ impl ProtoArray {
.get_mut(index)
.ok_or(Error::InvalidNodeIndex(index))?;

if node.root == latest_valid_ancestor_root {
// It might be new knowledge that this block is valid, ensure that it and all
// ancestors are marked as valid.
self.propagate_execution_payload_validation(index)?;
break;
match node.execution_status {
ExecutionStatus::Valid(hash)
| ExecutionStatus::Invalid(hash)
| ExecutionStatus::Unknown(hash) => {
if hash == latest_valid_ancestor_hash {
// It might be new knowledge that this block is valid, ensure that it and all
// ancestors are marked as valid.
self.propagate_execution_payload_validation(index)?;
break;
}
}
ExecutionStatus::Irrelevant(_) => break,
}

match &node.execution_status {
Expand Down
4 changes: 2 additions & 2 deletions consensus/proto_array/src/proto_array_fork_choice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,11 @@ impl ProtoArrayForkChoice {

pub fn process_execution_payload_invalidation(
&mut self,
invalid_root: Hash256,
head_block_root: Hash256,
latest_valid_ancestor_root: Hash256,
) -> Result<(), String> {
self.proto_array
.propagate_execution_payload_invalidation(invalid_root, latest_valid_ancestor_root)
.propagate_execution_payload_invalidation(head_block_root, latest_valid_ancestor_root)
.map_err(|e| format!("Failed to process invalid payload: {:?}", e))
}

Expand Down

0 comments on commit d5fae8f

Please sign in to comment.