From dd7c52c368d6ff31be7ec17f1aa26cdec390e73a Mon Sep 17 00:00:00 2001
From: Bart <11445373+bthomee@users.noreply.github.com>
Date: Sat, 22 Aug 2026 22:11:11 -0400
Subject: [PATCH] fix: Signal an InboundLedger that fails on local data

tryDB() can decide an acquisition can never succeed (a header hash/sequence
mismatch, or a zero account hash) without ever calling done(), so nothing
signals whatever is waiting, and logFailure() never records the hash in
recentFailures_ - the next round asks for the same doomed ledger again.
init() and trigger() now call done() on that path too, matching
checkLocal(), which already did.
---
 src/test/app/InboundLedger_test.cpp           | 66 +++++++++++++++++++
 src/xrpld/app/ledger/detail/InboundLedger.cpp |  7 ++
 2 files changed, 73 insertions(+)

diff --git a/src/test/app/InboundLedger_test.cpp b/src/test/app/InboundLedger_test.cpp
index 92673c2518..e5091f5418 100644
--- a/src/test/app/InboundLedger_test.cpp
+++ b/src/test/app/InboundLedger_test.cpp
@@ -45,6 +45,16 @@ struct TestableInboundLedger final : InboundLedger
         ScopedLockType collectionLock(collectionMutex);
         init(collectionLock);
     }
+
+    /**
+     * Ask for more nodes, or judge what has been collected, as a fresh
+     * acquisition does.
+     */
+    void
+    triggerAdded()
+    {
+        trigger(nullptr, TriggerReason::Added);
+    }
 };
 
 struct InboundLedger_test : public beast::unit_test::Suite
@@ -229,6 +239,61 @@ struct InboundLedger_test : public beast::unit_test::Suite
         BEAST_EXPECT(!env.app().getInboundLedgers().isFailure(header.hash));
     }
 
+    /**
+     * An acquisition that fails on local data must still signal.
+     *
+     * Both entry points that reach tryDB() are covered, since without
+     * done() the object never signals, logFailure() never runs, and the
+     * hash never lands in recentFailures_ - so the same doomed ledger is
+     * asked for again on the next round. recentFailures_ is what the
+     * assertions watch, since it is the caller-visible consequence of
+     * having signalled.
+     *
+     * @param env The environment to run in.
+     */
+    void
+    testLocalFailureSignalsDone(jtx::Env& env)
+    {
+        testcase("An acquisition that fails locally still signals");
+
+        // A zero account hash is a ledger no acquisition can ever finish, and tryDB() says so as
+        // soon as it has the header.
+        auto const header = makeHeader(uint256{}, uint256{});
+        storeHeader(env, header);
+
+        BEAST_EXPECT(!env.app().getInboundLedgers().isFailure(header.hash));
+
+        // acquire() is the only caller of init(), and hands back nothing for a failed acquisition.
+        BEAST_EXPECT(
+            env.app().getInboundLedgers().acquire(
+                header.hash, header.seq, InboundLedger::Reason::GENERIC) == nullptr);
+
+        // The failure reached recentFailures_, which is what stops the next round asking again.
+        BEAST_EXPECT(waitFor([&] { return env.app().getInboundLedgers().isFailure(header.hash); }));
+
+        // The other route into tryDB(): a trigger() on an acquisition that has no header yet. A
+        // hash of its own, so the entry above cannot answer for it.
+        auto const otherHeader = makeHeader(uint256{1}, uint256{});
+        storeHeader(env, otherHeader);
+
+        auto viaTrigger = std::make_shared<TestableInboundLedger>(
+            env.app(),
+            otherHeader.hash,
+            otherHeader.seq,
+            InboundLedger::Reason::GENERIC,
+            stopwatch(),
+            std::make_unique<RequestCountingPeerSet>());
+
+        BEAST_EXPECT(!env.app().getInboundLedgers().isFailure(otherHeader.hash));
+
+        viaTrigger->triggerAdded();
+
+        BEAST_EXPECT(viaTrigger->isFailed());
+        BEAST_EXPECT(!viaTrigger->isComplete());
+        BEAST_EXPECT(
+            waitFor([&] { return env.app().getInboundLedgers().isFailure(otherHeader.hash); }));
+    }
+
     /**
      * The retry timer re-asks, then gives up and signals.
      *
@@ -298,6 +363,7 @@ struct InboundLedger_test : public beast::unit_test::Suite
         jtx::Env env{*this};
 
         testLocalLedgerCompletesAcquire(env);
+        testLocalFailureSignalsDone(env);
 
         // Last: the only case that waits out a whole timeout chain.
         testTimerRetriesThenGivesUp(env);
diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp
index 4d3d3780a3..5fa1f96ba1 100644
--- a/src/xrpld/app/ledger/detail/InboundLedger.cpp
+++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp
@@ -93,8 +93,13 @@ InboundLedger::init(ScopedLockType& collectionLock)
     collectionLock.unlock();
 
     tryDB(app_.getNodeFamily().db());
+    // Matches checkLocal(): without done() this object never signals, so logFailure() never runs
+    // and the hash never lands in recentFailures_.
     if (failed_)
+    {
+        done();
         return;
+    }
 
     if (!complete_)
     {
@@ -493,6 +498,8 @@ InboundLedger::trigger(std::shared_ptr<Peer> const& peer, TriggerReason reason)
         if (failed_)
         {
             JLOG(journal_.warn()) << " failed local for " << hash_;
+            // See init() for why done() must be called here rather than just returning.
+            done();
             return;
         }
     }