From 2432f13903a3bf38fc41b009de34966a385267a8 Mon Sep 17 00:00:00 2001 From: Edward Hennis Date: Wed, 28 Nov 2018 18:50:02 -0500 Subject: [PATCH] Reserve correct vector size for fee calculations: * Using txnsExpected_, which is influenced by both the config and network behavior, can reserve far too much or far too little memory, wasting time and resources. * Not an issue during normal operation, but a user could cause problems on their local node with extreme configuration settings. --- src/ripple/app/misc/impl/TxQ.cpp | 21 +++++++++++++-------- src/test/app/Regression_test.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/ripple/app/misc/impl/TxQ.cpp b/src/ripple/app/misc/impl/TxQ.cpp index 5394cc98f..4ee348b06 100644 --- a/src/ripple/app/misc/impl/TxQ.cpp +++ b/src/ripple/app/misc/impl/TxQ.cpp @@ -83,15 +83,20 @@ TxQ::FeeMetrics::update(Application& app, TxQ::Setup const& setup) { std::vector feeLevels; - feeLevels.reserve(txnsExpected_); - for (auto const& tx : view.txs) - { - auto const baseFee = calculateBaseFee(view, *tx.first); - feeLevels.push_back(getFeeLevelPaid(*tx.first, - baseLevel, baseFee, setup)); - } + auto const txBegin = view.txs.begin(); + auto const txEnd = view.txs.end(); + auto const size = std::distance(txBegin, txEnd); + feeLevels.reserve(size); + std::for_each(txBegin, txEnd, + [&](auto const& tx) + { + auto const baseFee = calculateBaseFee(view, *tx.first); + feeLevels.push_back(getFeeLevelPaid(*tx.first, + baseLevel, baseFee, setup)); + } + ); std::sort(feeLevels.begin(), feeLevels.end()); - auto const size = feeLevels.size(); + assert(size == feeLevels.size()); JLOG(j_.debug()) << "Ledger " << view.info().seq << " has " << size << " transactions. " << diff --git a/src/test/app/Regression_test.cpp b/src/test/app/Regression_test.cpp index aaacf2f35..eb1ef64e7 100644 --- a/src/test/app/Regression_test.cpp +++ b/src/test/app/Regression_test.cpp @@ -198,6 +198,32 @@ struct Regression_test : public beast::unit_test::suite } } + void testFeeEscalationExtremeConfig() + { + testcase("Fee escalation shouldn't allocate extreme memory"); + using clock_type = std::chrono::steady_clock; + using namespace jtx; + using namespace std::chrono_literals; + + Env env(*this, envconfig([](std::unique_ptr cfg) + { + auto& s = cfg->section("transaction_queue"); + s.set("minimum_txn_in_ledger_standalone", "4294967295"); + s.set("minimum_txn_in_ledger", "4294967295"); + s.set("target_txn_in_ledger", "4294967295"); + s.set("normal_consensus_increase_percent", "4294967295"); + + return cfg; + })); + + env(noop(env.master)); + // This test will probably fail if any breakpoints are encountered, + // but should pass on even the slowest machines. + auto const start = clock_type::now(); + env.close(); + BEAST_EXPECT(clock_type::now() - start < 1s); + } + void testJsonInvalid() { using namespace jtx; @@ -221,6 +247,7 @@ struct Regression_test : public beast::unit_test::suite testLowBalanceDestroy(); testSecp256r1key(); testFeeEscalationAutofill(); + testFeeEscalationExtremeConfig(); testJsonInvalid(); } };