#36156 mining: include chunks that reach block limits

ReviewabilityReviewsAgreementSize
#36156 l0rinc mining: include chunks that reach block limitsReady0Mild16 + 16 tests
  • Allow block templates to include transactions that bring block weight or sigops exactly to limits
  • Prevents leaving valid block capacity unused and dropping transactions that fit consensus boundaries
  • Reviewing now is worthwhile
  • Nothing known that would invalidate a review
  • Concept approval to match consensus limits so pools do not need to patch templates themselves (ismaelsadeeq)
  • Objection: off-by-one errors risk producing invalid blocks and losing the reward for tiny fee gains (ajtowns)
  • Author added boundary tests and clarified miner budget docs, awaiting objector follow-up
  • 16 lines added or modified outside tests
  • 16 lines added or modified in tests
  • 15 lines removed in total
  • 5 files, 5 commits
Mining#15 of 17
P3 · bug fix
  • P3 because the fee upside from fitting an extra transaction at exact limit boundaries is tiny
  • Aligning template building with consensus limits is correct but brings marginal real-world benefit
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

Switches `BlockAssembler::TestChunkBlockLimits` comparisons from `>=` to `>` for block weight and sigop costs. This allows block templates to include transactions that bring accounted weight or sigops exactly to their configured maximum limits.

Problem: `BlockAssembler` rejects mempool chunks that would bring total block weight to `block_max_weight` or total sigops to `MAX_BLOCK_SIGOPS_COST` exactly, even though BIP 141 defines both consensus limits inclusively (≤). This leaves valid block capacity unused and drops fee-paying transactions that fit exact limits.

Evidence of importance:

  • PR description notes that BIP 141 defines both consensus limits using ≤, making exact equality valid.
  • PR description cites an unaddressed 2016 review comment on PR #7600 pointing out the same inclusive-limit issue.
  • Addresses part of issue #35596 (`BlockAssembler underfill blocks and mixes reserve weights`).

Risk of a mistake: medium (other). A boundary error in miner block limit calculations could cause miners to produce invalid blocks exceeding consensus limits that get rejected by the network.

Changed since the previous assessment: Added release notes and documentation updates clarifying that miners must reserve adequate weight and sigop budgets for coinbase construction, and updated unit tests.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: response from ajtowns or other reviewers to author's rationale and boundary tests 2026-09-28

Agreement

Mild

objection that needs a change was answered, no reply from the objector (ajtowns: off-by-one errors could produce an invalid block and forfeit the entire block reward for negligible fee upside)

Thread notes: The author responded to ajtowns' objection explaining the consistency rationale with existing mining options, mentioning added boundary tests, and asking for further review.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
ajtowns (member)objection, safetyoff-by-one errors could produce an invalid block and forfeit the entire block reward for negligible fee upsideanswered · author replied defending consistency across mining options, noting boundary tests were added, and requesting feedback · v1, not yet re-checkedchangedyes2026-09-03: 'the risk of a mistake here is that the block becomes invalid losing all possible reward from mining it, while the benefit of success is perhaps a single larger / additional transaction'
Status from: 2026-09-28: l0rinc: 'I understand the concern about risking an invalid block for such a small gain, so I added test cases one unit over each limit and clarified the existing reservations in the docs.'

Support:

  • ismaelsadeeq (member) (Concept ACK, 2026-09-03): block assembler code should be correct and reflect consensus limits rather than incentivizing pools to patch binaries themselves

Participants:

  • ajtowns (member, objection, 2 statements 2026-09-03 to 2026-09-03): objects that removing the defensive buffer against off-by-one errors risks producing an invalid block for negligible fee gains
  • ismaelsadeeq (member, support, 1 statements 2026-09-03 to 2026-09-03): Concept ACK, arguing block assembly should reflect consensus limits to prevent pools patching it themselves

Files

21 lines under test/bench/ci.

  • src/test/miner_tests.cpp +16/-5
  • src/node/mining_types.h +8/-7
  • doc/release-notes-36156.md +5/-0
  • src/node/miner.cpp +2/-2
  • src/init.cpp +1/-1

Card

The short text the ranking pass compares across PRs.

This PR modifies BlockAssembler to use strict greater-than checks when testing whether a mempool chunk exceeds block weight or sigops limits. Currently, checks use greater-than-or-equal, preventing chunks from filling the template to its exact maximum capacity despite BIP 141 consensus limits permitting equality. The change allows exact-fit transactions to be included, resolving an off-by-one underfill issue in block template creation.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-29T14:29, confidence high, input hash 69d67d3e5a5d2e57
thread read through 2026-09-28 (4 statements), head 8e7de8ef71
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Mining

Categories

Mining

P3 · bug fix

Corrects an off-by-one limit check where BlockAssembler rejects chunks that reach block weight and sigop limits exactly. While aligning block template construction with BIP 141 consensus boundaries is correct, the fee upside is tiny and the benefit does not justify higher priority.

Membership: Has the Mining label and modifies block template construction in src/node/miner.cpp and src/node/mining_types.h.

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