ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-29 23:45:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Review feedback: test coverage, round debt total

Browse Source 
- Add some test cases to improve coverage, and exclude some lines from
  coverage.
- Rounds the Broker.DebtTotal to the Vault scale any time it is
  modified. This should reduce rounding errors.
- Ensure that no debt is left for the Broker after the last loan is
  deleted. This ensures that any accumulated rounding errors are
  resolved.
This commit is contained in:
Ed Hennis
2025-11-22 15:28:27 -05:00
parent 1cec91e72d
commit 973a105959
9 changed files with 93 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
src/test/app/LoanBroker_test.cpp
View File

@@ -673,11 +673,16 @@ class LoanBroker_test : public beast::unit_test::suite
coverRateMinimum(maxCoverRate),
coverRateLiquidation(maxCoverRate),
ter(tecNO_PERMISSION));
// Cover: too big
// CoverMinimum: too big
env(set(evan, vault.vaultID),
coverRateMinimum(maxCoverRate + 1),
coverRateLiquidation(maxCoverRate + 1),
ter(temINVALID));
// CoverLiquidation: too big
env(set(evan, vault.vaultID),
coverRateMinimum(maxCoverRate / 2),
coverRateLiquidation(maxCoverRate + 1),
ter(temINVALID));
// Cover: zero min, non-zero liquidation - implicit and
// explicit zero values.
env(set(evan, vault.vaultID),

6
src/test/app/Loan_test.cpp
View File

@@ -141,11 +141,7 @@ protected:
using namespace jtx;
auto const vaultSle = env.le(keylet::vault(vaultID));
if (!vaultSle)
// This function is not important enough to return an optional.
// Return an impossibly small number
return STAmount::cMinOffset - 1;
return vaultSle->at(sfAssetsTotal).exponent();
return getVaultScale(vaultSle);
}
};

25
src/xrpld/app/misc/LendingHelpers.h
View File

@@ -102,6 +102,31 @@ struct LoanState
}
};
// Some values get re-rounded to the vault scale any time they are adjusted. In
// addition, they are prevented from ever going below zero. This helps avoid
// accumulated rounding errors and leftover dust amounts.
template <class NumberProxy>
void
adjustImpreciseNumber(
NumberProxy value,
Number const& adjustment,
Asset const& asset,
int vaultScale)
{
value = roundToAsset(asset, value + adjustment, vaultScale);
if (*value < beast::zero)
value = 0;
}
inline int
getVaultScale(SLE::const_ref vaultSle)
{
if (!vaultSle)
return Number::minExponent - 1; // LCOV_EXCL_LINE
return vaultSle->at(sfAssetsTotal).exponent();
}
TER
checkLoanGuards(
Asset const& vaultAsset,

24
src/xrpld/app/tx/detail/LoanBrokerDelete.cpp
View File

@@ -43,6 +43,30 @@ LoanBrokerDelete::preclaim(PreclaimContext const& ctx)
JLOG(ctx.j.warn()) << "LoanBrokerDelete: Owner count is " << ownerCount;
return tecHAS_OBLIGATIONS;
}
if (auto const debtTotal = sleBroker->at(sfDebtTotal);
debtTotal != beast::zero)
{
// Any remaining debt should have been wiped out by the last Loan
// Delete. This check is purely defensive.
auto const vault =
ctx.view.read(keylet::vault(sleBroker->at(sfVaultID)));
if (!vault)
return tefINTERNAL; // LCOV_EXCL_LINE
auto const asset = vault->at(sfAsset);
auto const scale = getVaultScale(vault);
auto const rounded =
roundToAsset(asset, debtTotal, scale, Number::towards_zero);
if (rounded != beast::zero)
{
// LCOV_EXCL_START
JLOG(ctx.j.warn()) << "LoanBrokerDelete: Debt total is "
<< debtTotal << ", which rounds to " << rounded;
return tecHAS_OBLIGATIONS;
// LCOV_EXCL_START
}
}
return tesSUCCESS;
}

6
src/xrpld/app/tx/detail/LoanBrokerSet.cpp
View File

@@ -144,9 +144,9 @@ LoanBrokerSet::doApply()
std::make_shared<SLE>(keylet::loanbroker(account_, sequence));
if (auto const ter = dirLink(view, account_, broker))
return ter;
return ter; // LCOV_EXCL_LINE
if (auto const ter = dirLink(view, vaultPseudoID, broker, sfVaultNode))
return ter;
return ter; // LCOV_EXCL_LINE
adjustOwnerCount(view, owner, 2, j_);
auto const ownerCount = owner->at(sfOwnerCount);
@@ -156,7 +156,7 @@ LoanBrokerSet::doApply()
auto maybePseudo =
createPseudoAccount(view, broker->key(), sfLoanBrokerID);
if (!maybePseudo)
return maybePseudo.error();
return maybePseudo.error(); // LCOV_EXCL_LINE
auto& pseudo = *maybePseudo;
auto pseudoId = pseudo->at(sfAccount);

20
src/xrpld/app/tx/detail/LoanDelete.cpp
View File

@@ -101,7 +101,27 @@ LoanDelete::doApply()
view.erase(loanSle);
// Decrement the LoanBroker's owner count.
// The broker's owner count is solely for the number of outstanding loans,
// and is distinct from the broker's pseudo-account's owner count
adjustOwnerCount(view, brokerSle, -1, j_);
// If there are no loans left, then any remaining debt must be forgiven,
// because there is no other way to pay it back.
if (brokerSle->at(sfOwnerCount) == 0)
{
auto debtTotalProxy = brokerSle->at(sfDebtTotal);
if (*debtTotalProxy != beast::zero)
{
XRPL_ASSERT_PARTS(
roundToAsset(
vaultSle->at(sfAsset),
debtTotalProxy,
getVaultScale(vaultSle),
Number::towards_zero) == beast::zero,
"ripple::LoanDelete::doApply",
"last loan, remaining debt rounds to zero");
debtTotalProxy = 0;
}
}
// Decrement the borrower's owner count
adjustOwnerCount(view, borrowerSle, -1, j_);

23
src/xrpld/app/tx/detail/LoanManage.cpp
View File

@@ -171,16 +171,16 @@ LoanManage::defaultLoan(
// Update the Vault object:
// The vault may be at a different scale than the loan. Reduce rounding
// errors during the accounting by rounding some of the values to that
// scale.
auto const vaultScale = getVaultScale(vaultSle);
{
// Decrease the Total Value of the Vault:
auto vaultTotalProxy = vaultSle->at(sfAssetsTotal);
auto vaultAvailableProxy = vaultSle->at(sfAssetsAvailable);
// The vault may be at a different scale than the loan. Reduce rounding
// errors during the accounting by rounding some of the values to that
// scale.
auto const vaultScale = vaultTotalProxy.value().exponent();
if (vaultTotalProxy < vaultDefaultAmount)
{
// LCOV_EXCL_START
@@ -246,16 +246,11 @@ LoanManage::defaultLoan(
// Update the LoanBroker object:
{
auto const asset = *vaultSle->at(sfAsset);
// Decrease the Debt of the LoanBroker:
if (brokerDebtTotalProxy < totalDefaultAmount)
{
// LCOV_EXCL_START
JLOG(j.warn())
<< "LoanBroker debt total is less than the default amount";
return tefBAD_LEDGER;
// LCOV_EXCL_STOP
}
brokerDebtTotalProxy -= totalDefaultAmount;
adjustImpreciseNumber(
brokerDebtTotalProxy, -totalDefaultAmount, asset, vaultScale);
// Decrease the First-Loss Capital Cover Available:
auto coverAvailableProxy = brokerSle->at(sfCoverAvailable);
if (coverAvailableProxy < defaultCovered)

6
src/xrpld/app/tx/detail/LoanPay.cpp
View File

@@ -410,10 +410,8 @@ LoanPay::doApply()
// Despite our best efforts, it's possible for rounding errors to accumulate
// in the loan broker's debt total. This is because the broker may have more
// that one loan with significantly different scales.
if (totalPaidToVaultForDebt >= debtTotalProxy)
debtTotalProxy = 0;
else
debtTotalProxy -= totalPaidToVaultForDebt;
adjustImpreciseNumber(
debtTotalProxy, -totalPaidToVaultForDebt, asset, vaultScale);
//------------------------------------------------------
// Vault object state changes

5
src/xrpld/app/tx/detail/LoanSet.cpp
View File

@@ -354,7 +354,7 @@ LoanSet::doApply()
auto vaultAvailableProxy = vaultSle->at(sfAssetsAvailable);
auto vaultTotalProxy = vaultSle->at(sfAssetsTotal);
auto const vaultScale = vaultTotalProxy.value().exponent();
auto const vaultScale = getVaultScale(vaultSle);
if (vaultAvailableProxy < principalRequested)
{
JLOG(j_.warn())
@@ -574,7 +574,8 @@ LoanSet::doApply()
view.update(vaultSle);
// Update the balances in the loan broker
brokerSle->at(sfDebtTotal) += newDebtDelta;
adjustImpreciseNumber(
brokerSle->at(sfDebtTotal), newDebtDelta, vaultAsset, vaultScale);
// The broker's owner count is solely for the number of outstanding loans,
// and is distinct from the broker's pseudo-account's owner count
adjustOwnerCount(view, brokerSle, 1, j_);