diff --git a/src/ripple/app/misc/NegativeUNLVote.cpp b/src/ripple/app/misc/NegativeUNLVote.cpp index 01d1241d6..fba02637e 100644 --- a/src/ripple/app/misc/NegativeUNLVote.cpp +++ b/src/ripple/app/misc/NegativeUNLVote.cpp @@ -167,7 +167,7 @@ NegativeUNLVote::buildScoreTable( // Ask the validation container to keep enough validation message history // for next time. auto const seq = prevLedger->info().seq + 1; - validations.setSeqToKeep(seq - 1); + validations.setSeqToKeep(seq - 1, seq + FLAG_LEDGER_INTERVAL); // Find FLAG_LEDGER_INTERVAL (i.e. 256) previous ledger hashes auto const hashIndex = prevLedger->read(keylet::skip()); diff --git a/src/ripple/consensus/Validations.h b/src/ripple/consensus/Validations.h index f883f773e..2cbddaa51 100644 --- a/src/ripple/consensus/Validations.h +++ b/src/ripple/consensus/Validations.h @@ -324,8 +324,13 @@ class Validations beast::uhash<>> bySequence_; - // Sequence of the earliest validation to keep from expire - std::optional toKeep_; + // A range [low_, high_) of validations to keep from expire + struct KeepRange + { + Seq low_; + Seq high_; + }; + std::optional toKeep_; // Represents the ancestry of validated ledgers LedgerTrie trie_; @@ -698,14 +703,17 @@ public: } /** - * Set the smallest sequence number of validations to keep from expire - * @param s the sequence number + * Set the range [low, high) of validations to keep from expire + * @param low the lower sequence number + * @param high the higher sequence number + * @note high must be greater than low */ void - setSeqToKeep(Seq const& s) + setSeqToKeep(Seq const& low, Seq const& high) { std::lock_guard lock{mutex_}; - toKeep_ = s; + assert(low < high); + toKeep_ = {low, high}; } /** Expire old validation sets @@ -719,21 +727,35 @@ public: std::lock_guard lock{mutex_}; if (toKeep_) { - for (auto i = byLedger_.begin(); i != byLedger_.end(); ++i) + // We only need to refresh the keep range when it's just about to + // expire. Track the next time we need to refresh. + static std::chrono::steady_clock::time_point refreshTime; + if (auto const now = byLedger_.clock().now(); refreshTime <= now) { - auto const& validationMap = i->second; - if (!validationMap.empty() && - validationMap.begin()->second.seq() >= toKeep_) - { - byLedger_.touch(i); - } - } + // The next refresh time is shortly before the expiration + // time from now. + refreshTime = now + parms_.validationSET_EXPIRES - + parms_.validationFRESHNESS; - for (auto i = bySequence_.begin(); i != bySequence_.end(); ++i) - { - if (i->first >= toKeep_) + for (auto i = byLedger_.begin(); i != byLedger_.end(); ++i) { - bySequence_.touch(i); + auto const& validationMap = i->second; + if (!validationMap.empty()) + { + auto const seq = validationMap.begin()->second.seq(); + if (toKeep_->low_ <= seq && seq < toKeep_->high_) + { + byLedger_.touch(i); + } + } + } + + for (auto i = bySequence_.begin(); i != bySequence_.end(); ++i) + { + if (toKeep_->low_ <= i->first && i->first < toKeep_->high_) + { + bySequence_.touch(i); + } } } } diff --git a/src/test/consensus/Validations_test.cpp b/src/test/consensus/Validations_test.cpp index 4dc1df7de..beb43421c 100644 --- a/src/test/consensus/Validations_test.cpp +++ b/src/test/consensus/Validations_test.cpp @@ -705,23 +705,49 @@ class Validations_test : public beast::unit_test::suite testcase("Expire validations"); LedgerHistoryHelper h; TestHarness harness(h.oracle); - Node a = harness.makeNode(); + Node const a = harness.makeNode(); + constexpr Ledger::Seq one(1); + constexpr Ledger::Seq two(2); - Ledger ledgerA = h["a"]; + // simple cases + Ledger const ledgerA = h["a"]; BEAST_EXPECT(ValStatus::current == harness.add(a.validate(ledgerA))); - BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerA.id())); - - // Keep the validation from expire - harness.clock().advance(harness.parms().validationSET_EXPIRES); - harness.vals().setSeqToKeep(ledgerA.seq()); + BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerA.id()) == 1); harness.vals().expire(); - BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerA.id())); - - // Allow the validation to expire + BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerA.id()) == 1); harness.clock().advance(harness.parms().validationSET_EXPIRES); - harness.vals().setSeqToKeep(++ledgerA.seq()); harness.vals().expire(); - BEAST_EXPECT(!harness.vals().numTrustedForLedger(ledgerA.id())); + BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerA.id()) == 0); + + // use setSeqToKeep to keep the validation from expire + Ledger const ledgerB = h["ab"]; + BEAST_EXPECT(ValStatus::current == harness.add(a.validate(ledgerB))); + BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerB.id()) == 1); + harness.vals().setSeqToKeep(ledgerB.seq(), ledgerB.seq() + one); + harness.clock().advance(harness.parms().validationSET_EXPIRES); + harness.vals().expire(); + BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerB.id()) == 1); + // change toKeep + harness.vals().setSeqToKeep(ledgerB.seq() + one, ledgerB.seq() + two); + // advance clock slowly + int const loops = harness.parms().validationSET_EXPIRES / + harness.parms().validationFRESHNESS + + 1; + for (int i = 0; i < loops; ++i) + { + harness.clock().advance(harness.parms().validationFRESHNESS); + harness.vals().expire(); + } + BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerB.id()) == 0); + + // Allow the validation with high seq to expire + Ledger const ledgerC = h["abc"]; + BEAST_EXPECT(ValStatus::current == harness.add(a.validate(ledgerC))); + BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerC.id()) == 1); + harness.vals().setSeqToKeep(ledgerC.seq() - one, ledgerC.seq()); + harness.clock().advance(harness.parms().validationSET_EXPIRES); + harness.vals().expire(); + BEAST_EXPECT(harness.vals().numTrustedForLedger(ledgerC.id()) == 0); } void