fix(telemetry): count each validated ledger once in ValidationTracker

Both record entry points reached their record with pending_[ledgerHash], and
operator[] default-constructs on a missing key. An all-zero recordTime was the
only signal for "first time seeing this ledger", so it could not tell a new
ledger apart from one already counted and evicted.

Reconciled entries are removed on purpose once they leave the late-repair
window, and reconcile() adds a ledger to the totals only on its first
reconcile. A validation arriving after its entry was counted and removed
therefore rebuilt a blank record, looked new, and was counted a second time -
in the lifetime totals and in all three rolling windows, so the agreement
percentages skewed too. A re-arrival carrying only one side manufactured a miss
for a ledger first counted as an agreement.

Both entry points now go through pendingEvent(), which looks in pending_
first so a ledger still awaiting late repair keeps updating its record, then
checks tallied_ and records nothing for a ledger already counted. That retires
the all-zero sentinel, which was the defect itself.

tallied_ holds hashes only, since the record is gone by then, and is bounded at
kMaxTalliedEvents with oldest-first eviction to match evictOldPending. A
validation for a ledger counted more than that many ledgers ago is counted
again; the comment says so.

The message counters keep incrementing on every call, including for an ignored
ledger. They count messages rather than ledgers.

kMaxPendingEvents becomes public so the new test sizes its fill from the
production constant instead of copying the number. Its value and meaning are
unchanged.

src/tests/libxrpl/telemetry/ValidationTracker.cpp

@@ -287,3 +287,74 @@ TEST_F(ValidationTrackerTest, OnlyWeValidated)
     EXPECT_EQ(tracker_.missed1h(), 1u);
     EXPECT_DOUBLE_EQ(tracker_.agreementPct1h(), 0.0);
 }
+
+// ---------------------------------------------------------------
+// 10. A counted ledger is never counted twice
+//     reconcile() drops the oldest reconciled events once the
+//     pending map passes kMaxPendingEvents. A validation arriving
+//     for one of those ledgers afterwards must not reach the
+//     agreement or missed totals a second time.
+//
+//     Two hashes are made the oldest so the trim drops both:
+//       - evictedMiss  (network only) reconciles as a miss. Our late
+//         validation cannot repair an entry the trim dropped, so
+//         totalMissed staying at 1 proves the trim really dropped
+//         it. A fixture where the trim did not run would repair it
+//         and report 0 misses.
+//       - evictedAgreed (both sides) reconciles as an agreement.
+//         Re-recording both sides is what double-counts an
+//         agreement.
+// ---------------------------------------------------------------
+TEST_F(ValidationTrackerTest, CountedLedgerNotCountedTwice)
+{
+    // The trim drops the oldest reconciled entries first. Each pause makes
+    // the next record time strictly larger, so these two are the oldest.
+    auto const evictedMiss = makeHash(1);
+    tracker_.recordNetworkValidation(evictedMiss, 1);
+    std::this_thread::sleep_for(std::chrono::milliseconds(5));
+
+    auto const evictedAgreed = makeHash(2);
+    tracker_.recordOurValidation(evictedAgreed, 2);
+    tracker_.recordNetworkValidation(evictedAgreed, 2);
+    std::this_thread::sleep_for(std::chrono::milliseconds(5));
+
+    // Fill to three over the bound so the trim drops three entries: the two
+    // above plus one filler.
+    constexpr std::size_t kFill = ValidationTracker::kMaxPendingEvents + 1;
+    for (std::size_t i = 0; i < kFill; ++i)
+    {
+        auto const hash = makeHash(i + 3);
+        auto const seq = static_cast<LedgerIndex>(i + 3);
+        tracker_.recordOurValidation(hash, seq);
+        tracker_.recordNetworkValidation(hash, seq);
+    }
+
+    std::this_thread::sleep_for(std::chrono::seconds(9));
+    tracker_.reconcile();
+
+    // Every filler plus evictedAgreed agrees; evictedMiss is the one miss.
+    EXPECT_EQ(tracker_.totalAgreements(), kFill + 1);
+    EXPECT_EQ(tracker_.totalMissed(), 1u);
+    EXPECT_EQ(tracker_.agreements1h(), kFill + 1);
+    EXPECT_EQ(tracker_.missed1h(), 1u);
+
+    // Validations arrive again for the two dropped ledgers.
+    tracker_.recordOurValidation(evictedMiss, 1);
+    tracker_.recordOurValidation(evictedAgreed, 2);
+    tracker_.recordNetworkValidation(evictedAgreed, 2);
+
+    // Long enough for a re-created pending entry to pass the grace period.
+    std::this_thread::sleep_for(std::chrono::seconds(9));
+    tracker_.reconcile();
+
+    // Both ledgers were already counted, so every total is unchanged.
+    EXPECT_EQ(tracker_.totalAgreements(), kFill + 1);
+    EXPECT_EQ(tracker_.totalMissed(), 1u);
+    EXPECT_EQ(tracker_.agreements1h(), kFill + 1);
+    EXPECT_EQ(tracker_.missed1h(), 1u);
+
+    // The send and check counters count messages, not ledgers, so the
+    // repeated validations do count towards them.
+    EXPECT_EQ(tracker_.totalValidationsSent(), kFill + 3);
+    EXPECT_EQ(tracker_.totalValidationsChecked(), kFill + 3);
+}

src/xrpld/telemetry/ValidationTracker.h

@@ -47,6 +47,7 @@ namespace xrpl::telemetry {
  *  |   ValidationTracker       |
  *  |---------------------------|
  *  |  pending_  (hash_map)     |----> LedgerEvent per hash
+ *  |  tallied_  (hash_set)     |----> hashes already counted
  *  |  window1h_ (deque)        |----> WindowEvent sliding window
  *  |  window24h_ (deque)       |----> WindowEvent sliding window
  *  |  atomic totals            |
@@ -104,6 +105,13 @@ public:
      */
     using TimePoint = Clock::time_point;
 
+    /**
+     * Maximum number of pending (unreconciled + recently reconciled) events.
+     * Once the pending map passes this size, reconcile() drops the oldest
+     * reconciled events. Public so a test can size a fixture against it.
+     */
+    static constexpr std::size_t kMaxPendingEvents = 1000;
+
     /**
      * Record that this node sent a validation for the given ledger.
      * @param ledgerHash Hash of the ledger we validated.
@@ -267,9 +275,12 @@ private:
     static constexpr auto kLateRepairWindow = std::chrono::minutes(5);
 
     /**
-     * Maximum number of pending (unreconciled + recently reconciled) events.
+     * Maximum number of ledger hashes remembered as already counted.
+     * At one ledger every four seconds this spans about eleven hours.
+     * A validation arriving for a ledger counted before that is counted
+     * again.
      */
-    static constexpr std::size_t kMaxPendingEvents = 1000;
+    static constexpr std::size_t kMaxTalliedEvents = 10000;
 
     /**
      * Duration of the short rolling window.
@@ -287,7 +298,8 @@ private:
     static constexpr auto kWindow7d = std::chrono::hours(168);
 
     /**
-     * Protects pending_, window1h_, window24h_, and window7d_.
+     * Protects pending_, tallied_, talliedOrder_, window1h_, window24h_,
+     * and window7d_.
      */
     mutable std::mutex mutex_;
 
@@ -296,6 +308,19 @@ private:
      */
     hash_map<uint256, LedgerEvent> pending_;
 
+    /**
+     * Ledger hashes already counted into the agreement and missed totals.
+     * Membership survives eviction from pending_, so a ledger reaches the
+     * totals once. Holds at most kMaxTalliedEvents hashes.
+     */
+    hash_set<uint256> tallied_;
+
+    /**
+     * The hashes in tallied_ in the order they were counted. The front is
+     * the oldest and is dropped first once the bound is reached.
+     */
+    std::deque<uint256> talliedOrder_;
+
     /**
      * Sliding window of reconciled events (last 1 hour).
      */
@@ -331,6 +356,27 @@ private:
      */
     std::atomic<uint64_t> totalValidationsChecked_{0};
 
+    /**
+     * Locate the pending event for a ledger, creating it on first sight.
+     * @param ledgerHash Hash of the ledger being recorded.
+     * @param seq        Ledger sequence number, stored only on creation.
+     * @return Pointer to the event, or nullptr for a ledger that already
+     * reached the totals and left pending_. The caller records nothing in
+     * that case.
+     * @note Called with mutex_ held.
+     */
+    [[nodiscard]] LedgerEvent*
+    pendingEvent(uint256 const& ledgerHash, LedgerIndex seq);
+
+    /**
+     * Remember a ledger hash as counted, dropping the oldest remembered
+     * hash once kMaxTalliedEvents is reached.
+     * @param ledgerHash Hash of the ledger just counted into the totals.
+     * @note Called with mutex_ held.
+     */
+    void
+    noteTallied(uint256 const& ledgerHash);
+
     /**
      * Remove entries older than their respective window durations.
      * @param now Current time point.

src/xrpld/telemetry/detail/ValidationTracker.cpp

@@ -17,35 +17,58 @@
 
 namespace xrpl::telemetry {
 
+ValidationTracker::LedgerEvent*
+ValidationTracker::pendingEvent(uint256 const& ledgerHash, LedgerIndex seq)
+{
+    if (auto const it = pending_.find(ledgerHash); it != pending_.end())
+        return &it->second;
+
+    // A hash in tallied_ already reached the totals and left pending_.
+    // Building a fresh record for it would count the same ledger twice.
+    if (tallied_.contains(ledgerHash))
+        return nullptr;
+
+    auto& evt = pending_[ledgerHash];
+    evt.ledgerHash = ledgerHash;
+    evt.seq = seq;
+    evt.recordTime = Clock::now();
+    return &evt;
+}
+
+void
+ValidationTracker::noteTallied(uint256 const& ledgerHash)
+{
+    if (!tallied_.insert(ledgerHash).second)
+        return;
+
+    talliedOrder_.push_back(ledgerHash);
+    while (talliedOrder_.size() > kMaxTalliedEvents)
+    {
+        tallied_.erase(talliedOrder_.front());
+        talliedOrder_.pop_front();
+    }
+}
+
 void
 ValidationTracker::recordOurValidation(uint256 const& ledgerHash, LedgerIndex seq)
 {
     std::scoped_lock const lock(mutex_);
-    auto& evt = pending_[ledgerHash];
-    if (evt.recordTime == TimePoint{})
-    {
-        // First time seeing this ledger hash -- initialize.
-        evt.ledgerHash = ledgerHash;
-        evt.seq = seq;
-        evt.recordTime = Clock::now();
-    }
-    evt.weValidated = true;
     totalValidationsSent_.fetch_add(1, std::memory_order_relaxed);
+
+    // The counter above counts messages, so it also counts a ledger that is
+    // already tallied. Only the per-ledger record is skipped.
+    if (auto* const evt = pendingEvent(ledgerHash, seq))
+        evt->weValidated = true;
 }
 
 void
 ValidationTracker::recordNetworkValidation(uint256 const& ledgerHash, LedgerIndex seq)
 {
     std::scoped_lock const lock(mutex_);
-    auto& evt = pending_[ledgerHash];
-    if (evt.recordTime == TimePoint{})
-    {
-        evt.ledgerHash = ledgerHash;
-        evt.seq = seq;
-        evt.recordTime = Clock::now();
-    }
-    evt.networkValidated = true;
     totalValidationsChecked_.fetch_add(1, std::memory_order_relaxed);
+
+    if (auto* const evt = pendingEvent(ledgerHash, seq))
+        evt->networkValidated = true;
 }
 
 void
@@ -61,6 +84,7 @@ ValidationTracker::reconcile()
             // Initial reconciliation after grace period.
             evt.reconciled = true;
             evt.agreed = evt.weValidated && evt.networkValidated;
+            noteTallied(hash);
 
             if (evt.agreed)
             {