From 66f195eeaa8b0448526df6dd2bf2863ca29a80e5 Mon Sep 17 00:00:00 2001 From: Edward Hennis Date: Wed, 27 Feb 2019 11:56:02 -0500 Subject: [PATCH 1/2] Prevent divide by zero in mulDiv: * Every call to mulDiv either uses a literal or constexpr, checks for 0, or throws on overflow. --- src/libxrpl/basics/mulDiv.cpp | 3 ++ src/test/app/TxQ_test.cpp | 90 ++++++++++++++++++++++++++++++- src/xrpld/app/misc/detail/TxQ.cpp | 4 +- 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/src/libxrpl/basics/mulDiv.cpp b/src/libxrpl/basics/mulDiv.cpp index 993cff7eb63..061c4a77b65 100644 --- a/src/libxrpl/basics/mulDiv.cpp +++ b/src/libxrpl/basics/mulDiv.cpp @@ -12,7 +12,10 @@ namespace ripple { std::optional mulDiv(std::uint64_t value, std::uint64_t mul, std::uint64_t div) { + XRPL_ASSERT(div != 0, "ripple::mulDiv : non-zero divisor); + boost::multiprecision::uint128_t result; + result = multiply(result, value, mul); result /= div; diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index d176744c5cb..6ac3c4204f1 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -4142,9 +4142,97 @@ class TxQPosNegFlows_test : public beast::unit_test::suite txCount -= 4; checkMetrics(*this, env, txCount, 54, 4, 3); - // From 28 expected back down to 3 in 3 "slow" ledgers. + // With 100% decrease percent, went from 28 + // expected back down to 3 in 1 "slow" ledger. BEAST_EXPECT(!txCount); + + // Ensure it stays stable + env.close(env.now() + 5s, 10000ms); + checkMetrics(__LINE__, env, txCount, 54, 0, 3, 256); + env.close(env.now() + 5s, 10000ms); + checkMetrics(__LINE__, env, txCount, 54, 0, 3, 256); + } + + { + // Use a mostly default config + Env env( + *this, makeConfig({{"minimum_txn_in_ledger_standalone", "3"}})); + auto alice = Account("alice"); + + checkMetrics(__LINE__, env, 0, std::nullopt, 0, 3, 256); + env.fund(XRP(50000000), alice); + + fillQueue(env, alice); + checkMetrics(__LINE__, env, 0, std::nullopt, 4, 3, 256); + + // Verify that the numbers stay steady by creating + // a relatively large open ledger, then doing a slow + // close + for (int i = 0; i < 50; ++i) + { + env(noop(alice), openLedgerFee(env)); + } + checkMetrics(__LINE__, env, 0, std::nullopt, 54, 3, 256); + // Close the ledger with a delay. + env.close(env.now() + 5s, 10000ms); + + // expectedPerLedger should never go below 3 + checkMetrics(__LINE__, env, 0, std::nullopt, 0, 3, 256, 9991129); + + // Do it again + for (int i = 0; i < 50; ++i) + { + env(noop(alice), openLedgerFee(env)); + } + checkMetrics(__LINE__, env, 0, std::nullopt, 50, 3, 256, 9991129); + + // Close the ledger with a delay. + env.close(env.now() + 5s, 10000ms); + + // expectedPerLedger should never go below 3 + checkMetrics(__LINE__, env, 0, std::nullopt, 0, 3, 256, 666630336); + + // we don't crash anymore + env(noop(alice)); + } + + { + // Use a "bad" config - minimum of 0. Treated as 1 for + // escalation calculations + Env env( + *this, makeConfig({{"minimum_txn_in_ledger_standalone", "0"}})); + + checkMetrics(__LINE__, env, 0, std::nullopt, 0, 0, 256); + // we don't crash anymore + env(noop(env.master)); + + { + // The open ledger fee is 256 - as if min was 1. + auto const& view = *env.current(); + auto const metrics = env.app().getTxQ().getMetrics(view); + BEAST_EXPECT(metrics.openLedgerFeeLevel == 256); + auto const openLedgerDrops = mulDiv( + metrics.openLedgerFeeLevel, + view.fees().base, + metrics.referenceFeeLevel); + BEAST_EXPECT(openLedgerDrops && *openLedgerDrops + 1 == 11); + } + + // another transaction will trigger escalation + env(noop(env.master)); + + { + // But the open ledger fee does grow + auto const& view = *env.current(); + auto metrics = env.app().getTxQ().getMetrics(view); + BEAST_EXPECT(metrics.openLedgerFeeLevel == 512000); + auto const openLedgerDrops = mulDiv( + metrics.openLedgerFeeLevel, + view.fees().base, + metrics.referenceFeeLevel); + BEAST_EXPECT(openLedgerDrops && *openLedgerDrops == 20000); + } } } diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index fd975f9178d..0b50c138317 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -156,7 +156,7 @@ TxQ::FeeMetrics::scaleFeeLevel(Snapshot const& snapshot, OpenView const& view) // Transactions in the open ledger so far auto const current = view.txCount(); - auto const target = snapshot.txnsExpected; + auto const target = snapshot.txnsExpected == 0 ? 1 : snapshot.txnsExpected; auto const multiplier = snapshot.escalationMultiplier; // Once the open ledger bypasses the target, @@ -228,7 +228,7 @@ TxQ::FeeMetrics::escalatedSeriesFeeLevel( */ auto const last = current + seriesSize - 1; - auto const target = snapshot.txnsExpected; + auto const target = snapshot.txnsExpected == 0 ? 1 : snapshot.txnsExpected; auto const multiplier = snapshot.escalationMultiplier; XRPL_ASSERT( From f35c81b06918b01ca3756f112928f13335aaca8f Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 28 Jan 2026 20:23:46 -0500 Subject: [PATCH 2/2] Fix formatting --- src/test/app/TxQ_test.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index d37fcba30d3..c44b7b7e130 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -3710,8 +3710,7 @@ class TxQPosNegFlows_test : public beast::unit_test::suite { // Use a mostly default config - Env env( - *this, makeConfig({{"minimum_txn_in_ledger_standalone", "3"}})); + Env env(*this, makeConfig({{"minimum_txn_in_ledger_standalone", "3"}})); auto alice = Account("alice"); checkMetrics(__LINE__, env, 0, std::nullopt, 0, 3, 256); @@ -3754,8 +3753,7 @@ class TxQPosNegFlows_test : public beast::unit_test::suite { // Use a "bad" config - minimum of 0. Treated as 1 for // escalation calculations - Env env( - *this, makeConfig({{"minimum_txn_in_ledger_standalone", "0"}})); + Env env(*this, makeConfig({{"minimum_txn_in_ledger_standalone", "0"}})); checkMetrics(__LINE__, env, 0, std::nullopt, 0, 0, 256); // we don't crash anymore @@ -3766,10 +3764,8 @@ class TxQPosNegFlows_test : public beast::unit_test::suite auto const& view = *env.current(); auto const metrics = env.app().getTxQ().getMetrics(view); BEAST_EXPECT(metrics.openLedgerFeeLevel == 256); - auto const openLedgerDrops = mulDiv( - metrics.openLedgerFeeLevel, - view.fees().base, - metrics.referenceFeeLevel); + auto const openLedgerDrops = + mulDiv(metrics.openLedgerFeeLevel, view.fees().base, metrics.referenceFeeLevel); BEAST_EXPECT(openLedgerDrops && *openLedgerDrops + 1 == 11); } @@ -3781,10 +3777,8 @@ class TxQPosNegFlows_test : public beast::unit_test::suite auto const& view = *env.current(); auto metrics = env.app().getTxQ().getMetrics(view); BEAST_EXPECT(metrics.openLedgerFeeLevel == 512000); - auto const openLedgerDrops = mulDiv( - metrics.openLedgerFeeLevel, - view.fees().base, - metrics.referenceFeeLevel); + auto const openLedgerDrops = + mulDiv(metrics.openLedgerFeeLevel, view.fees().base, metrics.referenceFeeLevel); BEAST_EXPECT(openLedgerDrops && *openLedgerDrops == 20000); } }