Remove undocumented experimental options from RPC sign (RIPD-1653):

The `x_assume_tx` and `x_queue_okay` experimental options were
associated with the transaction queue that were not officially
supported.
This commit is contained in:
Edward Hennis
2018-10-15 18:43:04 -04:00
committed by Nik Bougalis
parent c587012e5c
commit a96cb8fc1c
5 changed files with 86 additions and 137 deletions

View File

@@ -320,8 +320,7 @@ public:
FeeEscalation amendment is not enabled. FeeEscalation amendment is not enabled.
*/ */
boost::optional<Metrics> boost::optional<Metrics>
getMetrics(OpenView const& view, getMetrics(OpenView const& view) const;
std::uint32_t txCountPadding = 0) const;
/** Returns information about the transactions currently /** Returns information about the transactions currently
in the queue for the account. in the queue for the account.
@@ -452,8 +451,7 @@ private:
*/ */
static static
std::uint64_t std::uint64_t
scaleFeeLevel(Snapshot const& snapshot, OpenView const& view, scaleFeeLevel(Snapshot const& snapshot, OpenView const& view);
std::uint32_t txCountPadding = 0);
/** /**
Computes the total fee level for all transactions in a series. Computes the total fee level for all transactions in a series.

View File

@@ -164,10 +164,10 @@ TxQ::FeeMetrics::update(Application& app,
std::uint64_t std::uint64_t
TxQ::FeeMetrics::scaleFeeLevel(Snapshot const& snapshot, TxQ::FeeMetrics::scaleFeeLevel(Snapshot const& snapshot,
OpenView const& view, std::uint32_t txCountPadding) OpenView const& view)
{ {
// Transactions in the open ledger so far // 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 target = snapshot.txnsExpected;
auto const multiplier = snapshot.escalationMultiplier; auto const multiplier = snapshot.escalationMultiplier;
@@ -1375,7 +1375,7 @@ TxQ::accept(Application& app,
} }
auto auto
TxQ::getMetrics(OpenView const& view, std::uint32_t txCountPadding) const TxQ::getMetrics(OpenView const& view) const
-> boost::optional<Metrics> -> boost::optional<Metrics>
{ {
auto const allowEscalation = auto const allowEscalation =
@@ -1397,8 +1397,7 @@ TxQ::getMetrics(OpenView const& view, std::uint32_t txCountPadding) const
result.minProcessingFeeLevel = isFull() ? byFee_.rbegin()->feeLevel + 1 : result.minProcessingFeeLevel = isFull() ? byFee_.rbegin()->feeLevel + 1 :
baseLevel; baseLevel;
result.medFeeLevel = snapshot.escalationMultiplier; result.medFeeLevel = snapshot.escalationMultiplier;
result.openLedgerFeeLevel = FeeMetrics::scaleFeeLevel(snapshot, view, result.openLedgerFeeLevel = FeeMetrics::scaleFeeLevel(snapshot, view);
txCountPadding);
return result; return result;
} }

View File

@@ -699,10 +699,7 @@ Json::Value checkFee (
ledger->fees(), isUnlimited (role)); ledger->fees(), isUnlimited (role));
std::uint64_t fee = loadFee; std::uint64_t fee = loadFee;
{ {
auto const assumeTx = request.isMember("x_assume_tx") && auto const metrics = txQ.getMetrics(*ledger);
request["x_assume_tx"].isConvertibleTo(Json::uintValue) ?
request["x_assume_tx"].asUInt() : 0;
auto const metrics = txQ.getMetrics(*ledger, assumeTx);
if(metrics) if(metrics)
{ {
auto const baseFee = ledger->fees().base; auto const baseFee = ledger->fees().base;
@@ -729,13 +726,6 @@ Json::Value checkFee (
return result.second; 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) if (fee > limit)
{ {
std::stringstream ss; std::stringstream ss;

View File

@@ -1830,11 +1830,6 @@ public:
auto const bob = Account("bob"); auto const bob = Account("bob");
env.fund(XRP(100000), alice, 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); fillQueue(env, alice);
checkMetrics(env, 0, boost::none, 7, 6, 256); checkMetrics(env, 0, boost::none, 7, 6, 256);
@@ -1842,15 +1837,16 @@ public:
auto const aliceSeq = env.seq(alice); auto const aliceSeq = env.seq(alice);
auto const lastLedgerSeq = env.current()->info().seq + 2; auto const lastLedgerSeq = env.current()->info().seq + 2;
auto submitParams = Json::Value(Json::objectValue);
for (int i = 0; i < 5; ++i) for (int i = 0; i < 5; ++i)
{ {
if (i == 2) if (i == 2)
envs(noop(alice), fee(none), seq(none), envs(noop(alice), fee(1000), seq(none),
json(jss::LastLedgerSequence, lastLedgerSeq), json(jss::LastLedgerSequence, lastLedgerSeq),
ter(terQUEUED))(params); ter(terQUEUED))(submitParams);
else else
envs(noop(alice), fee(none), seq(none), envs(noop(alice), fee(1000), seq(none),
ter(terQUEUED))(params); ter(terQUEUED))(submitParams);
} }
checkMetrics(env, 5, boost::none, 7, 6, 256); checkMetrics(env, 5, boost::none, 7, 6, 256);
{ {
@@ -1920,7 +1916,7 @@ public:
} }
} }
// Now, fill the gap. // 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); checkMetrics(env, 5, 18, 10, 9, 256);
{ {
auto aliceStat = txQ.getAccountTxs(alice.id(), *env.current()); auto aliceStat = txQ.getAccountTxs(alice.id(), *env.current());
@@ -1970,11 +1966,6 @@ public:
R"(", "queue": true, "ledger_index": 3 })"; R"(", "queue": true, "ledger_index": 3 })";
BEAST_EXPECT(env.current()->info().seq > 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. // account_info without the "queue" argument.
auto const info = env.rpc("json", "account_info", withoutQueue); auto const info = env.rpc("json", "account_info", withoutQueue);
@@ -2021,10 +2012,11 @@ public:
BEAST_EXPECT(!queue_data.isMember(jss::transactions)); BEAST_EXPECT(!queue_data.isMember(jss::transactions));
} }
envs(noop(alice), fee(none), seq(none), ter(terQUEUED))(submitParams); auto submitParams = Json::Value(Json::objectValue);
envs(noop(alice), fee(none), seq(none), ter(terQUEUED))(submitParams); envs(noop(alice), fee(100), seq(none), ter(terQUEUED))(submitParams);
envs(noop(alice), fee(none), seq(none), ter(terQUEUED))(submitParams); envs(noop(alice), fee(100), seq(none), ter(terQUEUED))(submitParams);
envs(noop(alice), fee(none), 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); checkMetrics(env, 4, 6, 4, 3, 256);
{ {
@@ -2076,7 +2068,7 @@ public:
} }
// Queue up a blocker // Queue up a blocker
envs(fset(alice, asfAccountTxnID), fee(none), seq(none), envs(fset(alice, asfAccountTxnID), fee(100), seq(none),
json(jss::LastLedgerSequence, 10), json(jss::LastLedgerSequence, 10),
ter(terQUEUED))(submitParams); ter(terQUEUED))(submitParams);
checkMetrics(env, 5, 6, 4, 3, 256); checkMetrics(env, 5, 6, 4, 3, 256);
@@ -2131,7 +2123,7 @@ public:
} }
} }
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); checkMetrics(env, 5, 6, 4, 3, 256);
{ {
@@ -2229,12 +2221,6 @@ public:
env.fund(XRP(1000000), alice); env.fund(XRP(1000000), alice);
env.close(); 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"); auto const server_info = env.rpc("server_info");
BEAST_EXPECT(server_info.isMember(jss::result) && BEAST_EXPECT(server_info.isMember(jss::result) &&
@@ -2270,8 +2256,9 @@ public:
checkMetrics(env, 0, 6, 4, 3, 256); checkMetrics(env, 0, 6, 4, 3, 256);
auto aliceSeq = env.seq(alice); auto aliceSeq = env.seq(alice);
auto submitParams = Json::Value(Json::objectValue);
for (auto i = 0; i < 4; ++i) 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); ter(terQUEUED))(submitParams);
checkMetrics(env, 4, 6, 4, 3, 256); checkMetrics(env, 4, 6, 4, 3, 256);

View File

@@ -1965,11 +1965,10 @@ public:
LoadFeeTrack const& feeTrack = env.app().getFeeTrack(); LoadFeeTrack const& feeTrack = env.app().getFeeTrack();
{ {
// 1: high mult, no queue, no pad // high mult, no tx
Json::Value req; Json::Value req;
Json::Reader ().parse (R"({ Json::Reader ().parse (R"({
"fee_mult_max" : 1000, "fee_mult_max" : 1000,
"x_queue_okay" : false,
"tx_json" : { } "tx_json" : { }
})", req); })", req);
Json::Value result = Json::Value result =
@@ -1983,29 +1982,33 @@ public:
} }
{ {
// 2: high mult, can queue, no pad // low mult, no tx
Json::Value req; Json::Value req;
Json::Reader().parse(R"({ Json::Reader().parse(R"({
"fee_mult_max" : 1000, "fee_mult_max" : 5,
"x_queue_okay" : true,
"tx_json" : { } "tx_json" : { }
})", req); })", req);
Json::Value result = Json::Value result =
checkFee(req, Role::ADMIN, true, checkFee(req, Role::ADMIN, true,
env.app().config(), feeTrack, env.app().config(), feeTrack,
env.app().getTxQ(), env.current()); env.app().getTxQ(), env.current());
BEAST_EXPECT(!RPC::contains_error(result)); BEAST_EXPECT(!RPC::contains_error(result));
BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) && BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) &&
req[jss::tx_json][jss::Fee] == 10); 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 env(noop(env.master));
}
{
// high mult, 4 txs
Json::Value req; Json::Value req;
Json::Reader().parse(R"({ Json::Reader().parse(R"({
"fee_mult_max" : 1000, "fee_mult_max" : 1000,
"x_assume_tx" : 4,
"tx_json" : { } "tx_json" : { }
})", req); })", req);
Json::Value result = Json::Value result =
@@ -2019,69 +2022,12 @@ public:
} }
{ {
// 4: high mult, can queue, 4 pad // low mult, 4 tx
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 =
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);
}
///////////////////
{
// 5: low mult, no queue, no pad
Json::Value req; Json::Value req;
Json::Reader().parse(R"({ Json::Reader().parse(R"({
"fee_mult_max" : 5, "fee_mult_max" : 5,
"tx_json" : { } "tx_json" : { }
})", req); })", 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
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 = Json::Value result =
checkFee(req, Role::ADMIN, true, checkFee(req, Role::ADMIN, true,
env.app().config(), feeTrack, env.app().config(), feeTrack,
@@ -2092,32 +2038,28 @@ public:
} }
{ {
// 8: : low mult, can queue, 4 pad // different low mult, 4 tx
Json::Value req; Json::Value req;
Json::Reader().parse(R"({ Json::Reader().parse(R"({
"fee_mult_max" : 5, "fee_mult_max" : 1000,
"x_assume_tx" : 4, "fee_div_max" : 3,
"x_queue_okay" : true,
"tx_json" : { } "tx_json" : { }
})", req); })", req);
Json::Value result = Json::Value result =
checkFee(req, Role::ADMIN, true, checkFee(req, Role::ADMIN, true,
env.app().config(), feeTrack, env.app().config(), feeTrack,
env.app().getTxQ(), env.current()); env.app().getTxQ(), env.current());
BEAST_EXPECT(!RPC::contains_error(result)); BEAST_EXPECT(RPC::contains_error(result));
BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) && BEAST_EXPECT(!req[jss::tx_json].isMember(jss::Fee));
req[jss::tx_json][jss::Fee] == 50);
} }
{ {
// 8a: : different low mult, can queue, 4 pad // high mult, 4 tx
Json::Value req; Json::Value req;
Json::Reader().parse(R"({ Json::Reader().parse(R"({
"fee_mult_max" : 1000, "fee_mult_max" : 8000,
"fee_div_max" : 3, "fee_div_max" : 3,
"x_assume_tx" : 4,
"x_queue_okay" : true,
"tx_json" : { } "tx_json" : { }
})", req); })", req);
Json::Value result = Json::Value result =
@@ -2127,15 +2069,14 @@ public:
BEAST_EXPECT(!RPC::contains_error(result)); BEAST_EXPECT(!RPC::contains_error(result));
BEAST_EXPECT(req[jss::tx_json].isMember(jss::Fee) && 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::Value req;
Json::Reader().parse(R"({ Json::Reader().parse(R"({
"fee_mult_max" : -5, "fee_mult_max" : -5,
"x_queue_okay" : true,
"tx_json" : { } "tx_json" : { }
})", req); })", req);
Json::Value result = Json::Value result =
@@ -2147,11 +2088,10 @@ public:
} }
{ {
// 9: negative div // negative div
Json::Value req; Json::Value req;
Json::Reader().parse(R"({ Json::Reader().parse(R"({
"fee_div_max" : -2, "fee_div_max" : -2,
"x_queue_okay" : true,
"tx_json" : { } "tx_json" : { }
})", req); })", req);
Json::Value result = Json::Value result =
@@ -2163,12 +2103,11 @@ public:
} }
{ {
// 9: negative mult & div // negative mult & div
Json::Value req; Json::Value req;
Json::Reader().parse(R"({ Json::Reader().parse(R"({
"fee_mult_max" : -2, "fee_mult_max" : -2,
"fee_div_max" : -3, "fee_div_max" : -3,
"x_queue_okay" : true,
"tx_json" : { } "tx_json" : { }
})", req); })", req);
Json::Value result = Json::Value result =
@@ -2179,8 +2118,10 @@ public:
BEAST_EXPECT(RPC::contains_error(result)); 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; Json::Value toSign;
toSign[jss::tx_json] = noop(env.master); toSign[jss::tx_json] = noop(env.master);
toSign[jss::secret] = "masterpassphrase"; toSign[jss::secret] = "masterpassphrase";
@@ -2196,7 +2137,7 @@ public:
} }
{ {
// 11: Call "sign" with enough transactions in the open ledger // Call "sign" with enough transactions in the open ledger
// to escalate the fee. // to escalate the fee.
for (;;) for (;;)
{ {
@@ -2218,7 +2159,7 @@ public:
BEAST_EXPECT(! RPC::contains_error(result)); BEAST_EXPECT(! RPC::contains_error(result));
BEAST_EXPECT(result[jss::tx_json].isMember(jss::Fee) && 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) && BEAST_EXPECT(result[jss::tx_json].isMember(jss::Sequence) &&
result[jss::tx_json][jss::Sequence].isConvertibleTo( result[jss::tx_json][jss::Sequence].isConvertibleTo(
Json::ValueType::uintValue)); Json::ValueType::uintValue));
@@ -2227,7 +2168,7 @@ public:
} }
{ {
// 12: Call "sign" with higher server load // Call "sign" with higher server load
{ {
auto& feeTrack = env.app().getFeeTrack(); auto& feeTrack = env.app().getFeeTrack();
BEAST_EXPECT(feeTrack.getLoadFactor() == 256); BEAST_EXPECT(feeTrack.getLoadFactor() == 256);
@@ -2250,6 +2191,40 @@ public:
Json::ValueType::uintValue)); 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. // A function that can be called as though it would process a transaction.