From d733e78ff4f21beb1dd0e08d7a202134f9af251f Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Sun, 5 Nov 2023 16:08:52 -0500 Subject: [PATCH 1/5] fix: use visit_f64 to deserialize mainnet TTD --- bin/reth/src/args/utils.rs | 45 +++++-------------- .../rpc-types/src/serde_helpers/json_u256.rs | 45 +++++++++++++++++++ 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/bin/reth/src/args/utils.rs b/bin/reth/src/args/utils.rs index 572d735d23f1..59422389ce4f 100644 --- a/bin/reth/src/args/utils.rs +++ b/bin/reth/src/args/utils.rs @@ -59,42 +59,17 @@ pub fn genesis_value_parser(s: &str) -> eyre::Result, eyre::Error "base" => BASE_MAINNET.clone(), _ => { // try to read json from path first - let mut raw = - match fs::read_to_string(PathBuf::from(shellexpand::full(s)?.into_owned())) { - Ok(raw) => raw, - Err(io_err) => { - // valid json may start with "\n", but must contain "{" - if s.contains('{') { - s.to_string() - } else { - return Err(io_err.into()) // assume invalid path - } + let raw = match fs::read_to_string(PathBuf::from(shellexpand::full(s)?.into_owned())) { + Ok(raw) => raw, + Err(io_err) => { + // valid json may start with "\n", but must contain "{" + if s.contains('{') { + s.to_string() + } else { + return Err(io_err.into()) // assume invalid path } - }; - - // The ethereum mainnet TTD is 58750000000000000000000, and geth serializes this - // without quotes, because that is how golang `big.Int`s marshal in JSON. Numbers - // are arbitrary precision in JSON, so this is valid JSON. This number is also - // greater than a `u64`. - // - // Unfortunately, serde_json only supports parsing up to `u64`, resorting to `f64` - // once `u64` overflows: - // - // - // - // - // serde_json does have an arbitrary precision feature, but this breaks untagged - // enums in serde: - // - // - // - // To solve this, we surround the mainnet TTD with quotes, which our custom Visitor - // accepts. - if raw.contains("58750000000000000000000") && - !raw.contains("\"58750000000000000000000\"") - { - raw = raw.replacen("58750000000000000000000", "\"58750000000000000000000\"", 1); - } + } + }; // both serialized Genesis and ChainSpec structs supported let genesis: AllGenesisFormats = serde_json::from_str(&raw)?; diff --git a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs index 5566a2f7d428..7bbac0341f5a 100644 --- a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs +++ b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs @@ -64,6 +64,38 @@ impl<'a> Visitor<'a> for JsonU256Visitor { Ok(JsonU256(U256::from(value))) } + fn visit_f64(self, value: f64) -> Result + where + E: Error, + { + // The ethereum mainnet TTD is 58750000000000000000000, and geth serializes this + // without quotes, because that is how golang `big.Int`s marshal in JSON. Numbers + // are arbitrary precision in JSON, so this is valid JSON. This number is also + // greater than a `u64`. + // + // Unfortunately, serde_json only supports parsing up to `u64`, resorting to `f64` + // once `u64` overflows: + // + // + // + // + // serde_json does have an arbitrary precision feature, but this breaks untagged + // enums in serde: + // + // + // + // To solve this, we use the captured float and return the TTD as a U256 if it's equal. + if value == 5.875e22 { + return Ok(JsonU256(U256::from(58750000000000000000000u128))); + } else if value.is_sign_negative() { + return Err(Error::custom("Negative numbers are not supported for JsonU256")); + } + + // We could try to convert to a u128 here but there would probably be loss of + // precision, so we just return an error. + Err(Error::custom("Float that are not the mainnet TTD are not supported for JsonU256")) + } + fn visit_str(self, value: &str) -> Result where E: Error, @@ -137,4 +169,17 @@ mod test { assert_eq!(serialized, r#""0x10""#); } + + #[test] + fn jsonu256_deserialize_ttd() { + let deserialized: Vec = + serde_json::from_str(r#"["58750000000000000000000",58750000000000000000000]"#).unwrap(); + assert_eq!( + deserialized, + vec![ + JsonU256(U256::from(58750000000000000000000u128)), + JsonU256(U256::from(58750000000000000000000u128)), + ] + ); + } } From e17d719d94a7d2bd9667f8e848abc977f40964b4 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Sun, 5 Nov 2023 17:15:56 -0500 Subject: [PATCH 2/5] use helper fn instead of JsonU256 --- crates/primitives/src/genesis.rs | 4 +- .../rpc-types/src/serde_helpers/json_u256.rs | 136 +++++++++++------- 2 files changed, 90 insertions(+), 50 deletions(-) diff --git a/crates/primitives/src/genesis.rs b/crates/primitives/src/genesis.rs index de8b4f326a46..dd1f4a0ffd64 100644 --- a/crates/primitives/src/genesis.rs +++ b/crates/primitives/src/genesis.rs @@ -2,7 +2,7 @@ use crate::{ constants::EMPTY_ROOT_HASH, keccak256, serde_helper::{ - deserialize_json_u256, deserialize_json_u256_opt, deserialize_storage_map, + deserialize_json_ttd_opt, deserialize_json_u256, deserialize_storage_map, num::{u64_hex_or_decimal, u64_hex_or_decimal_opt}, }, trie::{HashBuilder, Nibbles}, @@ -330,7 +330,7 @@ pub struct ChainConfig { /// Total difficulty reached that triggers the merge consensus upgrade. #[serde( skip_serializing_if = "Option::is_none", - deserialize_with = "deserialize_json_u256_opt" + deserialize_with = "deserialize_json_ttd_opt" )] pub terminal_total_difficulty: Option, diff --git a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs index 7bbac0341f5a..b457f9eac231 100644 --- a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs +++ b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs @@ -1,10 +1,10 @@ //! Json U256 serde helpers. - use alloy_primitives::U256; use serde::{ de::{Error, Visitor}, Deserialize, Deserializer, Serialize, Serializer, }; +use serde_json::Value; use std::{fmt, str::FromStr}; /// Wrapper around primitive U256 type that also supports deserializing numbers @@ -64,53 +64,11 @@ impl<'a> Visitor<'a> for JsonU256Visitor { Ok(JsonU256(U256::from(value))) } - fn visit_f64(self, value: f64) -> Result - where - E: Error, - { - // The ethereum mainnet TTD is 58750000000000000000000, and geth serializes this - // without quotes, because that is how golang `big.Int`s marshal in JSON. Numbers - // are arbitrary precision in JSON, so this is valid JSON. This number is also - // greater than a `u64`. - // - // Unfortunately, serde_json only supports parsing up to `u64`, resorting to `f64` - // once `u64` overflows: - // - // - // - // - // serde_json does have an arbitrary precision feature, but this breaks untagged - // enums in serde: - // - // - // - // To solve this, we use the captured float and return the TTD as a U256 if it's equal. - if value == 5.875e22 { - return Ok(JsonU256(U256::from(58750000000000000000000u128))); - } else if value.is_sign_negative() { - return Err(Error::custom("Negative numbers are not supported for JsonU256")); - } - - // We could try to convert to a u128 here but there would probably be loss of - // precision, so we just return an error. - Err(Error::custom("Float that are not the mainnet TTD are not supported for JsonU256")) - } - fn visit_str(self, value: &str) -> Result where E: Error, { - let value = match value.len() { - 0 => U256::ZERO, - 2 if value.starts_with("0x") => U256::ZERO, - _ if value.starts_with("0x") => U256::from_str(value).map_err(|e| { - Error::custom(format!("Parsing JsonU256 as hex failed {value}: {e}")) - })?, - _ => U256::from_str_radix(value, 10).map_err(|e| { - Error::custom(format!("Parsing JsonU256 as decimal failed {value}: {e:?}")) - })?, - }; - + let value = u256_from_str(value)?; Ok(JsonU256(value)) } @@ -140,10 +98,89 @@ where Ok(num.map(Into::into)) } +/// Supports deserializing a [U256] from a [String]. +pub fn u256_from_str(raw: &str) -> Result +where + E: Error, +{ + let value = match raw.len() { + 0 => U256::ZERO, + 2 if raw.starts_with("0x") => U256::ZERO, + _ if raw.starts_with("0x") => U256::from_str(raw) + .map_err(|e| Error::custom(format!("Parsing JsonU256 as hex failed {raw}: {e}")))?, + _ => U256::from_str_radix(raw, 10).map_err(|e| { + Error::custom(format!("Parsing JsonU256 as decimal failed {raw}: {e:?}")) + })?, + }; + + Ok(value) +} + +/// Supports parsing the TTD as an `Option`, or `Option` specifically for the mainnet TTD +/// (5.875e22). +pub fn deserialize_json_ttd_opt<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let value = Option::::deserialize(deserializer)?; + value.map(|value| ttd_from_value::<'de, D>(value)).transpose() +} + +/// Converts the given [serde_json::Value] into a `U256` value for TTD deserialization. +fn ttd_from_value<'de, D>(val: Value) -> Result +where + D: Deserializer<'de>, +{ + let val = match val { + Value::Number(num) => num, + Value::String(raw) => return u256_from_str(&raw), + _ => return Err(Error::custom("TTD must be a number")), + }; + + let num = if val.is_u64() { + // SAFETY: is_u64 is true - as_u64 should succeed + U256::from(val.as_u64().unwrap()) + } else if val.is_f64() { + // SAFETY: is_f64 is true - as_f64 should succeed + let value = val.as_f64().unwrap(); + + // The ethereum mainnet TTD is 58750000000000000000000, and geth serializes this + // without quotes, because that is how golang `big.Int`s marshal in JSON. Numbers + // are arbitrary precision in JSON, so this is valid JSON. This number is also + // greater than a `u64`. + // + // Unfortunately, serde_json only supports parsing up to `u64`, resorting to `f64` + // once `u64` overflows: + // + // + // + // + // serde_json does have an arbitrary precision feature, but this breaks untagged + // enums in serde: + // + // + // + // To solve this, we use the captured float and return the TTD as a U256 if it's equal. + if value == 5.875e22 { + U256::from(58750000000000000000000u128) + } else { + // We could try to convert to a u128 here but there would probably be loss of + // precision, so we just return an error. + return Err(Error::custom("Deserializing a large non-mainnet TTD is not supported")); + } + } else { + // must be i64 - negative numbers are not supported + return Err(Error::custom("Negative TTD values are invalid and will not be deserialized")); + }; + + Ok(num) +} + #[cfg(test)] mod test { use super::JsonU256; use alloy_primitives::U256; + use serde::{Deserialize, Serialize}; #[test] fn jsonu256_deserialize() { @@ -171,14 +208,17 @@ mod test { } #[test] - fn jsonu256_deserialize_ttd() { - let deserialized: Vec = + fn deserialize_ttd() { + #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] + struct Ttd(#[serde(deserialize_with = "super::deserialize_json_ttd_opt")] Option); + + let deserialized: Vec = serde_json::from_str(r#"["58750000000000000000000",58750000000000000000000]"#).unwrap(); assert_eq!( deserialized, vec![ - JsonU256(U256::from(58750000000000000000000u128)), - JsonU256(U256::from(58750000000000000000000u128)), + Ttd(Some(U256::from(58750000000000000000000u128))), + Ttd(Some(U256::from(58750000000000000000000u128))), ] ); } From 0c5957d129e51d847bba2ba9696cdd1990495f6e Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:42:14 -0500 Subject: [PATCH 3/5] use Number::as_str instead of using captured float --- .../rpc-types/src/serde_helpers/json_u256.rs | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs index b457f9eac231..f520de93f5cb 100644 --- a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs +++ b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs @@ -137,13 +137,9 @@ where _ => return Err(Error::custom("TTD must be a number")), }; - let num = if val.is_u64() { - // SAFETY: is_u64 is true - as_u64 should succeed - U256::from(val.as_u64().unwrap()) - } else if val.is_f64() { - // SAFETY: is_f64 is true - as_f64 should succeed - let value = val.as_f64().unwrap(); - + let num = if let Some(val) = val.as_u64() { + U256::from(val) + } else { // The ethereum mainnet TTD is 58750000000000000000000, and geth serializes this // without quotes, because that is how golang `big.Int`s marshal in JSON. Numbers // are arbitrary precision in JSON, so this is valid JSON. This number is also @@ -160,17 +156,9 @@ where // // // - // To solve this, we use the captured float and return the TTD as a U256 if it's equal. - if value == 5.875e22 { - U256::from(58750000000000000000000u128) - } else { - // We could try to convert to a u128 here but there would probably be loss of - // precision, so we just return an error. - return Err(Error::custom("Deserializing a large non-mainnet TTD is not supported")); - } - } else { - // must be i64 - negative numbers are not supported - return Err(Error::custom("Negative TTD values are invalid and will not be deserialized")); + // To solve this, we instead deserialize as string if it is not captured as a `u64`. + U256::from_str_radix(val.as_str(), 10) + .map_err(|e| Error::custom(format!("Parsing TTD as decimal failed {val}: {e:?}")))? }; Ok(num) @@ -212,11 +200,17 @@ mod test { #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] struct Ttd(#[serde(deserialize_with = "super::deserialize_json_ttd_opt")] Option); - let deserialized: Vec = - serde_json::from_str(r#"["58750000000000000000000",58750000000000000000000]"#).unwrap(); + let deserialized: Vec = serde_json::from_str( + r#"["",0,"0","0x0","58750000000000000000000",58750000000000000000000]"#, + ) + .unwrap(); assert_eq!( deserialized, vec![ + Ttd(Some(U256::ZERO)), + Ttd(Some(U256::ZERO)), + Ttd(Some(U256::ZERO)), + Ttd(Some(U256::ZERO)), Ttd(Some(U256::from(58750000000000000000000u128))), Ttd(Some(U256::from(58750000000000000000000u128))), ] From 4b8ff0fbe2a37d72986cd4664a9f46427027e6e8 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:43:38 -0500 Subject: [PATCH 4/5] improve non-string non-number error message --- crates/rpc/rpc-types/src/serde_helpers/json_u256.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs index f520de93f5cb..94b06bf872ea 100644 --- a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs +++ b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs @@ -134,7 +134,7 @@ where let val = match val { Value::Number(num) => num, Value::String(raw) => return u256_from_str(&raw), - _ => return Err(Error::custom("TTD must be a number")), + _ => return Err(Error::custom("TTD must be a number or string")), }; let num = if let Some(val) = val.as_u64() { From eae8cfb3d844674aa9f4ff208aba02749c0331b5 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 6 Nov 2023 16:49:42 -0500 Subject: [PATCH 5/5] use captured float if as_f64 succeeds --- .../rpc/rpc-types/src/serde_helpers/json_u256.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs index 94b06bf872ea..3ed3859a2c84 100644 --- a/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs +++ b/crates/rpc/rpc-types/src/serde_helpers/json_u256.rs @@ -139,7 +139,7 @@ where let num = if let Some(val) = val.as_u64() { U256::from(val) - } else { + } else if let Some(value) = val.as_f64() { // The ethereum mainnet TTD is 58750000000000000000000, and geth serializes this // without quotes, because that is how golang `big.Int`s marshal in JSON. Numbers // are arbitrary precision in JSON, so this is valid JSON. This number is also @@ -156,9 +156,17 @@ where // // // - // To solve this, we instead deserialize as string if it is not captured as a `u64`. - U256::from_str_radix(val.as_str(), 10) - .map_err(|e| Error::custom(format!("Parsing TTD as decimal failed {val}: {e:?}")))? + // To solve this, we use the captured float and return the TTD as a U256 if it's equal. + if value == 5.875e22 { + U256::from(58750000000000000000000u128) + } else { + // We could try to convert to a u128 here but there would probably be loss of + // precision, so we just return an error. + return Err(Error::custom("Deserializing a large non-mainnet TTD is not supported")); + } + } else { + // must be i64 - negative numbers are not supported + return Err(Error::custom("Negative TTD values are invalid and will not be deserialized")); }; Ok(num)