Several changes to improve Consensus stability: (#4505)

* Verify accepted ledger becomes validated, and retry
   with a new consensus transaction set if not.
 * Always store proposals.
 * Track proposals by ledger sequence. This helps slow peers catch
   up with the rest of the network.
 * Acquire transaction sets for proposals with future ledger sequences.
   This also helps slow peers catch up.
 * Optimize timer delay for establish phase to wait based on how
   long validators have been sending proposals. This also helps slow
   peers to catch up.
 * Fix impasse achieving close time consensus.
 * Don't wait between open and establish phases.

src/test/consensus/Consensus_test.cpp

@@ -44,34 +44,35 @@ public:
         // Use default parameters
         ConsensusParms const p{};
 
+        std::optional<std::chrono::milliseconds> delay;
         // Bizarre times forcibly close
         BEAST_EXPECT(shouldCloseLedger(
-            true, 10, 10, 10, -10s, 10s, 1s, 1s, p, journal_));
+            true, 10, 10, 10, -10s, 10s, 1s, delay, 1s, p, journal_));
         BEAST_EXPECT(shouldCloseLedger(
-            true, 10, 10, 10, 100h, 10s, 1s, 1s, p, journal_));
+            true, 10, 10, 10, 100h, 10s, 1s, delay, 1s, p, journal_));
         BEAST_EXPECT(shouldCloseLedger(
-            true, 10, 10, 10, 10s, 100h, 1s, 1s, p, journal_));
+            true, 10, 10, 10, 10s, 100h, 1s, delay, 1s, p, journal_));
 
         // Rest of network has closed
-        BEAST_EXPECT(
-            shouldCloseLedger(true, 10, 3, 5, 10s, 10s, 10s, 10s, p, journal_));
+        BEAST_EXPECT(shouldCloseLedger(
+            true, 10, 3, 5, 10s, 10s, 10s, delay, 10s, p, journal_));
 
         // No transactions means wait until end of internval
-        BEAST_EXPECT(
-            !shouldCloseLedger(false, 10, 0, 0, 1s, 1s, 1s, 10s, p, journal_));
-        BEAST_EXPECT(
-            shouldCloseLedger(false, 10, 0, 0, 1s, 10s, 1s, 10s, p, journal_));
+        BEAST_EXPECT(!shouldCloseLedger(
+            false, 10, 0, 0, 1s, 1s, 1s, delay, 10s, p, journal_));
+        BEAST_EXPECT(shouldCloseLedger(
+            false, 10, 0, 0, 1s, 10s, 1s, delay, 10s, p, journal_));
 
         // Enforce minimum ledger open time
-        BEAST_EXPECT(
-            !shouldCloseLedger(true, 10, 0, 0, 10s, 10s, 1s, 10s, p, journal_));
+        BEAST_EXPECT(!shouldCloseLedger(
+            true, 10, 0, 0, 10s, 10s, 1s, delay, 10s, p, journal_));
 
         // Don't go too much faster than last time
-        BEAST_EXPECT(
-            !shouldCloseLedger(true, 10, 0, 0, 10s, 10s, 3s, 10s, p, journal_));
+        BEAST_EXPECT(!shouldCloseLedger(
+            true, 10, 0, 0, 10s, 10s, 3s, delay, 10s, p, journal_));
 
-        BEAST_EXPECT(
-            shouldCloseLedger(true, 10, 0, 0, 10s, 10s, 10s, 10s, p, journal_));
+        BEAST_EXPECT(shouldCloseLedger(
+            true, 10, 0, 0, 10s, 10s, 10s, delay, 10s, p, journal_));
     }
 
     void

src/test/csf/Peer.h

@@ -19,6 +19,9 @@
 #ifndef RIPPLE_TEST_CSF_PEER_H_INCLUDED
 #define RIPPLE_TEST_CSF_PEER_H_INCLUDED
 
+#include <ripple/app/ledger/LedgerMaster.h>
+#include <ripple/basics/chrono.h>
+#include <ripple/beast/unit_test.h>
 #include <ripple/beast/utility/WrappedSink.h>
 #include <ripple/consensus/Consensus.h>
 #include <ripple/consensus/Validations.h>
@@ -26,6 +29,10 @@
 #include <boost/container/flat_map.hpp>
 #include <boost/container/flat_set.hpp>
 #include <algorithm>
+#include <chrono>
+#include <memory>
+#include <mutex>
+#include <optional>
 #include <test/csf/CollectorRef.h>
 #include <test/csf/Scheduler.h>
 #include <test/csf/TrustGraph.h>
@@ -33,6 +40,7 @@
 #include <test/csf/Validation.h>
 #include <test/csf/events.h>
 #include <test/csf/ledgers.h>
+#include <test/jtx/Env.h>
 
 namespace ripple {
 namespace test {
@@ -158,10 +166,13 @@ struct Peer
     using NodeID_t = PeerID;
     using NodeKey_t = PeerKey;
     using TxSet_t = TxSet;
+    using CanonicalTxSet_t = TxSet;
     using PeerPosition_t = Position;
     using Result = ConsensusResult<Peer>;
     using NodeKey = Validation::NodeKey;
 
+    using clock_type = Stopwatch;
+
     //! Logging support that prefixes messages with the peer ID
     beast::WrappedSink sink;
     beast::Journal j;
@@ -240,7 +251,7 @@ struct Peer
 
     // Quorum of validations needed for a ledger to be fully validated
     // TODO: Use the logic in ValidatorList to set this dynamically
-    std::size_t quorum = 0;
+    std::size_t q = 0;
 
     hash_set<NodeKey_t> trustedKeys;
 
@@ -250,6 +261,16 @@ struct Peer
     //! The collectors to report events to
     CollectorRefs& collectors;
 
+    mutable std::recursive_mutex mtx;
+
+    std::optional<std::chrono::milliseconds> delay;
+
+    struct Null_test : public beast::unit_test::suite
+    {
+        void
+        run() override{};
+    };
+
     /** Constructor
 
         @param i Unique PeerID
@@ -496,7 +517,8 @@ struct Peer
     onClose(
         Ledger const& prevLedger,
         NetClock::time_point closeTime,
-        ConsensusMode mode)
+        ConsensusMode mode,
+        clock_type& clock)
     {
         issue(CloseLedger{prevLedger, openTxs});
 
@@ -508,7 +530,9 @@ struct Peer
                 TxSet::calcID(openTxs),
                 closeTime,
                 now(),
-                id));
+                id,
+                prevLedger.seq() + typename Ledger_t::Seq{1},
+                scheduler.clock()));
     }
 
     void
@@ -520,11 +544,10 @@ struct Peer
         ConsensusMode const& mode,
         Json::Value&& consensusJson)
     {
-        onAccept(
+        buildAndValidate(
             result,
             prevLedger,
             closeResolution,
-            rawCloseTimes,
             mode,
             std::move(consensusJson));
     }
@@ -532,10 +555,19 @@ struct Peer
     void
     onAccept(
         Result const& result,
-        Ledger const& prevLedger,
-        NetClock::duration const& closeResolution,
         ConsensusCloseTimes const& rawCloseTimes,
         ConsensusMode const& mode,
+        Json::Value&& consensusJson,
+        std::pair<CanonicalTxSet_t, Ledger_t>&& txsBuilt)
+    {
+    }
+
+    std::pair<CanonicalTxSet_t, Ledger_t>
+    buildAndValidate(
+        Result const& result,
+        Ledger_t const& prevLedger,
+        NetClock::duration const& closeResolution,
+        ConsensusMode const& mode,
         Json::Value&& consensusJson)
     {
         schedule(delays.ledgerAccept, [=, this]() {
@@ -599,6 +631,8 @@ struct Peer
                 startRound();
             }
         });
+
+        return {};
     }
 
     // Earliest allowed sequence number when checking for ledgers with more
@@ -694,8 +728,8 @@ struct Peer
 
         std::size_t const count = validations.numTrustedForLedger(ledger.id());
         std::size_t const numTrustedPeers = trustGraph.graph().outDegree(this);
-        quorum = static_cast<std::size_t>(std::ceil(numTrustedPeers * 0.8));
-        if (count >= quorum && ledger.isAncestor(fullyValidatedLedger))
+        q = static_cast<std::size_t>(std::ceil(numTrustedPeers * 0.8));
+        if (count >= q && ledger.isAncestor(fullyValidatedLedger))
         {
             issue(FullyValidateLedger{ledger, fullyValidatedLedger});
             fullyValidatedLedger = ledger;
@@ -850,7 +884,13 @@ struct Peer
         hash_set<NodeKey_t> keys;
         for (auto const p : trustGraph.trustedPeers(this))
             keys.insert(p->key);
-        return {quorum, keys};
+        return {q, keys};
+    }
+
+    std::size_t
+    quorum() const
+    {
+        return q;
     }
 
     std::size_t
@@ -973,6 +1013,70 @@ struct Peer
 
         return TxSet{res};
     }
+
+    LedgerMaster&
+    getLedgerMaster() const
+    {
+        Null_test test;
+        jtx::Env env(test);
+
+        return env.app().getLedgerMaster();
+    }
+
+    void
+    clearValidating()
+    {
+    }
+
+    bool
+    retryAccept(
+        Ledger_t const& newLedger,
+        std::optional<std::chrono::time_point<std::chrono::steady_clock>>&
+            start) const
+    {
+        return false;
+    }
+
+    std::recursive_mutex&
+    peekMutex() const
+    {
+        return mtx;
+    }
+
+    void
+    endConsensus() const
+    {
+    }
+
+    bool
+    validating() const
+    {
+        return false;
+    }
+
+    std::optional<std::chrono::milliseconds>
+    getValidationDelay() const
+    {
+        return delay;
+    }
+
+    void
+    setValidationDelay(
+        std::optional<std::chrono::milliseconds> vd = std::nullopt) const
+    {
+    }
+
+    std::optional<std::chrono::milliseconds>
+    getTimerDelay() const
+    {
+        return delay;
+    }
+
+    void
+    setTimerDelay(
+        std::optional<std::chrono::milliseconds> vd = std::nullopt) const
+    {
+    }
 };
 
 }  // namespace csf

src/test/csf/Proposal.h

@@ -30,7 +30,7 @@ namespace csf {
 /** Proposal is a position taken in the consensus process and is represented
     directly from the generic types.
 */
-using Proposal = ConsensusProposal<PeerID, Ledger::ID, TxSet::ID>;
+using Proposal = ConsensusProposal<PeerID, Ledger::ID, TxSet::ID, Ledger::Seq>;
 
 }  // namespace csf
 }  // namespace test