`BlockAssembler` underfill blocks and mixes reserve weights #35596

BlockAssembler fills a block template greedily. It pulls the best chunk from the TxGraph builder via CTxMemPool::GetBlockBuilderChunk, checks it with TestChunkBlockLimits and TestChunkTransactions, then either adds the chunk to the template or skips it. After enough consecutive skips near the weight ceiling, it gives up.

There are three issues with the current BlockAssembler.

1. >= limit checks make the consensus boundary unreachable

TestChunkBlockLimits currently rejects chunks that would make the block exactly reach the configured weight or sigops limit:

if (nBlockWeight + chunk_feerate.size >= *m_options.block_max_weight) {
    return false;
}
if (nBlockSigOpsCost + chunk_sigops_cost >= MAX_BLOCK_SIGOPS_COST) {
    return false;
}

Consensus rejects blocks only when these limits are exceeded:

• GetBlockWeight(block) > MAX_BLOCK_WEIGHT is invalid, so == MAX_BLOCK_WEIGHT is valid.
• nSigOps * WITNESS_SCALE_FACTOR > MAX_BLOCK_SIGOPS_COST is invalid, so == MAX_BLOCK_SIGOPS_COST is valid.

This means a chunk that lands the block exactly on block_max_weight or MAX_BLOCK_SIGOPS_COST is skipped even though the resulting block would be consensus-valid. As a result, the full weight and sigops allowances are unreachable.

2. The weight check uses sigop-adjusted weight, while accounting uses actual weight

chunk_feerate.size from GetBlockBuilderChunk is the sigop-adjusted weight stored in TxGraph:

FeePerWeight feerate(fee, GetSigOpsAdjustedWeight(GetTransactionWeight(*tx),
                                                  sigops_cost, ::nBytesPerSigOp));

int64_t GetSigOpsAdjustedWeight(int64_t weight, int64_t sigop_cost, unsigned int bytes_per_sigop)
{
    return std::max(weight, sigop_cost * bytes_per_sigop); // default bytes_per_sigop = 20
}

But AddToBlock accounts for actual transaction weight:

nBlockWeight += entry.GetTxWeight();

So TestChunkBlockLimits compares sigop-adjusted weight against block_max_weight.

This can cause BlockAssembler to skip chunks whose actual weight fits, potentially leaving higher-fee transactions out of the template.

This is fixed in #35580.

3. Block reserved weight mixes consensus-determined and caller-determined weight

A CBlock is a CBlockHeader plus vtx. Its weight is computed as:

weight = GetSerializeSize(TX_NO_WITNESS(block)) * (WITNESS_SCALE_FACTOR - 1)
       + GetSerializeSize(TX_WITH_WITNESS(block));

With WITNESS_SCALE_FACTOR = 4, this is:

base_bytes * 4 + witness_bytes

The block header is 80 base bytes, or 320 WU:

resetBlock seeds the block with reserved weight. The reserved components include:

The problem is that -blockreservedweight combines two different categories of weight. Consensus, which is pre-determined all the above except "Coinbase legacy size " which is a user-determined weight. They can instead be categorized into

consensus reserved weight (added at the beginning of blockassembler):

• block header: 320 WU
• tx-count CompactSize 12 WU
• coinbase marker + flag: 2 WU
• witness commitment: 34 WU

user-determined weight (-blockreservedweight)

coinbase transaction legacy size * 4 (the coinbase transaction serialized with TX_NO_WITNESS)

Issues 1 and 2 are straightforward. Issue 2 already has a PR: #35580.

For issue 3, the reserve accounting could be clarified by

i. Reserve the consensus-determined weight inside BlockAssembler instead of accounting for it through -blockreservedweight.

ii. Redefine MINIMUM_BLOCK_RESERVED_WEIGHT so that it represents only the minimum caller-determined coinbase transaction legacy weight.

iii). Redefine DEFAULT_BLOCK_RESERVED_WEIGHT as a conservative default for caller-determined coinbase transaction legacy weight.

With issues 1 and 2 fixed, and reserve accounting separated this way, an exactly filled consensus-valid block template could become achievable.

I am curious whether this approach is compatible with clients such as Stratum V2 (I assume it is because most of the fields it needs from the block assembler (CoinbaseTx) size can be known before calling createNewBlock) and DATUM, and whether I am missing something.

cc @Sjors @gmaxwell

❤️