Skip to content

Conversation

sipa
Copy link
Member

@sipa sipa commented Oct 30, 2021

This PR adds code to test/functional/feature_taproot.py which runs through a (deterministic) scenario covering several aspects of the wallet side of BIP341 (scriptPubKey computation from keys/scripts, control block computation, key path spending), with the ability to output test vectors in mediawiki format based on this scenario. The generated tests are then also included directly in src/test/script_tests.cpp and src/test/script_standard_tests.cpp.

The test vectors generated here were added to BIP341 in bitcoin/bips#1225

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 30, 2021

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

@sipa sipa marked this pull request as draft November 2, 2021 22:33
@sipa
Copy link
Member Author

sipa commented Nov 2, 2021

I'm going to convert this to construct tests in JSON format rather than Mediawiki, and also actually run it in that form directly.

@sipa sipa force-pushed the 202110_taprootunit branch from c7465b9 to 41f002d Compare November 5, 2021 20:20
@sipa sipa marked this pull request as ready for review November 5, 2021 20:21
@sipa sipa force-pushed the 202110_taprootunit branch from 41f002d to b9770ac Compare November 6, 2021 15:24
@sipa sipa force-pushed the 202110_taprootunit branch 2 times, most recently from 22d1f3f to 5c4550c Compare November 7, 2021 17:31
@sipa
Copy link
Member Author

sipa commented Nov 7, 2021

Updated to address bitcoin/bips#1225 (review)

@sipa sipa force-pushed the 202110_taprootunit branch 2 times, most recently from b9de4f1 to c35c276 Compare November 8, 2021 16:56
@sipa sipa force-pushed the 202110_taprootunit branch from c35c276 to 19dd87c Compare November 10, 2021 22:38
@sipa
Copy link
Member Author

sipa commented Nov 10, 2021

Rebased.

sipa added 5 commits November 12, 2021 12:04
libsecp256k1's secp256k1_schnorrsig_sign only follows BIP340 exactly
if an aux_rand32 argument is passed. When no randomness is used
(as is the case in the current codebase here), there is no impact
on security between not providing aux_rand32 at all, or providing
an empty one. Yet, for repeatability/testability it is simpler
to always use an all-zero one.
This does the following:
* Adds a rfc6979 argument to test_framework/key.py's sign_ecdsa to
  select (deterministic) RFC6979-based nonce generation.
* Add a flag in feature_taproot.py's framework called "deterministic".
* Make the Schnorr signing in feature_taproot.py randomized by default,
  reverting to the old deterministic (aux_rnd=0x0000...00) behavior
  if the deterministic context flag is set.
* Make the ECDSA signing in feature_taproot.py use RFC6979-based nonces
  when the deterministic context flag is set (keeping the old randomized
  behavior otherwise).
@sipa sipa force-pushed the 202110_taprootunit branch from 19dd87c to f1c33ee Compare November 12, 2021 17:05
@sipa
Copy link
Member Author

sipa commented Nov 12, 2021

Updated to address bitcoin/bips#1225 (comment).

@sipa
Copy link
Member Author

sipa commented Nov 13, 2021

Corresponding BIP change was merged: bitcoin/bips#1225

@laanwj
Copy link
Member

laanwj commented Nov 15, 2021

Code review ACK f1c33ee
Also checked that src/test/data/bip341_wallet_vectors.json matches the file in bip341.

laanwj added a commit to bitcoin-core/gui that referenced this pull request Nov 15, 2021
…tests)

f1c33ee tests: implement BIP341 test vectors (Pieter Wuille)
ac3037d tests: BIP341 test vector generation (Pieter Wuille)
ca83ffc tests: add deterministic signing mode to ECDSA (Pieter Wuille)
c98c53f tests: abstract out precomputed BIP341 signature hash elements (Pieter Wuille)
a5bde01 tests: give feature_taproot access to sighash preimages (Pieter Wuille)
5140825 tests: add more fields to TaprootInfo (Pieter Wuille)
2478c67 Make signing follow BIP340 exactly w.r.t. aux randomness (Pieter Wuille)

Pull request description:

  This PR adds code to `test/functional/feature_taproot.py` which runs through a (deterministic) scenario covering several aspects of the wallet side of BIP341 (scriptPubKey computation from keys/scripts, control block computation, key path spending), with the ability to output test vectors in mediawiki format based on this scenario. The generated tests are then also included directly in `src/test/script_tests.cpp` and `src/test/script_standard_tests.cpp`.

  I intend to add these test vectors to BIP341 itself: bitcoin/bips#1225

ACKs for top commit:
  laanwj:
    Code review ACK f1c33ee

Tree-SHA512: fcf7109539cb214d3190516b205cd32d2b1b452f14aa66f4107acfaa8bfc7d368f626857f1935665a4342eabc0b9ee8aba608a7c0a2494bec0b498e723439c9d
@laanwj
Copy link
Member

laanwj commented Nov 15, 2021

Closing: PR was merged, github didn't detect it

@laanwj laanwj closed this Nov 15, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 16, 2021
@bitcoin bitcoin locked and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants