Set version to 2.5.1

Fix stalled consensus detection to prevent false positives in situations where there are no disputed transactions.

Stalled consensus detection was added to 2.5.0 in response to a network consensus halt that caused a round to run for over an hour. However, it has a flaw that makes it very easy to have false positives. Those false positives are usually mitigated by other checks that prevent them from having an effect, but there have been several instances of validators "running ahead" because there are circumstances where the other checks are "successful", allowing the stall state to be checked.

.github/workflows/libxrpl.yml

@@ -1,6 +1,6 @@
 name: Check libXRPL compatibility with Clio
 env:
-  CONAN_URL: http://18.143.149.228:8081/artifactory/api/conan/conan-non-prod
+  CONAN_URL: http://18.143.149.228:8081/artifactory/api/conan/dev
   CONAN_LOGIN_USERNAME_RIPPLE: ${{ secrets.CONAN_USERNAME }}
   CONAN_PASSWORD_RIPPLE: ${{ secrets.CONAN_TOKEN }}
 on:

src/libxrpl/protocol/BuildInfo.cpp

@@ -36,7 +36,7 @@ namespace BuildInfo {
 //  and follow the format described at http://semver.org/
 //------------------------------------------------------------------------------
 // clang-format off
-char const* const versionString = "2.5.0-rc2"
+char const* const versionString = "2.5.1"
 // clang-format on
 
 #if defined(DEBUG) || defined(SANITIZER)

src/libxrpl/protocol/STTx.cpp

@@ -760,6 +760,12 @@ isRawTransactionOkay(STObject const& st, std::string& reason)
         {
             TxType const tt =
                 safe_cast<TxType>(raw.getFieldU16(sfTransactionType));
+            if (tt == ttBATCH)
+            {
+                reason = "Raw Transactions may not contain batch transactions.";
+                return false;
+            }
+
             raw.applyTemplate(getTxFormat(tt)->getSOTemplate());
         }
         catch (std::exception const& e)

src/test/app/Batch_test.cpp

@@ -24,6 +24,7 @@
 #include <xrpld/app/misc/HashRouter.h>
 #include <xrpld/app/misc/Transaction.h>
 #include <xrpld/app/tx/apply.h>
+#include <xrpld/app/tx/detail/Batch.h>
 
 #include <xrpl/protocol/Batch.h>
 #include <xrpl/protocol/Feature.h>
@@ -317,7 +318,8 @@ class Batch_test : public beast::unit_test::suite
             env.close();
         }
 
-        // temINVALID: Batch: batch cannot have inner batch txn.
+        // DEFENSIVE: temINVALID: Batch: batch cannot have inner batch txn.
+        // ACTUAL: telENV_RPC_FAILED: isRawTransactionOkay()
         {
             auto const seq = env.seq(alice);
             auto const batchFee = batch::calcBatchFee(env, 0, 2);
@@ -325,7 +327,7 @@ class Batch_test : public beast::unit_test::suite
                 batch::inner(
                     batch::outer(alice, seq, batchFee, tfAllOrNothing), seq),
                 batch::inner(pay(alice, bob, XRP(1)), seq + 2),
-                ter(temINVALID));
+                ter(telENV_RPC_FAILED));
             env.close();
         }
 
@@ -3953,6 +3955,176 @@ class Batch_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testValidateRPCResponse(FeatureBitset features)
+    {
+        // Verifying that the RPC response from submit includes
+        // the account_sequence_available, account_sequence_next,
+        // open_ledger_cost and validated_ledger_index fields.
+        testcase("Validate RPC response");
+
+        using namespace jtx;
+        Env env(*this);
+        Account const alice("alice");
+        Account const bob("bob");
+        env.fund(XRP(10000), alice, bob);
+        env.close();
+
+        // tes
+        {
+            auto const baseFee = env.current()->fees().base;
+            auto const aliceSeq = env.seq(alice);
+            auto jtx = env.jt(pay(alice, bob, XRP(1)));
+
+            Serializer s;
+            jtx.stx->add(s);
+            auto const jr = env.rpc("submit", strHex(s.slice()))[jss::result];
+            env.close();
+
+            BEAST_EXPECT(jr.isMember(jss::account_sequence_available));
+            BEAST_EXPECT(
+                jr[jss::account_sequence_available].asUInt() == aliceSeq + 1);
+            BEAST_EXPECT(jr.isMember(jss::account_sequence_next));
+            BEAST_EXPECT(
+                jr[jss::account_sequence_next].asUInt() == aliceSeq + 1);
+            BEAST_EXPECT(jr.isMember(jss::open_ledger_cost));
+            BEAST_EXPECT(jr[jss::open_ledger_cost] == to_string(baseFee));
+            BEAST_EXPECT(jr.isMember(jss::validated_ledger_index));
+        }
+
+        // tec failure
+        {
+            auto const baseFee = env.current()->fees().base;
+            auto const aliceSeq = env.seq(alice);
+            env(fset(bob, asfRequireDest));
+            auto jtx = env.jt(pay(alice, bob, XRP(1)), seq(aliceSeq));
+
+            Serializer s;
+            jtx.stx->add(s);
+            auto const jr = env.rpc("submit", strHex(s.slice()))[jss::result];
+            env.close();
+
+            BEAST_EXPECT(jr.isMember(jss::account_sequence_available));
+            BEAST_EXPECT(
+                jr[jss::account_sequence_available].asUInt() == aliceSeq + 1);
+            BEAST_EXPECT(jr.isMember(jss::account_sequence_next));
+            BEAST_EXPECT(
+                jr[jss::account_sequence_next].asUInt() == aliceSeq + 1);
+            BEAST_EXPECT(jr.isMember(jss::open_ledger_cost));
+            BEAST_EXPECT(jr[jss::open_ledger_cost] == to_string(baseFee));
+            BEAST_EXPECT(jr.isMember(jss::validated_ledger_index));
+        }
+
+        // tem failure
+        {
+            auto const baseFee = env.current()->fees().base;
+            auto const aliceSeq = env.seq(alice);
+            auto jtx = env.jt(pay(alice, bob, XRP(1)), seq(aliceSeq + 1));
+
+            Serializer s;
+            jtx.stx->add(s);
+            auto const jr = env.rpc("submit", strHex(s.slice()))[jss::result];
+            env.close();
+
+            BEAST_EXPECT(jr.isMember(jss::account_sequence_available));
+            BEAST_EXPECT(
+                jr[jss::account_sequence_available].asUInt() == aliceSeq);
+            BEAST_EXPECT(jr.isMember(jss::account_sequence_next));
+            BEAST_EXPECT(jr[jss::account_sequence_next].asUInt() == aliceSeq);
+            BEAST_EXPECT(jr.isMember(jss::open_ledger_cost));
+            BEAST_EXPECT(jr[jss::open_ledger_cost] == to_string(baseFee));
+            BEAST_EXPECT(jr.isMember(jss::validated_ledger_index));
+        }
+    }
+
+    void
+    testBatchCalculateBaseFee(FeatureBitset features)
+    {
+        using namespace jtx;
+        Env env(*this);
+        Account const alice("alice");
+        Account const bob("bob");
+        Account const carol("carol");
+        env.fund(XRP(10000), alice, bob, carol);
+        env.close();
+
+        auto getBaseFee = [&](JTx const& jtx) -> XRPAmount {
+            Serializer s;
+            jtx.stx->add(s);
+            return Batch::calculateBaseFee(*env.current(), *jtx.stx);
+        };
+
+        // bad: Inner Batch transaction found
+        {
+            auto const seq = env.seq(alice);
+            XRPAmount const batchFee = batch::calcBatchFee(env, 0, 2);
+            auto jtx = env.jt(
+                batch::outer(alice, seq, batchFee, tfAllOrNothing),
+                batch::inner(
+                    batch::outer(alice, seq, batchFee, tfAllOrNothing), seq),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 2));
+            XRPAmount const txBaseFee = getBaseFee(jtx);
+            BEAST_EXPECT(txBaseFee == XRPAmount(INITIAL_XRP));
+        }
+
+        // bad: Raw Transactions array exceeds max entries.
+        {
+            auto const seq = env.seq(alice);
+            XRPAmount const batchFee = batch::calcBatchFee(env, 0, 2);
+
+            auto jtx = env.jt(
+                batch::outer(alice, seq, batchFee, tfAllOrNothing),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 1),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 2),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 3),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 4),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 5),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 6),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 7),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 8),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 9));
+
+            XRPAmount const txBaseFee = getBaseFee(jtx);
+            BEAST_EXPECT(txBaseFee == XRPAmount(INITIAL_XRP));
+        }
+
+        // bad: Signers array exceeds max entries.
+        {
+            auto const seq = env.seq(alice);
+            XRPAmount const batchFee = batch::calcBatchFee(env, 0, 2);
+
+            auto jtx = env.jt(
+                batch::outer(alice, seq, batchFee, tfAllOrNothing),
+                batch::inner(pay(alice, bob, XRP(10)), seq + 1),
+                batch::inner(pay(alice, bob, XRP(5)), seq + 2),
+                batch::sig(
+                    bob,
+                    carol,
+                    alice,
+                    bob,
+                    carol,
+                    alice,
+                    bob,
+                    carol,
+                    alice,
+                    alice));
+            XRPAmount const txBaseFee = getBaseFee(jtx);
+            BEAST_EXPECT(txBaseFee == XRPAmount(INITIAL_XRP));
+        }
+
+        // good:
+        {
+            auto const seq = env.seq(alice);
+            XRPAmount const batchFee = batch::calcBatchFee(env, 0, 2);
+            auto jtx = env.jt(
+                batch::outer(alice, seq, batchFee, tfAllOrNothing),
+                batch::inner(pay(alice, bob, XRP(1)), seq + 1),
+                batch::inner(pay(bob, alice, XRP(2)), seq + 2));
+            XRPAmount const txBaseFee = getBaseFee(jtx);
+            BEAST_EXPECT(txBaseFee == batchFee);
+        }
+    }
+
     void
     testWithFeats(FeatureBitset features)
     {
@@ -3983,6 +4155,8 @@ class Batch_test : public beast::unit_test::suite
         testBatchTxQueue(features);
         testBatchNetworkOps(features);
         testBatchDelegate(features);
+        testValidateRPCResponse(features);
+        testBatchCalculateBaseFee(features);
     }
 
 public:

src/test/consensus/Consensus_test.cpp

@@ -1136,6 +1136,10 @@ public:
         ConsensusParms p;
         std::size_t peersUnchanged = 0;
 
+        auto logs = std::make_unique<Logs>(beast::severities::kError);
+        auto j = logs->journal("Test");
+        auto clog = std::make_unique<std::stringstream>();
+
         // Three cases:
         // 1 proposing, initial vote yes
         // 2 proposing, initial vote no
@@ -1172,10 +1176,15 @@ public:
             BEAST_EXPECT(proposingFalse.getOurVote() == false);
             BEAST_EXPECT(followingTrue.getOurVote() == true);
             BEAST_EXPECT(followingFalse.getOurVote() == false);
-            BEAST_EXPECT(!proposingTrue.stalled(p, true, peersUnchanged));
+            BEAST_EXPECT(
-            BEAST_EXPECT(!proposingFalse.stalled(p, true, peersUnchanged));
+                !proposingTrue.stalled(p, true, peersUnchanged, j, clog));
-            BEAST_EXPECT(!followingTrue.stalled(p, false, peersUnchanged));
+            BEAST_EXPECT(
-            BEAST_EXPECT(!followingFalse.stalled(p, false, peersUnchanged));
+                !proposingFalse.stalled(p, true, peersUnchanged, j, clog));
+            BEAST_EXPECT(
+                !followingTrue.stalled(p, false, peersUnchanged, j, clog));
+            BEAST_EXPECT(
+                !followingFalse.stalled(p, false, peersUnchanged, j, clog));
+            BEAST_EXPECT(clog->str() == "");
 
             // I'm in the majority, my vote should not change
             BEAST_EXPECT(!proposingTrue.updateVote(5, true, p));
@@ -1189,10 +1198,15 @@ public:
             BEAST_EXPECT(!followingFalse.updateVote(10, false, p));
 
             peersUnchanged = 2;
-            BEAST_EXPECT(!proposingTrue.stalled(p, true, peersUnchanged));
+            BEAST_EXPECT(
-            BEAST_EXPECT(!proposingFalse.stalled(p, true, peersUnchanged));
+                !proposingTrue.stalled(p, true, peersUnchanged, j, clog));
-            BEAST_EXPECT(!followingTrue.stalled(p, false, peersUnchanged));
+            BEAST_EXPECT(
-            BEAST_EXPECT(!followingFalse.stalled(p, false, peersUnchanged));
+                !proposingFalse.stalled(p, true, peersUnchanged, j, clog));
+            BEAST_EXPECT(
+                !followingTrue.stalled(p, false, peersUnchanged, j, clog));
+            BEAST_EXPECT(
+                !followingFalse.stalled(p, false, peersUnchanged, j, clog));
+            BEAST_EXPECT(clog->str() == "");
 
             // Right now, the vote is 51%. The requirement is about to jump to
             // 65%
@@ -1282,10 +1296,15 @@ public:
             BEAST_EXPECT(followingFalse.getOurVote() == false);
 
             peersUnchanged = 3;
-            BEAST_EXPECT(!proposingTrue.stalled(p, true, peersUnchanged));
+            BEAST_EXPECT(
-            BEAST_EXPECT(!proposingFalse.stalled(p, true, peersUnchanged));
+                !proposingTrue.stalled(p, true, peersUnchanged, j, clog));
-            BEAST_EXPECT(!followingTrue.stalled(p, false, peersUnchanged));
+            BEAST_EXPECT(
-            BEAST_EXPECT(!followingFalse.stalled(p, false, peersUnchanged));
+                !proposingFalse.stalled(p, true, peersUnchanged, j, clog));
+            BEAST_EXPECT(
+                !followingTrue.stalled(p, false, peersUnchanged, j, clog));
+            BEAST_EXPECT(
+                !followingFalse.stalled(p, false, peersUnchanged, j, clog));
+            BEAST_EXPECT(clog->str() == "");
 
             // Threshold jumps to 95%
             BEAST_EXPECT(proposingTrue.updateVote(220, true, p));
@@ -1322,12 +1341,60 @@ public:
 
             for (peersUnchanged = 0; peersUnchanged < 6; ++peersUnchanged)
             {
-                BEAST_EXPECT(!proposingTrue.stalled(p, true, peersUnchanged));
+                BEAST_EXPECT(
-                BEAST_EXPECT(!proposingFalse.stalled(p, true, peersUnchanged));
+                    !proposingTrue.stalled(p, true, peersUnchanged, j, clog));
-                BEAST_EXPECT(!followingTrue.stalled(p, false, peersUnchanged));
+                BEAST_EXPECT(
-                BEAST_EXPECT(!followingFalse.stalled(p, false, peersUnchanged));
+                    !proposingFalse.stalled(p, true, peersUnchanged, j, clog));
+                BEAST_EXPECT(
+                    !followingTrue.stalled(p, false, peersUnchanged, j, clog));
+                BEAST_EXPECT(
+                    !followingFalse.stalled(p, false, peersUnchanged, j, clog));
+                BEAST_EXPECT(clog->str() == "");
             }
 
+            auto expectStalled = [this, &clog](
+                                     int txid,
+                                     bool ourVote,
+                                     int ourTime,
+                                     int peerTime,
+                                     int support,
+                                     std::uint32_t line) {
+                using namespace std::string_literals;
+
+                auto const s = clog->str();
+                expect(s.find("stalled"), s, __FILE__, line);
+                expect(
+                    s.starts_with("Transaction "s + std::to_string(txid)),
+                    s,
+                    __FILE__,
+                    line);
+                expect(
+                    s.find("voting "s + (ourVote ? "YES" : "NO")) != s.npos,
+                    s,
+                    __FILE__,
+                    line);
+                expect(
+                    s.find("for "s + std::to_string(ourTime) + " rounds."s) !=
+                        s.npos,
+                    s,
+                    __FILE__,
+                    line);
+                expect(
+                    s.find(
+                        "votes in "s + std::to_string(peerTime) + " rounds.") !=
+                        s.npos,
+                    s,
+                    __FILE__,
+                    line);
+                expect(
+                    s.ends_with(
+                        "has "s + std::to_string(support) + "% support. "s),
+                    s,
+                    __FILE__,
+                    line);
+                clog = std::make_unique<std::stringstream>();
+            };
+
             for (int i = 0; i < 1; ++i)
             {
                 BEAST_EXPECT(!proposingTrue.updateVote(250 + 10 * i, true, p));
@@ -1342,22 +1409,34 @@ public:
                 BEAST_EXPECT(followingFalse.getOurVote() == false);
 
                 // true vote has changed recently, so not stalled
-                BEAST_EXPECT(!proposingTrue.stalled(p, true, 0));
+                BEAST_EXPECT(!proposingTrue.stalled(p, true, 0, j, clog));
+                BEAST_EXPECT(clog->str() == "");
                 // remaining votes have been unchanged in so long that we only
                 // need to hit the second round at 95% to be stalled, regardless
                 // of peers
-                BEAST_EXPECT(proposingFalse.stalled(p, true, 0));
+                BEAST_EXPECT(proposingFalse.stalled(p, true, 0, j, clog));
-                BEAST_EXPECT(followingTrue.stalled(p, false, 0));
+                expectStalled(98, false, 11, 0, 2, __LINE__);
-                BEAST_EXPECT(followingFalse.stalled(p, false, 0));
+                BEAST_EXPECT(followingTrue.stalled(p, false, 0, j, clog));
+                expectStalled(97, true, 11, 0, 97, __LINE__);
+                BEAST_EXPECT(followingFalse.stalled(p, false, 0, j, clog));
+                expectStalled(96, false, 11, 0, 3, __LINE__);
 
                 // true vote has changed recently, so not stalled
-                BEAST_EXPECT(!proposingTrue.stalled(p, true, peersUnchanged));
+                BEAST_EXPECT(
+                    !proposingTrue.stalled(p, true, peersUnchanged, j, clog));
+                BEAST_EXPECTS(clog->str() == "", clog->str());
                 // remaining votes have been unchanged in so long that we only
                 // need to hit the second round at 95% to be stalled, regardless
                 // of peers
-                BEAST_EXPECT(proposingFalse.stalled(p, true, peersUnchanged));
+                BEAST_EXPECT(
-                BEAST_EXPECT(followingTrue.stalled(p, false, peersUnchanged));
+                    proposingFalse.stalled(p, true, peersUnchanged, j, clog));
-                BEAST_EXPECT(followingFalse.stalled(p, false, peersUnchanged));
+                expectStalled(98, false, 11, 6, 2, __LINE__);
+                BEAST_EXPECT(
+                    followingTrue.stalled(p, false, peersUnchanged, j, clog));
+                expectStalled(97, true, 11, 6, 97, __LINE__);
+                BEAST_EXPECT(
+                    followingFalse.stalled(p, false, peersUnchanged, j, clog));
+                expectStalled(96, false, 11, 6, 3, __LINE__);
             }
             for (int i = 1; i < 3; ++i)
             {
@@ -1374,19 +1453,31 @@ public:
 
                 // true vote changed 2 rounds ago, and peers are changing, so
                 // not stalled
-                BEAST_EXPECT(!proposingTrue.stalled(p, true, 0));
+                BEAST_EXPECT(!proposingTrue.stalled(p, true, 0, j, clog));
+                BEAST_EXPECTS(clog->str() == "", clog->str());
                 // still stalled
-                BEAST_EXPECT(proposingFalse.stalled(p, true, 0));
+                BEAST_EXPECT(proposingFalse.stalled(p, true, 0, j, clog));
-                BEAST_EXPECT(followingTrue.stalled(p, false, 0));
+                expectStalled(98, false, 11 + i, 0, 2, __LINE__);
-                BEAST_EXPECT(followingFalse.stalled(p, false, 0));
+                BEAST_EXPECT(followingTrue.stalled(p, false, 0, j, clog));
+                expectStalled(97, true, 11 + i, 0, 97, __LINE__);
+                BEAST_EXPECT(followingFalse.stalled(p, false, 0, j, clog));
+                expectStalled(96, false, 11 + i, 0, 3, __LINE__);
 
                 // true vote changed 2 rounds ago, and peers are NOT changing,
                 // so stalled
-                BEAST_EXPECT(proposingTrue.stalled(p, true, peersUnchanged));
+                BEAST_EXPECT(
+                    proposingTrue.stalled(p, true, peersUnchanged, j, clog));
+                expectStalled(99, true, 1 + i, 6, 97, __LINE__);
                 // still stalled
-                BEAST_EXPECT(proposingFalse.stalled(p, true, peersUnchanged));
+                BEAST_EXPECT(
-                BEAST_EXPECT(followingTrue.stalled(p, false, peersUnchanged));
+                    proposingFalse.stalled(p, true, peersUnchanged, j, clog));
-                BEAST_EXPECT(followingFalse.stalled(p, false, peersUnchanged));
+                expectStalled(98, false, 11 + i, 6, 2, __LINE__);
+                BEAST_EXPECT(
+                    followingTrue.stalled(p, false, peersUnchanged, j, clog));
+                expectStalled(97, true, 11 + i, 6, 97, __LINE__);
+                BEAST_EXPECT(
+                    followingFalse.stalled(p, false, peersUnchanged, j, clog));
+                expectStalled(96, false, 11 + i, 6, 3, __LINE__);
             }
             for (int i = 3; i < 5; ++i)
             {
@@ -1401,15 +1492,27 @@ public:
                 BEAST_EXPECT(followingTrue.getOurVote() == true);
                 BEAST_EXPECT(followingFalse.getOurVote() == false);
 
-                BEAST_EXPECT(proposingTrue.stalled(p, true, 0));
+                BEAST_EXPECT(proposingTrue.stalled(p, true, 0, j, clog));
-                BEAST_EXPECT(proposingFalse.stalled(p, true, 0));
+                expectStalled(99, true, 1 + i, 0, 97, __LINE__);
-                BEAST_EXPECT(followingTrue.stalled(p, false, 0));
+                BEAST_EXPECT(proposingFalse.stalled(p, true, 0, j, clog));
-                BEAST_EXPECT(followingFalse.stalled(p, false, 0));
+                expectStalled(98, false, 11 + i, 0, 2, __LINE__);
+                BEAST_EXPECT(followingTrue.stalled(p, false, 0, j, clog));
+                expectStalled(97, true, 11 + i, 0, 97, __LINE__);
+                BEAST_EXPECT(followingFalse.stalled(p, false, 0, j, clog));
+                expectStalled(96, false, 11 + i, 0, 3, __LINE__);
 
-                BEAST_EXPECT(proposingTrue.stalled(p, true, peersUnchanged));
+                BEAST_EXPECT(
-                BEAST_EXPECT(proposingFalse.stalled(p, true, peersUnchanged));
+                    proposingTrue.stalled(p, true, peersUnchanged, j, clog));
-                BEAST_EXPECT(followingTrue.stalled(p, false, peersUnchanged));
+                expectStalled(99, true, 1 + i, 6, 97, __LINE__);
-                BEAST_EXPECT(followingFalse.stalled(p, false, peersUnchanged));
+                BEAST_EXPECT(
+                    proposingFalse.stalled(p, true, peersUnchanged, j, clog));
+                expectStalled(98, false, 11 + i, 6, 2, __LINE__);
+                BEAST_EXPECT(
+                    followingTrue.stalled(p, false, peersUnchanged, j, clog));
+                expectStalled(97, true, 11 + i, 6, 97, __LINE__);
+                BEAST_EXPECT(
+                    followingFalse.stalled(p, false, peersUnchanged, j, clog));
+                expectStalled(96, false, 11 + i, 6, 3, __LINE__);
             }
         }
     }

src/test/jtx/impl/Env.cpp

@@ -96,7 +96,7 @@ Env::AppBundle::~AppBundle()
     if (app)
     {
         app->getJobQueue().rendezvous();
-        app->signalStop();
+        app->signalStop("~AppBundle");
     }
     if (thread.joinable())
         thread.join();

src/xrpld/app/consensus/RCLValidations.cpp

@@ -136,7 +136,7 @@ RCLValidationsAdaptor::acquire(LedgerHash const& hash)
 
     if (!ledger)
     {
-        JLOG(j_.debug())
+        JLOG(j_.warn())
             << "Need validated ledger for preferred ledger analysis " << hash;
 
         Application* pApp = &app_;

src/xrpld/app/ledger/detail/InboundLedgers.cpp

@@ -384,10 +384,17 @@ public:
     {
         auto const start = m_clock.now();
 
+        // Make a list of things to sweep, while holding the lock
+        std::vector<MapType::mapped_type> stuffToSweep;
+        std::size_t total;
+
         {
             ScopedLockType sl(mLock);
             MapType::iterator it(mLedgers.begin());
-            
+            total = mLedgers.size();
+
+            stuffToSweep.reserve(total);
+
             while (it != mLedgers.end())
             {
                 auto const la = it->second->getLastAction();
@@ -397,8 +404,11 @@ public:
                     it->second->touch();
                     ++it;
                 }
-                else if ((la + std::chrono::seconds(10)) < start)
+                else if ((la + std::chrono::minutes(1)) < start)
                 {
+                    stuffToSweep.push_back(it->second);
+                    // shouldn't cause the actual final delete
+                    // since we are holding a reference in the vector.
                     it = mLedgers.erase(it);
                 }
                 else
@@ -409,6 +419,14 @@ public:
 
             beast::expire(mRecentFailures, kReacquireInterval);
         }
+
+        JLOG(j_.debug())
+            << "Swept " << stuffToSweep.size() << " out of " << total
+            << " inbound ledgers. Duration: "
+            << std::chrono::duration_cast<std::chrono::milliseconds>(
+                   m_clock.now() - start)
+                   .count()
+            << "ms";
     }
 
     void

src/xrpld/app/main/Application.cpp

@@ -285,7 +285,7 @@ public:
                   config_->CONFIG_DIR),
               *this,
               logs_->journal("PerfLog"),
-              [this] { signalStop(); }))
+              [this] { signalStop("PerfLog"); }))
 
         , m_txMaster(*this)
 
@@ -505,7 +505,7 @@ public:
     void
     run() override;
     void
-    signalStop(std::string msg = "") override;
+    signalStop(std::string msg) override;
     bool
     checkSigs() const override;
     void
@@ -977,7 +977,7 @@ public:
         if (!config_->standalone() &&
             !getRelationalDatabase().transactionDbHasSpace(*config_))
         {
-            signalStop();
+            signalStop("Out of transaction DB space");
         }
 
         // VFALCO NOTE Does the order of calls matter?
@@ -1193,7 +1193,7 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline)
             JLOG(m_journal.info()) << "Received signal " << signum;
 
             if (signum == SIGTERM || signum == SIGINT)
-                signalStop();
+                signalStop("Signal: " + to_string(signum));
         });
 
     auto debug_log = config_->getDebugLogFile();

src/xrpld/app/main/Application.h

@@ -141,7 +141,7 @@ public:
     virtual void
     run() = 0;
     virtual void
-    signalStop(std::string msg = "") = 0;
+    signalStop(std::string msg) = 0;
     virtual bool
     checkSigs() const = 0;
     virtual void

src/xrpld/app/misc/NetworkOPs.cpp

@@ -1701,7 +1701,7 @@ NetworkOPsImp::apply(std::unique_lock<std::mutex>& batchLock)
                 }
             }
 
-            if (!isTemMalformed(e.result) && validatedLedgerIndex)
+            if (validatedLedgerIndex)
             {
                 auto [fee, accountSeq, availableSeq] =
                     app_.getTxQ().getTxRequiredFeeAndSeq(

src/xrpld/app/tx/detail/Batch.cpp

@@ -61,7 +61,10 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
 
     // LCOV_EXCL_START
     if (baseFee > maxAmount - view.fees().base)
-        throw std::overflow_error("XRPAmount overflow");
+    {
+        JLOG(debugLog().error()) << "BatchTrace: Base fee overflow detected.";
+        return XRPAmount{INITIAL_XRP};
+    }
     // LCOV_EXCL_STOP
 
     XRPAmount const batchBase = view.fees().base + baseFee;
@@ -72,32 +75,36 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
     {
         auto const& txns = tx.getFieldArray(sfRawTransactions);
 
-        XRPL_ASSERT(
-            txns.size() <= maxBatchTxCount,
-            "Raw Transactions array exceeds max entries.");
-
         // LCOV_EXCL_START
         if (txns.size() > maxBatchTxCount)
-            throw std::length_error(
+        {
-                "Raw Transactions array exceeds max entries");
+            JLOG(debugLog().error())
+                << "BatchTrace: Raw Transactions array exceeds max entries.";
+            return XRPAmount{INITIAL_XRP};
+        }
         // LCOV_EXCL_STOP
 
         for (STObject txn : txns)
         {
             STTx const stx = STTx{std::move(txn)};
 
-            XRPL_ASSERT(
-                stx.getTxnType() != ttBATCH, "Inner Batch transaction found.");
-
             // LCOV_EXCL_START
             if (stx.getTxnType() == ttBATCH)
-                throw std::invalid_argument("Inner Batch transaction found");
+            {
+                JLOG(debugLog().error())
+                    << "BatchTrace: Inner Batch transaction found.";
+                return XRPAmount{INITIAL_XRP};
+            }
             // LCOV_EXCL_STOP
 
             auto const fee = ripple::calculateBaseFee(view, stx);
             // LCOV_EXCL_START
             if (txnFees > maxAmount - fee)
-                throw std::overflow_error("XRPAmount overflow");
+            {
+                JLOG(debugLog().error())
+                    << "BatchTrace: XRPAmount overflow in txnFees calculation.";
+                return XRPAmount{INITIAL_XRP};
+            }
             // LCOV_EXCL_STOP
             txnFees += fee;
         }
@@ -108,13 +115,14 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
     if (tx.isFieldPresent(sfBatchSigners))
     {
         auto const& signers = tx.getFieldArray(sfBatchSigners);
-        XRPL_ASSERT(
-            signers.size() <= maxBatchTxCount,
-            "Batch Signers array exceeds max entries.");
 
         // LCOV_EXCL_START
         if (signers.size() > maxBatchTxCount)
-            throw std::length_error("Batch Signers array exceeds max entries");
+        {
+            JLOG(debugLog().error())
+                << "BatchTrace: Batch Signers array exceeds max entries.";
+            return XRPAmount{INITIAL_XRP};
+        }
         // LCOV_EXCL_STOP
 
         for (STObject const& signer : signers)
@@ -128,16 +136,28 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
 
     // LCOV_EXCL_START
     if (signerCount > 0 && view.fees().base > maxAmount / signerCount)
-        throw std::overflow_error("XRPAmount overflow");
+    {
+        JLOG(debugLog().error())
+            << "BatchTrace: XRPAmount overflow in signerCount calculation.";
+        return XRPAmount{INITIAL_XRP};
+    }
     // LCOV_EXCL_STOP
 
     XRPAmount signerFees = signerCount * view.fees().base;
 
     // LCOV_EXCL_START
     if (signerFees > maxAmount - txnFees)
-        throw std::overflow_error("XRPAmount overflow");
+    {
+        JLOG(debugLog().error())
+            << "BatchTrace: XRPAmount overflow in signerFees calculation.";
+        return XRPAmount{INITIAL_XRP};
+    }
     if (txnFees + signerFees > maxAmount - batchBase)
-        throw std::overflow_error("XRPAmount overflow");
+    {
+        JLOG(debugLog().error())
+            << "BatchTrace: XRPAmount overflow in total fee calculation.";
+        return XRPAmount{INITIAL_XRP};
+    }
     // LCOV_EXCL_STOP
 
     // 10 drops per batch signature + sum of inner tx fees + batchBase

src/xrpld/consensus/Consensus.cpp

@@ -139,11 +139,11 @@ checkConsensusReached(
         return false;
     }
 
-    // We only get stalled when every disputed transaction unequivocally has 80%
+    // We only get stalled when there are disputed transactions and all of them
-    // (minConsensusPct) agreement, either for or against. That is: either under
+    // unequivocally have 80% (minConsensusPct) agreement, either for or
-    // 20% or over 80% consensus (repectively "nay" or "yay"). This prevents
+    // against. That is: either under 20% or over 80% consensus (repectively
-    // manipulation by a minority of byzantine peers of which transactions make
+    // "nay" or "yay"). This prevents manipulation by a minority of byzantine
-    // the cut to get into the ledger.
+    // peers of which transactions make the cut to get into the ledger.
     if (stalled)
     {
         CLOG(clog) << "consensus stalled. ";

src/xrpld/consensus/Consensus.h

@@ -84,8 +84,8 @@ shouldCloseLedger(
                             agree
     @param stalled the network appears to be stalled, where
            neither we nor our peers have changed their vote on any disputes in a
-           while. This is undesirable, and will cause us to end consensus
+           while. This is undesirable, and should be rare, and will cause us to
-           without 80% agreement.
+           end consensus without 80% agreement.
     @param parms            Consensus constant parameters
     @param proposing        whether we should count ourselves
     @param j                journal for logging
@@ -1712,15 +1712,29 @@ Consensus<Adaptor>::haveConsensus(
                      << ", disagree=" << disagree;
 
     ConsensusParms const& parms = adaptor_.parms();
-    // Stalling is BAD
+    // Stalling is BAD. It means that we have a consensus on the close time, so
+    // peers are talking, but we have disputed transactions that peers are
+    // unable or unwilling to come to agreement on one way or the other.
     bool const stalled = haveCloseTimeConsensus_ &&
+        !result_->disputes.empty() &&
         std::ranges::all_of(result_->disputes,
-                            [this, &parms](auto const& dispute) {
+                            [this, &parms, &clog](auto const& dispute) {
                                 return dispute.second.stalled(
                                     parms,
                                     mode_.get() == ConsensusMode::proposing,
-                                    peerUnchangedCounter_);
+                                    peerUnchangedCounter_,
+                                    j_,
+                                    clog);
                             });
+    if (stalled)
+    {
+        std::stringstream ss;
+        ss << "Consensus detects as stalled with " << (agree + disagree) << "/"
+           << prevProposers_ << " proposers, and " << result_->disputes.size()
+           << " stalled disputed transactions.";
+        JLOG(j_.error()) << ss.str();
+        CLOG(clog) << ss.str();
+    }
 
     // Determine if we actually have consensus or not
     result_->state = checkConsensus(

src/xrpld/consensus/DisputedTx.h

@@ -85,7 +85,12 @@ public:
     //! Are we and our peers "stalled" where we probably won't change
     //! our vote?
     bool
-    stalled(ConsensusParms const& p, bool proposing, int peersUnchanged) const
+    stalled(
+        ConsensusParms const& p,
+        bool proposing,
+        int peersUnchanged,
+        beast::Journal j,
+        std::unique_ptr<std::stringstream> const& clog) const
     {
         // at() can throw, but the map is built by hand to ensure all valid
         // values are available.
@@ -123,8 +128,24 @@ public:
         int const weight = support / total;
         // Returns true if the tx has more than minCONSENSUS_PCT (80) percent
         // agreement. Either voting for _or_ voting against the tx.
-        return weight > p.minCONSENSUS_PCT ||
+        bool const stalled =
-            weight < (100 - p.minCONSENSUS_PCT);
+            weight > p.minCONSENSUS_PCT || weight < (100 - p.minCONSENSUS_PCT);
+
+        if (stalled)
+        {
+            // stalling is an error condition for even a single
+            // transaction.
+            std::stringstream s;
+            s << "Transaction " << ID() << " is stalled. We have been voting "
+              << (getOurVote() ? "YES" : "NO") << " for " << currentVoteCounter_
+              << " rounds. Peers have not changed their votes in "
+              << peersUnchanged << " rounds. The transaction has " << weight
+              << "% support. ";
+            JLOG(j_.error()) << s.str();
+            CLOG(clog) << s.str();
+        }
+
+        return stalled;
     }
 
     //! The disputed transaction.

src/xrpld/overlay/detail/PeerImp.cpp

@@ -3440,7 +3440,7 @@ PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
                 if (!m->has_ledgerhash())
                     info += ", no hash specified";
 
-                JLOG(p_journal_.error())
+                JLOG(p_journal_.warn())
                     << "processLedgerRequest: getNodeFat with nodeId "
                     << *shaMapNodeId << " and ledger info type " << info
                     << " throws exception: " << e.what();

src/xrpld/rpc/handlers/Stop.cpp

@@ -31,7 +31,7 @@ struct JsonContext;
 Json::Value
 doStop(RPC::JsonContext& context)
 {
-    context.app.signalStop();
+    context.app.signalStop("RPC");
     return RPC::makeObjectValue(systemName() + " server stopping");
 }