From b651e0146dc1fb91a3c5cafff8a8e71c3a11b407 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 1 Oct 2014 11:35:05 -0700 Subject: [PATCH] Fix some fee logic: (RIPD-614) * fee_default sets cost in drops of reference transaction * Offline signing uses fee_default * Signing multiplier maximum works correctly * Fix bugs in load fee track * Remove dead code, add comments --- doc/rippled-example.cfg | 7 +++++++ src/ripple/app/ledger/Ledger.cpp | 3 ++- src/ripple/app/ledger/Ledger.h | 8 +++++--- src/ripple/app/transactors/Transactor.cpp | 3 ++- src/ripple/core/impl/Config.cpp | 5 ++++- src/ripple/core/impl/LoadFeeTrackImp.h | 10 +++++----- src/ripple/rpc/impl/TransactionSign.cpp | 16 +++------------- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/doc/rippled-example.cfg b/doc/rippled-example.cfg index 44e43dc382..e92d2ffc52 100644 --- a/doc/rippled-example.cfg +++ b/doc/rippled-example.cfg @@ -602,6 +602,13 @@ # # # +# [fee_default] +# +# Sets the base cost of a transaction in drops. Used when the server has +# no other source of fee information, such as signing transactions offline. +# +# +# #------------------------------------------------------------------------------- # # 6. HTTPS Client diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 66091876e1..84a50aa5a5 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -2018,7 +2018,7 @@ void Ledger::updateFees () if (mBaseFee) return; std::uint64_t baseFee = getConfig ().FEE_DEFAULT; - std::uint32_t referenceFeeUnits = 10; + std::uint32_t referenceFeeUnits = getConfig ().TRANSACTION_FEE_BASE; std::uint32_t reserveBase = getConfig ().FEE_ACCOUNT_RESERVE; std::int64_t reserveIncrement = getConfig ().FEE_OWNER_RESERVE; @@ -2054,6 +2054,7 @@ void Ledger::updateFees () std::uint64_t Ledger::scaleFeeBase (std::uint64_t fee) { + // Converts a fee in fee units to a fee in drops updateFees (); return getApp().getFeeTrack ().scaleFeeBase ( fee, mBaseFee, mReferenceFeeUnits); diff --git a/src/ripple/app/ledger/Ledger.h b/src/ripple/app/ledger/Ledger.h index 22de78e09d..fc52655e8b 100644 --- a/src/ripple/app/ledger/Ledger.h +++ b/src/ripple/app/ledger/Ledger.h @@ -451,22 +451,24 @@ public: std::uint32_t getReferenceFeeUnits () { + // Returns the cost of the reference transaction in fee units updateFees (); return mReferenceFeeUnits; } std::uint64_t getBaseFee () { + // Returns the cost of the reference transaction in drops updateFees (); return mBaseFee; } std::uint64_t getReserve (int increments) { + // Returns the required reserve in drops updateFees (); - return scaleFeeBase ( - static_cast (increments) * mReserveIncrement - + mReserveBase); + return static_cast (increments) * mReserveIncrement + + mReserveBase; } std::uint64_t getReserveInc () diff --git a/src/ripple/app/transactors/Transactor.cpp b/src/ripple/app/transactors/Transactor.cpp index db72df1502..dae445062a 100644 --- a/src/ripple/app/transactors/Transactor.cpp +++ b/src/ripple/app/transactors/Transactor.cpp @@ -99,7 +99,8 @@ void Transactor::calculateFee () std::uint64_t Transactor::calculateBaseFee () { - return getConfig ().FEE_DEFAULT; + // Returns the fee in fee units + return getConfig ().TRANSACTION_FEE_BASE; } TER Transactor::payFee () diff --git a/src/ripple/core/impl/Config.cpp b/src/ripple/core/impl/Config.cpp index 71a9a957fc..c33c4f1c98 100644 --- a/src/ripple/core/impl/Config.cpp +++ b/src/ripple/core/impl/Config.cpp @@ -49,6 +49,9 @@ namespace ripple { #define DEFAULT_FEE_OFFER DEFAULT_FEE_DEFAULT #define DEFAULT_FEE_OPERATION 1 +// Fee in fee units +#define DEFAULT_TRANSACTION_FEE_BASE 10 + #define SECTION_DEFAULT_NAME "" IniFileSections @@ -323,7 +326,7 @@ Config::Config () PEER_PRIVATE = false; PEERS_MAX = 0; // indicates "use default" - TRANSACTION_FEE_BASE = DEFAULT_FEE_DEFAULT; + TRANSACTION_FEE_BASE = DEFAULT_TRANSACTION_FEE_BASE; NETWORK_QUORUM = 0; // Don't need to see other nodes VALIDATION_QUORUM = 1; // Only need one node to vouch diff --git a/src/ripple/core/impl/LoadFeeTrackImp.h b/src/ripple/core/impl/LoadFeeTrackImp.h index a9ec56e2b3..ccb28f02a9 100644 --- a/src/ripple/core/impl/LoadFeeTrackImp.h +++ b/src/ripple/core/impl/LoadFeeTrackImp.h @@ -46,9 +46,9 @@ public: bool big = (fee > midrange); if (big) // big fee, divide first to avoid overflow - fee /= baseFee; + fee /= referenceFeeUnits; else // normal fee, multiply first for accuracy - fee *= referenceFeeUnits; + fee *= baseFee; std::uint32_t feeFactor = std::max (mLocalTxnLoadFee, mRemoteTxnLoadFee); @@ -63,9 +63,9 @@ public: } if (big) // Fee was big to start, must now multiply - fee *= referenceFeeUnits; + fee *= baseFee; else // Fee was small to start, mst now divide - fee /= baseFee; + fee /= referenceFeeUnits; return fee; } @@ -73,7 +73,7 @@ public: // Scale from fee units to millionths of a ripple std::uint64_t scaleFeeBase (std::uint64_t fee, std::uint64_t baseFee, std::uint32_t referenceFeeUnits) { - return mulDiv (fee, referenceFeeUnits, baseFee); + return mulDiv (fee, baseFee, referenceFeeUnits); } std::uint32_t getRemoteFee () diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 280a066cf5..92483f319f 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -73,11 +73,12 @@ static void autofill_fee ( } } - std::uint64_t const feeDefault = getConfig().FEE_DEFAULT; + // Default fee in fee units + std::uint64_t const feeDefault = getConfig().TRANSACTION_FEE_BASE; // Administrative endpoints are exempt from local fees std::uint64_t const fee = ledger->scaleFeeLoad (feeDefault, admin); - std::uint64_t const limit = mult * feeDefault; + std::uint64_t const limit = mult * ledger->scaleFeeBase (feeDefault); if (fee > limit) { @@ -280,17 +281,6 @@ Json::Value transactionSign ( return e; } - if (!tx_json.isMember ("Fee")) { - auto const& transactionType = tx_json["TransactionType"].asString (); - if ("AccountSet" == transactionType - || "OfferCreate" == transactionType - || "OfferCancel" == transactionType - || "TrustSet" == transactionType) - { - tx_json["Fee"] = (int) getConfig ().FEE_DEFAULT; - } - } - if (!tx_json.isMember ("Sequence")) tx_json["Sequence"] = asSrc->getSeq ();