Revert #4505, #4760 (#4842)

* Revert "Optimize calculation of close time to avoid impasse and minimize gratuitous proposal changes (#4760)"

This reverts commit 8ce85a9750.

* Revert "Several changes to improve Consensus stability: (#4505)"

This reverts commit f259cc1ab6.

* Add missing include

---------

Co-authored-by: seelabs <scott.determan@yahoo.com>

src/test/consensus/Consensus_test.cpp

@@ -44,35 +44,34 @@ 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, delay, 1s, p, journal_));
+            true, 10, 10, 10, -10s, 10s, 1s, 1s, p, journal_));
         BEAST_EXPECT(shouldCloseLedger(
-            true, 10, 10, 10, 100h, 10s, 1s, delay, 1s, p, journal_));
+            true, 10, 10, 10, 100h, 10s, 1s, 1s, p, journal_));
         BEAST_EXPECT(shouldCloseLedger(
-            true, 10, 10, 10, 10s, 100h, 1s, delay, 1s, p, journal_));
+            true, 10, 10, 10, 10s, 100h, 1s, 1s, p, journal_));
 
         // Rest of network has closed
-        BEAST_EXPECT(shouldCloseLedger(
-            true, 10, 3, 5, 10s, 10s, 10s, delay, 10s, p, journal_));
+        BEAST_EXPECT(
+            shouldCloseLedger(true, 10, 3, 5, 10s, 10s, 10s, 10s, p, journal_));
 
         // No transactions means wait until end of internval
-        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_));
+        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_));
 
         // Enforce minimum ledger open time
-        BEAST_EXPECT(!shouldCloseLedger(
-            true, 10, 0, 0, 10s, 10s, 1s, delay, 10s, p, journal_));
+        BEAST_EXPECT(
+            !shouldCloseLedger(true, 10, 0, 0, 10s, 10s, 1s, 10s, p, journal_));
 
         // Don't go too much faster than last time
-        BEAST_EXPECT(!shouldCloseLedger(
-            true, 10, 0, 0, 10s, 10s, 3s, delay, 10s, p, journal_));
+        BEAST_EXPECT(
+            !shouldCloseLedger(true, 10, 0, 0, 10s, 10s, 3s, 10s, p, journal_));
 
-        BEAST_EXPECT(shouldCloseLedger(
-            true, 10, 0, 0, 10s, 10s, 10s, delay, 10s, p, journal_));
+        BEAST_EXPECT(
+            shouldCloseLedger(true, 10, 0, 0, 10s, 10s, 10s, 10s, p, journal_));
     }
 
     void

src/test/csf/Peer.h

@@ -19,9 +19,6 @@
 #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>
@@ -29,10 +26,6 @@
 #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>
@@ -40,7 +33,6 @@
 #include <test/csf/Validation.h>
 #include <test/csf/events.h>
 #include <test/csf/ledgers.h>
-#include <test/jtx/Env.h>
 
 namespace ripple {
 namespace test {
@@ -166,13 +158,10 @@ 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;
@@ -251,7 +240,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 q = 0;
+    std::size_t quorum = 0;
 
     hash_set<NodeKey_t> trustedKeys;
 
@@ -261,16 +250,6 @@ 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
@@ -517,8 +496,7 @@ struct Peer
     onClose(
         Ledger const& prevLedger,
         NetClock::time_point closeTime,
-        ConsensusMode mode,
-        clock_type& clock)
+        ConsensusMode mode)
     {
         issue(CloseLedger{prevLedger, openTxs});
 
@@ -530,9 +508,7 @@ struct Peer
                 TxSet::calcID(openTxs),
                 closeTime,
                 now(),
-                id,
-                prevLedger.seq() + typename Ledger_t::Seq{1},
-                scheduler.clock()));
+                id));
     }
 
     void
@@ -544,10 +520,11 @@ struct Peer
         ConsensusMode const& mode,
         Json::Value&& consensusJson)
     {
-        buildAndValidate(
+        onAccept(
             result,
             prevLedger,
             closeResolution,
+            rawCloseTimes,
             mode,
             std::move(consensusJson));
     }
@@ -555,18 +532,9 @@ struct Peer
     void
     onAccept(
         Result const& result,
-        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,
+        Ledger const& prevLedger,
         NetClock::duration const& closeResolution,
+        ConsensusCloseTimes const& rawCloseTimes,
         ConsensusMode const& mode,
         Json::Value&& consensusJson)
     {
@@ -631,8 +599,6 @@ struct Peer
                 startRound();
             }
         });
-
-        return {};
     }
 
     // Earliest allowed sequence number when checking for ledgers with more
@@ -728,8 +694,8 @@ struct Peer
 
         std::size_t const count = validations.numTrustedForLedger(ledger.id());
         std::size_t const numTrustedPeers = trustGraph.graph().outDegree(this);
-        q = static_cast<std::size_t>(std::ceil(numTrustedPeers * 0.8));
-        if (count >= q && ledger.isAncestor(fullyValidatedLedger))
+        quorum = static_cast<std::size_t>(std::ceil(numTrustedPeers * 0.8));
+        if (count >= quorum && ledger.isAncestor(fullyValidatedLedger))
         {
             issue(FullyValidateLedger{ledger, fullyValidatedLedger});
             fullyValidatedLedger = ledger;
@@ -884,13 +850,7 @@ struct Peer
         hash_set<NodeKey_t> keys;
         for (auto const p : trustGraph.trustedPeers(this))
             keys.insert(p->key);
-        return {q, keys};
-    }
-
-    std::size_t
-    quorum() const
-    {
-        return q;
+        return {quorum, keys};
     }
 
     std::size_t
@@ -1013,70 +973,6 @@ 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, Ledger::Seq>;
+using Proposal = ConsensusProposal<PeerID, Ledger::ID, TxSet::ID>;
 
 }  // namespace csf
 }  // namespace test