Merge branch 'develop' into ximinez/lending-sendmulti

* XRPLF/develop: (105 commits)
  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)
  fix: Minor RPC fixes (6730)
  fix: Prevent deletion of MPTokens with active escrow (6635)
  fix: Clamp VaultClawback to assetsAvailable for zero-amount clawback (6646)
  fix: Add assorted Lending Protocol fixes (6678)
  fix: Change variable signedness and correctly handle `std::optional` (6657)
  refactor: Reorganize RPC handler files (6628)
  chore: Update XRPLF/actions (6713)
  docs: Add explanatory comment to checkFee (6631)
  fix: Decouple reserve from fee in delegate payment (6568)
  fix: Check trustline limits for share-denominated vault withdrawals (6645)
  fix: Remove fatal assertion on Linux thread name truncation  (6690)
  chore: Enable clang-tidy `coreguidelines` checks (6698)
  ci: Allow uploading artifacts for XRPLF org (6702)
  fix: Enforce aggregate MaximumAmount in multi-send MPT (6644)
  chore: Use nudb recipe from the upstream (6701)
  fix: Fix previous ledger size typo in RCLConsensus (6696)
  ...

include/xrpl/protocol/STAmount.h

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

src/libxrpl/ledger/View.cpp

@@ -452,6 +452,489 @@ doWithdraw(
     return accountSend(view, sourceAcct, dstAcct, amount, j, WaiveTransferFee::Yes);
 }
 
+// Direct send w/o fees:
+// - Redeeming IOUs and/or sending sender's own IOUs.
+// - Create trust line if needed.
+// --> bCheckIssuer : normally require issuer to be involved.
+static TER
+rippleCreditIOU(
+    ApplyView& view,
+    AccountID const& uSenderID,
+    AccountID const& uReceiverID,
+    STAmount const& saAmount,
+    bool bCheckIssuer,
+    beast::Journal j)
+{
+    AccountID const& issuer = saAmount.getIssuer();
+    Currency const& currency = saAmount.getCurrency();
+
+    // Make sure issuer is involved.
+    XRPL_ASSERT(
+        !bCheckIssuer || uSenderID == issuer || uReceiverID == issuer,
+        "xrpl::rippleCreditIOU : matching issuer or don't care");
+    (void)issuer;
+
+    // Disallow sending to self.
+    XRPL_ASSERT(uSenderID != uReceiverID, "xrpl::rippleCreditIOU : sender is not receiver");
+
+    bool const bSenderHigh = uSenderID > uReceiverID;
+    auto const index = keylet::line(uSenderID, uReceiverID, currency);
+
+    XRPL_ASSERT(
+        !isXRP(uSenderID) && uSenderID != noAccount(), "xrpl::rippleCreditIOU : sender is not XRP");
+    XRPL_ASSERT(
+        !isXRP(uReceiverID) && uReceiverID != noAccount(),
+        "xrpl::rippleCreditIOU : receiver is not XRP");
+
+    // If the line exists, modify it accordingly.
+    if (auto const sleRippleState = view.peek(index))
+    {
+        STAmount saBalance = sleRippleState->getFieldAmount(sfBalance);
+
+        if (bSenderHigh)
+            saBalance.negate();  // Put balance in sender terms.
+
+        view.creditHook(uSenderID, uReceiverID, saAmount, saBalance);
+
+        STAmount const saBefore = saBalance;
+
+        saBalance -= saAmount;
+
+        JLOG(j.trace()) << "rippleCreditIOU: " << to_string(uSenderID) << " -> "
+                        << to_string(uReceiverID) << " : before=" << saBefore.getFullText()
+                        << " amount=" << saAmount.getFullText()
+                        << " after=" << saBalance.getFullText();
+
+        std::uint32_t const uFlags(sleRippleState->getFieldU32(sfFlags));
+        bool bDelete = false;
+
+        // FIXME This NEEDS to be cleaned up and simplified. It's impossible
+        //       for anyone to understand.
+        if (saBefore > beast::zero
+            // Sender balance was positive.
+            && saBalance <= beast::zero
+            // Sender is zero or negative.
+            && (uFlags & (!bSenderHigh ? lsfLowReserve : lsfHighReserve))
+            // Sender reserve is set.
+            && static_cast<bool>(uFlags & (!bSenderHigh ? lsfLowNoRipple : lsfHighNoRipple)) !=
+                static_cast<bool>(
+                    view.read(keylet::account(uSenderID))->getFlags() & lsfDefaultRipple) &&
+            !(uFlags & (!bSenderHigh ? lsfLowFreeze : lsfHighFreeze)) &&
+            !sleRippleState->getFieldAmount(!bSenderHigh ? sfLowLimit : sfHighLimit)
+            // Sender trust limit is 0.
+            && !sleRippleState->getFieldU32(!bSenderHigh ? sfLowQualityIn : sfHighQualityIn)
+            // Sender quality in is 0.
+            && !sleRippleState->getFieldU32(!bSenderHigh ? sfLowQualityOut : sfHighQualityOut))
+        // Sender quality out is 0.
+        {
+            // Clear the reserve of the sender, possibly delete the line!
+            adjustOwnerCount(view, view.peek(keylet::account(uSenderID)), -1, j);
+
+            // Clear reserve flag.
+            sleRippleState->setFieldU32(
+                sfFlags, uFlags & (!bSenderHigh ? ~lsfLowReserve : ~lsfHighReserve));
+
+            // Balance is zero, receiver reserve is clear.
+            bDelete = !saBalance  // Balance is zero.
+                && !(uFlags & (bSenderHigh ? lsfLowReserve : lsfHighReserve));
+            // Receiver reserve is clear.
+        }
+
+        if (bSenderHigh)
+            saBalance.negate();
+
+        // Want to reflect balance to zero even if we are deleting line.
+        sleRippleState->setFieldAmount(sfBalance, saBalance);
+        // ONLY: Adjust ripple balance.
+
+        if (bDelete)
+        {
+            return trustDelete(
+                view,
+                sleRippleState,
+                bSenderHigh ? uReceiverID : uSenderID,
+                !bSenderHigh ? uReceiverID : uSenderID,
+                j);
+        }
+
+        view.update(sleRippleState);
+        return tesSUCCESS;
+    }
+
+    STAmount const saReceiverLimit(Issue{currency, uReceiverID});
+    STAmount saBalance{saAmount};
+
+    saBalance.setIssuer(noAccount());
+
+    JLOG(j.debug()) << "rippleCreditIOU: "
+                       "create line: "
+                    << to_string(uSenderID) << " -> " << to_string(uReceiverID) << " : "
+                    << saAmount.getFullText();
+
+    auto const sleAccount = view.peek(keylet::account(uReceiverID));
+    if (!sleAccount)
+        return tefINTERNAL;  // LCOV_EXCL_LINE
+
+    bool const noRipple = (sleAccount->getFlags() & lsfDefaultRipple) == 0;
+
+    return trustCreate(
+        view,
+        bSenderHigh,
+        uSenderID,
+        uReceiverID,
+        index.key,
+        sleAccount,
+        false,
+        noRipple,
+        false,
+        false,
+        saBalance,
+        saReceiverLimit,
+        0,
+        0,
+        j);
+}
+
+// Send regardless of limits.
+// --> saAmount: Amount/currency/issuer to deliver to receiver.
+// <-- saActual: Amount actually cost.  Sender pays fees.
+static TER
+rippleSendIOU(
+    ApplyView& view,
+    AccountID const& uSenderID,
+    AccountID const& uReceiverID,
+    STAmount const& saAmount,
+    STAmount& saActual,
+    beast::Journal j,
+    WaiveTransferFee waiveFee)
+{
+    auto const& issuer = saAmount.getIssuer();
+
+    XRPL_ASSERT(
+        !isXRP(uSenderID) && !isXRP(uReceiverID),
+        "xrpl::rippleSendIOU : neither sender nor receiver is XRP");
+    XRPL_ASSERT(uSenderID != uReceiverID, "xrpl::rippleSendIOU : sender is not receiver");
+
+    if (uSenderID == issuer || uReceiverID == issuer || issuer == noAccount())
+    {
+        // Direct send: redeeming IOUs and/or sending own IOUs.
+        auto const ter = rippleCreditIOU(view, uSenderID, uReceiverID, saAmount, false, j);
+        if (ter != tesSUCCESS)
+            return ter;
+        saActual = saAmount;
+        return tesSUCCESS;
+    }
+
+    // Sending 3rd party IOUs: transit.
+
+    // Calculate the amount to transfer accounting
+    // for any transfer fees if the fee is not waived:
+    saActual = (waiveFee == WaiveTransferFee::Yes) ? saAmount
+                                                   : multiply(saAmount, transferRate(view, issuer));
+
+    JLOG(j.debug()) << "rippleSendIOU> " << to_string(uSenderID) << " - > "
+                    << to_string(uReceiverID) << " : deliver=" << saAmount.getFullText()
+                    << " cost=" << saActual.getFullText();
+
+    TER terResult = rippleCreditIOU(view, issuer, uReceiverID, saAmount, true, j);
+
+    if (tesSUCCESS == terResult)
+        terResult = rippleCreditIOU(view, uSenderID, issuer, saActual, true, j);
+
+    return terResult;
+}
+
+template <class TAsset>
+static TER
+doSendMulti(
+    std::string const& name,
+    ApplyView& view,
+    AccountID const& senderID,
+    TAsset const& issue,
+    MultiplePaymentDestinations const& receivers,
+    STAmount& actual,
+    beast::Journal j,
+    WaiveTransferFee waiveFee,
+    // Don't pass back parameters that the caller already has
+    std::function<
+        TER(AccountID const& senderID,
+            AccountID const& receiverID,
+            STAmount const& amount,
+            bool checkIssuer)> doCredit,
+    std::function<
+        TER(AccountID const& issuer, STAmount const& takeFromSender, STAmount const& amount)>
+        preMint = {})
+{
+    // Use the same pattern for all the SendMulti functions to help avoid
+    // divergence and copy/paste errors.
+    auto const& issuer = issue.getIssuer();
+
+    // These values may not stay in sync
+    STAmount takeFromSender{issue};
+    actual = takeFromSender;
+
+    // Failures return immediately.
+    for (auto const& r : receivers)
+    {
+        auto const& receiverID = r.first;
+        STAmount amount{issue, r.second};
+
+        if (amount < beast::zero)
+        {
+            return tecINTERNAL;  // LCOV_EXCL_LINE
+        }
+
+        /* If we aren't sending anything or if the sender is the same as the
+         * receiver then we don't need to do anything.
+         */
+        if (!amount || (senderID == receiverID))
+            continue;
+
+        using namespace std::string_literals;
+        XRPL_ASSERT(!isXRP(receiverID), ("xrpl::"s + name + " : receiver is not XRP").c_str());
+
+        if (senderID == issuer || receiverID == issuer || issuer == noAccount())
+        {
+            if (preMint)
+            {
+                if (auto const ter = preMint(issuer, takeFromSender, amount))
+                    return ter;
+            }
+            // Direct send: redeeming IOUs and/or sending own IOUs.
+            if (auto const ter = doCredit(senderID, receiverID, amount, false))
+                return ter;
+            actual += amount;
+            // Do not add amount to takeFromSender, because doCredit took
+            // it.
+
+            continue;
+        }
+
+        // Sending 3rd party: transit.
+
+        // Calculate the amount to transfer accounting
+        // for any transfer fees if the fee is not waived:
+        STAmount actualSend = (waiveFee == WaiveTransferFee::Yes || issue.native())
+            ? amount
+            : multiply(amount, transferRate(view, amount));
+        actual += actualSend;
+        takeFromSender += actualSend;
+
+        JLOG(j.debug()) << name << "> " << to_string(senderID) << " - > " << to_string(receiverID)
+                        << " : deliver=" << amount.getFullText()
+                        << " cost=" << actualSend.getFullText();
+
+        if (TER const terResult = doCredit(issuer, receiverID, amount, true))
+            return terResult;
+    }
+
+    if (senderID != issuer && takeFromSender)
+    {
+        if (TER const terResult = doCredit(senderID, issuer, takeFromSender, true))
+            return terResult;
+    }
+
+    return tesSUCCESS;
+}
+
+// Send regardless of limits.
+// --> receivers: Amount/currency/issuer to deliver to receivers.
+// <-- saActual: Amount actually cost to sender.  Sender pays fees.
+static TER
+rippleSendMultiIOU(
+    ApplyView& view,
+    AccountID const& senderID,
+    Issue const& issue,
+    MultiplePaymentDestinations const& receivers,
+    STAmount& actual,
+    beast::Journal j,
+    WaiveTransferFee waiveFee)
+{
+    XRPL_ASSERT(!isXRP(senderID), "xrpl::rippleSendMultiIOU : sender is not XRP");
+
+    auto doCredit = [&view, j](
+                        AccountID const& senderID,
+                        AccountID const& receiverID,
+                        STAmount const& amount,
+                        bool checkIssuer) {
+        return rippleCreditIOU(view, senderID, receiverID, amount, checkIssuer, j);
+    };
+
+    return doSendMulti(
+        "rippleSendMultiIOU", view, senderID, issue, receivers, actual, j, waiveFee, doCredit);
+}
+
+static TER
+rippleCreditMPT(
+    ApplyView& view,
+    AccountID const& uSenderID,
+    AccountID const& uReceiverID,
+    STAmount const& saAmount,
+    beast::Journal j)
+{
+    // Do not check MPT authorization here - it must have been checked earlier
+    auto const mptID = keylet::mptIssuance(saAmount.get<MPTIssue>().getMptID());
+    auto const& issuer = saAmount.getIssuer();
+    auto sleIssuance = view.peek(mptID);
+    if (!sleIssuance)
+        return tecOBJECT_NOT_FOUND;
+    if (uSenderID == issuer)
+    {
+        (*sleIssuance)[sfOutstandingAmount] += saAmount.mpt().value();
+        view.update(sleIssuance);
+    }
+    else
+    {
+        auto const mptokenID = keylet::mptoken(mptID.key, uSenderID);
+        if (auto sle = view.peek(mptokenID))
+        {
+            auto const amt = sle->getFieldU64(sfMPTAmount);
+            auto const pay = saAmount.mpt().value();
+            if (amt < pay)
+                return tecINSUFFICIENT_FUNDS;
+            (*sle)[sfMPTAmount] = amt - pay;
+            view.update(sle);
+        }
+        else
+            return tecNO_AUTH;
+    }
+
+    if (uReceiverID == issuer)
+    {
+        auto const outstanding = sleIssuance->getFieldU64(sfOutstandingAmount);
+        auto const redeem = saAmount.mpt().value();
+        if (outstanding >= redeem)
+        {
+            sleIssuance->setFieldU64(sfOutstandingAmount, outstanding - redeem);
+            view.update(sleIssuance);
+        }
+        else
+            return tecINTERNAL;  // LCOV_EXCL_LINE
+    }
+    else
+    {
+        auto const mptokenID = keylet::mptoken(mptID.key, uReceiverID);
+        if (auto sle = view.peek(mptokenID))
+        {
+            (*sle)[sfMPTAmount] += saAmount.mpt().value();
+            view.update(sle);
+        }
+        else
+            return tecNO_AUTH;
+    }
+
+    return tesSUCCESS;
+}
+
+static TER
+rippleSendMPT(
+    ApplyView& view,
+    AccountID const& uSenderID,
+    AccountID const& uReceiverID,
+    STAmount const& saAmount,
+    STAmount& saActual,
+    beast::Journal j,
+    WaiveTransferFee waiveFee)
+{
+    XRPL_ASSERT(uSenderID != uReceiverID, "xrpl::rippleSendMPT : sender is not receiver");
+
+    // Safe to get MPT since rippleSendMPT is only called by accountSendMPT
+    auto const& issuer = saAmount.getIssuer();
+
+    auto const sle = view.read(keylet::mptIssuance(saAmount.get<MPTIssue>().getMptID()));
+    if (!sle)
+        return tecOBJECT_NOT_FOUND;
+
+    if (uSenderID == issuer || uReceiverID == issuer)
+    {
+        // if sender is issuer, check that the new OutstandingAmount will not
+        // exceed MaximumAmount
+        if (uSenderID == issuer)
+        {
+            auto const sendAmount = saAmount.mpt().value();
+            auto const maximumAmount = sle->at(~sfMaximumAmount).value_or(maxMPTokenAmount);
+            if (sendAmount > maximumAmount ||
+                sle->getFieldU64(sfOutstandingAmount) > maximumAmount - sendAmount)
+                return tecPATH_DRY;
+        }
+
+        // Direct send: redeeming MPTs and/or sending own MPTs.
+        auto const ter = rippleCreditMPT(view, uSenderID, uReceiverID, saAmount, j);
+        if (ter != tesSUCCESS)
+            return ter;
+        saActual = saAmount;
+        return tesSUCCESS;
+    }
+
+    // Sending 3rd party MPTs: transit.
+    saActual = (waiveFee == WaiveTransferFee::Yes)
+        ? saAmount
+        : multiply(saAmount, transferRate(view, saAmount.get<MPTIssue>().getMptID()));
+
+    JLOG(j.debug()) << "rippleSendMPT> " << to_string(uSenderID) << " - > "
+                    << to_string(uReceiverID) << " : deliver=" << saAmount.getFullText()
+                    << " cost=" << saActual.getFullText();
+
+    if (auto const terResult = rippleCreditMPT(view, issuer, uReceiverID, saAmount, j);
+        terResult != tesSUCCESS)
+        return terResult;
+
+    return rippleCreditMPT(view, uSenderID, issuer, saActual, j);
+}
+
+static TER
+rippleSendMultiMPT(
+    ApplyView& view,
+    AccountID const& senderID,
+    MPTIssue const& mptIssue,
+    MultiplePaymentDestinations const& receivers,
+    STAmount& actual,
+    beast::Journal j,
+    WaiveTransferFee waiveFee)
+{
+    auto const sle = view.read(keylet::mptIssuance(mptIssue.getMptID()));
+    if (!sle)
+        return tecOBJECT_NOT_FOUND;
+
+    auto preMint = [&](AccountID const& issuer,
+                       STAmount const& takeFromSender,
+                       STAmount const& amount) -> TER {
+        // if sender is issuer, check that the new OutstandingAmount will
+        // not exceed MaximumAmount
+        if (senderID == issuer)
+        {
+            XRPL_ASSERT_PARTS(
+                takeFromSender == beast::zero,
+                "rippler::rippleSendMultiMPT",
+                "sender == issuer, takeFromSender == zero");
+            auto const sendAmount = amount.mpt().value();
+            auto const maximumAmount = sle->at(~sfMaximumAmount).value_or(maxMPTokenAmount);
+            if (sendAmount > maximumAmount ||
+                sle->getFieldU64(sfOutstandingAmount) > maximumAmount - sendAmount)
+                return tecPATH_DRY;
+        }
+
+        return tesSUCCESS;
+    };
+    auto doCredit =
+        [&view, j](
+            AccountID const& senderID, AccountID const& receiverID, STAmount const& amount, bool) {
+            return rippleCreditMPT(view, senderID, receiverID, amount, j);
+        };
+
+    return doSendMulti(
+        "rippleSendMultiMPT",
+        view,
+        senderID,
+        mptIssue,
+        receivers,
+        actual,
+        j,
+        waiveFee,
+        doCredit,
+        preMint);
+}
+
 TER
 cleanupOnAccountDelete(
     ApplyView& view,

src/libxrpl/ledger/helpers/OfferHelpers.cpp

@@ -21,7 +21,7 @@ offerDelete(ApplyView& view, std::shared_ptr<SLE> const& sle, beast::Journal j)
     if (!sle)
         return tesSUCCESS;
     auto offerIndex = sle->key();
-    auto owner = sle->getAccountID(sfAccount);
+    auto const owner = sle->getAccountID(sfAccount);
 
     // Detect legacy directories.
     uint256 const uDirectory = sle->getFieldH256(sfBookDirectory);

src/libxrpl/ledger/helpers/RippleStateHelpers.cpp

@@ -579,7 +579,7 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account,
     return tesSUCCESS;
 }
 
-TER
+[[nodiscard]] TER
 canTransfer(ReadView const& view, Issue const& issue, AccountID const& from, AccountID const& to)
 {
     if (issue.native())
@@ -617,7 +617,7 @@ canTransfer(ReadView const& view, Issue const& issue, AccountID const& from, Acc
 //
 //------------------------------------------------------------------------------
 
-TER
+[[nodiscard]] TER
 addEmptyHolding(
     ApplyView& view,
     AccountID const& accountID,
@@ -671,7 +671,7 @@ addEmptyHolding(
         journal);
 }
 
-TER
+[[nodiscard]] TER
 removeEmptyHolding(
     ApplyView& view,
     AccountID const& accountID,

src/libxrpl/ledger/helpers/TokenHelpers.cpp

@@ -938,6 +938,85 @@ accountSendMultiIOU(
                << receivers.size() << " receivers.";
     }
 
+    auto doCredit = [&view, &sender, &receivers, j](
+                        AccountID const& senderID,
+                        AccountID const& receiverID,
+                        STAmount const& amount,
+                        bool /*checkIssuer*/) -> TER {
+        if (!senderID)
+        {
+            SLE::pointer receiver =
+                receiverID != beast::zero ? view.peek(keylet::account(receiverID)) : SLE::pointer();
+
+            if (auto stream = j.trace())
+            {
+                std::string receiver_bal("-");
+
+                if (receiver)
+                    receiver_bal = receiver->getFieldAmount(sfBalance).getFullText();
+
+                stream << "accountSendMultiIOU> " << to_string(senderID) << " -> "
+                       << to_string(receiverID) << " (" << receiver_bal
+                       << ") : " << amount.getFullText();
+            }
+
+            if (receiver)
+            {
+                // Increment XRP balance.
+                auto const rcvBal = receiver->getFieldAmount(sfBalance);
+                receiver->setFieldAmount(sfBalance, rcvBal + amount);
+                view.creditHook(xrpAccount(), receiverID, amount, -rcvBal);
+
+                view.update(receiver);
+            }
+
+            if (auto stream = j.trace())
+            {
+                std::string receiver_bal("-");
+
+                if (receiver)
+                    receiver_bal = receiver->getFieldAmount(sfBalance).getFullText();
+
+                stream << "accountSendMultiIOU< " << to_string(senderID) << " -> "
+                       << to_string(receiverID) << " (" << receiver_bal
+                       << ") : " << amount.getFullText();
+            }
+            return tesSUCCESS;
+        }
+        // Sender
+        if (sender)
+        {
+            if (sender->getFieldAmount(sfBalance) < amount)
+            {
+                return TER{tecFAILED_PROCESSING};
+            }
+            else
+            {
+                auto const sndBal = sender->getFieldAmount(sfBalance);
+                view.creditHook(senderID, xrpAccount(), amount, sndBal);
+
+                // Decrement XRP balance.
+                sender->setFieldAmount(sfBalance, sndBal - amount);
+                view.update(sender);
+            }
+        }
+
+        if (auto stream = j.trace())
+        {
+            std::string sender_bal("-");
+
+            if (sender)
+                sender_bal = sender->getFieldAmount(sfBalance).getFullText();
+
+            stream << "accountSendMultiIOU< " << to_string(senderID) << " (" << sender_bal
+                   << ") -> " << receivers.size() << " receivers.";
+        }
+
+        return tesSUCCESS;
+    };
+    return doSendMulti(
+        "accountSendMultiIOU", view, senderID, issue, receivers, actual, j, waiveFee, doCredit);
+
     // Failures return immediately.
     STAmount takeFromSender{issue};
     for (auto const& r : receivers)

src/libxrpl/tx/transactors/lending/LendingHelpers.cpp

@@ -1847,18 +1847,8 @@ 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 < roundedAmount &&
+        paymentRemainingProxy > 0 && totalPaid < amount &&
         numPayments < loanMaximumPaymentsPerTransaction)
     {
         TenthBips32 const overpaymentInterestRate{loan->at(sfOverpaymentInterestRate)};
@@ -1867,7 +1857,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(roundedAmount - totalPaid, *totalValueOutstandingProxy);
+        Number const overpayment = std::min(amount - totalPaid, *totalValueOutstandingProxy);
 
         detail::ExtendedPaymentComponents const overpaymentComponents =
             detail::computeOverpaymentComponents(

src/libxrpl/tx/transactors/lending/LoanPay.cpp

@@ -27,7 +27,6 @@
 #include <algorithm>
 #include <bit>
 #include <cstdint>
-#include <vector>
 
 namespace xrpl {
 
@@ -438,10 +437,9 @@ 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,
@@ -465,6 +463,16 @@ 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
@@ -490,68 +498,12 @@ LoanPay::doApply()
     associateAsset(*vaultSle, asset);
 
     // Duplicate some checks after rounding
-    Number const assetsAvailableAfter = *assetsAvailableProxy;
-    Number const assetsTotalAfter = *assetsTotalProxy;
-
     XRPL_ASSERT_PARTS(
-        assetsAvailableAfter <= assetsTotalAfter,
+        *assetsAvailableProxy <= *assetsTotalProxy,
         "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
-    }
 
-    // These three values are used to check that funds are conserved after the transfers
+#if !NDEBUG
     auto const accountBalanceBefore = accountHolds(
         view,
         account_,
@@ -580,6 +532,7 @@ LoanPay::doApply()
               ahIGNORE_AUTH,
               j_,
               SpendableHandling::shFULL_BALANCE);
+#endif
 
     if (totalPaidToVaultRounded != beast::zero)
     {
@@ -615,22 +568,19 @@ LoanPay::doApply()
         return ter;
 
 #if !NDEBUG
-    {
-        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
+    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");
 
-    // Check that funds are conserved
     auto const accountBalanceAfter = accountHolds(
         view,
         account_,
@@ -659,114 +609,14 @@ 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(
-        totalBalanceBeforeRounded == totalBalanceAfterRounded ||
-            totalBalanceChangeRounded == beast::zero,
+        accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore ==
+            accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter,
         "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",
@@ -787,6 +637,7 @@ LoanPay::doApply()
         vaultBalanceAfter > vaultBalanceBefore || brokerBalanceAfter > brokerBalanceBefore,
         "xrpl::LoanPay::doApply",
         "vault and/or broker balance increased");
+#endif
 
     return tesSUCCESS;
 }

src/test/app/Loan_test.cpp

@@ -368,11 +368,16 @@ protected:
                         env.balance(vaultPseudo, broker.asset).number());
                     if (ownerCount == 0)
                     {
-                        // The Vault must be perfectly balanced if there
-                        // are no loans outstanding
+                        // Allow some slop for rounding IOUs
+
+                        // TODO: This needs to be an exact match once all the
+                        // other rounding issues are worked out.
                         auto const total = vaultSle->at(sfAssetsTotal);
                         auto const available = vaultSle->at(sfAssetsAvailable);
-                        env.test.BEAST_EXPECT(total == available);
+                        env.test.BEAST_EXPECT(
+                            total == available ||
+                            (!broker.asset.integral() && available != 0 &&
+                             ((total - available) / available < Number(1, -6))));
                         env.test.BEAST_EXPECT(vaultSle->at(sfLossUnrealized) == 0);
                     }
                 }
@@ -7062,129 +7067,6 @@ 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
@@ -7193,11 +7075,6 @@ public:
         testLoanPayLateFullPaymentBypassesPenalties();
         testLoanCoverMinimumRoundingExploit();
 #endif
-        for (auto const flags : {0u, tfLoanOverpayment})
-        {
-            testYieldTheftRounding(flags);
-        }
-
         testInvalidLoanSet();
 
         auto const all = jtx::testable_amendments();