Slight cleanup of signet commitment description#976
Conversation
maflcko
left a comment
maflcko
left a comment
There was a problem hiding this comment.
I think the either refers to the fact that one of (scriptSig, scriptWitness) can be empty, but not both.
|
Could mention the BIP number in the pull subject line? Also, cc @kallewoof |
|
ACK. Thanks for making that more understandable! Ping @luke-jr. |
| A new type of network ("signet"), which takes an additional consensus parameter called the challenge (scriptPubKey). The challenge can be a simple pubkey (P2PKH style), or a k-of-n multisig, or any other script you would want. | ||
|
|
||
| The witness commitment of the coinbase transaction is extended to include a secondary commitment (the signature/solution) of either: | ||
| Signet requires all blocks to have a BIP 141 commitment in the coinbase transaction. The BIP 141 commitment's optional data must include an additional commitment of the signature/solution for the block: |
There was a problem hiding this comment.
This seems like it contradicts the text from #983: "There is one other acceptable special case: if a block's challenge is e.g. OP_TRUE (0x51), where an empty solution would result in success, the block is also considered valid if the signet commitment is absent"
There was a problem hiding this comment.
Yeah, good point. @instagibbs there is an exception now for the case where if the challenge is enough to satisfy (e.g. OP_TRUE); in this case, the signet commitment may be omitted entirely.
There was a problem hiding this comment.
In which case the BIP 141 commitment isn't required either
There was a problem hiding this comment.
How about:
Signet requires all blocks to have a BIP 141 commitment in the coinbase transaction. In order to provide a solution to the block challenge, the BIP 141 commitment's optional data must include an additional commitment of the solution for the block: [...]. In the special case where an empty solution is valid (ie scriptSig and scriptWitness are both empty) this additional commitment can optionally be left out. This special case is to allow non-signet-aware block generation code to be used to test a custom signet chain where the challenge is
OP_TRUE.
There was a problem hiding this comment.
Took a modified version of what you posted, noting that bip141 commitment is only required with non-empty solutions, and that any "trivial" challenge can be used to avoid requiring fancy mining software for non-signature tests.
There was a problem hiding this comment.
I believe the current code requires a bip141 commitment even with empty solutions. Should this be changed?
There was a problem hiding this comment.
you are correct: bitcoin/bitcoin@d0f364c#diff-4bdd7c44636dae0429a4d61909eeed17R90
I think for simplicity it's fine to require it. Any objections?
There was a problem hiding this comment.
Simplicity is good. Let's require
| The witness commitment of the coinbase transaction is extended to include a secondary commitment (the signature/solution) of either: | ||
| Signet requires all blocks to have a BIP 141 commitment in the coinbase transaction. The BIP 141 commitment's optional data must include an additional commitment of the signature/solution for the block: | ||
|
|
||
| 1-4 bytes - Push the following (4 + x + y) bytes |
There was a problem hiding this comment.
I believe this should be 1-5 bytes (1 byte if 4+x+y <= 75, 2 bytes for PUSHDATA1 if 4+x+y < 256, 3 bytes for PUSHDATA2 if 4+x+y < 65536, and 5 bytes for PUSHDATA4).
|
@instagibbs needs rebase |
|
rebased/squashed |
|
ACK, please merge at your convenience, @luke-jr. |
maflcko
left a comment
maflcko
left a comment
There was a problem hiding this comment.
Better with or without my suggested change
Co-authored-by: MarcoFalke <falkemarco@gmail.com>
|
ACK |
|
@luke-jr ;) |
5 participants
"either" and "Signet scriptSig header" seem to make no sense here, if other touchups are disagreeable I can drop them.