ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-06-05 09:46:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

These tiny rounding errors are going to be the death of me

Browse Source
This commit is contained in:
Ed Hennis
2025-05-14 19:05:58 +01:00
parent 784b3ae64d
commit 2d767f2733
7 changed files with 171 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
include/xrpl/protocol/STAmount.h
View File

@@ -695,16 +695,47 @@ divRoundStrict(
std::uint64_t
getRate(STAmount const& offerOut, STAmount const& offerIn);
inline STAmount
roundToReference(
STAmount const value,
STAmount const referenceValue,
Number::rounding_mode rounding = Number::getround())
{
NumberRoundModeGuard mg(rounding);
if (value >= referenceValue)
return value;
// With an IOU, the total will be truncated to the precision of the
// larger value: referenceValue
STAmount const total = referenceValue + value;
STAmount const result = total - referenceValue;
XRPL_ASSERT_PARTS(
(!value.asset().native() && value.asset().holds<Issue>()) ||
value == result,
"ripple::roundToReference",
"rounding only on IOU");
return result;
}
/** Round an arbitrary precision Number to the precision of a given Asset.
*
* @param asset The relevant asset
* @param value The value to be rounded
* @param referenceValue Only relevant to IOU assets. A reference value to
* establish the precision limit of `value`. Should be larger than
* `value`.
* @param rounding Optional Number rounding mode
*/
template <AssetType A>
Number
roundToAsset(
A const& asset,
Number const& value,
Number const& referenceValue,
Number::rounding_mode rounding = Number::getround())
{
NumberRoundModeGuard mg(rounding);
STAmount const amount{asset, value};
return amount;
return roundToReference(
STAmount{asset, value}, STAmount{asset, referenceValue});
}
//------------------------------------------------------------------------------
@@ -715,10 +746,10 @@ isXRP(STAmount const& amount)
return amount.native();
}
// Since `canonicalize` does not have access to a ledger, this is needed to put
// the low-level routine stAmountCanonicalize on an amendment switch. Only
// transactions need to use this switchover. Outside of a transaction it's safe
// to unconditionally use the new behavior.
// Since `canonicalize` does not have access to a ledger, this is needed to
// put the low-level routine stAmountCanonicalize on an amendment switch.
// Only transactions need to use this switchover. Outside of a transaction
// it's safe to unconditionally use the new behavior.
bool
getSTAmountCanonicalizeSwitchover();

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

@@ -1485,29 +1485,37 @@ class Loan_test : public beast::unit_test::suite
{
testcase << "Payments remaining: "
<< state.paymentRemaining;
STAmount const principalOutstandingAmount{
broker.asset, state.principalOutstanding};
// Compute the payment based on the number of payments
// remaining
auto const rateFactor =
power(1 + periodicRate, state.paymentRemaining);
Number const periodicPayment{
Number const rawPeriodicPayment =
state.principalOutstanding * periodicRate *
rateFactor / (rateFactor - 1)};
rateFactor / (rateFactor - 1);
STAmount const periodicPayment = roundToReference(
STAmount{broker.asset, rawPeriodicPayment},
principalOutstandingAmount);
// Only check the first payment since the rounding may
// drift as payments are made
BEAST_EXPECT(
state.paymentRemaining < 12 ||
STAmount(broker.asset, periodicPayment) ==
STAmount(broker.asset, rawPeriodicPayment) ==
broker.asset(Number(8333457001162141, -14)));
// Include the service fee
STAmount const totalDue{
broker.asset,
periodicPayment + broker.asset(2).value()};
STAmount const totalDue = roundToReference(
periodicPayment + broker.asset(2),
principalOutstandingAmount);
// Only check the first payment since the rounding may
// drift as payments are made
BEAST_EXPECT(
state.paymentRemaining < 12 ||
totalDue ==
broker.asset(Number(8533457001162141, -14)));
roundToReference(
broker.asset(Number(8533457001162141, -14)),
principalOutstandingAmount));
// Try to pay a little extra to show that it's _not_
// taken
@@ -1518,33 +1526,51 @@ class Loan_test : public beast::unit_test::suite
BEAST_EXPECT(
state.paymentRemaining < 12 ||
transactionAmount ==
broker.asset(Number(9533457001162141, -14)));
roundToReference(
broker.asset(Number(9533457001162141, -14)),
principalOutstandingAmount));
auto const totalDueAmount =
STAmount{broker.asset, totalDue};
// Compute the expected principal amount
STAmount const interest{
broker.asset,
state.principalOutstanding * periodicRate};
Number const rawInterest =
state.principalOutstanding * periodicRate;
STAmount const interest = roundToReference(
STAmount{broker.asset, rawInterest},
principalOutstandingAmount);
BEAST_EXPECT(
state.paymentRemaining < 12 ||
interest ==
broker.asset(Number(2283105022831050, -18)));
roundToReference(
STAmount{broker.asset, rawInterest},
principalOutstandingAmount) ==
roundToReference(
broker.asset(Number(2283105022831050, -18)),
principalOutstandingAmount));
BEAST_EXPECT(interest >= Number(0));
auto const principal =
STAmount(broker.asset, periodicPayment - interest);
auto const rawPrincipal =
rawPeriodicPayment - rawInterest;
BEAST_EXPECT(
state.paymentRemaining < 12 ||
principal ==
broker.asset(Number(8333228690659858, -14)));
roundToReference(
STAmount{broker.asset, rawPrincipal},
principalOutstandingAmount) ==
roundToReference(
broker.asset(Number(8333228690659858, -14)),
principalOutstandingAmount));
auto const principal = roundToReference(
STAmount{broker.asset, periodicPayment - interest},
principalOutstandingAmount);
BEAST_EXPECT(
principal > Number(0) &&
principal <= state.principalOutstanding);
BEAST_EXPECT(
state.paymentRemaining > 1 ||
principal == state.principalOutstanding);
BEAST_EXPECT(
rawPrincipal + rawInterest == rawPeriodicPayment);
BEAST_EXPECT(principal + interest == periodicPayment);
auto const borrowerBalanceBeforePayment =
env.balance(borrower, broker.asset);

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

@@ -75,11 +75,15 @@ template <AssetType A>
Number
loanTotalValueOutstanding(
A asset,
Number principalOutstanding,
Number periodicPayment,
std::uint32_t paymentsRemaining)
{
return roundToAsset(
asset, periodicPayment * paymentsRemaining, Number::upward);
asset,
periodicPayment * paymentsRemaining,
principalOutstanding,
Number::upward);
}
template <AssetType A>
@@ -93,6 +97,7 @@ loanTotalValueOutstanding(
{
return loanTotalValueOutstanding(
asset,
principalOutstanding,
loanPeriodicPayment(
principalOutstanding,
interestRate,
@@ -157,16 +162,28 @@ computePeriodicPaymentParts(
A const& asset,
Number const& principalOutstanding,
Number const& periodicPaymentAmount,
Number const& periodicRate)
Number const& periodicRate,
std::uint32_t paymentRemaining)
{
Number const interest =
roundToAsset(asset, principalOutstanding * periodicRate);
if (paymentRemaining == 1)
{
// If there's only one payment left, we need to pay off the principal.
Number const interest = roundToAsset(
asset,
periodicPaymentAmount - principalOutstanding,
principalOutstanding);
return {interest, principalOutstanding};
}
Number const interest = roundToAsset(
asset, principalOutstanding * periodicRate, principalOutstanding);
XRPL_ASSERT(
interest >= 0,
"ripple::detail::computePeriodicPayment : valid interest");
auto const roundedPayment =
roundToAsset(asset, periodicPaymentAmount, principalOutstanding);
Number const principal =
roundToAsset(asset, periodicPaymentAmount - interest);
roundToAsset(asset, roundedPayment - interest, principalOutstanding);
XRPL_ASSERT(
principal > 0 && principal <= principalOutstanding,
"ripple::detail::computePeriodicPayment : valid principal");
@@ -190,7 +207,7 @@ valueMinusManagementFee(
TenthBips32 managementFeeRate)
{
return roundToAsset(
asset, detail::minusManagementFee(value, managementFeeRate));
asset, detail::minusManagementFee(value, managementFeeRate), value);
}
template <AssetType A>
@@ -229,7 +246,8 @@ loanPeriodicPayment(
principalOutstanding,
interestRate,
paymentInterval,
paymentsRemaining));
paymentsRemaining),
principalOutstanding);
}
template <AssetType A>
@@ -249,7 +267,8 @@ loanLatePaymentInterest(
lateInterestRate,
parentCloseTime,
startDate,
prevPaymentDate));
prevPaymentDate),
principalOutstanding);
}
struct LoanPaymentParts
@@ -280,8 +299,8 @@ loanComputePaymentParts(
Number const serviceFee = loan->at(sfLoanServiceFee);
Number const latePaymentFee = loan->at(sfLatePaymentFee);
Number const closePaymentFee =
roundToAsset(asset, loan->at(sfClosePaymentFee));
Number const closePaymentFee = roundToAsset(
asset, loan->at(sfClosePaymentFee), principalOutstandingField);
TenthBips32 const overpaymentFee{loan->at(sfOverpaymentFee)};
std::uint32_t const paymentInterval = loan->at(sfPaymentInterval);
@@ -314,10 +333,17 @@ loanComputePaymentParts(
"ripple::computePeriodicPayment : valid payment");
auto const periodic = detail::computePeriodicPaymentParts(
asset, principalOutstandingField, periodicPaymentAmount, periodicRate);
asset,
principalOutstandingField,
periodicPaymentAmount,
periodicRate,
paymentRemainingField);
Number const totalValueOutstanding = detail::loanTotalValueOutstanding(
asset, periodicPaymentAmount, paymentRemainingField);
asset,
principalOutstandingField,
periodicPaymentAmount,
paymentRemainingField);
XRPL_ASSERT(
totalValueOutstanding > 0,
"ripple::loanComputePaymentParts : valid total value");
@@ -393,14 +419,16 @@ loanComputePaymentParts(
view.parentCloseTime(),
startDate,
prevPaymentDateField,
paymentInterval));
paymentInterval),
principalOutstandingField);
XRPL_ASSERT(
accruedInterest >= 0,
"ripple::loanComputePaymentParts : valid accrued interest");
auto const closePrepaymentInterest = roundToAsset(
asset,
tenthBipsOfValue(
principalOutstandingField.value(), closeInterestRate));
principalOutstandingField.value(), closeInterestRate),
principalOutstandingField);
XRPL_ASSERT(
closePrepaymentInterest >= 0,
"ripple::loanComputePaymentParts : valid prepayment "
@@ -437,8 +465,11 @@ loanComputePaymentParts(
// if the payment is not late nor if it's a full payment, then it must be a
// periodic one, with possible overpayments
auto const totalDue =
roundToAsset(asset, periodicPaymentAmount + serviceFee, Number::upward);
auto const totalDue = roundToAsset(
asset,
periodicPaymentAmount + serviceFee,
principalOutstandingField,
Number::upward);
std::optional<NumberRoundModeGuard> mg(Number::downward);
std::int64_t const fullPeriodicPayments = [&]() {
@@ -477,7 +508,8 @@ loanComputePaymentParts(
asset,
principalOutstandingField,
periodicPaymentAmount,
periodicRate);
periodicRate,
paymentRemainingField);
XRPL_ASSERT(
future->interest <= periodic.interest,
"ripple::loanComputePaymentParts : decreasing interest");
@@ -501,14 +533,20 @@ loanComputePaymentParts(
principalOutstandingField.value(),
amount - (totalPrincipalPaid + totalInterestPaid + totalFeePaid));
if (roundToAsset(asset, overpayment) > 0)
if (roundToAsset(asset, overpayment, principalOutstandingField) > 0)
{
Number const interestPortion = roundToAsset(
asset, tenthBipsOfValue(overpayment, overpaymentInterestRate));
asset,
tenthBipsOfValue(overpayment, overpaymentInterestRate),
principalOutstandingField);
Number const feePortion = roundToAsset(
asset, tenthBipsOfValue(overpayment, overpaymentFee));
Number const remainder =
roundToAsset(asset, overpayment - interestPortion - feePortion);
asset,
tenthBipsOfValue(overpayment, overpaymentFee),
principalOutstandingField);
Number const remainder = roundToAsset(
asset,
overpayment - interestPortion - feePortion,
principalOutstandingField);
// Don't process an overpayment if the whole amount (or more!) gets
// eaten by fees
@@ -540,16 +578,20 @@ loanComputePaymentParts(
// Check the final results are rounded, to double-check that the
// intermediate steps were rounded.
XRPL_ASSERT(
roundToAsset(asset, totalPrincipalPaid) == totalPrincipalPaid,
roundToAsset(asset, totalPrincipalPaid, principalOutstandingField) ==
totalPrincipalPaid,
"ripple::loanComputePaymentParts : totalPrincipalPaid rounded");
XRPL_ASSERT(
roundToAsset(asset, totalInterestPaid) == totalInterestPaid,
roundToAsset(asset, totalInterestPaid, principalOutstandingField) ==
totalInterestPaid,
"ripple::loanComputePaymentParts : totalInterestPaid rounded");
XRPL_ASSERT(
roundToAsset(asset, loanValueChange) == loanValueChange,
roundToAsset(asset, loanValueChange, principalOutstandingField) ==
loanValueChange,
"ripple::loanComputePaymentParts : loanValueChange rounded");
XRPL_ASSERT(
roundToAsset(asset, totalFeePaid) == totalFeePaid,
roundToAsset(asset, totalFeePaid, principalOutstandingField) ==
totalFeePaid,
"ripple::loanComputePaymentParts : totalFeePaid rounded");
return LoanPaymentParts{
totalPrincipalPaid, totalInterestPaid, loanValueChange, totalFeePaid};

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

@@ -105,11 +105,12 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx)
auto const coverAvail = sleBroker->at(sfCoverAvailable);
// Cover Rate is in 1/10 bips units
auto const currentDebtTotal = sleBroker->at(sfDebtTotal);
auto const minimumCover = roundToAsset(
vaultAsset,
tenthBipsOfValue(
sleBroker->at(sfDebtTotal),
TenthBips32(sleBroker->at(sfCoverRateMinimum))));
currentDebtTotal, TenthBips32(sleBroker->at(sfCoverRateMinimum))),
currentDebtTotal);
if (coverAvail < amount)
return tecINSUFFICIENT_FUNDS;
if ((coverAvail - amount) < minimumCover)

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

@@ -191,7 +191,8 @@ defaultLoan(
tenthBipsOfValue(
brokerDebtTotalProxy.value(), coverRateMinimum),
coverRateLiquidation),
defaultAmount));
defaultAmount),
std::max(brokerDebtTotalProxy.value(), defaultAmount));
auto const returnToVault = defaultCovered + loanAssetsAvailableProxy;
auto const vaultDefaultAmount = defaultAmount - defaultCovered;

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

@@ -177,6 +177,9 @@ LoanPay::doApply()
//------------------------------------------------------
// Loan object state changes
auto const originalPrincipalOutstanding =
loanSle->at(sfPrincipalOutstanding);
view.update(loanSle);
Expected<LoanPaymentParts, TER> paymentParts =
@@ -207,7 +210,9 @@ LoanPay::doApply()
TenthBips32 managementFeeRate{brokerSle->at(sfManagementFeeRate)};
auto const managementFee = roundToAsset(
asset, tenthBipsOfValue(paymentParts->interestPaid, managementFeeRate));
asset,
tenthBipsOfValue(paymentParts->interestPaid, managementFeeRate),
originalPrincipalOutstanding);
auto const totalPaidToVault = paymentParts->principalPaid +
paymentParts->interestPaid - managementFee;
@@ -219,10 +224,10 @@ LoanPay::doApply()
auto debtTotalField = brokerSle->at(sfDebtTotal);
TenthBips32 const coverRateMinimum{brokerSle->at(sfCoverRateMinimum)};
bool const sufficientCover =
coverAvailableField >=
roundToAsset(
asset, tenthBipsOfValue(debtTotalField.value(), coverRateMinimum));
bool const sufficientCover = coverAvailableField >=
roundToAsset(asset,
tenthBipsOfValue(debtTotalField.value(), coverRateMinimum),
originalPrincipalOutstanding);
if (!sufficientCover)
{
// Add the fee to to First Loss Cover Pool
@@ -233,7 +238,12 @@ LoanPay::doApply()
// and subtract the change in the management fee
auto const vaultValueChange = valueMinusManagementFee(
asset, paymentParts->valueChange, managementFeeRate);
debtTotalField += vaultValueChange - totalPaidToVault;
// debtDecrease may be negative, increasing the debt
auto const debtDecrease = totalPaidToVault - vaultValueChange;
if (debtDecrease >= debtTotalField)
debtTotalField = 0;
else
debtTotalField -= debtDecrease;
//------------------------------------------------------
// Vault object state changes

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

@@ -302,7 +302,8 @@ LoanSet::doApply()
{
return tefBAD_LEDGER; // LCOV_EXCL_LINE
}
auto const principalRequested = tx[sfPrincipalRequested];
auto const principalRequested = roundToAsset(
vaultAsset, tx[sfPrincipalRequested], tx[sfPrincipalRequested]);
TenthBips32 const interestRate{tx[~sfInterestRate].value_or(0)};
auto const originationFee = tx[~sfLoanOriginationFee];
auto const loanAssetsAvailable =
@@ -405,7 +406,6 @@ LoanSet::doApply()
loan->at(sfNextPaymentDueDate) = startDate + paymentInterval;
loan->at(sfPaymentRemaining) = paymentTotal;
loan->at(sfAssetsAvailable) = loanAssetsAvailable;
loan->at(sfPrincipalOutstanding) = principalRequested;
view.insert(loan);
// Update the balances in the vault