Refactor Taproot execution to be more straightforward #116
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
763484a1e5 f fix undefined behavior when shifting an int 31 places 5af66e7f79 f expose nonce_function_bipschnorr 594e3abb69 f hash noncedata into nonce in nonce_function_bipschnorr 318d55155c f make helper functions static d65adc82f8 Add schnorrsig module which implements BIP-schnorr [0] compatible signing, verification and batch verification. f4153a29ab add chacha20 function REVERT: b19c000 Merge bitcoin#607: Use size_t shifts when computing a size_t REVERT: 4d01bc2 Merge bitcoin#606: travis: Remove unused sudo:false REVERT: e6d01e9 Use size_t shifts when computing a size_t REVERT: 7667532 travis: Remove unused sudo:false REVERT: ee99f12 Merge bitcoin#599: Switch x86_64 asm to use "i" instead of "n" for immediate values. REVERT: d58bc93 Switch x86_64 asm to use "i" instead of "n" for immediate values. REVERT: 05362ee Merge bitcoin#597: Add $(COMMON_LIB) to exhaustive tests to fix ARM asm build REVERT: 8348386 Add $(COMMON_LIB) to exhaustive tests to fix ARM asm build REVERT: aa15154 Merge bitcoin#568: Fix integer overflow in ecmult_multi_var when n is large REVERT: 2277af5 Fix integer overflow in ecmult_multi_var when n is large REVERT: 85d0e1b Merge bitcoin#591: Make bench_internal obey secp256k1_fe_sqrt's contract wrt aliasing. REVERT: 1419637 Merge bitcoin#580: Add trivial ecmult_multi algorithm which does not require a scratch space REVERT: a697d82 Add trivial ecmult_multi to the benchmark tool REVERT: bade617 Add trivial ecmult_multi algorithm. It is selected when no scratch space is given and just multiplies and adds the points. REVERT: 5545e13 Merge bitcoin#584: configure: Use CFLAGS_FOR_BUILD when checking native compiler REVERT: 20c5869 Merge bitcoin#516: improvements to random seed in src/tests.c REVERT: b76e45d Make bench_internal obey secp256k1_fe_sqrt's contract wrt aliasing. REVERT: 870a977 Merge bitcoin#562: Make use of TAG_PUBKEY constants in secp256k1_eckey_pubkey_parse REVERT: be40c4d Fixup for C90 mixed declarations. REVERT: c71dd2c Merge bitcoin#509: Fix algorithm selection in bench_ecmult REVERT: 6492bf8 Merge bitcoin#518: Summarize build options after running configure REVERT: 0e9ada1 Merge bitcoin#567: Correct order of libs returned on pkg-config --libs --static libsecp2… REVERT: e96901a Merge bitcoin#587: Make randomization of a non-signing context a noop REVERT: 58df8d0 Merge bitcoin#511: Portability fix for the configure scripts generated REVERT: 2ebdad7 Merge bitcoin#552: Make constants static: REVERT: 1c131af Merge bitcoin#551: secp256k1_fe_sqrt: Verify that the arguments don't alias. REVERT: ba698f8 Merge bitcoin#539: Assorted minor corrections REVERT: 949e85b Merge bitcoin#550: Optimize secp256k1_fe_normalize_weak calls. REVERT: a34bcaa Actually pass CFLAGS_FOR_BUILD and LDFLAGS_FOR_BUILD to linker REVERT: 2d5f4ce configure: Use CFLAGS_FOR_BUILD when checking native compiler REVERT: b408c6a Merge bitcoin#579: Use __GNUC_PREREQ for detecting __builtin_expect REVERT: 6198375 Make randomization of a non-signing context a noop REVERT: c663397 Use __GNUC_PREREQ for detecting __builtin_expect REVERT: e34ceb3 Merge bitcoin#557: Eliminate scratch memory used when generating contexts REVERT: b3bf5f9 ecmult_impl: expand comment to explain how effective affine interacts with everything REVERT: efa783f Store z-ratios in the 'x' coord they'll recover REVERT: ffd3b34 add `secp256k1_ge_set_all_gej_var` test which deals with many infinite points REVERT: 84740ac ecmult_impl: save one fe_inv_var REVERT: 4704527 ecmult_impl: eliminate scratch memory used when generating context REVERT: 7f7a2ed ecmult_gen_impl: eliminate scratch memory used when generating context REVERT: 314a61d Merge bitcoin#553: add static context object which has no capabilities REVERT: 89a20a8 Correct order of libs returned on pkg-config --libs --static libsecp256k1 call. REVERT: d3cb1f9 Make use of TAG_PUBKEY constants in secp256k1_eckey_pubkey_parse REVERT: 40fde61 prevent attempts to modify `secp256k1_context_no_precomp` REVERT: ed7c084 add static context object which has no capabilities REVERT: 496c5b4 Make constants static: static const secp256k1_ge secp256k1_ge_const_g; static const int CURVE_B; REVERT: bf8b86c secp256k1_fe_sqrt: Verify that the arguments don't alias. REVERT: 9bd89c8 Optimize secp256k1_fe_normalize_weak calls. Move secp256k1_fe_normalize_weak calls out of ECMULT_TABLE_GET_GE and ECMULT_TABLE_GET_GE_STORAGE and into secp256k1_ge_globalz_set_table_gej instead. REVERT: 52ab96f clean dependendies in field_*_impl.h REVERT: deff5ed Correct math typos in field_*.h REVERT: 4efb3f8 Add check that restrict pointers don't alias with all parameters. REVERT: 3965027 Summarize build options in configure script REVERT: 0f05173 Fix algorithm selection in bench_ecmult REVERT: 8b3841c fix bug in fread() failure check REVERT: cddef0c tests: add warning message when /dev/urandom fails REVERT: 270f6c8 Portability fix for the configure scripts generated git-subtree-dir: src/secp256k1 git-subtree-split: 763484a1e5bed2b8b990e71c2f66129ae1038d59
This removes the dependency on OpenSSL for the interaction tests, by providing a pure-Python toy implementation of secp256k1.
* Integration into CheckSig by Pieter Wuille.
This includes key path spending and script path spending, but not the Tapscript execution implementation.
Includes sighashing code and many tests by Johnson Lau.
675abd2 to
abf59fa
Compare
1 task
252e920 to
1cbc472
Compare
c1936f1 to
d7b5d63
Compare
7cd6e7d to
b8048b8
Compare
|
It seems like @sipa has addressed most, if not all, of the feedback here, so closing. |
sipa
pushed a commit
that referenced
this pull request
Jul 7, 2025
67dc752 cmake, test: Disable tests instead of ignoring them (Hennadii Stepanov) bb9157d cmake, refactor: Switch to `Python3::Interpreter` imported target (Hennadii Stepanov) Pull request description: This PR: 1. Switches to a modern CMake approach by using the `Python3::Interpreter` imported target, which is more robust than using variables. 2. Disables the `util_rpcauth_test` test explicitly instead of silently ignoring it. A build and test log for the case when Python is unavailable is provided below: ``` $ cmake -B build $ cmake --build build -j 16 $ ctest --test-dir build -j $(nproc) -R "^util" Internal ctest changing into directory: /bitcoin/build Test project /bitcoin/build Start 115: util_tests Start 117: util_trace_tests Start 114: util_string_tests Start 116: util_threadnames_tests Start 1: util_rpcauth_test 1/5 Test bitcoin#1: util_rpcauth_test ................***Not Run (Disabled) 0.00 sec 2/5 Test #114: util_string_tests ................ Passed 0.11 sec 3/5 Test #117: util_trace_tests ................. Passed 0.11 sec 4/5 Test #116: util_threadnames_tests ........... Passed 0.11 sec 5/5 Test #115: util_tests ....................... Passed 0.13 sec 100% tests passed, 0 tests failed out of 4 Total Test time (real) = 0.13 sec The following tests did not run: 1 - util_rpcauth_test (Disabled) ``` ACKs for top commit: purpleKarrot: ACK 67dc752 janb84: tACK 67dc752 Tree-SHA512: 5fc7ebe31ac03f4b8a53ecfcfc1cace0f647a1d2c989651988edae96bdfbbe2dee171714e57cb028e65ead1bb40806a82d9821746451dbf005538601fd33ea88
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.
Hi @sipa!
I've been reviewing the taproot code -- I believe this version to be equivalent to the one proposed.
However, I think that it is much easier to read and review, despite being slightly longer.
Non exhaustive, but here's a list of changes made:
It also seems to me to be easier to review to be consistent with the prior code, given that there are minimal changes to the existing segwit v0 flow.
I would also suggest interface-wise another EvalScript method with no ExecData argument and set it to an EvalScript local execdata given that SegWit v0 passes an empty argument, but that's larger churn that might not be worth it.