[FOLD] Change full validation sequence number invariant:

Require a new full validation to be for a sequence number larger than
any unexpired previously issued full validation sequence number from
that validator.

This is less strict than the earlier changeset which did not allow
expiration of what is the largest issued full validation sequence
number.

src/ripple/app/consensus/RCLConsensus.cpp

@@ -821,10 +821,11 @@ RCLConsensus::Adaptor::validate(RCLCxLedger const& ledger, bool proposing)
         validationTime = lastValidationTime_ + 1s;
     lastValidationTime_ = validationTime;
 
-    // Can only fully validate later sequenced ledgers
-    const bool isFull = proposing && ledger.seq() > largestFullValidationSeq_;
-    largestFullValidationSeq_ =
-        std::max(largestFullValidationSeq_, ledger.seq());
+    // Can only fully validate if proposed and increasing full sequence invariant
+    // satisfied
+    const bool isFull = proposing &&
+        fullSeqEnforcer_.tryAdvance(
+            stopwatch().now(), ledger.seq(), app_.getValidations().parms());
 
     // Build validation
     auto v = std::make_shared<STValidation>(
@@ -943,9 +944,6 @@ RCLConsensus::Adaptor::preStartRound(RCLCxLedger const & prevLgr)
                   prevLgr.seq() >= app_.getMaxDisallowedLedger() &&
                   !app_.getOPs().isAmendmentBlocked();
 
-    largestFullValidationSeq_ =
-        std::max(largestFullValidationSeq_, app_.getMaxDisallowedLedger());
-
     const bool synced = app_.getOPs().getOperatingMode() == NetworkOPs::omFULL;
 
     if (validating_)

src/ripple/app/consensus/RCLConsensus.h

@@ -29,6 +29,7 @@
 #include <ripple/basics/Log.h>
 #include <ripple/beast/utility/Journal.h>
 #include <ripple/consensus/Consensus.h>
+#include <ripple/consensus/Validations.h>
 #include <ripple/core/JobQueue.h>
 #include <ripple/overlay/Message.h>
 #include <ripple/protocol/RippleLedgerHash.h>
@@ -69,8 +70,8 @@ class RCLConsensus
         // The timestamp of the last validation we used
         NetClock::time_point lastValidationTime_;
 
-        // Largest sequence number fully validated
-        LedgerIndex largestFullValidationSeq_ = 0;
+        // Enforces invariants on issuing full validations
+        FullSeqEnforcer<LedgerIndex> fullSeqEnforcer_;
 
         // These members are queried via public accesors and are atomic for
         // thread safety.

src/ripple/consensus/Validations.h

@@ -73,6 +73,43 @@ struct ValidationParms
     std::chrono::seconds validationSET_EXPIRES = std::chrono::minutes{10};
 };
 
+/** Enforce full validation increasing sequence requirement.
+
+    Helper class for enforcing that a full validation must be larger than all
+    unexpired full validation sequence numbers previously received from the
+    issuing validator tracked by the instance of this class.
+*/
+template <class Seq>
+class FullSeqEnforcer
+{
+    using time_point = std::chrono::steady_clock::time_point;
+    Seq seq_{0};
+    time_point when_;
+public:
+    /** Try advancing the largest observed full validation ledger sequence
+
+        Try setting the largest full sequence observed, but return false if it
+        violates the invaraint that a full validation must be larger than all
+        unexpired full validation sequence numbers.
+
+        @param now The current time
+        @param s The sequence number we want to fully validate
+        @param p Validation parameters
+
+        @return Whether the validation can be marked full
+    */
+    bool
+    tryAdvance(time_point now, Seq s, ValidationParms const & p)
+    {
+        if(now > (when_ + p.validationSET_EXPIRES))
+            seq_ = Seq{0};
+        if(seq_ != Seq{0} && s <= seq_)
+            return false;
+        seq_ = s;
+        when_ = now;
+        return true;
+    }
+};
 /** Whether a validation is still current
 
     Determines whether a validation can still be considered the current
@@ -269,7 +306,7 @@ class Validations
     hash_map<NodeKey, Validation> current_;
 
     // Sequence of the largest full validation received from each node
-    hash_map<NodeKey, Seq> largestFullValidation_;
+    hash_map<NodeKey, FullSeqEnforcer<Seq>> fullSeqEnforcers_;
 
     //! Validations from listed nodes, indexed by ledger id (partial and full)
     beast::aged_unordered_map<
@@ -536,16 +573,14 @@ class Validations
         {
             ScopedLock lock{mutex_};
 
-            // Ensure full validations are for increasing sequence numbers
+            // Check that full validation is greater than any non-expired
+            // full validations
             if (val.full() && val.seq() != Seq{0})
             {
-                auto const ins = largestFullValidation_.emplace(key, val.seq());
-                if (!ins.second)
-                {
-                    if (val.seq() <= ins.first->second)
-                        return ValStatus::badFullSeq;
-                    ins.first->second = val.seq();
-                }
+                auto const now = byLedger_.clock().now();
+                FullSeqEnforcer<Seq>& enforcer = fullSeqEnforcers_[key];
+                if (!enforcer.tryAdvance(now, val.seq(), parms_))
+                    return ValStatus::badFullSeq;
             }
 
             // This validation is a repeat if we already have

src/test/consensus/Consensus_test.cpp

@@ -992,6 +992,7 @@ class Consensus_test : public beast::unit_test::suite
             BEAST_EXPECT(sim.synchronized(groupNotFastC) == 1);
         }
     }
+
     void
     run() override
     {

src/test/consensus/Validations_test.cpp

@@ -302,6 +302,7 @@ class Validations_test : public beast::unit_test::suite
         LedgerHistoryHelper h;
         Ledger ledgerA = h["a"];
         Ledger ledgerAB = h["ab"];
+        Ledger ledgerAZ = h["az"];
         Ledger ledgerABC = h["abc"];
         Ledger ledgerABCD = h["abcd"];
         Ledger ledgerABCDE = h["abcde"];
@@ -427,6 +428,14 @@ class Validations_test : public beast::unit_test::suite
                 ValStatus::badFullSeq == harness.add(n.validate(ledgerAB)));
             BEAST_EXPECT(
                 ValStatus::current == harness.add(n.partial(ledgerAB)));
+            // If we advance far enough for AB to expire, we can fully validate
+            // that sequence number again
+            BEAST_EXPECT(
+                ValStatus::badFullSeq == harness.add(n.validate(ledgerAZ)));
+            harness.clock().advance(
+                harness.parms().validationSET_EXPIRES + 1ms);
+            BEAST_EXPECT(
+                ValStatus::current == harness.add(n.validate(ledgerAZ)));
         }
     }
 
@@ -986,6 +995,28 @@ class Validations_test : public beast::unit_test::suite
         BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerA.id()) == 1);
     }
 
+    void
+    testFullSeqEnforcer()
+    {
+        testcase("FullSeqEnforcer");
+        using Seq = Ledger::Seq;
+        using namespace std::chrono;
+
+        beast::manual_clock<steady_clock> clock;
+        FullSeqEnforcer<Seq> enforcer;
+
+        ValidationParms p;
+
+        BEAST_EXPECT(enforcer.tryAdvance(clock.now(), Seq{1}, p));
+        BEAST_EXPECT(enforcer.tryAdvance(clock.now(), Seq{10}, p));
+        BEAST_EXPECT(!enforcer.tryAdvance(clock.now(), Seq{9}, p));
+        BEAST_EXPECT(!enforcer.tryAdvance(clock.now(), Seq{5}, p));
+        clock.advance(p.validationSET_EXPIRES - 1ms);
+        BEAST_EXPECT(!enforcer.tryAdvance(clock.now(), Seq{1}, p));
+        clock.advance(2ms);
+        BEAST_EXPECT(enforcer.tryAdvance(clock.now(), Seq{1}, p));
+    }
+
     void
     run() override
     {
@@ -1001,6 +1032,7 @@ class Validations_test : public beast::unit_test::suite
         testGetPreferredLCL();
         testAcquireValidatedLedger();
         testNumTrustedForLedger();
+        testFullSeqEnforcer();
     }
 };
 

src/test/csf/Peer.h

@@ -238,8 +238,8 @@ struct Peer
     //! Whether to simulate running as validator or a tracking node
     bool runAsValidator = true;
 
-    //! Sequence number of largest full validation thus far
-    Ledger::Seq largestFullValidation{0};
+    //! Enforce invariants on full validation sequence numbers
+    FullSeqEnforcer<Ledger::Seq> fullSeqEnforcer;
 
     //TODO: Consider removing these two, they are only a convenience for tests
     // Number of proposers in the prior round
@@ -575,10 +575,9 @@ struct Peer
             if (runAsValidator && isCompatible && !consensusFail)
             {
                 // Can only send one fully validated ledger per seq
-                bool isFull =
-                    proposing && newLedger.seq() > largestFullValidation;
-                largestFullValidation =
-                    std::max(largestFullValidation, newLedger.seq());
+                bool isFull = proposing &&
+                    fullSeqEnforcer.tryAdvance(
+                        scheduler.now(), newLedger.seq(), validations.parms());
 
                 Validation v{newLedger.id(),
                              newLedger.seq(),