#36670 fuzz: check GetStale() returns exactly the stale private broadcast txs

ReviewabilityReviewsAgreementSize
#36670 frankomosh fuzz: check GetStale() returns exactly the stale private broadcast txsReady0Crickets0 + 26 tests
  • Ensure fuzz testing catches bugs where stale private broadcast transactions are silently omitted
  • Prevents unretried private broadcast transactions from slipping past tests unnoticed
  • Ready to review
  • No review verdicts recorded
  • Little feedback so far
  • Suggested also asserting that returned stale transactions contain no duplicates (andrewtoth)
  • 0 lines added or modified outside tests
  • 26 lines added or modified in tests
  • 0 lines removed in total
  • 1 files, 1 commits
P2P#64 of 71
P3 · test coverage
  • P3 because it closes a test gap in private broadcast retry logic without changing production code
  • Mutant testing showed silent omissions of stale transactions previously passed all existing tests
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Strengthens the `private_broadcast` fuzz target to verify that `PrivateBroadcast::GetStale()` returns the exact set of expected stale transactions. The harness now records transaction addition timestamps and peer confirmation times to compute expected staleness and asserts that the returned transactions match.

Problem: The existing fuzz harness verifies that returned transactions are valid candidates, but does not check for completeness. A bug or mutation in `GetStale()` that silently omits stale transactions (such as prematurely breaking out of iteration) would leave them unretried by `ReattemptPrivateBroadcast()` while still passing fuzz tests.

Evidence of importance:

  • PR description notes that in mutant testing (mutanthub.space/mutants/3752), changing `continue` to `break` in `GetStale()` passed all current tests.
  • PR description references earlier review discussion in PR #35129 where missing stale transactions was raised.

Scope: Small mechanical addition to a single fuzz target.

Risk of a mistake: low (tests). Touches only fuzz harness assertions and cannot affect production node behavior or consensus.

Changed since the previous assessment: Diff provided; implementation matches the earlier description.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • author: response to andrewtoth's suggestion on asserting no duplicates 2026-10-31

Agreement

Crickets

no substantive comment either way

Thread notes: andrewtoth suggested asserting that stale returns no duplicates in the fuzz test.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
andrewtoth (member)suggestion, correctnessAssert that stale does not return any duplicatesopenansweredno2026-10-31: 'We can also assert that stale does not return any duplicates.'

Participants:

  • fanquake (member, neutral, 1 statements 2026-10-26 to 2026-10-26): tagged reviewers
  • andrewtoth (member, neutral, 1 statements 2026-10-31 to 2026-10-31): suggested asserting that stale does not return duplicates

Files

26 lines under test/bench/ci.

  • src/test/fuzz/private_broadcast.cpp +26/-0

Card

The short text the ranking pass compares across PRs.

Strengthens the private broadcast fuzz target by tracking transaction addition and peer confirmation times to assert that GetStale() returns the exact expected set of stale transactions. The fuzz harness previously only checked that returned transactions were valid candidates, allowing mutations like changing continue to break to go undetected. Because ReattemptPrivateBroadcast() only retries transactions returned by GetStale(), any omitted transactions are silently abandoned.

Data

dossier JSON · extract JSON · model ledger, generated 2026-11-01T01:17, confidence high, input hash 8a2226d8b5601aa3
thread read through 2026-10-31 (2 statements), head 986cdf27b4
labels: Fuzzing, Private Broadcast

Categories

P2P

P3 · test coverage

Improves test coverage for private broadcast transaction retry logic by asserting that GetStale() returns the complete set of stale transactions. As noted in the description, mutant testing showed that replacing continue with break in GetStale() previously passed all tests without detection.

Membership: Has the Private Broadcast label and tests the behavior of P2P private broadcast transaction staleness tracking in src/test/fuzz/private_broadcast.cpp.

Factors: security/stability 1, bug 0, performance 0, user value 0, leverage 0