ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-08-21 22:30:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Eliminate cross-scale arithmetic in LoanManage::defaultLoan

Browse Source 
Gated behind fixCleanup3_4_0. Pre-amendment, `defaultLoan` mutated
`sfAssetsTotal` and `sfAssetsAvailable` with values at different scales --
`vaultDefaultAmount` rounded down to `vaultScale`, but `defaultCovered`
kept at the finer loan scale. That asymmetry could leave
`sfAssetsAvailable > sfAssetsTotal` from the arithmetic alone, and was
patched by a dust-reconciliation branch that snapped `Total` up to
`Available`. That snap effectively minted phantom assets on `Total`
(see `LoanRounding_test::testDustManipulation`).

Under the fix, the vault-side update is composed from two well-defined
STAmount operations at the vault asset's own scale:

  (1) Write-off: `sfAssetsTotal -= totalDefaultAmount` -- the defaulted
      loan's exposure leaves the vault.
  (2) Symmetric cash inflow: `defaultCovered` returns from first-loss
      capital. Apply the same STAmount to both `sfAssetsTotal` and
      `sfAssetsAvailable`, preserving the gap.

Because both deltas are STAmounts of the same asset, no cross-scale
rounding step is needed and `Available > Total` cannot occur from
arithmetic. The dust-branch snap and its downstream `tecINTERNAL`
guard are unreachable under the fix and only run on the pre-amendment
path.

The pre-mutation sanity check on the fix path tightens to the correct
invariant (`sfAssetsTotal - sfAssetsAvailable >= totalDefaultAmount`);
the pre-amendment path retains its original `vaultTotalProxy <
vaultDefaultAmount` check for byte-for-byte compatibility with existing
ledgers.

`LoanRounding_test` now runs `testDustManipulation` (and the other
amendment-sensitive rounding tests) under both branches by adding
`fixCleanup3_4_0` to the `amendmentCombinations` matrix. Both branches
land the same post-default equality for the specific test setup; the
fix path reaches it by clean arithmetic rather than a snap.
This commit is contained in:
Vito
2026-08-11 15:22:45 +02:00
parent a3147740f2
commit 95850644a9
2 changed files with 78 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

99
src/libxrpl/tx/transactors/lending/LoanManage.cpp
View File

@@ -165,8 +165,6 @@ LoanManage::defaultLoan(
return std::min(covered, coverAvailable);
}();
auto const vaultDefaultAmount = totalDefaultAmount - defaultCovered;
// Update the Vault object:
// The vault may be at a different scale than the loan. Reduce rounding
@@ -174,7 +172,64 @@ LoanManage::defaultLoan(
// scale.
auto const vaultScale = getAssetsTotalScale(vaultSle);
// Under fixCleanup3_4_0, both vault-side fields are mutated through a
// single asset-typed STAmount pair. `amount = STAmount{vaultAsset,
// defaultCovered}` is applied to both sfAssetsTotal and
// sfAssetsAvailable, and `writeOff = STAmount{vaultAsset,
// totalDefaultAmount}` is applied to sfAssetsTotal only. Because the
// shared `amount` is normalized to STAmount precision exactly once and
// absorbed by both fields symmetrically, sfAssetsAvailable cannot
// overshoot sfAssetsTotal from arithmetic alone -- the residual is
// exactly `-writeOff` on Total. Pre-amendment, the two fields were
// adjusted via values obtained through different Number->STAmount
// paths (vaultDefaultAmount rounded down to vaultScale on one side,
// defaultCovered kept at the finer loan scale on the other), so the
// normalization was asymmetric and a dust-reconciliation snap was
// required to paper over the resulting cross-side mismatch -- a snap
// that itself minted phantom assets on sfAssetsTotal. See the
// sibling change in LoanPay for the same fix on the payment path.
bool const useUnifiedAssetArithmetic = view.rules().enabled(fixCleanup3_4_0);
if (useUnifiedAssetArithmetic)
{
// Prior to realizing the default, the vault must already carry
// at least this loan's exposure. A violation here is a corrupt
// ledger, not a transaction-input error.
//
// This guard is strictly stronger than the pre-amendment
// `vaultTotalBefore < vaultDefaultAmount` form, but activation is
// safe: the ValidVault invariant continuously enforces Total >=
// Available (so Total - Available >= 0 on every ledger), and the
// pre-amendment cross-scale rounding could only inflate the
// difference Total - Available, never deflate it. Any ledger
// reaching this point post-activation will therefore already
// satisfy the stronger form.
Number const vaultTotalBefore = *vaultSle->at(sfAssetsTotal);
Number const vaultAvailableBefore = *vaultSle->at(sfAssetsAvailable);
if (vaultTotalBefore - vaultAvailableBefore < totalDefaultAmount)
{
// LCOV_EXCL_START
JLOG(j.warn()) << "Vault exposure is less than the loan default amount";
return tefBAD_LEDGER;
// LCOV_EXCL_STOP
}
// Mutate the two vault fields through a single asset-typed pair
// (see the block comment on useUnifiedAssetArithmetic above for
// why this is safe):
// (1) write off totalDefaultAmount from Total only;
// (2) add defaultCovered symmetrically to both fields.
STAmount const amount{vaultAsset, defaultCovered};
STAmount const writeOff{vaultAsset, totalDefaultAmount};
vaultSle->at(sfAssetsTotal) += amount - writeOff;
vaultSle->at(sfAssetsAvailable) += amount;
}
else
{
// Pre-amendment behavior.
auto const vaultDefaultAmount = totalDefaultAmount - defaultCovered;
// Decrease the Total Value of the Vault:
auto vaultTotalProxy = vaultSle->at(sfAssetsTotal);
auto vaultAvailableProxy = vaultSle->at(sfAssetsAvailable);
@@ -190,8 +245,8 @@ LoanManage::defaultLoan(
auto const vaultDefaultRounded = roundToAsset(
vaultAsset, vaultDefaultAmount, vaultScale, Number::RoundingMode::Downward);
vaultTotalProxy -= vaultDefaultRounded;
// Increase the Asset Available of the Vault by liquidated First-Loss
// Capital and any unclaimed funds amount:
// Increase the Asset Available of the Vault by liquidated
// First-Loss Capital and any unclaimed funds amount:
vaultAvailableProxy += defaultCovered;
if (*vaultAvailableProxy > *vaultTotalProxy && !vaultAsset.integral())
{
@@ -203,8 +258,8 @@ LoanManage::defaultLoan(
<< "(" << difference.exponent() << ")";
if (vaultAvailableProxy.value().exponent() - difference.exponent() > 13)
{
// If the difference is dust, bring the total up to match
// the available
// If the difference is dust, bring the total up to
// match the available
JLOG(j.debug()) << "Difference between vault assets available and total is "
"dust. Set both to the larger value.";
vaultTotalProxy = vaultAvailableProxy;
@@ -219,24 +274,24 @@ LoanManage::defaultLoan(
return tecINTERNAL;
// LCOV_EXCL_STOP
}
// The loss has been realized
if (loanSle->isFlag(lsfLoanImpaired))
{
auto vaultLossUnrealizedProxy = vaultSle->at(sfLossUnrealized);
if (vaultLossUnrealizedProxy < totalDefaultAmount)
{
// LCOV_EXCL_START
JLOG(j.warn()) << "Vault unrealized loss is less than the default amount";
return tefBAD_LEDGER;
// LCOV_EXCL_STOP
}
adjustImpreciseNumber(
vaultLossUnrealizedProxy, -totalDefaultAmount, vaultAsset, vaultScale);
}
view.update(vaultSle);
}
// The loss has been realized
if (loanSle->isFlag(lsfLoanImpaired))
{
auto vaultLossUnrealizedProxy = vaultSle->at(sfLossUnrealized);
if (vaultLossUnrealizedProxy < totalDefaultAmount)
{
// LCOV_EXCL_START
JLOG(j.warn()) << "Vault unrealized loss is less than the default amount";
return tefBAD_LEDGER;
// LCOV_EXCL_STOP
}
adjustImpreciseNumber(
vaultLossUnrealizedProxy, -totalDefaultAmount, vaultAsset, vaultScale);
}
view.update(vaultSle);
// Update the LoanBroker object:
{

2
src/test/app/lending/LoanRounding_test.cpp
View File

@@ -987,7 +987,7 @@ public:
{
runAmendmentIndependent();
for (auto const& features : jtx::amendmentCombinations(
{fixCleanup3_1_3, fixCleanup3_2_0, featureMPTokensV2}, all_))
{fixCleanup3_1_3, fixCleanup3_2_0, fixCleanup3_4_0, featureMPTokensV2}, all_))
runAmendmentSensitive(features);
}
};