Give a transaction more chances to be retried:

* Hold if the transaction gets a ter, tel, or tef result.
* Use the new SF_HELD flag to ultimately prevent the transaction from
  being held and retried too many times.

Author: ximinez

src/ripple/app/ledger/LocalTxs.h

@@ -33,6 +33,11 @@ namespace ripple {
 class LocalTxs
 {
 public:
+    // The number of ledgers to hold a transaction is essentially
+    // arbitrary. It should be sufficient to allow the transaction to
+    // get into a fully-validated ledger.
+    static constexpr int holdLedgers = 5;
+
     virtual ~LocalTxs() = default;
 
     // Add a new local transaction

src/ripple/app/ledger/impl/LocalTxs.cpp

@@ -53,14 +53,9 @@ namespace ripple {
 class LocalTx
 {
 public:
-    // The number of ledgers to hold a transaction is essentially
-    // arbitrary. It should be sufficient to allow the transaction to
-    // get into a fully-validated ledger.
-    static int const holdLedgers = 5;
-
     LocalTx(LedgerIndex index, std::shared_ptr<STTx const> const& txn)
         : m_txn(txn)
-        , m_expire(index + holdLedgers)
+        , m_expire(index + LocalTxs::holdLedgers)
         , m_id(txn->getTransactionID())
         , m_account(txn->getAccountID(sfAccount))
         , m_seqProxy(txn->getSeqProxy())

src/ripple/app/misc/HashRouter.h

@@ -33,6 +33,7 @@ namespace ripple {
 // TODO convert these macros to int constants or an enum
 #define SF_BAD 0x02  // Temporarily bad
 #define SF_SAVED 0x04
+#define SF_HELD 0x08    // Held by LedgerMaster after potential processing failure
 #define SF_TRUSTED 0x10  // comes from trusted source
 
 // Private flags, used internally in apply.cpp.

src/ripple/app/misc/NetworkOPs.cpp

@@ -1432,16 +1432,54 @@ NetworkOPsImp::apply(std::unique_lock<std::mutex>& batchLock)
                 e.transaction->setQueued();
                 e.transaction->setKept();
             }
-            else if (isTerRetry(e.result))
+            else if (
+                isTerRetry(e.result) || isTelLocal(e.result) ||
+                isTefFailure(e.result))
             {
                 if (e.failType != FailHard::yes)
                 {
-                    // transaction should be held
-                    JLOG(m_journal.debug())
-                        << "Transaction should be held: " << e.result;
-                    e.transaction->setStatus(HELD);
-                    m_ledgerMaster.addHeldTransaction(e.transaction);
-                    e.transaction->setKept();
+                    auto const lastLedgerSeq =
+                        e.transaction->getSTransaction()->at(
+                            ~sfLastLedgerSequence);
+                    auto const ledgersLeft = lastLedgerSeq
+                        ? *lastLedgerSeq -
+                            m_ledgerMaster.getCurrentLedgerIndex()
+                        : std::optional<LedgerIndex>{};
+                    // If any of these conditions are met, the transaction can
+                    // be held:
+                    // 1. It was submitted locally. (Note that this flag is only
+                    //    true on the initial submission.)
+                    // 2. The transaction has a LastLedgerSequence, and the
+                    //    LastLedgerSequence is fewer than LocalTxs::holdLedgers
+                    //    (5) ledgers into the future. (Remember that an
+                    //    unseated optional compares as less than all seated
+                    //    values, so it has to be checked explicitly first.)
+                    // 3. The SF_HELD flag is not set on the txID. (setFlags
+                    //    checks before setting. If the flag is set, it returns
+                    //    false, which means it's been held once without one of
+                    //    the other conditions, so don't hold it again. Time's
+                    //    up!)
+                    //
+                    if (e.local ||
+                        (ledgersLeft && ledgersLeft <= LocalTxs::holdLedgers) ||
+                        app_.getHashRouter().setFlags(
+                            e.transaction->getID(), SF_HELD))
+                    {
+                        // transaction should be held
+                        JLOG(m_journal.debug())
+                            << "Transaction should be held: " << e.result;
+                        e.transaction->setStatus(HELD);
+                        m_ledgerMaster.addHeldTransaction(e.transaction);
+                        e.transaction->setKept();
+                    }
+                    else
+                        JLOG(m_journal.debug())
+                            << "Not holding transaction "
+                            << e.transaction->getID() << ": "
+                            << (e.local ? "local" : "network") << ", "
+                            << "result: " << e.result << " ledgers left: "
+                            << (ledgersLeft ? to_string(*ledgersLeft)
+                                            : "unspecified");
                 }
             }
             else