[segnet3 hard-fork] Allow additional data in coinbase witness #60
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rebased by Pieter Wuille. Cleanup by Matt Corallo.
Contains refactorings by Eric Lombrozo. Contains fixup by Nicolas Dorier.
Includes simplifications by Eric Lombrozo.
Includes support for pushkeyhash wit v0 by Alex Morcos.
Includes fixup by Suhas Daftuar.
Includes fixup by Thomas Kerin.
…en verify_script receives VERIFY_WITNESS flag script_tests: always test bitcoinconsensus_verify_script_with_amount if VERIFY_WITNESS isn't set Rename internal method + make it static trim bitcoinconsensus_ prefix Add SERIALIZE_TRANSACTION_WITNESS flag
Witness blocks can be greater than 2MiB, but cannot be validly greater than 4MB.
Break out witness commit position search and witness commitment and nonce check into separate functions.
Rather than having the coinbase witness commitment be the hash of the concatenation of the transaction witness merkle root and a single 32-byte nonce from the coinbase witness, treat it instead as the merkle root of a tree with the leaves being the transaction witness merkle root, and each of the items from the coinbase stack. For the case where the coinbase witness stack contains a single, 32-byte item this change has no effect, however it should generalise well to allow the coinbase witness stack to contain an arbitrary number of elements. (If an element on the coinbase stack is not already exactly 32 bytes, it is first hashed before being added as a leaf to the merkle tree)
This relaxes the restriction on the coinbase witness that limited it to only having one entry that must be 32 bytes; instead there may be up to 255 items on the coinbase witness stack, and they may be any length. In order to have multiple entries on the coinbase witness stack, the number of entries must be specified in the coinbase commitment, by changing the witness commitment from a 36 byte push: OP_RETURN [ 0xaa 0x21 0xa9 0xed HASH ] to a 37 byte push: OP_RETURN [ 0xaa 0x21 0xa9 0xed HASH N ] where N is the number of entries on the coinbase witness stack (from 0 to 255).
ajtowns
force-pushed
the
segwit-coinbase-witness
branch
from
a5462fd to
86c55b0
Compare
sipa
force-pushed
the
segwit
branch
4 times, most recently
from
a88b686 to
38a8b36
Compare
|
I have not included this in segwit4, but we should keep thinking about this. |
sipa
force-pushed
the
segwit
branch
5 times, most recently
from
c700b59 to
85b6ae6
Compare
sipa
force-pushed
the
segwit
branch
2 times, most recently
from
1b904a5 to
fb4873e
Compare
sipa
pushed a commit
that referenced
this pull request
Apr 22, 2022
…d enum 0e5dedb qt/wallettests: sort includes (William Casarin) 0554251 qt: Skip displayUnitChanged signal if unit is not actually changed (Hennadii Stepanov) ffbc2fe qt, refactor: Remove default cases for scoped enum (Hennadii Stepanov) 152d5ba qt, refactor: Remove BitcoinUnits::valid function (Hennadii Stepanov) aa23960 qt, refactor: Make BitcoinUnits::Unit a scoped enum (Hennadii Stepanov) 75832fd qt: Use QVariant instead of int for BitcoinUnit in QSettings (Hennadii Stepanov) Pull request description: This is a rebased version of #60 Since Qt 5.5 there are [means](https://doc.qt.io/qt-5/qobject.html#Q_ENUM) to register an enum type with the meta-object system (such enum still lacks an ability to interact with [QSettings::setValue()](https://doc.qt.io/qt-5/qsettings.html#setValue) and [QSettings::value()](https://doc.qt.io/qt-5/qsettings.html#value) without defined stream operators). In order to reduce global namespace polluting and to force strong type checking, this PR makes BitcoinUnits::Unit a scoped enum (typedef BitcoinUnits::Unit BitcoinUnit;). No behavior change. ACKs for top commit: jonatack: ACK 0e5dedb, review and debug build of each commit after rebase on current master, lightly tested running the GUI, changing units a few times, and verifying persistence after restarting promag: Code review ACK 0e5dedb Tree-SHA512: 39ec0d7e4f0b9b25be287888121a8db6b282339674e37ec3a3554da63a9e22d6fe079e8310ca289b2a0356a19b3c7e55afa17d09dd34e0f222177f603bb053a3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on:
https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2016-January/012301.html
https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2016-February/012313.html
An alternative to #48 . It's probably not acceptable as is, but hopefully useful for discussion?
This aims to be a reasonably simple patch set that allows future soft-forks to have additional commitments made by including them in the coinbase witness.
With this patchset, the coinbase witness stack can contain up to 255 elements, of any size, however the coinbase must commit to the number of elements. The coinbase commits to the merkle root of a tree whose first leaf is the merkle root of the tree of all the witnesses for the other transactions, and whose remaining leaves are the coinbase witness stack elements.