Skip to content

Commit

Permalink
Add units to all fee calculations:
Browse files Browse the repository at this point in the history
* Uses existing XRPAmount with units for drops, and a new TaggedFee for
  fee units (LoadFeeTrack), and fee levels (TxQ).
* Resolves XRPLF#2451
  • Loading branch information
ximinez committed Jul 16, 2019
1 parent 60fc035 commit ab9735c
Show file tree
Hide file tree
Showing 73 changed files with 1,694 additions and 589 deletions.
4 changes: 3 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1737,11 +1737,13 @@ install (
src/ripple/basics/StringUtilities.h
src/ripple/basics/ToString.h
src/ripple/basics/UnorderedContainers.h
src/ripple/basics/XRPAmount.h
src/ripple/basics/algorithm.h
src/ripple/basics/base_uint.h
src/ripple/basics/chrono.h
src/ripple/basics/contract.h
src/ripple/basics/date.h
src/ripple/basics/feeunits.h
src/ripple/basics/hardened_hash.h
src/ripple/basics/strHex.h
DESTINATION include/ripple/basics)
Expand Down Expand Up @@ -1819,7 +1821,6 @@ install (
src/ripple/protocol/TxFlags.h
src/ripple/protocol/TxFormats.h
src/ripple/protocol/UintTypes.h
src/ripple/protocol/XRPAmount.h
src/ripple/protocol/digest.h
src/ripple/protocol/jss.h
src/ripple/protocol/tokens.h
Expand Down Expand Up @@ -2346,6 +2347,7 @@ else ()
src/test/basics/base64_test.cpp
src/test/basics/base_uint_test.cpp
src/test/basics/contract_test.cpp
src/test/basics/feeunits_test.cpp
src/test/basics/hardened_hash_test.cpp
src/test/basics/mulDiv_test.cpp
src/test/basics/qalloc_test.cpp
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/Ledger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ Ledger::Ledger (
, rules_{config.features}
{
info_.seq = 1;
info_.drops = SYSTEM_CURRENCY_START;
info_.drops = INITIAL_XRP;
info_.closeTimeResolution = ledgerDefaultTimeResolution;

static auto const id = calcAccountID(
Expand Down
8 changes: 4 additions & 4 deletions src/ripple/app/misc/FeeVote.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ class FeeVote
struct Setup
{
/** The cost of a reference transaction in drops. */
std::uint64_t reference_fee = 10;
XRPAmount reference_fee{ 10 };

/** The cost of a reference transaction in fee units. */
std::uint32_t const reference_fee_units = 10;
static constexpr FeeUnit32 reference_fee_units{ 10 };

/** The account reserve requirement in drops. */
std::uint64_t account_reserve = 20 * SYSTEM_CURRENCY_PARTS;
XRPAmount account_reserve{ 20 * DROPS_PER_XRP };

/** The per-owned item reserve requirement in drops. */
std::uint64_t owner_reserve = 5 * SYSTEM_CURRENCY_PARTS;
XRPAmount owner_reserve{ 5 * DROPS_PER_XRP };
};

virtual ~FeeVote () = default;
Expand Down
30 changes: 15 additions & 15 deletions src/ripple/app/misc/FeeVoteImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,13 @@ namespace ripple {

namespace detail {

template <typename Integer>
class VotableInteger
{
private:
using map_type = std::map <Integer, int>;
using Integer = XRPAmount;
Integer mCurrent; // The current setting
Integer mTarget; // The setting we want
map_type mVoteMap;
std::map <Integer, int> mVoteMap;

public:
VotableInteger (Integer current, Integer target)
Expand All @@ -62,9 +61,9 @@ class VotableInteger
getVotes() const;
};

template <class Integer>
Integer
VotableInteger <Integer>::getVotes() const
auto
VotableInteger::getVotes() const
-> Integer
{
Integer ourVote = mCurrent;
int weight = 0;
Expand Down Expand Up @@ -153,13 +152,14 @@ FeeVoteImpl::doVoting(
// LCL must be flag ledger
assert ((lastClosedLedger->info().seq % 256) == 0);

detail::VotableInteger<std::uint64_t> baseFeeVote (
detail::VotableInteger baseFeeVote (
lastClosedLedger->fees().base, target_.reference_fee);

detail::VotableInteger<std::uint32_t> baseReserveVote (
lastClosedLedger->fees().accountReserve(0).drops(), target_.account_reserve);
detail::VotableInteger baseReserveVote(
lastClosedLedger->fees().accountReserve(0),
target_.account_reserve);

detail::VotableInteger<std::uint32_t> incReserveVote (
detail::VotableInteger incReserveVote (
lastClosedLedger->fees().increment, target_.owner_reserve);

for (auto const& val : set)
Expand Down Expand Up @@ -196,15 +196,15 @@ FeeVoteImpl::doVoting(
}

// choose our positions
std::uint64_t const baseFee = baseFeeVote.getVotes ();
std::uint32_t const baseReserve = baseReserveVote.getVotes ();
std::uint32_t const incReserve = incReserveVote.getVotes ();
std::uint32_t const feeUnits = target_.reference_fee_units;
XRPAmount const baseFee = baseFeeVote.getVotes ();
XRPAmount const baseReserve = baseReserveVote.getVotes ();
XRPAmount const incReserve = incReserveVote.getVotes ();
constexpr FeeUnit32 feeUnits = Setup::reference_fee_units;
auto const seq = lastClosedLedger->info().seq + 1;

// add transactions to our position
if ((baseFee != lastClosedLedger->fees().base) ||
(baseReserve != lastClosedLedger->fees().accountReserve(0)) ||
(baseReserve != lastClosedLedger->fees().accountReserve(0)) ||
(incReserve != lastClosedLedger->fees().increment))
{
JLOG(journal_.warn()) <<
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/app/misc/LoadFeeTrack.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#ifndef RIPPLE_CORE_LOADFEETRACK_H_INCLUDED
#define RIPPLE_CORE_LOADFEETRACK_H_INCLUDED

#include <ripple/basics/feeunits.h>
#include <ripple/json/json_value.h>
#include <ripple/beast/utility/Journal.h>
#include <algorithm>
Expand Down Expand Up @@ -140,7 +141,8 @@ class LoadFeeTrack final
//------------------------------------------------------------------------------

// Scale using load as well as base rate
std::uint64_t scaleFeeLoad(std::uint64_t fee, LoadFeeTrack const& feeTrack,
XRPAmount
scaleFeeLoad(FeeUnit64 fee, LoadFeeTrack const& feeTrack,
Fees const& fees, bool bUnlimited);

} // ripple
Expand Down
120 changes: 59 additions & 61 deletions src/ripple/app/misc/NetworkOPs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class NetworkOPsImp final
{
ServerFeeSummary() = default;

ServerFeeSummary(std::uint64_t fee,
ServerFeeSummary(XRPAmount fee,
TxQ::Metrics&& escalationMetrics,
LoadFeeTrack const & loadFeeTrack);
bool
Expand All @@ -188,7 +188,7 @@ class NetworkOPsImp final

std::uint32_t loadFactorServer = 256;
std::uint32_t loadBaseServer = 256;
std::uint64_t baseFee = 10;
XRPAmount baseFee{ 10 };
boost::optional<TxQ::Metrics> em = boost::none;
};

Expand Down Expand Up @@ -1605,7 +1605,7 @@ void NetworkOPsImp::pubManifest (Manifest const& mo)
}

NetworkOPsImp::ServerFeeSummary::ServerFeeSummary(
std::uint64_t fee,
XRPAmount fee,
TxQ::Metrics&& escalationMetrics,
LoadFeeTrack const & loadFeeTrack)
: loadFactorServer{loadFeeTrack.getLoadFactor()}
Expand Down Expand Up @@ -1636,6 +1636,15 @@ NetworkOPsImp::ServerFeeSummary::operator !=(NetworkOPsImp::ServerFeeSummary con
return false;
}

// Need to cap to uint64 to uint32 due to JSON limitations
static std::uint32_t trunc32(std::uint64_t v)
{
constexpr std::uint64_t max32 =
std::numeric_limits<std::uint32_t>::max();

return std::min(max32, v);
};

void NetworkOPsImp::pubServer ()
{
// VFALCO TODO Don't hold the lock across calls to send...make a copy of the
Expand All @@ -1652,21 +1661,11 @@ void NetworkOPsImp::pubServer ()
app_.getTxQ().getMetrics(*app_.openLedger().current()),
app_.getFeeTrack()};

// Need to cap to uint64 to uint32 due to JSON limitations
auto clamp = [](std::uint64_t v)
{
constexpr std::uint64_t max32 =
std::numeric_limits<std::uint32_t>::max();

return static_cast<std::uint32_t>(std::min(max32, v));
};


jvObj [jss::type] = "serverStatus";
jvObj [jss::server_status] = strOperatingMode ();
jvObj [jss::load_base] = f.loadBaseServer;
jvObj [jss::load_factor_server] = f.loadFactorServer;
jvObj [jss::base_fee] = clamp(f.baseFee);
jvObj [jss::base_fee] = f.baseFee.json();

if(f.em)
{
Expand All @@ -1675,11 +1674,13 @@ void NetworkOPsImp::pubServer ()
mulDiv(f.em->openLedgerFeeLevel, f.loadBaseServer,
f.em->referenceFeeLevel).second);

jvObj [jss::load_factor] = clamp(loadFactor);
jvObj [jss::load_factor_fee_escalation] = clamp(f.em->openLedgerFeeLevel);
jvObj [jss::load_factor_fee_queue] = clamp(f.em->minProcessingFeeLevel);
jvObj [jss::load_factor_fee_reference]
= clamp(f.em->referenceFeeLevel);
jvObj [jss::load_factor] = trunc32(loadFactor);
jvObj [jss::load_factor_fee_escalation] =
f.em->openLedgerFeeLevel.json();
jvObj [jss::load_factor_fee_queue] =
f.em->minProcessingFeeLevel.json();
jvObj [jss::load_factor_fee_reference] =
f.em->referenceFeeLevel.json();

}
else
Expand Down Expand Up @@ -2236,16 +2237,13 @@ Json::Value NetworkOPsImp::getServerInfo (bool human, bool admin, bool counters)
escalationMetrics.referenceFeeLevel;

auto const loadFactor = std::max(safe_cast<std::uint64_t>(loadFactorServer),
mulDiv(loadFactorFeeEscalation, loadBaseServer, loadBaseFeeEscalation).second);
mulDiv(loadFactorFeeEscalation, loadBaseServer,
loadBaseFeeEscalation).second);

if (!human)
{
constexpr std::uint64_t max32 =
std::numeric_limits<std::uint32_t>::max();

info[jss::load_base] = loadBaseServer;
info[jss::load_factor] = static_cast<std::uint32_t>(
std::min(max32, loadFactor));
info[jss::load_factor] = trunc32(loadFactor);
info[jss::load_factor_server] = loadFactorServer;

/* Json::Value doesn't support uint64, so clamp to max
Expand All @@ -2254,14 +2252,11 @@ Json::Value NetworkOPsImp::getServerInfo (bool human, bool admin, bool counters)
that high.
*/
info[jss::load_factor_fee_escalation] =
static_cast<std::uint32_t> (std::min(
max32, loadFactorFeeEscalation));
loadFactorFeeEscalation.json();
info[jss::load_factor_fee_queue] =
static_cast<std::uint32_t> (std::min(
max32, escalationMetrics.minProcessingFeeLevel));
escalationMetrics.minProcessingFeeLevel.json();
info[jss::load_factor_fee_reference] =
static_cast<std::uint32_t> (std::min(
max32, loadBaseFeeEscalation));
loadBaseFeeEscalation.json();
}
else
{
Expand All @@ -2288,14 +2283,15 @@ Json::Value NetworkOPsImp::getServerInfo (bool human, bool admin, bool counters)
}
if (loadFactorFeeEscalation !=
escalationMetrics.referenceFeeLevel &&
(admin || loadFactorFeeEscalation != loadFactor))
(admin ||
loadFactorFeeEscalation != FeeLevel64{ loadFactor }))
info[jss::load_factor_fee_escalation] =
static_cast<double> (loadFactorFeeEscalation) /
static_cast<FeeLevelDouble>(loadFactorFeeEscalation) /
escalationMetrics.referenceFeeLevel;
if (escalationMetrics.minProcessingFeeLevel !=
escalationMetrics.referenceFeeLevel)
info[jss::load_factor_fee_queue] =
static_cast<double> (
static_cast<FeeLevelDouble>(
escalationMetrics.minProcessingFeeLevel) /
escalationMetrics.referenceFeeLevel;
}
Expand All @@ -2310,33 +2306,35 @@ Json::Value NetworkOPsImp::getServerInfo (bool human, bool admin, bool counters)

if (lpClosed)
{
std::uint64_t baseFee = lpClosed->fees().base;
std::uint64_t baseRef = lpClosed->fees().units;
XRPAmount const baseFee = lpClosed->fees().base;
Json::Value l (Json::objectValue);
l[jss::seq] = Json::UInt (lpClosed->info().seq);
l[jss::hash] = to_string (lpClosed->info().hash);

if (!human)
{
l[jss::base_fee] = Json::Value::UInt (baseFee);
l[jss::reserve_base] = Json::Value::UInt (lpClosed->fees().accountReserve(0).drops());
l[jss::base_fee] = baseFee.json();
l[jss::reserve_base] = lpClosed->fees().accountReserve(0).json();
l[jss::reserve_inc] =
Json::Value::UInt (lpClosed->fees().increment);
l[jss::close_time] =
Json::Value::UInt (lpClosed->info().closeTime.time_since_epoch().count());
lpClosed->fees().increment.json();
l[jss::close_time] = Json::Value::UInt (
lpClosed->info().closeTime.time_since_epoch().count());
}
else
{
l[jss::base_fee_xrp] = static_cast<double> (baseFee) /
SYSTEM_CURRENCY_PARTS;
// Make a local specialization of the TaggedFee class, using
// XRPAmount as the "unit" to make some of the math here easier.
using DropsDouble = units::TaggedFee<XRPAmount, double>;

l[jss::base_fee_xrp] =
static_cast<DropsDouble>(baseFee) /
DROPS_PER_XRP;
l[jss::reserve_base_xrp] =
static_cast<double> (Json::UInt (
lpClosed->fees().accountReserve(0).drops() * baseFee / baseRef))
/ SYSTEM_CURRENCY_PARTS;
static_cast<DropsDouble>(
lpClosed->fees().accountReserve(0)) / DROPS_PER_XRP;
l[jss::reserve_inc_xrp] =
static_cast<double> (Json::UInt (
lpClosed->fees().increment * baseFee / baseRef))
/ SYSTEM_CURRENCY_PARTS;
static_cast<DropsDouble>(
lpClosed->fees().increment) / DROPS_PER_XRP;

auto const nowOffset = app_.timeKeeper().nowOffset();
if (std::abs (nowOffset.count()) >= 60)
Expand Down Expand Up @@ -2454,11 +2452,11 @@ void NetworkOPsImp::pubLedger (
jvObj[jss::ledger_time]
= Json::Value::UInt (lpAccepted->info().closeTime.time_since_epoch().count());

jvObj[jss::fee_ref]
= Json::UInt (lpAccepted->fees().units);
jvObj[jss::fee_base] = Json::UInt (lpAccepted->fees().base);
jvObj[jss::reserve_base] = Json::UInt (lpAccepted->fees().accountReserve(0).drops());
jvObj[jss::reserve_inc] = Json::UInt (lpAccepted->fees().increment);
jvObj[jss::fee_ref] = lpAccepted->fees().units.json();
jvObj[jss::fee_base] = lpAccepted->fees().base.json();
jvObj[jss::reserve_base] =
lpAccepted->fees().accountReserve(0).json();
jvObj[jss::reserve_inc] = lpAccepted->fees().increment.json();

jvObj[jss::txn_count] = Json::UInt (alpAccepted->getTxnCount ());

Expand Down Expand Up @@ -2823,13 +2821,13 @@ bool NetworkOPsImp::subLedger (InfoSub::ref isrListener, Json::Value& jvResult)
{
jvResult[jss::ledger_index] = lpClosed->info().seq;
jvResult[jss::ledger_hash] = to_string (lpClosed->info().hash);
jvResult[jss::ledger_time]
= Json::Value::UInt(lpClosed->info().closeTime.time_since_epoch().count());
jvResult[jss::fee_ref]
= Json::UInt (lpClosed->fees().units);
jvResult[jss::fee_base] = Json::UInt (lpClosed->fees().base);
jvResult[jss::reserve_base] = Json::UInt (lpClosed->fees().accountReserve(0).drops());
jvResult[jss::reserve_inc] = Json::UInt (lpClosed->fees().increment);
jvResult[jss::ledger_time] = Json::Value::UInt(
lpClosed->info().closeTime.time_since_epoch().count());
jvResult[jss::fee_ref] = lpClosed->fees().units.json();
jvResult[jss::fee_base] = lpClosed->fees().base.json();
jvResult[jss::reserve_base] =
lpClosed->fees().accountReserve(0).json();
jvResult[jss::reserve_inc] = lpClosed->fees().increment.json();
}

if ((mMode >= omSYNCING) && !isNeedNetworkLedger ())
Expand Down
Loading

1 comment on commit ab9735c

@ripplelabs-jenkins
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jenkins Build Summary

Built from this commit

Built at 20190717 - 00:18:55

Test Results

Build Type Log Result Status
msvc.Debug logfile 1179 cases, 0 failed, t: 577s PASS ✅
msvc.Debug,
NINJA_BUILD=true
logfile 1179 cases, 0 failed, t: 604s PASS ✅
msvc.Debug
-Dunity=OFF
logfile 1178 cases, 0 failed, t: 1124s PASS ✅
msvc.Release logfile 1179 cases, 0 failed, t: 414s PASS ✅

Please sign in to comment.