From 77052d21f5f9f56a316a78a9d974d7b11e1b91a3 Mon Sep 17 00:00:00 2001
From: Mark Travis <mtravis@ripple.com>
Date: Mon, 26 Jun 2023 12:54:27 -0700
Subject: [PATCH] Don't reach consensus as fast if no other proposals seen.

---
 src/ripple/consensus/Consensus.cpp    | 32 ++++++++++++++++++++++-----
 src/ripple/consensus/Consensus.h      |  2 +-
 src/ripple/consensus/ConsensusParms.h |  2 +-
 src/test/consensus/Consensus_test.cpp |  9 ++++++--
 4 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/src/ripple/consensus/Consensus.cpp b/src/ripple/consensus/Consensus.cpp
index c40bd1294e7..a76ad648d71 100644
--- a/src/ripple/consensus/Consensus.cpp
+++ b/src/ripple/consensus/Consensus.cpp
@@ -94,11 +94,24 @@ checkConsensusReached(
     std::size_t agreeing,
     std::size_t total,
     bool count_self,
-    std::size_t minConsensusPct)
+    std::size_t minConsensusPct,
+    bool reachedMax)
 {
-    // If we are alone, we have a consensus
+    // If we are alone for too long, we have a consensus
+    // Delaying consensus like this avoids a circumstance where a peer
+    // gets ahead of proposers insofar as it has not received any proposals.
+    // This could happen if there's a slowdown in receiving proposals. Reaching
+    // consensus prematurely in this way means that the peer with desync.
+    // The check for reachedMax should allow plenty of time for proposals to
+    // arrive, and there should be no downside. If a peer is truly not
+    // receiving any proposals, then there should be no hurry. There's
+    // really nowhere to go.
     if (total == 0)
-        return true;
+    {
+        if (reachedMax)
+            return true;
+        return false;
+    }
 
     if (count_self)
     {
@@ -127,7 +140,12 @@ checkConsensus(
                     << prevProposers << " agree=" << currentAgree
                     << " validated=" << currentFinished
                     << " time=" << currentAgreeTime.count() << "/"
-                    << previousAgreeTime.count();
+                    << previousAgreeTime.count()
+                    << " proposing? " << proposing
+                    << " minimum duration to reach consensus: "
+                    << parms.ledgerMIN_CONSENSUS.count() << "ms"
+                    << " max consensus time " << parms.ledgerMAX_CONSENSUS.count() << 's'
+                    << " minimum consensus percentage: " << parms.minCONSENSUS_PCT;
 
     if (currentProposers < (prevProposers * 3 / 4))
     {
@@ -143,7 +161,8 @@ checkConsensus(
     // Have we, together with the nodes on our UNL list, reached the threshold
     // to declare consensus?
     if (checkConsensusReached(
-            currentAgree, currentProposers, proposing, parms.minCONSENSUS_PCT))
+            currentAgree, currentProposers, proposing, parms.minCONSENSUS_PCT,
+            currentAgreeTime > parms.ledgerMAX_CONSENSUS))
     {
         JLOG(j.debug()) << "normal consensus";
         return ConsensusState::Yes;
@@ -152,7 +171,8 @@ checkConsensus(
     // Have sufficient nodes on our UNL list moved on and reached the threshold
     // to declare consensus?
     if (checkConsensusReached(
-            currentFinished, currentProposers, false, parms.minCONSENSUS_PCT))
+            currentFinished, currentProposers, false, parms.minCONSENSUS_PCT,
+            currentAgreeTime > parms.ledgerMAX_CONSENSUS))
     {
         JLOG(j.warn()) << "We see no consensus, but 80% of nodes have moved on";
         return ConsensusState::MovedOn;
diff --git a/src/ripple/consensus/Consensus.h b/src/ripple/consensus/Consensus.h
index 8115e88c9e1..911c8596bcd 100644
--- a/src/ripple/consensus/Consensus.h
+++ b/src/ripple/consensus/Consensus.h
@@ -1341,7 +1341,7 @@ Consensus<Adaptor>::shouldPause() const
     std::size_t const offline = trustedKeys.size();
 
     std::stringstream vars;
-    vars << " (working seq: " << previousLedger_.seq() << ", "
+    vars << " consensuslog (working seq: " << previousLedger_.seq() << ", "
          << "validated seq: " << adaptor_.getValidLedgerIndex() << ", "
          << "am validator: " << adaptor_.validator() << ", "
          << "have validated: " << adaptor_.haveValidated() << ", "
diff --git a/src/ripple/consensus/ConsensusParms.h b/src/ripple/consensus/ConsensusParms.h
index 61722e2c439..cdc798df3f2 100644
--- a/src/ripple/consensus/ConsensusParms.h
+++ b/src/ripple/consensus/ConsensusParms.h
@@ -94,7 +94,7 @@ struct ConsensusParms
      *  validators don't appear to be offline that are merely waiting for
      *  laggards.
      */
-    std::chrono::milliseconds ledgerMAX_CONSENSUS = std::chrono::seconds{10};
+    std::chrono::milliseconds ledgerMAX_CONSENSUS = std::chrono::seconds{15};
 
     //! Minimum number of seconds to wait to ensure others have computed the LCL
     std::chrono::milliseconds ledgerMIN_CLOSE = std::chrono::seconds{2};
diff --git a/src/test/consensus/Consensus_test.cpp b/src/test/consensus/Consensus_test.cpp
index 6b0b4817875..3fcbb4d03ff 100644
--- a/src/test/consensus/Consensus_test.cpp
+++ b/src/test/consensus/Consensus_test.cpp
@@ -110,10 +110,15 @@ class Consensus_test : public beast::unit_test::suite
             ConsensusState::MovedOn ==
             checkConsensus(10, 2, 1, 8, 3s, 10s, p, true, journal_));
 
-        // No peers makes it easy to agree
+        // If no peers, don't agree until time has passed.
         BEAST_EXPECT(
-            ConsensusState::Yes ==
+            ConsensusState::No ==
             checkConsensus(0, 0, 0, 0, 3s, 10s, p, true, journal_));
+
+        // Agree if no peers and enough time has passed.
+        BEAST_EXPECT(
+            ConsensusState::Yes ==
+            checkConsensus(0, 0, 0, 0, 3s, 16s, p, true, journal_));
     }
 
     void