Don't reach consensus as fast if no other proposals seen.

XRPLF · Nov 14, 2023 · d0b0186 · d0b0186
1 parent ac27089
commit d0b0186
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 10 deletions.
diff --git 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 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 will likely 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,13 @@ 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 +162,11 @@ 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 +175,11 @@ 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
@@ -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
@@ -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
@@ -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