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

Fix Uint256 deserialization #2786

Merged
merged 8 commits into from
Nov 10, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 5 additions & 11 deletions beacon_node/execution_layer/src/engine_api/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ impl<T: EthSpec> From<ExecutionPayload<T>> for JsonExecutionPayload<T> {
gas_used: e.gas_used,
timestamp: e.timestamp,
extra_data: e.extra_data,
base_fee_per_gas: Uint256::from_little_endian(e.base_fee_per_gas.as_bytes()),
base_fee_per_gas: e.base_fee_per_gas,
block_hash: e.block_hash,
transactions: e.transactions,
}
Expand All @@ -347,19 +347,13 @@ impl<T: EthSpec> From<JsonExecutionPayload<T>> for ExecutionPayload<T> {
gas_used: e.gas_used,
timestamp: e.timestamp,
extra_data: e.extra_data,
base_fee_per_gas: uint256_to_hash256(e.base_fee_per_gas),
base_fee_per_gas: e.base_fee_per_gas,
block_hash: e.block_hash,
transactions: e.transactions,
}
}
}

fn uint256_to_hash256(u: Uint256) -> Hash256 {
let mut bytes = [0; 32];
u.to_little_endian(&mut bytes);
Hash256::from_slice(&bytes)
}

#[derive(Debug, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct JsonConsensusValidatedRequest {
Expand Down Expand Up @@ -797,7 +791,7 @@ mod test {
gas_used: 2,
timestamp: 42,
extra_data: vec![].into(),
base_fee_per_gas: uint256_to_hash256(Uint256::from(1)),
base_fee_per_gas: Uint256::from(1),
block_hash: Hash256::repeat_byte(1),
transactions: vec![].into(),
})
Expand Down Expand Up @@ -960,7 +954,7 @@ mod test {
gas_used: 0,
timestamp: 5,
extra_data: vec![].into(),
base_fee_per_gas: uint256_to_hash256(Uint256::from(0)),
base_fee_per_gas: Uint256::from(0),
block_hash: Hash256::from_str("0xb084c10440f05f5a23a55d1d7ebcb1b3892935fb56f23cdc9a7f42c348eed174").unwrap(),
transactions: vec![].into(),
};
Expand All @@ -984,7 +978,7 @@ mod test {
gas_used: 0,
timestamp: 5,
extra_data: vec![].into(),
base_fee_per_gas: uint256_to_hash256(Uint256::from(0)),
base_fee_per_gas: Uint256::from(0),
block_hash: Hash256::from_str("0xb084c10440f05f5a23a55d1d7ebcb1b3892935fb56f23cdc9a7f42c348eed174").unwrap(),
transactions: vec![].into(),
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
gas_used: GAS_USED,
timestamp: payload.timestamp,
extra_data: "block gen was here".as_bytes().to_vec().into(),
base_fee_per_gas: Hash256::from_low_u64_le(1),
base_fee_per_gas: Uint256::one(),
block_hash: Hash256::zero(),
transactions: vec![].into(),
};
Expand Down
1 change: 1 addition & 0 deletions consensus/serde_utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ license = "Apache-2.0"
serde = { version = "1.0.116", features = ["derive"] }
serde_derive = "1.0.116"
hex = "0.4.2"
ethereum-types = "0.12.1"

[dev-dependencies]
serde_json = "1.0.58"
2 changes: 1 addition & 1 deletion consensus/serde_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ pub mod u32_hex;
pub mod u64_hex_be;
pub mod u8_hex;

pub use quoted_int::{quoted_u32, quoted_u64, quoted_u8};
pub use quoted_int::{quoted_u256, quoted_u32, quoted_u64, quoted_u8};
76 changes: 76 additions & 0 deletions consensus/serde_utils/src/quoted_int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//!
//! Quotes can be optional during decoding.

use ethereum_types::U256;
use serde::{Deserializer, Serializer};
use serde_derive::{Deserialize, Serialize};
use std::convert::TryFrom;
Expand Down Expand Up @@ -56,6 +57,17 @@ macro_rules! define_mod {
}
}

/// Compositional wrapper type that allows quotes or no quotes.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Deserialize, Serialize)]
#[serde(transparent)]
pub struct MaybeQuoted<T>
where
T: From<$int> + Into<$int> + Copy + TryFrom<u64>,
{
#[serde(with = "self")]
pub value: T,
}

/// Wrapper type for requiring quotes on a `$int`-like type.
///
/// Unlike using `serde(with = "quoted_$int::require_quotes")` this is composable, and can be nested
Expand Down Expand Up @@ -142,3 +154,67 @@ pub mod quoted_u64 {

define_mod!(u64, visit_u64);
}

pub mod quoted_u256 {
use super::*;

struct U256Visitor;

impl<'de> serde::de::Visitor<'de> for U256Visitor {
type Value = U256;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
formatter.write_str("a quoted U256 integer")
}

fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
U256::from_dec_str(v).map_err(serde::de::Error::custom)
}
}

/// Serialize with quotes.
pub fn serialize<S>(value: &U256, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_str(&format!("{}", value))
}

/// Deserialize with quotes.
pub fn deserialize<'de, D>(deserializer: D) -> Result<U256, D::Error>
where
D: Deserializer<'de>,
{
deserializer.deserialize_str(U256Visitor)
}
}


#[cfg(test)]
mod test {
use super::*;

#[derive(Debug, PartialEq, Serialize, Deserialize)]
#[serde(transparent)]
struct WrappedU256(#[serde(with = "quoted_u256")] U256);

#[test]
fn u256_with_quotes() {
assert_eq!(
&serde_json::to_string(&WrappedU256(U256::one())).unwrap(),
"\"1\""
);
assert_eq!(
serde_json::from_str::<WrappedU256>("\"1\"").unwrap(),
WrappedU256(U256::one())
);
}

#[test]
fn u256_without_quotes() {
serde_json::from_str::<WrappedU256>("1").unwrap_err();
}
}
2 changes: 1 addition & 1 deletion consensus/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ tempfile = "3.1.0"
derivative = "2.1.1"
rusqlite = { version = "0.25.3", features = ["bundled"], optional = true }
arbitrary = { version = "1.0", features = ["derive"], optional = true }
eth2_serde_utils = "0.1.0"
eth2_serde_utils = { path = "../serde_utils" }
regex = "1.3.9"
lazy_static = "1.4.0"
parking_lot = "0.11.1"
Expand Down
5 changes: 3 additions & 2 deletions consensus/types/src/execution_payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ pub struct ExecutionPayload<T: EthSpec> {
pub timestamp: u64,
#[serde(with = "ssz_types::serde_utils::hex_var_list")]
pub extra_data: VariableList<u8, T::MaxExtraDataBytes>,
pub base_fee_per_gas: Hash256,
#[serde(with = "eth2_serde_utils::quoted_u256")]
pub base_fee_per_gas: Uint256,
pub block_hash: Hash256,
#[serde(with = "ssz_types::serde_utils::list_of_hex_var_list")]
pub transactions:
Expand All @@ -51,7 +52,7 @@ impl<T: EthSpec> ExecutionPayload<T> {
gas_used: 0,
timestamp: 0,
extra_data: VariableList::empty(),
base_fee_per_gas: Hash256::zero(),
base_fee_per_gas: Uint256::zero(),
block_hash: Hash256::zero(),
transactions: VariableList::empty(),
}
Expand Down
3 changes: 2 additions & 1 deletion consensus/types/src/execution_payload_header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ pub struct ExecutionPayloadHeader<T: EthSpec> {
pub timestamp: u64,
#[serde(with = "ssz_types::serde_utils::hex_var_list")]
pub extra_data: VariableList<u8, T::MaxExtraDataBytes>,
pub base_fee_per_gas: Hash256,
#[serde(with = "eth2_serde_utils::quoted_u256")]
pub base_fee_per_gas: Uint256,
pub block_hash: Hash256,
pub transactions_root: Hash256,
}
Expand Down
7 changes: 1 addition & 6 deletions lcli/src/create_payload_header.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use bls::Hash256;
use clap::ArgMatches;
use clap_utils::{parse_optional, parse_required};
use int_to_bytes::int_to_bytes32;
use ssz::Encode;
use std::fs::File;
use std::io::Write;
Expand All @@ -16,10 +14,7 @@ pub fn run<T: EthSpec>(matches: &ArgMatches) -> Result<(), String> {
.map_err(|e| format!("Unable to get time: {:?}", e))?
.as_secs(),
);
let base_fee_per_gas = Hash256::from_slice(&int_to_bytes32(parse_required(
matches,
"base-fee-per-gas",
)?));
let base_fee_per_gas = parse_required(matches, "base-fee-per-gas")?;
let gas_limit = parse_required(matches, "gas-limit")?;
let file_name = matches.value_of("file").ok_or("No file supplied")?;

Expand Down