diff --git a/src/ripple/app/misc/TxQ.h b/src/ripple/app/misc/TxQ.h index fc95a19a7b4..d58343af899 100644 --- a/src/ripple/app/misc/TxQ.h +++ b/src/ripple/app/misc/TxQ.h @@ -320,8 +320,7 @@ class TxQ FeeEscalation amendment is not enabled. */ boost::optional - getMetrics(OpenView const& view, - std::uint32_t txCountPadding = 0) const; + getMetrics(OpenView const& view) const; /** Returns information about the transactions currently in the queue for the account. @@ -452,8 +451,7 @@ class TxQ */ static std::uint64_t - scaleFeeLevel(Snapshot const& snapshot, OpenView const& view, - std::uint32_t txCountPadding = 0); + scaleFeeLevel(Snapshot const& snapshot, OpenView const& view); /** Computes the total fee level for all transactions in a series. diff --git a/src/ripple/app/misc/impl/TxQ.cpp b/src/ripple/app/misc/impl/TxQ.cpp index 2d6f5974482..0bd1e722fb6 100644 --- a/src/ripple/app/misc/impl/TxQ.cpp +++ b/src/ripple/app/misc/impl/TxQ.cpp @@ -164,10 +164,10 @@ TxQ::FeeMetrics::update(Application& app, std::uint64_t TxQ::FeeMetrics::scaleFeeLevel(Snapshot const& snapshot, - OpenView const& view, std::uint32_t txCountPadding) + OpenView const& view) { // Transactions in the open ledger so far - auto const current = view.txCount() + txCountPadding; + auto const current = view.txCount(); auto const target = snapshot.txnsExpected; auto const multiplier = snapshot.escalationMultiplier; @@ -1375,7 +1375,7 @@ TxQ::accept(Application& app, } auto -TxQ::getMetrics(OpenView const& view, std::uint32_t txCountPadding) const +TxQ::getMetrics(OpenView const& view) const -> boost::optional { auto const allowEscalation = @@ -1397,8 +1397,7 @@ TxQ::getMetrics(OpenView const& view, std::uint32_t txCountPadding) const result.minProcessingFeeLevel = isFull() ? byFee_.rbegin()->feeLevel + 1 : baseLevel; result.medFeeLevel = snapshot.escalationMultiplier; - result.openLedgerFeeLevel = FeeMetrics::scaleFeeLevel(snapshot, view, - txCountPadding); + result.openLedgerFeeLevel = FeeMetrics::scaleFeeLevel(snapshot, view); return result; } diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 23629640191..81c77dd1485 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -699,10 +699,7 @@ Json::Value checkFee ( ledger->fees(), isUnlimited (role)); std::uint64_t fee = loadFee; { - auto const assumeTx = request.isMember("x_assume_tx") && - request["x_assume_tx"].isConvertibleTo(Json::uintValue) ? - request["x_assume_tx"].asUInt() : 0; - auto const metrics = txQ.getMetrics(*ledger, assumeTx); + auto const metrics = txQ.getMetrics(*ledger); if(metrics) { auto const baseFee = ledger->fees().base; @@ -729,13 +726,6 @@ Json::Value checkFee ( return result.second; }(); - if (fee > limit && fee != loadFee && - request.isMember("x_queue_okay") && - request["x_queue_okay"].isBool() && - request["x_queue_okay"].asBool()) - { - fee = std::max(loadFee, limit); - } if (fee > limit) { std::stringstream ss; diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index a24985f5e3c..1bcf56a6cfe 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -1830,11 +1830,6 @@ class TxQ_test : public beast::unit_test::suite auto const bob = Account("bob"); env.fund(XRP(100000), alice, bob); - auto params = Json::Value(Json::objectValue); - // Max fee = 50k drops - params[jss::fee_mult_max] = 100; - params["x_queue_okay"] = true; - fillQueue(env, alice); checkMetrics(env, 0, boost::none, 7, 6, 256); @@ -1842,15 +1837,16 @@ class TxQ_test : public beast::unit_test::suite auto const aliceSeq = env.seq(alice); auto const lastLedgerSeq = env.current()->info().seq + 2; + auto submitParams = Json::Value(Json::objectValue); for (int i = 0; i < 5; ++i) { if (i == 2) - envs(noop(alice), fee(none), seq(none), + envs(noop(alice), fee(1000), seq(none), json(jss::LastLedgerSequence, lastLedgerSeq), - ter(terQUEUED))(params); + ter(terQUEUED))(submitParams); else - envs(noop(alice), fee(none), seq(none), - ter(terQUEUED))(params); + envs(noop(alice), fee(1000), seq(none), + ter(terQUEUED))(submitParams); } checkMetrics(env, 5, boost::none, 7, 6, 256); { @@ -1920,7 +1916,7 @@ class TxQ_test : public beast::unit_test::suite } } // Now, fill the gap. - envs(noop(alice), fee(none), seq(none), ter(terQUEUED))(params); + envs(noop(alice), fee(1000), seq(none), ter(terQUEUED))(submitParams); checkMetrics(env, 5, 18, 10, 9, 256); { auto aliceStat = txQ.getAccountTxs(alice.id(), *env.current()); @@ -1970,11 +1966,6 @@ class TxQ_test : public beast::unit_test::suite R"(", "queue": true, "ledger_index": 3 })"; BEAST_EXPECT(env.current()->info().seq > 3); - auto submitParams = Json::Value(Json::objectValue); - // Max fee = 100 drops - submitParams[jss::fee_mult_max] = 10; - submitParams["x_queue_okay"] = true; - { // account_info without the "queue" argument. auto const info = env.rpc("json", "account_info", withoutQueue); @@ -2021,10 +2012,11 @@ class TxQ_test : public beast::unit_test::suite BEAST_EXPECT(!queue_data.isMember(jss::transactions)); } - envs(noop(alice), fee(none), seq(none), ter(terQUEUED))(submitParams); - envs(noop(alice), fee(none), seq(none), ter(terQUEUED))(submitParams); - envs(noop(alice), fee(none), seq(none), ter(terQUEUED))(submitParams); - envs(noop(alice), fee(none), seq(none), ter(terQUEUED))(submitParams); + auto submitParams = Json::Value(Json::objectValue); + envs(noop(alice), fee(100), seq(none), ter(terQUEUED))(submitParams); + envs(noop(alice), fee(100), seq(none), ter(terQUEUED))(submitParams); + envs(noop(alice), fee(100), seq(none), ter(terQUEUED))(submitParams); + envs(noop(alice), fee(100), seq(none), ter(terQUEUED))(submitParams); checkMetrics(env, 4, 6, 4, 3, 256); { @@ -2076,7 +2068,7 @@ class TxQ_test : public beast::unit_test::suite } // Queue up a blocker - envs(fset(alice, asfAccountTxnID), fee(none), seq(none), + envs(fset(alice, asfAccountTxnID), fee(100), seq(none), json(jss::LastLedgerSequence, 10), ter(terQUEUED))(submitParams); checkMetrics(env, 5, 6, 4, 3, 256); @@ -2131,7 +2123,7 @@ class TxQ_test : public beast::unit_test::suite } } - envs(noop(alice), fee(none), seq(none), ter(telCAN_NOT_QUEUE_BLOCKED))(submitParams); + envs(noop(alice), fee(100), seq(none), ter(telCAN_NOT_QUEUE_BLOCKED))(submitParams); checkMetrics(env, 5, 6, 4, 3, 256); { @@ -2229,12 +2221,6 @@ class TxQ_test : public beast::unit_test::suite env.fund(XRP(1000000), alice); env.close(); - auto submitParams = Json::Value(Json::objectValue); - // Max fee = 100 drops - submitParams[jss::fee_mult_max] = 10; - submitParams["x-queue-okay"] = true; - submitParams["x_queue_okay"] = true; - { auto const server_info = env.rpc("server_info"); BEAST_EXPECT(server_info.isMember(jss::result) && @@ -2270,8 +2256,9 @@ class TxQ_test : public beast::unit_test::suite checkMetrics(env, 0, 6, 4, 3, 256); auto aliceSeq = env.seq(alice); + auto submitParams = Json::Value(Json::objectValue); for (auto i = 0; i < 4; ++i) - envs(noop(alice), fee(none), seq(aliceSeq + i), + envs(noop(alice), fee(100), seq(aliceSeq + i), ter(terQUEUED))(submitParams); checkMetrics(env, 4, 6, 4, 3, 256); diff --git a/src/test/rpc/JSONRPC_test.cpp b/src/test/rpc/JSONRPC_test.cpp index a1fc6b50aa9..7cd56f8c22f 100644 --- a/src/test/rpc/JSONRPC_test.cpp +++ b/src/test/rpc/JSONRPC_test.cpp @@ -1965,11 +1965,10 @@ class JSONRPC_test : public beast::unit_test::suite LoadFeeTrack const& feeTrack = env.app().getFeeTrack(); { - // 1: high mult, no queue, no pad + // high mult, no tx Json::Value req; Json::Reader ().parse (R"({ "fee_mult_max" : 1000, - "x_queue_okay" : false, "tx_json" : { } })", req); Json::Value result = @@ -1983,48 +1982,33 @@ class JSONRPC_test : public beast::unit_test::suite } { - // 2: high mult, can queue, no pad + // low mult, no tx Json::Value req; Json::Reader().parse(R"({ - "fee_mult_max" : 1000, - "x_queue_okay" : true, + "fee_mult_max" : 5, "tx_json" : { } })", req); Json::Value result = checkFee(req, Role::ADMIN, true, env.app().config(), feeTrack, - env.app().getTxQ(), env.current()); + env.app().getTxQ(), env.current()); BEAST_EXPECT(!RPC::contains_error(result)); BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) && req[jss::tx_json][jss::Fee] == 10); } + // put 4 transactions into the open ledger + for (auto i = 0; i < 4; ++i) { - // 3: high mult, no queue, 4 pad - Json::Value req; - Json::Reader().parse(R"({ - "fee_mult_max" : 1000, - "x_assume_tx" : 4, - "tx_json" : { } - })", req); - Json::Value result = - checkFee(req, Role::ADMIN, true, - env.app().config(), feeTrack, - env.app().getTxQ(), env.current()); - - BEAST_EXPECT(!RPC::contains_error(result)); - BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) && - req[jss::tx_json][jss::Fee] == 8889); + env(noop(env.master)); } { - // 4: high mult, can queue, 4 pad + // high mult, 4 txs Json::Value req; Json::Reader().parse(R"({ "fee_mult_max" : 1000, - "x_assume_tx" : 4, - "x_queue_okay" : true, "tx_json" : { } })", req); Json::Value result = @@ -2037,49 +2021,11 @@ class JSONRPC_test : public beast::unit_test::suite req[jss::tx_json][jss::Fee] == 8889); } - /////////////////// - { - // 5: low mult, no queue, no pad - Json::Value req; - Json::Reader().parse(R"({ - "fee_mult_max" : 5, - "tx_json" : { } - })", req); - Json::Value result = - checkFee(req, Role::ADMIN, true, - env.app().config(), feeTrack, - env.app().getTxQ(), env.current()); - - BEAST_EXPECT(!RPC::contains_error(result)); - BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) && - req[jss::tx_json][jss::Fee] == 10); - } - { - // 6: low mult, can queue, no pad + // low mult, 4 tx Json::Value req; Json::Reader().parse(R"({ "fee_mult_max" : 5, - "x_queue_okay" : true, - "tx_json" : { } - })", req); - Json::Value result = - checkFee(req, Role::ADMIN, true, - env.app().config(), feeTrack, - env.app().getTxQ(), env.current()); - - BEAST_EXPECT(!RPC::contains_error(result)); - BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) && - req[jss::tx_json][jss::Fee] == 10); - } - - { - // 7: low mult, no queue, 4 pad - Json::Value req; - Json::Reader().parse(R"({ - "fee_mult_max" : 5, - "x_assume_tx" : 4, - "x_queue_okay" : false, "tx_json" : { } })", req); Json::Value result = @@ -2092,32 +2038,28 @@ class JSONRPC_test : public beast::unit_test::suite } { - // 8: : low mult, can queue, 4 pad + // different low mult, 4 tx Json::Value req; Json::Reader().parse(R"({ - "fee_mult_max" : 5, - "x_assume_tx" : 4, - "x_queue_okay" : true, + "fee_mult_max" : 1000, + "fee_div_max" : 3, "tx_json" : { } })", req); Json::Value result = checkFee(req, Role::ADMIN, true, env.app().config(), feeTrack, - env.app().getTxQ(), env.current()); + env.app().getTxQ(), env.current()); - BEAST_EXPECT(!RPC::contains_error(result)); - BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) && - req[jss::tx_json][jss::Fee] == 50); + BEAST_EXPECT(RPC::contains_error(result)); + BEAST_EXPECT(!req[jss::tx_json].isMember(jss::Fee)); } { - // 8a: : different low mult, can queue, 4 pad + // high mult, 4 tx Json::Value req; Json::Reader().parse(R"({ - "fee_mult_max" : 1000, + "fee_mult_max" : 8000, "fee_div_max" : 3, - "x_assume_tx" : 4, - "x_queue_okay" : true, "tx_json" : { } })", req); Json::Value result = @@ -2127,15 +2069,14 @@ class JSONRPC_test : public beast::unit_test::suite BEAST_EXPECT(!RPC::contains_error(result)); BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) && - req[jss::tx_json][jss::Fee] == 3333); + req[jss::tx_json][jss::Fee] == 8889); } { - // 9: negative mult + // negative mult Json::Value req; Json::Reader().parse(R"({ "fee_mult_max" : -5, - "x_queue_okay" : true, "tx_json" : { } })", req); Json::Value result = @@ -2147,11 +2088,10 @@ class JSONRPC_test : public beast::unit_test::suite } { - // 9: negative div + // negative div Json::Value req; Json::Reader().parse(R"({ "fee_div_max" : -2, - "x_queue_okay" : true, "tx_json" : { } })", req); Json::Value result = @@ -2163,12 +2103,11 @@ class JSONRPC_test : public beast::unit_test::suite } { - // 9: negative mult & div + // negative mult & div Json::Value req; Json::Reader().parse(R"({ "fee_mult_max" : -2, "fee_div_max" : -3, - "x_queue_okay" : true, "tx_json" : { } })", req); Json::Value result = @@ -2179,8 +2118,10 @@ class JSONRPC_test : public beast::unit_test::suite BEAST_EXPECT(RPC::contains_error(result)); } + env.close(); + { - // 10: Call "sign" with nothing in the open ledger + // Call "sign" with nothing in the open ledger Json::Value toSign; toSign[jss::tx_json] = noop(env.master); toSign[jss::secret] = "masterpassphrase"; @@ -2196,7 +2137,7 @@ class JSONRPC_test : public beast::unit_test::suite } { - // 11: Call "sign" with enough transactions in the open ledger + // Call "sign" with enough transactions in the open ledger // to escalate the fee. for (;;) { @@ -2218,7 +2159,7 @@ class JSONRPC_test : public beast::unit_test::suite BEAST_EXPECT(! RPC::contains_error(result)); BEAST_EXPECT(result[jss::tx_json].isMember(jss::Fee) && - result[jss::tx_json][jss::Fee] == "8889"); + result[jss::tx_json][jss::Fee] == "7813"); BEAST_EXPECT(result[jss::tx_json].isMember(jss::Sequence) && result[jss::tx_json][jss::Sequence].isConvertibleTo( Json::ValueType::uintValue)); @@ -2227,7 +2168,7 @@ class JSONRPC_test : public beast::unit_test::suite } { - // 12: Call "sign" with higher server load + // Call "sign" with higher server load { auto& feeTrack = env.app().getFeeTrack(); BEAST_EXPECT(feeTrack.getLoadFactor() == 256); @@ -2250,6 +2191,40 @@ class JSONRPC_test : public beast::unit_test::suite Json::ValueType::uintValue)); } + { + // Call "sign" with higher server load and + // enough transactions to escalate the fee + BEAST_EXPECT(feeTrack.getLoadFactor() == 1220); + + for (;;) + { + auto metrics = env.app().getTxQ().getMetrics(*env.current()); + if (!BEAST_EXPECT(metrics)) + break; + if (metrics->openLedgerFeeLevel > + metrics->minProcessingFeeLevel) + break; + env(noop(env.master), fee(47)); + } + + Env_ss envs(env); + + + Json::Value toSign; + toSign[jss::tx_json] = noop(env.master); + toSign[jss::secret] = "masterpassphrase"; + // Max fee = 7000 drops + toSign[jss::fee_mult_max] = 700; + auto rpcResult = env.rpc("json", "sign", to_string(toSign)); + auto result = rpcResult[jss::result]; + + BEAST_EXPECT(! RPC::contains_error(result)); + BEAST_EXPECT(result[jss::tx_json].isMember(jss::Fee) && + result[jss::tx_json][jss::Fee] == "6806"); + BEAST_EXPECT(result[jss::tx_json].isMember(jss::Sequence) && + result[jss::tx_json][jss::Sequence].isConvertibleTo( + Json::ValueType::uintValue)); + } } // A function that can be called as though it would process a transaction.