add coinbase_witness field to NewTemplate#198
Conversation
- add `NewTemplate.coinbase_witness` to TDP as `B0_32` - define conditional encoding: - empty when witness commitment output is omitted - exactly 32 bytes when witness commitment output is present - codify BIP141 empty-block behavior (witness commitment optional) - require clients to preserve TP-provided output ordering as `<client-added-outputs> || <NewTemplate.coinbase_tx_outputs>` - clarify field purpose: serialize coinbase witness coherently with TP commitment output; no witness-commitment recomputation required - update Protocol Overview BIP141 text to match the new rules
5faa219 to
1bd0189
Compare
|
ack |
|
ACK 1bd0189 I updated stratum-mining/sv2-tp#108 to match. How do you want to go about deploying this? It's probably safest if you first update SRI to optionally handle this extra field. I could then release a new sv2-tp version that sets the field, and note in its release notes the minimum expected SRI release. Or we could do something more graceful like bumping a protocol number? |
| | coinbase_tx_value_remaining | U64 | The value, in satoshis, available for spending in coinbase outputs added by the client. Includes both transaction fees and block subsidy. | | ||
| | coinbase_tx_outputs_count | U32 | The number of transaction outputs included in coinbase_tx_outputs | | ||
| | coinbase_tx_outputs | B0_64K | Bitcoin transaction outputs to be included as the last outputs in the coinbase transaction | | ||
| | coinbase_witness | B0_32 | Coinbase witness bytes. MUST be empty if no witness commitment output is present in `coinbase_tx_outputs`; otherwise MUST contain exactly 32 bytes | |
There was a problem hiding this comment.
during WG call @Fi3 is suggesting we look into potentially usingOPTION[B0_32] instead of B0_32
There was a problem hiding this comment.
no hard opinions from me nor @Fi3
just observation
There was a problem hiding this comment.
That seems more clear to me.
There was a problem hiding this comment.
here's a summary of how coinbase_witness description would be worded, according to the two potential types and the two different scenarios (with vs without witness commitment):
OPTION[B0_32] |
B0_32 |
|
|---|---|---|
| ✅ WITH witness commitment | "MUST NOT be empty, and MUST contain exactly 32 bytes" | "MUST contain exactly 32 bytes" |
| ❌ WITHOUT witness commitment | "MUST be empty" | "MUST be empty" |
or as whole description sentences:
B0_32 |
Coinbase witness bytes. MUST be empty if no witness commitment output is present in coinbase_tx_outputs; otherwise MUST contain exactly 32 bytes |
OPTION[B0_32] |
Coinbase witness bytes. MUST be empty if no witness commitment output is present in coinbase_tx_outputs; otherwise MUST NOT be empty, and MUST contain exactly 32 bytes |
There was a problem hiding this comment.
Saying "MUST not be empty" makes the text more confusing :-)
With OPTION[B0_32] this should be enough:
Coinbase witness bytes. MUST be empty if no witness commitment output is present in coinbase_tx_outputs; otherwise MUST contain exactly 32 bytes.
Also why not OPTION[U256] so the length is non-ambiguous?
Coinbase witness (32 bytes by consensus rules). MUST be empty if no witness commitment output is present in coinbase_tx_outputs.
There was a problem hiding this comment.
eureka I think OPTION[U256] hits the sweet spot!
it optimally forbids both edge cases without room for interpretation footguns
cc @Fi3
|
Ack |
4 participants
close #166
as discussed in #156 (TLDR: future-proofing Sv2 TDP wrt future/potential BIP141-related consensus changes)
note: this PR is similar to #15, which was opened in 2022 and closed in 2023 with pending review comments and no merging
so I'm trying to preemptively address some confusion points that were left as pending review comments on #15