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.

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

1.1.5 merge spec tests #2781

Merged
merged 12 commits into from
Nov 10, 2021
24 changes: 0 additions & 24 deletions beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,18 +278,6 @@ pub enum ExecutionPayloadError {
///
/// The block is invalid and the peer is faulty
InvalidPayloadTimestamp { expected: u64, found: u64 },
/// The gas used in the block exceeds the gas limit
///
/// ## Peer scoring
///
/// The block is invalid and the peer is faulty
GasUsedExceedsLimit,
/// The payload block hash equals the parent hash
///
/// ## Peer scoring
///
/// The block is invalid and the peer is faulty
BlockHashEqualsParentHash,
/// The execution payload transaction list data exceeds size limits
///
/// ## Peer scoring
Expand Down Expand Up @@ -1353,18 +1341,6 @@ fn validate_execution_payload<T: BeaconChainTypes>(
},
));
}
// Gas used is less than the gas limit
if execution_payload.gas_used > execution_payload.gas_limit {
return Err(BlockError::ExecutionPayloadError(
ExecutionPayloadError::GasUsedExceedsLimit,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this GasUsedExceedsLimit error variant.

));
}
// The execution payload block hash is not equal to the parent hash
if execution_payload.block_hash == execution_payload.parent_hash {
return Err(BlockError::ExecutionPayloadError(
ExecutionPayloadError::BlockHashEqualsParentHash,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this BlockHashEqualsParentHash error variant.

));
}
// The execution payload transaction list data is within expected size limits
if execution_payload.transactions.len() > T::EthSpec::max_transactions_per_payload() {
return Err(BlockError::ExecutionPayloadError(
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/client/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ where
.terminal_total_difficulty_override
.unwrap_or(spec.terminal_total_difficulty);
let terminal_block_hash = config
.terminal_block_hash
.terminal_block_hash_override
.unwrap_or(spec.terminal_block_hash);

let execution_layer = if let Some(execution_endpoints) = config.execution_endpoints {
Expand Down
7 changes: 5 additions & 2 deletions beacon_node/client/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use beacon_chain::types::Epoch;
use directory::DEFAULT_ROOT_DIR;
use network::NetworkConfig;
use sensitive_url::SensitiveUrl;
Expand Down Expand Up @@ -76,7 +77,8 @@ pub struct Config {
pub eth1: eth1::Config,
pub execution_endpoints: Option<Vec<SensitiveUrl>>,
pub terminal_total_difficulty_override: Option<Uint256>,
pub terminal_block_hash: Option<Hash256>,
pub terminal_block_hash_override: Option<Hash256>,
pub terminal_block_hash_epoch_override: Option<Epoch>,
pub fee_recipient: Option<Address>,
pub http_api: http_api::Config,
pub http_metrics: http_metrics::Config,
Expand All @@ -100,7 +102,8 @@ impl Default for Config {
eth1: <_>::default(),
execution_endpoints: None,
terminal_total_difficulty_override: None,
terminal_block_hash: None,
terminal_block_hash_override: None,
terminal_block_hash_epoch_override: None,
fee_recipient: None,
disabled_forks: Vec::new(),
graffiti: Graffiti::default(),
Expand Down
66 changes: 35 additions & 31 deletions beacon_node/execution_layer/src/engine_api/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,8 @@ pub struct JsonExecutionPayload<T: EthSpec> {
pub base_fee_per_gas: Uint256,
pub block_hash: Hash256,
#[serde(with = "serde_transactions")]
pub transactions: VariableList<Transaction<T>, T::MaxTransactionsPerPayload>,
pub transactions:
VariableList<Transaction<T::MaxBytesPerTransaction>, T::MaxTransactionsPerPayload>,
}

impl<T: EthSpec> From<ExecutionPayload<T>> for JsonExecutionPayload<T> {
Expand Down Expand Up @@ -410,16 +411,16 @@ pub mod serde_transactions {
use serde::{de, Deserializer, Serializer};
use std::marker::PhantomData;

type Value<T, N> = VariableList<Transaction<T>, N>;
type Value<M, N> = VariableList<Transaction<M>, N>;

#[derive(Default)]
pub struct ListOfBytesListVisitor<T, N> {
_phantom_t: PhantomData<T>,
pub struct ListOfBytesListVisitor<M, N> {
_phantom_m: PhantomData<M>,
_phantom_n: PhantomData<N>,
}

impl<'a, T: EthSpec, N: Unsigned> serde::de::Visitor<'a> for ListOfBytesListVisitor<T, N> {
type Value = Value<T, N>;
impl<'a, M: Unsigned, N: Unsigned> serde::de::Visitor<'a> for ListOfBytesListVisitor<M, N> {
type Value = Value<M, N>;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(formatter, "a list of 0x-prefixed byte lists")
Expand All @@ -433,10 +434,9 @@ pub mod serde_transactions {

while let Some(val) = seq.next_element::<String>()? {
let inner_vec = hex::decode(&val).map_err(de::Error::custom)?;
let opaque_transaction = VariableList::new(inner_vec).map_err(|e| {
let transaction = VariableList::new(inner_vec).map_err(|e| {
serde::de::Error::custom(format!("transaction too large: {:?}", e))
})?;
let transaction = Transaction::OpaqueTransaction(opaque_transaction);
outer.push(transaction).map_err(|e| {
serde::de::Error::custom(format!("too many transactions: {:?}", e))
})?;
Expand All @@ -446,8 +446,8 @@ pub mod serde_transactions {
}
}

pub fn serialize<S, T: EthSpec, N: Unsigned>(
value: &Value<T, N>,
pub fn serialize<S, M: Unsigned, N: Unsigned>(
value: &Value<M, N>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
Expand All @@ -458,21 +458,19 @@ pub mod serde_transactions {
// It's important to match on the inner values of the transaction. Serializing the
// entire `Transaction` will result in appending the SSZ union prefix byte. The
// execution node does not want that.
let hex = match transaction {
Transaction::OpaqueTransaction(val) => hex::encode(&val[..]),
};
let hex = hex::encode(&transaction[..]);
seq.serialize_element(&hex)?;
}
seq.end()
}

pub fn deserialize<'de, D, T: EthSpec, N: Unsigned>(
pub fn deserialize<'de, D, M: Unsigned, N: Unsigned>(
deserializer: D,
) -> Result<Value<T, N>, D::Error>
) -> Result<Value<M, N>, D::Error>
where
D: Deserializer<'de>,
{
let visitor: ListOfBytesListVisitor<T, N> = <_>::default();
let visitor: ListOfBytesListVisitor<M, N> = <_>::default();
deserializer.deserialize_any(visitor)
}
}
Expand Down Expand Up @@ -558,7 +556,10 @@ mod test {
const LOGS_BLOOM_01: &str = "0x01010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101";

fn encode_transactions<E: EthSpec>(
transactions: VariableList<Transaction<E>, E::MaxTransactionsPerPayload>,
transactions: VariableList<
Transaction<E::MaxBytesPerTransaction>,
E::MaxTransactionsPerPayload,
>,
) -> Result<serde_json::Value, serde_json::Error> {
let ep: JsonExecutionPayload<E> = JsonExecutionPayload {
transactions,
Expand All @@ -570,7 +571,10 @@ mod test {

fn decode_transactions<E: EthSpec>(
transactions: serde_json::Value,
) -> Result<VariableList<Transaction<E>, E::MaxTransactionsPerPayload>, serde_json::Error> {
) -> Result<
VariableList<Transaction<E::MaxBytesPerTransaction>, E::MaxTransactionsPerPayload>,
serde_json::Error,
> {
let json = json!({
"parentHash": HASH_00,
"coinbase": ADDRESS_01,
Expand All @@ -593,35 +597,35 @@ mod test {

fn assert_transactions_serde<E: EthSpec>(
name: &str,
as_obj: VariableList<Transaction<E>, E::MaxTransactionsPerPayload>,
as_obj: VariableList<Transaction<E::MaxBytesPerTransaction>, E::MaxTransactionsPerPayload>,
as_json: serde_json::Value,
) {
assert_eq!(
encode_transactions(as_obj.clone()).unwrap(),
encode_transactions::<E>(as_obj.clone()).unwrap(),
as_json,
"encoding for {}",
name
);
assert_eq!(
decode_transactions(as_json).unwrap(),
decode_transactions::<E>(as_json).unwrap(),
as_obj,
"decoding for {}",
name
);
}

/// Example: if `spec == &[1, 1]`, then two one-byte transactions will be created.
fn generate_opaque_transactions<E: EthSpec>(
fn generate_transactions<E: EthSpec>(
spec: &[usize],
) -> VariableList<Transaction<E>, E::MaxTransactionsPerPayload> {
) -> VariableList<Transaction<E::MaxBytesPerTransaction>, E::MaxTransactionsPerPayload> {
let mut txs = VariableList::default();

for &num_bytes in spec {
let mut tx = VariableList::default();
for _ in 0..num_bytes {
tx.push(0).unwrap();
}
txs.push(Transaction::OpaqueTransaction(tx)).unwrap();
txs.push(tx).unwrap();
}

txs
Expand All @@ -631,32 +635,32 @@ mod test {
fn transaction_serde() {
assert_transactions_serde::<MainnetEthSpec>(
"empty",
generate_opaque_transactions(&[]),
generate_transactions::<MainnetEthSpec>(&[]),
json!([]),
);
assert_transactions_serde::<MainnetEthSpec>(
"one empty tx",
generate_opaque_transactions(&[0]),
generate_transactions::<MainnetEthSpec>(&[0]),
json!(["0x"]),
);
assert_transactions_serde::<MainnetEthSpec>(
"two empty txs",
generate_opaque_transactions(&[0, 0]),
generate_transactions::<MainnetEthSpec>(&[0, 0]),
json!(["0x", "0x"]),
);
assert_transactions_serde::<MainnetEthSpec>(
"one one-byte tx",
generate_opaque_transactions(&[1]),
generate_transactions::<MainnetEthSpec>(&[1]),
json!(["0x00"]),
);
assert_transactions_serde::<MainnetEthSpec>(
"two one-byte txs",
generate_opaque_transactions(&[1, 1]),
generate_transactions::<MainnetEthSpec>(&[1, 1]),
json!(["0x00", "0x00"]),
);
assert_transactions_serde::<MainnetEthSpec>(
"mixed bag",
generate_opaque_transactions(&[0, 1, 3, 0]),
generate_transactions::<MainnetEthSpec>(&[0, 1, 3, 0]),
json!(["0x", "0x00", "0x000000", "0x"]),
);

Expand All @@ -680,7 +684,7 @@ mod test {

use eth2_serde_utils::hex;

let num_max_bytes = <MainnetEthSpec as EthSpec>::MaxBytesPerOpaqueTransaction::to_usize();
let num_max_bytes = <MainnetEthSpec as EthSpec>::MaxBytesPerTransaction::to_usize();
let max_bytes = (0..num_max_bytes).map(|_| 0_u8).collect::<Vec<_>>();
let too_many_bytes = (0..=num_max_bytes).map(|_| 0_u8).collect::<Vec<_>>();
decode_transactions::<MainnetEthSpec>(
Expand Down
13 changes: 13 additions & 0 deletions beacon_node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,19 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
the broad Ethereum community has elected to override the terminal PoW block. \
Incorrect use of this flag will cause your node to experience a consensus
failure. Be extremely careful with this flag.")
.requires("terminal-block-hash-epoch-override")
.takes_value(true)
)
.arg(
Arg::with_name("terminal-block-hash-epoch-override")
.long("terminal-block-hash-epoch-override")
.value_name("EPOCH")
.help("Used to coordinate manual overrides to the TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH \
parameter. This flag should only be used if the user has a clear understanding \
that the broad Ethereum community has elected to override the terminal PoW block. \
Incorrect use of this flag will cause your node to experience a consensus
failure. Be extremely careful with this flag.")
.requires("terminal-block-hash-override")
.takes_value(true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make a "requires" dependency between both "terminal-block-hash-override" and "terminal-block-hash-epoch-override" (i.e. you can't supply one without the other).

I can't see how they'd be effective without each other and it would be nice to fail early.

)
.arg(
Expand Down
6 changes: 4 additions & 2 deletions beacon_node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,10 @@ pub fn get_config<E: EthSpec>(
}

client_config.fee_recipient = clap_utils::parse_optional(cli_args, "fee-recipient")?;
client_config.terminal_block_hash =
clap_utils::parse_optional(cli_args, "terminal-block-hash")?;
client_config.terminal_block_hash_override =
clap_utils::parse_optional(cli_args, "terminal-block-hash-override")?;
client_config.terminal_block_hash_epoch_override =
clap_utils::parse_optional(cli_args, "terminal-block-hash-epoch-override")?;

if let Some(freezer_dir) = cli_args.value_of("freezer-dir") {
client_config.freezer_db_path = Some(PathBuf::from(freezer_dir));
Expand Down
77 changes: 77 additions & 0 deletions consensus/ssz_types/src/serde_utils/list_of_hex_var_list.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//! Serialize `VaraibleList<VariableList<u8, M>, N>` as list of 0x-prefixed hex string.
use crate::VariableList;
use serde::{ser::SerializeSeq, Deserialize, Deserializer, Serialize, Serializer};
use std::marker::PhantomData;
use typenum::Unsigned;

#[derive(Deserialize)]
#[serde(transparent)]
pub struct WrappedListOwned<N: Unsigned>(
#[serde(with = "crate::serde_utils::hex_var_list")] VariableList<u8, N>,
);

#[derive(Serialize)]
#[serde(transparent)]
pub struct WrappedListRef<'a, N: Unsigned>(
#[serde(with = "crate::serde_utils::hex_var_list")] &'a VariableList<u8, N>,
);

pub fn serialize<S, M, N>(
list: &VariableList<VariableList<u8, M>, N>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
S: Serializer,
M: Unsigned,
N: Unsigned,
{
let mut seq = serializer.serialize_seq(Some(list.len()))?;
for bytes in list {
seq.serialize_element(&WrappedListRef(bytes))?;
}
seq.end()
}

#[derive(Default)]
pub struct Visitor<M, N> {
_phantom_m: PhantomData<M>,
_phantom_n: PhantomData<N>,
}

impl<'a, M, N> serde::de::Visitor<'a> for Visitor<M, N>
where
M: Unsigned,
N: Unsigned,
{
type Value = VariableList<VariableList<u8, M>, N>;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(formatter, "a list of 0x-prefixed hex bytes")
}

fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
where
A: serde::de::SeqAccess<'a>,
{
let mut list: VariableList<VariableList<u8, M>, N> = <_>::default();

while let Some(val) = seq.next_element::<WrappedListOwned<M>>()? {
list.push(val.0).map_err(|e| {
serde::de::Error::custom(format!("failed to push value to list: {:?}.", e))
})?;
}

Ok(list)
}
}

pub fn deserialize<'de, D, M, N>(
deserializer: D,
) -> Result<VariableList<VariableList<u8, M>, N>, D::Error>
where
D: Deserializer<'de>,
M: Unsigned,
N: Unsigned,
{
deserializer.deserialize_seq(Visitor::default())
}
1 change: 1 addition & 0 deletions consensus/ssz_types/src/serde_utils/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod hex_fixed_vec;
pub mod hex_var_list;
pub mod list_of_hex_var_list;
pub mod quoted_u64_fixed_vec;
pub mod quoted_u64_var_list;
Loading