Compare commits

..

31 Commits

Author SHA1 Message Date
Ed Hennis
b37f1a0837 Fix build error: missing XRPL_ASSERT params 2026-04-26 23:19:11 -05:00
Ed Hennis
44e438b2f2 Merge remote-tracking branch 'XRPLF/develop' into ximinez/loanpay-assertion-develop
* XRPLF/develop:
  chore: Enable clang-tidy modernize-use-nodiscard check (7015)
  fix: Resolve MSVC Debug build failure in JobQueue.h; re-enable _CRTDBG_MAP_ALLOC in CI (6993)
  docs: Update hybrid offer invariant comment (7007)
  fix: Fix flaky CI tests (7005)
  docs: Update bug bounty information (7006)
  fix: Make assorted Payments fixes (6585)
  refactor: Move `LendingHelpers` into `libxrpl/ledger/helpers` (6638)
2026-04-25 13:43:53 -05:00
Ed Hennis
8efafbe993 Merge branch 'develop' into ximinez/loanpay-assertion-develop 2026-04-23 15:55:54 -04:00
Ed Hennis
2c6ad964dc Address AI code review: check rounding mode 2026-04-23 00:04:37 -04:00
Ed Hennis
9d8cede450 Update src/test/app/Loan_test.cpp
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-04-23 00:00:31 -04:00
Ed Hennis
2a019fde43 Merge branch 'develop' into ximinez/loanpay-assertion-develop 2026-04-22 23:39:34 -04:00
Ed Hennis
934e431af5 Merge branch 'develop' into ximinez/loanpay-assertion-develop 2026-04-22 14:48:59 -04:00
Ed Hennis
168b4dff5f Address AI review feedback: Avoid null derefs in test 2026-04-22 13:31:53 -04:00
Ed Hennis
1a97a6b24f Apply suggestions from AI code review
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-04-22 13:26:07 -04:00
Ed Hennis
ec1faeff0f Merge remote-tracking branch 'XRPLF/develop' into ximinez/loanpay-assertion-develop
* XRPLF/develop:
  chore: Optionally run clang-tidy via pre-commit (6680)
  refactor: Add transaction-specific invariant checking (6551)
  style: Add bashate pre-commit hook to unify bash style (6994)
2026-04-22 13:07:55 -04:00
Ed Hennis
94327d04ba clang-tidy fixes: Use std::ranges::minmax_element 2026-04-21 19:33:26 -04:00
Ed Hennis
afe8614d94 Merge branch 'develop' into ximinez/loanpay-assertion-develop 2026-04-21 18:48:16 -04:00
Ed Hennis
3b1b5ef522 Merge remote-tracking branch 'XRPLF/develop' into ximinez/loanpay-assertion-develop
* XRPLF/develop:
  chore: Remove empty Taker.h (6984)
  chore: Enable clang-tidy modernize checks (6975)
  ci: Upload clang-tidy git diff (6983)
  fix: Add rounding to Vault invariants (6217) (6955)
2026-04-21 15:04:10 -04:00
Ed Hennis
c22b38dd03 Merge branch 'develop' into ximinez/loanpay-assertion-develop 2026-04-20 17:49:31 -04:00
Ed Hennis
60984c19a3 Merge branch 'develop' into ximinez/loanpay-assertion-develop 2026-04-20 15:44:48 -04:00
Ed Hennis
068d8458bb Merge branch 'develop' into ximinez/loanpay-assertion-develop 2026-04-20 11:38:29 -04:00
Ed Hennis
4304b3acb4 Fix clang-tidy issues 2026-04-17 18:51:07 -04:00
Ed Hennis
29af96bfa1 Address copilot review feedback: correct sign of borrowerDelta 2026-04-17 18:46:50 -04:00
Ed Hennis
7944d03795 Merge remote-tracking branch 'upstream/develop' into ximinez/loanpay-assertion-develop
* upstream/develop: (30 commits)
  chore: Enable clang-tidy include cleaner (6947)
  fix: Change AMMClawback return code to tecNO_PERMISSION (6946)
  ci: [DEPENDABOT] bump actions/upload-pages-artifact from 4.0.0 to 5.0.0 (6927)
  ci: [DEPENDABOT] bump actions/upload-artifact from 7.0.0 to 7.0.1 (6928)
  chore: Enable clang-tidy readability checks (6930)
  fix: Fix unity build for book step (6942)
  chore: Move codegen venv setup into build stage (6617)
  chore: Enable most clang-tidy bugprone checks (6929)
  refactor: Improve exception handling (6540) (6735)
  refactor: Remove unused notTooManyOffers function from NFTokenUtils (6737)
  fix: Change `Tuning::bookOffers` minimum limit to 1 (6812)
  chore: Make pre-commit line ending conversions work on Windows (6832) (6833)
  fix: Add description for `terLOCKED` error (6811)
  fix: Address AI reviewer comments for Permission Delegation (6675)
  refactor: Combine `AMMHelpers` and `AMMUtils` (6733)
  feat: Add MPT support to DEX (5285)
  fix: Handle WSClient write failure when server closes WebSocket (6671)
  ci: Change conditions for uploading artifacts in public/private/org repos (6734)
  refactor: Rename non-functional uses of `ripple(d)` to `xrpl(d)` (6676)
  refactor: Move more helper files into `libxrpl/ledger/helpers` (6731)
  ...
2026-04-17 16:13:26 -04:00
Ed Hennis
b4b47e80ba Merge branch 'develop' into ximinez/loanpay-assertion 2026-04-01 13:39:09 -04:00
Ed Hennis
c8e1d02dee Merge branch 'develop' into ximinez/loanpay-assertion 2026-04-01 11:45:36 -04:00
Ed Hennis
f68b3326df Review feedback from @tapanito
- Return a default value in LoanPay balanceScale if the exponent list is
  empty.
- Amendment gate the "roundedAmount" change in overpayment.
- Improve comments and logging.
documentation
2026-03-31 21:02:28 -04:00
Ed Hennis
da1cb2d7ce Merge remote-tracking branch 'upstream/develop' into ximinez/loanpay-assertion
* upstream/develop: (149 commits)
  fix: Fix previous ledger size typo in RCLConsensus (6696)
  chore: Enable clang-tidy misc checks (6655)
  ci: Use pull_request_target to check for signed commits (6697)
  chore: Remove unnecessary clang-format off/on directives (6682)
  fix: Fix Workers::stop() race between m_allPaused and m_runningTaskCount (6574)
  ci: Only publish docs in public repos (6687)
  chore: Enable remaining clang-tidy `performance` checks (6648)
  refactor: Address PR comments after the modularisation PRs (6389)
  chore: Fix clang-tidy header filter (6686)
  ci: [DEPENDABOT] bump actions/deploy-pages from 4.0.5 to 5.0.0 (6684)
  ci: [DEPENDABOT] bump codecov/codecov-action from 5.5.3 to 6.0.0 (6685)
  fix: Guard Coro::resume() against completed coroutines (6608)
  refactor: Split LoanInvariant into LoanBrokerInvariant and LoanInvariant (6674)
  ci: Don't publish docs on release branches (6673)
  refactor: Make function naming in ServiceRegistry consistent (6390)
  chore: Shorten job names to stay within Linux 15-char thread limit (6669)
  fix: Improve loan invariant message (6668)
  ci: Upload artifacts only in public repositories (6670)
  ci: Add conflicting-pr workflow (6656)
  chore: Add more AI tools to .gitignore (6658)
  ...
2026-03-31 18:54:27 -04:00
Ed Hennis
0797a5d4db Address an outdated TODO comment 2026-03-31 18:53:10 -04:00
Ed Hennis
d33ee7ed46 Merge commit '2c1fad1023' into HEAD
* commit '2c1fad1023':
  chore: Apply clang-format width 100 (6387)
2026-03-31 18:53:02 -04:00
Ed Hennis
53183c71e8 Update formatting 2026-03-31 18:44:32 -04:00
Ed Hennis
05eaef5233 Merge commit '25cca465538a56cce501477f9e5e2c1c7ea2d84c' into HEAD
* commit '25cca465538a56cce501477f9e5e2c1c7ea2d84c': (22 commits)
  chore: Set clang-format width to 100 in config file (6387)
  chore: Set cmake-format width to 100 (6386)
  ci: Add clang tidy workflow to ci (6369)
  refactor: Modularize app/tx (6228)
  refactor: Decouple app/tx from `Application` and `Config` (6227)
  chore: Update clang-format to 21.1.8 (6352)
  refactor: Modularize `HashRouter`, `Conditions`, and `OrderBookDB` (6226)
  chore: Fix minor issues in comments (6346)
  refactor: Modularize the NetworkOPs interface (6225)
  chore: Fix `gcov` lib coverage build failure on macOS (6350)
  refactor: Modularize RelationalDB (6224)
  refactor: Modularize WalletDB and Manifest (6223)
  fix: Update invariant checks for Permissioned Domains (6134)
  refactor: Change main thread name to `xrpld-main` (6336)
  refactor: Fix spelling issues in tests (6199)
  test: Add file and line location to Env (6276)
  chore: Remove CODEOWNERS (6337)
  perf: Remove unnecessary caches (5439)
  chore: Restore unity builds (6328)
  refactor: Update secp256k1 to 0.7.1 (6331)
  ...
2026-03-31 18:42:49 -04:00
Ed Hennis
e44fc46ad0 Verify and log LoanPay fund conservation in all builds
- A warning will be logged if there's a mismatch.
2026-03-31 18:22:16 -04:00
Ed Hennis
3f8f5a081f Review feedback from @tapanito: readability, duplicate checks 2026-03-31 18:22:16 -04:00
Ed Hennis
14236fb767 Fix formatting 2026-02-04 17:26:45 -05:00
Ed Hennis
c0b6712064 Fix touchy "funds are conserved" assertion in LoanPay
- Add "Yield Theft via Rounding Manipulation" test, which used to
  reliably triggered it. The test now verifies that no "yield theft"
  occurs.
2026-02-04 17:18:10 -05:00
6 changed files with 372 additions and 86 deletions

View File

@@ -1857,8 +1857,18 @@ loanMakePayment(
// -------------------------------------------------------------
// overpayment handling
//
// If the "fixSecurity3_1_3" amendment is enabled, truncate "amount",
// at the loan scale. If the raw value is used, the overpayment
// amount could be meaningless dust. Trying to process such a small
// amount will, at best, waste time when all the result values round
// to zero. At worst, it can cause logical errors with tiny amounts
// of interest that don't add up correctly.
auto const roundedAmount = view.rules().enabled(fixSecurity3_1_3)
? roundToAsset(asset, amount, loanScale, Number::towards_zero)
: amount;
if (paymentType == LoanPaymentType::overpayment && loan->isFlag(lsfLoanOverpayment) &&
paymentRemainingProxy > 0 && totalPaid < amount &&
paymentRemainingProxy > 0 && totalPaid < roundedAmount &&
numPayments < loanMaximumPaymentsPerTransaction)
{
TenthBips32 const overpaymentInterestRate{loan->at(sfOverpaymentInterestRate)};
@@ -1867,7 +1877,7 @@ loanMakePayment(
// It shouldn't be possible for the overpayment to be greater than
// totalValueOutstanding, because that would have been processed as
// another normal payment. But cap it just in case.
Number const overpayment = std::min(amount - totalPaid, *totalValueOutstandingProxy);
Number const overpayment = std::min(roundedAmount - totalPaid, *totalValueOutstandingProxy);
detail::ExtendedPaymentComponents const overpaymentComponents =
detail::computeOverpaymentComponents(

View File

@@ -100,7 +100,7 @@ SField::SField(private_access_tag_t, int fc, char const* fn)
, fieldName(fn)
, fieldMeta(sMD_Never)
, fieldNum(++num)
, signingField(IsSigning::no)
, signingField(IsSigning::yes)
, jsonName(fieldName.c_str())
{
XRPL_ASSERT(

View File

@@ -30,6 +30,7 @@
#include <bit>
#include <cstdint>
#include <memory>
#include <vector>
namespace xrpl {
@@ -143,6 +144,7 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx)
Number const numPaymentEstimate = static_cast<std::int64_t>(amount / regularPayment);
// Charge one base fee per paymentsPerFeeIncrement payments, rounding up.
// This set round is safe because there's a mode guard just above
Number::setround(Number::upward);
auto const feeIncrements = std::max(
std::int64_t(1),
@@ -440,9 +442,10 @@ LoanPay::doApply()
// Vault object state changes
view.update(vaultSle);
Number const assetsAvailableBefore = *assetsAvailableProxy;
Number const assetsTotalBefore = *assetsTotalProxy;
#if !NDEBUG
{
Number const assetsAvailableBefore = *assetsAvailableProxy;
Number const pseudoAccountBalanceBefore = accountHolds(
view,
vaultPseudoAccount,
@@ -466,16 +469,6 @@ LoanPay::doApply()
"xrpl::LoanPay::doApply",
"assets available must not be greater than assets outstanding");
if (*assetsAvailableProxy > *assetsTotalProxy)
{
// LCOV_EXCL_START
JLOG(j_.fatal()) << "Vault assets available must not be greater "
"than assets outstanding. Available: "
<< *assetsAvailableProxy << ", Total: " << *assetsTotalProxy;
return tecINTERNAL;
// LCOV_EXCL_STOP
}
JLOG(j_.debug()) << "total paid to vault raw: " << totalPaidToVaultRaw
<< ", total paid to vault rounded: " << totalPaidToVaultRounded
<< ", total paid to broker: " << totalPaidToBroker
@@ -501,12 +494,68 @@ LoanPay::doApply()
associateAsset(*vaultSle, asset);
// Duplicate some checks after rounding
Number const assetsAvailableAfter = *assetsAvailableProxy;
Number const assetsTotalAfter = *assetsTotalProxy;
XRPL_ASSERT_PARTS(
*assetsAvailableProxy <= *assetsTotalProxy,
assetsAvailableAfter <= assetsTotalAfter,
"xrpl::LoanPay::doApply",
"assets available must not be greater than assets outstanding");
if (assetsAvailableAfter == assetsAvailableBefore)
{
// An unchanged assetsAvailable indicates that the amount paid to the
// vault was zero, or rounded to zero. That should be impossible, but I
// can't rule it out for extreme edge cases, so fail gracefully if it
// happens.
//
// LCOV_EXCL_START
JLOG(j_.warn()) << "LoanPay: Vault assets available unchanged after rounding: " //
<< "Before: " << assetsAvailableBefore //
<< ", After: " << assetsAvailableAfter;
return tecPRECISION_LOSS;
// LCOV_EXCL_STOP
}
if (paymentParts->valueChange != beast::zero && assetsTotalAfter == assetsTotalBefore)
{
// Non-zero valueChange with an unchanged assetsTotal indicates that the
// actual value change rounded to zero. That should be impossible, but I
// can't rule it out for extreme edge cases, so fail gracefully if it
// happens.
//
// LCOV_EXCL_START
JLOG(j_.warn())
<< "LoanPay: Vault assets expected change, but unchanged after rounding: " //
<< "Before: " << assetsTotalBefore //
<< ", After: " << assetsTotalAfter //
<< ", ValueChange: " << paymentParts->valueChange;
return tecPRECISION_LOSS;
// LCOV_EXCL_STOP
}
if (paymentParts->valueChange == beast::zero && assetsTotalAfter != assetsTotalBefore)
{
// A change in assetsTotal when there was no valueChange indicates that
// something really weird happened. That should be flat out impossible.
//
// LCOV_EXCL_START
JLOG(j_.fatal()) << "LoanPay: Vault assets changed unexpectedly after rounding: " //
<< "Before: " << assetsTotalBefore //
<< ", After: " << assetsTotalAfter //
<< ", ValueChange: " << paymentParts->valueChange;
return tecINTERNAL;
// LCOV_EXCL_STOP
}
if (assetsAvailableAfter > assetsTotalAfter)
{
// Assets available are not allowed to be larger than assets total.
// LCOV_EXCL_START
JLOG(j_.fatal()) << "LoanPay: Vault assets available must not be greater "
"than assets outstanding. Available: "
<< assetsAvailableAfter << ", Total: " << assetsTotalAfter;
return tecINTERNAL;
// LCOV_EXCL_STOP
}
#if !NDEBUG
// These three values are used to check that funds are conserved after the transfers
auto const accountBalanceBefore = accountHolds(
view,
account_,
@@ -535,7 +584,6 @@ LoanPay::doApply()
ahIGNORE_AUTH,
j_,
SpendableHandling::shFULL_BALANCE);
#endif
if (totalPaidToVaultRounded != beast::zero)
{
@@ -571,19 +619,22 @@ LoanPay::doApply()
return ter;
#if !NDEBUG
Number const assetsAvailableAfter = *assetsAvailableProxy;
Number const pseudoAccountBalanceAfter = accountHolds(
view,
vaultPseudoAccount,
asset,
FreezeHandling::fhIGNORE_FREEZE,
AuthHandling::ahIGNORE_AUTH,
j_);
XRPL_ASSERT_PARTS(
assetsAvailableAfter == pseudoAccountBalanceAfter,
"xrpl::LoanPay::doApply",
"vault pseudo balance agrees after");
{
Number const pseudoAccountBalanceAfter = accountHolds(
view,
vaultPseudoAccount,
asset,
FreezeHandling::fhIGNORE_FREEZE,
AuthHandling::ahIGNORE_AUTH,
j_);
XRPL_ASSERT_PARTS(
assetsAvailableAfter == pseudoAccountBalanceAfter,
"xrpl::LoanPay::doApply",
"vault pseudo balance agrees after");
}
#endif
// Check that funds are conserved
auto const accountBalanceAfter = accountHolds(
view,
account_,
@@ -612,14 +663,121 @@ LoanPay::doApply()
ahIGNORE_AUTH,
j_,
SpendableHandling::shFULL_BALANCE);
auto const balanceScale = [&]() {
// Find a reasonable scale to use for the balance comparisons.
//
// First find the minimum and maximum exponent of all the non-zero balances, before and
// after. If min and max are equal, use that value. If they are not, use "max + 1" to reduce
// rounding discrepancies without making the result meaningless. Cap the scale at
// STAmount::cMaxOffset, just in case the numbers are all very large.
std::vector<int> exponents;
for (auto const& a : {
accountBalanceBefore,
vaultBalanceBefore,
brokerBalanceBefore,
accountBalanceAfter,
vaultBalanceAfter,
brokerBalanceAfter,
})
{
// Exclude zeroes
if (a != beast::zero)
exponents.push_back(a.exponent());
}
if (exponents.empty())
{
UNREACHABLE("xrpl::LoanPay::doApply : all zeroes");
return 0;
}
auto const [minItr, maxItr] = std::ranges::minmax_element(exponents);
auto const min = *minItr;
auto const max = *maxItr;
JLOG(j_.trace()) << "Min scale: " << min << ", max scale: " << max;
// IOU rounding can be interesting. We want all the balance checks to agree, but don't want
// to round to such an extreme that it becomes meaningless. e.g. Everything rounds to one
// digit. So add 1 to the max (reducing the number of digits after the decimal point by 1)
// if the scales are not already all the same.
return std::min(min == max ? max : max + 1, STAmount::cMaxOffset);
}();
// No object changes are made below this point
XRPL_ASSERT_PARTS(
accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore ==
accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter,
Number::getround() == Number::to_nearest,
"xrpl::LoanPay::doApply",
"Number rounding to_nearest");
NumberRoundModeGuard mg(Number::to_nearest);
auto const accountBalanceBeforeRounded = roundToScale(accountBalanceBefore, balanceScale);
auto const vaultBalanceBeforeRounded = roundToScale(vaultBalanceBefore, balanceScale);
auto const brokerBalanceBeforeRounded = roundToScale(brokerBalanceBefore, balanceScale);
auto const totalBalanceBefore = accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore;
auto const totalBalanceBeforeRounded = roundToScale(totalBalanceBefore, balanceScale);
JLOG(j_.trace()) << "Before: " //
<< "account " << Number(accountBalanceBeforeRounded) << " ("
<< Number(accountBalanceBefore) << ")"
<< ", vault " << Number(vaultBalanceBeforeRounded) << " ("
<< Number(vaultBalanceBefore) << ")"
<< ", broker " << Number(brokerBalanceBeforeRounded) << " ("
<< Number(brokerBalanceBefore) << ")"
<< ", total " << Number(totalBalanceBeforeRounded) << " ("
<< Number(totalBalanceBefore) << ")";
auto const accountBalanceAfterRounded = roundToScale(accountBalanceAfter, balanceScale);
auto const vaultBalanceAfterRounded = roundToScale(vaultBalanceAfter, balanceScale);
auto const brokerBalanceAfterRounded = roundToScale(brokerBalanceAfter, balanceScale);
auto const totalBalanceAfter = accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter;
auto const totalBalanceAfterRounded = roundToScale(totalBalanceAfter, balanceScale);
JLOG(j_.trace()) << "After: " //
<< "account " << Number(accountBalanceAfterRounded) << " ("
<< Number(accountBalanceAfter) << ")"
<< ", vault " << Number(vaultBalanceAfterRounded) << " ("
<< Number(vaultBalanceAfter) << ")"
<< ", broker " << Number(brokerBalanceAfterRounded) << " ("
<< Number(brokerBalanceAfter) << ")"
<< ", total " << Number(totalBalanceAfterRounded) << " ("
<< Number(totalBalanceAfter) << ")";
auto const accountBalanceChange = accountBalanceAfter - accountBalanceBefore;
auto const vaultBalanceChange = vaultBalanceAfter - vaultBalanceBefore;
auto const brokerBalanceChange = brokerBalanceAfter - brokerBalanceBefore;
auto const totalBalanceChange = accountBalanceChange + vaultBalanceChange + brokerBalanceChange;
auto const totalBalanceChangeRounded = roundToScale(totalBalanceChange, balanceScale);
JLOG(j_.trace()) << "Changes: " //
<< "account " << to_string(accountBalanceChange) //
<< ", vault " << to_string(vaultBalanceChange) //
<< ", broker " << to_string(brokerBalanceChange) //
<< ", total " << to_string(totalBalanceChangeRounded) << " ("
<< Number(totalBalanceChange) << ")";
if (totalBalanceBeforeRounded != totalBalanceAfterRounded)
{
JLOG(j_.warn()) << "Total rounded balances don't match"
<< (totalBalanceChangeRounded == beast::zero ? ", but total changes do"
: "");
}
if (totalBalanceChangeRounded != beast::zero)
{
JLOG(j_.warn()) << "Total balance changes don't match"
<< (totalBalanceBeforeRounded == totalBalanceAfterRounded
? ", but total balances do"
: "");
}
// Rounding for IOUs can be weird, so check a few different ways to show
// that funds are conserved.
XRPL_ASSERT_PARTS(
totalBalanceBeforeRounded == totalBalanceAfterRounded ||
totalBalanceChangeRounded == beast::zero,
"xrpl::LoanPay::doApply",
"funds are conserved (with rounding)");
XRPL_ASSERT_PARTS(
accountBalanceAfter >= beast::zero, "xrpl::LoanPay::doApply", "positive account balance");
XRPL_ASSERT_PARTS(
accountBalanceAfter < accountBalanceBefore || account_ == asset.getIssuer(),
"xrpl::LoanPay::doApply",
@@ -640,7 +798,6 @@ LoanPay::doApply()
vaultBalanceAfter > vaultBalanceBefore || brokerBalanceAfter > brokerBalanceBefore,
"xrpl::LoanPay::doApply",
"vault and/or broker balance increased");
#endif
return tesSUCCESS;
}

View File

@@ -370,16 +370,11 @@ protected:
env.balance(vaultPseudo, broker.asset).number());
if (ownerCount == 0)
{
// Allow some slop for rounding IOUs
// TODO: This needs to be an exact match once all the
// other rounding issues are worked out.
// The Vault must be perfectly balanced if there
// are no loans outstanding
auto const total = vaultSle->at(sfAssetsTotal);
auto const available = vaultSle->at(sfAssetsAvailable);
env.test.BEAST_EXPECT(
total == available ||
(!broker.asset.integral() && available != 0 &&
((total - available) / available < Number(1, -6))));
env.test.BEAST_EXPECT(total == available);
env.test.BEAST_EXPECT(vaultSle->at(sfLossUnrealized) == 0);
}
}
@@ -7068,6 +7063,140 @@ protected:
BEAST_EXPECT(afterSecondCoverAvailable == 0);
}
void
testYieldTheftRounding(std::uint32_t flags)
{
testcase("Rounding manipulation does not permit yield theft");
using namespace jtx;
using namespace loan;
// 1. Setup Environment
Env env(*this, all);
Account const issuer{"issuer"};
Account const lender{"lender"};
Account const borrower{"borrower"};
env.fund(XRP(1000), issuer, lender, borrower);
env.close();
// 2. Asset Selection
PrettyAsset const iou = issuer["USD"];
env(trust(lender, iou(100'000'000)));
env(trust(borrower, iou(100'000'000)));
env(pay(issuer, lender, iou(100'000'000)));
env(pay(issuer, borrower, iou(100'000'000)));
env.close();
// 3. Create Vault and Broker with High Debt Limit (100M)
auto const brokerInfo = createVaultAndBroker(
env,
iou,
lender,
{
.vaultDeposit = 5'000'000,
.debtMax = Number{100'000'000},
.coverDeposit = 500'000,
});
auto const [currentSeq, vaultKeylet] = [&]() {
auto const brokerSle = env.le(keylet::loanbroker(brokerInfo.brokerID));
auto const currentSeq = brokerSle->at(sfLoanSequence);
auto const vaultKeylet = keylet::vault(brokerSle->at(sfVaultID));
return std::make_tuple(currentSeq, vaultKeylet);
}();
// 4. Loan Parameters (Attack Vector)
Number const principal = 1'000'000;
TenthBips32 const interestRate = TenthBips32{1}; // 0.001%
std::uint32_t const paymentInterval = 86400;
std::uint32_t const paymentTotal = 3650;
auto const loanSetFee = fee(env.current()->fees().base * 2);
env(set(borrower, brokerInfo.brokerID, iou(principal).value(), flags),
sig(sfCounterpartySignature, lender),
loan::interestRate(interestRate),
loan::paymentInterval(paymentInterval),
loan::paymentTotal(paymentTotal),
fee(loanSetFee));
env.close();
// --- RETRIEVE OBJECTS & SETUP ATTACK ---
auto borrowerBalance = [&]() { return env.balance(borrower, iou); };
auto const borrowerScale = static_cast<STAmount const&>(borrowerBalance()).exponent();
auto const loanKeylet = keylet::loan(brokerInfo.brokerID, currentSeq);
auto const maybePeriodicPayment = [&]() -> std::optional<STAmount> {
auto const loanSle = env.le(loanKeylet);
if (!BEAST_EXPECT(loanSle))
return std::nullopt;
// Construct Payment
return STAmount{iou, loanSle->at(sfPeriodicPayment)};
}();
if (!maybePeriodicPayment)
return;
auto const periodicPayment = *maybePeriodicPayment;
auto const roundedPayment = roundToScale(periodicPayment, borrowerScale, Number::upward);
// ATTACK: Add dust buffer (1e-9) to force 'excess' logic execution
STAmount const paymentBuffer{iou, Number(1, -9)};
STAmount const attackPayment = periodicPayment + paymentBuffer;
auto const maybeInitialVaultAssets = [&]() -> std::optional<Number> {
auto const vault = env.le(vaultKeylet);
if (!BEAST_EXPECT(vault))
return std::nullopt;
return vault->at(sfAssetsTotal);
}();
if (!maybeInitialVaultAssets)
return;
auto const initialVaultAssets = *maybeInitialVaultAssets;
// 5. Execution Loop
int yieldTheftCount = 0;
auto previousAssetsTotal = initialVaultAssets;
for (int i = 0; i < 100; ++i)
{
auto const balanceBefore = borrowerBalance();
env(pay(borrower, loanKeylet.key, attackPayment, flags));
env.close();
auto const borrowerDelta = balanceBefore - borrowerBalance();
BEAST_EXPECT(borrowerDelta.signum() == roundedPayment.signum());
auto const loanSle = env.le(loanKeylet);
if (!BEAST_EXPECT(loanSle))
break;
auto const updatedPayment = STAmount{iou, loanSle->at(sfPeriodicPayment)};
BEAST_EXPECT(
(roundToScale(updatedPayment, borrowerScale, Number::upward) == roundedPayment));
BEAST_EXPECT(
(updatedPayment == periodicPayment) ||
(flags == tfLoanOverpayment && i >= 2 && updatedPayment < periodicPayment));
auto const currentVaultSle = env.le(vaultKeylet);
if (!BEAST_EXPECT(currentVaultSle))
break;
auto const currentAssetsTotal = currentVaultSle->at(sfAssetsTotal);
auto const delta = currentAssetsTotal - previousAssetsTotal;
BEAST_EXPECT(
(delta == beast::zero && borrowerDelta <= roundedPayment) ||
(delta > beast::zero && borrowerDelta > roundedPayment));
// If tx succeeded but Assets Total didn't change, interest was
// stolen.
if (delta == beast::zero && borrowerDelta > roundedPayment)
{
yieldTheftCount++;
}
previousAssetsTotal = currentAssetsTotal;
}
BEAST_EXPECTS(yieldTheftCount == 0, std::to_string(yieldTheftCount));
}
// 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.
@@ -7206,6 +7335,11 @@ public:
testLoanPayLateFullPaymentBypassesPenalties();
testLoanCoverMinimumRoundingExploit();
#endif
for (auto const flags : {0u, tfLoanOverpayment})
{
testYieldTheftRounding(flags);
}
testWithdrawReflectsUnrealizedLoss();
testInvalidLoanSet();

View File

@@ -8,9 +8,6 @@
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/protocol/jss.h>
#include <set>
#include <string>
namespace xrpl::test {
class ServerDefinitions_test : public beast::unit_test::suite
@@ -40,23 +37,20 @@ public:
{
auto const firstField = result[jss::result][jss::FIELDS][0u];
BEAST_EXPECT(firstField[0u].asString() == "Invalid");
BEAST_EXPECT(firstField[0u].asString() == "Generic");
BEAST_EXPECT(firstField[1][jss::isSerialized].asBool() == false);
BEAST_EXPECT(firstField[1][jss::isSigningField].asBool() == false);
BEAST_EXPECT(firstField[1][jss::isVLEncoded].asBool() == false);
BEAST_EXPECT(firstField[1][jss::nth].asInt() == -1);
BEAST_EXPECT(firstField[1][jss::nth].asUInt() == 0);
BEAST_EXPECT(firstField[1][jss::type].asString() == "Unknown");
}
{
auto const field = result[jss::result][jss::FIELDS][6u];
BEAST_EXPECT(field[0u].asString() == "LedgerEntryType");
BEAST_EXPECT(field[1][jss::isSerialized].asBool() == true);
BEAST_EXPECT(field[1][jss::isSigningField].asBool() == true);
BEAST_EXPECT(field[1][jss::isVLEncoded].asBool() == false);
BEAST_EXPECT(field[1][jss::nth].asUInt() == 1);
BEAST_EXPECT(field[1][jss::type].asString() == "UInt16");
}
BEAST_EXPECT(
result[jss::result][jss::LEDGER_ENTRY_TYPES]["AccountRoot"].asUInt() == 97);
BEAST_EXPECT(
result[jss::result][jss::TRANSACTION_RESULTS]["tecDIR_FULL"].asUInt() == 121);
BEAST_EXPECT(result[jss::result][jss::TRANSACTION_TYPES]["Payment"].asUInt() == 0);
BEAST_EXPECT(result[jss::result][jss::TYPES]["AccountID"].asUInt() == 8);
// check exception SFields
{
@@ -80,34 +74,17 @@ public:
BEAST_EXPECT(fieldExists("index"));
}
// verify no duplicate field names in FIELDS array
{
std::set<std::string> fieldNames;
for (auto const& field : result[jss::result][jss::FIELDS])
{
auto const name = field[0u].asString();
BEAST_EXPECT(fieldNames.insert(name).second);
}
}
// test that base_uint types are replaced with "Hash" prefix
{
auto const types = result[jss::result][jss::TYPES];
BEAST_EXPECT(types.isMember("Hash128") && types["Hash128"].asUInt() == 4);
BEAST_EXPECT(types.isMember("Hash160") && types["Hash160"].asUInt() == 17);
BEAST_EXPECT(types.isMember("Hash192") && types["Hash192"].asUInt() == 21);
BEAST_EXPECT(types.isMember("Hash256") && types["Hash256"].asUInt() == 5);
BEAST_EXPECT(types.isMember("Hash384") && types["Hash384"].asUInt() == 22);
BEAST_EXPECT(types.isMember("Hash512") && types["Hash512"].asUInt() == 23);
BEAST_EXPECT(types["Hash128"].asUInt() == 4);
BEAST_EXPECT(types["Hash160"].asUInt() == 17);
BEAST_EXPECT(types["Hash192"].asUInt() == 21);
BEAST_EXPECT(types["Hash256"].asUInt() == 5);
BEAST_EXPECT(types["Hash384"].asUInt() == 22);
BEAST_EXPECT(types["Hash512"].asUInt() == 23);
}
BEAST_EXPECT(
result[jss::result][jss::LEDGER_ENTRY_TYPES]["AccountRoot"].asUInt() == 97);
BEAST_EXPECT(
result[jss::result][jss::TRANSACTION_RESULTS]["tecDIR_FULL"].asUInt() == 121);
BEAST_EXPECT(result[jss::result][jss::TRANSACTION_TYPES]["Payment"].asUInt() == 0);
BEAST_EXPECT(result[jss::result][jss::TYPES]["AccountID"].asUInt() == 8);
// test the properties of the LEDGER_ENTRY_FLAGS section
{
BEAST_EXPECT(result[jss::result].isMember(jss::LEDGER_ENTRY_FLAGS));

View File

@@ -149,6 +149,18 @@ ServerDefinitions::ServerDefinitions() : defs_{Json::objectValue}
defs_[jss::FIELDS] = Json::arrayValue;
uint32_t i = 0;
{
Json::Value a = Json::arrayValue;
a[0U] = "Generic";
Json::Value v = Json::objectValue;
v[jss::nth] = 0;
v[jss::isVLEncoded] = false;
v[jss::isSerialized] = false;
v[jss::isSigningField] = false;
v[jss::type] = "Unknown";
a[1U] = v;
defs_[jss::FIELDS][i++] = a;
}
{
Json::Value a = Json::arrayValue;
@@ -215,25 +227,21 @@ ServerDefinitions::ServerDefinitions() : defs_{Json::objectValue}
defs_[jss::FIELDS][i++] = a;
}
// copy into a sorted map to ensure deterministic output order (sorted by fieldCode)
static std::map<int, SField const*> const sortedFields(
xrpl::SField::getKnownCodeToField().begin(), xrpl::SField::getKnownCodeToField().end());
for (auto const& [code, field] : sortedFields)
for (auto const& [code, field] : xrpl::SField::getKnownCodeToField())
{
if (field->fieldName.empty())
continue;
Json::Value innerObj = Json::objectValue;
int32_t const type = field->fieldType;
uint32_t type = field->fieldType;
innerObj[jss::nth] = field->fieldValue;
// whether the field is variable-length encoded this means that the length is included
// before the content
innerObj[jss::isVLEncoded] =
(type == STI_VL || type == STI_ACCOUNT || type == STI_VECTOR256);
(type == 7U /* Blob */ || type == 8U /* AccountID */ || type == 19U /* Vector256 */);
// whether the field is included in serialization
innerObj[jss::isSerialized] =