bitcoin
bitcoin copied to clipboard
bitcoin
BIP-348 (OP_CHECKSIGFROMSTACK) (regtest only)
This implements BIP-348 (OP_CHECKSIGFROMSTACK), but only specifies a regtest deployment. There is no effective policy change, since the SCRIPT_VERIFY_* flags (as used) result in the same OP_SUCCESS-like behavior.
This change can be composed with other opcode specifications (e.g. CTV, see #31989) and bundled into the same deployment (yet to be specified).
I encourage more general, conceptual discussion to happen on Delving Bitcoin and not on this pull request.
Some related discussion on Delving Bitcoin here:
- https://delvingbitcoin.org/t/ctv-csfs-can-we-reach-consensus-on-a-first-step-towards-covenants/
See also:
- https://bitcoinops.org/en/topics/op_checksigfromstack/
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage & Benchmarks
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32247.
Reviews
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | jonatack, shahsb, yuvicc, delta1, pinheadmz |
| Stale ACK | JeremyRubin |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #32998 (Bump SCRIPT_VERIFY flags to 64 bit by ajtowns)
- #32473 (Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts by sipa)
- #32453 ([Policy] Discourage Unsigned Annexes by JeremyRubin)
- #31989 (BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only) by jamesob)
- #29843 (policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only) by ajtowns)
- #29247 (CAT in Tapscript (BIP-347) by arminsabouri)
- #26201 (Remove Taproot activation height by Sjors)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
cr ACK cb0c9f6
This matches the BIP's semantics, and the implementation is reasonable. Minor nits above to reduce possibility of behavior changes for other users of Schnorr APIs.
I spent a bit of time writing tests (reading directly from the BIP) for feature_taproot.py: https://github.com/instagibbs/bitcoin/tree/2025-04-bip348_tests
This should hopefully give some nice coverage of border conditions, including sigops budget and maybe inspired people to add more if they want to contribute in a positive way.
https://github.com/instagibbs/bitcoin/tree/2025-03-bip348-inq-28 this was an alternative implementation of the core logic, but I think it matches here
Thanks @instagibbs; I've compressed your test changes into a single commit and added that here.
In an effort to keep pull requests focused on technical discussion, I invite all contributors of conceptual review to post here:
https://github.com/bitcoin-core/meta/discussions/28
Please try to keep pull request comments focused on the code changes, and move all other comments including "concept N/ACK" to the discussion page.
