From c35becfa9ce8ff62d6999a762995a8d1a7bbbf9b Mon Sep 17 00:00:00 2001 From: JCW Date: Tue, 18 Aug 2026 17:45:45 +0100 Subject: [PATCH] Add tests and self review --- src/libxrpl/tx/invariants/InvariantCheck.cpp | 9 +- .../tx/transactors/lending/LoanAccept.cpp | 38 +- .../tx/transactors/lending/LoanDelete.cpp | 13 +- .../tx/transactors/lending/LoanSet.cpp | 28 +- src/test/app/lending/LoanLifecycle_test.cpp | 37 +- src/test/app/lending/LoanTwoStep_test.cpp | 355 ++++++++++++++++-- src/test/app/lending/LoanValidation_test.cpp | 41 +- src/test/app/lending/Loan_test.cpp | 3 +- 8 files changed, 447 insertions(+), 77 deletions(-) diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index b4b741fbd3..8e22706ed3 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -1148,7 +1148,6 @@ NoModifiedUnmodifiableFields::finalize( break; case ltLOAN: bad = bad || kFieldChanged(before, after, sfSequence) || - kFieldChanged(before, after, sfOwnerNode) || kFieldChanged(before, after, sfLoanBrokerNode) || kFieldChanged(before, after, sfLoanBrokerID) || kFieldChanged(before, after, sfBorrower) || @@ -1165,7 +1164,13 @@ NoModifiedUnmodifiableFields::finalize( kFieldChanged(before, after, sfPaymentInterval) || kFieldChanged(before, after, sfGracePeriod) || kFieldChanged(before, after, sfLoanScale); - if (!view.rules().enabled(featureLendingProtocolV1_1)) + // Pre-V1.1, sfOwnerNode is set at loan creation and immutable + // thereafter. V1.1 introduces the two-step flow: a pending + // loan is created without sfOwnerNode and LoanAccept adds it + // when the borrower accepts. Allow only that specific + // transition; any other tx modifying sfOwnerNode is a bug. + if (!view.rules().enabled(featureLendingProtocolV1_1) || + tx.getTxnType() != ttLOAN_ACCEPT) { bad = bad || kFieldChanged(before, after, sfOwnerNode); } diff --git a/src/libxrpl/tx/transactors/lending/LoanAccept.cpp b/src/libxrpl/tx/transactors/lending/LoanAccept.cpp index e25acfca62..ea23881147 100644 --- a/src/libxrpl/tx/transactors/lending/LoanAccept.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanAccept.cpp @@ -30,6 +30,7 @@ LoanAccept::checkExtraFeatures(PreflightContext const& ctx) NotTEC LoanAccept::preflight(PreflightContext const& ctx) { + // 3.9.3.1.1 LoanID is zero. (temINVALID) if (ctx.tx[sfLoanID] == beast::kZero) return temINVALID; @@ -44,24 +45,30 @@ LoanAccept::preclaim(PreclaimContext const& ctx) auto const loanID = tx[sfLoanID]; auto const loanSle = ctx.view.read(keylet::loan(loanID)); + // 3.9.3.2.1 The Loan object with the specified LoanID does not exist on the ledger. + // (tecNO_ENTRY) if (!loanSle) { JLOG(ctx.j.warn()) << "Loan does not exist."; return tecNO_ENTRY; } + // 3.9.3.2.2 The Loan object does not have the lsfLoanPending flag set. (tecNO_PERMISSION) if (!isLoanPending(loanSle)) { JLOG(ctx.j.warn()) << "Loan is not pending acceptance."; return tecNO_PERMISSION; } + // 3.9.3.2.3 The Account submitting the transaction is not the Loan.Borrower. (tecNO_PERMISSION) if (loanSle->at(sfBorrower) != account) { JLOG(ctx.j.warn()) << "LoanAccept can only be submitted by the Borrower."; return tecNO_PERMISSION; } + // 3.9.3.2.4 The current ledger timestamp is greater than or equal to Loan.StartDate (the + // proposal has expired). (tecEXPIRED) if (hasExpired(ctx.view, loanSle->at(sfStartDate))) { JLOG(ctx.j.warn()) << "Loan proposal has expired."; @@ -80,6 +87,15 @@ LoanAccept::preclaim(PreclaimContext const& ctx) Asset const asset = vaultSle->at(sfAsset); auto const vaultPseudo = vaultSle->at(sfAccount); + // 3.9.3.2.6 The Vault pseudo-account is frozen for the asset. (tecFROZEN for IOUs, tecLOCKED + // for MPTs) + // 3.9.3.2.7 The LoanBroker pseudo-account is deep frozen for the asset. (tecFROZEN for IOUs, + // tecLOCKED for MPTs) + // 3.9.3.2.8 The Borrower is frozen for the asset. (tecFROZEN for IOUs, tecLOCKED for MPTs) + // 3.9.3.2.9 The LoanBroker.Owner is deep frozen for the asset. (tecFROZEN for IOUs, tecLOCKED + // for MPTs) + // 3.9.3.2.10 Cannot add asset holding for the Vault.Asset (e.g., MPToken or TrustLine issues). + // (tecNO_PERMISSION) if (auto const ter = checkLoanFreeze( ctx.view, asset, vaultPseudo, brokerPseudo, account, brokerOwner, ctx.j)) return ter; @@ -88,8 +104,10 @@ LoanAccept::preclaim(PreclaimContext const& ctx) // receive funds at disbursement) are authorised to hold the vault asset. // WeakAuth is used because the holdings need not exist yet; they are // created at disbursement. + // 3.9.3.2.11 The Borrower is not authorized for the asset. (tecNO_AUTH) if (auto const ter = requireAuth(ctx.view, asset, account, AuthType::WeakAuth)) return ter; + // 3.9.3.2.12 The LoanBroker.Owner is not authorized for the asset. (tecNO_AUTH) if (auto const ter = requireAuth(ctx.view, asset, brokerOwner, AuthType::WeakAuth)) return ter; @@ -130,19 +148,23 @@ LoanAccept::doApply() Number const originationFee = loanSle->at(sfLoanOriginationFee); auto const loanAssetsToBorrower = principalOutstanding - originationFee; - // The loan is no longer pending; it becomes active. + // 3.9.4.1 Clear the lsfLoanPending flag on the Loan object. loanSle->clearFlag(lsfLoanPending); - auto applyViewContext = ctx_.getApplyViewContext(); - // Release the owner reserve that was charged to the LoanBroker.Owner when - // the loan was proposed, and charge it to the borrower instead. + // 3.9.4.2 Release the reserve from the Loan Broker: Decrement + // AccountRoot(LoanBroker.Owner).OwnerCount by 1. decreaseOwnerCount(view, brokerOwnerSle, {}, 1, j_); + + // 3.9.4.3 Charge the reserve to the Borrower: Increment AccountRoot(Borrower).OwnerCount by 1. + // 3.9.3.2.5 The Borrower does not have sufficient reserve for the Loan object. + // (tecINSUFFICIENT_RESERVE) if (auto const ter = reserveLoanOwner(view, borrower, borrowerSle, accountID_, preFeeBalance_, j_)) return ter; - // Disburse the principal to the borrower and the origination fee, if any, - // to the broker owner. + // 3.9.4.4 - 3.9.4.6 Disburse the principal to the borrower and the origination fee, if any, to + // the broker owner. + auto applyViewContext = ctx_.getApplyViewContext(); if (auto const ter = disburseLoan( applyViewContext, borrowerSle, @@ -156,12 +178,12 @@ LoanAccept::doApply() j_)) return ter; - // Release the reserved principal now that it has been paid out. auto vaultAssetReservedProxy = vaultSle->at(sfAssetsReserved); + // 3.9.4.7 Update Vault object: Decrease Vault.AssetsReserved by Loan.PrincipalOutstanding. vaultAssetReservedProxy -= principalOutstanding; view.update(vaultSle); - // Make the borrower the owner of the loan. + // 3.9.4.8 Make the borrower the owner of the loan. if (auto const ter = dirLink(view, borrower, loanSle, sfOwnerNode)) return ter; view.update(loanSle); diff --git a/src/libxrpl/tx/transactors/lending/LoanDelete.cpp b/src/libxrpl/tx/transactors/lending/LoanDelete.cpp index 0f894c9c46..a280032b75 100644 --- a/src/libxrpl/tx/transactors/lending/LoanDelete.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanDelete.cpp @@ -45,30 +45,31 @@ deletePendingLoan( Number const principalOutstanding = loanSle->at(sfPrincipalOutstanding); auto const state = constructLoanState(loanSle); - // Remove LoanID from the broker pseudo-account's directory. + // 3.10.4.1.1 Remove LoanID from the broker pseudo-account's directory. if (!view.dirRemove( keylet::ownerDir(brokerPseudoAccount), loanSle->at(sfLoanBrokerNode), loanID, false)) return tefBAD_LEDGER; // LCOV_EXCL_LINE - // Delete the Loan object + // 3.10.4.1.2 Delete the Loan object view.erase(loanSle); - // Reverse the vault bookkeeping from the proposal. + // 3.10.4.1.3 Reverse the vault bookkeeping from the proposal. vaultSle->at(sfAssetsAvailable) += principalOutstanding; vaultSle->at(sfAssetsReserved) -= principalOutstanding; vaultSle->at(sfAssetsTotal) -= state.interestDue; view.update(vaultSle); - // Reverse the broker debt and outstanding loan count. + // 3.10.4.1.4 Reverse the broker debt and outstanding loan count. adjustImpreciseNumber( brokerSle->at(sfDebtTotal), -(principalOutstanding + state.interestDue), vaultAsset, vaultScale); + // 3.10.4.1.4 Decrement LoanBroker.OwnerCount by 1. adjustLoanBrokerOwnerCount(view, brokerSle, -1, j); - // Release the owner reserve charged to the LoanBroker owner when the - // loan was proposed. + // 3.10.4.1.5 Release the reserve from the Loan Broker: Decrement + // AccountRoot(LoanBroker.Owner).OwnerCount by 1. decreaseOwnerCount(view, brokerOwnerSle, {}, 1, j); associateAsset(*brokerSle, vaultAsset); diff --git a/src/libxrpl/tx/transactors/lending/LoanSet.cpp b/src/libxrpl/tx/transactors/lending/LoanSet.cpp index 41b8862f2f..631845b60d 100644 --- a/src/libxrpl/tx/transactors/lending/LoanSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanSet.cpp @@ -630,17 +630,14 @@ LoanSet::preflight(PreflightContext const& ctx) return temINVALID_FLAG; } - // Special case for Batch inner transactions. A Batch inner LoanSet - // must identify the borrower explicitly, since the inner transaction - // cannot carry a CounterpartySignature. That means either a Counterparty - // (immediate flow) or, once V1.1 enables it, a Borrower (two-step flow). + // 3.8.5.1.3 The transaction is a Batch inner transaction and the Counterparty field is not + // specified and the Borrower field is not specified. (temBAD_SIGNER) if (tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatchV1_1) && - !tx.isFieldPresent(sfCounterparty) && - !(isTwoStepFlowEnabled(ctx.rules) && tx.isFieldPresent(sfBorrower))) + !tx.isFieldPresent(sfCounterparty) && !tx.isFieldPresent(sfBorrower)) { auto const parentBatchId = ctx.parentBatchId.value_or(uint256{0}); JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: " - << "no Counterparty or Borrower for inner LoanSet transaction."; + << "no Counterparty for inner LoanSet transaction."; return temBAD_SIGNER; } @@ -651,16 +648,19 @@ LoanSet::preflight(PreflightContext const& ctx) return std::nullopt; }(); + // 3.8.5.1.2 CounterpartySignature is not present and the transaction is not part of a Batch + // inner transaction and the Borrower field is not specified. (temBAD_SIGNER) + if (!counterPartySig && !tx.isFieldPresent(sfBorrower)) + { + JLOG(ctx.j.warn()) << "LoanSet transaction must have a CounterpartySignature."; + return temBAD_SIGNER; + } + bool const twoStepFlowEnabled = isTwoStepFlowEnabled(ctx.rules); + // 3.8.5.1.5 Both Borrower and Counterparty fields are specified. (temINVALID) + // 3.8.5.1.6 Both Borrower and CounterpartySignature fields are specified. (temINVALID) if (getLoanFlow(tx, twoStepFlowEnabled) == LoanFlow::Invalid) { - // Before the two-step (Borrower) flow was introduced by V1.1, a - // CounterpartySignature was mandatory for every non-batch transaction. - if (!twoStepFlowEnabled) - { - JLOG(ctx.j.warn()) << "LoanSet transaction must have a CounterpartySignature."; - return temBAD_SIGNER; - } JLOG(ctx.j.warn()) << "LoanSet transaction must specify either a Borrower with a " "StartDate or a CounterpartySignature."; return temINVALID; diff --git a/src/test/app/lending/LoanLifecycle_test.cpp b/src/test/app/lending/LoanLifecycle_test.cpp index 5b4260a28c..10865eb557 100644 --- a/src/test/app/lending/LoanLifecycle_test.cpp +++ b/src/test/app/lending/LoanLifecycle_test.cpp @@ -593,11 +593,12 @@ private: BEAST_EXPECT(objects.size() == 0); } - // A Batch inner LoanSet with no Counterparty (and no Borrower) - // is rejected in preflight with temBAD_SIGNER. Inside a Batch, - // the immediate flow still applies but the inner transaction - // cannot carry a CounterpartySignature, so the Counterparty - // must be named explicitly on the inner transaction. + // XLS-66 spec 3.8.5.2.1 (Batch-inner refinement): a Batch inner + // LoanSet with no Counterparty and no Borrower is rejected with + // temBAD_SIGNER in preflight. Inside a Batch, the immediate flow + // still applies but the inner transaction cannot carry a + // CounterpartySignature, so the Counterparty must be named + // explicitly on the inner transaction. { auto const jtx = env.jt(set(lender, broker.brokerID, principalRequest), Txflags(tfInnerBatchTxn)); @@ -609,9 +610,9 @@ private: } } - // Once V1.1 enables the two-step flow, a Batch inner LoanSet may - // name a Borrower (with a StartDate) instead of a Counterparty: - // the borrower is identified explicitly on the inner tx and no + // XLS-66 flow (Batch + V1.1): a Batch inner LoanSet may name a + // Borrower (with a StartDate) instead of a Counterparty: the + // borrower is identified explicitly on the inner tx and no // CounterpartySignature is required. Preflight must accept it. if (features[featureLendingProtocolV1_1]) { @@ -627,10 +628,11 @@ private: BEAST_EXPECT(Transactor::invokePreflight(pfCtx) == tesSUCCESS); } - // A Batch inner LoanSet with Borrower but no StartDate is not a - // valid two-step proposal and no longer masquerades as a - // missing-Counterparty error: it is rejected as temINVALID by - // getLoanFlow, past the Batch-specific check. + // XLS-66 flow (Batch + V1.1): a Batch inner LoanSet with + // Borrower but no StartDate is not a valid two-step proposal + // and no longer masquerades as a missing-Counterparty error: + // it is rejected as temINVALID by getLoanFlow, past the + // Batch-specific check. auto const jtxNoStart = env.jt( set(lender, broker.brokerID, principalRequest), Txflags(tfInnerBatchTxn), @@ -648,11 +650,12 @@ private: } } - // Success: a Batch containing an inner LoanSet that names a - // Counterparty (but carries no CounterpartySignature) is accepted - // when the counterparty signs the outer Batch. The immediate flow's - // counterparty consent is satisfied by the batch signature rather - // than an inner CounterpartySignature. Requires both the Batch and + // XLS-66 flow (Batch + V1.1) success: a Batch containing an inner + // LoanSet that names a Counterparty (but carries no + // CounterpartySignature) is accepted when the counterparty signs + // the outer Batch. The immediate flow's counterparty consent is + // satisfied by the batch signature rather than an inner + // CounterpartySignature. Requires both the Batch and // LendingProtocolV1_1 amendments. if (features[featureLendingProtocolV1_1] && lendingBatchEnabled) { diff --git a/src/test/app/lending/LoanTwoStep_test.cpp b/src/test/app/lending/LoanTwoStep_test.cpp index 5f40602ddb..3f2c2de7f1 100644 --- a/src/test/app/lending/LoanTwoStep_test.cpp +++ b/src/test/app/lending/LoanTwoStep_test.cpp @@ -3,11 +3,18 @@ #include #include #include +#include +#include #include +#include #include #include +#include +#include +#include #include #include +#include #include #include @@ -23,9 +30,12 @@ #include #include #include +#include +#include #include #include +#include namespace xrpl::test { @@ -138,7 +148,7 @@ private: extra...); }; - // Per spec §4.3, a failed LoanAccept must leave the pending Loan + // Per spec 4.3, a failed LoanAccept must leave the pending Loan // intact so the borrower can rectify the issue and retry until the // StartDate expires. auto const expectStillPending = [this](Env& env, Keylet const& k) { @@ -165,14 +175,14 @@ private: (env.now() + 1h).time_since_epoch().count(), Ter(temDISABLED)); - // A LoanSet with no CounterpartySignature, not inside a Batch - // inner transaction, and with no Borrower field is rejected as - // before, because the immediate flow still requires a - // CounterpartySignature. + // XLS-66 spec 3.8.5.2.1: CounterpartySignature is not present + // (temBAD_SIGNER). With V1.1 disabled, the immediate flow still + // requires a CounterpartySignature; no Batch inner, no Borrower. env(set(lender, broker.brokerID, broker.asset(200).number()), Ter(temBAD_SIGNER)); - // LoanAccept is introduced by the two-step amendment, so with the - // amendment disabled the transaction type itself is rejected. + // XLS-66 amendment gate: LoanAccept is introduced by + // featureLendingProtocolV1_1, so with the amendment disabled the + // transaction type itself is rejected (temDISABLED). env(accept(borrower, keylet::loan(broker.brokerID, SeqProxy::rawSequence(1)).key), Ter(temDISABLED)); @@ -429,7 +439,7 @@ private: auto const epoch = env.now(); auto const broker = makeBroker(env, AssetType::XRP); - // The submitter must be the LoanBroker owner. + // XLS-66 spec 3.8.5.3.1: Account != LoanBroker.Owner (tecNO_PERMISSION). // A StartDate comfortably in the future. propose( env, @@ -439,35 +449,36 @@ private: (env.now() + 1h).time_since_epoch().count(), Ter(tecNO_PERMISSION)); - // The StartDate must be in the future. + // XLS-66 flow: two-step preclaim rejects a past StartDate (tecEXPIRED). std::uint32_t const pastDate = epoch.time_since_epoch().count(); propose(env, broker, lender, borrower, pastDate, Ter(tecEXPIRED)); - // A LoanSet with no CounterpartySignature, not inside a Batch - // inner transaction, and with no Borrower field matches neither - // the one-step nor the two-step (Borrower) flow. + // XLS-66 flow: no CounterpartySignature, no Borrower, not a Batch + // inner: matches neither one-step nor two-step (temINVALID with + // V1.1 enabled; temBAD_SIGNER without, exercised earlier). env(set(lender, broker.brokerID, broker.asset(200).number()), Ter(temINVALID)); - // A LoanSet with Borrower but no StartDate matches neither the - // one-step nor the two-step (Borrower) flow. + // XLS-66 flow: Borrower without StartDate is not a valid two-step + // proposal (temINVALID). env(set(lender, broker.brokerID, broker.asset(200).number()), kBorrower(borrower), Ter(temINVALID)); - // A LoanSet with StartDate but no Borrower matches neither the - // one-step nor the two-step (Borrower) flow. + // XLS-66 flow: StartDate without Borrower is not a valid two-step + // proposal (temINVALID). env(set(lender, broker.brokerID, broker.asset(200).number()), kStartDate((env.now() + 1h).time_since_epoch().count()), Ter(temINVALID)); - // A LoanSet with Borrower and Counterparty is ambiguous. + // XLS-66 flow: Borrower + Counterparty is ambiguous (temINVALID). env(set(lender, broker.brokerID, broker.asset(200).number()), kBorrower(borrower), kStartDate((env.now() + 1h).time_since_epoch().count()), kCounterparty(borrower), Ter(temINVALID)); - // A LoanSet with Borrower and CounterpartySignature is ambiguous. + // XLS-66 flow: Borrower + CounterpartySignature is ambiguous + // (temINVALID). env(set(lender, broker.brokerID, broker.asset(200).number()), kBorrower(borrower), kStartDate((env.now() + 1h).time_since_epoch().count()), @@ -481,10 +492,11 @@ private: Env env(*this, features); auto const broker = makeBroker(env, AssetType::XRP); - // Zero LoanID fails preflight. + // XLS-66 spec 3.9.3.1.1: LoanID is zero (temINVALID). env(accept(borrower, uint256{}), Ter(temINVALID)); - // A LoanID that does not resolve to a Loan object. + // XLS-66 spec 3.9.3.2.1: Loan with the specified LoanID does not + // exist (tecNO_ENTRY). env(accept(borrower, keylet::loan(broker.brokerID, SeqProxy::rawSequence(999)).key), Ter(tecNO_ENTRY)); @@ -493,7 +505,8 @@ private: propose(env, broker, lender, borrower, (env.now() + 1h).time_since_epoch().count()); env.close(); - // Only the borrower may accept. + // XLS-66 spec 3.9.3.2.3: Account submitting the tx is not the + // Loan.Borrower (tecNO_PERMISSION). env(accept(evan, loanKeylet.key), Ter(tecNO_PERMISSION)); env(accept(lender, loanKeylet.key), Ter(tecNO_PERMISSION)); expectStillPending(env, loanKeylet); @@ -502,7 +515,9 @@ private: env(accept(borrower, loanKeylet.key)); env.close(); - // The loan is no longer pending, so it cannot be accepted again. + // XLS-66 spec 3.9.3.2.2: Loan does not have lsfLoanPending set + // (tecNO_PERMISSION). Here, the loan was already accepted and is + // no longer pending. env(accept(borrower, loanKeylet.key), Ter(tecNO_PERMISSION)); } @@ -594,6 +609,82 @@ private: expectStillPending(env, loanKeylet); } + { + testcase("Two-step: LoanSet StartDate expiry boundary"); + + // XLS-66 flow: hasExpired uses Inclusive comparison + // (parentCloseTime() >= StartDate counts as expired), so the + // exact-equal case is on the expired side of the boundary. + // Lock that in for the two-step LoanSet preclaim check. + Env env(*this, features); + auto const broker = makeBroker(env, AssetType::XRP); + + Number const principal = broker.asset(200).number(); + auto const parentClose = + env.current()->parentCloseTime().time_since_epoch().count(); + + // StartDate == parentCloseTime is inclusive-expired. + env(set(lender, broker.brokerID, principal), + kBorrower(borrower), + kStartDate(parentClose), + kInterestRate(interest), + kPaymentTotal(payTotal), + kPaymentInterval(payInterval), + Ter(tecEXPIRED)); + + // StartDate == parentCloseTime + 1 is just above the boundary + // and must succeed. + auto const loanKeylet = nextLoanKeylet(env, broker); + env(set(lender, broker.brokerID, principal), + kBorrower(borrower), + kStartDate(parentClose + 1), + kInterestRate(interest), + kPaymentTotal(payTotal), + kPaymentInterval(payInterval)); + env.close(); + if (auto const loan = env.le(loanKeylet); BEAST_EXPECT(loan)) + BEAST_EXPECT(loan->isFlag(lsfLoanPending)); + } + + { + testcase("Two-step: StartDate near kMaxTime triggers overflow guard"); + + // XLS-66 flow: the two-step flow is the first place where + // LoanSet::preclaim sees a fully caller-controlled StartDate + // (getStartDate returns tx[sfStartDate] for two-step, not the + // ledger's own close time). Push StartDate near kMaxTime and + // verify the schedule-overflow guard still triggers tecKILLED + // through this newly-external input path. Mirrors the one-step + // overflow suite in LoanPay_test.cpp:540-618. + using timeType = decltype(sfNextPaymentDueDate)::type::value_type; + static_assert(std::is_same_v); + constexpr timeType kMaxTime = std::numeric_limits::max(); + static_assert(kMaxTime == 4'294'967'295); + + Env env(*this, features); + auto const broker = makeBroker(env, AssetType::XRP); + Number const principal = broker.asset(200).number(); + + // PaymentInterval alone exceeds kMaxTime - StartDate. + env(set(lender, broker.brokerID, principal), + kBorrower(borrower), + kStartDate(kMaxTime - (payInterval - 1)), + kInterestRate(interest), + kPaymentTotal(payTotal), + kPaymentInterval(payInterval), + Ter(tecKILLED)); + + // Interval fits but interval * total exceeds the remaining + // time available for the schedule. + env(set(lender, broker.brokerID, principal), + kBorrower(borrower), + kStartDate(kMaxTime - (payInterval * payTotal / 2)), + kInterestRate(interest), + kPaymentTotal(payTotal), + kPaymentInterval(payInterval), + Ter(tecKILLED)); + } + { testcase("Two-step: LoanDelete of pending loan after StartDate expired"); @@ -642,6 +733,8 @@ private: { testcase("Two-step: LoanSet with insufficient reserve"); + // XLS-66 spec 3.8.5.3.2: LoanBroker.Owner does not have + // sufficient reserve for the Loan object (tecINSUFFICIENT_RESERVE). // Use an IOU so the lender's XRP balance is only relevant to // the owner reserve for the Loan object created by LoanSet. Env env(*this, features); @@ -664,6 +757,8 @@ private: Ter(tecINSUFFICIENT_RESERVE)); } + // XLS-66 spec 3.8.5.3.4 → 3.8.5.2.9: Vault pseudo-account is frozen + // for the asset (tecFROZEN for IOUs, tecLOCKED for MPTs). // The issuer freezes the trust line (IOU) or locks the MPToken (MPT) // on the vault pseudo-account before LoanSet is submitted. The // proposal must be rejected by checkLoanFreeze in preclaim, and no @@ -707,6 +802,8 @@ private: BEAST_EXPECT(!env.le(loanKeylet)); } + // XLS-66 spec 3.8.5.3.4 → 3.8.5.2.10: LoanBroker pseudo-account is + // deep frozen for the asset (tecFROZEN for IOUs, tecLOCKED for MPTs). // Same as above, but for the LoanBroker pseudo-account (deep freeze). for (auto const assetType : {AssetType::IOU, AssetType::MPT}) { @@ -747,6 +844,8 @@ private: BEAST_EXPECT(!env.le(loanKeylet)); } + // XLS-66 spec 3.8.5.3.4 → 3.8.5.2.11: Borrower is frozen for the + // asset (tecFROZEN for IOUs, tecLOCKED for MPTs). // Same as above, but for the Borrower. for (auto const assetType : {AssetType::IOU, AssetType::MPT}) { @@ -782,6 +881,8 @@ private: BEAST_EXPECT(!env.le(loanKeylet)); } + // XLS-66 spec 3.8.5.3.4 → 3.8.5.2.12: LoanBroker.Owner is deep frozen + // for the asset (tecFROZEN for IOUs, tecLOCKED for MPTs). // Same as above, but for the LoanBroker owner (deep freeze). for (auto const assetType : {AssetType::IOU, AssetType::MPT}) { @@ -820,6 +921,8 @@ private: { testcase("Two-step: LoanAccept with insufficient reserve"); + // XLS-66 spec 3.9.3.2.5: Borrower does not have sufficient reserve + // for the Loan object (tecINSUFFICIENT_RESERVE). // Use an IOU so the borrower's XRP balance is only relevant to // the owner reserve, not to receiving the loan asset. Env env(*this, features); @@ -841,6 +944,8 @@ private: expectStillPending(env, loanKeylet); } + // XLS-66 spec 3.9.3.2.6: Vault pseudo-account is frozen for the asset + // (tecFROZEN for IOUs, tecLOCKED for MPTs). // Between the LoanSet proposal and the LoanAccept, the issuer // freezes the trust line (IOU) or locks the MPToken (MPT) on the // vault pseudo-account, which is about to disburse the principal. @@ -882,6 +987,8 @@ private: expectStillPending(env, loanKeylet); } + // XLS-66 spec 3.9.3.2.7: LoanBroker pseudo-account is deep frozen for + // the asset (tecFROZEN for IOUs, tecLOCKED for MPTs). // Between the LoanSet proposal and the LoanAccept, the issuer deep // freezes the trust line (IOU) or locks the MPToken (MPT) on the // LoanBroker pseudo-account, which is the fallback recipient of @@ -923,6 +1030,8 @@ private: expectStillPending(env, loanKeylet); } + // XLS-66 spec 3.9.3.2.8: Borrower is frozen for the asset + // (tecFROZEN for IOUs, tecLOCKED for MPTs). // Between the LoanSet proposal and the LoanAccept, the issuer // freezes the trust line (IOU) or locks the MPToken (MPT) on the // borrower, who is about to receive the principal. Acceptance must @@ -958,6 +1067,8 @@ private: expectStillPending(env, loanKeylet); } + // XLS-66 spec 3.9.3.2.9: LoanBroker.Owner is deep frozen for the + // asset (tecFROZEN for IOUs, tecLOCKED for MPTs). // Between the LoanSet proposal and the LoanAccept, the issuer deep // freezes the trust line (IOU) or locks the MPToken (MPT) on the // LoanBroker owner, who receives the origination fee. Acceptance @@ -996,6 +1107,9 @@ private: { testcase("Two-step: LoanAccept when a holding cannot be added"); + // XLS-66 spec 3.9.3.2.10: cannot add asset holding for the + // Vault.Asset (tecNO_PERMISSION / terNO_RIPPLE for IOU with + // asfDefaultRipple cleared). // Between the LoanSet proposal and the LoanAccept, the IOU // issuer clears asfDefaultRipple, so a fresh holding for the // vault asset can no longer be established. Acceptance must be @@ -1019,6 +1133,8 @@ private: { testcase("Two-step: LoanAccept with unauthorized borrower (MPT)"); + // XLS-66 spec 3.9.3.2.11: Borrower is not authorized for the + // asset (tecNO_AUTH). // The MPT requires holder authorization. The borrower is // authorized at LoanSet proposal time so the proposal succeeds, // then the issuer revokes the borrower's MPToken authorization @@ -1058,6 +1174,8 @@ private: { testcase("Two-step: LoanAccept with unauthorized broker owner (MPT)"); + // XLS-66 spec 3.9.3.2.12: LoanBroker.Owner is not authorized for + // the asset (tecNO_AUTH). // Same rationale as the unauthorized-borrower case, but this // time the issuer revokes the broker owner's MPToken // authorization between proposal and accept. disburseLoan's @@ -1151,11 +1269,12 @@ private: { testcase("Two-step: LoanBrokerDelete blocked by pending loan"); - // A pending loan bumps the LoanBroker's OwnerCount, so - // LoanBrokerDelete must fail with tecHAS_OBLIGATIONS while the - // pending loan is outstanding, just as it does for an active - // (accepted) loan. Once the pending loan is deleted, the broker - // can be deleted too. + // XLS-66 spec 3.4.3.2.3: LoanBroker.OwnerCount != 0 (has + // outstanding loans) → tecHAS_OBLIGATIONS. A pending loan bumps + // the LoanBroker's OwnerCount, so LoanBrokerDelete must fail + // while the pending loan is outstanding, just as it does for an + // active (accepted) loan. Once the pending loan is deleted, the + // broker can be deleted too. Env env(*this, features); auto const broker = makeBroker(env, AssetType::XRP); @@ -1181,6 +1300,188 @@ private: env.close(); BEAST_EXPECT(!env.le(broker.brokerKeylet())); } + + { + testcase("Two-step: two pending loans coexist on the same broker"); + + // XLS-66 flow: two pending proposals from the same broker each + // contribute independently to DebtTotal, AssetsReserved, and + // OwnerCount. Deleting one pending loan must leave the other's + // bookkeeping untouched. + Env env(*this, features); + auto const broker = makeBroker(env, AssetType::XRP); + + Number const principal = broker.asset(200).number(); + auto const vault0 = readVault(env, broker); + auto const broker0 = readBroker(env, broker); + + // Propose L1 (borrower) to establish a baseline delta. + auto const l1Keylet = nextLoanKeylet(env, broker); + propose(env, broker, lender, borrower, + (env.now() + 1h).time_since_epoch().count()); + env.close(); + + auto const vault1 = readVault(env, broker); + auto const broker1 = readBroker(env, broker); + Number const l1DebtDelta = broker1.debtTotal - broker0.debtTotal; + BEAST_EXPECT(vault1.reserved == vault0.reserved + principal); + BEAST_EXPECT(broker1.ownerCount == broker0.ownerCount + 1); + + // Propose L2 (evan) on the same broker while L1 is still + // pending. Each proposal contributes an equal delta. + auto const l2Keylet = nextLoanKeylet(env, broker); + propose(env, broker, lender, evan, + (env.now() + 1h).time_since_epoch().count()); + env.close(); + + auto const vault2 = readVault(env, broker); + auto const broker2 = readBroker(env, broker); + BEAST_EXPECT(broker2.debtTotal - broker1.debtTotal == l1DebtDelta); + BEAST_EXPECT(vault2.reserved == vault0.reserved + principal + principal); + BEAST_EXPECT(broker2.ownerCount == broker0.ownerCount + 2); + + // Both loans exist and remain pending. + if (auto const l1 = env.le(l1Keylet); BEAST_EXPECT(l1)) + BEAST_EXPECT(l1->isFlag(lsfLoanPending)); + if (auto const l2 = env.le(l2Keylet); BEAST_EXPECT(l2)) + BEAST_EXPECT(l2->isFlag(lsfLoanPending)); + + // Delete L1. L2's bookkeeping is untouched; broker state + // reflects exactly the L2-only contribution. + env(del(lender, l1Keylet.key)); + env.close(); + BEAST_EXPECT(!env.le(l1Keylet)); + + auto const vault3 = readVault(env, broker); + auto const broker3 = readBroker(env, broker); + BEAST_EXPECT(broker3.debtTotal == broker0.debtTotal + l1DebtDelta); + BEAST_EXPECT(vault3.reserved == vault0.reserved + principal); + BEAST_EXPECT(broker3.ownerCount == broker0.ownerCount + 1); + if (auto const l2 = env.le(l2Keylet); BEAST_EXPECT(l2)) + BEAST_EXPECT(l2->isFlag(lsfLoanPending)); + } + + { + testcase("Two-step: DebtMaximum constrains a second pending proposal"); + + // XLS-66 spec 3.8.5.3.4 → 3.8.5.2.19: a first pending loan's + // DebtTotal contribution counts toward the LoanBroker's debt + // cap. Set DebtMaximum to L1's DebtTotal so a same-sized L2 + // fails with tecLIMIT_EXCEEDED. + Env env(*this, features); + auto const broker = makeBroker(env, AssetType::XRP); + + auto const l1Keylet = nextLoanKeylet(env, broker); + propose(env, broker, lender, borrower, + (env.now() + 1h).time_since_epoch().count()); + env.close(); + + auto const brokerL1 = env.le(broker.brokerKeylet()); + if (!BEAST_EXPECT(brokerL1)) + return; + Number const debtAfterL1 = brokerL1->at(sfDebtTotal); + + // Tighten DebtMaximum to exactly L1's DebtTotal. + env(jtx::loan_broker::set(lender, broker.vaultID), + jtx::loan_broker::kLoanBrokerId(broker.brokerID), + jtx::loan_broker::kDebtMaximum(debtAfterL1)); + env.close(); + + // Second proposal exceeds the debt cap. + propose(env, broker, lender, evan, + (env.now() + 1h).time_since_epoch().count(), + Ter(tecLIMIT_EXCEEDED)); + env.close(); + + // L1 remains pending; L2 was not created. + if (auto const l1 = env.le(l1Keylet); BEAST_EXPECT(l1)) + BEAST_EXPECT(l1->isFlag(lsfLoanPending)); + } + + // XLS-66 flow (Batch + V1.1) two-step: a Batch containing an inner + // LoanSet with Borrower + StartDate (no Counterparty, no + // CounterpartySignature) is the analogue of the immediate-flow + // batch-success path (LoanLifecycle_test.cpp "Batch Bypass + // Counterparty"). The outer batch is signed by the LoanBroker.Owner + // (lender); no additional batch signer is required since two-step + // has no counterparty consent step. Gated on lendingBatchEnabled to + // match the existing pattern: while ttLOAN_SET is on + // Batch::kDisabledTxTypes, the batch fails with temINVALID_INNER_BATCH; + // once the disabled-list is updated, it must create a pending loan. + { + bool const lendingBatchEnabled = + !std::ranges::any_of(Batch::kDisabledTxTypes, [](auto const& disabled) { + return disabled == ttLOAN_SET; + }); + + testcase( + lendingBatchEnabled + ? "Two-step: Batch inner LoanSet creates a pending loan" + : "Two-step: Batch inner LoanSet rejected while ttLOAN_SET is disabled"); + + Env env(*this, features); + auto const broker = makeBroker(env, AssetType::XRP); + + Number const principal = broker.asset(200).number(); + std::uint32_t const startDate = (env.now() + 1h).time_since_epoch().count(); + + auto const brokerState0 = env.le(broker.brokerKeylet()); + if (!BEAST_EXPECT(brokerState0)) + return; + Number const debtTotal0 = brokerState0->at(sfDebtTotal); + std::uint32_t const brokerOwnerCount0 = brokerState0->at(sfOwnerCount); + + auto const loanKeylet = nextLoanKeylet(env, broker); + auto const lenderSeq = env.seq(lender); + auto const batchFee = batch::calcBatchFee(env, 0, 2); + + env(batch::outer(lender, lenderSeq, batchFee, tfAllOrNothing), + batch::Inner( + env.json( + set(lender, broker.brokerID, principal), + kBorrower(borrower.id()), + kStartDate(startDate), + kInterestRate(interest), + kPaymentTotal(payTotal), + kPaymentInterval(payInterval), + Sig(kNone), + Fee(kNone), + Seq(kNone)), + lenderSeq + 1), + batch::Inner(pay(lender, borrower, XRP(1)), lenderSeq + 2), + Ter(lendingBatchEnabled ? TER(tesSUCCESS) : TER(temINVALID_INNER_BATCH))); + env.close(); + + if (lendingBatchEnabled) + { + if (auto const loan = env.le(loanKeylet); BEAST_EXPECT(loan)) + { + BEAST_EXPECT(loan->isFlag(lsfLoanPending)); + BEAST_EXPECT(loan->at(sfBorrower) == borrower.id()); + BEAST_EXPECT(loan->at(sfStartDate) == startDate); + } + + // Broker bookkeeping matches a non-batch two-step proposal: + // DebtTotal grows by principal + interestDue, and OwnerCount + // grows by one (the pending loan). + if (auto const b = env.le(broker.brokerKeylet()); BEAST_EXPECT(b)) + { + BEAST_EXPECT(b->at(sfDebtTotal) > debtTotal0); + BEAST_EXPECT(b->at(sfOwnerCount) == brokerOwnerCount0 + 1); + } + } + else + { + // The batch was rejected up front; no loan was created and + // broker bookkeeping is unchanged. + BEAST_EXPECT(!env.le(loanKeylet)); + if (auto const b = env.le(broker.brokerKeylet()); BEAST_EXPECT(b)) + { + BEAST_EXPECT(b->at(sfDebtTotal) == debtTotal0); + BEAST_EXPECT(b->at(sfOwnerCount) == brokerOwnerCount0); + } + } + } } public: diff --git a/src/test/app/lending/LoanValidation_test.cpp b/src/test/app/lending/LoanValidation_test.cpp index 1b9a5747b0..a8e876c0dd 100644 --- a/src/test/app/lending/LoanValidation_test.cpp +++ b/src/test/app/lending/LoanValidation_test.cpp @@ -182,7 +182,7 @@ private: testZeroBrokerID(to_string(uint256{}), tfFullyCanonicalSig); } - // both Borrower and Counterparty are specified + // XLS-66 flow: Borrower + Counterparty is ambiguous (temINVALID). env(set(borrower, brokerInfo.brokerID, debtMaximumRequest), kBorrower(borrower), kCounterparty(lender), @@ -190,7 +190,8 @@ private: loanSetFee, Ter(temINVALID)); - // both Borrower and CounterpartySignature are specified + // XLS-66 flow: Borrower + CounterpartySignature is ambiguous + // (temINVALID). env(set(lender, brokerInfo.brokerID, debtMaximumRequest), kBorrower(borrower), Sig(sfCounterpartySignature, borrower), @@ -324,6 +325,41 @@ private: } } + void + testInvalidLoanAccept() + { + testcase("Invalid LoanAccept"); + using namespace jtx; + using namespace loan; + + // Mirrors testInvalidLoanSet/Delete/Manage/Pay for the + // transaction-level preflight/preclaim guards of LoanAccept. + // Two-step-specific failures (frozen, unauthorised, insufficient + // reserve, expired proposal) are covered inline in + // LoanTwoStep_test.cpp. + Account const alice{"alice"}; + Env env(*this); + env.fund(XRP(1'000), alice); + env.close(); + + // XLS-66 spec 3.9.3.1.1: LoanID is zero (temINVALID). + env(accept(alice, beast::kZero), Ter(temINVALID)); + + auto const bogusLoanID = + keylet::loan(uint256{1}, SeqProxy::rawSequence(1)).key; + + // preflight: temINVALID_FLAG. LoanAccept does not override + // getFlagsMask, so only universal flags (tfFullyCanonicalSig, + // tfInnerBatchTxn) are permitted. Any other bit must be rejected. + // Reuses tfLoanImpair (a LoanManage flag) as a stand-in for "any + // non-universal flag". + env(accept(alice, bogusLoanID, tfLoanImpair), Ter(temINVALID_FLAG)); + + // XLS-66 spec 3.9.3.2.1: Loan with the specified LoanID does not + // exist (tecNO_ENTRY). + env(accept(alice, bogusLoanID), Ter(tecNO_ENTRY)); + } + void testInvalidLoanPay() { @@ -618,6 +654,7 @@ private: testInvalidLoanSet(kind); testInvalidLoanDelete(); testInvalidLoanManage(); + testInvalidLoanAccept(); testInvalidLoanPay(); testRequireAuth(); testLimitExceeded(); diff --git a/src/test/app/lending/Loan_test.cpp b/src/test/app/lending/Loan_test.cpp index 717387665e..3d6a5f08ad 100644 --- a/src/test/app/lending/Loan_test.cpp +++ b/src/test/app/lending/Loan_test.cpp @@ -18,7 +18,7 @@ class Loan_test : public beast::unit_test::Suite void run() override { - static constexpr std::array kMembers{ + static constexpr std::array kMembers{ "LendingHelpers", "LoanBroker", "LoanCashBasis", @@ -30,6 +30,7 @@ class Loan_test : public beast::unit_test::Suite "LoanRounding", "LoanSecurity", "LoanSet", + "LoanTwoStep", "LoanValidation", };