From dee093876066fa3e3b9939c9001aef02792d70ac Mon Sep 17 00:00:00 2001 From: Zhiyuan Wang <1830604455@qq.com> Date: Tue, 14 Jul 2026 15:33:19 -0400 Subject: [PATCH 1/6] fix mpt trans fee small amount escrow rounding --- include/xrpl/ledger/helpers/EscrowHelpers.h | 14 +++- include/xrpl/protocol/detail/features.macro | 1 + src/test/app/EscrowToken_test.cpp | 72 +++++++++++++++++++++ 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/include/xrpl/ledger/helpers/EscrowHelpers.h b/include/xrpl/ledger/helpers/EscrowHelpers.h index 9f54e537699..843930939cc 100644 --- a/include/xrpl/ledger/helpers/EscrowHelpers.h +++ b/include/xrpl/ledger/helpers/EscrowHelpers.h @@ -136,9 +136,13 @@ escrowUnlockApplyHelper( auto finalAmt = amount; if ((!senderIssuer && !receiverIssuer) && lockedRate != kParityRate) { + // Round the net delivered amount down so the transfer fee is rounded + // up in favor of the issuer. Rounding the net up (the legacy behavior) + // rounds the fee down, which collapses to zero for small amounts. + bool const roundUp = !ctx.view.rules().enabled(fixCleanup3_4_0); // compute transfer fee, if any auto const xferFee = - amount.value() - divideRound(amount, lockedRate, amount.get(), true); + amount.value() - divideRound(amount, lockedRate, amount.get(), roundUp); // compute balance to transfer finalAmt = amount.value() - xferFee; } @@ -241,8 +245,14 @@ escrowUnlockApplyHelper( auto finalAmt = amount; if ((!senderIssuer && !receiverIssuer) && lockedRate != kParityRate) { + // Round the net delivered amount down so the transfer fee is rounded + // up in favor of the issuer. Rounding the net up (the legacy behavior) + // rounds the fee down, which for integral MPT amounts collapses the + // fee to zero for small amounts and bypasses the transfer fee. + bool const roundUp = !ctx.view.rules().enabled(fixCleanup3_4_0); // compute transfer fee, if any - auto const xferFee = amount.value() - divideRound(amount, lockedRate, amount.asset(), true); + auto const xferFee = + amount.value() - divideRound(amount, lockedRate, amount.asset(), roundUp); // compute balance to transfer finalAmt = amount.value() - xferFee; } diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index bfe03a63037..4f1fac82da9 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/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) diff --git a/src/test/app/EscrowToken_test.cpp b/src/test/app/EscrowToken_test.cpp index 4015f5ddc8a..3ac22a0580a 100644 --- a/src/test/app/EscrowToken_test.cpp +++ b/src/test/app/EscrowToken_test.cpp @@ -3723,6 +3723,76 @@ struct EscrowToken_test : public beast::unit_test::Suite env.close(); } + void + testMPTTransferFeeRoundingBypass(FeatureBitset features) + { + testcase("MPT Transfer Fee Rounding Bypass"); + using namespace test::jtx; + using namespace std::literals; + + bool const withV1 = features[fixTokenEscrowV1]; + bool const withV2 = features[fixCleanup3_4_0]; + + // Escrow a small MPT amount with a non-zero transfer fee, then finish + // it. Before fixCleanup3_4_0, the net delivered amount is rounded up + // which rounds the fee down to zero for small amounts, letting a sender + // deliver the full amount to the receiver while the issuer collects no + // fee. With the fix the fee is rounded up in the issuer's favor. + auto testChunk = [&](std::uint16_t transferFee, + std::uint64_t amount, + std::uint64_t feeWithoutV2, + std::uint64_t feeWithV2) { + Env env{*this, features}; + auto const baseFee = env.current()->fees().base; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const gw = Account("gw"); + + MPTTester mptGw(env, gw, {.holders = {alice, bob}}); + mptGw.create( + {.transferFee = transferFee, + .ownerCount = 1, + .holderCount = 0, + .flags = tfMPTCanEscrow | tfMPTCanTransfer}); + mptGw.authorize({.account = alice}); + mptGw.authorize({.account = bob}); + auto const mpt = mptGw["MPT"]; + env(pay(gw, alice, mpt(10'000))); + env(pay(gw, bob, mpt(10'000))); + env.close(); + + auto const preAlice = env.balance(alice, mpt); + auto const preBob = env.balance(bob, mpt); + auto const preOutstanding = env.balance(gw, mpt); + + auto const seq1 = env.seq(alice); + env(escrow::create(alice, bob, mpt(amount)), + escrow::kFinishTime(env.now() + 1s), + Fee(baseFee * 150)); + env.close(); + + env(escrow::finish(bob, alice, seq1), Fee(baseFee * 150)); + env.close(); + + std::uint64_t const fee = withV2 ? feeWithV2 : feeWithoutV2; + std::uint64_t const net = amount - fee; + // The fee is only burned from OutstandingAmount when the gross vs. + // net accounting fix (fixTokenEscrowV1) is active. + std::uint64_t const burn = withV1 ? fee : 0; + + BEAST_EXPECT(env.balance(alice, mpt) == preAlice - mpt(amount)); + BEAST_EXPECT(env.balance(bob, mpt) == preBob + mpt(net)); + BEAST_EXPECT(env.balance(gw, mpt) == preOutstanding + mpt(burn)); + }; + + // 25% fee, MPT(4): bug delivers 4 with zero fee; fix charges fee 1. + testChunk(25000, 4, 0, 1); + // 1% fee, MPT(90): max zero-fee chunk under the bug; fix charges fee 1. + testChunk(1000, 90, 0, 1); + // 1% fee, MPT(91): boundary just above the window; fee 1 either way. + testChunk(1000, 91, 1, 1); + } + void testMPTLock(FeatureBitset features) { @@ -3978,6 +4048,7 @@ struct EscrowToken_test : public beast::unit_test::Suite testMPTMetaAndOwnership(features); testMPTGateway(features); testMPTLockedRate(features); + testMPTTransferFeeRoundingBypass(features); testMPTRequireAuth(features); testMPTLock(features); testMPTCanTransfer(features); @@ -3997,6 +4068,7 @@ struct EscrowToken_test : public beast::unit_test::Suite testIOUWithFeats(feats - fixCleanup3_2_0); testMPTWithFeats(feats); testMPTWithFeats(feats - fixTokenEscrowV1); + testMPTTransferFeeRoundingBypass(feats - fixCleanup3_4_0); } } }; From 812425101886374077e945b0cc38b980f5dae1bc Mon Sep 17 00:00:00 2001 From: Zhiyuan Wang <1830604455@qq.com> Date: Wed, 15 Jul 2026 13:14:31 -0400 Subject: [PATCH 2/6] fix --- include/xrpl/ledger/helpers/EscrowHelpers.h | 42 +++++++++----- include/xrpl/protocol/Rate.h | 3 + src/libxrpl/protocol/Rate2.cpp | 11 ++++ src/test/app/EscrowToken_test.cpp | 62 +++++++++++++++++++++ 4 files changed, 105 insertions(+), 13 deletions(-) diff --git a/include/xrpl/ledger/helpers/EscrowHelpers.h b/include/xrpl/ledger/helpers/EscrowHelpers.h index 843930939cc..3471c8dc776 100644 --- a/include/xrpl/ledger/helpers/EscrowHelpers.h +++ b/include/xrpl/ledger/helpers/EscrowHelpers.h @@ -136,15 +136,23 @@ escrowUnlockApplyHelper( auto finalAmt = amount; if ((!senderIssuer && !receiverIssuer) && lockedRate != kParityRate) { - // Round the net delivered amount down so the transfer fee is rounded - // up in favor of the issuer. Rounding the net up (the legacy behavior) - // rounds the fee down, which collapses to zero for small amounts. - bool const roundUp = !ctx.view.rules().enabled(fixCleanup3_4_0); + // Round the net delivered amount down (fee rounded up in the issuer's + // favor). Rounding the net up (the legacy behavior) rounds the fee + // down, which collapses to zero for small amounts. Note that the + // legacy divideRound rounds to nearest, so the strict variant is used + // to floor the net. + bool const v2 = ctx.view.rules().enabled(fixCleanup3_4_0); + auto const netAmt = v2 ? divideRoundStrict(amount, lockedRate, amount.get(), false) + : divideRound(amount, lockedRate, amount.get(), true); // compute transfer fee, if any - auto const xferFee = - amount.value() - divideRound(amount, lockedRate, amount.get(), roundUp); + auto const xferFee = amount.value() - netAmt; // compute balance to transfer finalAmt = amount.value() - xferFee; + + // If the fee would consume the entire amount, the net delivered rounds + // to zero. Fail rather than silently delivering nothing to the receiver. + if (v2 && finalAmt <= beast::kZero) + return tecPRECISION_LOSS; } // validate the line limit if the account submitting txn is not the receiver @@ -245,16 +253,24 @@ escrowUnlockApplyHelper( auto finalAmt = amount; if ((!senderIssuer && !receiverIssuer) && lockedRate != kParityRate) { - // Round the net delivered amount down so the transfer fee is rounded - // up in favor of the issuer. Rounding the net up (the legacy behavior) - // rounds the fee down, which for integral MPT amounts collapses the - // fee to zero for small amounts and bypasses the transfer fee. - bool const roundUp = !ctx.view.rules().enabled(fixCleanup3_4_0); + // Round the net delivered amount down (fee rounded up in the issuer's + // favor). Rounding the net up (the legacy behavior) rounds the fee + // down, which for integral MPT amounts 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. + bool const v2 = ctx.view.rules().enabled(fixCleanup3_4_0); + auto const netAmt = v2 ? divideRoundStrict(amount, lockedRate, amount.asset(), false) + : divideRound(amount, lockedRate, amount.asset(), true); // compute transfer fee, if any - auto const xferFee = - amount.value() - divideRound(amount, lockedRate, amount.asset(), roundUp); + auto const xferFee = amount.value() - netAmt; // compute balance to transfer finalAmt = amount.value() - xferFee; + + // If the fee would consume the entire amount, the net delivered rounds + // to zero. Fail rather than silently delivering nothing to the receiver. + if (v2 && finalAmt <= beast::kZero) + return tecPRECISION_LOSS; } return unlockEscrowMPT( ctx.view, diff --git a/include/xrpl/protocol/Rate.h b/include/xrpl/protocol/Rate.h index 048787cab55..32290fdc7ee 100644 --- a/include/xrpl/protocol/Rate.h +++ b/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. diff --git a/src/libxrpl/protocol/Rate2.cpp b/src/libxrpl/protocol/Rate2.cpp index fd56f83bf5c..b5354f646ed 100644 --- a/src/libxrpl/protocol/Rate2.cpp +++ b/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 diff --git a/src/test/app/EscrowToken_test.cpp b/src/test/app/EscrowToken_test.cpp index 3ac22a0580a..afc0e06cc2f 100644 --- a/src/test/app/EscrowToken_test.cpp +++ b/src/test/app/EscrowToken_test.cpp @@ -3785,12 +3785,74 @@ struct EscrowToken_test : public beast::unit_test::Suite BEAST_EXPECT(env.balance(gw, mpt) == preOutstanding + mpt(burn)); }; + // 25% fee, MPT(2): net 2/1.25 = 1.6. The bug (round up) delivers 2 with + // zero fee; rounding to nearest would also deliver 2 (fee 0); only + // flooring the net charges a fee (net 1, fee 1). + testChunk(25000, 2, 0, 1); // 25% fee, MPT(4): bug delivers 4 with zero fee; fix charges fee 1. testChunk(25000, 4, 0, 1); // 1% fee, MPT(90): max zero-fee chunk under the bug; fix charges fee 1. testChunk(1000, 90, 0, 1); // 1% fee, MPT(91): boundary just above the window; fee 1 either way. testChunk(1000, 91, 1, 1); + + // When the escrowed amount is so small that the net delivered rounds to + // zero (the entire amount would be consumed by the fee), the legacy + // behavior silently delivers the full amount to the receiver with no + // fee. With fixCleanup3_4_0 the finish fails with tecPRECISION_LOSS + // instead of delivering nothing to the receiver. + { + std::uint16_t const transferFee = 25000; // 25% + std::uint64_t const amount = 1; + + Env env{*this, features}; + auto const baseFee = env.current()->fees().base; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const gw = Account("gw"); + + MPTTester mptGw(env, gw, {.holders = {alice, bob}}); + mptGw.create( + {.transferFee = transferFee, + .ownerCount = 1, + .holderCount = 0, + .flags = tfMPTCanEscrow | tfMPTCanTransfer}); + mptGw.authorize({.account = alice}); + mptGw.authorize({.account = bob}); + auto const mpt = mptGw["MPT"]; + env(pay(gw, alice, mpt(10'000))); + env(pay(gw, bob, mpt(10'000))); + env.close(); + + auto const preAlice = env.balance(alice, mpt); + auto const preBob = env.balance(bob, mpt); + + auto const seq1 = env.seq(alice); + env(escrow::create(alice, bob, mpt(amount)), + escrow::kFinishTime(env.now() + 1s), + Fee(baseFee * 150)); + env.close(); + + if (withV2) + { + // The net delivered rounds to zero; the finish is rejected and + // the escrowed amount remains locked (alice down by amount, + // bob unchanged). + env(escrow::finish(bob, alice, seq1), Fee(baseFee * 150), Ter(tecPRECISION_LOSS)); + env.close(); + BEAST_EXPECT(env.balance(alice, mpt) == preAlice - mpt(amount)); + BEAST_EXPECT(env.balance(bob, mpt) == preBob); + } + else + { + // Legacy behavior: the net rounds up to the full amount with a + // zero fee, so bob receives everything. + env(escrow::finish(bob, alice, seq1), Fee(baseFee * 150)); + env.close(); + BEAST_EXPECT(env.balance(alice, mpt) == preAlice - mpt(amount)); + BEAST_EXPECT(env.balance(bob, mpt) == preBob + mpt(amount)); + } + } } void From 30a5272d7624446fdd8632ffab770ff13da1a786 Mon Sep 17 00:00:00 2001 From: Zhiyuan Wang <1830604455@qq.com> Date: Wed, 15 Jul 2026 13:57:05 -0400 Subject: [PATCH 3/6] fix --- include/xrpl/ledger/helpers/EscrowHelpers.h | 117 ++++++++++---------- src/test/protocol/STAmount_test.cpp | 64 +++++++++++ 2 files changed, 120 insertions(+), 61 deletions(-) diff --git a/include/xrpl/ledger/helpers/EscrowHelpers.h b/include/xrpl/ledger/helpers/EscrowHelpers.h index 3471c8dc776..dc73878c4c7 100644 --- a/include/xrpl/ledger/helpers/EscrowHelpers.h +++ b/include/xrpl/ledger/helpers/EscrowHelpers.h @@ -23,8 +23,51 @@ #include #include +#include + 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 +escrowDeductTransferFee( + ReadView const& view, + Rate const& lockedRate, + STAmount const& amount, + bool senderIssuer, + bool receiverIssuer) +{ + if ((senderIssuer || receiverIssuer) || lockedRate == kParityRate) + return amount; + + bool const v2 = view.rules().enabled(fixCleanup3_4_0); + auto const netAmt = v2 ? 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 (v2 && finalAmt <= beast::kZero) + return std::unexpected(tecPRECISION_LOSS); + + return finalAmt; +} + template TER escrowUnlockApplyHelper( @@ -124,36 +167,12 @@ escrowUnlockApplyHelper( 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) - { - // Round the net delivered amount down (fee rounded up in the issuer's - // favor). Rounding the net up (the legacy behavior) rounds the fee - // down, which collapses to zero for small amounts. Note that the - // legacy divideRound rounds to nearest, so the strict variant is used - // to floor the net. - bool const v2 = ctx.view.rules().enabled(fixCleanup3_4_0); - auto const netAmt = v2 ? divideRoundStrict(amount, lockedRate, amount.get(), false) - : divideRound(amount, lockedRate, amount.get(), true); - // compute transfer fee, if any - auto const xferFee = amount.value() - netAmt; - // compute balance to transfer - finalAmt = amount.value() - xferFee; - - // If the fee would consume the entire amount, the net delivered rounds - // to zero. Fail rather than silently delivering nothing to the receiver. - if (v2 && finalAmt <= beast::kZero) - return tecPRECISION_LOSS; - } + // 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 @@ -241,37 +260,13 @@ escrowUnlockApplyHelper( 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) - { - // Round the net delivered amount down (fee rounded up in the issuer's - // favor). Rounding the net up (the legacy behavior) rounds the fee - // down, which for integral MPT amounts 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. - bool const v2 = ctx.view.rules().enabled(fixCleanup3_4_0); - auto const netAmt = v2 ? 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 - finalAmt = amount.value() - xferFee; - - // If the fee would consume the entire amount, the net delivered rounds - // to zero. Fail rather than silently delivering nothing to the receiver. - if (v2 && finalAmt <= beast::kZero) - return tecPRECISION_LOSS; - } + // 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, diff --git a/src/test/protocol/STAmount_test.cpp b/src/test/protocol/STAmount_test.cpp index f6c5a947524..b085b641e4f 100644 --- a/src/test/protocol/STAmount_test.cpp +++ b/src/test/protocol/STAmount_test.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -1252,6 +1253,68 @@ class STAmount_test : public beast::unit_test::Suite //-------------------------------------------------------------------------- + void + testDivideRoundStrict() + { + testcase("divideRoundStrict"); + + // Transfer-fee rates: 1.25 == 25% fee, 1.01 == 1% fee. + Rate const rate25{1'250'000'000}; + Rate const rate1{1'010'000'000}; + + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; + + // Parity rate is a no-op for both variants. + { + STAmount const amt{mpt, 8}; + BEAST_EXPECT(divideRound(amt, kParityRate, amt.asset(), true) == amt); + BEAST_EXPECT(divideRoundStrict(amt, kParityRate, amt.asset(), false) == amt); + } + + // For integral (MPT) amounts, the non-strict divideRound rounds the + // quotient to nearest, while divideRoundStrict floors it (toward zero) + // when roundUp is false. The escrow transfer-fee fix relies on this so + // the fee cannot collapse to zero for small amounts. + auto mptRound = [&](std::uint64_t v, Rate const& r, bool roundUp) { + STAmount const a{mpt, v}; + return divideRound(a, r, a.asset(), roundUp).mpt().value(); + }; + auto mptStrict = [&](std::uint64_t v, Rate const& r, bool roundUp) { + STAmount const a{mpt, v}; + return divideRoundStrict(a, r, a.asset(), roundUp).mpt().value(); + }; + + // 25% fee (rate 1.25): + // 1 / 1.25 = 0.8 -> nearest 1, floor 0 + // 2 / 1.25 = 1.6 -> nearest 2, floor 1 + // 4 / 1.25 = 3.2 -> nearest 3, floor 3 + // 6 / 1.25 = 4.8 -> nearest 5, floor 4 + BEAST_EXPECT(mptRound(1, rate25, false) == 1); + BEAST_EXPECT(mptStrict(1, rate25, false) == 0); + BEAST_EXPECT(mptRound(2, rate25, false) == 2); + BEAST_EXPECT(mptStrict(2, rate25, false) == 1); + BEAST_EXPECT(mptRound(4, rate25, false) == 3); + BEAST_EXPECT(mptStrict(4, rate25, false) == 3); + BEAST_EXPECT(mptRound(6, rate25, false) == 5); + BEAST_EXPECT(mptStrict(6, rate25, false) == 4); + + // roundUp=true rounds the quotient up: 3.2 -> 4. + BEAST_EXPECT(mptStrict(4, rate25, true) == 4); + + // 1% fee (rate 1.01): floor of a value just above an integer. + // 90 / 1.01 = 89.10 -> floor 89 + // 91 / 1.01 = 90.09 -> floor 90 + BEAST_EXPECT(mptStrict(90, rate1, false) == 89); + BEAST_EXPECT(mptStrict(91, rate1, false) == 90); + + // IOU amounts: an exact division is unaffected by the rounding mode. + { + STAmount const iou{noIssue(), 4}; + STAmount const expected{noIssue(), 32, -1}; // 3.2 + BEAST_EXPECT(divideRoundStrict(iou, rate25, iou.asset(), false) == expected); + } + } + void run() override { @@ -1261,6 +1324,7 @@ class STAmount_test : public beast::unit_test::Suite testArithmetic(); testUnderflow(); testRounding(); + testDivideRoundStrict(); testParseJson(); testConvertXRP(); testConvertIOU(); From 131154ea0a1a30e5bea35f56333aa37bfeefb9f5 Mon Sep 17 00:00:00 2001 From: Zhiyuan Wang <1830604455@qq.com> Date: Wed, 15 Jul 2026 14:05:58 -0400 Subject: [PATCH 4/6] naming --- include/xrpl/ledger/helpers/EscrowHelpers.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/xrpl/ledger/helpers/EscrowHelpers.h b/include/xrpl/ledger/helpers/EscrowHelpers.h index dc73878c4c7..7e8ee5120a8 100644 --- a/include/xrpl/ledger/helpers/EscrowHelpers.h +++ b/include/xrpl/ledger/helpers/EscrowHelpers.h @@ -54,15 +54,16 @@ escrowDeductTransferFee( if ((senderIssuer || receiverIssuer) || lockedRate == kParityRate) return amount; - bool const v2 = view.rules().enabled(fixCleanup3_4_0); - auto const netAmt = v2 ? divideRoundStrict(amount, lockedRate, amount.asset(), false) - : divideRound(amount, lockedRate, amount.asset(), true); + 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 (v2 && finalAmt <= beast::kZero) + if (strictRounding && finalAmt <= beast::kZero) return std::unexpected(tecPRECISION_LOSS); return finalAmt; From d1cdda78a098d5a11c9a6b1adece3c4ab47b3a78 Mon Sep 17 00:00:00 2001 From: Zhiyuan Wang <1830604455@qq.com> Date: Fri, 17 Jul 2026 15:47:45 -0400 Subject: [PATCH 5/6] clang-tidy --- include/xrpl/ledger/helpers/EscrowHelpers.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/xrpl/ledger/helpers/EscrowHelpers.h b/include/xrpl/ledger/helpers/EscrowHelpers.h index 7e8ee5120a8..8e5050ef9f3 100644 --- a/include/xrpl/ledger/helpers/EscrowHelpers.h +++ b/include/xrpl/ledger/helpers/EscrowHelpers.h @@ -2,7 +2,9 @@ #include #include +#include #include +#include #include #include #include From f04f9c0e43acf66c4ce34f9c5305ea353be83785 Mon Sep 17 00:00:00 2001 From: Zhiyuan Wang <1830604455@qq.com> Date: Fri, 17 Jul 2026 16:15:11 -0400 Subject: [PATCH 6/6] test --- src/test/app/EscrowToken_test.cpp | 120 ++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/src/test/app/EscrowToken_test.cpp b/src/test/app/EscrowToken_test.cpp index afc0e06cc2f..07c231b3054 100644 --- a/src/test/app/EscrowToken_test.cpp +++ b/src/test/app/EscrowToken_test.cpp @@ -1760,6 +1760,67 @@ struct EscrowToken_test : public beast::unit_test::Suite } } + void + testIOUTransferFeePrecisionLoss(FeatureBitset features) + { + testcase("IOU Transfer Fee Precision Loss"); + using namespace test::jtx; + using namespace std::literals; + + bool const withV2 = features[fixCleanup3_4_0]; + + // Escrow the smallest positive IOU amount with a non-zero transfer + // rate, then finish it. The net delivered amount underflows to zero. + // The legacy behavior rounds the net up to the smallest positive + // value, delivering the full amount with no fee. With fixCleanup3_4_0 + // the finish fails with tecPRECISION_LOSS instead. + Env env{*this, features}; + auto const baseFee = env.current()->fees().base; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const gw = Account{"gateway"}; + auto const usd = gw["USD"]; + env.fund(XRP(10'000), alice, bob, gw); + env(fset(gw, asfAllowTrustLineLocking)); + env(rate(gw, 1.25)); + env.close(); + env.trust(usd(100), alice); + env.trust(usd(100), bob); + env.close(); + + // the smallest positive IOU amount + auto const tiny = usd(kEpsilon); + env(pay(gw, alice, tiny)); + env.close(); + + auto const preAlice = env.balance(alice, usd); + auto const preBob = env.balance(bob, usd); + auto const seq1 = env.seq(alice); + env(escrow::create(alice, bob, tiny), + escrow::kFinishTime(env.now() + 1s), + Fee(baseFee * 150)); + env.close(); + + if (withV2) + { + // The net delivered rounds to zero; the finish is rejected and + // the escrowed amount remains locked. + env(escrow::finish(bob, alice, seq1), Fee(baseFee * 150), Ter(tecPRECISION_LOSS)); + env.close(); + BEAST_EXPECT(env.balance(alice, usd) == preAlice - tiny); + BEAST_EXPECT(env.balance(bob, usd) == preBob); + } + else + { + // Legacy behavior: the net rounds up to the smallest positive + // value, so bob receives the full amount with no fee. + env(escrow::finish(bob, alice, seq1), Fee(baseFee * 150)); + env.close(); + BEAST_EXPECT(env.balance(alice, usd) == preAlice - tiny); + BEAST_EXPECT(env.balance(bob, usd) == preBob + tiny); + } + } + void testIOULimitAmount(FeatureBitset features) { @@ -3678,6 +3739,63 @@ struct EscrowToken_test : public beast::unit_test::Suite BEAST_EXPECT(issuerMPTEscrowed(env, mpt) == 0); BEAST_EXPECT(env.balance(gw, mpt) == mpt(-19'875)); } + + // test locked rate: issuer lowers the transfer fee after creation + if (features[featureDynamicMPT]) + { + Env env{*this, features}; + auto const baseFee = env.current()->fees().base; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const gw = Account("gw"); + + MPTTester mptGw(env, gw, {.holders = {alice, bob}}); + mptGw.create( + {.transferFee = 25000, + .ownerCount = 1, + .holderCount = 0, + .flags = tfMPTCanEscrow | tfMPTCanTransfer, + .mutableFlags = tmfMPTCanMutateTransferFee}); + mptGw.authorize({.account = alice}); + mptGw.authorize({.account = bob}); + auto const mpt = mptGw["MPT"]; + env(pay(gw, alice, mpt(10'000))); + env(pay(gw, bob, mpt(10'000))); + env.close(); + + auto const preAlice = env.balance(alice, mpt); + auto const preBob = env.balance(bob, mpt); + auto const preOutstanding = env.balance(gw, mpt); + + // alice can create escrow w/ 25% xfer rate locked in + auto const seq1 = env.seq(alice); + env(escrow::create(alice, bob, mpt(110)), + escrow::kCondition(escrow::kCb1), + escrow::kFinishTime(env.now() + 1s), + Fee(baseFee * 150)); + env.close(); + auto const transferRate = escrow::rate(env, alice, seq1); + BEAST_EXPECT(transferRate.value == std::uint32_t(1'000'000'000 * 1.25)); + + // issuer lowers the transfer fee to 10% + mptGw.set({.transferFee = 10000}); + env.close(); + + // bob can finish escrow - the lower rate applies: + // net 110 / 1.10 = 100, fee 10 + env(escrow::finish(bob, alice, seq1), + escrow::kCondition(escrow::kCb1), + escrow::kFulfillment(escrow::kFb1), + Fee(baseFee * 150)); + env.close(); + + // The fee is only burned from OutstandingAmount when the gross vs. + // net accounting fix (fixTokenEscrowV1) is active. + std::uint64_t const burn = features[fixTokenEscrowV1] ? 10 : 0; + BEAST_EXPECT(env.balance(alice, mpt) == preAlice - mpt(110)); + BEAST_EXPECT(env.balance(bob, mpt) == preBob + mpt(100)); + BEAST_EXPECT(env.balance(gw, mpt) == preOutstanding + mpt(burn)); + } } void @@ -4090,6 +4208,7 @@ struct EscrowToken_test : public beast::unit_test::Suite testIOURippleState(features); testIOUGateway(features); testIOULockedRate(features); + testIOUTransferFeePrecisionLoss(features); testIOULimitAmount(features); testIOURequireAuth(features); testIOUFreeze(features); @@ -4131,6 +4250,7 @@ struct EscrowToken_test : public beast::unit_test::Suite testMPTWithFeats(feats); testMPTWithFeats(feats - fixTokenEscrowV1); testMPTTransferFeeRoundingBypass(feats - fixCleanup3_4_0); + testIOUTransferFeePrecisionLoss(feats - fixCleanup3_4_0); } } };