#36694 test, bench: construct mempool inputs/outputs declaratively

ReviewabilityReviewsAgreementSize
#36694 alexanderwiederin test, bench: construct mempool inputs/outputs declarativelyReady0Crickets0 + 132 tests
  • Make transaction setup in mempool tests and benchmarks less verbose and easier to maintain
  • Prepare test call sites for potential direct immutable transaction construction
  • Ready for review
  • No review verdicts recorded
  • No substantive reviews yet
  • Suggested waiting for issue #36692 and conflicting PR #36521 before proceeding (maflcko)
  • 0 lines added or modified outside tests
  • 132 lines added or modified in tests
  • 155 lines removed in total
  • 3 files, 3 commits
Test infrastructure#46 of 54
P4 · cleanup
  • P4 because it is an internal cleanup of test and benchmark code
  • It improves transaction setup syntax without fixing bugs or adding test coverage
  • Ranking pass: Moved up ahead of #35994: it is the manual approach the #35994 author now prefers for the same #36692 goal.
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

Refactors transaction construction in mempool unit tests and benchmarks to use declarative constructor syntax for inputs and outputs instead of resizing vectors and setting fields manually. It modifies `mempool_tests`, `rbf_tests`, and the `mempool_eviction` benchmark without altering transaction data. This standardizes transaction setup and prepares call sites for potential direct `CTransaction` construction.

Problem: Transaction setup in mempool tests and benchmarks relies on imperative, field-by-field mutation of `CMutableTransaction` fields and vector resizing, which is verbose and complicates future transitions to immutable transaction constructors.

Evidence of importance:

  • PR description notes this is part of #36692 ('Declarative construction of test data') to ease future direct construction of CTransaction instances.

Risk of a mistake: low (tests). Touches only unit tests and benchmarks without altering consensus, validation, or node runtime logic.

Changed since the previous assessment: Diff provided; the patch matches the earlier high-level description across the three files.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

Agreement

Crickets

no substantive comment either way

Thread notes: Initial record. maflcko suggested waiting on related issue #36692 and conflicting PR #36521. / Initial record. maflcko suggested waiting for resolution on issue #36692 and PR #36521 before proceeding.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
maflcko (member)suggestion, approachWait for the outcome of issue #36692 and for conflicting PR #36521 to merge before proceeding.openansweredno2026-10-29: 'Let's wait a week or two on that issue. Also, this pull conflicts with https://github.com/bitcoin/bitcoin/pull/36521, so let's wait for that to be merged first.'

Participants:

  • maflcko (member, neutral, 1 statements 2026-10-29 to 2026-10-29): suggested waiting for issue #36692 and PR #36521

Files

287 lines under test/bench/ci.

  • src/test/mempool_tests.cpp +79/-90
  • src/bench/mempool_eviction.cpp +50/-56
  • src/test/rbf_tests.cpp +3/-9

Card

The short text the ranking pass compares across PRs.

This PR refactors transaction creation across mempool unit tests and benchmarks to use declarative CTxIn and CTxOut constructor calls rather than manual vector resizing and field assignments. It covers mempool_tests, rbf_tests, and the mempool_eviction benchmark as part of #36692. The change is restricted to tests and benchmarks, alters no transaction semantics, and simplifies future conversion to immutable CTransaction constructors.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-31T01:14, confidence high, input hash 3565b8e370d04c9f
thread read through 2026-10-29 (1 statements), head 30ad3ca75d
labels: none

Categories

Test infrastructure

P4 · cleanup (ranking pass: Moved up ahead of #35994: it is the manual approach the #35994 author now prefers for the same #36692 goal.)

Refactors test and benchmark code to use declarative constructors rather than manual vector resizing and field mutations. In the test infrastructure category, test code cleanliness ranks lowest as an internal refactoring without bug fixes, signal improvements, or performance gains.

Membership: Touches src/bench/mempool_eviction.cpp and refactors transaction construction syntax across unit tests and benchmarks.

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