fix: Skip processing transaction batch if the batch is empty (#5670)

Avoids an assertion failure in NetworkOPsImp::apply in the unlikely event that all incoming transactions are invalid.

Author: a1q123456

src/test/app/NetworkOPs_test.cpp

@@ -0,0 +1,80 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2020 Dev Null Productions
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#include <test/jtx.h>
+#include <test/jtx/CaptureLogs.h>
+#include <test/jtx/Env.h>
+
+#include <xrpld/app/misc/HashRouter.h>
+
+namespace ripple {
+namespace test {
+
+class NetworkOPs_test : public beast::unit_test::suite
+{
+public:
+    void
+    run() override
+    {
+        testAllBadHeldTransactions();
+    }
+
+    void
+    testAllBadHeldTransactions()
+    {
+        // All trasactions are already marked as SF_BAD, and we should be able
+        // to handle the case properly without an assertion failure
+        testcase("No valid transactions in batch");
+
+        std::string logs;
+
+        {
+            using namespace jtx;
+            auto const alice = Account{"alice"};
+            Env env{
+                *this,
+                envconfig(),
+                std::make_unique<CaptureLogs>(&logs),
+                beast::severities::kAll};
+            env.memoize(env.master);
+            env.memoize(alice);
+
+            auto const jtx = env.jt(ticket::create(alice, 1), seq(1), fee(10));
+
+            auto transacionId = jtx.stx->getTransactionID();
+            env.app().getHashRouter().setFlags(
+                transacionId, HashRouterFlags::HELD);
+
+            env(jtx, json(jss::Sequence, 1), ter(terNO_ACCOUNT));
+
+            env.app().getHashRouter().setFlags(
+                transacionId, HashRouterFlags::BAD);
+
+            env.close();
+        }
+
+        BEAST_EXPECT(
+            logs.find("No transaction to process!") != std::string::npos);
+    }
+};
+
+BEAST_DEFINE_TESTSUITE(NetworkOPs, app, ripple);
+
+}  // namespace test
+}  // namespace ripple

src/xrpld/app/misc/NetworkOPs.cpp

@@ -1448,6 +1448,11 @@ NetworkOPsImp::processTransactionSet(CanonicalTXSet const& set)
         for (auto& t : transactions)
             mTransactions.push_back(std::move(t));
     }
+    if (mTransactions.empty())
+    {
+        JLOG(m_journal.debug()) << "No transaction to process!";
+        return;
+    }
 
     doTransactionSyncBatch(lock, [&](std::unique_lock<std::mutex> const&) {
         XRPL_ASSERT(

src/xrpld/overlay/detail/PeerImp.cpp

@@ -1286,8 +1286,7 @@ PeerImp::handleTransaction(
 
         // Charge strongly for attempting to relay a txn with tfInnerBatchTxn
         // LCOV_EXCL_START
-        if (stx->isFlag(tfInnerBatchTxn) &&
-            getCurrentTransactionRules()->enabled(featureBatch))
+        if (stx->isFlag(tfInnerBatchTxn))
         {
             JLOG(p_journal_.warn()) << "Ignoring Network relayed Tx containing "
                                        "tfInnerBatchTxn (handleTransaction).";
@@ -2851,8 +2850,7 @@ PeerImp::checkTransaction(
     {
         // charge strongly for relaying batch txns
         // LCOV_EXCL_START
-        if (stx->isFlag(tfInnerBatchTxn) &&
-            getCurrentTransactionRules()->enabled(featureBatch))
+        if (stx->isFlag(tfInnerBatchTxn))
         {
             JLOG(p_journal_.warn()) << "Ignoring Network relayed Tx containing "
                                        "tfInnerBatchTxn (checkSignature).";
@@ -2866,6 +2864,9 @@ PeerImp::checkTransaction(
             (stx->getFieldU32(sfLastLedgerSequence) <
              app_.getLedgerMaster().getValidLedgerIndex()))
         {
+            JLOG(p_journal_.info())
+                << "Marking transaction " << stx->getTransactionID()
+                << "as BAD because it's expired";
             app_.getHashRouter().setFlags(
                 stx->getTransactionID(), HashRouterFlags::BAD);
             charge(Resource::feeUselessData, "expired tx");
@@ -2922,7 +2923,7 @@ PeerImp::checkTransaction(
             {
                 if (!validReason.empty())
                 {
-                    JLOG(p_journal_.trace())
+                    JLOG(p_journal_.debug())
                         << "Exception checking transaction: " << validReason;
                 }
 
@@ -2949,7 +2950,7 @@ PeerImp::checkTransaction(
         {
             if (!reason.empty())
             {
-                JLOG(p_journal_.trace())
+                JLOG(p_journal_.debug())
                     << "Exception checking transaction: " << reason;
             }
             app_.getHashRouter().setFlags(