Reduce Transactor virtual interface

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

@@ -34,17 +34,29 @@ Change::preflight (PreflightContext const& ctx)
     auto account = ctx.tx.getAccountID(sfAccount);
     if (account != zero)
     {
-        JLOG(ctx.j.warning) << "Bad source id";
+        JLOG(ctx.j.warning) << "Change: Bad source id";
         return temBAD_SRC_ACCOUNT;
     }
 
     auto const fee = ctx.tx.getTransactionFee ();
     if (!fee.native () || fee != beast::zero)
     {
-        JLOG(ctx.j.warning) << "Non-zero fee";
+        JLOG(ctx.j.warning) << "Change: Non-zero fee";
         return temBAD_FEE;
     }
 
+    if (!ctx.tx.getSigningPubKey ().empty () || !ctx.tx.getSignature ().empty ())
+    {
+        JLOG(ctx.j.warning) << "Change: Bad signature";
+        return temBAD_SIGNATURE;
+    }
+
+    if (ctx.tx.getSequence () != 0 || ctx.tx.isFieldPresent (sfPreviousTxnID))
+    {
+        JLOG(ctx.j.warning) << "Change: Bad sequence";
+        return temBAD_SEQUENCE;
+    }
+
     return tesSUCCESS;
 }
 
@@ -68,42 +80,6 @@ Change::doApply()
     return temUNKNOWN;
 }
 
-TER
-Change::checkSign()
-{
-    if (!tx().getSigningPubKey ().empty () || !tx().getSignature ().empty ())
-    {
-        j_.warning << "Bad signature";
-        return temBAD_SIGNATURE;
-    }
-
-    return tesSUCCESS;
-}
-
-TER
-Change::checkSeq()
-{
-    if ((tx().getSequence () != 0) || tx().isFieldPresent (sfPreviousTxnID))
-    {
-        j_.warning << "Bad sequence";
-        return temBAD_SEQUENCE;
-    }
-
-    return tesSUCCESS;
-}
-
-TER
-Change::payFee()
-{
-    if (tx().getTransactionFee () != beast::zero)
-    {
-        j_.warning << "Non-zero fee";
-        return temBAD_FEE;
-    }
-
-    return tesSUCCESS;
-}
-
 void
 Change::preCompute()
 {

src/ripple/app/tx/impl/Change.h

@@ -43,9 +43,6 @@ public:
     preflight (PreflightContext const& ctx);
 
     TER doApply () override;
-    TER checkSign () override;
-    TER checkSeq () override;
-    TER payFee () override;
     void preCompute() override;
 
 private:
@@ -53,10 +50,9 @@ private:
 
     TER applyFee ();
 
-    // VFALCO TODO Can this be removed?
-    bool mustHaveValidAccount () override
+    std::uint64_t calculateBaseFee () override
     {
-        return false;
+        return 0;
     }
 };
 

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

@@ -96,13 +96,6 @@ Transactor::Transactor(
 {
 }
 
-void Transactor::calculateFee ()
-{
-    mFeeDue = STAmount (getApp().getFeeTrack().scaleFeeLoad(
-        calculateBaseFee(), view().fees().base,
-            view().fees().units, view().flags() & tapADMIN));
-}
-
 std::uint64_t Transactor::calculateBaseFee ()
 {
     // Returns the fee in fee units
@@ -113,22 +106,18 @@ TER Transactor::payFee ()
 {
     STAmount saPaid = tx().getTransactionFee ();
 
-    if (!isLegalNet (saPaid))
-        return temBAD_AMOUNT;
+    if (!isLegalNet (saPaid) || saPaid < zero)
+        return temBAD_FEE;
 
     // Only check fee is sufficient when the ledger is open.
     if (view().open() && saPaid < mFeeDue)
     {
         JLOG(j_.trace) << "Insufficient fee paid: " <<
             saPaid.getText () << "/" << mFeeDue.getText ();
-
         return telINSUF_FEE_P;
     }
 
-    if (saPaid < zero || !saPaid.native ())
-        return temBAD_FEE;
-
-    if (!saPaid)
+    if (saPaid == zero)
         return tesSUCCESS;
 
     auto const sle = view().peek(
@@ -216,44 +205,41 @@ TER Transactor::apply ()
 {
     preCompute();
 
-    // Find source account
+    // If the transactor requires a valid account and the transaction doesn't
+    // list one, preflight will have already a flagged a failure.
     auto const sle = view().peek (keylet::account(account_));
 
-    calculateFee ();
-
-    // If are only forwarding, due to resource limitations, we might verifying
-    // only some transactions, this would be probabilistic.
-    if (!sle)
+    if (sle == nullptr && account_ != zero)
     {
-        if (mustHaveValidAccount ())
-        {
-            JLOG(j_.trace) <<
-                "applyTransaction: delay: source account does not exist " <<
-                toBase58(tx().getAccountID(sfAccount));
-            return terNO_ACCOUNT;
-        }
+        JLOG (j_.trace) <<
+            "apply: source account " << toBase58(account_) << " not found.";
+        return terNO_ACCOUNT;
     }
-    else
+
+    mFeeDue = STAmount (getApp().getFeeTrack().scaleFeeLoad(
+        calculateBaseFee(), view().fees().base,
+            view().fees().units, view().flags() & tapADMIN));
+
+    if (sle)
     {
         mPriorBalance   = sle->getFieldAmount (sfBalance);
         mSourceBalance  = mPriorBalance;
         mHasAuthKey     = sle->isFieldPresent (sfRegularKey);
-    }
 
-    auto terResult = checkSeq ();
+        auto terResult = checkSeq ();
 
-    if (terResult != tesSUCCESS) return terResult;
+        if (terResult != tesSUCCESS) return terResult;
 
-    terResult = payFee ();
+        terResult = payFee ();
 
-    if (terResult != tesSUCCESS) return terResult;
+        if (terResult != tesSUCCESS) return terResult;
 
-    terResult = checkSign ();
+        terResult = checkSign ();
 
-    if (terResult != tesSUCCESS) return terResult;
+        if (terResult != tesSUCCESS) return terResult;
 
-    if (sle)
         view().update (sle);
+    }
 
     return doApply ();
 }

src/ripple/app/tx/impl/Transactor.h

@@ -94,28 +94,17 @@ protected:
     explicit
     Transactor (ApplyContext& ctx);
 
-    void calculateFee ();
-
-    // VFALCO This is the equivalent of dynamic_cast
-    //        to discover the type of the derived class,
-    //        and therefore bad.
-    virtual
-    bool
-    mustHaveValidAccount()
-    {
-        return true;
-    }
-
     // Returns the fee, not scaled for load (Should be in fee units. FIXME)
     virtual std::uint64_t calculateBaseFee ();
 
     virtual void preCompute();
-    virtual TER checkSeq ();
-    virtual TER payFee ();
-    virtual TER checkSign ();
+
     virtual TER doApply () = 0;
 
 private:
+    TER checkSeq ();
+    TER checkSign ();
+    TER payFee ();
     TER checkSingleSign ();
     TER checkMultiSign ();
 };