diff --git a/src/ripple/app/misc/impl/TxQ.cpp b/src/ripple/app/misc/impl/TxQ.cpp index a911b85996..a88b681dde 100644 --- a/src/ripple/app/misc/impl/TxQ.cpp +++ b/src/ripple/app/misc/impl/TxQ.cpp @@ -86,8 +86,7 @@ TxQ::FeeMetrics::update(Application& app, feeLevels.reserve(txnsExpected_); for (auto const& tx : view.txs) { - auto const baseFee = calculateBaseFee(app, view, - *tx.first, j_); + auto const baseFee = calculateBaseFee(view, *tx.first); feeLevels.push_back(getFeeLevelPaid(*tx.first, baseLevel, baseFee, setup)); } @@ -660,7 +659,7 @@ TxQ::apply(Application& app, OpenView& view, // or transaction replacement, so just pull it up now. // TODO: Do we want to avoid doing it again during // preclaim? - auto const baseFee = calculateBaseFee(app, view, *tx, j); + auto const baseFee = calculateBaseFee(view, *tx); auto const feeLevelPaid = getFeeLevelPaid(*tx, baseLevel, baseFee, setup_); auto const requiredFeeLevel = [&]() diff --git a/src/ripple/app/tx/applySteps.h b/src/ripple/app/tx/applySteps.h index b57e89a716..1cba92d8d7 100644 --- a/src/ripple/app/tx/applySteps.h +++ b/src/ripple/app/tx/applySteps.h @@ -86,35 +86,23 @@ public: /// Intermediate transaction result TER const ter; - /// Transaction-specific base fee - std::uint64_t const baseFee; /// Success flag - whether the transaction is likely to /// claim a fee bool const likelyToClaimFee; /// Constructor template - PreclaimResult(Context const& ctx_, - TER ter_, std::uint64_t const& baseFee_) + PreclaimResult(Context const& ctx_, TER ter_) : view(ctx_.view) , tx(ctx_.tx) , flags(ctx_.flags) , j(ctx_.j) , ter(ter_) - , baseFee(baseFee_) , likelyToClaimFee(ter == tesSUCCESS || isTecClaim(ter)) { } - /// Constructor - template - PreclaimResult(Context const& ctx_, - std::pair const& result) - : PreclaimResult(ctx_, result.first, result.second) - { - } - PreclaimResult(PreclaimResult const&) = default; /// Deleted copy assignment operator PreclaimResult& operator=(PreclaimResult const&) = delete; @@ -231,16 +219,14 @@ preclaim(PreflightResult const& preflightResult, Since none should be thrown, that will usually mean terminating. - @param app The current running `Application`. @param view The current open ledger. @param tx The transaction to be checked. - @param j A journal. @return The base fee. */ std::uint64_t -calculateBaseFee(Application& app, ReadView const& view, - STTx const& tx, beast::Journal j); +calculateBaseFee(ReadView const& view, + STTx const& tx); /** Determine the XRP balance consequences if a transaction consumes the maximum XRP allowed. diff --git a/src/ripple/app/tx/impl/ApplyContext.h b/src/ripple/app/tx/impl/ApplyContext.h index 53190931db..701a5efa52 100644 --- a/src/ripple/app/tx/impl/ApplyContext.h +++ b/src/ripple/app/tx/impl/ApplyContext.h @@ -41,7 +41,7 @@ public: ApplyContext (Application& app, OpenView& base, STTx const& tx, TER preclaimResult, std::uint64_t baseFee, ApplyFlags flags, - beast::Journal = {}); + beast::Journal = {}); Application& app; STTx const& tx; diff --git a/src/ripple/app/tx/impl/Change.h b/src/ripple/app/tx/impl/Change.h index 72a475fd1f..e5d84cf1a2 100644 --- a/src/ripple/app/tx/impl/Change.h +++ b/src/ripple/app/tx/impl/Change.h @@ -48,7 +48,8 @@ public: static std::uint64_t calculateBaseFee ( - PreclaimContext const& ctx) + ReadView const& view, + STTx const& tx) { return 0; } diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 4b9de741d0..4d97b030b0 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -347,17 +347,19 @@ EscrowFinish::preflight (PreflightContext const& ctx) } std::uint64_t -EscrowFinish::calculateBaseFee (PreclaimContext const& ctx) +EscrowFinish::calculateBaseFee ( + ReadView const& view, + STTx const& tx) { std::uint64_t extraFee = 0; - if (auto const fb = ctx.tx[~sfFulfillment]) + if (auto const fb = tx[~sfFulfillment]) { - extraFee += ctx.view.fees().units * + extraFee += view.fees().units * (32 + static_cast (fb->size() / 16)); } - return Transactor::calculateBaseFee (ctx) + extraFee; + return Transactor::calculateBaseFee (view, tx) + extraFee; } TER diff --git a/src/ripple/app/tx/impl/Escrow.h b/src/ripple/app/tx/impl/Escrow.h index 45e617dafb..3c600868ae 100644 --- a/src/ripple/app/tx/impl/Escrow.h +++ b/src/ripple/app/tx/impl/Escrow.h @@ -64,7 +64,9 @@ public: static std::uint64_t - calculateBaseFee (PreclaimContext const& ctx); + calculateBaseFee ( + ReadView const& view, + STTx const& tx); TER doApply() override; diff --git a/src/ripple/app/tx/impl/SetRegularKey.cpp b/src/ripple/app/tx/impl/SetRegularKey.cpp index 5d3be92c6a..0f1b242c8a 100644 --- a/src/ripple/app/tx/impl/SetRegularKey.cpp +++ b/src/ripple/app/tx/impl/SetRegularKey.cpp @@ -25,16 +25,17 @@ namespace ripple { std::uint64_t SetRegularKey::calculateBaseFee ( - PreclaimContext const& ctx) + ReadView const& view, + STTx const& tx) { - auto const id = ctx.tx.getAccountID(sfAccount); - auto const spk = ctx.tx.getSigningPubKey(); + auto const id = tx.getAccountID(sfAccount); + auto const spk = tx.getSigningPubKey(); if (publicKeyType (makeSlice (spk))) { if (calcAccountID(PublicKey (makeSlice(spk))) == id) { - auto const sle = ctx.view.read(keylet::account(id)); + auto const sle = view.read(keylet::account(id)); if (sle && (! (sle->getFlags () & lsfPasswordSpent))) { @@ -44,7 +45,7 @@ SetRegularKey::calculateBaseFee ( } } - return Transactor::calculateBaseFee (ctx); + return Transactor::calculateBaseFee (view, tx); } NotTEC @@ -70,10 +71,10 @@ SetRegularKey::preflight (PreflightContext const& ctx) TER SetRegularKey::doApply () { - auto const sle = view().peek( - keylet::account(account_)); + auto const sle = view ().peek ( + keylet::account (account_)); - if (mFeeDue == zero) + if (!minimumFee (ctx_.app, ctx_.baseFee, view ().fees (), view ().flags ())) sle->setFlag (lsfPasswordSpent); if (ctx_.tx.isFieldPresent (sfRegularKey)) diff --git a/src/ripple/app/tx/impl/SetRegularKey.h b/src/ripple/app/tx/impl/SetRegularKey.h index 4fe493bfa0..0b51f5841c 100644 --- a/src/ripple/app/tx/impl/SetRegularKey.h +++ b/src/ripple/app/tx/impl/SetRegularKey.h @@ -50,7 +50,8 @@ public: static std::uint64_t calculateBaseFee ( - PreclaimContext const& ctx); + ReadView const& view, + STTx const& tx); TER doApply () override; }; diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 8535eea8a9..632d628805 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -102,15 +102,6 @@ preflight2 (PreflightContext const& ctx) return tesSUCCESS; } -static -XRPAmount -calculateFee(Application& app, std::uint64_t const baseFee, - Fees const& fees, ApplyFlags flags) -{ - return scaleFeeLoad(baseFee, app.getFeeTrack(), - fees, flags & tapUNLIMITED); -} - //------------------------------------------------------------------------------ PreflightContext::PreflightContext(Application& app_, STTx const& tx_, @@ -134,20 +125,21 @@ Transactor::Transactor( } std::uint64_t Transactor::calculateBaseFee ( - PreclaimContext const& ctx) + ReadView const& view, + STTx const& tx) { // Returns the fee in fee units. // The computation has two parts: // * The base fee, which is the same for most transactions. // * The additional cost of each multisignature on the transaction. - std::uint64_t baseFee = ctx.view.fees().units; + std::uint64_t baseFee = view.fees().units; // Each signer adds one more baseFee to the minimum required fee // for the transaction. std::uint32_t signerCount = 0; - if (ctx.tx.isFieldPresent (sfSigners)) - signerCount = ctx.tx.getFieldArray (sfSigners).size(); + if (tx.isFieldPresent (sfSigners)) + signerCount = tx.getFieldArray (sfSigners).size(); return baseFee + (signerCount * baseFee); } @@ -158,6 +150,14 @@ Transactor::calculateFeePaid(STTx const& tx) return tx[sfFee].xrp(); } +XRPAmount +Transactor::minimumFee (Application& app, std::uint64_t baseFee, + Fees const& fees, ApplyFlags flags) +{ + return scaleFeeLoad (baseFee, app.getFeeTrack (), + fees, flags & tapUNLIMITED); +} + XRPAmount Transactor::calculateMaxSpend(STTx const& tx) { @@ -165,13 +165,14 @@ Transactor::calculateMaxSpend(STTx const& tx) } TER -Transactor::checkFee (PreclaimContext const& ctx, std::uint64_t baseFee) +Transactor::checkFee (PreclaimContext const& ctx, + std::uint64_t baseFee) { auto const feePaid = calculateFeePaid(ctx.tx); if (!isLegalAmount (feePaid) || feePaid < beast::zero) return temBAD_FEE; - auto const feeDue = ripple::calculateFee(ctx.app, + auto const feeDue = minimumFee(ctx.app, baseFee, ctx.view.fees(), ctx.flags); // Only check fee is sufficient when the ledger is open. @@ -307,9 +308,6 @@ TER Transactor::apply () // that allow zero account. assert(sle != nullptr || account_ == zero); - mFeeDue = calculateFee(ctx_.app, ctx_.baseFee, - view().fees(), view().flags()); - if (sle) { mPriorBalance = STAmount ((*sle)[sfBalance]).xrp (); diff --git a/src/ripple/app/tx/impl/Transactor.h b/src/ripple/app/tx/impl/Transactor.h index 15ee64223e..59a488be2f 100644 --- a/src/ripple/app/tx/impl/Transactor.h +++ b/src/ripple/app/tx/impl/Transactor.h @@ -81,7 +81,6 @@ protected: beast::Journal j_; AccountID account_; - XRPAmount mFeeDue; XRPAmount mPriorBalance; // Balance before fees. XRPAmount mSourceBalance; // Balance after fees. @@ -132,7 +131,8 @@ public: static std::uint64_t calculateBaseFee ( - PreclaimContext const& ctx); + ReadView const& view, + STTx const& tx); static bool @@ -170,6 +170,20 @@ protected: virtual TER doApply () = 0; + /** Compute the minimum fee required to process a transaction + with a given baseFee based on the current server load. + + @param app The application hosting the server + @param baseFee The base fee of a candidate transaction + @see ripple::calculateBaseFee + @param fees Fee settings from the current ledger + @param flags Transaction processing fees + */ + static + XRPAmount + minimumFee (Application& app, std::uint64_t baseFee, + Fees const& fees, ApplyFlags flags); + private: XRPAmount reset(XRPAmount fee); diff --git a/src/ripple/app/tx/impl/applySteps.cpp b/src/ripple/app/tx/impl/applySteps.cpp index 0f8051ecf9..bedc3a48d4 100644 --- a/src/ripple/app/tx/impl/applySteps.cpp +++ b/src/ripple/app/tx/impl/applySteps.cpp @@ -77,38 +77,38 @@ invoke_preflight (PreflightContext const& ctx) */ template static -std::pair +TER invoke_preclaim(PreclaimContext const& ctx) { // If the transactor requires a valid account and the transaction doesn't // list one, preflight will have already a flagged a failure. auto const id = ctx.tx.getAccountID(sfAccount); - auto const baseFee = T::calculateBaseFee(ctx); if (id != zero) { TER result = T::checkSeq(ctx); if (result != tesSUCCESS) - return { result, baseFee }; + return result; - result = T::checkFee(ctx, baseFee); + result = T::checkFee(ctx, + calculateBaseFee(ctx.view, ctx.tx)); if (result != tesSUCCESS) - return { result, baseFee }; + return result; result = T::checkSign(ctx); if (result != tesSUCCESS) - return { result, baseFee }; + return result; } - return{ T::preclaim(ctx), baseFee }; + return T::preclaim(ctx); } static -std::pair +TER invoke_preclaim (PreclaimContext const& ctx) { switch(ctx.tx.getTxnType()) @@ -136,37 +136,39 @@ invoke_preclaim (PreclaimContext const& ctx) case ttFEE: return invoke_preclaim(ctx); default: assert(false); - return { temUNKNOWN, 0 }; + return temUNKNOWN; } } static std::uint64_t -invoke_calculateBaseFee(PreclaimContext const& ctx) +invoke_calculateBaseFee( + ReadView const& view, + STTx const& tx) { - switch (ctx.tx.getTxnType()) + switch (tx.getTxnType()) { - case ttACCOUNT_SET: return SetAccount::calculateBaseFee(ctx); - case ttCHECK_CANCEL: return CancelCheck::calculateBaseFee(ctx); - case ttCHECK_CASH: return CashCheck::calculateBaseFee(ctx); - case ttCHECK_CREATE: return CreateCheck::calculateBaseFee(ctx); - case ttDEPOSIT_PREAUTH: return DepositPreauth::calculateBaseFee(ctx); - case ttOFFER_CANCEL: return CancelOffer::calculateBaseFee(ctx); - case ttOFFER_CREATE: return CreateOffer::calculateBaseFee(ctx); - case ttESCROW_CREATE: return EscrowCreate::calculateBaseFee(ctx); - case ttESCROW_FINISH: return EscrowFinish::calculateBaseFee(ctx); - case ttESCROW_CANCEL: return EscrowCancel::calculateBaseFee(ctx); - case ttPAYCHAN_CLAIM: return PayChanClaim::calculateBaseFee(ctx); - case ttPAYCHAN_CREATE: return PayChanCreate::calculateBaseFee(ctx); - case ttPAYCHAN_FUND: return PayChanFund::calculateBaseFee(ctx); - case ttPAYMENT: return Payment::calculateBaseFee(ctx); - case ttREGULAR_KEY_SET: return SetRegularKey::calculateBaseFee(ctx); - case ttSIGNER_LIST_SET: return SetSignerList::calculateBaseFee(ctx); - case ttTICKET_CANCEL: return CancelTicket::calculateBaseFee(ctx); - case ttTICKET_CREATE: return CreateTicket::calculateBaseFee(ctx); - case ttTRUST_SET: return SetTrust::calculateBaseFee(ctx); + case ttACCOUNT_SET: return SetAccount::calculateBaseFee(view, tx); + case ttCHECK_CANCEL: return CancelCheck::calculateBaseFee(view, tx); + case ttCHECK_CASH: return CashCheck::calculateBaseFee(view, tx); + case ttCHECK_CREATE: return CreateCheck::calculateBaseFee(view, tx); + case ttDEPOSIT_PREAUTH: return DepositPreauth::calculateBaseFee(view, tx); + case ttOFFER_CANCEL: return CancelOffer::calculateBaseFee(view, tx); + case ttOFFER_CREATE: return CreateOffer::calculateBaseFee(view, tx); + case ttESCROW_CREATE: return EscrowCreate::calculateBaseFee(view, tx); + case ttESCROW_FINISH: return EscrowFinish::calculateBaseFee(view, tx); + case ttESCROW_CANCEL: return EscrowCancel::calculateBaseFee(view, tx); + case ttPAYCHAN_CLAIM: return PayChanClaim::calculateBaseFee(view, tx); + case ttPAYCHAN_CREATE: return PayChanCreate::calculateBaseFee(view, tx); + case ttPAYCHAN_FUND: return PayChanFund::calculateBaseFee(view, tx); + case ttPAYMENT: return Payment::calculateBaseFee(view, tx); + case ttREGULAR_KEY_SET: return SetRegularKey::calculateBaseFee(view, tx); + case ttSIGNER_LIST_SET: return SetSignerList::calculateBaseFee(view, tx); + case ttTICKET_CANCEL: return CancelTicket::calculateBaseFee(view, tx); + case ttTICKET_CREATE: return CreateTicket::calculateBaseFee(view, tx); + case ttTRUST_SET: return SetTrust::calculateBaseFee(view, tx); case ttAMENDMENT: - case ttFEE: return Change::calculateBaseFee(ctx); + case ttFEE: return Change::calculateBaseFee(view, tx); default: assert(false); return 0; @@ -295,26 +297,22 @@ preclaim (PreflightResult const& preflightResult, try { if (ctx->preflightResult != tesSUCCESS) - return { *ctx, ctx->preflightResult, 0 }; + return { *ctx, ctx->preflightResult }; return{ *ctx, invoke_preclaim(*ctx) }; } catch (std::exception const& e) { JLOG(ctx->j.fatal()) << "apply: " << e.what(); - return{ *ctx, tefEXCEPTION, 0 }; + return{ *ctx, tefEXCEPTION }; } } std::uint64_t -calculateBaseFee(Application& app, ReadView const& view, - STTx const& tx, beast::Journal j) +calculateBaseFee(ReadView const& view, + STTx const& tx) { - PreclaimContext const ctx( - app, view, tesSUCCESS, tx, - tapNONE, j); - - return invoke_calculateBaseFee(ctx); + return invoke_calculateBaseFee(view, tx); } TxConsequences @@ -344,8 +342,8 @@ doApply(PreclaimResult const& preclaimResult, return{ preclaimResult.ter, false }; ApplyContext ctx(app, view, preclaimResult.tx, preclaimResult.ter, - preclaimResult.baseFee, preclaimResult.flags, - preclaimResult.j); + calculateBaseFee(view, preclaimResult.tx), + preclaimResult.flags, preclaimResult.j); return invoke_apply(ctx); } catch (std::exception const& e)