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.
This commit is contained in:
Bart
2026-08-22 22:11:11 -04:00
parent d8754775b5
commit ddba2e7fce
2 changed files with 73 additions and 0 deletions

View File

@@ -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);