Skip to content

Expand Number to support the full integer range#6025

Merged
ximinez merged 1 commit intodevelopfrom
ximinez/lending-number-simple
Jan 13, 2026
Merged

Expand Number to support the full integer range#6025
ximinez merged 1 commit intodevelopfrom
ximinez/lending-number-simple

Conversation

@ximinez
Copy link
Copy Markdown
Collaborator

@ximinez ximinez commented Nov 12, 2025

Supersedes #6000.

  • Update the conversion points between Number and {XRP,MPT,IOU,ST}Amount & STNumber.

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@ximinez ximinez force-pushed the ximinez/lending-number-simple branch from 5cd623f to 4abb6d9 Compare November 13, 2025 05:52
@ximinez ximinez added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Nov 13, 2025
@ximinez ximinez force-pushed the ximinez/lending-number-simple branch 2 times, most recently from b17225c to c9ad49f Compare November 15, 2025 00:41
@ximinez ximinez mentioned this pull request Nov 16, 2025
13 tasks
@ximinez ximinez force-pushed the ximinez/lending-number-simple branch from 470c9c3 to 248d267 Compare November 16, 2025 18:10
@ximinez ximinez force-pushed the ximinez/lending-number-simple branch from 248d267 to 470c9c3 Compare November 16, 2025 22:22
Copy link
Copy Markdown
Collaborator

@Bronek Bronek left a comment

Choose a reason for hiding this comment

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

Pls fix compilation errors because several functions are no longer constexpr

@kennyzlei kennyzlei removed the request for review from shawnxie999 November 17, 2025 19:12
@ximinez ximinez removed the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Nov 19, 2025
@ximinez ximinez force-pushed the ximinez/lending-number-simple branch from ca041c0 to ae919ad Compare November 27, 2025 07:02
@ximinez ximinez changed the title DRAFT: Expand number Number to support the full integer range DRAFT: Expand Number to support the full integer range Dec 1, 2025
@ximinez ximinez added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Dec 1, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 2, 2025

Codecov Report

❌ Patch coverage is 96.03340% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.3%. Comparing base (2601442) to head (59b07d5).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
src/libxrpl/basics/Number.cpp 97.6% 5 Missing ⚠️
src/libxrpl/protocol/STAmount.cpp 72.2% 5 Missing ⚠️
src/xrpld/app/tx/detail/LoanSet.cpp 70.6% 5 Missing ⚠️
include/xrpl/basics/Number.h 97.2% 2 Missing ⚠️
src/libxrpl/protocol/Issue.cpp 0.0% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #6025    +/-   ##
========================================
  Coverage     79.2%   79.3%            
========================================
  Files          837     839     +2     
  Lines        71339   71609   +270     
  Branches      8273    8280     +7     
========================================
+ Hits         56524   56772   +248     
- Misses       14815   14837    +22     
Files with missing lines Coverage Δ
include/xrpl/protocol/AmountConversions.h 87.7% <100.0%> (ø)
include/xrpl/protocol/IOUAmount.h 100.0% <100.0%> (ø)
include/xrpl/protocol/Issue.h 100.0% <ø> (ø)
include/xrpl/protocol/MPTIssue.h 100.0% <ø> (ø)
include/xrpl/protocol/Protocol.h 100.0% <ø> (ø)
include/xrpl/protocol/SField.h 100.0% <ø> (ø)
include/xrpl/protocol/STAmount.h 96.0% <100.0%> (+0.3%) ⬆️
include/xrpl/protocol/STNumber.h 100.0% <ø> (ø)
include/xrpl/protocol/STTakesAsset.h 100.0% <100.0%> (ø)
include/xrpl/protocol/SystemParameters.h 100.0% <ø> (ø)
... and 26 more

... and 9 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ximinez ximinez changed the base branch from develop to ximinez/lending-XLS-66-ongoing December 4, 2025 17:33
@ximinez ximinez force-pushed the ximinez/lending-number-simple branch 5 times, most recently from e4e2ead to aecd7ab Compare December 10, 2025 01:15
@ximinez ximinez marked this pull request as ready for review December 10, 2025 17:34
@ximinez ximinez requested a review from a team December 10, 2025 17:34
@ximinez
Copy link
Copy Markdown
Collaborator Author

ximinez commented Dec 17, 2025

Want to make sure I understood this correctly: with largeRange, we can support full precision up to 2^63 - 1, so integer types (MPT or XRP) don’t lose precision even for large values. We’re still bounded by the signed int64 limit, but this works for existing integers since none of them exceed int64.

That is correct.

However, if we ever want to go beyond the 63-bit range (e.g., increasing MPT’s MaximumAmount to max(uint64) in the future), this implementation is not sufficient.

That is correct, however my current understanding is that there is no need or plan to extend the current limit.

Such a change would require an amendment, so a second expansion of Number would need to be included in that amendment work.

(Expounded in this comment: #6025 (comment))

{
// LCOV_EXCL_START
[[maybe_unused]]
auto const available = *assetsAvailableProxy;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why are available and total needed? They are not used.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Why are available and total needed? They are not used.

This whole block has since been replaced.

sleBroker->at(sfCoverAvailable) -= clawAmount;
view().update(sleBroker);

associateAsset(*sleBroker, vaultAsset);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I keep on thinking if there is a way to make this association table driven. There is already a new field property defined sMD_NeedsAsset. What if instead adding this property in sfields, it's added in ledger_entries. Then we just need a way to point it to the associated asset, which requires accessing up to two linked objects right now. What if we add an integral flag to each related ledger entry (Vault, LoanBroker, Loan) to indicate if an asset is integral? Then in STObject::applyTemplate(), which iterates over all fields , for any STNumber field with sMD_NeedsAsset, we can set the integral flag. Does it make sense to explore this approach?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I keep on thinking if there is a way to make this association table driven. There is already a new field property defined sMD_NeedsAsset. What if instead adding this property in sfields, it's added in ledger_entries. Then we just need a way to point it to the associated asset, which requires accessing up to two linked objects right now. What if we add an integral flag to each related ledger entry (Vault, LoanBroker, Loan) to indicate if an asset is integral? Then in STObject::applyTemplate(), which iterates over all fields , for any STNumber field with sMD_NeedsAsset, we can set the integral flag. Does it make sense to explore this approach?

Well, sMD_NeedsAsset is an SField-only property, so it would have to be set there. SOEStyle does not use bit flags, and isn't as flexible.

That aside, what I think you're suggesting is define a new LedgerSpecificFlags called lsfIntegralAsset or something, that could be used with Vault, LoanBroker, and Loan. Instead of needing to pull the Vault.Asset in every transaction, we would set that flag on creation of the Vault, and then read and set the flag when creating the derived objects.

Then instead of STTakesAsset, we'd check the Flags field in STObject::applyTemplate(), and if that flag is set, also set an isIntegral flag in every STNumber we come across that has the sMD_NeedsAsset property.

If the flag is set, STNumber would round to integer when serializing, and we're done.

It's a neat idea, and worth exploring. There are a couple of potential problems I can think of offhand:

  1. I don't think the flag could be global, because it would be difficult to keep it unique. Ledger flags doesn't have a concept of "universal flags" like transactions do. The reason that's significant is that applyTemplate would then have to understand
    1. that the object is a ledger object
    2. which type of ledger object it is
    3. which flag corresponds to "integral" for that ledger object type.
  2. I'm not entirely sure where in the lifecycle of an object applyTemplate is called. I think it's toward the beginning - when the object is created or read. Problem with that, as I just documented in 4605bbf is that setting the STNumber flag would have to be done after modifications are made and fields are set, because there's no STNumber to work with when the field is missing/unset. There may be a better call point just before serializing, so this isn't an insurmountable problem. (Maybe STObject::add.)
  3. It might take too long to implement and review this before we want to get this PR merged, and the amendments are enabled. It could be added later with another amendment, but then you have backwards compatibility to deal with. Unless we define the flags, and set them in their respective creation transactions, before enabling the amendments, then come back and update the implementation later... That seems hinky, though.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, maybe can think about it as an enhancement after the Lending Protocol is released. This is not going to require an amendment.

@ximinez ximinez force-pushed the ximinez/lending-number-simple branch from 451b474 to 565e62f Compare December 19, 2025 00:40
@ximinez ximinez force-pushed the ximinez/lending-number-simple branch from 8c64508 to 16ab78d Compare January 6, 2026 01:17
Copy link
Copy Markdown
Collaborator

@gregtatcam gregtatcam left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for adding extensive comments to the Number and MantissaRange classes.

Please add some documentation to STTakesAsset.h explaining why it's needed and how to use it. I'll approve once those are in!

@ximinez
Copy link
Copy Markdown
Collaborator Author

ximinez commented Jan 7, 2026

Want to make sure I understood this correctly: with largeRange, we can support full precision up to 2^63 - 1, so integer types (MPT or XRP) don’t lose precision even for large values. We’re still bounded by the signed int64 limit, but this works for existing integers since none of them exceed int64.

That is correct.

However, if we ever want to go beyond the 63-bit range (e.g., increasing MPT’s MaximumAmount to max(uint64) in the future), this implementation is not sufficient.

That is correct, however my current understanding is that there is no need or plan to extend the current limit.

Such a change would require an amendment, so a second expansion of Number would need to be included in that amendment work.

The updated class documentation now explicitly mentions that the limits for Number were chosen partially because of maxMPTokenAmount.

I've added a couple of static_asserts to ensure this invariant remains true in c88dd55

Copy link
Copy Markdown
Collaborator

@gregtatcam gregtatcam left a comment

Choose a reason for hiding this comment

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

LGTM

ximinez added a commit that referenced this pull request Jan 12, 2026
- Refactor Number internals away from int64 to uint64 & a sign flag
  - ctors and accessors return `rep`. Very few things expose
    `internalrep`.
  - An exception is "unchecked" and the new "normalized", which explicitly
    take an internalrep. But with those special control flags, it's easier
    to distinguish and control when they are used.

- For now, skip the larger mantissas in AMM transactions and tests

- Remove trailing zeros from scientific notation Number strings
  - Update tests. This has the happy side effect of making some of the string
    representations _more_ consistent between the small and large
    mantissa ranges.

- Add semi-automatic rounding of STNumbers based on Asset types
  - Create a new SField metadata enum, sMD_NeedsAsset, which indicates
    the field should be associated with an Asset so it can be rounded.
  - Add a new STTakesAsset intermediate class to handle the Asset
    association to a derived ST class. Currently only used in STNumber,
    but could be used by other types in the future.
  - Add "associateAsset" which takes an SLE and an Asset, finds the
    sMD_NeedsAsset fields, and associates the Asset to them. In the case
    of STNumber, that both stores the Asset, and rounds the value
    immediately.
  - Transactors only need to add a call to associateAsset _after_ all of
    the STNumbers have been set. Unfortunately, the inner workings of
    STObject do not do the association correctly with uninitialized
    fields.
  - When serializing an STNumber that has an Asset, round it before
    serializing.
  - Add an override of roundToAsset, which rounds a Number value in place
    to an Asset, but without any additional scale.
  - Update and fix a bunch of Loan-related tests to accommodate the
    expanded Number class.

---------

Co-authored-by: Vito <5780819+Tapanito@users.noreply.github.com>
@ximinez ximinez force-pushed the ximinez/lending-XLS-66-ongoing branch from c6fcce0 to b314566 Compare January 13, 2026 18:56
Base automatically changed from ximinez/lending-XLS-66-ongoing to develop January 13, 2026 19:43
- Refactor Number internals away from int64 to uint64 & a sign flag
  - ctors and accessors return `rep`. Very few things expose
    `internalrep`.
  - An exception is "unchecked" and the new "normalized", which explicitly
    take an internalrep. But with those special control flags, it's easier
    to distinguish and control when they are used.

- For now, skip the larger mantissas in AMM transactions and tests

- Remove trailing zeros from scientific notation Number strings
  - Update tests. This has the happy side effect of making some of the string
    representations _more_ consistent between the small and large
    mantissa ranges.

- Add semi-automatic rounding of STNumbers based on Asset types
  - Create a new SField metadata enum, sMD_NeedsAsset, which indicates
    the field should be associated with an Asset so it can be rounded.
  - Add a new STTakesAsset intermediate class to handle the Asset
    association to a derived ST class. Currently only used in STNumber,
    but could be used by other types in the future.
  - Add "associateAsset" which takes an SLE and an Asset, finds the
    sMD_NeedsAsset fields, and associates the Asset to them. In the case
    of STNumber, that both stores the Asset, and rounds the value
    immediately.
  - Transactors only need to add a call to associateAsset _after_ all of
    the STNumbers have been set. Unfortunately, the inner workings of
    STObject do not do the association correctly with uninitialized
    fields.
  - When serializing an STNumber that has an Asset, round it before
    serializing.
  - Add an override of roundToAsset, which rounds a Number value in place
    to an Asset, but without any additional scale.
  - Update and fix a bunch of Loan-related tests to accommodate the
    expanded Number class.

---------

Co-authored-by: Vito <5780819+Tapanito@users.noreply.github.com>
@ximinez ximinez force-pushed the ximinez/lending-number-simple branch from 4ec2527 to 59b07d5 Compare January 13, 2026 20:21
@ximinez
Copy link
Copy Markdown
Collaborator Author

ximinez commented Jan 13, 2026

As with the parent PR (#6102), this branch had missing signatures preventing merge. I squashed the commits down to one and pushed.

ximinez force-pushed the ximinez/lending-number-simple branch from 4ec2527 to 59b07d5

$ git diff 4ec2527 59b07d5 

$ git diff 4ec2527 59b07d5 | wc
      0       0       0

@ximinez ximinez enabled auto-merge (squash) January 13, 2026 20:25
@ximinez ximinez merged commit 33f4c92 into develop Jan 13, 2026
46 of 48 checks passed
@ximinez ximinez deleted the ximinez/lending-number-simple branch January 13, 2026 21:01
pratikmankawde added a commit that referenced this pull request Jan 21, 2026
refactor: Update Conan dependencies: protobuf and grpc (#5589)

This PR updates protobuf and grpc to their latest versions. The latest protobuf version no longer requires patches, so we can use it directly from the official Conan Center Index, while the latest grpc still needed a patch, which was added to our own Conan Center Index fork in XRPLF/conan-center-index#8.

cleanup

docs: Infer version of Conan dependency to export (#6112)

This change updates a script in the documentation to automatically infer the version of a patched Conan dependency from the conan.lock file.

chore: Use updated secp256k1 recipe (#6118)

This change updates the secp256k1 recipe that defines the SECP256K1_STATIC, so it no longer needs to be defined in the code here. Running the Conan update script also updated two other recipes in the lock file.

chore: Clean up .gitignore and .gitattributes (#6001)

The .gitignore and .gitattributes files contain references to files and directories that the current build no longer produces, so this change removes obsolete entries in these files, and does some general reorganizing of the remaining entries.

chore: Fix docs readme and cmake (#6122)

This change removes the unused `with_docs` option and fixes the README instructions on how to build the `docs` target.

removed amendment changes

added limit to reply size

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

minor clean-up

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

refactor: clean up `RPCHelpers` (#5684)

This PR cleans up `RPCHelpers.h` and `RPCHelpers.cpp`. It splits out all the fetch-ledger functions to a new set of files, `RPCLedgerHelpers.h`/`RPCLedgerHelpers.cpp`, and moves the general-API functions to `ApiVersion.h`. There is no functionality change.

refactor: rename `LedgerInfo` to `LedgerHeader` (#6136)

This PR renames `LedgerInfo` to `LedgerHeader`. Namely, `LedgerInfo` was already an alias for `LedgerHeader`, and the comments next to the alias suggested that it would make sense to rename it, since that makes it clearer what it is.

refactor: rename info() to header() (#6138)

This change renames all the `info()` functions to `header()`, since they return `LedgerHeader` structs. It also renames the underlying variables from `info_` to `header_`.

refactor: Rename `rippled` binary to `xrpld` (#5983)

Per [XLS-0095](https://xls.xrpl.org/xls/XLS-0095-rename-rippled-to-xrpld.html), we are taking steps to rename ripple(d) to xrpl(d).

This change modifies the binary name from `rippled` to `xrpld`, and creates a symlink named `rippled` that points to the `xrpld` binary.

Note that #5975 renamed any references to `rippled` in the CMake files and their contents, but explicitly maintained the `rippled` binary name by adding an exception. This change now undoes this exception and adds an explicit symlink instead.

subscription test was failing, so trying with longer timeout,

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

Update src/xrpld/overlay/detail/PeerImp.cpp

update comparison operator.

Co-authored-by: Ed Hennis <ed@ripple.com>

Update src/xrpld/overlay/detail/PeerImp.cpp

combine strings

Co-authored-by: Ed Hennis <ed@ripple.com>

refactor: Move JobQueue and related classes into xrpl.core module (#6121)

refactor: Rename `ripple` namespace to `xrpl` (#5982)

This change renames all occurrences of `namespace ripple` and `ripple::` to `namespace xrpl` and `xrpl::`, respectively, as well as the names of test suites. It also provides a script to allow developers to replicate the changes in their local branch or fork to avoid conflicts.

chore: Fix some typos in comments (#6082)

ci: Update shared actions (#6147)

The latest update to `cleanup-workspace`, `get-nproc`, and `prepare-runner` moved the action to the repository root directory, and also includes some ccache changes. In response, this change updates the various shared actions to the latest commit hash.

refactor: remove `Json::Object` and related files/classes (#5894)

`Json::Object` and related objects are not used at all, so this change removes `include/xrpl/json/Object.h` and all downstream files. There are a number of minor downstream changes as well.

Full list of deleted classes and functions:
* `Json::Collections`
* `Json::Object`
* `Json::Array`
* `Json::WriterObject`
* `Json::setArray`
* `Json::addObject`
* `Json::appendArray`
* `Json::appendObject`

The last helper function, `copyFrom`, seemed a bit more complex and was actually used in a few places, so it was moved to `LedgerToJson.h` instead of deleting it.

Set version to 3.2.0-b0 (#6153)

ci: Remove superfluous build directory creation (#6159)

This change modifies the build directory structure from `build/build/xxx` or `.build/build/xxx` to just `build/xxx`. Namely, the `conanfile.py` has the CMake generators build directory hardcoded to `build/generators`. We may as well leverage the top-level build directory without introducing another layer of directory nesting.

fix: Remove cryptographic libs from libxrpl Conan package (#6163)

* fix: rm crypto libs and fix protobuf path

* update/rm comments

chore: Pin ruamel.yaml<0.19 in pre-commit-hooks (#6166)

See pre-commit/pre-commit-hooks#1229 for more details.

Revert "chore: Pin ruamel.yaml<0.19 in pre-commit-hooks (#6166)" (#6167)

This reverts commit 0f23ad8.

refactor: Rename `rippled.cfg` to `xrpld.cfg` (#6098)

This change renames all occurrences of `rippled.cfg` to `xrpld.cfg`. It also provides a script to allow developers to replicate the changes in their local branch or fork to avoid conflicts. For the time being it maintains support for `rippled.cfg` as config file, if `xrpld.cfg` does not exist.

test: add more tests for `ledger_entry` RPC (#5858)

This change adds some basic tests for all the `ledger_entry` helper functions, so each ledger entry type is covered. There are further some minor refactors in `parseAMM` to provide better error messages. Finally, to improve readability, alphabetization was applied in the helper functions.

ci: Use ccache to cache build objects for speeding up building (#6104)

Right now, each pipeline invocation builds the source code from scratch. Although compiled Conan dependencies are cached in a remote server, the source build objects are not. We are able to further speed up our builds by leveraging `ccache`. This change enables caching of build objects using `ccache` on Linux, macOS, and Windows.

ci: Move variable into right place (#6179)

This change moves the `enable_ccache` variable in the `on-trigger.yml` file to the correct location.

refactor: Fix typos in comments, configure cspell (#6164)

This change sets up a `cspell `configuration and fixes lots of typos in comments. There are no other code changes.

refactor: Fix spelling issues in private/local variables and functions (#6182)

This change fixes several typos in private/local variables and private functions. There is no functionality change.

refactor: Fix spelling issues in all variables/functions (#6184)

This change fixes many typos in comments, variables, and public functions. There is no functionality change.

refactor: Remove unused credentials signature hash prefix (#6186)

This change removes the unused credentials signature hash prefix from `HashPrefix.h`.

fix: Reorder Batch Preflight Errors (#6176)

This change fixes #6058.

refactor: Fix typos, enable cspell pre-commit (#5719)

This change fixes the last of the spelling issues, and enables the pre-commit (and CI) check for spelling. There are no functionality changes, but it does rename some enum values.

ci: Use updated prepare-runner in actions and worfklows (#6188)

This change updates the XRPLF pre-commit workflow and prepare-runner action to their latest versions. For naming consistency the prepare-runner action changed the disable_ccache variable into enable_ccache, which matches our naming.

docs: Fix minor spelling issues in comments (#6194)

fix: Truncate thread name to 15 chars on Linux (#5758)

This change:
* Truncates thread names if more than 15 chars with `snprintf`.
* Adds warnings for truncated thread names if `-DTRUNCATED_THREAD_NAME_LOGS=ON`.
* Add a static assert for string literals to stop compiling if > 15 chars.
* Shortens `Resource::Manager` to `Resource::Mngr` to fix the static assert failure.
* Updates `CurrentThreadName_test` unit test specifically for Linux to verify truncation.

VaultClawback: Burn shares of an empty vault (#6120)

- Adds a mechanism for the vault owner to burn user shares when the vault is stuck. If the Vault has 0 AssetsAvailable and Total, the owner may submit a VaultClawback to reclaim the worthless fees, and thus allow the Vault to be deleted. The Amount must be left off (unless the owner is the asset issuer), specified as 0 Shares, or specified as the number of Shares held.

chore: Change `/Zi` to `/Z7` for ccache, remove debug symbols in CI (#6198)

As the `/Zi` compiler flag is unsupported by ccache, this change switches it to `/Z7` instead. For CI runs all debug info is omitted.

fix: Inner batch transactions never have valid signatures (#6069)

- Introduces amendment `fixBatchInnerSigs`
- Update Batch unit tests
  - Fix all the Env instantiations to _use_ the "features" parameter.
  - testInnerSubmitRPC runs with Batch enabled and disabled.
  - Add a test to testInnerSubmitRPC for a correctly signed tx incorrectly
    using the tfInnerBatchTxn flag.
  - Generalize the submitAndValidate lambda in testInnerSubmitRPC.
  - With the fix amendment, a transaction never reaches the transaction
    engine (Transactor and derived classes.)
  - Test submitting a pseudo-transaction. Stopped before reaching the
    transaction engine, but with different errors.
- The tests verify that without the amendment, a transaction with
  tfInnerBatchTxn is immediately rejected. Without the amendment, things
  are safe. The amendment just makes things safer and more future-proof.

chore: Pin pre-commit hooks to commit hashes (#6205)

This change updates and pins the Black and CSpell pre-commit hooks.

refactor: Remove unnecessary version number and options in cmake find_package (#6169)

This change removes unnecessary version numbers in the OpenSSL and Boost `find_package` CMake statements. An unnecessary OpenSSL definition is removed, while Conan options for SSL are updated to disable insecure ciphers. Moreover, the statements are now ordered alphabetically and more logically.

ci: Update actions/images to use cmake 4.2.1 and conan 2.24.0 (#6209)

fix: Update Conan lock file with changed OpenSSL recipe (#6211)

This change updates the `conan.lock` file with a changed OpenSSL recipe that contains a fix regarding options passed to the compiler

Improve and fix bugs in Lending Protocol (#6102)

- Spec: XLS-66

    Fix overpayment asserts (#6084)

    MPTTester::operator() parameter should be std::int64_t
    - Originally defined as uint64_t, but the testIssuerLoan() test called
      it with a negative number, causing an overflow to a very large number
      that in some circumstances could be silently cast back to an int64_t,
      but might not be. I believe this is UB, and we don't want to rely on
      that.

    Review feedback from @Tapanito: overpayment value change
    - In overpayment results, the management fee was being calculated twice:
      once as part of the value change, and as part of the fees paid.
      Exclude it from the value change.

    Fix Overpayment Calculation  (#6087)
    - Adds additional unit tests to cover math calculations.
    - Removes unused methods.

    Review feedback from @shawnxie999: even more rounding
    - Round the initial total value computation upward, unless there is
      0-interest.
    - Rename getVaultScale to getAssetsTotalScale, and convert one incorrect
      computation to use it.
    - Use adjustImpreciseNumber for LossUnrealized.
    - Add some logging to computeLoanProperties.

    Fix LoanBrokerSet debtMaximum limits (#6116)

    Fix some minor bugs in Lending Protocol (#6101)
    - add nodiscard to unimpairLoan, and check result in LoanPay
    - add a check to verify that issuer exists
    - improve LoanManage error code for dust amounts

    Check permissions in LoanSet and LoanPay (#6108)

    Disallow pseudo accounts to be Destination for LoanBrokerCoverWithdraw (#6106)

    Ensure vault asset cap is not exceeded (#6124)

    Fix Overpayment ValueChange calculation in Lending Protocol (#6114)
    - Adds loan state to LoanProperties.
    - Cleans up computeLoanProperties.
    - Fixes missing management fee from overpayment.

    fix: Enable LP Deposits when the broker is the asset issuer (#6119)
    * Replace accountHolds with accountSpendable when checking
    for account funds in VaultDeposit and LoanBrokerCoverDeposit

    Add a few minor changes (#6158)
    - Updates or fixes a couple of things I noticed while reviewing changes
      to the spec.
    - Rename sfPreviousPaymentDate to sfPreviousPaymentDueDate.
    - Make the vault asset cap check added in #6124 a little more robust:
      1. Check in preflight if the vault is _already_ over the limit.
      2. Prevent overflow when checking with the loan value. (Subtract
         instead of adding, in case the values are near maxint. Both return
         the same result. Also add a unit test so each case is covered.

    Add minimum grace period validation (#6133)

    Fix bugs: frozen pseudo-account, and FLC cutoff (#6170)

    refactor: Rename raw state to theoretical state (#6187)

    Check if a withdrawal amount exceeds any applicable receiving limit. (#6117)

    Fix overpayment result calculation (#6195)

    Address review feedback from Lending Protocol re-review (#6161)

---------

Co-authored-by: Gregory Tsipenyuk <gregtatcam@users.noreply.github.com>
Co-authored-by: Bronek Kozicki <brok@incorrekt.com>
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
Co-authored-by: Shawn Xie <35279399+shawnxie999@users.noreply.github.com>
Co-authored-by: Jingchen <a1q123456@users.noreply.github.com>

Expand Number to support the full integer range (#6025)

- Refactor Number internals away from int64 to uint64 & a sign flag
  - ctors and accessors use `rep`. Very few things expose
    `internalrep`.
  - An exception is "unchecked" and the new "normalized", which explicitly
    take an internalrep. But with those special control flags, it's easier
    to distinguish and control when they are used.

- For now, skip the larger mantissas in AMM transactions and tests

- Remove trailing zeros from scientific notation Number strings
  - Update tests. This has the happy side effect of making some of the string
    representations _more_ consistent between the small and large
    mantissa ranges.

- Add semi-automatic rounding of STNumbers based on Asset types
  - Create a new SField metadata enum, sMD_NeedsAsset, which indicates
    the field should be associated with an Asset so it can be rounded.
  - Add a new STTakesAsset intermediate class to handle the Asset
    association to a derived ST class. Currently only used in STNumber,
    but could be used by other types in the future.
  - Add "associateAsset" which takes an SLE and an Asset, finds the
    sMD_NeedsAsset fields, and associates the Asset to them. In the case
    of STNumber, that both stores the Asset, and rounds the value
    immediately.
  - Transactors only need to add a call to associateAsset _after_ all of
    the STNumbers have been set. Unfortunately, the inner workings of
    STObject do not do the association correctly with uninitialized
    fields.
  - When serializing an STNumber that has an Asset, round it before
    serializing.
  - Add an override of roundToAsset, which rounds a Number value in place
    to an Asset, but without any additional scale.
  - Update and fix a bunch of Loan-related tests to accommodate the
    expanded Number class.

---------

Co-authored-by: Vito <5780819+Tapanito@users.noreply.github.com>

Change LendingProtocol feature and dependencies to supported (#6146)

minor code review changes

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

test: Replace `failed` string in Vault test case (#6214)

The word `failed` in the test case makes it hard to search through the test logs when an actual test failure occurs, so this change renames the word to just `fail` instead.

test: Suppress "parse failed" message in Batch tests (#6207)

test: Use gtest instead of doctest (#6216)

This change switches over the doctest framework to the gtest framework.

ci: Add sanitizers to CI builds (#5996)

This change adds support for sanitizer build options in CI builds workflow. Currently `asan+ubsan` is enabled, while `tsan+ubsan` is left disabled as more changes are required.

added unit test

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

updated levelization

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

Improve ledger_entry lookups for fee, amendments, NUNL, and hashes (#5644)

These "fixed location" objects can be found in multiple ways:

1. The lookup parameters use the same format as other ledger objects, but the only valid value is true or the valid index of the object:
  - Amendments: "amendments" : true
  - FeeSettings: "fee" : true
  - NegativeUNL: "nunl" : true
  - LedgerHashes: "hashes" : true (For the "short" list. See below.)

2. With RPC API >= 3, using special case values to "index", such as "index" : "amendments". Uses the same names as above. Note that for "hashes", this option will only return the recent ledger hashes / "short" skip list.

3. LedgerHashes has two types: "short", which stores recent ledger hashes, and "long", which stores the flag ledger hashes for a particular ledger range.
  - To find a "long" LedgerHashes object, request '"hashes" : <ledger sequence>'. <ledger sequence> must be a number that evaluates to an unsigned integer.
  - To find the "short" LedgerHashes object, request "hashes": true as with the other fixed objects.

The following queries are all functionally equivalent:

  - "amendments" : true
  - "index" : "amendments" (API >=3 only)
  - "amendments" : "7DB0788C020F02780A673DC74757F23823FA3014C1866E72CC4CD8B226CD6EF4"
  - "index" : "7DB0788C020F02780A673DC74757F23823FA3014C1866E72CC4CD8B226CD6EF4"

Finally, whether the object is found or not, if a valid index is computed, that index will be returned. This can be used to confirm the query was valid, or to save the index for future use.

ci: remove 'master' branch as a trigger (#6234)

This change removes the `master` branch as a trigger for the CI pipelines, and updates comments accordingly. It also fixes the pre-commit workflow, so it will run on all release branches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants