Rename random to prev_randao (#3040)

## Issue Addressed

As discussed on last-night's consensus call, the testnets next week will target the [Kiln Spec v2](https://hackmd.io/@n0ble/kiln-spec).

Presently, we support Kiln V1. V2 is backwards compatible, except for renaming `random` to `prev_randao` in:

- https://github.com/ethereum/execution-apis/pull/180
- https://github.com/ethereum/consensus-specs/pull/2835

With this PR we'll no longer be compatible with the existing Kintsugi and Kiln testnets, however we'll be ready for the testnets next week. I raised this breaking change in the call last night, we are all keen to move forward and break things.

We now target the [`merge-kiln-v2`](https://github.com/MariusVanDerWijden/go-ethereum/tree/merge-kiln-v2) branch for interop with Geth. This required adding the `--http.aauthport` to the tester to avoid a port conflict at startup.

### Changes to exec integration tests

There's some change in the `merge-kiln-v2` version of Geth that means it can't compile on a vanilla Github runner. Bumping the `go` version on the runner solved this issue.

Whilst addressing this, I refactored the `testing/execution_integration` crate to be a *binary* rather than a *library* with tests. This means that we don't need to run the `build.rs` and build Geth whenever someone runs `make lint` or `make test-release`. This is nice for everyday users, but it's also nice for CI so that we can have a specific runner for these tests and we don't need to ensure *all* runners support everything required to build all execution clients.

## More Info

- [x] ~~EF tests are failing since the rename has broken some tests that reference the old field name. I have been told there will be new tests released in the coming days (25/02/22 or 26/02/22).~~
This commit is contained in:
Paul Hauner
2022-03-03 02:10:57 +00:00
parent 4bf1af4e85
commit aea43b626b
31 changed files with 304 additions and 221 deletions

View File

@@ -4,6 +4,7 @@ mod no_votes;
mod votes;
use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkChoice};
use crate::InvalidationOperation;
use serde_derive::{Deserialize, Serialize};
use types::{
AttestationShufflingId, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256,
@@ -238,12 +239,22 @@ impl ForkChoiceTestDefinition {
Operation::InvalidatePayload {
head_block_root,
latest_valid_ancestor_root,
} => fork_choice
.process_execution_payload_invalidation(
head_block_root,
latest_valid_ancestor_root,
)
.unwrap(),
} => {
let op = if let Some(latest_valid_ancestor) = latest_valid_ancestor_root {
InvalidationOperation::InvalidateMany {
head_block_root,
always_invalidate_head: true,
latest_valid_ancestor,
}
} else {
InvalidationOperation::InvalidateOne {
block_root: head_block_root,
}
};
fork_choice
.process_execution_payload_invalidation(&op)
.unwrap()
}
Operation::AssertWeight { block_root, weight } => assert_eq!(
fork_choice.get_weight(&block_root).unwrap(),
weight,

View File

@@ -4,6 +4,7 @@ mod proto_array;
mod proto_array_fork_choice;
mod ssz_container;
pub use crate::proto_array::InvalidationOperation;
pub use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkChoice};
pub use error::Error;

View File

@@ -15,6 +15,56 @@ use types::{
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.
pub enum InvalidationOperation {
/// Invalidate only `block_root` and it's descendants. Don't invalidate any ancestors.
InvalidateOne { block_root: Hash256 },
/// Invalidate blocks between `head_block_root` and `latest_valid_ancestor`.
///
/// If the `latest_valid_ancestor` is known to fork choice, invalidate all blocks between
/// `head_block_root` and `latest_valid_ancestor`. The `head_block_root` will be invalidated,
/// whilst the `latest_valid_ancestor` will not.
///
/// If `latest_valid_ancestor` is *not* known to fork choice, only invalidate the
/// `head_block_root` if `always_invalidate_head == true`.
InvalidateMany {
head_block_root: Hash256,
always_invalidate_head: bool,
latest_valid_ancestor: ExecutionBlockHash,
},
}
impl InvalidationOperation {
pub fn block_root(&self) -> Hash256 {
match self {
InvalidationOperation::InvalidateOne { block_root } => *block_root,
InvalidationOperation::InvalidateMany {
head_block_root, ..
} => *head_block_root,
}
}
pub fn latest_valid_ancestor(&self) -> Option<ExecutionBlockHash> {
match self {
InvalidationOperation::InvalidateOne { .. } => None,
InvalidationOperation::InvalidateMany {
latest_valid_ancestor,
..
} => Some(*latest_valid_ancestor),
}
}
pub fn invalidate_block_root(&self) -> bool {
match self {
InvalidationOperation::InvalidateOne { .. } => true,
InvalidationOperation::InvalidateMany {
always_invalidate_head,
..
} => *always_invalidate_head,
}
}
}
#[derive(Clone, PartialEq, Debug, Encode, Decode, Serialize, Deserialize)]
pub struct ProtoNode {
/// The `slot` is not necessary for `ProtoArray`, it just exists so external components can
@@ -328,43 +378,15 @@ impl ProtoArray {
}
}
/// Invalidate the relevant ancestors and descendants of a block with an invalid execution
/// payload.
/// Invalidate zero or more blocks, as specified by the `InvalidationOperation`.
///
/// The `head_block_root` should be the beacon block root of the block with the invalid
/// execution payload, _or_ its parent where the block with the invalid payload has not yet
/// been applied to `self`.
///
/// The `latest_valid_hash` should be the hash of most recent *valid* execution payload
/// contained in an ancestor block of `head_block_root`.
///
/// This function will invalidate:
///
/// * The block matching `head_block_root` _unless_ that block has a payload matching `latest_valid_hash`.
/// * All ancestors of `head_block_root` back to the block with payload matching
/// `latest_valid_hash` (endpoint > exclusive). In the case where the `head_block_root` is the parent
/// of the invalid block and itself matches `latest_valid_hash`, no ancestors will be invalidated.
/// * All descendants of `latest_valid_hash` if supplied and consistent with `head_block_root`,
/// or else all descendants of `head_block_root`.
///
/// ## Details
///
/// If `head_block_root` is not known to fork choice, an error is returned.
///
/// If `latest_valid_hash` is `Some(hash)` where `hash` is either not known to fork choice
/// (perhaps it's junk or pre-finalization), then only the `head_block_root` block will be
/// invalidated (no ancestors). No error will be returned in this case.
///
/// If `latest_valid_hash` is `Some(hash)` where `hash` is a known ancestor of
/// `head_block_root`, then all blocks between `head_block_root` and `latest_valid_hash` will
/// be invalidated. Additionally, all blocks that descend from a newly-invalidated block will
/// also be invalidated.
/// See the documentation of `InvalidationOperation` for usage.
pub fn propagate_execution_payload_invalidation(
&mut self,
head_block_root: Hash256,
latest_valid_ancestor_hash: Option<ExecutionBlockHash>,
op: &InvalidationOperation,
) -> Result<(), Error> {
let mut invalidated_indices: HashSet<usize> = <_>::default();
let head_block_root = op.block_root();
/*
* Step 1:
@@ -379,7 +401,8 @@ impl ProtoArray {
.ok_or(Error::NodeUnknown(head_block_root))?;
// Try to map the ancestor payload *hash* to an ancestor beacon block *root*.
let latest_valid_ancestor_root = latest_valid_ancestor_hash
let latest_valid_ancestor_root = op
.latest_valid_ancestor()
.and_then(|hash| self.execution_block_hash_to_beacon_block_root(&hash));
// Set to `true` if both conditions are satisfied:
@@ -414,7 +437,7 @@ impl ProtoArray {
// an invalid justified checkpoint.
if !latest_valid_ancestor_is_descendant && node.root != head_block_root {
break;
} else if Some(hash) == latest_valid_ancestor_hash {
} else if op.latest_valid_ancestor() == Some(hash) {
// If the `best_child` or `best_descendant` of the latest valid hash was
// invalidated, set those fields to `None`.
//
@@ -444,36 +467,44 @@ impl ProtoArray {
ExecutionStatus::Irrelevant(_) => break,
}
match &node.execution_status {
// It's illegal for an execution client to declare that some previously-valid block
// is now invalid. This is a consensus failure on their behalf.
ExecutionStatus::Valid(hash) => {
return Err(Error::ValidExecutionStatusBecameInvalid {
block_root: node.root,
payload_block_hash: *hash,
})
}
ExecutionStatus::Unknown(hash) => {
node.execution_status = ExecutionStatus::Invalid(*hash);
// Only invalidate the head block if either:
//
// - The head block was specifically indicated to be invalidated.
// - The latest valid hash is a known ancestor.
if node.root != head_block_root
|| op.invalidate_block_root()
|| latest_valid_ancestor_is_descendant
{
match &node.execution_status {
// It's illegal for an execution client to declare that some previously-valid block
// is now invalid. This is a consensus failure on their behalf.
ExecutionStatus::Valid(hash) => {
return Err(Error::ValidExecutionStatusBecameInvalid {
block_root: node.root,
payload_block_hash: *hash,
})
}
ExecutionStatus::Unknown(hash) => {
invalidated_indices.insert(index);
node.execution_status = ExecutionStatus::Invalid(*hash);
// It's impossible for an invalid block to lead to a "best" block, so set these
// fields to `None`.
//
// Failing to set these values will result in `Self::node_leads_to_viable_head`
// returning `false` for *valid* ancestors of invalid blocks.
node.best_child = None;
node.best_descendant = None;
// It's impossible for an invalid block to lead to a "best" block, so set these
// fields to `None`.
//
// Failing to set these values will result in `Self::node_leads_to_viable_head`
// returning `false` for *valid* ancestors of invalid blocks.
node.best_child = None;
node.best_descendant = None;
}
// The block is already invalid, but keep going backwards to ensure all ancestors
// are updated.
ExecutionStatus::Invalid(_) => (),
// This block is pre-merge, therefore it has no execution status. Nor do its
// ancestors.
ExecutionStatus::Irrelevant(_) => break,
}
// The block is already invalid, but keep going backwards to ensure all ancestors
// are updated.
ExecutionStatus::Invalid(_) => (),
// This block is pre-merge, therefore it has no execution status. Nor do its
// ancestors.
ExecutionStatus::Irrelevant(_) => break,
}
invalidated_indices.insert(index);
if let Some(parent_index) = node.parent {
index = parent_index
} else {

View File

@@ -1,5 +1,5 @@
use crate::error::Error;
use crate::proto_array::{Iter, ProposerBoost, ProtoArray};
use crate::proto_array::{InvalidationOperation, Iter, ProposerBoost, ProtoArray};
use crate::ssz_container::SszContainer;
use serde_derive::{Deserialize, Serialize};
use ssz::{Decode, Encode};
@@ -191,11 +191,10 @@ impl ProtoArrayForkChoice {
/// See `ProtoArray::propagate_execution_payload_invalidation` for documentation.
pub fn process_execution_payload_invalidation(
&mut self,
head_block_root: Hash256,
latest_valid_ancestor_root: Option<ExecutionBlockHash>,
op: &InvalidationOperation,
) -> Result<(), String> {
self.proto_array
.propagate_execution_payload_invalidation(head_block_root, latest_valid_ancestor_root)
.propagate_execution_payload_invalidation(op)
.map_err(|e| format!("Failed to process invalid payload: {:?}", e))
}