-
Notifications
You must be signed in to change notification settings - Fork 150
Update segwit v0 signature hashing to use the new caching implementation #480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
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
e7eefe8 to
d14b2bd
Compare
3e74155 to
f8a4625
Compare
Taproot signing is extremely inefficient without several levels of caching (e.g. pre-computing tagged hashes, and caching sub-hashes when signing). This is especially true for Elements which adds much more data to the signature hash. Wally's initial hash generation relied on creating the entire preimage and then hashing it in one call. Since segwit and particularly taproot, this approach has not aged well, and is not compatible with efficient tagged hashing. For bip341 Elements signing, this change implements a new approach which hashes the data piecemeal rather than concatenating it first. This uses much less cpu, memory and stack, and eliminates the need to pre-compute the final allocation length of the preimage before generating it, leading to less, more readable code. Additionally, we implement an on-demand cache for preimage data when signing. By passing in a wally_map to hold the cached data, callers can greatly decrease the time required to sign. We replace the existing BTC taproot implementation with this new one, and fix several bugs that were found as a result: - The sub-length calculation for tapscripts was incorrect - The sighash mask for taproot was incorrect - ANYONECANPAY required all input data when only the given inputs data was used.
Use the new signature hash function for both btc and elements. For signing via PSBT input instead of via the PSBT, this requires that EC_FLAG_ELEMENTS is passed in flags, in order to correctly use the Elements-specific tagged hashes. This is because the input alone does not know if it belongs to a PSET.
This test case was generated from Elements with extra processing to work around Elements bugs and lack of ELIP-0101 support there.
Having the cache as part of the PSBT is not ideal, but it allows us to support caching without having to modify a bunch of psbt calls in an incompatible fashion. Also includes a drive-by fix to add genesis_blockhash to the tests ctypes wrapper, as it was missed previously.
…TESTS=1 Note we leave these tests enabled for the CI, this is just for faster development when hacking on other parts of wally.
Don't bother checking for leaks, since we don't clean up in these tests.
For valgrind runs we run all tests including Elements ones, even if Elements support is disabled. Make the tests friendly to that use-case.
Segwit uses SHA-256D instead of taproot's SHA-256 for commitments. Support this by using a flag on cache keys indicating a double hash is required. Update the various functions that hash portions of the tx data to support the segwit v0 variant (in many cases the same data, just double hashed). Note that Elements taproot hashes some data in a different order, which makes this change more painful than it should be. It also makes it impossible to cleanly/automatically cache taproot data when generating segwit data and vice-versa, meaning mixed segwit v0 and taproot transactions are less efficient than they could potentially be otherwise.
f8a4625 to
4812310
Compare
Share parameter checking code between bip143/bip431, and reject all-zero genesis hashes in the signing code itself.
Note the indentation of the existing (no pre-segwit-only) impl is not changed, since this makes the diff smaller, and it too will be migrated in a future commit.
4812310 to
fd74e27
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Includes and requires #479