Segwit commitment structure is not midstate-compatible #8307

issue maaku opened this issue on July 6, 2016
  1. maaku commented at 5:19 PM on July 6, 2016: contributor

    The issue is on this line of code:

    https://github.com/bitcoin/bitcoin/blob/8b49040854be2e26b66366aeae1cba4716f93d93/src/main.cpp#L3477

    Instead of checking for the commitment at the start of an output, it should be placed at the end of the very last output. Then midstate compression can be used to reduce the size of the coinbase tx for proofs that require access to the witness commitment.

  2. luke-jr commented at 5:32 PM on July 6, 2016: member

    Miners can put it in the very last output as the only data. BIP 145 requires this for GBT. Surely that is good enough?

  3. gmaxwell commented at 5:37 PM on July 6, 2016: contributor

    It cannot be required to be in the last output without making some existing hardware unable to mine.

  4. maaku commented at 5:38 PM on July 6, 2016: contributor

    @luke-jr No, the consensus rule needs to be that it is the very last data of the last output, otherwise there's no way to know (without seeing the full transaction) that one is not inside of a pushdata that looks like an output. This means that a miner could make the last output [realCommitment ... padding ... fakeCommitment] and then after midstate compression all the validator of the proof sees is [padding ... fakeCommitment].

    To make it concrete and real, let's say that namecoin changes to using the segwit nonce to contain their aux header for merged mining. A single miner could commit to two blocks in this way.

    EDIT: The reason the end-of-last-output trick works is that the validator then checks that all that follows is the 4-byte locktime, which is information it does have.

  5. sipa commented at 6:10 PM on July 6, 2016: member

    Namecoin should not use the segwit nonce at all. It is reserved for future consensus-critical extensions.

    If they (or anyone else creating a non-consensus critical commitment) need a guarantee that the commitment is unique, they should pick a place that has that property. Including adding their own last-position coinbase output.

  6. MarcoFalke added the label Consensus on Jul 6, 2016
  7. MarcoFalke added the label Brainstorming on Jul 6, 2016
  8. maaku commented at 6:16 PM on July 6, 2016: contributor

    Pick any other example, focusing on namecoin is a red herring.

    TXO commitments. You could commit to two different TXO structures.

  9. sipa commented at 6:29 PM on July 6, 2016: member

    No, you cannot. Only the last commitment matters for consensus.

    Yes, that means validation needs to see the full coinbase transaction, which they already do anyway.

  10. petertodd commented at 7:23 PM on July 6, 2016: contributor

    In addition to @gmaxwell's point, p2pool also makes use of midstate-compressed witnesses commitments, so we'd have to change p2pool if we also made use of it.

    Given that SPV can't make use of witness data right now, I have no qualms about the current design; at worst proving a witness commitment requires 1MB worth of additional data, which isn't a disaster, and average case would be just a few KB; full nodes of course already have the relevant data as @sipa mentions. In the future we'll probably eventually have a hard-fork that moves the commitment right next to the block header anyway.

    edit: fixed typo

  11. maaku commented at 8:00 AM on July 7, 2016: contributor

    @sipa The last commitment which starts an output. In a midstate utilizing proof one cannot be certain if the proof given is at the start of an output or inside a push statement later in that output. Full validators need the whole coinbase, yes, but proof validators may not. They might only be interested in the the commitment itself, in which case it is a huge drag to include the whole coinbase and its kilobytes of coinbase payouts, the data for which might be larger than the rest of the proof. To illustrate, it would also be acceptable if it was made a consensus rule that after the actual commitment that is the last one to start an output, there is no instance in the serialization of the sequence of bytes which indicate a commitment. Then the proof checker just does a linear scan of the remaining bytes for the segwit commitment marker and fails if it sees two. But this is layer violating to the extreme and a pain to enforce. @gmaxwell It is important to consider which hardware is broken. The only hardware I can think of that is affected by this change is one that bakes in coinbase construction logic so as to force the owner of the hardware to pay a share to the manufacturer, while the manufacturer recouped costs on sale and isn't paying any portion of the electricity. This is firmware-enforced rent collection, and I'm very surprised if bricking hardware with that 'feature' is something we'd feel badly about. Are there other devices that would be broken by this change? @petertodd I have lost track of how many times (8? 9?) p2pool has hardforked its share chain. That's really not a roadblock.

  12. petertodd commented at 2:36 PM on July 7, 2016: contributor

    Actually, worth noting that we aren't breaking any hardware if we continue to allow non-segwit-containing blocks to leave off the commitment... however that leads to subtle issues around peers withholding commitments. Probably not an issue in the current way we use the commitment, but could be in the future.

    Anyway, what specific use cases do you think a 1MB worst case, and few KB average case, proof size hold back in the next year or two?

  13. sipa commented at 2:51 PM on July 7, 2016: member

    It is also possible to add an additional commitment location for the same data in a future softfork that is more proof-compatible, if wanted.

  14. petertodd commented at 3:25 PM on July 7, 2016: contributor

    @sipa Quite true, and we need to eventually fix the design of the commitments anyway to fix the upgrade data propagation problem properly.

  15. MarcoFalke commented at 6:14 PM on June 19, 2021: member

    Closing for now due to inactivity. The mailing list might be a better place to discuss network wide consensus changes.

  16. MarcoFalke closed this on Jun 19, 2021

  17. DrahtBot locked this on Aug 18, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me