Remove Transactor::mFeeDue member variable

* mFeeDue is only used in one place by one derived class, so
  only compute it as a local in that function.
* The baseFee needs to be calculated outside of the Transactor class
  because, it can change during transaction processing, and the function
  is static, so we need to be sure to call the right version
* Rename Transactor::calculateFee to minimumFee

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 = [&]()

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<class Context>
-    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<class Context>
-    PreclaimResult(Context const& ctx_,
-        std::pair<TER, std::uint64_t> 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.

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;

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;
     }

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<std::uint64_t> (fb->size() / 16));
     }
 
-    return Transactor::calculateBaseFee (ctx) + extraFee;
+    return Transactor::calculateBaseFee (view, tx) + extraFee;
 }
 
 TER

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;

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))

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;
 };

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 ();

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);
 

src/ripple/app/tx/impl/applySteps.cpp

@@ -77,38 +77,38 @@ invoke_preflight (PreflightContext const& ctx)
 */
 template<class T>
 static
-std::pair<TER, std::uint64_t>
+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, std::uint64_t>
+TER
 invoke_preclaim (PreclaimContext const& ctx)
 {
     switch(ctx.tx.getTxnType())
@@ -136,37 +136,39 @@ invoke_preclaim (PreclaimContext const& ctx)
     case ttFEE:             return invoke_preclaim<Change>(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)