Skip to content

rpc: fill the coinbase witness reserved value in submitblock like Core - #1066

Merged
reardencode merged 3 commits into
reardencode:masterfrom
smolcars:rpc/submitblock-witness-nonce
Oct 18, 2026
Merged

reardencode merged 3 commits into
reardencode:masterfrom
smolcars:rpc/submitblock-witness-nonce

Conversation

@niteshbalusu11

@niteshbalusu11 niteshbalusu11 commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Bitcoin Core's submitblock adds the coinbase witness reserved value (32 zero bytes) when the block has a witness commitment but the coinbase has no witness. It only does this once segwit is active. See UpdateUncommittedBlockStructures in Core's rpc/mining.cpp.

rbitcoin did not do this. The same block was rejected with bad-witness-nonce-size. Mining software that leaves the coinbase witness out would lose the block.

 submit_received_block
   prev-blk-not-found check
+  fill_witness_reserved_value   # Core UpdateUncommittedBlockStructures
   cheap_submit_tx_reject
   accept_received_block

The fill is skipped below segwit and when the coinbase already has a witness, like Core.

Evidence

Same block bytes sent to Core v31.1 and rbitcoin on regtest:

Block Core 31.1 rbitcoin before rbitcoin after
commitment, coinbase witness left out accepted bad-witness-nonce-size accepted
same, plus a segwit spend accepted bad-witness-nonce-size accepted

Core's own p2p_segwit.py test_submit_block now passes too. The only difference left is the reject string for a wrong nonce (witness commitment mismatch vs bad-witness-merkle-match).

New test submitblock_adds_the_witness_reserved_value_once_segwit_is_active (segwit@2):

  • Before: height 2 fails with bad-witness-nonce-size.
  • After: height 1 stays bare, height 2 gets 32 zero bytes, height 3 keeps the miner's own value, height 4 (no commitment) stays bare, and height 6 is filled even when its parent is only a stored header (headers first).

Merge Danger

Door: two-way

Blast Radius: small. Only submitblock. P2P block handling does not change.

🤖 Generated with Claude Code

niteshbalusu11 and others added 2 commits October 18, 2026 02:44
Core's submitblock adds 32 zero bytes as the coinbase witness when the
block has a witness commitment, the coinbase has no witness, and segwit
is active for the block (UpdateUncommittedBlockStructures). rbitcoin
rejected the same block as bad-witness-nonce-size.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review: `header_height` drops a header-only parent once its child's
header arrives, so the fill was skipped where Core's LookupBlockIndex
finds the parent. Use `stored_header_height`. The test also pins a
block with no commitment and the headers-first case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

submitblock now inserts the BIP141 coinbase witness reserved value (one 32-byte zero stack item) before the usual accept path. That matches Core UpdateUncommittedBlockStructures: only when segwit is active for this block, the coinbase has a witness-commitment output, and the coinbase has no witness. An existing reserved value is left alone, a pre-segwit commitment stays bare data, and P2P accept_received_block does not grow a second fill. The reserved value is always zeros, so a wrong parent cannot invent a different nonce. The gap is fail-closed: an unresolved parent height is treated as segwit-off, and the block this PR exists to accept is still rejected with bad-witness-nonce-size.

Issue counts by severity

  • bugs: 0
  • suggestions: 1
  • nits: 1

Comment thread crates/rbitcoin-rpc/src/methods/mine.rs
Comment thread crates/rbitcoin-rpc/src/methods_tests.rs Outdated
Review: assert the headers-first submit is held (`inconclusive`), and say
on the submitblock row that a parent past the 10,000-header walk is
treated as before segwit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@niteshbalusu11

Copy link
Copy Markdown
Contributor Author

@rearden-grok please re-review the latest commit (f49cd66).

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

submitblock fills the BIP141 coinbase witness reserved value the way Core's UpdateUncommittedBlockStructures does: 32 zero bytes only when segwit is active for this block, the coinbase has a witness commitment, and the coinbase witness stack is empty. An existing reserved value is left alone, a pre-segwit commitment stays ordinary data, and P2P accept_received_block does not grow a second fill. The headers-first case asserts inconclusive. The 10,000-header parent walk stays fail-closed past the cap, and docs/rpc.md matches that.

Prior notes on the parent walk and the held-result assert are addressed on f49cd66d. No new comment restates the code.

Issue counts by severity

  • bugs: 0
  • suggestions: 0
  • nits: 0

Rebase is not required. This head is mergeable, and the four commits on master since the base are the BIP30 ancestor work. They do not touch these files.

@reardencode
reardencode merged commit ec4865b into reardencode:master Oct 18, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants