DRAFT: Expand number Number to support the full integer range #6025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Update the conversion points between Number and *Amount & STNumber. - Tests probably don't pass.
- Track down and fix edge cases. - Some refactoring and renaming for clarity and simplicity
ximinez
force-pushed
the
ximinez/lending-number-simple
branch
from
5cd623f to
4abb6d9
Compare
ximinez
added
the
DraftRunCI
- Field will be absent in RPC results instead of returning 0.
…number-simple * upstream/develop: chore: Clean up incorrect comments (6031) refactor: Retire MultiSignReserve and ExpandedSignerList amendments (5981)
- Update tests. Unfinished. - TODO: Finish Number tests. Use both modes for STNumber tests. Move mantissa_scale into MantissaRange.
- Fix cross-compiler build issues
ximinez
force-pushed
the
ximinez/lending-number-simple
branch
from
b1079c5 to
9310991
Compare
- Nothing really needed to be changed in the tests, but I added a couple of test cases for the min and max int64.
ximinez
force-pushed
the
ximinez/lending-number-simple
branch
from
b17225c to
c9ad49f
Compare
- Added test cases min int64. - Updated numberFromJson range checking to use the larger range available from Number.
- Default Number outside of transaction processing to be "large" so RPC will work.
ximinez
force-pushed
the
ximinez/lending-number-simple
branch
from
470c9c3 to
248d267
Compare
7 tasks
ximinez
force-pushed
the
ximinez/lending-number-simple
branch
from
248d267 to
470c9c3
Compare
Bronek
reviewed
Nov 17, 2025
Collaborator
Bronek
left a comment
Bronek
left a comment
There was a problem hiding this comment.
Pls fix compilation errors because several functions are no longer constexpr
| Number::lowest() noexcept | ||
| { | ||
| return -Number{maxMantissa, maxExponent, unchecked{}}; | ||
| return -Number{range_.get().max, maxExponent, unchecked{}}; |
Collaborator
There was a problem hiding this comment.
this function is no longer constexpr because it reads from static thread_local range_
| Number::max() noexcept | ||
| { | ||
| return Number{maxMantissa, maxExponent, unchecked{}}; | ||
| return Number{range_.get().max, maxExponent, unchecked{}}; |
Collaborator
There was a problem hiding this comment.
this function is no longer constexpr because it reads from static thread_local range_
| Number::min() noexcept | ||
| { | ||
| return Number{minMantissa, minExponent, unchecked{}}; | ||
| return Number{range_.get().min, minExponent, unchecked{}}; |
Collaborator
There was a problem hiding this comment.
this function is no longer constexpr because it reads from static thread_local range_
| inline constexpr bool | ||
| Number::isnormal() const noexcept | ||
| { | ||
| MantissaRange const& range = range_; |
Collaborator
There was a problem hiding this comment.
this function is no longer constexpr because it reads from static thread_local range_
Bronek
reviewed
Nov 17, 2025
| *this = y; | ||
| return *this; | ||
| } | ||
| XRPL_ASSERT( |
Collaborator
There was a problem hiding this comment.
This assert fails in AMM unit tests
ripple.app.AMM Basic Payment
rippled: ../src/libxrpl/basics/Number.cpp:451: ripple::Number& ripple::Number::operator*=(const ripple::Number&): Assertion `("ripple::Number::operator*=(Number) : is normal") && (isnormal() && y.isnormal())' failed.
[1] 364083 IOT instruction (core dumped) ./rippled -u AMM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Still in progress....
This work is on hold in favor of the solution in #6000.
If successful, this will supersede #6000.High Level Overview of Change
Context of Change
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)