Improve transaction fee and execution logic (RIPD-323):

* tecINSUFF_FEE if balance doesn't cover fee
* Ensure transaction recovery is deterministic
* Reduce transaction retries

src/ripple/module/app/consensus/DisputedTx.h

@@ -82,8 +82,13 @@ private:
     ripple::unordered_map <NodeID, bool> mVotes;
 };
 
-#define LEDGER_TOTAL_PASSES 8
-#define LEDGER_RETRY_PASSES 5
+// How many total extra passes we make
+// We must ensure we make at least one non-retriable pass
+#define LEDGER_TOTAL_PASSES 3
+
+// How many extra retry passes we
+// make if the previous retry pass made changes
+#define LEDGER_RETRY_PASSES 1
 
 } // ripple
 

src/ripple/module/app/consensus/LedgerConsensus.cpp

@@ -972,6 +972,7 @@ private:
 
             // Apply disputed transactions that didn't get in
             TransactionEngine engine (newOL);
+            bool anyDisputes = false;
             for (auto& it : mDisputes)
             {
                 if (!it.second->getOurVote ())
@@ -986,10 +987,8 @@ private:
                         SerializedTransaction::pointer txn
                             = std::make_shared<SerializedTransaction>(sit);
 
-                        if (applyTransaction (engine, txn, newOL, true, false))
-                        {
-                            failedTransactions.push_back (txn);
-                        }
+                        failedTransactions.push_back (txn);
+                        anyDisputes = true;
                     }
                     catch (...)
                     {
@@ -998,12 +997,22 @@ private:
                     }
                 }
             }
+            if (anyDisputes)
+            {
+                applyTransactions (std::shared_ptr<SHAMap>(),
+                    newOL, newLCL, failedTransactions, true);
+            }
 
-            WriteLog (lsDEBUG, LedgerConsensus)
-                << "Applying transactions from current open ledger";
-            applyTransactions (getApp().getLedgerMaster ().getCurrentLedger
-                ()->peekTransactionMap (), newOL, newLCL,
-                failedTransactions, true);
+	    {
+	        Ledger::pointer oldOL = getApp().getLedgerMaster().getCurrentLedger();
+	        if (oldOL->peekTransactionMap()->getHash().isNonZero ())
+	        {
+                    WriteLog (lsDEBUG, LedgerConsensus)
+                        << "Applying transactions from current open ledger";
+                    applyTransactions (oldOL->peekTransactionMap (),
+                        newOL, newLCL, failedTransactions, true);
+                }
+	    }
 
             {
                 TransactionEngine engine (newOL);
@@ -1252,6 +1261,7 @@ private:
     {
         TransactionEngine engine (applyLedger);
 
+        if (set)
         for (SHAMapItem::pointer item = set->peekFirstItem (); !!item;
             item = set->peekNextItem (item->getTag ()))
             if (!checkLedger->hasTransaction (item->getTag ()))
@@ -1334,6 +1344,10 @@ private:
             if ((!changes) || (pass >= LEDGER_RETRY_PASSES))
                 certainRetry = false;
         }
+
+        // If there are any transactions left, we must have
+        // tried them in at least one final pass
+        assert (failedTransactions.empty() || !certainRetry);
     }
 
     /** Apply a transaction to a ledger

src/ripple/module/app/transactors/Transactor.cpp

@@ -111,19 +111,29 @@ TER Transactor::payFee ()
     if (saPaid < zero || !saPaid.isNative ())
         return temBAD_FEE;
 
-    if (!saPaid) return tesSUCCESS;
+    if (!saPaid)
+        return tesSUCCESS;
 
-    // Deduct the fee, so it's not available during the transaction.
-    // Will only write the account back, if the transaction succeeds.
     if (mSourceBalance < saPaid)
     {
         m_journal.trace << "Insufficient balance:" <<
             " balance=" << mSourceBalance.getText () <<
             " paid=" << saPaid.getText ();
 
+        if ((mSourceBalance > zero) && (!(mParams & tapOPEN_LEDGER)))
+        {
+            // Closed ledger, non-zero balance, less than fee
+            mSourceBalance.clear ();
+            mTxnAccount->setFieldAmount (sfBalance, mSourceBalance);
+            return tecINSUFF_FEE;
+        }
+
         return terINSUF_FEE_B;
     }
 
+    // Deduct the fee, so it's not available during the transaction.
+    // Will only write the account back, if the transaction succeeds.
+
     mSourceBalance -= saPaid;
     mTxnAccount->setFieldAmount (sfBalance, mSourceBalance);
 

src/ripple/module/app/tx/TransactionEngine.cpp

@@ -149,10 +149,19 @@ TER TransactionEngine::applyTransaction (const SerializedTransaction& txn, Trans
                 STAmount fee        = txn.getTransactionFee ();
                 STAmount balance    = txnAcct->getFieldAmount (sfBalance);
 
-                if (balance < fee)
+                // We retry/reject the transaction if the account
+                // balance is zero or we're applying against an open
+                // ledger and the balance is less than the fee
+                if ((balance == zero) ||
+                    ((params & tapOPEN_LEDGER) && (balance < fee)))
+                {
+                    // Account has no funds or ledger is open
                     terResult = terINSUF_FEE_B;
+                }
                 else
                 {
+                    if (fee > balance)
+                        fee = balance;
                     txnAcct->setFieldAmount (sfBalance, balance - fee);
                     txnAcct->setFieldU32 (sfSequence, t_seq + 1);
                     entryModify (txnAcct);

src/ripple/module/data/protocol/TER.cpp

@@ -50,6 +50,7 @@ bool transResultInfo (TER terCode, std::string& strToken, std::string& strHuman)
         {   tecNO_ISSUER,           "tecNO_ISSUER",             "Issuer account does not exist."                        },
         {   tecNO_AUTH,             "tecNO_AUTH",               "Not authorized to hold asset."                         },
         {   tecNO_LINE,             "tecNO_LINE",               "No such line."                                         },
+        {   tecINSUFF_FEE,          "tecINSUFF_FEE",            "Insufficient balance to pay fee."                      },
 
         {   tefALREADY,             "tefALREADY",               "The exact transaction was already in this ledger."     },
         {   tefBAD_ADD_AUTH,        "tefBAD_ADD_AUTH",          "Not authorized to add account."                        },

src/ripple/module/data/protocol/TER.h

@@ -183,6 +183,7 @@ enum TER    // aka TransactionEngineResult
     tecNO_ISSUER                = 133,
     tecNO_AUTH                  = 134,
     tecNO_LINE                  = 135,
+    tecINSUFF_FEE               = 136,
 };
 
 inline bool isTelLocal(TER x)