Signet commitment rewrite matches Core byte for byte; MTP needs a full window

[bkeroack]

Stacked on #945 → #941 → #936 (merge those first).

Problem

Signet commitment rewrite. A signet solution signs the block with the solution cut out of the coinbase witness commitment, so the bytes left behind are part of the signed message. satd rebuilt that script from rust-bitcoin's instructions(), which differs from Core's FetchAndClearCommitmentSection (src/signet.cpp:32-57):

• A zero-length OP_PUSHDATA1/2/4 (4c00, 4d0000, 4e00000000) came back as an empty push, written as OP_0 (0x00). Core writes an op with no data back as its own opcode byte (replacement << opcode): 0x4c, 0x4d or 0x4e.
• An op that does not parse made satd drop the whole scan and check the block as if it had no solution. Core's GetOp loop stops at that op, keeps the section it already found, and drops the rest of the script.

In both cases satd's signed merkle root differed from Core's.

Median time past. connect::median_time_past_with_plan (chain/connect.rs) and ChainState::get_median_time_past (chain/state.rs) skipped any height in the 11-block window they could not resolve and took the median of the rest, or returned 0 for an empty window. Core's GetMedianTimePast (src/chain.h:233-245) walks pprev and has fewer than 11 entries only near genesis. A short window moved BIP 113 / BIP 68 decisions; an empty coin window met every BIP 68 time lock; a refusal it caused was recorded as Invalid.

Block work. work_for_bits (storage/blockindex.rs) gave a zero target 2^256-1 work and ignored the sign and overflow of nBits. Core's GetBitsProof (src/chain.cpp:121-134) returns 0 when SetCompact (src/arith_uint256.cpp:176-194) flags negative or overflow, or the target is zero. add_u256 wrapped.

Fix

• fetch_and_clear_signet_section is now a port of Core's function: GetScriptOp (script.cpp:312-362) to read each op and stop at the first that does not parse, the opcode byte for an op with no data, and AppendDataSize (script.h:407-425) for a push with data. Note: copying the original bytes would not match Core, which also rewrites pushes that used a longer length prefix (4c0101 → 0101); the port does what Core does.
• A median-time-past window with a missing block is ConnectError::MedianTimeWindowGap, which is not a verdict on the block (is_verdict_on_block() == false): the connect fails, nothing is marked, the block connects once the index is whole. Every connect path uses the strict form (ChainState::median_time_past_for_connect, MTP cache first): accept_block and both reorg loops, the IBD connector, reindex, the background chainstate, TestBlockValidity. The prefetcher hands such a block to the direct read. BIP 68 takes the coin's median from GetAncestor(max(nCoinHeight - 1, 0)) as Core does (consensus/tx_verify.cpp:74); this differs only for a coin at height 0, which cannot exist. The mempool, templates and RPCs keep the lenient get_median_time_past (no behaviour change there).
• work_for_bits decodes nBits exactly as SetCompact and returns zero work where GetBitsProof does. For every exponent 1..32 with a positive mantissa (every target a header can meet) the target is unchanged, so stored chainwork stays valid. add_u256 saturates instead of wrapping.

Tests

Expected values come from Core's own code: FetchAndClearCommitmentSection and GetBitsProof copied verbatim and compiled with Core's script.cpp / arith_uint256.cpp (Core 16613c9de9), run over each input.

• validation::signet::consharden_tests
  • the_commitment_rewrite_is_cores_byte_for_byte (18 vectors: zero-length PUSHDATA1/2/4 before/after, five truncated tails, long prefixes, OP_0, two sections, an 80-byte section) — fails without the fix: yes (zero-length PUSHDATA1 before: rewritten commitment).
  • a_zero_length_pushdata_is_signed_as_its_own_opcode (P2WPKH challenge; signed over Core's rewrite → accepted; signed over the OP_0 form → refused) — fails without the fix: yes (a block signed over Core's rewrite 4c04ecc7daa2 must verify).
  • a_section_before_an_unparseable_push_is_checked — fails without: yes (section then 4c: Core checks the section and accepts the block).
  • a_bad_section_before_an_unparseable_push_is_refused (OP_TRUE challenge, paired with an accepted case) — fails without: yes.
  • cores_signet_parse_tests (port of validation_tests.cpp:69-128), a_commitment_without_a_section_is_left_alone — parity, pass before and after.
• chain::connect::consharden_tests: a_missing_height_row_is_an_error_not_a_shorter_median, a_height_row_without_its_index_entry_is_an_error, a_replay_plan_that_does_not_reach_the_window_is_an_error, bip68_with_a_hole_in_the_coins_window_is_not_a_verdict (old code: SequenceLockNotMet), an_empty_coin_window_no_longer_meets_every_time_lock (old code: Ok) — all fail without the fix: yes. the_median_is_cores_over_a_full_window_and_near_genesis, bip68_takes_the_coins_median_over_its_whole_window (lock met at +512 s, not at +511 s) — must-succeed pairs.
• chain::state::consharden_tests::a_hole_in_the_mtp_window_stops_the_connect_without_marking_the_block — fails without: yes (old accept_block returned Connected); also fails when only accept_block is switched back to the lenient helper, and when the gap is classed as a verdict (a window this node cannot read is not a verdict on the block). After the row is restored the same block connects. the_connect_median_is_cores_from_the_cache_and_from_the_index checks heights 1..16 against a pprev walk.
• storage::blockindex::consharden_tests::block_work_is_cores_get_bits_proof (27 nBits values) and bits_without_work_add_nothing_to_chainwork — fail without: yes (bits 00000000). work_is_unchanged_for_every_target_a_header_can_meet. tests::test_add_u256_max_overflow edited in place to assert saturation (fails on the old wrapping add).

Notes

• validation/pow.rs::check_timestamp (time-too-old) also takes a median over a window cut short by a missing parent. Not touched here: pow.rs is changed by a separate PR in flight.
• validation/versionbits.rs median_time_past likewise stops early on a missing header; it is not on a connect path and is left as is.
• TestBlockValidity still reports any error from inside connect_block (including this one, for a BIP 68 coin window) as a BIP 22 reason string; that mapping is out of scope here.

Release notes

On signet, satd now rebuilds the signed coinbase commitment exactly as Bitcoin Core does, so blocks whose witness commitment contains an empty OP_PUSHDATA1/2/4 or ends in an unparseable push are judged the same way Core judges them. If the block index is missing a block inside a median-time-past window, satd now stops connecting with a storage error instead of computing the median from fewer blocks, and does not mark the block invalid. Block work for malformed nBits is now zero, as in Core.

🤖 Generated with Claude Code