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.

Sign up for GitHub

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

Jump to bottom

Add support for 256 bit JSON numbers (vs. strings), including scientific notation #76

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Add support for 256 bit JSON numbers (vs. strings), including scientific notation #76

merged 3 commits into from
Aug 27, 2024

Conversation

matthew1001
Copy link
Contributor

@matthew1001 matthew1001 commented Aug 23, 2024 •
edited
Loading

This PR relates to firefly-common PR hyperledger/firefly-common#147

That PR introduces support for large JSON numbers where previously they would be limited to the size of float64 2^64-1. This is particularly useful when dealing with the scale of numeric parameters typical of Ethereum smart contracts. See PR hyperledger/firefly-common#147 for more information.

This firefly-signer PR does 2 things:

  1. Pulls in the latest version of firefly-common to utilize the new large number support
  2. Adds a fallback approach to parsing strings to handle scientific notation numbers e.g. 1e+25.

@matthew1001 matthew1001 changed the title Add unit tests for huge and scientific numbers Add support for 256 bit numbers, including scientific notation Aug 23, 2024
@matthew1001 matthew1001 marked this pull request as ready for review August 23, 2024 15:17
This was referenced Aug 23, 2024
Merged
Closed
@peterbroadhurst peterbroadhurst changed the title Add support for 256 bit numbers, including scientific notation Add support for 256 bit JSON numbers (vs. strings), including scientific notation Aug 23, 2024
@matthew1001
Pull in latest ff-common
3c6f71c 
Signed-off-by: Matthew Whitehead <[email protected]>
@matthew1001
Add unit test for json number
70863b7 
Signed-off-by: Matthew Whitehead <[email protected]>
This was referenced Aug 27, 2024
Merged
Merged
peterbroadhurst
peterbroadhurst approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks great - but I'm not sure it's complete in terms of all the places the types in firefly-signer are used.

So I will make a proposal of a follow-up PR to try and cover some more

@peterbroadhurst peterbroadhurst merged commit dc2c727 into hyperledger:main Aug 27, 2024
2 checks passed
@peterbroadhurst peterbroadhurst mentioned this pull request Aug 27, 2024
Merged
@matthew1001 matthew1001 mentioned this pull request Sep 2, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterbroadhurst peterbroadhurst peterbroadhurst approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@matthew1001 @peterbroadhurst