From 1407d868fa229a9fbbf6e6b21f7c3c2506fe56b0 Mon Sep 17 00:00:00 2001 From: 0xferrous <0xferrous@proton.me> Date: Sat, 11 Oct 2025 03:11:02 +0000 Subject: [PATCH] fix(cast): consistent serialization of Uint/Ints depending on actual type The current implementation dynamically tries to determine if the runtime value can fit in 64 bits, but this leads to inconsistent serialization. For instance if you were decoding an `uint[]`, some of the values that fit in 64 bits will serialize as number while others serialize as string making it require special handling on the user that is consuming the json. This change makes it so it uses the type information to determine the serialization. So the user will always know that specific types will always serialize to a number or a string depending on the number of bits that type uses. --- Cargo.lock | 1 + crates/cast/src/args.rs | 2 +- crates/cast/src/lib.rs | 4 +- crates/cast/tests/cli/selectors.rs | 4 +- crates/cheatcodes/src/json.rs | 23 ++-- crates/common/fmt/Cargo.toml | 1 + .../fmt/proptest-regressions/dynamic.txt | 7 + crates/common/fmt/src/dynamic.rs | 120 +++++++++++++++--- 8 files changed, 130 insertions(+), 32 deletions(-) create mode 100644 crates/common/fmt/proptest-regressions/dynamic.txt diff --git a/Cargo.lock b/Cargo.lock index 27bceb54eb3cd..3f9f0838e2b8a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4543,6 +4543,7 @@ dependencies = [ "chrono", "eyre", "foundry-macros", + "proptest", "revm", "serde", "serde_json", diff --git a/crates/cast/src/args.rs b/crates/cast/src/args.rs index 12c6e762824bb..4fab15299a0b5 100644 --- a/crates/cast/src/args.rs +++ b/crates/cast/src/args.rs @@ -761,7 +761,7 @@ pub async fn run_command(args: CastArgs) -> Result<()> { let tokens: Vec = tokens .iter() .cloned() - .map(|t| serialize_value_as_json(t, None)) + .map(|t| serialize_value_as_json(t, None, true)) .collect::>>() .unwrap(); let _ = sh_println!("{}", serde_json::to_string_pretty(&tokens).unwrap()); diff --git a/crates/cast/src/lib.rs b/crates/cast/src/lib.rs index 12778fb4bc486..aba146a293ac0 100644 --- a/crates/cast/src/lib.rs +++ b/crates/cast/src/lib.rs @@ -202,7 +202,7 @@ impl> Cast

{ } else if shell::is_json() { let tokens = decoded .into_iter() - .map(|value| serialize_value_as_json(value, None)) + .map(|value| serialize_value_as_json(value, None, true)) .collect::>>()?; serde_json::to_string_pretty(&tokens).unwrap() } else { @@ -2500,7 +2500,7 @@ mod tests { let calldata = "0xdb5b0ed700000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000006772bf190000000000000000000000000000000000000000000000000000000000020716000000000000000000000000af9d27ffe4d51ed54ac8eec78f2785d7e11e5ab100000000000000000000000000000000000000000000000000000000000002c0000000000000000000000000000000000000000000000000000000000000000404366a6dc4b2f348a85e0066e46f0cc206fca6512e0ed7f17ca7afb88e9a4c27000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000093922dee6e380c28a50c008ab167b7800bb24c2026cd1b22f1c6fb884ceed7400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060f85e59ecad6c1a6be343a945abedb7d5b5bfad7817c4d8cc668da7d391faf700000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000093dfbf04395fbec1f1aed4ad0f9d3ba880ff58a60485df5d33f8f5e0fb73188600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000aa334a426ea9e21d5f84eb2d4723ca56b92382b9260ab2b6769b7c23d437b6b512322a25cecc954127e60cf91ef056ac1da25f90b73be81c3ff1872fa48d10c7ef1ccb4087bbeedb54b1417a24abbb76f6cd57010a65bb03c7b6602b1eaf0e32c67c54168232d4edc0bfa1b815b2af2a2d0a5c109d675a4f2de684e51df9abb324ab1b19a81bac80f9ce3a45095f3df3a7cf69ef18fc08e94ac3cbc1c7effeacca68e3bfe5d81e26a659b5"; let sig = "sequenceBatchesValidium((bytes32,bytes32,uint64,bytes32)[],uint64,uint64,address,bytes)"; let decoded = Cast::calldata_decode(sig, calldata, true).unwrap(); - let json_value = serialize_value_as_json(DynSolValue::Array(decoded), None).unwrap(); + let json_value = serialize_value_as_json(DynSolValue::Array(decoded), None, true).unwrap(); let expected = serde_json::json!([ [ [ diff --git a/crates/cast/tests/cli/selectors.rs b/crates/cast/tests/cli/selectors.rs index a400e209862dc..4695b1f8e4d73 100644 --- a/crates/cast/tests/cli/selectors.rs +++ b/crates/cast/tests/cli/selectors.rs @@ -140,7 +140,7 @@ casttest!(event_decode_with_sig, |_prj, cmd| { cmd.args(["--json"]).assert_success().stdout_eq(str![[r#" [ - 78, + "78", "0x0000000000000000000000000000000000D0004F" ] @@ -168,7 +168,7 @@ casttest!(error_decode_with_sig, |_prj, cmd| { cmd.args(["--json"]).assert_success().stdout_eq(str![[r#" [ - 101, + "101", "0x0000000000000000000000000000000000D0004F" ] diff --git a/crates/cheatcodes/src/json.rs b/crates/cheatcodes/src/json.rs index bd4515bbbf0ec..a7a246906fa77 100644 --- a/crates/cheatcodes/src/json.rs +++ b/crates/cheatcodes/src/json.rs @@ -317,7 +317,8 @@ impl Cheatcode for serializeJsonType_0Call { let Self { typeDescription, value } = self; let ty = resolve_type(typeDescription, state.struct_defs())?; let value = ty.abi_decode(value)?; - let value = foundry_common::fmt::serialize_value_as_json(value, state.struct_defs())?; + let value = + foundry_common::fmt::serialize_value_as_json(value, state.struct_defs(), false)?; Ok(value.to_string().abi_encode()) } } @@ -654,7 +655,7 @@ fn serialize_json( value_key: &str, value: DynSolValue, ) -> Result { - let value = foundry_common::fmt::serialize_value_as_json(value, state.struct_defs())?; + let value = foundry_common::fmt::serialize_value_as_json(value, state.struct_defs(), false)?; let map = state.serialized_jsons.entry(object_key.into()).or_default(); map.insert(value_key.into(), value); let stringified = serde_json::to_string(map).unwrap(); @@ -886,7 +887,7 @@ mod tests { proptest::proptest! { #[test] fn test_json_roundtrip_guessed(v in guessable_types()) { - let json = serialize_value_as_json(v.clone(), None).unwrap(); + let json = serialize_value_as_json(v.clone(), None, false).unwrap(); let value = json_value_to_token(&json, None).unwrap(); // do additional abi_encode -> abi_decode to avoid zero signed integers getting decoded as unsigned and causing assert_eq to fail. @@ -896,14 +897,14 @@ mod tests { #[test] fn test_json_roundtrip(v in any::().prop_filter("filter out values without type", |v| v.as_type().is_some())) { - let json = serialize_value_as_json(v.clone(), None).unwrap(); + let json = serialize_value_as_json(v.clone(), None, false).unwrap(); let value = parse_json_as(&json, &v.as_type().unwrap()).unwrap(); assert_eq!(value, v); } #[test] fn test_json_roundtrip_with_struct_defs((struct_defs, v) in custom_struct_strategy()) { - let json = serialize_value_as_json(v.clone(), Some(&struct_defs)).unwrap(); + let json = serialize_value_as_json(v.clone(), Some(&struct_defs), false).unwrap(); let sol_type = v.as_type().unwrap(); let parsed_value = parse_json_as(&json, &sol_type).unwrap(); assert_eq!(parsed_value, v); @@ -1062,7 +1063,8 @@ mod tests { }; // Serialize the value to JSON and verify that the order is preserved. - let json_value = serialize_value_as_json(item_struct, Some(&struct_defs.into())).unwrap(); + let json_value = + serialize_value_as_json(item_struct, Some(&struct_defs.into()), false).unwrap(); let json_string = serde_json::to_string(&json_value).unwrap(); assert_eq!(json_string, r#"{"name":"Test Item","id":123,"active":true}"#); } @@ -1094,9 +1096,12 @@ mod tests { }; // Serialize it. The resulting JSON should respect the struct definition order. - let json_value = - serialize_value_as_json(original_wallet.clone(), Some(&struct_defs.clone().into())) - .unwrap(); + let json_value = serialize_value_as_json( + original_wallet.clone(), + Some(&struct_defs.clone().into()), + false, + ) + .unwrap(); let json_string = serde_json::to_string(&json_value).unwrap(); assert_eq!( json_string, diff --git a/crates/common/fmt/Cargo.toml b/crates/common/fmt/Cargo.toml index e9ca842748648..a5681bd97f952 100644 --- a/crates/common/fmt/Cargo.toml +++ b/crates/common/fmt/Cargo.toml @@ -32,3 +32,4 @@ yansi.workspace = true [dev-dependencies] foundry-macros.workspace = true similar-asserts.workspace = true +proptest.workspace = true diff --git a/crates/common/fmt/proptest-regressions/dynamic.txt b/crates/common/fmt/proptest-regressions/dynamic.txt new file mode 100644 index 0000000000000..9e16cd7d3eae1 --- /dev/null +++ b/crates/common/fmt/proptest-regressions/dynamic.txt @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 885aa25152cd93b8ddf5e98d7bfdc995d70d059b823b5589e793df41be92d9ce # shrinks to l = 0, h = 18446744073709551616 diff --git a/crates/common/fmt/src/dynamic.rs b/crates/common/fmt/src/dynamic.rs index 6388871c15c24..51d4fd216bc26 100644 --- a/crates/common/fmt/src/dynamic.rs +++ b/crates/common/fmt/src/dynamic.rs @@ -153,15 +153,20 @@ pub fn format_token_raw(value: &DynSolValue) -> String { pub fn serialize_value_as_json( value: DynSolValue, defs: Option<&StructDefinitions>, + strict: bool, ) -> Result { if let Some(defs) = defs { - _serialize_value_as_json(value, defs) + _serialize_value_as_json(value, defs, strict) } else { - _serialize_value_as_json(value, &StructDefinitions::default()) + _serialize_value_as_json(value, &StructDefinitions::default(), strict) } } -fn _serialize_value_as_json(value: DynSolValue, defs: &StructDefinitions) -> Result { +fn _serialize_value_as_json( + value: DynSolValue, + defs: &StructDefinitions, + strict: bool, +) -> Result { match value { DynSolValue::Bool(b) => Ok(Value::Bool(b)), DynSolValue::String(s) => { @@ -175,34 +180,38 @@ fn _serialize_value_as_json(value: DynSolValue, defs: &StructDefinitions) -> Res } DynSolValue::Bytes(b) => Ok(Value::String(hex::encode_prefixed(b))), DynSolValue::FixedBytes(b, size) => Ok(Value::String(hex::encode_prefixed(&b[..size]))), - DynSolValue::Int(i, _) => { - if let Ok(n) = i64::try_from(i) { - // Use `serde_json::Number` if the number can be accurately represented. - Ok(Value::Number(n.into())) - } else { + DynSolValue::Int(i, bits) => { + match (i64::try_from(i), strict) { + // In strict mode, return as number only if the type dictates so + (Ok(n), true) if bits <= 64 => Ok(Value::Number(n.into())), + // In normal mode, return as number if the number can be accurately represented. + (Ok(n), false) => Ok(Value::Number(n.into())), // Otherwise, fallback to its string representation to preserve precision and ensure // compatibility with alloy's `DynSolType` coercion. - Ok(Value::String(i.to_string())) + _ => Ok(Value::String(i.to_string())), } } - DynSolValue::Uint(i, _) => { - if let Ok(n) = u64::try_from(i) { - // Use `serde_json::Number` if the number can be accurately represented. - Ok(Value::Number(n.into())) - } else { + DynSolValue::Uint(i, bits) => { + match (u64::try_from(i), strict) { + // In strict mode, return as number only if the type dictates so + (Ok(n), true) if bits <= 64 => Ok(Value::Number(n.into())), + // In normal mode, return as number if the number can be accurately represented. + (Ok(n), false) => Ok(Value::Number(n.into())), // Otherwise, fallback to its string representation to preserve precision and ensure // compatibility with alloy's `DynSolType` coercion. - Ok(Value::String(i.to_string())) + _ => Ok(Value::String(i.to_string())), } } DynSolValue::Address(a) => Ok(Value::String(a.to_string())), DynSolValue::Array(e) | DynSolValue::FixedArray(e) => Ok(Value::Array( - e.into_iter().map(|v| _serialize_value_as_json(v, defs)).collect::>()?, + e.into_iter() + .map(|v| _serialize_value_as_json(v, defs, strict)) + .collect::>()?, )), DynSolValue::CustomStruct { name, prop_names, tuple } => { let values = tuple .into_iter() - .map(|v| _serialize_value_as_json(v, defs)) + .map(|v| _serialize_value_as_json(v, defs, strict)) .collect::>>()?; let mut map: HashMap = prop_names.into_iter().zip(values).collect(); @@ -222,7 +231,10 @@ fn _serialize_value_as_json(value: DynSolValue, defs: &StructDefinitions) -> Res Ok(Value::Object(map.into_iter().collect::>())) } DynSolValue::Tuple(values) => Ok(Value::Array( - values.into_iter().map(|v| _serialize_value_as_json(v, defs)).collect::>()?, + values + .into_iter() + .map(|v| _serialize_value_as_json(v, defs, strict)) + .collect::>()?, )), DynSolValue::Function(_) => eyre::bail!("cannot serialize function pointer"), } @@ -318,4 +330,76 @@ mod tests { "0xFb6916095cA1Df60bb79ce92cE3EA74c37c5d359" ); } + + proptest::proptest! { + #[test] + fn test_serialize_uint_as_json(l in 0u64..u64::MAX, h in ((u64::MAX as u128) + 1)..u128::MAX) { + let l_min_bits = (64 - l.leading_zeros()) as usize; + let h_min_bits = (128 - h.leading_zeros()) as usize; + + // values that fit in u64 should be serialized as a number in !strict mode + assert_eq!( + serialize_value_as_json(DynSolValue::Uint(l.try_into().unwrap(), l_min_bits), None, false).unwrap(), + serde_json::json!(l) + ); + // values that dont fit in u64 should be serialized as a string in !strict mode + assert_eq!( + serialize_value_as_json(DynSolValue::Uint(h.try_into().unwrap(), h_min_bits), None, false).unwrap(), + serde_json::json!(h.to_string()) + ); + + // values should be serialized according to the type + // since l_min_bits <= 64, expect the serialization to be a number + assert_eq!( + serialize_value_as_json(DynSolValue::Uint(l.try_into().unwrap(), l_min_bits), None, true).unwrap(), + serde_json::json!(l) + ); + // since `h_min_bits` is specified for the number `l`, expect the serialization to be a string + // even though `l` fits in a u64 + assert_eq!( + serialize_value_as_json(DynSolValue::Uint(l.try_into().unwrap(), h_min_bits), None, true).unwrap(), + serde_json::json!(l.to_string()) + ); + // since `h_min_bits` is specified for the number `h`, expect the serialization to be a string + assert_eq!( + serialize_value_as_json(DynSolValue::Uint(h.try_into().unwrap(), h_min_bits), None, true).unwrap(), + serde_json::json!(h.to_string()) + ); + } + + #[test] + fn test_serialize_int_as_json(l in 0i64..=i64::MAX, h in ((i64::MAX as i128) + 1)..=i128::MAX) { + let l_min_bits = (64 - (l as u64).leading_zeros()) as usize + 1; + let h_min_bits = (128 - (h as u128).leading_zeros()) as usize + 1; + + // values that fit in i64 should be serialized as a number in !strict mode + assert_eq!( + serialize_value_as_json(DynSolValue::Int(l.try_into().unwrap(), l_min_bits), None, false).unwrap(), + serde_json::json!(l) + ); + // values that dont fit in i64 should be serialized as a string in !strict mode + assert_eq!( + serialize_value_as_json(DynSolValue::Int(h.try_into().unwrap(), h_min_bits), None, false).unwrap(), + serde_json::json!(h.to_string()) + ); + + // values should be serialized according to the type + // since l_min_bits <= 64, expect the serialization to be a number + assert_eq!( + serialize_value_as_json(DynSolValue::Int(l.try_into().unwrap(), l_min_bits), None, true).unwrap(), + serde_json::json!(l) + ); + // since `h_min_bits` is specified for the number `l`, expect the serialization to be a string + // even though `l` fits in an i64 + assert_eq!( + serialize_value_as_json(DynSolValue::Int(l.try_into().unwrap(), h_min_bits), None, true).unwrap(), + serde_json::json!(l.to_string()) + ); + // since `h_min_bits` is specified for the number `h`, expect the serialization to be a string + assert_eq!( + serialize_value_as_json(DynSolValue::Int(h.try_into().unwrap(), h_min_bits), None, true).unwrap(), + serde_json::json!(h.to_string()) + ); + } + } }