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

Add serializeUintQuantity cheatcode #7758

Closed
2 tasks done
mothran opened this issue Apr 22, 2024 · 6 comments · Fixed by #7767
Closed
2 tasks done

Add serializeUintQuantity cheatcode #7758

mothran opened this issue Apr 22, 2024 · 6 comments · Fixed by #7767
Assignees
Labels
first issue A good way to start contributing T-bug Type: bug

Comments

@mothran
Copy link

mothran commented Apr 22, 2024

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (63fff35 2024-04-22T00:18:27.941506275Z)

What command(s) is the bug in?

forge script

Operating System

Linux

Describe the bug

When using serializeUint to serialize out deployment parameters (wallet address + private key) for end to end testing I noticed when serializing uint256's (from makeAddrAndKey) that the JSON output would be invalid and hard to parse with standard tools because serializeUint emits normal JSON integers instead of string-ifed integers in hex or integer representation. Because javascript's (and thus lots of JSON parsers) max int is 2^53 full uint256's can't be held within a normal integer type.

Example output from serializeUint to json:

  "test_key": 53286107159197333440670927965046399095550357839745059449004494585891422388798,

It would be nice if this was in hex form in a string (like how serializeAddress emits things).

@mothran mothran added the T-bug Type: bug label Apr 22, 2024
@mattsse
Copy link
Member

mattsse commented Apr 22, 2024

I see,
we still want a way to serialize numbers,
perhaps we should add serializeQuantity/Hex that serializes them as hex?

and consider renaming serializeUint into serializeNumber

@mothran
Copy link
Author

mothran commented Apr 22, 2024

Yeah that would make more sense in my mind, the easy work around is something like:

        vm.serializeBytes32(obj, "test_key", bytes32(test_key));

So might not be as much of a pain, but still worth considering.

@mattsse
Copy link
Member

mattsse commented Apr 22, 2024

wdyt @mds1 @DaniPopes ?

imo large uint should be serialized as quantity (hex: 0x..) because even stringified numbers are not ideal

@yash-atreya
Copy link
Member

wdyt @mds1 @DaniPopes ?

imo large uint should be serialized as quantity (hex: 0x..) because even stringified numbers are not ideal

+1 on serializing large uints as hex.

@mattsse mattsse added the first issue A good way to start contributing label Apr 22, 2024
@mattsse mattsse changed the title serializeUint creates invalid JSON Add serializeUintQuantity cheatcode Apr 22, 2024
@kamuik16
Copy link
Contributor

wdyt @mds1 @DaniPopes ?

imo large uint should be serialized as quantity (hex: 0x..) because even stringified numbers are not ideal

I can take this, if everyone is on board.

@mattsse
Copy link
Member

mattsse commented Apr 23, 2024

assigned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first issue A good way to start contributing T-bug Type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants