Merge bitcoin/bitcoin#33961: script: Add a separate ScriptError for empty pubkeys encountered in Tapscript

9d5021a05b script: add SCRIPT_ERR_TAPSCRIPT_EMPTY_PUBKEY (billymcbip)

Pull request description:

  We currently have two callsites for `SCRIPT_ERR_PUBKEYTYPE`:
  - A pre-tapscript policy error behind the `SCRIPT_VERIFY_STRICTENC` flag: 4de26b111f/src/script/interpreter.cpp (L220)
  - A [consensus error](https://github.com/bitcoin/bips/blob/master/bip-0342.mediawiki?plain=1#L93) in Tapscript: 4de26b111f/src/script/interpreter.cpp (L368)

  It would be good for readability and testability to have separate errors for both cases, as they are quite distinct (policy vs. consensus, format vs. emptiness).

  **This PR adds `SCRIPT_ERR_TAPSCRIPT_EMPTY_PUBKEY` for the consensus error path.**

  This change would make our error handling more consistent. We have more granular errors for other pubkey error paths already: `SCRIPT_ERR_WITNESS_PUBKEYTYPE`,  `SCRIPT_ERR_DISCOURAGE_UPGRADABLE_PUBKEYTYPE`. We also have separate errors for MINIMAL_IF: `SCRIPT_ERR_MINIMALIF` for the policy error pre-tapscript, and `SCRIPT_ERR_TAPSCRIPT_MINIMALIF` for the consensus error post-tapscript.

  Tests:

  Added a test case to `script_tests` and ran `build/bin/test_bitcoin --run_test=script_tests --log_level=success`.
  ```
  test/script_tests.cpp:144: info: check '[["aa","#SCRIPT# 0 CHECKSIG","#CONTROLBLOCK#",0.00000001],"","0x51 0x20 #TAPROOTOUTPUT#","P2SH,WITNESS,TAPROOT","TAPSCRIPT_EMPTY_PUBKEY","TAPSCRIPT: OP_CHECKSIG with empty pubkey must fail"] (with flags 165d5d)' has passed
  ...
  ```

  Ran `DIR_UNIT_TEST_DATA="$(pwd)/../qa-assets/unit_test_data" build/bin/test_bitcoin --run_test=script_assets_tests --log_level=success`.

  Updated `feature_taproot.py` and ran `build/test/functional/feature_taproot.py`.

  Looking forward to your feedback.

ACKs for top commit:
  sedited:
    ACK 9d5021a05b
  darosior:
    utACK 9d5021a05b
  sipa:
    ACK 9d5021a05b

Tree-SHA512: bc0b7f64454313fe392ffb2d23aa4eca3deadc5ea1d10b3fba0b3ab4cb0575a5ddcb002dc27b4fa7aa3c180840a83d1b3e5c89351009ce7ffe684d58e1980ace
This commit is contained in:
merge-script
2025-12-09 08:01:49 -08:00
6 changed files with 29 additions and 12 deletions

View File

@@ -631,7 +631,7 @@ ERR_PUSH_SIZE = {"err_msg": "Push value size limit exceeded"}
ERR_DISABLED_OPCODE = {"err_msg": "Attempted to use a disabled opcode"}
ERR_TAPSCRIPT_CHECKMULTISIG = {"err_msg": "OP_CHECKMULTISIG(VERIFY) is not available in tapscript"}
ERR_TAPSCRIPT_MINIMALIF = {"err_msg": "OP_IF/NOTIF argument must be minimal in tapscript"}
ERR_PUBKEYTYPE = {"err_msg": "Public key is neither compressed or uncompressed"}
ERR_TAPSCRIPT_EMPTY_PUBKEY = {"err_msg": "Empty public key in tapscript"}
ERR_STACK_SIZE = {"err_msg": "Stack size limit exceeded"}
ERR_CLEANSTACK = {"err_msg": "Stack size must be exactly one after execution"}
ERR_INVALID_STACK_OPERATION = {"err_msg": "Operation not valid with the current stack size"}
@@ -1047,27 +1047,27 @@ def spenders_taproot_active():
add_spender(spenders, "tapscript/minimalif", leaf="t5", **common, inputs=[getter("sign"), b'\x01'], failure={"inputs": [getter("sign"), b'\x0001']}, **ERR_TAPSCRIPT_MINIMALIF)
add_spender(spenders, "tapscript/minimalnotif", leaf="t6", **common, inputs=[getter("sign"), b'\x01'], failure={"inputs": [getter("sign"), b'\x0100']}, **ERR_TAPSCRIPT_MINIMALIF)
# Test that 1-byte public keys (which are unknown) are acceptable but nonstandard with unrelated signatures, but 0-byte public keys are not valid.
add_spender(spenders, "tapscript/unkpk/checksig", leaf="t16", standard=False, **common, **SINGLE_SIG, failure={"leaf": "t7"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/unkpk/checksigadd", leaf="t17", standard=False, **common, **SINGLE_SIG, failure={"leaf": "t10"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/unkpk/checksigverify", leaf="t18", standard=False, **common, **SINGLE_SIG, failure={"leaf": "t8"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/unkpk/checksig", leaf="t16", standard=False, **common, **SINGLE_SIG, failure={"leaf": "t7"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
add_spender(spenders, "tapscript/unkpk/checksigadd", leaf="t17", standard=False, **common, **SINGLE_SIG, failure={"leaf": "t10"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
add_spender(spenders, "tapscript/unkpk/checksigverify", leaf="t18", standard=False, **common, **SINGLE_SIG, failure={"leaf": "t8"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
# Test that 33-byte public keys (which are unknown) are acceptable but nonstandard with valid signatures, but normal pubkeys are not valid in that case.
add_spender(spenders, "tapscript/oldpk/checksig", leaf="t30", standard=False, **common, **SINGLE_SIG, sighash=bitflipper(default_sighash), failure={"leaf": "t1"}, **ERR_SCHNORR_SIG)
add_spender(spenders, "tapscript/oldpk/checksigadd", leaf="t31", standard=False, **common, **SINGLE_SIG, sighash=bitflipper(default_sighash), failure={"leaf": "t2"}, **ERR_SCHNORR_SIG)
add_spender(spenders, "tapscript/oldpk/checksigverify", leaf="t32", standard=False, **common, **SINGLE_SIG, sighash=bitflipper(default_sighash), failure={"leaf": "t28"}, **ERR_SCHNORR_SIG)
# Test that 0-byte public keys are not acceptable.
add_spender(spenders, "tapscript/emptypk/checksig", leaf="t1", **SINGLE_SIG, **common, failure={"leaf": "t7"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/emptypk/checksigverify", leaf="t2", **SINGLE_SIG, **common, failure={"leaf": "t8"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/emptypk/checksigadd", leaf="t9", **SINGLE_SIG, **common, failure={"leaf": "t10"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/emptypk/checksigadd", leaf="t35", standard=False, **SINGLE_SIG, **common, failure={"leaf": "t10"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/emptypk/checksig", leaf="t1", **SINGLE_SIG, **common, failure={"leaf": "t7"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
add_spender(spenders, "tapscript/emptypk/checksigverify", leaf="t2", **SINGLE_SIG, **common, failure={"leaf": "t8"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
add_spender(spenders, "tapscript/emptypk/checksigadd", leaf="t9", **SINGLE_SIG, **common, failure={"leaf": "t10"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
add_spender(spenders, "tapscript/emptypk/checksigadd", leaf="t35", standard=False, **SINGLE_SIG, **common, failure={"leaf": "t10"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
# Test that OP_CHECKSIGADD results are as expected
add_spender(spenders, "tapscript/checksigaddresults", leaf="t28", **SINGLE_SIG, **common, failure={"leaf": "t27"}, err_msg="unknown error")
add_spender(spenders, "tapscript/checksigaddoversize", leaf="t29", **SINGLE_SIG, **common, failure={"leaf": "t27"}, err_msg="unknown error")
# Test that OP_CHECKSIGADD requires 3 stack elements.
add_spender(spenders, "tapscript/checksigadd3args", leaf="t9", **SINGLE_SIG, **common, failure={"leaf": "t11"}, **ERR_INVALID_STACK_OPERATION)
# Test that empty signatures do not cause script failure in OP_CHECKSIG and OP_CHECKSIGADD (but do fail with empty pubkey, and do fail OP_CHECKSIGVERIFY)
add_spender(spenders, "tapscript/emptysigs/checksig", leaf="t12", **common, inputs=[b'', getter("sign")], failure={"leaf": "t13"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/emptysigs/nochecksigverify", leaf="t12", **common, inputs=[b'', getter("sign")], failure={"leaf": "t20"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/emptysigs/checksigadd", leaf="t14", **common, inputs=[b'', getter("sign")], failure={"leaf": "t15"}, **ERR_PUBKEYTYPE)
add_spender(spenders, "tapscript/emptysigs/checksig", leaf="t12", **common, inputs=[b'', getter("sign")], failure={"leaf": "t13"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
add_spender(spenders, "tapscript/emptysigs/nochecksigverify", leaf="t12", **common, inputs=[b'', getter("sign")], failure={"leaf": "t20"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
add_spender(spenders, "tapscript/emptysigs/checksigadd", leaf="t14", **common, inputs=[b'', getter("sign")], failure={"leaf": "t15"}, **ERR_TAPSCRIPT_EMPTY_PUBKEY)
# Test that scripts over 10000 bytes (and over 201 non-push ops) are acceptable.
add_spender(spenders, "tapscript/no10000limit", leaf="t19", **SINGLE_SIG, **common)
# Test that a stack size of 1000 elements is permitted, but 1001 isn't.