XRPFees: Fee setting and handling improvements (#4247)

* Introduces amendment `XRPFees`
* Convert fee voting and protocol messages to use XRPAmounts
* Includes Validations, Change transactions, the "Fees" ledger object,
  and subscription messages

* Improve handling of 0 drop reference fee with TxQ. For use with networks that do not want to require fees
* Note that fee escalation logic is still in place, which may cause the
  open ledger fee to rise if the network is busy. 0 drop transactions
  will still queue, and fee escalation can be effectively disabled by
  modifying the configuration on all nodes

* Change default network reserves to match Mainnet

* Name the new SFields *Drops (not *XRP)
* Reserve SField IDs for Hooks

* Clarify comments explaining the ttFEE transaction field validation
This commit is contained in:
Ed Hennis
2023-02-02 16:20:35 -08:00
committed by GitHub
parent 0ce15e0e35
commit e4b17d1cf2
46 changed files with 691 additions and 337 deletions

View File

@@ -515,7 +515,10 @@ public:
// All it takes is a large enough XRP payment to resurrect
// becky's account. Try too small a payment.
env(pay(alice, becky, XRP(9)), ter(tecNO_DST_INSUF_XRP));
env(pay(alice,
becky,
drops(env.current()->fees().accountReserve(0)) - XRP(1)),
ter(tecNO_DST_INSUF_XRP));
env.close();
// Actually resurrect becky's account.

View File

@@ -29,13 +29,14 @@ class FeeVote_test : public beast::unit_test::suite
void
testSetup()
{
FeeVote::Setup const defaultSetup;
{
// defaults
Section config;
auto setup = setup_FeeVote(config);
BEAST_EXPECT(setup.reference_fee == 10);
BEAST_EXPECT(setup.account_reserve == 10 * DROPS_PER_XRP);
BEAST_EXPECT(setup.owner_reserve == 2 * DROPS_PER_XRP);
BEAST_EXPECT(setup.reference_fee == defaultSetup.reference_fee);
BEAST_EXPECT(setup.account_reserve == defaultSetup.account_reserve);
BEAST_EXPECT(setup.owner_reserve == defaultSetup.owner_reserve);
}
{
Section config;
@@ -56,9 +57,9 @@ class FeeVote_test : public beast::unit_test::suite
"owner_reserve = foo"});
// Illegal values are ignored, and the defaults left unchanged
auto setup = setup_FeeVote(config);
BEAST_EXPECT(setup.reference_fee == 10);
BEAST_EXPECT(setup.account_reserve == 10 * DROPS_PER_XRP);
BEAST_EXPECT(setup.owner_reserve == 2 * DROPS_PER_XRP);
BEAST_EXPECT(setup.reference_fee == defaultSetup.reference_fee);
BEAST_EXPECT(setup.account_reserve == defaultSetup.account_reserve);
BEAST_EXPECT(setup.owner_reserve == defaultSetup.owner_reserve);
}
{
Section config;
@@ -68,7 +69,7 @@ class FeeVote_test : public beast::unit_test::suite
"owner_reserve = -1234"});
// Illegal values are ignored, and the defaults left unchanged
auto setup = setup_FeeVote(config);
BEAST_EXPECT(setup.reference_fee == 10);
BEAST_EXPECT(setup.reference_fee == defaultSetup.reference_fee);
BEAST_EXPECT(
setup.account_reserve == static_cast<std::uint32_t>(-1234567));
BEAST_EXPECT(
@@ -86,9 +87,9 @@ class FeeVote_test : public beast::unit_test::suite
"owner_reserve = " + big64});
// Illegal values are ignored, and the defaults left unchanged
auto setup = setup_FeeVote(config);
BEAST_EXPECT(setup.reference_fee == 10);
BEAST_EXPECT(setup.account_reserve == 10 * DROPS_PER_XRP);
BEAST_EXPECT(setup.owner_reserve == 2 * DROPS_PER_XRP);
BEAST_EXPECT(setup.reference_fee == defaultSetup.reference_fee);
BEAST_EXPECT(setup.account_reserve == defaultSetup.account_reserve);
BEAST_EXPECT(setup.owner_reserve == defaultSetup.owner_reserve);
}
}

View File

@@ -36,55 +36,52 @@ public:
Fees const fees = [&]() {
Fees f;
f.base = d.FEE_DEFAULT;
f.units = d.TRANSACTION_FEE_BASE;
f.reserve = 200 * DROPS_PER_XRP;
f.increment = 50 * DROPS_PER_XRP;
return f;
}();
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{0}, l, fees, false) == XRPAmount{0});
scaleFeeLoad(XRPAmount{0}, l, fees, false) == XRPAmount{0});
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{10000}, l, fees, false) ==
scaleFeeLoad(XRPAmount{10000}, l, fees, false) ==
XRPAmount{10000});
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{1}, l, fees, false) == XRPAmount{1});
scaleFeeLoad(XRPAmount{1}, l, fees, false) == XRPAmount{1});
}
{
Fees const fees = [&]() {
Fees f;
f.base = d.FEE_DEFAULT * 10;
f.units = d.TRANSACTION_FEE_BASE;
f.reserve = 200 * DROPS_PER_XRP;
f.increment = 50 * DROPS_PER_XRP;
return f;
}();
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{0}, l, fees, false) == XRPAmount{0});
scaleFeeLoad(XRPAmount{0}, l, fees, false) == XRPAmount{0});
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{10000}, l, fees, false) ==
XRPAmount{100000});
scaleFeeLoad(XRPAmount{10000}, l, fees, false) ==
XRPAmount{10000});
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{1}, l, fees, false) == XRPAmount{10});
scaleFeeLoad(XRPAmount{1}, l, fees, false) == XRPAmount{1});
}
{
Fees const fees = [&]() {
Fees f;
f.base = d.FEE_DEFAULT;
f.units = d.TRANSACTION_FEE_BASE * 10;
f.reserve = 200 * DROPS_PER_XRP;
f.increment = 50 * DROPS_PER_XRP;
return f;
}();
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{0}, l, fees, false) == XRPAmount{0});
scaleFeeLoad(XRPAmount{0}, l, fees, false) == XRPAmount{0});
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{10000}, l, fees, false) ==
XRPAmount{1000});
scaleFeeLoad(XRPAmount{10000}, l, fees, false) ==
XRPAmount{10000});
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{1}, l, fees, false) == XRPAmount{0});
scaleFeeLoad(XRPAmount{1}, l, fees, false) == XRPAmount{1});
}
}
};

View File

@@ -16,6 +16,7 @@
//==============================================================================
#include <ripple/app/tx/apply.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/STAccount.h>
#include <string>
#include <test/jtx.h>
@@ -27,17 +28,26 @@ namespace test {
struct PseudoTx_test : public beast::unit_test::suite
{
std::vector<STTx>
getPseudoTxs(std::uint32_t seq)
getPseudoTxs(Rules const& rules, std::uint32_t seq)
{
std::vector<STTx> res;
res.emplace_back(STTx(ttFEE, [&](auto& obj) {
obj[sfAccount] = AccountID();
obj[sfLedgerSequence] = seq;
obj[sfBaseFee] = 0;
obj[sfReserveBase] = 0;
obj[sfReserveIncrement] = 0;
obj[sfReferenceFeeUnits] = 0;
if (rules.enabled(featureXRPFees))
{
obj[sfBaseFeeDrops] = XRPAmount{0};
obj[sfReserveBaseDrops] = XRPAmount{0};
obj[sfReserveIncrementDrops] = XRPAmount{0};
}
else
{
obj[sfBaseFee] = 0;
obj[sfReserveBase] = 0;
obj[sfReserveIncrement] = 0;
obj[sfReferenceFeeUnits] = 0;
}
}));
res.emplace_back(STTx(ttAMENDMENT, [&](auto& obj) {
@@ -66,12 +76,13 @@ struct PseudoTx_test : public beast::unit_test::suite
}
void
testPrevented()
testPrevented(FeatureBitset features)
{
using namespace jtx;
Env env(*this);
Env env(*this, features);
for (auto const& stx : getPseudoTxs(env.closed()->seq() + 1))
for (auto const& stx :
getPseudoTxs(env.closed()->rules(), env.closed()->seq() + 1))
{
std::string reason;
BEAST_EXPECT(isPseudoTx(stx));
@@ -101,7 +112,12 @@ struct PseudoTx_test : public beast::unit_test::suite
void
run() override
{
testPrevented();
using namespace test::jtx;
FeatureBitset const all{supported_amendments()};
FeatureBitset const xrpFees{featureXRPFees};
testPrevented(all - featureXRPFees);
testPrevented(all);
testAllowed();
}
};

View File

@@ -144,9 +144,15 @@ class TxQ1_test : public beast::unit_test::suite
auto const& view = *env.current();
auto metrics = env.app().getTxQ().getMetrics(view);
auto const base = [&view]() {
auto base = view.fees().base;
if (!base)
base += 1;
return base;
}();
// Don't care about the overflow flag
return fee(toDrops(metrics.openLedgerFeeLevel, view.fees().base) + 1);
return fee(toDrops(metrics.openLedgerFeeLevel, base) + 1);
}
static std::unique_ptr<Config>
@@ -189,7 +195,6 @@ class TxQ1_test : public beast::unit_test::suite
std::size_t expectedPerLedger,
std::size_t ledgersInQueue,
std::uint32_t base,
std::uint32_t units,
std::uint32_t reserve,
std::uint32_t increment)
{
@@ -219,7 +224,6 @@ class TxQ1_test : public beast::unit_test::suite
checkMetrics(__LINE__, env, 0, flagMaxQueue, 0, expectedPerLedger, 256);
auto const fees = env.current()->fees();
BEAST_EXPECT(fees.base == XRPAmount{base});
BEAST_EXPECT(fees.units == FeeUnit64{units});
BEAST_EXPECT(fees.reserve == XRPAmount{reserve});
BEAST_EXPECT(fees.increment == XRPAmount{increment});
@@ -1095,7 +1099,7 @@ public:
checkMetrics(__LINE__, env, 0, std::nullopt, 0, 3, 256);
// ledgers in queue is 2 because of makeConfig
auto const initQueueMax = initFee(env, 3, 2, 10, 10, 200, 50);
auto const initQueueMax = initFee(env, 3, 2, 10, 200, 50);
// Create several accounts while the fee is cheap so they all apply.
env.fund(drops(2000), noripple(alice));
@@ -1742,7 +1746,7 @@ public:
auto queued = ter(terQUEUED);
// ledgers in queue is 2 because of makeConfig
auto const initQueueMax = initFee(env, 3, 2, 10, 10, 200, 50);
auto const initQueueMax = initFee(env, 3, 2, 10, 200, 50);
BEAST_EXPECT(env.current()->fees().base == 10);
@@ -2137,7 +2141,7 @@ public:
// queued before the open ledger fee approached the reserve,
// which would unnecessarily slow down this test.
// ledgers in queue is 2 because of makeConfig
auto const initQueueMax = initFee(env, 3, 2, 10, 10, 200, 50);
auto const initQueueMax = initFee(env, 3, 2, 10, 200, 50);
auto limit = 3;
@@ -4785,6 +4789,144 @@ public:
}
}
void
testZeroReferenceFee()
{
testcase("Zero reference fee");
using namespace jtx;
Account const alice("alice");
auto const queued = ter(terQUEUED);
Env env(
*this,
makeConfig(
{{"minimum_txn_in_ledger_standalone", "3"}},
{{"reference_fee", "0"},
{"account_reserve", "0"},
{"owner_reserve", "0"}}));
BEAST_EXPECT(env.current()->fees().base == 10);
checkMetrics(__LINE__, env, 0, std::nullopt, 0, 3, 256);
// ledgers in queue is 2 because of makeConfig
auto const initQueueMax = initFee(env, 3, 2, 0, 0, 0);
BEAST_EXPECT(env.current()->fees().base == 0);
{
auto const fee = env.rpc("fee");
if (BEAST_EXPECT(fee.isMember(jss::result)) &&
BEAST_EXPECT(!RPC::contains_error(fee[jss::result])))
{
auto const& result = fee[jss::result];
BEAST_EXPECT(result.isMember(jss::levels));
auto const& levels = result[jss::levels];
BEAST_EXPECT(
levels.isMember(jss::median_level) &&
levels[jss::median_level] == "128000");
BEAST_EXPECT(
levels.isMember(jss::minimum_level) &&
levels[jss::minimum_level] == "256");
BEAST_EXPECT(
levels.isMember(jss::open_ledger_level) &&
levels[jss::open_ledger_level] == "256");
BEAST_EXPECT(
levels.isMember(jss::reference_level) &&
levels[jss::reference_level] == "256");
auto const& drops = result[jss::drops];
BEAST_EXPECT(
drops.isMember(jss::base_fee) &&
drops[jss::base_fee] == "0");
BEAST_EXPECT(
drops.isMember(jss::median_fee) &&
drops[jss::base_fee] == "0");
BEAST_EXPECT(
drops.isMember(jss::minimum_fee) &&
drops[jss::base_fee] == "0");
BEAST_EXPECT(
drops.isMember(jss::open_ledger_fee) &&
drops[jss::base_fee] == "0");
}
}
checkMetrics(__LINE__, env, 0, initQueueMax, 0, 3, 256);
// The noripple is to reduce the number of transactions required to
// fund the accounts. There is no rippling in this test.
env.fund(XRP(100000), noripple(alice));
checkMetrics(__LINE__, env, 0, initQueueMax, 1, 3, 256);
env.close();
checkMetrics(__LINE__, env, 0, 6, 0, 3, 256);
fillQueue(env, alice);
checkMetrics(__LINE__, env, 0, 6, 4, 3, 256);
env(noop(alice), openLedgerFee(env));
checkMetrics(__LINE__, env, 0, 6, 5, 3, 256);
auto aliceSeq = env.seq(alice);
env(noop(alice), queued);
checkMetrics(__LINE__, env, 1, 6, 5, 3, 256);
env(noop(alice), seq(aliceSeq + 1), fee(10), queued);
checkMetrics(__LINE__, env, 2, 6, 5, 3, 256);
{
auto const fee = env.rpc("fee");
if (BEAST_EXPECT(fee.isMember(jss::result)) &&
BEAST_EXPECT(!RPC::contains_error(fee[jss::result])))
{
auto const& result = fee[jss::result];
BEAST_EXPECT(result.isMember(jss::levels));
auto const& levels = result[jss::levels];
BEAST_EXPECT(
levels.isMember(jss::median_level) &&
levels[jss::median_level] == "128000");
BEAST_EXPECT(
levels.isMember(jss::minimum_level) &&
levels[jss::minimum_level] == "256");
BEAST_EXPECT(
levels.isMember(jss::open_ledger_level) &&
levels[jss::open_ledger_level] == "355555");
BEAST_EXPECT(
levels.isMember(jss::reference_level) &&
levels[jss::reference_level] == "256");
auto const& drops = result[jss::drops];
BEAST_EXPECT(
drops.isMember(jss::base_fee) &&
drops[jss::base_fee] == "0");
BEAST_EXPECT(
drops.isMember(jss::median_fee) &&
drops[jss::median_fee] == "0");
BEAST_EXPECT(
drops.isMember(jss::minimum_fee) &&
drops[jss::minimum_fee] == "0");
BEAST_EXPECT(
drops.isMember(jss::open_ledger_fee) &&
drops[jss::open_ledger_fee] == "1389");
}
}
env.close();
checkMetrics(__LINE__, env, 0, 10, 2, 5, 256);
}
void
run() override
{
@@ -4825,6 +4967,7 @@ public:
testReexecutePreflight();
testQueueFullDropPenalty();
testCancelQueuedOffers();
testZeroReferenceFee();
}
};

View File

@@ -30,6 +30,8 @@ private:
void
testTypes()
{
using FeeLevel32 = FeeLevel<std::uint32_t>;
{
XRPAmount x{100};
BEAST_EXPECT(x.drops() == 100);
@@ -45,8 +47,8 @@ private:
BEAST_EXPECT(
(std::is_same_v<decltype(z)::unit_type, feeunit::dropTag>));
FeeUnit32 f{10};
FeeUnit32 baseFee{100};
FeeLevel32 f{10};
FeeLevel32 baseFee{100};
auto drops = mulDiv(baseFee, x, f).second;
@@ -65,8 +67,8 @@ private:
BEAST_EXPECT(
(std::is_same_v<decltype(y)::unit_type, feeunit::dropTag>));
FeeUnit64 f{10};
FeeUnit64 baseFee{100};
FeeLevel64 f{10};
FeeLevel64 baseFee{100};
auto drops = mulDiv(baseFee, x, f).second;
@@ -102,22 +104,24 @@ private:
testJson()
{
// Json value functionality
using FeeLevel32 = FeeLevel<std::uint32_t>;
{
FeeUnit32 x{std::numeric_limits<std::uint32_t>::max()};
FeeLevel32 x{std::numeric_limits<std::uint32_t>::max()};
auto y = x.jsonClipped();
BEAST_EXPECT(y.type() == Json::uintValue);
BEAST_EXPECT(y == Json::Value{x.fee()});
}
{
FeeUnit32 x{std::numeric_limits<std::uint32_t>::min()};
FeeLevel32 x{std::numeric_limits<std::uint32_t>::min()};
auto y = x.jsonClipped();
BEAST_EXPECT(y.type() == Json::uintValue);
BEAST_EXPECT(y == Json::Value{x.fee()});
}
{
FeeUnit64 x{std::numeric_limits<std::uint64_t>::max()};
FeeLevel64 x{std::numeric_limits<std::uint64_t>::max()};
auto y = x.jsonClipped();
BEAST_EXPECT(y.type() == Json::uintValue);
BEAST_EXPECT(
@@ -125,7 +129,7 @@ private:
}
{
FeeUnit64 x{std::numeric_limits<std::uint64_t>::min()};
FeeLevel64 x{std::numeric_limits<std::uint64_t>::min()};
auto y = x.jsonClipped();
BEAST_EXPECT(y.type() == Json::uintValue);
BEAST_EXPECT(y == Json::Value{0});
@@ -167,15 +171,17 @@ private:
{
// Explicitly test every defined function for the TaggedFee class
// since some of them are templated, but not used anywhere else.
using FeeLevel32 = FeeLevel<std::uint32_t>;
{
auto make = [&](auto x) -> FeeUnit64 { return x; };
auto explicitmake = [&](auto x) -> FeeUnit64 {
return FeeUnit64{x};
auto make = [&](auto x) -> FeeLevel64 { return x; };
auto explicitmake = [&](auto x) -> FeeLevel64 {
return FeeLevel64{x};
};
FeeUnit64 defaulted;
FeeLevel64 defaulted;
(void)defaulted;
FeeUnit64 test{0};
FeeLevel64 test{0};
BEAST_EXPECT(test.fee() == 0);
test = explicitmake(beast::zero);
@@ -187,13 +193,13 @@ private:
test = explicitmake(100u);
BEAST_EXPECT(test.fee() == 100);
FeeUnit64 const targetSame{200u};
FeeUnit32 const targetOther{300u};
FeeLevel64 const targetSame{200u};
FeeLevel32 const targetOther{300u};
test = make(targetSame);
BEAST_EXPECT(test.fee() == 200);
BEAST_EXPECT(test == targetSame);
BEAST_EXPECT(test < FeeUnit64{1000});
BEAST_EXPECT(test > FeeUnit64{100});
BEAST_EXPECT(test < FeeLevel64{1000});
BEAST_EXPECT(test > FeeLevel64{100});
test = make(targetOther);
BEAST_EXPECT(test.fee() == 300);
BEAST_EXPECT(test == targetOther);

View File

@@ -64,7 +64,7 @@ class Invariants_test : public beast::unit_test::suite
ov,
tx,
tesSUCCESS,
safe_cast<FeeUnit64>(env.current()->fees().units),
env.current()->fees().base,
tapNONE,
jlog};

View File

@@ -547,7 +547,10 @@ class AccountTx_test : public beast::unit_test::suite
// All it takes is a large enough XRP payment to resurrect
// becky's account. Try too small a payment.
env(pay(alice, becky, XRP(9)), ter(tecNO_DST_INSUF_XRP));
env(pay(alice,
becky,
drops(env.current()->fees().accountReserve(0)) - XRP(1)),
ter(tecNO_DST_INSUF_XRP));
env.close();
// Actually resurrect becky's account.

View File

@@ -20,6 +20,7 @@
#include <ripple/app/misc/NetworkOPs.h>
#include <ripple/beast/unit_test.h>
#include <ripple/core/ConfigSections.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/jss.h>
#include <test/jtx.h>
#include <test/jtx/WSClient.h>
@@ -326,11 +327,11 @@ public:
}
void
testValidations()
testValidations(FeatureBitset features)
{
using namespace jtx;
Env env{*this, envconfig(validator, "")};
Env env{*this, envconfig(validator, ""), features};
auto& cfg = env.app().config();
if (!BEAST_EXPECT(cfg.section(SECTION_VALIDATION_SEED).empty()))
return;
@@ -410,10 +411,25 @@ public:
if (jv.isMember(jss::server_version) != isFlagLedger)
return false;
if (jv.isMember(jss::reserve_base) != isFlagLedger)
bool xrpFees = env.closed()->rules().enabled(featureXRPFees);
if ((!xrpFees &&
jv.isMember(jss::reserve_base) != isFlagLedger) ||
(xrpFees && jv.isMember(jss::reserve_base)))
return false;
if (jv.isMember(jss::reserve_inc) != isFlagLedger)
if ((!xrpFees &&
jv.isMember(jss::reserve_inc) != isFlagLedger) ||
(xrpFees && jv.isMember(jss::reserve_inc)))
return false;
if ((xrpFees &&
jv.isMember(jss::reserve_base_drops) != isFlagLedger) ||
(!xrpFees && jv.isMember(jss::reserve_base_drops)))
return false;
if ((xrpFees &&
jv.isMember(jss::reserve_inc_drops) != isFlagLedger) ||
(!xrpFees && jv.isMember(jss::reserve_inc_drops)))
return false;
return true;
@@ -1140,11 +1156,16 @@ public:
void
run() override
{
using namespace test::jtx;
FeatureBitset const all{supported_amendments()};
FeatureBitset const xrpFees{featureXRPFees};
testServer();
testLedger();
testTransactions();
testManifests();
testValidations();
testValidations(all - xrpFees);
testValidations(all);
testSubErrors(true);
testSubErrors(false);
testSubByUrl();