From cfa2fd06ca9c10d39980ee4643791ec50b8b2f1c Mon Sep 17 00:00:00 2001
From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Date: Tue, 28 Jul 2026 17:44:15 +0100
Subject: [PATCH] fix(ledger): count acquisitions satisfied from the local
 store

recordCompletion() was reached only from done(), but init() can satisfy an
acquisition entirely from the local store and return without ever calling
done(). Those completions went uncounted, so acquire_completions read zero
for a whole class of outcome. On a live clean-database sync it read 0 for
510 seconds while the node reached full, which was misread as the node
completing nothing.

Counted in init() rather than by calling done(), because done() drives the
state machine: it stores the ledger, dispatches AcqDone, updates the
fetch-rate average, and sets signaled_, which would disarm a later genuine
done(). A counter must not change behaviour. A completionCounted_ latch,
separate from signaled_, makes the count idempotent across the two
independent exits so a completion is counted exactly once however it was
reached.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/xrpld/app/ledger/InboundLedger.h          | 25 +++++++++++++
 src/xrpld/app/ledger/detail/InboundLedger.cpp | 37 ++++++++++++++++---
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/src/xrpld/app/ledger/InboundLedger.h b/src/xrpld/app/ledger/InboundLedger.h
index c0ea5907c6..0cac2719a6 100644
--- a/src/xrpld/app/ledger/InboundLedger.h
+++ b/src/xrpld/app/ledger/InboundLedger.h
@@ -143,6 +143,21 @@ private:
     void
     done();
 
+    /**
+     * Count this acquisition as completed, at most once.
+     *
+     * done() is not the only place an acquisition finishes: init() can satisfy
+     * one entirely from the local store and return without ever reaching
+     * done(). Both call this, and the completionCounted_ latch makes the
+     * second call a no-op, so a completion is counted exactly once however it
+     * was reached.
+     *
+     * Does nothing unless the acquisition actually succeeded, so a failed or
+     * still-running acquisition is never counted.
+     */
+    void
+    recordCompletionOnce();
+
     void
     onTimer(bool progress, ScopedLockType& peerSetLock) override;
 
@@ -181,6 +196,16 @@ private:
     bool haveState_{false};
     bool haveTransactions_{false};
     bool signaled_{false};
+    /**
+     * Whether this acquisition has already been counted as completed.
+     *
+     * Separate from signaled_ because the two guard different things:
+     * signaled_ makes the done() state machine idempotent, while this makes
+     * the counter idempotent across the two independent exits that finish an
+     * acquisition (done() and init()'s local-hit path). Always accessed under
+     * mtx_.
+     */
+    bool completionCounted_{false};
     bool byHash_{true};
     std::uint32_t seq_;
     Reason const reason_;
diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp
index b03135e818..0f01b22b7b 100644
--- a/src/xrpld/app/ledger/detail/InboundLedger.cpp
+++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp
@@ -148,6 +148,14 @@ InboundLedger::init(ScopedLockType& collectionLock)
         "xrpl::InboundLedger::init : valid ledger fees");
     ledger_->setImmutable();
 
+    // The local store satisfied the whole acquisition, so this is a genuine
+    // completion. It is counted here rather than by calling done(), because
+    // done() drives the state machine (it stores the ledger, dispatches
+    // AcqDone, and would double-store on the paths below) and a counter must
+    // not change behaviour. recordCompletionOnce() is idempotent, so if
+    // done() is later reached for this same object the count stays at one.
+    recordCompletionOnce();
+
     if (reason_ == Reason::HISTORY)
         return;
 
@@ -449,6 +457,25 @@ InboundLedger::pmDowncast()
     return shared_from_this();
 }
 
+void
+InboundLedger::recordCompletionOnce()
+{
+    // A failed or still-running acquisition is not a completion. Checked here
+    // rather than at each caller so both exits share one definition of
+    // success.
+    if (!complete_ || failed_)
+        return;
+
+    // The latch, not the counter, is what makes this idempotent: the two
+    // exits that finish an acquisition are independent, and either can run
+    // first.
+    if (completionCounted_)
+        return;
+
+    completionCounted_ = true;
+    app_.getAcquireStats().recordCompletion();
+}
+
 void
 InboundLedger::done()
 {
@@ -458,12 +485,12 @@ InboundLedger::done()
     signaled_ = true;
     touch();
 
-    // Counted here rather than at any single caller because done() is the one
-    // funnel every peer-driven outcome passes through, and the signaled_ guard
-    // above makes it run at most once per acquisition. failed_ outcomes are
+    // done() is the funnel every peer-driven outcome passes through, but not
+    // every outcome: init() can satisfy an acquisition from the local store
+    // and return without reaching here. Both call the same idempotent helper
+    // so each completion is counted exactly once. failed_ outcomes are
     // excluded; the give-up path counts those itself.
-    if (complete_ && !failed_)
-        app_.getAcquireStats().recordCompletion();
+    recordCompletionOnce();
 
     // Finalize the acquire span with the outcome, timeout count, and peer
     // count. Keep it active as the ambient context across the finalize and