⚡ Quick Guide: Lending Protocol implementation XLS-66 by ximinez · Pull Request #5270 · XRPLF/rippled · GitHub

ximinez · 2025-01-31T23:28:35Z

High Level Overview of Change

Implement the Lending Protocol as described in XLS-66

Context of Change

Type of Change

New feature (non-breaking change which adds functionality)
Refactor (non-breaking change that only restructures code)
Tests (you added tests for code that already exists, or your new feature included in this PR)

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)

codecov · 2025-02-04T20:14:56Z

Codecov Report

❌ Patch coverage is 90.63685% with 247 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.0%. Comparing base (c9f17dd) to head (3ca1593).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/xrpld/app/misc/detail/LendingHelpers.cpp	85.7%	78 Missing ⚠️
src/xrpld/app/tx/detail/InvariantCheck.cpp	89.0%	27 Missing ⚠️
src/xrpld/app/tx/detail/LoanManage.cpp	84.9%	27 Missing ⚠️
src/xrpld/app/tx/detail/LoanSet.cpp	91.4%	27 Missing ⚠️
src/xrpld/app/tx/detail/LoanBrokerDelete.cpp	72.5%	25 Missing ⚠️
src/xrpld/rpc/handlers/LedgerEntry.cpp	0.0%	20 Missing ⚠️
src/libxrpl/ledger/View.cpp	94.5%	16 Missing ⚠️
src/xrpld/app/tx/detail/LoanPay.cpp	95.5%	11 Missing ⚠️
...rc/xrpld/app/tx/detail/LoanBrokerCoverClawback.cpp	97.8%	3 Missing ⚠️
src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp	94.8%	3 Missing ⚠️
... and 6 more

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5270     +/-   ##
=========================================
+ Coverage     78.6%   79.0%   +0.5%     
=========================================
  Files          818     839     +21     
  Lines        68938   71377   +2439     
  Branches      8240    8339     +99     
=========================================
+ Hits         54177   56422   +2245     
- Misses       14761   14955    +194

Files with missing lines	Coverage Δ
include/xrpl/basics/Number.h	`100.0% <ø> (ø)`
include/xrpl/json/json_value.h	`98.5% <ø> (ø)`
include/xrpl/ledger/ApplyView.h	`100.0% <ø> (ø)`
include/xrpl/ledger/View.h	`100.0% <100.0%> (ø)`
include/xrpl/protocol/Asset.h	`96.3% <100.0%> (+0.8%)`	⬆️
include/xrpl/protocol/Indexes.h	`100.0% <100.0%> (ø)`
include/xrpl/protocol/Protocol.h	`100.0% <100.0%> (ø)`
include/xrpl/protocol/SField.h	`100.0% <ø> (ø)`
include/xrpl/protocol/STAmount.h	`95.7% <100.0%> (+1.2%)`	⬆️
include/xrpl/protocol/STObject.h	`93.1% <ø> (ø)`
... and 50 more

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gregtatcam · 2025-11-21T15:57:12Z

+    bool
+    integral() const
+    {
+        return !holds<Issue>() || get<Issue>().native();


Since we are not going to include Number in the release (if releasing unsupported) then should be done via commit against this branch or perhaps another PR?

ximinez

Posting a bunch of pending comments?

ximinez · 2025-11-21T21:43:09Z

            return {
                Validity::SigBad,
                "Malformed: Invalid inner batch transaction."};
-


Does removing this check have to be amendment-gated?

Ugh. I really want to say no, but it's called from Transactor::preflight, so to be safe, I guess it needs to be.

I've restored the code on this branch, and made a separate PR with a fix amendment to skip over the block: #6069

ximinez · 2025-11-21T21:57:42Z

+
+    Valid values are between 0 and 10% inclusive.
+*/
+TenthBips16 constexpr maxManagementFeeRate(


Should it be moved to a namespace to avoid potential name collisions in the future?

I'm having a hard time imagining what would use the names that wasn't lending related, but a namespace doesn't hurt, so I'll do that.

ximinez · 2025-11-21T22:26:52Z

+    {
+        // Fixed fields can not be specified if we're modifying an existing
+        // LoanBroker Object
+        if (tx.isFieldPresent(sfManagementFeeRate) ||


As sfVaultID is also not allowed to be modified, do we want to check if it presents?

The VaultID field is defined as required in the transaction, so it will be present. It just has to match.

ximinez · 2025-11-21T23:36:47Z

+        broker->at(sfVaultID) = vaultID;
+        broker->at(sfOwner) = account_;
+        broker->at(sfAccount) = pseudoId;
+        broker->at(sfLoanSequence) = 1;


Shouldn't it be the default value 0 as we don't have a Loan object at this moment?

No. In XRPL, sequence numbers are read then incremented, so we start with 1. 0 is an invalid sequence.

If you look at the pre-DeletableAccounts code, you'll see that new account roots were originally created with a sequence of 1. This just follows that same pattern.

ximinez · 2025-11-23T04:33:49Z

    }

+    // These values are only going to decrease, and can't be less than 0, so
+    // there's no need for integer range enforcement.


Leaving a comment here since the github doesn't let adding it to unchanged code. There is this code in doApply() just before the funds are transferred:

auto const dstAcct = ctx_.tx[~sfDestination].value_or(account_); if (!vaultAsset.native() && // dstAcct != vaultAsset.getIssuer() && // dstAcct == account_) { if (auto const ter = addEmptyHolding( view(), account_, mPriorBalance, vaultAsset, j_); !isTesSuccess(ter) && ter != tecDUPLICATE) return ter; }

There is no need to check for native() and non issuer account - addEmptyHolding() has these checks.

These extra checks have since been removed in another PR.

ximinez · 2025-11-23T04:37:14Z

+    return valueFields;
+}
+
+std::uint32_t


It should be static function.

I assume you're talking about getStartDate, in which case, this is fixed in 1ceff3d

shawnxie999 · 2025-11-28T19:46:39Z

    [[maybe_unused]] bool const enforce =
        view.rules().enabled(featureInvariantsV1_1) ||
-        view.rules().enabled(featureSingleAssetVault);
+        view.rules().enabled(featureSingleAssetVault) ||
+        view.rules().enabled(featureLendingProtocol);


If featureInvariantsV1_1 is enabled, but one node hasn’t upgraded to the rippled version that includes lending while another node has (and therefore runs the new invariant checks), wouldn’t they end up producing different transaction results? (I guess it would be okay since invaraints are meant to be defensive checks)

If featureInvariantsV1_1 is enabled, but one node hasn’t upgraded to the rippled version that includes lending while another node has (and therefore runs the new invariant checks), wouldn’t they end up producing different transaction results? (I guess it would be okay since invaraints are meant to be defensive checks)

Nope. That's not a problem, since the checks are joined by "or". In your scenario, assuming both nodes are running a version that supports featureInvariantsV1_1 (it's not supported in any version at the moment), then they would both run the invariant check and get identical results. (If one of them did not support it, it would be amendment blocked.)

To clarify, I'm taking about the scenario where:

featureInvariantsV1_1 is enabled across the network and featureLendingProtocol is not enabled

Node A supports featureInvariantsV1_1 but does not support featureLendingProtocol (didn't upgrade to latest rippled version)

Node B supports both featureInvariantsV1_1 and featureLendingProtocol (upgraded to latest rippled version)

Now with the invariant checks below introduced in lending, gated by enforce

If one of these new checks fails for a transaction, Node A would still succeed (since it runs on an older version and does not have these checks) but Node B would fail.

Anyways, since featureInvariantsV1_1 is not supported, this can be left for later discussion.

dangell7

non amendment guarded code review

shawnxie999

Approving the PR (non amendment gated portion) in its current state, given that we are keeping the lending feature unsupported for the time being.

gregtatcam

👍 LGTM

ximinez · 2025-12-02T00:42:19Z

@dangell7 I've addressed your comments. Could you re-review ASAP, so I can resolve the issue with unsigned commits, and get this thing merged?

dangell7

Approving ONLY non amendment gated code

- Spec: XLS-66 - Introduces amendment "featureLendingProtocol", but leaves it UNSUPPORTED to allow for future development work. - AccountInfo RPC will indicate the type of pseudo-account when appropriate. - Refactors and improves several existing classes and functional areas, including Number, STAmount, STObject, json_value, Asset, directory handling, View helper functions, and unit test helpers.

ximinez · 2025-12-02T16:28:59Z

Commit message:

Implement Lending Protocol (unsupported) (#5270)

- Spec: XLS-66
- Introduces amendment "LendingProtocol", but leaves it UNSUPPORTED to
  allow for standalone testing, future development work, and potential
  bug fixes.
- AccountInfo RPC will indicate the type of pseudo-account when
  appropriate.
- Refactors and improves several existing classes and functional areas,
  including Number, STAmount, STObject, json_value, Asset, directory
  handling, View helper functions, and unit test helpers.

bthomee · 2026-03-20T20:28:15Z

/ai-review

ximinez force-pushed the ximinez/lending-XLS-66 branch 4 times, most recently from 1240135 to 6f5d1ad Compare February 4, 2025 19:35

ximinez force-pushed the ximinez/lending-XLS-66 branch 3 times, most recently from 27e4c4f to 2d209c4 Compare February 8, 2025 00:58

ximinez force-pushed the ximinez/lending-XLS-66 branch 5 times, most recently from e8fbd22 to a697138 Compare February 28, 2025 19:24

ximinez force-pushed the ximinez/lending-XLS-66 branch 7 times, most recently from 72d979e to 441d5b5 Compare March 17, 2025 20:23

ximinez changed the base branch from develop to ximinez/Bronek/vault March 17, 2025 22:52

ximinez force-pushed the ximinez/Bronek/vault branch from b1e091c to 2a8861d Compare March 19, 2025 00:44

ximinez force-pushed the ximinez/lending-XLS-66 branch 8 times, most recently from 5223459 to c7b296c Compare March 25, 2025 16:30

gregtatcam reviewed Nov 21, 2025

View reviewed changes

Comment thread src/libxrpl/ledger/ApplyView.cpp

gregtatcam reviewed Nov 21, 2025

View reviewed changes

ximinez mentioned this pull request Nov 21, 2025

fix: Inner batch transactions never have valid signatures #6069

Merged

1 task

ximinez commented Nov 23, 2025

View reviewed changes

shawnxie999 reviewed Nov 25, 2025

View reviewed changes

Comment thread src/xrpld/app/tx/detail/LoanBrokerCoverClawback.cpp Outdated

shawnxie999 reviewed Nov 28, 2025

View reviewed changes

Comment thread src/xrpld/app/tx/detail/LoanSet.cpp

shawnxie999 reviewed Nov 28, 2025

View reviewed changes

Comment thread src/xrpld/app/tx/detail/LoanSet.cpp

shawnxie999 reviewed Nov 28, 2025

View reviewed changes

dangell7 reviewed Dec 1, 2025

View reviewed changes

Comment thread src/xrpld/app/tx/detail/Transactor.cpp

Comment thread src/libxrpl/ledger/ApplyView.cpp

Comment thread src/libxrpl/ledger/ApplyView.cpp Outdated

Comment thread src/libxrpl/ledger/ApplyView.cpp Outdated

This was referenced Dec 1, 2025

Placeholder to use as a base for the next lending PR #6092

Closed

Lending protocol Part 2 for re-review - XLS-66 #6093

Closed

shawnxie999 approved these changes Dec 1, 2025

View reviewed changes

gregtatcam approved these changes Dec 1, 2025

View reviewed changes

Tapanito approved these changes Dec 2, 2025

View reviewed changes

dangell7 approved these changes Dec 2, 2025

View reviewed changes

This was referenced Dec 12, 2025

Number: Change LendingProtocol and SAV features to supported #6146

Merged

(Release 3.1) Lending protocol - ongoing work - XLS-66 #6156

Merged

pdp2121 mentioned this pull request Mar 25, 2026

Support LendingProtocol (XLS-66) XRPLF/xrpl-rust#144

Open

Uh oh!

Conversation

ximinez commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

Type of Change

API Impact

Uh oh!

codecov Bot commented Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ximinez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shawnxie999 Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dangell7 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shawnxie999 left a comment

Choose a reason for hiding this comment

Uh oh!

gregtatcam left a comment

Choose a reason for hiding this comment

Uh oh!

ximinez commented Dec 2, 2025

Uh oh!

dangell7 left a comment

Choose a reason for hiding this comment

Uh oh!

ximinez commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bthomee commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

ximinez commented Jan 31, 2025 •

edited

Loading

codecov Bot commented Feb 4, 2025 •

edited

Loading

shawnxie999 Dec 1, 2025 •

edited

Loading

ximinez commented Dec 2, 2025 •

edited

Loading