Skip to content

Conversation

0xrusowsky
Copy link
Contributor

@0xrusowsky 0xrusowsky commented Aug 21, 2025

Motivation

the reported bug was originated by the fact that alloy's DynSolType coercion of serde_json::Value into U256 and I256, only supports those values which can be expressed as:

  • .as_u64()/.as_i64() for numbers within 64-bit range
  • .as_str() for string representations

see impl here: https://github.com/alloy-rs/core/blob/main/crates/dyn-abi/src/eip712/coerce.rs#L59-L77

however, since fn serialize_value_as_json() delegates json serialization to serde, large numbers (> u64::MAX) were being serialized .as_float(). This means that they would return None in the different coercion attempts of the fns described above, and end up throwing an error.

Solution

modified JSON serialization to preserve precision for large numbers:

  1. fn serialize_value_as_json(): rather than delegating json serialization to serde, check if numbers fit in i64/u64:
    • fit → serialize as serde_json::Number
    • don't fit → serialize as string to preserve exact value
  2. fn json_value_to_token(): needed to be adjusted to not break the fn test_json_roundtrip_guessed test. I tried keeping the functionality as close as possible to the original, by only parsing strings as I256/U256 when they exceed i64/u64 capacity
  3. fn parse_json_as(): Support string-to-number coercion for uint/int types

Despite i believe that the applied changes are correct + i tried to minimize the impact of the changes + the fact that they don't strictly break functionality, they alter the behavior and could impact some users.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes: no, but could change behavior in some cases

@0xrusowsky 0xrusowsky marked this pull request as ready for review August 21, 2025 17:08
mattsse
mattsse previously approved these changes Aug 25, 2025
Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
0xrusowsky and others added 4 commits August 25, 2025 10:07
Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
@0xrusowsky 0xrusowsky enabled auto-merge (squash) August 25, 2025 08:17
@0xrusowsky 0xrusowsky merged commit 7178915 into master Aug 25, 2025
22 checks passed
@0xrusowsky 0xrusowsky deleted the rusowsky/json-large-numbers branch August 25, 2025 08:35
@github-project-automation github-project-automation bot moved this to Done in Foundry Aug 25, 2025
@smol-ninja
Copy link

Thanks for doing it @0xrusowsky. In which release will it be included?

@grandizzy
Copy link
Collaborator

grandizzy commented Aug 25, 2025

this will be 1.4, we can consider adding it to a 1.3.3 bug fix release if we need to

MerkleBoy pushed a commit to MerkleBoy/foundry that referenced this pull request Sep 17, 2025
…y-rs#11396)

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

bug(cheatcodes): either serializeJsonType or eip712HashTypedData fails when dealing with large uint128 values
5 participants