From 3422c11d021abe3928174907e8c57544d7817c4f Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 4 Feb 2026 11:14:39 +0100 Subject: [PATCH 01/17] adds lending v1.1 fix amendment --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/LoanBroker_test.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index d8498ffa2fc..1922b2eb0c7 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,7 +15,7 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. - +XRPL_FIX (LendingProtocolV1_1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (BatchInnerSigs, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(LendingProtocol, Supported::yes, VoteBehavior::DefaultNo) diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 139350a881b..2be150c11c2 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -1701,10 +1701,21 @@ class LoanBroker_test : public beast::unit_test::suite testRIPD4274MPT(); } + void + testFixAmendmentEnabled() + { + using namespace jtx; + testcase("testFixAmendmentEnabled"); + Env env{*this}; + + BEAST_EXPECT(env.enabled(fixLendingProtocolv1_1)); + } + public: void run() override { + testFixAmendmentEnabled(); testLoanBrokerSetDebtMaximum(); testLoanBrokerCoverDepositNullVault(); From 5e51893e9ba4a7ceef7cfa4dfd7b084bf3c86bb2 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 4 Feb 2026 11:31:58 +0100 Subject: [PATCH 02/17] fixes a typo --- src/test/app/LoanBroker_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 2be150c11c2..0945b4a04a1 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -1708,7 +1708,7 @@ class LoanBroker_test : public beast::unit_test::suite testcase("testFixAmendmentEnabled"); Env env{*this}; - BEAST_EXPECT(env.enabled(fixLendingProtocolv1_1)); + BEAST_EXPECT(env.enabled(fixLendingProtocolV1_1)); } public: From c5d7ebe93dc6cb092f991543d3bc87941b84d631 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Thu, 5 Feb 2026 10:23:55 +0100 Subject: [PATCH 03/17] restores missing linebreak --- include/xrpl/protocol/detail/features.macro | 1 + 1 file changed, 1 insertion(+) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 1922b2eb0c7..0196dccf867 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,6 +15,7 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. + XRPL_FIX (LendingProtocolV1_1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (BatchInnerSigs, Supported::yes, VoteBehavior::DefaultNo) From ba530260067a6032f2b3f6683e4985ed30ebda7e Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Thu, 26 Feb 2026 14:13:29 +0100 Subject: [PATCH 04/17] adds sfMemoData field to VaultDelete transaction (#6356) * adds sfMemoData field to VaultDelete transaction --- .../xrpl/protocol/detail/transactions.macro | 1 + .../tx/transactors/Vault/VaultDelete.cpp | 7 ++ src/test/app/Vault_test.cpp | 88 +++++++++++++++---- 3 files changed, 80 insertions(+), 16 deletions(-) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index b696a1d1c2c..c0ac1ba5266 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -868,6 +868,7 @@ TRANSACTION(ttVAULT_DELETE, 67, VaultDelete, mustDeleteAcct | destroyMPTIssuance | mustModifyVault, ({ {sfVaultID, soeREQUIRED}, + {sfMemoData, soeOPTIONAL}, })) /** This transaction trades assets for shares with a vault. */ diff --git a/src/libxrpl/tx/transactors/Vault/VaultDelete.cpp b/src/libxrpl/tx/transactors/Vault/VaultDelete.cpp index 0b3aef19a8d..25626720418 100644 --- a/src/libxrpl/tx/transactors/Vault/VaultDelete.cpp +++ b/src/libxrpl/tx/transactors/Vault/VaultDelete.cpp @@ -18,6 +18,13 @@ VaultDelete::preflight(PreflightContext const& ctx) return temMALFORMED; } + if (ctx.tx.isFieldPresent(sfMemoData) && !ctx.rules.enabled(fixLendingProtocolV1_1)) + return temDISABLED; + + // The sfMemoData field is an optional field used to record the deletion reason. + if (auto const data = ctx.tx[~sfMemoData]; data && !validDataLength(data, maxDataPayloadLength)) + return temMALFORMED; + return tesSUCCESS; } diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 93ac94d7ce4..541d3975f28 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -1064,14 +1064,13 @@ class Vault_test : public beast::unit_test::suite { using namespace test::jtx; - auto testCase = [this]( - std::function test) { + auto testCase = [this](std::function test) { Env env{*this, testable_amendments() | featureSingleAssetVault}; Account issuer{"issuer"}; Account owner{"owner"}; @@ -1354,14 +1353,13 @@ class Vault_test : public beast::unit_test::suite { using namespace test::jtx; - auto testCase = [this]( - std::function test) { + auto testCase = [this](std::function test) { Env env{*this, testable_amendments() | featureSingleAssetVault}; Account issuer{"issuer"}; Account owner{"owner"}; @@ -5357,6 +5355,63 @@ class Vault_test : public beast::unit_test::suite } } + void + testVaultDeleteData() + { + using namespace test::jtx; + + Env env{*this}; + + Account const owner{"owner"}; + env.fund(XRP(1'000'000), owner); + env.close(); + + Vault vault{env}; + + auto const keylet = keylet::vault(owner.id(), 1); + auto delTx = vault.del({.owner = owner, .id = keylet.key}); + + // Test VaultDelete with fixLendingProtocolV1_1 disabled + // Transaction fails if the data field is provided + { + testcase("VaultDelete data fixLendingProtocolV1_1 disabled"); + env.disableFeature(fixLendingProtocolV1_1); + delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); + env(delTx, ter(temDISABLED), THISLINE); + env.close(); + env.enableFeature(fixLendingProtocolV1_1); + } + + // Transaction fails if the data field is too large + { + testcase("VaultDelete data fixLendingProtocolV1_1 enabled data too large"); + delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength + 1, 'A')); + env(delTx, ter(temMALFORMED), THISLINE); + env.close(); + } + + // Transaction fails if the data field is set, but is empty + { + testcase("VaultDelete data fixLendingProtocolV1_1 enabled data empty"); + delTx[sfMemoData] = strHex(std::string(0, 'A')); + env(delTx, ter(temMALFORMED), THISLINE); + env.close(); + } + + { + testcase("VaultDelete data fixLendingProtocolV1_1 enabled data valid"); + PrettyAsset const xrpAsset = xrpIssue(); + auto [tx, keylet] = vault.create({.owner = owner, .asset = xrpAsset}); + env(tx, ter(tesSUCCESS), THISLINE); + env.close(); + // Recreate the transaction as the vault keylet changed + auto delTx = vault.del({.owner = owner, .id = keylet.key}); + delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); + env(delTx, ter(tesSUCCESS), THISLINE); + env.close(); + } + } + public: void run() override @@ -5378,6 +5433,7 @@ class Vault_test : public beast::unit_test::suite testVaultClawbackBurnShares(); testVaultClawbackAssets(); testAssetsMaximum(); + testVaultDeleteData(); } }; From b32209752948d8794e17964c92af067116022e7e Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 3 Mar 2026 13:51:15 +0100 Subject: [PATCH 05/17] fixes formatting errors --- src/test/app/Vault_test.cpp | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index e11861a2d2f..fef16cad71a 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -1064,14 +1064,16 @@ class Vault_test : public beast::unit_test::suite { using namespace test::jtx; - auto testCase = [this](std::function test) { + auto testCase = [this]( + std::function test) { Env env{*this, testable_amendments() | featureSingleAssetVault}; + Account issuer{"issuer"}; Account owner{"owner"}; Account depositor{"depositor"}; @@ -1353,13 +1355,14 @@ class Vault_test : public beast::unit_test::suite { using namespace test::jtx; - auto testCase = [this](std::function test) { + auto testCase = [this]( + std::function test) { Env env{*this, testable_amendments() | featureSingleAssetVault}; Account issuer{"issuer"}; Account owner{"owner"}; From 4067e5025f3d6f5da060110c8eb5bf2474f4c1eb Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Thu, 5 Mar 2026 10:38:42 +0100 Subject: [PATCH 06/17] Add rounding to Vault invariants (#6217) Co-authored-by: Ed Hennis --- include/xrpl/protocol/STAmount.h | 19 +- include/xrpl/tx/invariants/VaultInvariant.h | 15 +- .../tx/transactors/Lending/LendingHelpers.h | 2 +- src/libxrpl/tx/invariants/VaultInvariant.cpp | 276 +++++++++++++----- .../Lending/LoanBrokerCoverWithdraw.cpp | 2 +- src/test/app/Invariants_test.cpp | 128 ++++++++ src/test/app/LendingHelpers_test.cpp | 1 - src/test/app/Loan_test.cpp | 137 ++++++++- 8 files changed, 506 insertions(+), 74 deletions(-) diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index dadeec096f3..88a46421599 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -42,8 +42,8 @@ class STAmount final : public STBase, public CountedObject public: using value_type = STAmount; - static int const cMinOffset = -96; - static int const cMaxOffset = 80; + static int constexpr cMinOffset = -96; + static int constexpr cMaxOffset = 80; // Maximum native value supported by the code constexpr static std::uint64_t cMinValue = 1'000'000'000'000'000ull; @@ -739,6 +739,21 @@ canAdd(STAmount const& amt1, STAmount const& amt2); bool canSubtract(STAmount const& amt1, STAmount const& amt2); +/** Get the scale of a Number for a given asset. + * + * "scale" is similar to "exponent", but from the perspective of STAmount, which has different rules + * and mantissa ranges for determining the exponent than Number. + * + * @param number The Number to get the scale of. + * @param asset The asset to use for determining the scale. + * @return The scale of this Number for the given asset. + */ +inline int +scale(Number const& number, Asset const& asset) +{ + return STAmount{asset, number}.exponent(); +} + } // namespace xrpl //------------------------------------------------------------------------------ diff --git a/include/xrpl/tx/invariants/VaultInvariant.h b/include/xrpl/tx/invariants/VaultInvariant.h index ded9e4618bd..1e1ded6fa14 100644 --- a/include/xrpl/tx/invariants/VaultInvariant.h +++ b/include/xrpl/tx/invariants/VaultInvariant.h @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -60,11 +61,19 @@ class ValidVault Shares static make(SLE const&); }; +public: + struct DeltaInfo final + { + Number delta = numZero; + std::optional scale; + }; + +private: std::vector afterVault_ = {}; std::vector afterMPTs_ = {}; std::vector beforeVault_ = {}; std::vector beforeMPTs_ = {}; - std::unordered_map deltas_ = {}; + std::unordered_map deltas_ = {}; public: void @@ -72,6 +81,10 @@ class ValidVault bool finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&); + + // Compute the coarsest scale required to represent all numbers + [[nodiscard]] static std::int32_t + computeMinScale(Asset const& asset, std::vector const& numbers); }; } // namespace xrpl diff --git a/include/xrpl/tx/transactors/Lending/LendingHelpers.h b/include/xrpl/tx/transactors/Lending/LendingHelpers.h index 4057c9c1738..8dd6866ac34 100644 --- a/include/xrpl/tx/transactors/Lending/LendingHelpers.h +++ b/include/xrpl/tx/transactors/Lending/LendingHelpers.h @@ -171,7 +171,7 @@ getAssetsTotalScale(SLE::const_ref vaultSle) { if (!vaultSle) return Number::minExponent - 1; // LCOV_EXCL_LINE - return STAmount{vaultSle->at(sfAsset), vaultSle->at(sfAssetsTotal)}.exponent(); + return scale(vaultSle->at(sfAssetsTotal), vaultSle->at(sfAsset)); } TER diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index c3db3a563a5..69d29448d31 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -60,10 +61,12 @@ ValidVault::visitEntry( "xrpl::ValidVault::visitEntry : some object is available"); // Number balanceDelta will capture the difference (delta) between "before" - // state (zero if created) and "after" state (zero if destroyed), so the - // invariants can validate that the change in account balances matches the - // change in vault balances, stored to deltas_ at the end of this function. - Number balanceDelta{}; + // state (zero if created) and "after" state (zero if destroyed), and + // preserves value scale (exponent) to round values to the same scale during + // validation. It is used to validate that the change in account + // balances matches the change in vault balances, stored to deltas_ at the + // end of this function. + DeltaInfo balanceDelta{numZero, std::nullopt}; std::int8_t sign = 0; if (before) @@ -77,18 +80,34 @@ ValidVault::visitEntry( // At this moment we have no way of telling if this object holds // vault shares or something else. Save it for finalize. beforeMPTs_.push_back(Shares::make(*before)); - balanceDelta = static_cast(before->getFieldU64(sfOutstandingAmount)); + balanceDelta.delta = + static_cast(before->getFieldU64(sfOutstandingAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = 1; break; case ltMPTOKEN: - balanceDelta = static_cast(before->getFieldU64(sfMPTAmount)); + balanceDelta.delta = static_cast(before->getFieldU64(sfMPTAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = -1; break; case ltACCOUNT_ROOT: - case ltRIPPLE_STATE: - balanceDelta = before->getFieldAmount(sfBalance); + balanceDelta.delta = before->getFieldAmount(sfBalance); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; sign = -1; break; + case ltRIPPLE_STATE: { + auto const amount = before->getFieldAmount(sfBalance); + balanceDelta.delta = amount; + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + balanceDelta.scale = amount.exponent(); + sign = -1; + break; + } default:; } } @@ -104,19 +123,36 @@ ValidVault::visitEntry( // At this moment we have no way of telling if this object holds // vault shares or something else. Save it for finalize. afterMPTs_.push_back(Shares::make(*after)); - balanceDelta -= + balanceDelta.delta -= Number(static_cast(after->getFieldU64(sfOutstandingAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = 1; break; case ltMPTOKEN: - balanceDelta -= Number(static_cast(after->getFieldU64(sfMPTAmount))); + balanceDelta.delta -= + Number(static_cast(after->getFieldU64(sfMPTAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = -1; break; case ltACCOUNT_ROOT: - case ltRIPPLE_STATE: - balanceDelta -= Number(after->getFieldAmount(sfBalance)); + balanceDelta.delta -= Number(after->getFieldAmount(sfBalance)); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; + sign = -1; + break; + case ltRIPPLE_STATE: { + auto const amount = after->getFieldAmount(sfBalance); + balanceDelta.delta -= Number(amount); + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + if (amount.exponent() > balanceDelta.scale) + balanceDelta.scale = amount.exponent(); sign = -1; break; + } default:; } } @@ -128,7 +164,11 @@ ValidVault::visitEntry( // transferred to the account. We intentionally do not compare balanceDelta // against zero, to avoid missing such updates. if (sign != 0) - deltas_[key] = balanceDelta * sign; + { + XRPL_ASSERT_PARTS(balanceDelta.scale, "xrpl::ValidVault::visitEntry", "scale initialized"); + balanceDelta.delta *= sign; + deltas_[key] = balanceDelta; + } } bool @@ -390,13 +430,13 @@ ValidVault::finalize( } auto const& vaultAsset = afterVault.asset; - auto const deltaAssets = [&](AccountID const& id) -> std::optional { + auto const deltaAssets = [&](AccountID const& id) -> std::optional { auto const get = // - [&](auto const& it, std::int8_t sign = 1) -> std::optional { + [&](auto const& it, std::int8_t sign = 1) -> std::optional { if (it == deltas_.end()) return std::nullopt; - return it->second * sign; + return DeltaInfo{it->second.delta * sign, it->second.scale}; }; return std::visit( @@ -415,7 +455,7 @@ ValidVault::finalize( }, vaultAsset.value()); }; - auto const deltaAssetsTxAccount = [&]() -> std::optional { + auto const deltaAssetsTxAccount = [&]() -> std::optional { auto ret = deltaAssets(tx[sfAccount]); // Nothing returned or not XRP transaction if (!ret.has_value() || !vaultAsset.native()) @@ -426,20 +466,20 @@ ValidVault::finalize( delegate.has_value() && *delegate != tx[sfAccount]) return ret; - *ret += fee.drops(); - if (*ret == zero) + ret->delta += fee.drops(); + if (ret->delta == zero) return std::nullopt; return ret; }; - auto const deltaShares = [&](AccountID const& id) -> std::optional { + auto const deltaShares = [&](AccountID const& id) -> std::optional { auto const it = [&]() { if (id == afterVault.pseudoId) return deltas_.find(keylet::mptIssuance(afterVault.shareMPTID).key); return deltas_.find(keylet::mptoken(afterVault.shareMPTID, id).key); }(); - return it != deltas_.end() ? std::optional(it->second) : std::nullopt; + return it != deltas_.end() ? std::optional(it->second) : std::nullopt; }; auto const vaultHoldsNoAssets = [&](Vault const& vault) { @@ -566,16 +606,38 @@ ValidVault::finalize( !beforeVault_.empty(), "xrpl::ValidVault::finalize : deposit updated a vault"); auto const& beforeVault = beforeVault_[0]; - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); - - if (!vaultDeltaAssets) + auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId); + if (!maybeVaultDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault balance"; return false; // That's all we can do } - if (*vaultDeltaAssets > tx[sfAmount]) + // Get the coarsest scale to round calculations to + DeltaInfo totalDelta{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + scale(afterVault.assetsTotal, vaultAsset), + scale(beforeVault.assetsTotal, vaultAsset))}; + DeltaInfo availableDelta{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + scale(afterVault.assetsAvailable, vaultAsset), + scale(beforeVault.assetsAvailable, vaultAsset))}; + auto const minScale = computeMinScale( + vaultAsset, + { + *maybeVaultDeltaAssets, + totalDelta, + availableDelta, + }); + + auto const vaultDeltaAssets = + roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); + auto const txAmount = roundToAsset(vaultAsset, tx[sfAmount], minScale); + + if (vaultDeltaAssets > txAmount) { JLOG(j.fatal()) << // "Invariant failed: deposit must not change vault " @@ -583,7 +645,7 @@ ValidVault::finalize( result = false; } - if (*vaultDeltaAssets <= zero) + if (vaultDeltaAssets <= zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must increase vault balance"; @@ -600,16 +662,23 @@ ValidVault::finalize( if (!issuerDeposit) { - auto const accountDeltaAssets = deltaAssetsTxAccount(); - if (!accountDeltaAssets) + auto const maybeAccDeltaAssets = deltaAssetsTxAccount(); + if (!maybeAccDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: deposit must change depositor " "balance"; return false; } + auto const localMinScale = + std::max(minScale, computeMinScale(vaultAsset, {*maybeAccDeltaAssets})); + + auto const accountDeltaAssets = + roundToAsset(vaultAsset, maybeAccDeltaAssets->delta, localMinScale); + auto const localVaultDeltaAssets = + roundToAsset(vaultAsset, vaultDeltaAssets, localMinScale); - if (*accountDeltaAssets >= zero) + if (accountDeltaAssets >= zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must decrease depositor " @@ -617,7 +686,7 @@ ValidVault::finalize( result = false; } - if (*accountDeltaAssets * -1 != *vaultDeltaAssets) + if (localVaultDeltaAssets * -1 != accountDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault and " @@ -635,16 +704,17 @@ ValidVault::finalize( result = false; } - auto const accountDeltaShares = deltaShares(tx[sfAccount]); - if (!accountDeltaShares) + auto const maybeAccDeltaShares = deltaShares(tx[sfAccount]); + if (!maybeAccDeltaShares) { JLOG(j.fatal()) << // "Invariant failed: deposit must change depositor " "shares"; return false; // That's all we can do } - - if (*accountDeltaShares <= zero) + // We don't need to round shares, they are integral MPT + auto const& accountDeltaShares = *maybeAccDeltaShares; + if (accountDeltaShares.delta <= zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must increase depositor " @@ -652,15 +722,17 @@ ValidVault::finalize( result = false; } - auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + auto const maybeVaultDeltaShares = deltaShares(afterVault.pseudoId); + if (!maybeVaultDeltaShares || maybeVaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + // We don't need to round shares, they are integral MPT + auto const& vaultDeltaShares = *maybeVaultDeltaShares; + if (vaultDeltaShares.delta * -1 != accountDeltaShares.delta) { JLOG(j.fatal()) << // "Invariant failed: deposit must change depositor and " @@ -668,13 +740,18 @@ ValidVault::finalize( result = false; } - if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) + auto const assetTotalDelta = roundToAsset( + vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale); + if (assetTotalDelta != vaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: deposit and assets " "outstanding must add up"; result = false; } - if (beforeVault.assetsAvailable + *vaultDeltaAssets != afterVault.assetsAvailable) + + auto const assetAvailableDelta = roundToAsset( + vaultAsset, afterVault.assetsAvailable - beforeVault.assetsAvailable, minScale); + if (assetAvailableDelta != vaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: deposit and assets " "available must add up"; @@ -692,16 +769,33 @@ ValidVault::finalize( "vault"); auto const& beforeVault = beforeVault_[0]; - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); + auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId); - if (!vaultDeltaAssets) + if (!maybeVaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: withdrawal must " "change vault balance"; return false; // That's all we can do } - if (*vaultDeltaAssets >= zero) + // Get the most coarse scale to round calculations to + auto const totalDelta = DeltaInfo{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + scale(afterVault.assetsTotal, vaultAsset), + scale(beforeVault.assetsTotal, vaultAsset))}; + auto const availableDelta = DeltaInfo{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + scale(afterVault.assetsAvailable, vaultAsset), + scale(beforeVault.assetsAvailable, vaultAsset))}; + auto const minScale = computeMinScale( + vaultAsset, {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + + auto const vaultPseudoDeltaAssets = + roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); + + if (vaultPseudoDeltaAssets >= zero) { JLOG(j.fatal()) << "Invariant failed: withdrawal must " "decrease vault balance"; @@ -719,15 +813,15 @@ ValidVault::finalize( if (!issuerWithdrawal) { - auto const accountDeltaAssets = deltaAssetsTxAccount(); - auto const otherAccountDelta = [&]() -> std::optional { + auto const maybeAccDelta = deltaAssetsTxAccount(); + auto const maybeOtherAccDelta = [&]() -> std::optional { if (auto const destination = tx[~sfDestination]; destination && *destination != tx[sfAccount]) return deltaAssets(*destination); return std::nullopt; }(); - if (accountDeltaAssets.has_value() == otherAccountDelta.has_value()) + if (maybeAccDelta.has_value() == maybeOtherAccDelta.has_value()) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change one " @@ -736,9 +830,17 @@ ValidVault::finalize( } auto const destinationDelta = // - accountDeltaAssets ? *accountDeltaAssets : *otherAccountDelta; + maybeAccDelta ? *maybeAccDelta : *maybeOtherAccDelta; + + // the scale of destinationDelta can be coarser than + // minScale, so we take that into account when rounding + auto const localMinScale = + std::max(minScale, computeMinScale(vaultAsset, {destinationDelta})); - if (destinationDelta <= zero) + auto const roundedDestinationDelta = + roundToAsset(vaultAsset, destinationDelta.delta, localMinScale); + + if (roundedDestinationDelta <= zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must increase " @@ -746,7 +848,9 @@ ValidVault::finalize( result = false; } - if (*vaultDeltaAssets * -1 != destinationDelta) + auto const localPseudoDeltaAssets = + roundToAsset(vaultAsset, vaultPseudoDeltaAssets, localMinScale); + if (localPseudoDeltaAssets * -1 != roundedDestinationDelta) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change vault " @@ -755,6 +859,7 @@ ValidVault::finalize( } } + // We don't need to round shares, they are integral MPT auto const accountDeltaShares = deltaShares(tx[sfAccount]); if (!accountDeltaShares) { @@ -764,7 +869,7 @@ ValidVault::finalize( return false; } - if (*accountDeltaShares >= zero) + if (accountDeltaShares->delta >= zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must decrease depositor " @@ -772,15 +877,16 @@ ValidVault::finalize( result = false; } + // We don't need to round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + if (vaultDeltaShares->delta * -1 != accountDeltaShares->delta) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change depositor " @@ -788,15 +894,20 @@ ValidVault::finalize( result = false; } + auto const assetTotalDelta = roundToAsset( + vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale); // Note, vaultBalance is negative (see check above) - if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) + if (assetTotalDelta != vaultPseudoDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: withdrawal and " "assets outstanding must add up"; result = false; } - if (beforeVault.assetsAvailable + *vaultDeltaAssets != afterVault.assetsAvailable) + auto const assetAvailableDelta = roundToAsset( + vaultAsset, afterVault.assetsAvailable - beforeVault.assetsAvailable, minScale); + + if (assetAvailableDelta != vaultPseudoDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: withdrawal and " "assets available must add up"; @@ -827,10 +938,24 @@ ValidVault::finalize( } } - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); - if (vaultDeltaAssets) + auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId); + if (maybeVaultDeltaAssets) { - if (*vaultDeltaAssets >= zero) + auto const totalDelta = DeltaInfo{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + scale(afterVault.assetsTotal, vaultAsset), + scale(beforeVault.assetsTotal, vaultAsset))}; + auto const availableDelta = DeltaInfo{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + scale(afterVault.assetsAvailable, vaultAsset), + scale(beforeVault.assetsAvailable, vaultAsset))}; + auto const minScale = computeMinScale( + vaultAsset, {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + auto const vaultDeltaAssets = + roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); + if (vaultDeltaAssets >= zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must decrease vault " @@ -838,7 +963,9 @@ ValidVault::finalize( result = false; } - if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) + auto const assetsTotalDelta = roundToAsset( + vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale); + if (assetsTotalDelta != vaultDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: clawback and assets outstanding " @@ -846,8 +973,11 @@ ValidVault::finalize( result = false; } - if (beforeVault.assetsAvailable + *vaultDeltaAssets != - afterVault.assetsAvailable) + auto const assetAvailableDelta = roundToAsset( + vaultAsset, + afterVault.assetsAvailable - beforeVault.assetsAvailable, + minScale); + if (assetAvailableDelta != vaultDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: clawback and assets available " @@ -862,15 +992,15 @@ ValidVault::finalize( return false; // That's all we can do } - auto const accountDeltaShares = deltaShares(tx[sfHolder]); - if (!accountDeltaShares) + // We don't need to round shares, they are integral MPT + auto const maybeAccountDeltaShares = deltaShares(tx[sfHolder]); + if (!maybeAccountDeltaShares) { JLOG(j.fatal()) << // "Invariant failed: clawback must change holder shares"; return false; // That's all we can do } - - if (*accountDeltaShares >= zero) + if (maybeAccountDeltaShares->delta >= zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must decrease holder " @@ -878,15 +1008,16 @@ ValidVault::finalize( result = false; } + // We don't need to round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + if (vaultDeltaShares->delta * -1 != maybeAccountDeltaShares->delta) { JLOG(j.fatal()) << // "Invariant failed: clawback must change holder and " @@ -923,4 +1054,19 @@ ValidVault::finalize( return true; } +[[nodiscard]] std::int32_t +ValidVault::computeMinScale(Asset const& asset, std::vector const& numbers) +{ + if (numbers.size() == 0) + return 0; + + auto const max = + std::max_element(numbers.begin(), numbers.end(), [](auto const& a, auto const& b) -> bool { + return a.scale < b.scale; + }); + XRPL_ASSERT_PARTS( + max->scale, "xrpl::ValidVault::computeMinScale", "scale set for destinationDelta"); + return max->scale.value_or(STAmount::cMaxOffset); +} + } // namespace xrpl diff --git a/src/libxrpl/tx/transactors/Lending/LoanBrokerCoverWithdraw.cpp b/src/libxrpl/tx/transactors/Lending/LoanBrokerCoverWithdraw.cpp index 43ff3659ef0..075f6152102 100644 --- a/src/libxrpl/tx/transactors/Lending/LoanBrokerCoverWithdraw.cpp +++ b/src/libxrpl/tx/transactors/Lending/LoanBrokerCoverWithdraw.cpp @@ -121,7 +121,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) return roundToAsset( vaultAsset, tenthBipsOfValue(currentDebtTotal, TenthBips32(sleBroker->at(sfCoverRateMinimum))), - currentDebtTotal.exponent()); + scale(currentDebtTotal, vaultAsset)); }(); if (coverAvail < amount) return tecINSUFFICIENT_FUNDS; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index a01026c8eff..5be316c73eb 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -16,9 +16,14 @@ #include #include #include +#include #include +#include +#include +#include + namespace xrpl { namespace test { @@ -3793,6 +3798,128 @@ class Invariants_test : public beast::unit_test::suite precloseMpt); } + void + testVaultComputeMinScale() + { + using namespace jtx; + + Account const issuer{"issuer"}; + PrettyAsset const vaultAsset = issuer["IOU"]; + + struct TestCase + { + std::string name; + std::int32_t expectedMinScale; + std::vector values; + }; + + NumberMantissaScaleGuard g{MantissaRange::large}; + + auto makeDelta = [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { + return {n, scale(n, vaultAsset.raw())}; + }; + + auto const testCases = std::vector{ + { + .name = "No values", + .expectedMinScale = 0, + .values = {}, + }, + { + .name = "Mixed integer and Number values", + .expectedMinScale = -15, + .values = {makeDelta(1), makeDelta(-1), makeDelta(Number{10, -1})}, + }, + { + .name = "Mixed scales", + .expectedMinScale = -17, + .values = + {makeDelta(Number{1, -2}), makeDelta(Number{5, -3}), makeDelta(Number{3, -2})}, + }, + { + .name = "Equal scales", + .expectedMinScale = -16, + .values = + {makeDelta(Number{1, -1}), makeDelta(Number{5, -1}), makeDelta(Number{1, -1})}, + }, + { + .name = "Mixed mantissa sizes", + .expectedMinScale = -12, + .values = + {makeDelta(Number{1}), + makeDelta(Number{1234, -3}), + makeDelta(Number{12345, -6}), + makeDelta(Number{123, 1})}, + }, + }; + + for (auto const& tc : testCases) + { + testcase("vault computeMinScale: " + tc.name); + + auto const actualScale = ValidVault::computeMinScale(vaultAsset, tc.values); + + BEAST_EXPECTS( + actualScale == tc.expectedMinScale, + "expected: " + std::to_string(tc.expectedMinScale) + + ", actual: " + std::to_string(actualScale)); + for (auto const& num : tc.values) + { + // None of these scales are far enough apart that rounding the + // values would lose information, so check that the rounded + // value matches the original. + auto const actualRounded = roundToAsset(vaultAsset, num.delta, actualScale); + BEAST_EXPECTS( + actualRounded == num.delta, + "number " + to_string(num.delta) + " rounded to scale " + + std::to_string(actualScale) + " is " + to_string(actualRounded)); + } + } + + auto const testCases2 = std::vector{ + { + .name = "False equivalence", + .expectedMinScale = -15, + .values = + { + makeDelta(Number{1234567890123456789, -18}), + makeDelta(Number{12345, -4}), + makeDelta(Number{1}), + }, + }, + }; + + // Unlike the first set of test cases, the values in these test could + // look equivalent if using the wrong scale. + for (auto const& tc : testCases2) + { + testcase("vault computeMinScale: " + tc.name); + + auto const actualScale = ValidVault::computeMinScale(vaultAsset, tc.values); + + BEAST_EXPECTS( + actualScale == tc.expectedMinScale, + "expected: " + std::to_string(tc.expectedMinScale) + + ", actual: " + std::to_string(actualScale)); + std::optional first; + Number firstRounded; + for (auto const& num : tc.values) + { + if (!first) + { + first = num.delta; + firstRounded = roundToAsset(vaultAsset, num.delta, actualScale); + continue; + } + auto const numRounded = roundToAsset(vaultAsset, num.delta, actualScale); + BEAST_EXPECTS( + numRounded != firstRounded, + "at a scale of " + std::to_string(actualScale) + " " + to_string(num.delta) + + " == " + to_string(*first)); + } + } + } + public: void run() override @@ -3818,6 +3945,7 @@ class Invariants_test : public beast::unit_test::suite testValidPseudoAccounts(); testValidLoanBroker(); testVault(); + testVaultComputeMinScale(); } }; diff --git a/src/test/app/LendingHelpers_test.cpp b/src/test/app/LendingHelpers_test.cpp index aae60a252a8..c826b111e2c 100644 --- a/src/test/app/LendingHelpers_test.cpp +++ b/src/test/app/LendingHelpers_test.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include #include diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 607e84abeb7..ba28f25e714 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -2644,7 +2644,7 @@ class Loan_test : public beast::unit_test::suite env(manage(lender, loanKeylet.key, tfLoanDefault), ter(tecNO_PERMISSION)); }); -#if LOANTODO +#if LOAN_TODO // TODO /* @@ -5316,7 +5316,7 @@ class Loan_test : public beast::unit_test::suite } } -#if LOANTODO +#if LOAN_TODO void testLoanPayLateFullPaymentBypassesPenalties() { @@ -6967,14 +6967,145 @@ class Loan_test : public beast::unit_test::suite BEAST_EXPECT(afterSecondCoverAvailable == 0); } + // Tests that vault withdrawals work correctly when the vault has unrealized + // loss from an impaired loan, ensuring the invariant check properly + // accounts for the loss. + void + testWithdrawReflectsUnrealizedLoss() + { + using namespace jtx; + using namespace loan; + using namespace std::chrono_literals; + + testcase("Vault withdraw reflects sfLossUnrealized"); + + // Test constants + static constexpr std::int64_t INITIAL_FUNDING = 1'000'000; + static constexpr std::int64_t LENDER_INITIAL_IOU = 5'000'000; + static constexpr std::int64_t DEPOSITOR_INITIAL_IOU = 1'000'000; + static constexpr std::int64_t BORROWER_INITIAL_IOU = 100'000; + static constexpr std::int64_t DEPOSIT_AMOUNT = 5'000; + static constexpr std::int64_t PRINCIPAL_AMOUNT = 99; + static constexpr std::uint64_t EXPECTED_SHARES_PER_DEPOSITOR = 5'000'000'000; + static constexpr std::uint32_t PAYMENT_INTERVAL = 600; + static constexpr std::uint32_t PAYMENT_TOTAL = 2; + + Env env(*this, all); + + // Setup accounts + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const depositorA{"lpA"}; + Account const depositorB{"lpB"}; + Account const borrower{"borrowerA"}; + + env.fund(XRP(INITIAL_FUNDING), issuer, lender, depositorA, depositorB, borrower); + env.close(); + + // Setup trust lines + PrettyAsset const iouAsset = issuer[iouCurrency]; + env(trust(lender, iouAsset(10'000'000))); + env(trust(depositorA, iouAsset(10'000'000))); + env(trust(depositorB, iouAsset(10'000'000))); + env(trust(borrower, iouAsset(10'000'000))); + env.close(); + + // Fund accounts with IOUs + env(pay(issuer, lender, iouAsset(LENDER_INITIAL_IOU))); + env(pay(issuer, depositorA, iouAsset(DEPOSITOR_INITIAL_IOU))); + env(pay(issuer, depositorB, iouAsset(DEPOSITOR_INITIAL_IOU))); + env(pay(issuer, borrower, iouAsset(BORROWER_INITIAL_IOU))); + env.close(); + + // Create vault and broker, then add deposits from two depositors + auto const broker = createVaultAndBroker(env, iouAsset, lender); + Vault v{env}; + + env(v.deposit({ + .depositor = depositorA, + .id = broker.vaultKeylet().key, + .amount = iouAsset(DEPOSIT_AMOUNT), + }), + ter(tesSUCCESS)); + env(v.deposit({ + .depositor = depositorB, + .id = broker.vaultKeylet().key, + .amount = iouAsset(DEPOSIT_AMOUNT), + }), + ter(tesSUCCESS)); + env.close(); + + // Create a loan + auto const sleBroker = env.le(keylet::loanbroker(broker.brokerID)); + if (!BEAST_EXPECT(sleBroker)) + return; + + auto const loanKeylet = keylet::loan(broker.brokerID, sleBroker->at(sfLoanSequence)); + + env(set(borrower, broker.brokerID, PRINCIPAL_AMOUNT), + sig(sfCounterpartySignature, lender), + paymentTotal(PAYMENT_TOTAL), + paymentInterval(PAYMENT_INTERVAL), + fee(env.current()->fees().base * 2), + ter(tesSUCCESS)); + env.close(); + + // Impair the loan to create unrealized loss + env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tesSUCCESS)); + env.close(); + + // Verify unrealized loss is recorded in the vault + auto const vaultAfterImpair = env.le(broker.vaultKeylet()); + if (!BEAST_EXPECT(vaultAfterImpair)) + return; + + BEAST_EXPECT( + vaultAfterImpair->at(sfLossUnrealized) == broker.asset(PRINCIPAL_AMOUNT).value()); + + // Helper to get share balance for a depositor + auto const shareAsset = vaultAfterImpair->at(sfShareMPTID); + auto const getShareBalance = [&](Account const& depositor) -> std::uint64_t { + auto const token = env.le(keylet::mptoken(shareAsset, depositor.id())); + return token ? token->getFieldU64(sfMPTAmount) : 0; + }; + + // Verify both depositors have equal shares + auto const sharesLpA = getShareBalance(depositorA); + auto const sharesLpB = getShareBalance(depositorB); + BEAST_EXPECT(sharesLpA == EXPECTED_SHARES_PER_DEPOSITOR); + BEAST_EXPECT(sharesLpB == EXPECTED_SHARES_PER_DEPOSITOR); + BEAST_EXPECT(sharesLpA == sharesLpB); + + // Helper to attempt withdrawal + auto const attemptWithdrawShares = [&](Account const& depositor, + std::uint64_t shareAmount, + TER expected) { + STAmount const shareAmt{MPTIssue{shareAsset}, Number(shareAmount)}; + env(v.withdraw( + {.depositor = depositor, .id = broker.vaultKeylet().key, .amount = shareAmt}), + ter(expected)); + env.close(); + }; + + // Regression test: Both depositors should successfully withdraw despite + // unrealized loss. Previously failed with invariant violation: + // "withdrawal must change vault and destination balance by equal + // amount". This was caused by sharesToAssetsWithdraw rounding down, + // creating a mismatch where vaultDeltaAssets * -1 != destinationDelta + // when unrealized loss exists. + attemptWithdrawShares(depositorA, sharesLpA, tesSUCCESS); + attemptWithdrawShares(depositorB, sharesLpB, tesSUCCESS); + } + public: void run() override { -#if LOANTODO +#if LOAN_TODO testLoanPayLateFullPaymentBypassesPenalties(); testLoanCoverMinimumRoundingExploit(); #endif + testWithdrawReflectsUnrealizedLoss(); testInvalidLoanSet(); testCoverDepositWithdrawNonTransferableMPT(); From eb46d9e4171b8038b0f8d9e111dbf35146a1f7e5 Mon Sep 17 00:00:00 2001 From: JCW Date: Tue, 10 Mar 2026 15:54:37 +0000 Subject: [PATCH 07/17] Integrate Permissioned Domain & Credential Checks for Lending Protocol Signed-off-by: JCW --- include/xrpl/protocol/LedgerFormats.h | 5 +- include/xrpl/protocol/TxFlags.h | 4 + .../xrpl/protocol/detail/ledger_entries.macro | 1 + .../xrpl/protocol/detail/transactions.macro | 1 + .../tx/transactors/lending/LoanBrokerSet.h | 3 + src/libxrpl/tx/invariants/InvariantCheck.cpp | 3 +- src/libxrpl/tx/invariants/LoanInvariant.cpp | 6 + .../tx/transactors/lending/LoanBrokerSet.cpp | 83 +++++ .../tx/transactors/lending/LoanSet.cpp | 22 ++ src/test/app/LoanBroker_test.cpp | 338 ++++++++++++++++++ src/test/app/Loan_test.cpp | 244 +++++++++++++ src/test/jtx/TestHelpers.h | 2 + 12 files changed, 710 insertions(+), 2 deletions(-) diff --git a/include/xrpl/protocol/LedgerFormats.h b/include/xrpl/protocol/LedgerFormats.h index a43f6a71340..0e5a6547dd2 100644 --- a/include/xrpl/protocol/LedgerFormats.h +++ b/include/xrpl/protocol/LedgerFormats.h @@ -198,7 +198,10 @@ enum LedgerEntryType : std::uint16_t { LEDGER_OBJECT(Loan, \ LSF_FLAG(lsfLoanDefault, 0x00010000) \ LSF_FLAG(lsfLoanImpaired, 0x00020000) \ - LSF_FLAG(lsfLoanOverpayment, 0x00040000)) /* True, loan allows overpayments */ + LSF_FLAG(lsfLoanOverpayment, 0x00040000)) /* True, loan allows overpayments */ \ + \ + LEDGER_OBJECT(LoanBroker, \ + LSF_FLAG(lsfLoanBrokerPrivate, 0x00010000)) // clang-format on diff --git a/include/xrpl/protocol/TxFlags.h b/include/xrpl/protocol/TxFlags.h index 7c2085109ff..e80d43f7b1c 100644 --- a/include/xrpl/protocol/TxFlags.h +++ b/include/xrpl/protocol/TxFlags.h @@ -212,6 +212,10 @@ inline constexpr FlagValue tfUniversalMask = ~tfUniversal; TF_FLAG(tfLoanDefault, 0x00010000) \ TF_FLAG(tfLoanImpair, 0x00020000) \ TF_FLAG(tfLoanUnimpair, 0x00040000), \ + MASK_ADJ(0)) \ + \ + TRANSACTION(LoanBrokerSet, \ + TF_FLAG(tfLoanBrokerPrivate, 0x00010000) , \ MASK_ADJ(0)) // clang-format on diff --git a/include/xrpl/protocol/detail/ledger_entries.macro b/include/xrpl/protocol/detail/ledger_entries.macro index 216f404bec7..83d671e0cd9 100644 --- a/include/xrpl/protocol/detail/ledger_entries.macro +++ b/include/xrpl/protocol/detail/ledger_entries.macro @@ -515,6 +515,7 @@ LEDGER_ENTRY(ltLOAN_BROKER, 0x0088, LoanBroker, loan_broker, ({ {sfCoverAvailable, soeDEFAULT}, {sfCoverRateMinimum, soeDEFAULT}, {sfCoverRateLiquidation, soeDEFAULT}, + {sfDomainID, soeOPTIONAL}, })) /** A ledger object representing a loan between a Borrower and a Loan Broker diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 15881effc8f..4037944dd8e 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -943,6 +943,7 @@ TRANSACTION(ttLOAN_BROKER_SET, 74, LoanBrokerSet, {sfDebtMaximum, soeOPTIONAL}, {sfCoverRateMinimum, soeOPTIONAL}, {sfCoverRateLiquidation, soeOPTIONAL}, + {sfDomainID, soeOPTIONAL}, })) /** This transaction deletes a Loan Broker */ diff --git a/include/xrpl/tx/transactors/lending/LoanBrokerSet.h b/include/xrpl/tx/transactors/lending/LoanBrokerSet.h index ce1e0697918..94ff4d31a86 100644 --- a/include/xrpl/tx/transactors/lending/LoanBrokerSet.h +++ b/include/xrpl/tx/transactors/lending/LoanBrokerSet.h @@ -22,6 +22,9 @@ class LoanBrokerSet : public Transactor static std::vector> const& getValueFields(); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static TER preclaim(PreclaimContext const& ctx); diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index 79c593c57c2..f4bb65e0d78 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -947,7 +947,8 @@ NoModifiedUnmodifiableFields::finalize( fieldChanged(before, after, sfOwner) || fieldChanged(before, after, sfManagementFeeRate) || fieldChanged(before, after, sfCoverRateMinimum) || - fieldChanged(before, after, sfCoverRateLiquidation); + fieldChanged(before, after, sfCoverRateLiquidation) || + fieldChanged(before, after, sfFlags); break; case ltLOAN: /* diff --git a/src/libxrpl/tx/invariants/LoanInvariant.cpp b/src/libxrpl/tx/invariants/LoanInvariant.cpp index 01c4da46ac9..0c2dc8a3131 100644 --- a/src/libxrpl/tx/invariants/LoanInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanInvariant.cpp @@ -186,6 +186,12 @@ ValidLoanBroker::finalize( "is less than pseudo-account asset balance"; return false; } + + if (after->at(~sfDomainID) && !after->isFlag(lsfLoanBrokerPrivate)) + { + JLOG(j.fatal()) << "Invariant failed: DomainID is set on public Loan Broker"; + return false; + } } return true; } diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index c825a7fafe6..41922e1c18f 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -1,6 +1,7 @@ #include // #include +#include #include namespace xrpl { @@ -29,8 +30,11 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) if (!validNumericRange(tx[~sfDebtMaximum], Number(maxMPTokenAmount), Number(0))) return temINVALID; + auto const domainID = tx.at(~sfDomainID); + if (tx.isFieldPresent(sfLoanBrokerID)) { + // We're modifying an existing LoanBroker. // Fixed fields can not be specified if we're modifying an existing // LoanBroker Object if (tx.isFieldPresent(sfManagementFeeRate) || tx.isFieldPresent(sfCoverRateMinimum) || @@ -39,6 +43,43 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) if (tx[sfLoanBrokerID] == beast::zero) return temINVALID; + + if (ctx.rules.enabled(fixLendingProtocolV1_1)) + { + if (tx.isFlag(tfLoanBrokerPrivate)) + { + // Cannot change private flag on existing broker + return temINVALID; + } + } + } + else + { + // We're creating a new LoanBroker. + if (ctx.rules.enabled(fixLendingProtocolV1_1)) + { + if (domainID) + { + if (*domainID == beast::zero) + { + // DomainID must not be zero if provided + return temMALFORMED; + } + if (!tx.isFlag(tfLoanBrokerPrivate)) + { + // Public brokers cannot have a DomainID + return temINVALID; + } + } + } + } + + if (!ctx.rules.enabled(fixLendingProtocolV1_1)) + { + if (tx.isFlag(tfLoanBrokerPrivate) || tx.isFieldPresent(sfDomainID)) + { + return temDISABLED; + } } if (auto const vaultID = tx.at(~sfVaultID)) @@ -68,6 +109,16 @@ LoanBrokerSet::getValueFields() return valueFields; } +std::uint32_t +LoanBrokerSet::getFlagsMask(PreflightContext const& ctx) +{ + if (ctx.rules.enabled(fixLendingProtocolV1_1)) + { + return tfLoanSetMask; + } + return tfUniversalMask; +} + TER LoanBrokerSet::preclaim(PreclaimContext const& ctx) { @@ -90,6 +141,17 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; } + auto const domainID = tx[~sfDomainID]; + if (domainID && *domainID != beast::zero) + { + auto const sleDomain = ctx.view.read(keylet::permissionedDomain(*domainID)); + if (!sleDomain) + { + JLOG(ctx.j.warn()) << "Domain does not exist."; + return tecOBJECT_NOT_FOUND; + } + } + if (auto const brokerID = tx[~sfLoanBrokerID]) { // Updating an existing Broker @@ -121,6 +183,11 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx) return tecLIMIT_EXCEEDED; } } + + if (domainID && *domainID != beast::zero && !sleBroker->isFlag(lsfLoanBrokerPrivate)) + { + return tecNO_PERMISSION; + } } else { @@ -179,6 +246,14 @@ LoanBrokerSet::doApply() if (auto const debtMax = tx[~sfDebtMaximum]) broker->at(sfDebtMaximum) = *debtMax; + auto domainID = tx[~sfDomainID]; + if (domainID && *domainID == beast::zero) + { + domainID = std::nullopt; + } + + broker->at(~sfDomainID) = domainID; + view.update(broker); associateAsset(*broker, vaultAsset); @@ -250,6 +325,14 @@ LoanBrokerSet::doApply() if (auto const coverLiq = tx[~sfCoverRateLiquidation]) broker->at(sfCoverRateLiquidation) = *coverLiq; + if (tx.isFlag(tfLoanBrokerPrivate)) + { + broker->setFlag(lsfLoanBrokerPrivate); + } + + auto const domainID = tx[~sfDomainID]; + broker->at(~sfDomainID) = domainID; + view.insert(broker); associateAsset(*broker, vaultAsset); diff --git a/src/libxrpl/tx/transactors/lending/LoanSet.cpp b/src/libxrpl/tx/transactors/lending/LoanSet.cpp index f74522e737d..a3ce6f79f9d 100644 --- a/src/libxrpl/tx/transactors/lending/LoanSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanSet.cpp @@ -1,5 +1,6 @@ #include // +#include #include #include #include @@ -329,6 +330,27 @@ LoanSet::preclaim(PreclaimContext const& ctx) return ret; } + if (brokerSle->isFlag(lsfLoanBrokerPrivate)) + { + auto const domainID = brokerSle->at(~sfDomainID); + if (!domainID) + { + JLOG(ctx.j.warn()) << "Private LoanBroker must have a DomainID."; + return tecNO_AUTH; + } + + auto const sleDomain = ctx.view.read(keylet::permissionedDomain(*domainID)); + if (!sleDomain) + { + JLOG(ctx.j.warn()) << "Domain does not exist."; // LCOV_EXCL_LINE + return tecOBJECT_NOT_FOUND; // LCOV_EXCL_LINE + } + + if (auto const ter = credentials::validDomain(ctx.view, *domainID, borrower); + !isTesSuccess(ter)) + return ter; + } + return tesSUCCESS; } diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 4c108d9007f..1952508d473 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include #include @@ -1797,6 +1799,338 @@ class LoanBroker_test : public beast::unit_test::suite BEAST_EXPECT(env.enabled(fixLendingProtocolV1_1)); } + void + testPrivateLoanBroker() + { + testcase("Private Loan Broker with Permissioned Domain"); + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; + Account const credIssuer{"credIssuer"}; + std::string const credType = "LoanCredential"; + + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, alice, credIssuer); + env.close(); + + // Create an IOU asset and vault + env(trust(alice, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset{issuer["IOU"]}; + env(pay(issuer, alice, asset(100'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env.close(); + + // Create a permissioned domain + pdomain::Credentials const credentials{{credIssuer, credType}}; + env(pdomain::setTx(credIssuer, credentials)); + env.close(); + auto const domainId = pdomain::getNewDomain(env.meta()); + BEAST_EXPECT(domainId != beast::zero); + + // Test 1: Cannot set tfLoanBrokerPrivate without fixLendingProtocolV1_1 + { + Env envNoFix{*this}; + envNoFix.disableFeature(fixLendingProtocolV1_1); + Vault vault2{envNoFix}; + + envNoFix.fund(XRP(100'000), issuer, alice); + envNoFix.close(); + envNoFix(trust(alice, issuer["IOU"](1'000'000))); + envNoFix.close(); + envNoFix(pay(issuer, alice, asset(100'000))); + envNoFix.close(); + + auto [tx2, vaultKeylet2] = vault2.create({.owner = alice, .asset = asset}); + envNoFix(tx2); + envNoFix.close(); + + // tfLoanBrokerPrivate should be disabled + envNoFix(set(alice, vaultKeylet2.key, tfLoanBrokerPrivate), ter(temINVALID_FLAG)); + } + + // Test 2: Create a private loan broker with DomainID + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), domainID(domainId)); + env.close(); + + // Verify the broker was created with the correct flags and DomainID + { + auto const sleBroker = env.le(brokerKeylet); + BEAST_EXPECT(sleBroker); + if (sleBroker) + { + BEAST_EXPECT(sleBroker->isFlag(lsfLoanBrokerPrivate)); + BEAST_EXPECT(sleBroker->at(~sfDomainID) == domainId); + } + } + + // Test 3: Cannot create public broker with DomainID + { + // Public broker (no tfLoanBrokerPrivate) with DomainID should fail + env(set(alice, vaultKeylet.key), domainID(domainId), ter(temINVALID)); + } + + // Test 4: Cannot create broker with zero DomainID + { + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), + domainID(beast::zero), + ter(temMALFORMED)); + } + + // Test 5: Cannot create broker with non-existent DomainID + { + uint256 fakeDomainId; + BEAST_EXPECT(fakeDomainId.parseHex( + "1234567890ABCDEF1234567890ABCDEF1234567890ABCDEF1234567890ABCDEF")); + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), + domainID(fakeDomainId), + ter(tecOBJECT_NOT_FOUND)); + } + } + + void + testPrivateLoanBrokerModify() + { + testcase("Modify Private Loan Broker DomainID"); + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; + Account const credIssuer{"credIssuer"}; + std::string const credType = "LoanCredential"; + + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, alice, credIssuer); + env.close(); + + // Create an IOU asset and vault + env(trust(alice, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset{issuer["IOU"]}; + env(pay(issuer, alice, asset(100'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env.close(); + + // Create two permissioned domains + pdomain::Credentials const credentials{{credIssuer, credType}}; + env(pdomain::setTx(credIssuer, credentials)); + env.close(); + auto const domainId1 = pdomain::getNewDomain(env.meta()); + + env(pdomain::setTx(credIssuer, credentials)); + env.close(); + auto const domainId2 = pdomain::getNewDomain(env.meta()); + + // Create a private loan broker with DomainID + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), domainID(domainId1)); + env.close(); + + // Test 1: Modify DomainID to a different domain + env(set(alice, vaultKeylet.key), loanBrokerID(brokerKeylet.key), domainID(domainId2)); + env.close(); + { + auto const sleBroker = env.le(brokerKeylet); + BEAST_EXPECT(sleBroker); + if (sleBroker) + { + BEAST_EXPECT(sleBroker->at(~sfDomainID) == domainId2); + } + } + + // Test 2: Clear DomainID by setting to zero + env(set(alice, vaultKeylet.key), loanBrokerID(brokerKeylet.key), domainID(beast::zero)); + env.close(); + { + auto const sleBroker = env.le(brokerKeylet); + BEAST_EXPECT(sleBroker); + if (sleBroker) + { + BEAST_EXPECT(!sleBroker->at(~sfDomainID)); + } + } + + // Test 3: Set DomainID back + env(set(alice, vaultKeylet.key), loanBrokerID(brokerKeylet.key), domainID(domainId1)); + env.close(); + { + auto const sleBroker = env.le(brokerKeylet); + BEAST_EXPECT(sleBroker); + if (sleBroker) + { + BEAST_EXPECT(sleBroker->at(~sfDomainID) == domainId1); + } + } + + // Test 4: Cannot set tfLoanBrokerPrivate when modifying (flag is immutable) + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), + loanBrokerID(brokerKeylet.key), + ter(temINVALID)); + + // Test 5: Create a public broker and try to set DomainID on it + auto const publicBrokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); // No tfLoanBrokerPrivate = public + env.close(); + { + auto const sleBroker = env.le(publicBrokerKeylet); + BEAST_EXPECT(sleBroker); + if (sleBroker) + { + BEAST_EXPECT(!sleBroker->isFlag(lsfLoanBrokerPrivate)); + } + } + + // Cannot set non-zero DomainID on public broker + env(set(alice, vaultKeylet.key), + loanBrokerID(publicBrokerKeylet.key), + domainID(domainId1), + ter(tecNO_PERMISSION)); + + // But zero DomainID on public broker is allowed (no-op) + env(set(alice, vaultKeylet.key), + loanBrokerID(publicBrokerKeylet.key), + domainID(beast::zero)); + env.close(); + } + + void + testPrivateBrokerUnsetDomainBlocksLoans() + { + testcase("Unsetting DomainID on private broker blocks new loans"); + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; // Broker owner + Account const bob{"bob"}; // Borrower + Account const credIssuer{"credIssuer"}; + std::string const credType = "LoanCredential"; + + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, alice, bob, credIssuer); + env.close(); + + // Create an IOU asset and vault + env(trust(alice, issuer["IOU"](1'000'000))); + env(trust(bob, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset{issuer["IOU"]}; + env(pay(issuer, alice, asset(100'000))); + env(pay(issuer, bob, asset(10'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env.close(); + + // Deposit into vault + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50'000)})); + env.close(); + + // Create a permissioned domain + pdomain::Credentials const credentials{{credIssuer, credType}}; + env(pdomain::setTx(credIssuer, credentials)); + env.close(); + auto const domainId = pdomain::getNewDomain(env.meta()); + + // Create credential for bob and accept it + env(credentials::create(bob, credIssuer, credType)); + env.close(); + env(credentials::accept(bob, credIssuer, credType)); + env.close(); + + // Create a private loan broker with DomainID + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), domainID(domainId)); + env.close(); + + // Deposit cover into broker + env(coverDeposit(alice, brokerKeylet.key, asset(1000))); + env.close(); + + // Bob can create a loan (has credentials, domain is set) + { + auto const loanKeylet = keylet::loan(brokerKeylet.key, 1); + auto setTx = env.jt(loan::set(bob, brokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx); + env.close(); + + BEAST_EXPECT(env.le(loanKeylet)); + } + + // Now unset the DomainID by setting it to zero + env(set(alice, vaultKeylet.key), loanBrokerID(brokerKeylet.key), domainID(beast::zero)); + env.close(); + + // Verify DomainID is unset + { + auto const sleBroker = env.le(brokerKeylet); + BEAST_EXPECT(sleBroker); + if (sleBroker) + { + BEAST_EXPECT(!sleBroker->at(~sfDomainID)); + BEAST_EXPECT(sleBroker->isFlag(lsfLoanBrokerPrivate)); + } + } + + // Bob cannot create a new loan (private broker has no domain configured) + { + auto const loanKeylet2 = keylet::loan(brokerKeylet.key, 2); + auto setTx = env.jt(loan::set(bob, brokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx, ter(tecNO_AUTH)); + + BEAST_EXPECT(!env.le(loanKeylet2)); + } + + // Set the DomainID back + env(set(alice, vaultKeylet.key), loanBrokerID(brokerKeylet.key), domainID(domainId)); + env.close(); + + // Now bob can create a loan again + { + auto const loanKeylet3 = keylet::loan(brokerKeylet.key, 2); + auto setTx = env.jt(loan::set(bob, brokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx); + env.close(); + + BEAST_EXPECT(env.le(loanKeylet3)); + } + } + public: void run() override @@ -1819,6 +2153,10 @@ class LoanBroker_test : public beast::unit_test::suite testRIPD4274(); + testPrivateLoanBroker(); + testPrivateLoanBrokerModify(); + testPrivateBrokerUnsetDomainBlocksLoans(); + // TODO: Write clawback failure tests with an issuer / MPT that doesn't // have the right flags set. } diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 715d03c3df3..78cb34056a9 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -1,7 +1,9 @@ #include // #include +#include #include +#include #include #include @@ -7097,6 +7099,245 @@ class Loan_test : public beast::unit_test::suite attemptWithdrawShares(depositorB, sharesLpB, tesSUCCESS); } + void + testPrivateBrokerCredentials() + { + testcase("Private Broker Credential Validation"); + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; // Broker owner + Account const bob{"bob"}; // Borrower with credentials + Account const carol{"carol"}; // Borrower without credentials + Account const credIssuer{"credIssuer"}; + std::string const credType = "LoanCredential"; + + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, alice, bob, carol, credIssuer); + env.close(); + + // Create an IOU asset and vault + env(trust(alice, issuer["IOU"](1'000'000))); + env(trust(bob, issuer["IOU"](1'000'000))); + env(trust(carol, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset{issuer["IOU"]}; + env(pay(issuer, alice, asset(100'000))); + env(pay(issuer, bob, asset(10'000))); + env(pay(issuer, carol, asset(10'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env.close(); + + // Deposit into vault + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50'000)})); + env.close(); + + // Create a permissioned domain + pdomain::Credentials const credentials{{credIssuer, credType}}; + env(pdomain::setTx(credIssuer, credentials)); + env.close(); + auto const domainId = pdomain::getNewDomain(env.meta()); + + // Create a private loan broker with DomainID + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), domainID(domainId)); + env.close(); + + // Deposit cover into broker + env(coverDeposit(alice, brokerKeylet.key, asset(1000))); + env.close(); + + // Test 1: Borrower without credentials cannot create loan + { + auto setTx = env.jt(loan::set(carol, brokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx, ter(tecNO_AUTH)); + } + + // Create credential for bob and accept it + env(credentials::create(bob, credIssuer, credType)); + env.close(); + env(credentials::accept(bob, credIssuer, credType)); + env.close(); + + // Test 2: Borrower with valid credentials can create loan + { + auto const loanKeylet = keylet::loan(brokerKeylet.key, 1); + auto setTx = env.jt(loan::set(bob, brokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx); + env.close(); + + BEAST_EXPECT(env.le(loanKeylet)); + } + + // Test 3: Private broker without DomainID configured rejects loans + { + // Create another private broker without DomainID + auto const broker2Keylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate)); // No domainID + env.close(); + + env(coverDeposit(alice, broker2Keylet.key, asset(1000))); + env.close(); + + // Even bob with credentials should fail - broker has no domain configured + auto setTx = env.jt(loan::set(bob, broker2Keylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx, ter(tecNO_AUTH)); + } + + // Test 4: Public broker allows anyone to create loan + { + auto const publicBrokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); // No tfLoanBrokerPrivate = public + env.close(); + + env(coverDeposit(alice, publicBrokerKeylet.key, asset(1000))); + env.close(); + + // Carol without credentials can create loan on public broker + auto const loanKeylet = keylet::loan(publicBrokerKeylet.key, 1); + auto setTx = env.jt(loan::set(carol, publicBrokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx); + env.close(); + + BEAST_EXPECT(env.le(loanKeylet)); + } + } + + void + testPrivateBrokerLoanPayAfterCredentialRevoked() + { + testcase("LoanPay works after borrower credential is revoked"); + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; // Broker owner + Account const bob{"bob"}; // Borrower + Account const credIssuer{"credIssuer"}; + std::string const credType = "LoanCredential"; + + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, alice, bob, credIssuer); + env.close(); + + // Create an IOU asset and vault + env(trust(alice, issuer["IOU"](1'000'000))); + env(trust(bob, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset{issuer["IOU"]}; + env(pay(issuer, alice, asset(100'000))); + env(pay(issuer, bob, asset(10'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env.close(); + + // Deposit into vault + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50'000)})); + env.close(); + + // Create a permissioned domain + pdomain::Credentials const credentials{{credIssuer, credType}}; + env(pdomain::setTx(credIssuer, credentials)); + env.close(); + auto const domainId = pdomain::getNewDomain(env.meta()); + + // Create credential for bob and accept it + env(credentials::create(bob, credIssuer, credType)); + env.close(); + env(credentials::accept(bob, credIssuer, credType)); + env.close(); + + // Create a private loan broker with DomainID + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), domainID(domainId)); + env.close(); + + // Deposit cover into broker + env(coverDeposit(alice, brokerKeylet.key, asset(1000))); + env.close(); + + // Create a loan for bob + auto const loanKeylet = keylet::loan(brokerKeylet.key, 1); + { + auto setTx = env.jt(loan::set(bob, brokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx); + env.close(); + + BEAST_EXPECT(env.le(loanKeylet)); + } + + // Now delete bob's credential + env(credentials::deleteCred(bob, bob, credIssuer, credType)); + env.close(); + + // Verify credential is gone + auto const credKeylet = credentials::keylet(bob, credIssuer, credType); + BEAST_EXPECT(!env.le(credKeylet)); + + // Bob should still be able to make a loan payment even without credentials + env(loan::pay(bob, loanKeylet.key, asset(51))); + env.close(); + + // Verify the loan still exists and payment was processed + auto const sleLoan = env.le(loanKeylet); + BEAST_EXPECT(sleLoan); + + // Bob should NOT be able to create a NEW loan (no credentials) + { + auto const newLoanKeylet = keylet::loan(brokerKeylet.key, 2); + auto setTx = env.jt(loan::set(bob, brokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx, ter(tecNO_AUTH)); + + BEAST_EXPECT(!env.le(newLoanKeylet)); + } + } + public: void run() override @@ -7151,6 +7392,9 @@ class Loan_test : public beast::unit_test::suite testLoanPayBrokerOwnerNoPermissionedDomainMPT(); testLoanSetBrokerOwnerNoPermissionedDomainMPT(); testSequentialFLCDepletion(); + + testPrivateBrokerCredentials(); + testPrivateBrokerLoanPayAfterCredentialRevoked(); } }; diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index 2836748ec3c..d47338df094 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -717,6 +717,8 @@ auto const coverRateLiquidation = auto const destination = JTxFieldWrapper(sfDestination); +auto const domainID = JTxFieldWrapper(sfDomainID); + } // namespace loanBroker /* Loan */ From 85ede3e422ed9a29575a582ad81b0c96d06dddb5 Mon Sep 17 00:00:00 2001 From: JCW Date: Mon, 23 Mar 2026 17:45:23 +0000 Subject: [PATCH 08/17] Add unit tests for invariant checks Signed-off-by: JCW --- src/test/app/Invariants_test.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 5be316c73eb..e9fdeb3a079 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -1987,6 +1987,7 @@ class Invariants_test : public beast::unit_test::suite [](SLE::pointer& sle) { sle->at(sfManagementFeeRate) += 1; }, [](SLE::pointer& sle) { sle->at(sfCoverRateMinimum) += 1; }, [](SLE::pointer& sle) { sle->at(sfCoverRateLiquidation) += 1; }, + [](SLE::pointer& sle) { sle->setFlag(lsfLoanBrokerPrivate); }, [](SLE::pointer& sle) { sle->at(sfLedgerEntryType) += 1; }, [](SLE::pointer& sle) { sle->at(sfLedgerIndex) = sle->at(sfVaultID).value(); }, }); @@ -2251,6 +2252,25 @@ class Invariants_test : public beast::unit_test::suite STTx{ttLOAN_BROKER_SET, [](STObject& tx) {}}, {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, createLoanBroker); + + doInvariantCheck( + {{"DomainID is set on public Loan Broker"}}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + if (loanBrokerKeylet.type != ltLOAN_BROKER) + return false; + auto sleBroker = ac.view().peek(loanBrokerKeylet); + if (!sleBroker) + return false; + // Set DomainID without lsfLoanBrokerPrivate flag + sleBroker->clearFlag(lsfLoanBrokerPrivate); + sleBroker->at(sfDomainID) = uint256(42); + ac.view().update(sleBroker); + return true; + }, + XRPAmount{}, + STTx{ttLOAN_BROKER_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + createLoanBroker); } } From afa94e5cdcc14f1081c9d467859078c0d2ebeea4 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 23 Mar 2026 17:47:42 +0000 Subject: [PATCH 09/17] Apply suggestion from @Tapanito Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> --- src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index 41922e1c18f..1364b854c06 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -46,9 +46,9 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) if (ctx.rules.enabled(fixLendingProtocolV1_1)) { + // Cannot change private flag on existing broker if (tx.isFlag(tfLoanBrokerPrivate)) { - // Cannot change private flag on existing broker return temINVALID; } } From d908d205f2d014ce428023c974d6a126fc95a549 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 23 Mar 2026 17:51:21 +0000 Subject: [PATCH 10/17] Apply suggestion from @Tapanito Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> --- src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index 1364b854c06..ee41fbe2002 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -184,7 +184,11 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx) } } - if (domainID && *domainID != beast::zero && !sleBroker->isFlag(lsfLoanBrokerPrivate)) + if (ctx.view.rules().enabled(fixLendingProtocolV1_1)) + { + if (sleBroker->isFlag(lsfLoanBrokerPrivate) && domainID) + return tecNO_PERMISSION; + } { return tecNO_PERMISSION; } From ee61591d29519f4da988e879456fcd924e9db83f Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 23 Mar 2026 17:51:53 +0000 Subject: [PATCH 11/17] Apply suggestion from @Tapanito Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> --- .../tx/transactors/lending/LoanBrokerSet.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index ee41fbe2002..1fd8ada7613 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -250,14 +250,22 @@ LoanBrokerSet::doApply() if (auto const debtMax = tx[~sfDebtMaximum]) broker->at(sfDebtMaximum) = *debtMax; - auto domainID = tx[~sfDomainID]; - if (domainID && *domainID == beast::zero) + if (ctx_.view().rules().enabled(fixLendingProtocolV1_1) && + broker->isFlag(lsfLoanBrokerPrivate)) { - domainID = std::nullopt; + if (auto const domainID = tx[~sfDomainID]) + { + if (*domainID != beast::zero) + { + broker->setFieldH256(sfDomainID, *domainID); + } + else if (broker->isFieldPresent(sfDomainID)) + { + broker->makeFieldAbsent(sfDomainID); + } + } } - broker->at(~sfDomainID) = domainID; - view.update(broker); associateAsset(*broker, vaultAsset); From 54da6c147829879d0a6e01f400b7316f0f69edc7 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 23 Mar 2026 17:52:45 +0000 Subject: [PATCH 12/17] Apply suggestions from code review Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> --- src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp | 12 +++++++----- src/libxrpl/tx/transactors/lending/LoanSet.cpp | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index 1fd8ada7613..b056e937f02 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -337,14 +337,16 @@ LoanBrokerSet::doApply() if (auto const coverLiq = tx[~sfCoverRateLiquidation]) broker->at(sfCoverRateLiquidation) = *coverLiq; - if (tx.isFlag(tfLoanBrokerPrivate)) + if (ctx_.view().rules().enabled(fixLendingProtocolV1_1)) { - broker->setFlag(lsfLoanBrokerPrivate); + if (tx.isFlag(tfLoanBrokerPrivate)) + { + broker->setFlag(lsfLoanBrokerPrivate); + if (auto domainID = tx[~sfDomainID]) + broker->setFieldH256(sfDomainID, *domainID); + } } - auto const domainID = tx[~sfDomainID]; - broker->at(~sfDomainID) = domainID; - view.insert(broker); associateAsset(*broker, vaultAsset); diff --git a/src/libxrpl/tx/transactors/lending/LoanSet.cpp b/src/libxrpl/tx/transactors/lending/LoanSet.cpp index a3ce6f79f9d..7c13743aa8a 100644 --- a/src/libxrpl/tx/transactors/lending/LoanSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanSet.cpp @@ -330,7 +330,7 @@ LoanSet::preclaim(PreclaimContext const& ctx) return ret; } - if (brokerSle->isFlag(lsfLoanBrokerPrivate)) + if (ctx.view.rules().enabled(fixLendingProtocolV1_1) && brokerSle->isFlag(lsfLoanBrokerPrivate)) { auto const domainID = brokerSle->at(~sfDomainID); if (!domainID) From e708999e14d1b5616ae7cfa6826907e099041b7a Mon Sep 17 00:00:00 2001 From: JCW Date: Mon, 23 Mar 2026 18:42:48 +0000 Subject: [PATCH 13/17] Address PR comments Signed-off-by: JCW --- .../tx/transactors/lending/LoanBrokerSet.cpp | 36 ++--- .../tx/transactors/lending/LoanSet.cpp | 13 ++ src/test/app/LoanBroker_test.cpp | 133 +++++++++++++++++- 3 files changed, 162 insertions(+), 20 deletions(-) diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index b056e937f02..f1db33c0f01 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -30,7 +30,13 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) if (!validNumericRange(tx[~sfDebtMaximum], Number(maxMPTokenAmount), Number(0))) return temINVALID; - auto const domainID = tx.at(~sfDomainID); + if (!ctx.rules.enabled(fixLendingProtocolV1_1)) + { + if (tx.isFlag(tfLoanBrokerPrivate) || tx.isFieldPresent(sfDomainID)) + { + return temDISABLED; + } + } if (tx.isFieldPresent(sfLoanBrokerID)) { @@ -58,6 +64,7 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) // We're creating a new LoanBroker. if (ctx.rules.enabled(fixLendingProtocolV1_1)) { + auto const domainID = tx.at(~sfDomainID); if (domainID) { if (*domainID == beast::zero) @@ -74,14 +81,6 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) } } - if (!ctx.rules.enabled(fixLendingProtocolV1_1)) - { - if (tx.isFlag(tfLoanBrokerPrivate) || tx.isFieldPresent(sfDomainID)) - { - return temDISABLED; - } - } - if (auto const vaultID = tx.at(~sfVaultID)) { if (*vaultID == beast::zero) @@ -141,14 +140,17 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; } - auto const domainID = tx[~sfDomainID]; - if (domainID && *domainID != beast::zero) + if (ctx.view.rules().enabled(fixLendingProtocolV1_1)) { - auto const sleDomain = ctx.view.read(keylet::permissionedDomain(*domainID)); - if (!sleDomain) + auto const domainID = tx[~sfDomainID]; + if (domainID && *domainID != beast::zero) { - JLOG(ctx.j.warn()) << "Domain does not exist."; - return tecOBJECT_NOT_FOUND; + auto const sleDomain = ctx.view.read(keylet::permissionedDomain(*domainID)); + if (!sleDomain) + { + JLOG(ctx.j.warn()) << "Domain does not exist."; + return tecOBJECT_NOT_FOUND; + } } } @@ -186,12 +188,10 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx) if (ctx.view.rules().enabled(fixLendingProtocolV1_1)) { + auto const domainID = tx[~sfDomainID]; if (sleBroker->isFlag(lsfLoanBrokerPrivate) && domainID) return tecNO_PERMISSION; } - { - return tecNO_PERMISSION; - } } else { diff --git a/src/libxrpl/tx/transactors/lending/LoanSet.cpp b/src/libxrpl/tx/transactors/lending/LoanSet.cpp index 7c13743aa8a..dc1add691e1 100644 --- a/src/libxrpl/tx/transactors/lending/LoanSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanSet.cpp @@ -390,6 +390,19 @@ LoanSet::doApply() { return tefBAD_LEDGER; // LCOV_EXCL_LINE } + + if (brokerSle->isFlag(lsfLoanBrokerPrivate)) + { + auto const domainID = brokerSle->at(~sfDomainID); + if (!domainID) + { + return tefBAD_LEDGER; // LCOV_EXCL_LINE + } + + if (auto const ter = verifyValidDomain(view, account_, *domainID, j_); !isTesSuccess(ter)) + return ter; + } + auto const principalRequested = tx[sfPrincipalRequested]; auto vaultAvailableProxy = vaultSle->at(sfAssetsAvailable); diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 1952508d473..ef12f8f2de6 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -1854,6 +1854,9 @@ class LoanBroker_test : public beast::unit_test::suite // tfLoanBrokerPrivate should be disabled envNoFix(set(alice, vaultKeylet2.key, tfLoanBrokerPrivate), ter(temINVALID_FLAG)); + + // sfDomainID should also be disabled without fixLendingProtocolV1_1 + envNoFix(set(alice, vaultKeylet2.key), domainID(domainId), ter(temDISABLED)); } // Test 2: Create a private loan broker with DomainID @@ -1976,12 +1979,48 @@ class LoanBroker_test : public beast::unit_test::suite } } - // Test 4: Cannot set tfLoanBrokerPrivate when modifying (flag is immutable) + // Test 4: Updating other fields without specifying DomainID does not + // affect DomainID + env(set(alice, vaultKeylet.key), + loanBrokerID(brokerKeylet.key), + data("test_data"), + debtMaximum(Number(1000))); + env.close(); + { + auto const sleBroker = env.le(brokerKeylet); + BEAST_EXPECT(sleBroker); + if (sleBroker) + { + // DomainID should be preserved + BEAST_EXPECT(sleBroker->at(~sfDomainID) == domainId1); + // Other fields should be updated + BEAST_EXPECT(sleBroker->isFieldPresent(sfData)); + BEAST_EXPECT(sleBroker->at(~sfDebtMaximum) == Number(1000)); + } + } + + // Test 5: Updating only DomainID does not affect other fields + env(set(alice, vaultKeylet.key), loanBrokerID(brokerKeylet.key), domainID(domainId2)); + env.close(); + { + auto const sleBroker = env.le(brokerKeylet); + BEAST_EXPECT(sleBroker); + if (sleBroker) + { + // DomainID should be updated + BEAST_EXPECT(sleBroker->at(~sfDomainID) == domainId2); + // Other fields should be preserved + BEAST_EXPECT(sleBroker->isFieldPresent(sfData)); + BEAST_EXPECT(sleBroker->at(~sfDebtMaximum) == Number(1000)); + } + } + + // Test 6: Cannot set tfLoanBrokerPrivate when modifying (flag is immutable) env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), loanBrokerID(brokerKeylet.key), ter(temINVALID)); - // Test 5: Create a public broker and try to set DomainID on it + // Test 7: Create a public broker and try to set DomainID on it auto const publicBrokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); env(set(alice, vaultKeylet.key)); // No tfLoanBrokerPrivate = public env.close(); @@ -2131,6 +2170,95 @@ class LoanBroker_test : public beast::unit_test::suite } } + void + testPrivateBrokerRejectsNonMember() + { + testcase("Private broker rejects borrower not in permissioned domain"); + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; // Broker owner + Account const bob{"bob"}; // Borrower with credentials + Account const carol{"carol"}; // Borrower without credentials + Account const credIssuer{"credIssuer"}; + std::string const credType = "LoanCredential"; + + Env env{*this}; + Vault vault{env}; + + env.fund(XRP(100'000), issuer, alice, bob, carol, credIssuer); + env.close(); + + // Create an IOU asset and vault + env(trust(alice, issuer["IOU"](1'000'000))); + env(trust(bob, issuer["IOU"](1'000'000))); + env(trust(carol, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset{issuer["IOU"]}; + env(pay(issuer, alice, asset(100'000))); + env(pay(issuer, bob, asset(10'000))); + env(pay(issuer, carol, asset(10'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env.close(); + + // Deposit into vault + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50'000)})); + env.close(); + + // Create a permissioned domain + pdomain::Credentials const creds{{credIssuer, credType}}; + env(pdomain::setTx(credIssuer, creds)); + env.close(); + auto const domainId = pdomain::getNewDomain(env.meta()); + + // Only bob gets credentials; carol does not + env(credentials::create(bob, credIssuer, credType)); + env.close(); + env(credentials::accept(bob, credIssuer, credType)); + env.close(); + + // Create a private loan broker with DomainID + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key, tfLoanBrokerPrivate), domainID(domainId)); + env.close(); + + // Deposit cover into broker + env(coverDeposit(alice, brokerKeylet.key, asset(1000))); + env.close(); + + // Carol (no credentials) cannot create a loan + { + auto setTx = env.jt(loan::set(carol, brokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx, ter(tecNO_AUTH)); + } + + // Bob (has credentials) can create a loan + { + auto const loanKeylet = keylet::loan(brokerKeylet.key, 1); + auto setTx = env.jt(loan::set(bob, brokerKeylet.key, asset(100).number())); + sig(sfCounterpartySignature, alice)(env, setTx); + fee{env.current()->fees().base * 2}(env, setTx); + loan::counterparty(alice)(env, setTx); + loan::interestRate(TenthBips32{1000})(env, setTx); + loan::paymentTotal(2)(env, setTx); + loan::paymentInterval(100)(env, setTx); + env(setTx); + env.close(); + + BEAST_EXPECT(env.le(loanKeylet)); + } + } + public: void run() override @@ -2156,6 +2284,7 @@ class LoanBroker_test : public beast::unit_test::suite testPrivateLoanBroker(); testPrivateLoanBrokerModify(); testPrivateBrokerUnsetDomainBlocksLoans(); + testPrivateBrokerRejectsNonMember(); // TODO: Write clawback failure tests with an issuer / MPT that doesn't // have the right flags set. From ef3e62561fbf0bce11d00340f09a21460cea312e Mon Sep 17 00:00:00 2001 From: JCW Date: Mon, 23 Mar 2026 18:51:49 +0000 Subject: [PATCH 14/17] Address PR comments Signed-off-by: JCW --- src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp | 4 ++-- src/test/app/LoanBroker_test.cpp | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index f1db33c0f01..479446758ab 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -52,7 +52,7 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) if (ctx.rules.enabled(fixLendingProtocolV1_1)) { - // Cannot change private flag on existing broker + // Cannot change private flag on existing broker if (tx.isFlag(tfLoanBrokerPrivate)) { return temINVALID; @@ -189,7 +189,7 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx) if (ctx.view.rules().enabled(fixLendingProtocolV1_1)) { auto const domainID = tx[~sfDomainID]; - if (sleBroker->isFlag(lsfLoanBrokerPrivate) && domainID) + if (!sleBroker->isFlag(lsfLoanBrokerPrivate) && domainID) return tecNO_PERMISSION; } } diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index ef12f8f2de6..6a21d47b502 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -2033,16 +2033,17 @@ class LoanBroker_test : public beast::unit_test::suite } } - // Cannot set non-zero DomainID on public broker + // Cannot set DomainID on public broker env(set(alice, vaultKeylet.key), loanBrokerID(publicBrokerKeylet.key), domainID(domainId1), ter(tecNO_PERMISSION)); - // But zero DomainID on public broker is allowed (no-op) + // Cannot set DomainID to even zero on public broker env(set(alice, vaultKeylet.key), loanBrokerID(publicBrokerKeylet.key), - domainID(beast::zero)); + domainID(beast::zero), + ter(tecNO_PERMISSION)); env.close(); } From 7491a230800c89bed052c3bd7d615a4348431810 Mon Sep 17 00:00:00 2001 From: JCW Date: Fri, 5 Jun 2026 13:43:53 +0100 Subject: [PATCH 15/17] Address PR comments --- .../tx/invariants/LoanBrokerInvariant.cpp | 9 +++ src/libxrpl/tx/invariants/LoanInvariant.cpp | 6 -- .../tx/transactors/lending/LoanSet.cpp | 4 +- src/test/app/Loan_test.cpp | 56 +++++++++++++++++++ 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp index 8586a27be34..ab4b3f4d6d3 100644 --- a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp @@ -140,6 +140,15 @@ ValidLoanBroker::finalize( auto const& before = broker.brokerBefore; + if (view.rules().enabled(fixCleanup3_2_0)) + { + if (after->at(~sfDomainID) && !after->isFlag(lsfLoanBrokerPrivate)) + { + JLOG(j.fatal()) << "Invariant failed: DomainID is set on public Loan Broker"; + return false; + } + } + // https://github.com/Tapanito/XRPL-Standards/blob/xls-66-lending-protocol/XLS-0066d-lending-protocol/README.md#3123-invariants // If `LoanBroker.OwnerCount = 0` the `DirectoryNode` will have at most // one node (the root), which will only hold entries for `RippleState` diff --git a/src/libxrpl/tx/invariants/LoanInvariant.cpp b/src/libxrpl/tx/invariants/LoanInvariant.cpp index 49b8e1e5460..ce9a7c6e03b 100644 --- a/src/libxrpl/tx/invariants/LoanInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanInvariant.cpp @@ -89,12 +89,6 @@ ValidLoan::finalize( return false; } } - - if (after->at(~sfDomainID) && !after->isFlag(lsfLoanBrokerPrivate)) - { - JLOG(j.fatal()) << "Invariant failed: DomainID is set on public Loan Broker"; - return false; - } } return true; } diff --git a/src/libxrpl/tx/transactors/lending/LoanSet.cpp b/src/libxrpl/tx/transactors/lending/LoanSet.cpp index 0836e275195..d075e0e6e98 100644 --- a/src/libxrpl/tx/transactors/lending/LoanSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanSet.cpp @@ -380,7 +380,7 @@ LoanSet::preclaim(PreclaimContext const& ctx) } if (auto const ter = credentials::validDomain(ctx.view, *domainID, borrower); - !isTesSuccess(ter)) + !isTesSuccess(ter) && ter != tecEXPIRED) return ter; } @@ -432,7 +432,7 @@ LoanSet::doApply() return tefBAD_LEDGER; // LCOV_EXCL_LINE } - if (auto const ter = verifyValidDomain(view, accountID_, *domainID, j_); !isTesSuccess(ter)) + if (auto const ter = verifyValidDomain(view, borrower, *domainID, j_); !isTesSuccess(ter)) return ter; } diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 463cd523b93..3a013560f35 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -8834,6 +8834,40 @@ class Loan_test : public beast::unit_test::Suite BEAST_EXPECT(env.le(loanKeylet)); } + + // Test 5: When the broker owner submits, the domain check must validate + // the borrower (the counterparty), not the submitting account. The + // broker owner (alice) is the domain owner and holds credentials, so a + // bug that checked the submitter instead of the borrower would let a + // borrower without credentials (carol) through. + { + auto setTx = env.jt(loan::set(alice, brokerKeylet.key, asset(100).number())); + Sig(sfCounterpartySignature, carol)(env, setTx); + Fee{env.current()->fees().base * 2}(env, setTx); + loan::kCounterparty(carol)(env, setTx); + loan::kInterestRate(TenthBips32{1000})(env, setTx); + loan::kPaymentTotal(2)(env, setTx); + loan::kPaymentInterval(100)(env, setTx); + env(setTx, Ter(tecNO_AUTH)); + } + + // Test 6: Broker owner submits with a credentialed borrower (bob) as + // counterparty. This must succeed, confirming the domain check uses the + // borrower's credentials regardless of who submits. + { + auto const loanKeylet = keylet::loan(brokerKeylet.key, 2); + auto setTx = env.jt(loan::set(alice, brokerKeylet.key, asset(100).number())); + Sig(sfCounterpartySignature, bob)(env, setTx); + Fee{env.current()->fees().base * 2}(env, setTx); + loan::kCounterparty(bob)(env, setTx); + loan::kInterestRate(TenthBips32{1000})(env, setTx); + loan::kPaymentTotal(2)(env, setTx); + loan::kPaymentInterval(100)(env, setTx); + env(setTx); + env.close(); + + BEAST_EXPECT(env.le(loanKeylet)); + } } void @@ -8917,6 +8951,17 @@ class Loan_test : public beast::unit_test::Suite auto const credKeylet = credentials::keylet(bob, credIssuer, credType); BEAST_EXPECT(!env.le(credKeylet)); + // Capture the pre-payment loan state and borrower balance so we can + // confirm the payment is genuinely applied, not merely that the loan + // object survives. + auto const bobBalanceBefore = env.balance(bob, asset).value(); + auto const sleLoanBefore = env.le(loanKeylet); + if (!BEAST_EXPECT(sleLoanBefore)) + return; + auto const paymentRemainingBefore = sleLoanBefore->at(sfPaymentRemaining); + auto const principalBefore = sleLoanBefore->at(sfPrincipalOutstanding); + auto const totalValueBefore = sleLoanBefore->at(sfTotalValueOutstanding); + // Bob should still be able to make a loan payment even without credentials env(loan::pay(bob, loanKeylet.key, asset(51))); env.close(); @@ -8925,6 +8970,17 @@ class Loan_test : public beast::unit_test::Suite auto const sleLoan = env.le(loanKeylet); BEAST_EXPECT(sleLoan); + // Verify the payment was actually applied: the borrower's IOU balance + // dropped (fees are charged separately in XRP), one payment period was + // consumed, and the loan's outstanding figures decreased. + BEAST_EXPECT(env.balance(bob, asset).value() < bobBalanceBefore); + if (BEAST_EXPECT(sleLoan)) + { + BEAST_EXPECT(sleLoan->at(sfPaymentRemaining) == paymentRemainingBefore - 1); + BEAST_EXPECT(sleLoan->at(sfPrincipalOutstanding) < principalBefore); + BEAST_EXPECT(sleLoan->at(sfTotalValueOutstanding) < totalValueBefore); + } + // Bob should NOT be able to create a NEW loan (no credentials) { auto const newLoanKeylet = keylet::loan(brokerKeylet.key, 2); From b7660410b48f4464ee3e87a42cbc28df0e54d6bd Mon Sep 17 00:00:00 2001 From: JCW Date: Sat, 6 Jun 2026 21:37:46 +0100 Subject: [PATCH 16/17] Fix clang-tidy error --- src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp | 3 +++ src/test/app/LoanBroker_test.cpp | 10 +++++----- src/test/app/Loan_test.cpp | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index eb75b6dc021..3ee0378f952 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -9,7 +9,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -21,6 +23,7 @@ #include #include +#include #include #include diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index b97b1f5e769..0bd40171171 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -2254,7 +2254,7 @@ class LoanBroker_test : public beast::unit_test::Suite std::string const credType = "LoanCredential"; Env env{*this}; - Vault vault{env}; + Vault const vault{env}; env.fund(XRP(100'000), issuer, alice, credIssuer); env.close(); @@ -2281,7 +2281,7 @@ class LoanBroker_test : public beast::unit_test::Suite { Env envNoFix{*this}; envNoFix.disableFeature(fixCleanup3_2_0); - Vault vault2{envNoFix}; + Vault const vault2{envNoFix}; envNoFix.fund(XRP(100'000), issuer, alice); envNoFix.close(); @@ -2354,7 +2354,7 @@ class LoanBroker_test : public beast::unit_test::Suite std::string const credType = "LoanCredential"; Env env{*this}; - Vault vault{env}; + Vault const vault{env}; env.fund(XRP(100'000), issuer, alice, credIssuer); env.close(); @@ -2503,7 +2503,7 @@ class LoanBroker_test : public beast::unit_test::Suite std::string const credType = "LoanCredential"; Env env{*this}; - Vault vault{env}; + Vault const vault{env}; env.fund(XRP(100'000), issuer, alice, bob, credIssuer); env.close(); @@ -2628,7 +2628,7 @@ class LoanBroker_test : public beast::unit_test::Suite std::string const credType = "LoanCredential"; Env env{*this}; - Vault vault{env}; + Vault const vault{env}; env.fund(XRP(100'000), issuer, alice, bob, carol, credIssuer); env.close(); diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 3a013560f35..3ba4546e090 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -8717,7 +8717,7 @@ class Loan_test : public beast::unit_test::Suite std::string const credType = "LoanCredential"; Env env{*this}; - Vault vault{env}; + Vault const vault{env}; env.fund(XRP(100'000), issuer, alice, bob, carol, credIssuer); env.close(); @@ -8884,7 +8884,7 @@ class Loan_test : public beast::unit_test::Suite std::string const credType = "LoanCredential"; Env env{*this}; - Vault vault{env}; + Vault const vault{env}; env.fund(XRP(100'000), issuer, alice, bob, credIssuer); env.close(); From 710dd81e8368479fd680e00d7b7edbe41d7cb415 Mon Sep 17 00:00:00 2001 From: JCW Date: Sat, 6 Jun 2026 21:41:24 +0100 Subject: [PATCH 17/17] Update the feature flag --- include/xrpl/protocol/detail/features.macro | 1 + src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp | 2 +- .../tx/transactors/lending/LoanBrokerSet.cpp | 15 ++++++++------- src/libxrpl/tx/transactors/lending/LoanSet.cpp | 6 ++++-- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index c99c1e5ce81..911bfd05f50 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,6 +15,7 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. +XRPL_FEATURE(LendingPermissionedDomain, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_2_0, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_1_3, Supported::Yes, VoteBehavior::DefaultYes) diff --git a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp index ab4b3f4d6d3..43947e8eb30 100644 --- a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp @@ -140,7 +140,7 @@ ValidLoanBroker::finalize( auto const& before = broker.brokerBefore; - if (view.rules().enabled(fixCleanup3_2_0)) + if (view.rules().enabled(featureLendingPermissionedDomain)) { if (after->at(~sfDomainID) && !after->isFlag(lsfLoanBrokerPrivate)) { diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index 3ee0378f952..4b65abf1818 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -53,7 +53,7 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) if (!validNumericRange(tx[~sfDebtMaximum], Number(kMaxMpTokenAmount), Number(0))) return temINVALID; - if (!ctx.rules.enabled(fixCleanup3_2_0)) + if (!ctx.rules.enabled(featureLendingPermissionedDomain)) { if (tx.isFlag(tfLoanBrokerPrivate) || tx.isFieldPresent(sfDomainID)) { @@ -73,7 +73,7 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) if (tx[sfLoanBrokerID] == beast::kZero) return temINVALID; - if (ctx.rules.enabled(fixCleanup3_2_0)) + if (ctx.rules.enabled(featureLendingPermissionedDomain)) { // Cannot change private flag on existing broker if (tx.isFlag(tfLoanBrokerPrivate)) @@ -85,7 +85,7 @@ LoanBrokerSet::preflight(PreflightContext const& ctx) else { // We're creating a new LoanBroker. - if (ctx.rules.enabled(fixCleanup3_2_0)) + if (ctx.rules.enabled(featureLendingPermissionedDomain)) { auto const domainID = tx.at(~sfDomainID); if (domainID) @@ -163,7 +163,7 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; } - if (ctx.view.rules().enabled(fixCleanup3_2_0)) + if (ctx.view.rules().enabled(featureLendingPermissionedDomain)) { auto const domainID = tx[~sfDomainID]; if (domainID && *domainID != beast::kZero) @@ -209,7 +209,7 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx) } } - if (ctx.view.rules().enabled(fixCleanup3_2_0)) + if (ctx.view.rules().enabled(featureLendingPermissionedDomain)) { auto const domainID = tx[~sfDomainID]; if (!sleBroker->isFlag(lsfLoanBrokerPrivate) && domainID) @@ -273,7 +273,8 @@ LoanBrokerSet::doApply() if (auto const debtMax = tx[~sfDebtMaximum]) broker->at(sfDebtMaximum) = *debtMax; - if (ctx_.view().rules().enabled(fixCleanup3_2_0) && broker->isFlag(lsfLoanBrokerPrivate)) + if (ctx_.view().rules().enabled(featureLendingPermissionedDomain) && + broker->isFlag(lsfLoanBrokerPrivate)) { if (auto const domainID = tx[~sfDomainID]) { @@ -359,7 +360,7 @@ LoanBrokerSet::doApply() if (auto const coverLiq = tx[~sfCoverRateLiquidation]) broker->at(sfCoverRateLiquidation) = *coverLiq; - if (ctx_.view().rules().enabled(fixCleanup3_2_0)) + if (ctx_.view().rules().enabled(featureLendingPermissionedDomain)) { if (tx.isFlag(tfLoanBrokerPrivate)) { diff --git a/src/libxrpl/tx/transactors/lending/LoanSet.cpp b/src/libxrpl/tx/transactors/lending/LoanSet.cpp index d075e0e6e98..3f93aa50b3d 100644 --- a/src/libxrpl/tx/transactors/lending/LoanSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanSet.cpp @@ -363,7 +363,8 @@ LoanSet::preclaim(PreclaimContext const& ctx) return ret; } - if (ctx.view.rules().enabled(fixCleanup3_2_0) && brokerSle->isFlag(lsfLoanBrokerPrivate)) + if (ctx.view.rules().enabled(featureLendingPermissionedDomain) && + brokerSle->isFlag(lsfLoanBrokerPrivate)) { auto const domainID = brokerSle->at(~sfDomainID); if (!domainID) @@ -424,7 +425,8 @@ LoanSet::doApply() return tefBAD_LEDGER; // LCOV_EXCL_LINE } - if (ctx_.view().rules().enabled(fixCleanup3_2_0) && brokerSle->isFlag(lsfLoanBrokerPrivate)) + if (ctx_.view().rules().enabled(featureLendingPermissionedDomain) && + brokerSle->isFlag(lsfLoanBrokerPrivate)) { auto const domainID = brokerSle->at(~sfDomainID); if (!domainID)