Compare commits

..

15 Commits

Author SHA1 Message Date
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
7 changed files with 327 additions and 93 deletions

View File

@@ -20,10 +20,6 @@ removeTokenOffersWithLimit(
Keylet const& directory,
std::size_t maxDeletableOffers);
/** Returns tesSUCCESS if NFToken has few enough offers that it can be burned */
TER
notTooManyOffers(ReadView const& view, uint256 const& nftokenID);
/** Finds the specified token in the owner's token directory. */
std::optional<STObject>
findToken(ReadView const& view, AccountID const& owner, uint256 const& nftokenID);

View File

@@ -42,8 +42,8 @@ private:
public:
using value_type = STAmount;
static int const cMinOffset = -96;
static int const cMaxOffset = 80;
static constexpr int cMinOffset = -96;
static constexpr int cMaxOffset = 80;
// Maximum native value supported by the code
constexpr static std::uint64_t cMinValue = 1'000'000'000'000'000ull;

View File

@@ -630,33 +630,6 @@ removeTokenOffersWithLimit(ApplyView& view, Keylet const& directory, std::size_t
return deletedOffersCount;
}
TER
notTooManyOffers(ReadView const& view, uint256 const& nftokenID)
{
std::size_t totalOffers = 0;
{
Dir const buys(view, keylet::nft_buys(nftokenID));
for (auto iter = buys.begin(); iter != buys.end(); iter.next_page())
{
totalOffers += iter.page_size();
if (totalOffers > maxDeletableTokenOfferEntries)
return tefTOO_BIG;
}
}
{
Dir const sells(view, keylet::nft_sells(nftokenID));
for (auto iter = sells.begin(); iter != sells.end(); iter.next_page())
{
totalOffers += iter.page_size();
if (totalOffers > maxDeletableTokenOfferEntries)
return tefTOO_BIG;
}
}
return tesSUCCESS;
}
bool
deleteTokenOffer(ApplyView& view, std::shared_ptr<SLE> const& offer)
{

View File

@@ -1847,8 +1847,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)};
@@ -1857,7 +1867,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

@@ -27,6 +27,7 @@
#include <algorithm>
#include <bit>
#include <cstdint>
#include <vector>
namespace xrpl {
@@ -437,9 +438,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,
@@ -463,16 +465,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
@@ -498,12 +490,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_,
@@ -532,7 +580,6 @@ LoanPay::doApply()
ahIGNORE_AUTH,
j_,
SpendableHandling::shFULL_BALANCE);
#endif
if (totalPaidToVaultRounded != beast::zero)
{
@@ -568,19 +615,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_,
@@ -609,14 +659,114 @@ 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::minmax_element(exponents.begin(), exponents.end());
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);
}();
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(
accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore ==
accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter,
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",
@@ -637,7 +787,6 @@ LoanPay::doApply()
vaultBalanceAfter > vaultBalanceBefore || brokerBalanceAfter > brokerBalanceBefore,
"xrpl::LoanPay::doApply",
"vault and/or broker balance increased");
#endif
return tesSUCCESS;
}

View File

@@ -368,16 +368,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);
}
}
@@ -7067,6 +7062,129 @@ protected:
BEAST_EXPECT(afterSecondCoverAvailable == 0);
}
void
testYieldTheftRounding(std::uint32_t flags)
{
testcase("Yield Theft via Rounding Manipulation");
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, vaultId, vaultKeylet] = [&]() {
auto const brokerSle = env.le(keylet::loanbroker(brokerInfo.brokerID));
auto const currentSeq = brokerSle->at(sfLoanSequence);
auto const vaultKeylet = keylet::vault(brokerSle->at(sfVaultID));
auto const vaultId = brokerSle->at(sfVaultID);
return std::make_tuple(currentSeq, vaultId, 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 [periodicPayment, loanScale] = [&]() {
auto const loanSle = env.le(loanKeylet);
// Construct Payment
return std::make_tuple(
STAmount{iou, loanSle->at(sfPeriodicPayment)}, loanSle->at(sfLoanScale));
}();
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 initialVaultAssets = env.le(vaultKeylet)->at(sfAssetsTotal);
// 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));
}
public:
void
run() override
@@ -7075,6 +7193,11 @@ public:
testLoanPayLateFullPaymentBypassesPenalties();
testLoanCoverMinimumRoundingExploit();
#endif
for (auto const flags : {0u, tfLoanOverpayment})
{
testYieldTheftRounding(flags);
}
testInvalidLoanSet();
auto const all = jtx::testable_amendments();

View File

@@ -161,11 +161,7 @@ ValidatorSite::load(
{
try
{
// This is not super efficient, but it doesn't happen often.
bool found = std::ranges::any_of(
sites_, [&uri](auto const& site) { return site.loadedResource->uri == uri; });
if (!found)
sites_.emplace_back(uri);
sites_.emplace_back(uri);
}
catch (std::exception const& e)
{
@@ -226,16 +222,6 @@ ValidatorSite::setTimer(
std::lock_guard<std::mutex> const& site_lock,
std::lock_guard<std::mutex> const& state_lock)
{
if (!sites_.empty() && //
std::ranges::all_of(
sites_, [](auto const& site) { return site.lastRefreshStatus.has_value(); }))
{
// If all of the sites have been handled at least once (including
// errors and timeouts), call missingSite, which will load the cache
// files for any lists that are still unavailable.
missingSite(site_lock);
}
auto next = std::min_element(sites_.begin(), sites_.end(), [](Site const& a, Site const& b) {
return a.nextRefresh < b.nextRefresh;
});
@@ -346,7 +332,7 @@ ValidatorSite::onRequestTimeout(std::size_t siteIdx, error_code const& ec)
// processes a network error. Usually, this function runs first,
// but on extremely rare occasions, the response handler can run
// first, which will leave activeResource empty.
auto& site = sites_[siteIdx];
auto const& site = sites_[siteIdx];
if (site.activeResource)
{
JLOG(j_.warn()) << "Request for " << site.activeResource->uri << " took too long";
@@ -354,9 +340,6 @@ ValidatorSite::onRequestTimeout(std::size_t siteIdx, error_code const& ec)
else
JLOG(j_.error()) << "Request took too long, but a response has "
"already been processed";
if (!site.lastRefreshStatus)
site.lastRefreshStatus.emplace(
Site::Status{clock_type::now(), ListDisposition::invalid, "timeout"});
}
std::lock_guard const lock_state{state_mutex_};