fix: MPT Transfer Fee shouldn't be bypassable via Small-Amount Escrow Rounding

include/xrpl/ledger/helpers/EscrowHelpers.h

@@ -2,7 +2,9 @@
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/beast/utility/Journal.h>
+#include <xrpl/beast/utility/Zero.h>
 #include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/ReadView.h>
 #include <xrpl/ledger/helpers/AccountRootHelpers.h>
 #include <xrpl/ledger/helpers/MPTokenHelpers.h>
 #include <xrpl/ledger/helpers/RippleStateHelpers.h>
@@ -23,8 +25,52 @@
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/UintTypes.h>
 
+#include <expected>
+
 namespace xrpl {
 
+// Deduct the transfer fee (if any) from an escrowed amount, returning the net
+// amount delivered to the receiver. The transfer fee applies only when neither
+// the sender nor the receiver is the issuer and the locked rate differs from
+// parity.
+//
+// NOTE: Transfer fee in escrow works a bit differently from a normal payment.
+// In escrow, the fee is deducted from the locked/sending amount, whereas in a
+// normal payment, the transfer fee is taken on top of the sending amount.
+//
+// The net delivered amount is rounded down (fee rounded up in the issuer's
+// favor). Rounding the net up (the legacy behavior) rounds the fee down, which
+// for integral assets (e.g. MPT) collapses the fee to zero for small amounts
+// and bypasses the transfer fee. Note that the legacy divideRound rounds to
+// nearest, so the strict variant is used to floor the net. If the net rounds
+// to zero the entire amount would be consumed by the fee, so tecPRECISION_LOSS
+// is returned rather than silently delivering nothing to the receiver.
+[[nodiscard]] inline std::expected<STAmount, TER>
+escrowDeductTransferFee(
+    ReadView const& view,
+    Rate const& lockedRate,
+    STAmount const& amount,
+    bool senderIssuer,
+    bool receiverIssuer)
+{
+    if ((senderIssuer || receiverIssuer) || lockedRate == kParityRate)
+        return amount;
+
+    bool const strictRounding = view.rules().enabled(fixCleanup3_4_0);
+    auto const netAmt = strictRounding
+        ? divideRoundStrict(amount, lockedRate, amount.asset(), false)
+        : divideRound(amount, lockedRate, amount.asset(), true);
+    // compute transfer fee, if any
+    auto const xferFee = amount.value() - netAmt;
+    // compute balance to transfer
+    STAmount finalAmt = amount.value() - xferFee;
+
+    if (strictRounding && finalAmt <= beast::kZero)
+        return std::unexpected(tecPRECISION_LOSS);
+
+    return finalAmt;
+}
+
 template <ValidIssueType T>
 TER
 escrowUnlockApplyHelper(
@@ -124,24 +170,12 @@ escrowUnlockApplyHelper<Issue>(
     if (xferRate < lockedRate)
         lockedRate = xferRate;
 
-    // Transfer Rate only applies when:
-    // 1. Issuer is not involved in the transfer (senderIssuer or
-    // receiverIssuer)
-    // 2. The locked rate is different from the parity rate
-
-    // NOTE: Transfer fee in escrow works a bit differently from a normal
-    // payment. In escrow, the fee is deducted from the locked/sending amount,
-    // whereas in a normal payment, the transfer fee is taken on top of the
-    // sending amount.
-    auto finalAmt = amount;
-    if ((!senderIssuer && !receiverIssuer) && lockedRate != kParityRate)
-    {
-        // compute transfer fee, if any
-        auto const xferFee =
-            amount.value() - divideRound(amount, lockedRate, amount.get<Issue>(), true);
-        // compute balance to transfer
-        finalAmt = amount.value() - xferFee;
-    }
+    // Deduct the transfer fee (if any) from the escrowed amount.
+    auto const netResult =
+        escrowDeductTransferFee(ctx.view, lockedRate, amount, senderIssuer, receiverIssuer);
+    if (!netResult)
+        return netResult.error();
+    auto const finalAmt = *netResult;
 
     // validate the line limit if the account submitting txn is not the receiver
     // of the funds
@@ -229,23 +263,13 @@ escrowUnlockApplyHelper<MPTIssue>(
     if (xferRate < lockedRate)
         lockedRate = xferRate;
 
-    // Transfer Rate only applies when:
-    // 1. Issuer is not involved in the transfer (senderIssuer or
-    // receiverIssuer)
-    // 2. The locked rate is different from the parity rate
-
-    // NOTE: Transfer fee in escrow works a bit differently from a normal
-    // payment. In escrow, the fee is deducted from the locked/sending amount,
-    // whereas in a normal payment, the transfer fee is taken on top of the
-    // sending amount.
-    auto finalAmt = amount;
-    if ((!senderIssuer && !receiverIssuer) && lockedRate != kParityRate)
-    {
-        // compute transfer fee, if any
-        auto const xferFee = amount.value() - divideRound(amount, lockedRate, amount.asset(), true);
-        // compute balance to transfer
-        finalAmt = amount.value() - xferFee;
-    }
+    // Deduct the transfer fee (if any) from the escrowed amount.
+    auto const netResult =
+        escrowDeductTransferFee(ctx.view, lockedRate, amount, senderIssuer, receiverIssuer);
+    if (!netResult)
+        return netResult.error();
+    auto const finalAmt = *netResult;
+
     return unlockEscrowMPT(
         ctx.view,
         sender,

include/xrpl/protocol/Rate.h

@@ -65,6 +65,9 @@ divideRound(STAmount const& amount, Rate const& rate, bool roundUp);
 STAmount
 divideRound(STAmount const& amount, Rate const& rate, Asset const& asset, bool roundUp);
 
+STAmount
+divideRoundStrict(STAmount const& amount, Rate const& rate, Asset const& asset, bool roundUp);
+
 namespace nft {
 /**
  * Given a transfer fee (in basis points) convert it to a transfer rate.

include/xrpl/protocol/detail/features.macro

@@ -15,6 +15,7 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
+XRPL_FIX    (Cleanup3_4_0,                Supported::Yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(Sponsor,                     Supported::Yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(BatchV1_1,                   Supported::Yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(LendingProtocolV1_1,         Supported::No,  VoteBehavior::DefaultNo)

src/libxrpl/protocol/Rate2.cpp

@@ -98,4 +98,15 @@ divideRound(STAmount const& amount, Rate const& rate, Asset const& asset, bool r
     return divRound(amount, detail::asAmount(rate), asset, roundUp);
 }
 
+STAmount
+divideRoundStrict(STAmount const& amount, Rate const& rate, Asset const& asset, bool roundUp)
+{
+    XRPL_ASSERT(rate.value, "xrpl::nft::divideRoundStrict(Issue) : nonzero rate input");
+
+    if (rate == kParityRate)
+        return amount;
+
+    return divRoundStrict(amount, detail::asAmount(rate), asset, roundUp);
+}
+
 }  // namespace xrpl