feat: Allow lending transactions in Batch by tyalymov · Pull Request #8244 · XRPLF/rippled

tyalymov · 2026-09-17T20:52:54Z

High Level Overview of Change

Allows Single Asset Vault and Lending Protocol transactions to run as Batch inner transactions
when featureLendingProtocolV1_2 is enabled. Before the amendment, these transaction types remain
invalid inside a Batch.

Context of Change

Batch already applies inner transactions through the normal transaction pipeline, but explicitly
rejects all Vault and Lending transaction types.

This change gates that restriction on featureLendingProtocolV1_2. It also verifies the special
authorization flow for LoanSet, where both parties sign the outer Batch instead of using an
inner CounterpartySignature.

API Impact

libxrpl change (any change that may affect libxrpl or dependents of libxrpl)

Test Plan

Batch_test:

rejection of Vault and Lending inner transactions before the amendment;
successful Vault creation and deposit after the amendment;
required LoanSet counterparty and Batch signer validation;
successful LoanSet and cover deposit in one Batch;
rollback of a LoanSet when a later inner transaction fails.

New LendingBatch suite, each case run with LendingProtocolV1_1 on and off:

vault lifecycle in one Batch: VaultCreate, VaultDeposit, VaultWithdraw, VaultDelete,
on an open-ended vault and on a closed-ended vault in the Subscription phase;
loan lifecycle: VaultCreate, VaultDeposit, LoanBrokerSet, LoanSet, LoanPay. One Batch
on an open-ended vault with V1_1 off. Two Batches on a closed-ended vault with V1_1 on, split
at the Subscription to Investment boundary, because VaultDeposit and LoanSet are only
valid in different phases;
arbitrage under tfAllOrNothing: LoanSet, DEX buy, DEX sell, LoanPay. One run that
profits and one where the sell offer cannot fill and nothing persists;
LoanManage default and LoanBrokerCoverWithdraw in one Batch, once the grace period of a
loan created earlier has passed;
tfIndependent chains where a failing inner (VaultWithdraw, VaultDelete, LoanPay) does
not stop the inners after it.

Co-authored-by: Cursor <cursoragent@cursor.com>

xrplf-ai-reviewer

This is a small, well-scoped change that gates Batch's existing Vault/Lending inner-transaction rejection behind featureLendingProtocolV1_1. The logic in Batch.cpp is correct: pre-amendment it preserves the original any_of check against kDisabledTxTypes, post-amendment it bypasses that check entirely, matching the stated intent of allowing these transaction types as Batch inners once the amendment is enabled. The accompanying test updates (Batch_test.cpp, LoanLifecycle_test.cpp) consistently replace the old kDisabledTxTypes-based enablement checks with direct featureLendingProtocolV1_1 flag checks, and the new testLendingAmendment case exercises both the pre- and post-amendment paths for Vault create/deposit. No correctness, security, or resource-management issues were found in the changed lines; the removed includes (, LedgerFormats.h, TxFormats.h, Batch.h) line up with the removed std::ranges::any_of/kDisabledTxTypes usage in the test files.

codecov · 2026-09-17T20:55:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot

🟢 Approval recommended

The amendment gate and representative Vault/Loan Batch paths are covered without identified correctness issues.

Pull request overview

Allows Vault and Lending Protocol transactions inside Batch once featureLendingProtocolV1_1 is enabled, while preserving legacy rejection behavior.

Changes:

Gates Batch’s disabled transaction list on the lending amendment.
Adds pre/post-amendment Vault Batch coverage.
Updates LoanSet Batch tests to use the amendment feature flag.

File summaries

File	Description
`src/libxrpl/tx/transactors/system/Batch.cpp`	Applies amendment-gated inner transaction validation.
`src/test/app/Batch_test.cpp`	Tests Vault and Lending Batch behavior.
`src/test/app/lending/LoanLifecycle_test.cpp`	Updates LoanSet counterparty bypass coverage.

Review details

Files reviewed: 3/3 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…ing_batch_support

xrplf-ai-reviewer

The change cleanly gates the previously-hardcoded Batch inner-transaction rejection list (kDisabledTxTypes) behind featureLendingProtocolV1_2, correctly inverting the amendment check (isDisabledTxType is now false whenever the amendment is enabled, regardless of tx type). Test refactors consistently replace the old kDisabledTxTypes-based enablement checks with a direct features[featureLendingProtocolV1_2] flag, and the new testLendingAmendment test exercises both pre- and post-amendment Vault-in-Batch behavior. No correctness, security, or resource-leak issues were found in the added lines.

Copilot

Copilot review overview

🟢 Approval recommended

No unresolved review comments remain.

Review effort: Lite
Findings: None

Tapanito · 2026-09-23T12:25:38Z

+    void
+    testLendingAmendment(FeatureBitset features)
+    {
+        testcase("lending amendment");
+
+        using namespace test::jtx;
+
+        // Before LendingProtocolV1_2 a Vault inner transaction rejects the whole batch.
+        auto const checkVaultBatch = [this](FeatureBitset amendments) {
+            bool const lendingBatchEnabled = amendments[featureLendingProtocolV1_2];
+            Env env{*this, amendments};
+
+            Account const payer{"payer"};
+            Account const lender{"lender"};
+            env.fund(XRP(100'000), payer, lender);
+            env.close();
+
+            Vault const vault{env};
+            auto [create, vaultKeylet] = vault.create({.owner = lender, .asset = xrpIssue()});
+
+            auto const payerSeq = env.seq(payer);
+            auto const lenderSeq = env.seq(lender);
+            auto const batchFee = batch::calcBatchFee(env, 1, 2);
+            submitBatch(
+                env,
+                lendingBatchEnabled ? TER{tesSUCCESS} : TER{temINVALID_INNER_BATCH},
+                batch::outer(payer, payerSeq, batchFee, tfAllOrNothing),
+                batch::Inner(create, lenderSeq),
+                batch::Inner(
+                    vault.deposit(
+                        {.depositor = lender, .id = vaultKeylet.key, .amount = XRP(1'000)}),
+                    lenderSeq + 1),
+                batch::Sig(lender));
+            env.close();
+
+            BEAST_EXPECT(static_cast<bool>(env.le(vaultKeylet)) == lendingBatchEnabled);
+        };
+
+        checkVaultBatch(features - featureLendingProtocolV1_2);
+        checkVaultBatch(features);
+    }


Let's add more tests:

Full lifecycle, VaultCreate,Deposit,Withdraw,Delete

VaulCreate, Deposit, LoanBrokerCreate, LoanSet, LoanPay

@Tapanito Looks like we read the test scope differently, so here is what I propose to cover.
All with LendingProtocolV1_2 on, tfAllOrNothing unless noted:

VaultCreate → VaultDeposit → VaultWithdraw → VaultDelete

VaultCreate → VaultDeposit → LoanBrokerCreate → LoanSet → LoanPay
(borrower co-signs the batch, LoanPay is an early full payment)

Risk-free arbitrage from the original request: borrow → DEX buy → DEX sell → LoanPay.
One run succeeds, one where the sell offer does not fill and the whole batch rolls back.

LoanSet → LoanManage (default) → LoanBrokerCoverWithdraw

The same chains with tfIndependent, where one failing inner must not block the rest.

The existing pre-amendment check stays as is: a batch with a Vault inner is rejected with
temINVALID_INNER_BATCH. I don't plan to repeat it per transaction type.

Does this cover what you had in mind, or should I add or drop anything?

Yeah, the aim is to do a full integration test of vault/lending transactions and batch. Your plan sounds good!

Add the LendingBatch suite: vault lifecycle, loan lifecycle on open-ended and closed-ended vaults, arbitrage with rollback, default with cover withdraw, and tfIndependent chains, each run with LendingProtocolV1_1 on and off.

…ing_batch_support

xrplf-ai-reviewer

The core production change in Batch.cpp is a small, well-contained amendment gate: the pre-existing rejection of Vault/Lending inner transaction types is now skipped when featureLendingProtocolV1_2 is enabled. The logic is correct (short-circuiting && avoids the any_of scan once the amendment is live, and the early return is preserved). The remainder of the diff is test code (Batch_test.cpp simplification, new LendingBatch_test.cpp suite, LoanLifecycle_test.cpp cleanup) that exercises the new behavior thoroughly — vault lifecycle, loan lifecycle, arbitrage, independent-batch chains, and rollback scenarios all look internally consistent (sequence numbers, signer sets, and TER expectations line up with the described scenarios). I did not find any correctness, security, or resource-leak issues in the changed lines that meet the bar for flagging; the special LoanSet counterparty-via-outer-Batch-signature authorization logic mentioned in the MR description isn't present in this diff (it must live in code not shown/unchanged here), so there's nothing to verify there.

Copilot

Copilot review overview

🟡 Changes recommended

The arbitrage test must calculate fees for both Batch signers to avoid preflight rejection.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Calculate batch fee for both signers · New

+        auto const loanPayTx =
+            loan::pay(borrower, loanKeylet.key, principal.value(), tfLoanFullPayment);
+
+        auto const batchFee = batch::calcBatchFee(env, 1, 5);


gregtatcam

LGTM

xrplf-ai-reviewer

The core production change is a small, well-scoped amendment gate in Batch.cpp: the existing kDisabledTxTypes rejection for Vault/Lending inner transactions is now skipped when featureLendingProtocolV1_2 is enabled. The logic reads correctly (temINVALID_INNER_BATCH is only returned when the amendment is inactive AND the tx type is disabled), matching the described behavior and the pre-existing pattern of amendment-gating in preflight. The rest of the diff is test-only: Batch_test.cpp simplifies two ad-hoc checks to read the amendment flag directly (consistent with the removed kDisabledTxTypes-based checks and the now-unneeded algorithm/LedgerFormats includes), and a large new LendingBatch_test.cpp adds thorough coverage of vault/loan lifecycles, arbitrage, defaults, and tfIndependent chains as batch inners. I did not find correctness, security, or resource-management issues in the changed lines; the test logic (fee/balance math, keylet sequencing, signer sets) is internally consistent with the scenarios it documents.

Tapanito

I checked that the counterparty still has to sign the Batch for an inner LoanSet (LoanSet::preflight requires sfCounterparty on inners, and Batch::preflightSigValidated adds it to the required signers). The gate itself looks correct. The comments are mostly about test precision. The main question is why the impair inner in testLoan was dropped.

Tapanito · 2026-09-29T10:13:01Z

                        lenderSeq + 1),
-                    batch::Inner(manage(lender, loanKeylet.key, tfLoanImpair), lenderSeq + 2),
+                    batch::Inner(
+                        loan_broker::coverDeposit(lender, brokerKeylet.key, asset(100).value()),


The inner LoanManage(tfLoanImpair) was replaced with LoanBrokerCoverDeposit, and the lsfLoanImpaired check was removed. Why? Batch_test no longer runs impairment inside a Batch, and LendingBatch only covers the default path. If impair still works as an inner, can we keep it, or add it to LendingBatch?

Tapanito · 2026-09-29T10:13:01Z

-            {
-                BEAST_EXPECT(sleLoan->isFlag(lsfLoanImpaired));
-            }
+            BEAST_EXPECT(static_cast<bool>(env.le(loanKeylet)) == lendingBatchEnabled);


This only checks that the loan exists. Can we also check that the cover deposit inner applied, e.g. that sfCoverAvailable on the broker went up by 100 when lendingBatchEnabled is set?

Tapanito · 2026-09-29T10:13:01Z

+
+        if (auto const vaultSle = env.le(vaultKeylet); BEAST_EXPECT(vaultSle))
+            BEAST_EXPECT(vaultSle->at(sfAssetsTotal) == amount.value());
+        BEAST_EXPECT(env.seq(owner) == seq + 5);


The comments above describe specific inner failures (VaultWithdraw fails, VaultDelete returns tecHAS_OBLIGATIONS), but the test only checks the final state. Nothing shows that the withdraw inner actually failed. Could we assert each inner's result? Batch_test has validateInnerTxn for this. It could move into test/jtx/batch.h so both suites can use it.

Tapanito · 2026-09-29T10:13:01Z

+        env.close();
+
+        if (auto const loanSle = env.le(loanKeylet); BEAST_EXPECT(loanSle))
+            BEAST_EXPECT(loanSle->at(sfPrincipalOutstanding) == principal.value());


Same as the vault chain: can we assert that the LoanPay inner returned tecINSUFFICIENT_FUNDS and the cover deposit returned tesSUCCESS, instead of inferring it from the final state?

Tapanito · 2026-09-29T10:13:01Z

+    testIndependentLoanChain(FeatureBitset features)
+    {
+        // Same claim as testIndependentVaultChain, on a loan chain. The
+        // borrower starts with no IOU balance, so a LoanPay for twice the


Nit: the borrower holds the 1,000 principal by the time LoanPay runs. It fails because 2,000 is more than that balance, not because the balance is zero.

Tapanito · 2026-09-29T10:13:01Z

+    void
+    run() override
+    {
+        for (auto const& features : {all_, all_ | featureLendingProtocolV1_1})


This suite relies on all_ containing featureLendingProtocolV1_2, which is only true because testableAmendments() includes Supported::No amendments. Could we make it explicit (all_ | featureLendingProtocolV1_2)? Otherwise, if that changes, every case fails with temINVALID_INNER_BATCH and it's not obvious why.

Tapanito · 2026-09-29T10:13:01Z

-        {
+        // Pre-LendingProtocolV1_2: SAV and Lending transactions cannot be Batch inners.
+        // Post-LendingProtocolV1_2: they continue through the normal Batch checks.
+        bool const isDisabledTxType = !ctx.rules.enabled(featureLendingProtocolV1_2) &&


Nit: once V1_2 is enabled, this list only matters for replaying older ledgers. Maybe rename it to something like rejectedPreV1_2, and add a note on kDisabledTxTypes in Batch.h that it can go away when the amendment is retired.

Tapanito · 2026-09-29T10:14:45Z

rippled/src/libxrpl/tx/transactors/system/Batch.cpp

Lines 360 to 365 in 14f27e6

    
           // temINVALID_INNER_BATCH. Spec §6.1.1 names an inner Sponsor as a 
        
           // collectable slot, so forward TapProposal/TapDryRun. Always OR in 
        
           // TapBatch — PreflightContext with a parentBatchId requires it. 
        
           // LoanSet already short-circuits on tfInnerBatchTxn; it is also in 
        
           // kDisabledTxTypes, so it never reaches this call. 
        
           ApplyFlags const innerFlags = TapBatch | (ctx.flags & (TapProposal | TapDryRun));

With featureLendingProtocolV1_2, LoanSet can be a Batch inner, so this comment no longer holds. An inner LoanSet in a proposed Batch now gets TapProposal. It still needs sfCounterparty and the signer check is deferred to submission, so I don't see a bypass, but can we update the comment and add a test for a proposed Batch with a LoanSet inner?

feat: allow lending transactions in Batch

465a238

Co-authored-by: Cursor <cursoragent@cursor.com>

tyalymov requested review from Tapanito, a1q123456 and gregtatcam September 17, 2026 20:52

xrplf-ai-reviewer Bot approved these changes Sep 17, 2026

View reviewed changes

xrplf-bot requested a lite review from Copilot September 17, 2026 20:58

Copilot started reviewing on behalf of xrplf-bot September 17, 2026 20:58 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

tyalymov added 2 commits September 19, 2026 18:36

Merge remote-tracking branch 'origin/develop' into tialymov/FN-6-lend…

be00473

…ing_batch_support

fix: Gate lending transactions in Batch on LendingProtocolV1_2

1f3acb7

xrplf-ai-reviewer Bot approved these changes Sep 19, 2026

View reviewed changes

xrplf-bot requested a lite review from Copilot September 19, 2026 19:18

Copilot started reviewing on behalf of xrplf-bot September 19, 2026 19:18 View session

Copilot AI reviewed Sep 19, 2026

View reviewed changes

Tapanito added this to the 3.6.0 milestone Sep 23, 2026

Tapanito added the feature: LP1.2 label Sep 23, 2026

Tapanito requested changes Sep 23, 2026

View reviewed changes

tyalymov added 2 commits September 25, 2026 15:03

test: Cover Vault and Lending transactions as Batch inners

82c29e6

Add the LendingBatch suite: vault lifecycle, loan lifecycle on open-ended and closed-ended vaults, arbitrage with rollback, default with cover withdraw, and tfIndependent chains, each run with LendingProtocolV1_1 on and off.

Merge remote-tracking branch 'origin/develop' into tialymov/FN-6-lend…

280d3f0

…ing_batch_support

tyalymov requested a review from Tapanito September 25, 2026 16:44

xrplf-ai-reviewer Bot approved these changes Sep 25, 2026

View reviewed changes

xrplf-bot requested a lite review from Copilot September 25, 2026 16:57

Copilot started reviewing on behalf of xrplf-bot September 25, 2026 16:57 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread src/test/app/lending/LendingBatch_test.cpp

auto const loanPayTx =

loan::pay(borrower, loanKeylet.key, principal.value(), tfLoanFullPayment);

auto const batchFee = batch::calcBatchFee(env, 1, 5);

gregtatcam approved these changes Sep 25, 2026

View reviewed changes

Tapanito approved these changes Sep 29, 2026

View reviewed changes

Merge branch 'develop' into tialymov/FN-6-lending_batch_support

14f27e6

xrplf-ai-reviewer Bot approved these changes Sep 29, 2026

View reviewed changes

Tapanito requested changes Sep 29, 2026

View reviewed changes

Conversation

tyalymov commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

API Impact

Test Plan

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Sep 17, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

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!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

gregtatcam left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Tapanito left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tapanito commented Sep 29, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tyalymov commented Sep 17, 2026 •

edited

Loading