script: return proper error for CScriptNum errors #34381

1. 
   brunoerg commented at 1:16 pm on January 22, 2026: contributor

   When evaluating a script, the current code is bad for analyzing some errors because it returns SCRIPT_ERR_UNKNOWN_ERROR for errors that are clearly known.

   CScriptNum has two well defined errors: number overflow and non-minimally encoded number. However, for both errors we return as unknown. This PR changes it by adding a new ScriptError that is used for any CScriptNum error.

2. DrahtBot added the label Consensus on Jan 22, 2026
3. 
   DrahtBot commented at 1:16 pm on January 22, 2026: contributor

   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/34381.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	w0xlt, darosior, achow101
   Stale ACK	billymcbip

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #29491 ([EXPERIMENTAL] Schnorr batch verification for blocks by fjahr)
   • #29247 (CAT in Tapscript (BIP-347) by arminsabouri)

   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.

4. 
   billymcbip commented at 1:55 pm on January 22, 2026: contributor

   Neat. Unit tests and functional tests pass on my end. tACK 6b307a2d12c2047b983edf1fc8104f6ade231bf4

   You can remove {SCRIPT_ERR_UNKNOWN_ERROR, "UNKNOWN_ERROR"} from static ScriptErrorDesc script_errors[].

5. brunoerg marked this as a draft on Jan 22, 2026
6. brunoerg force-pushed on Jan 22, 2026
7. brunoerg marked this as ready for review on Jan 22, 2026
8. 
   brunoerg commented at 2:02 pm on January 22, 2026: contributor

   You can remove {SCRIPT_ERR_UNKNOWN_ERROR, "UNKNOWN_ERROR"} from static ScriptErrorDesc script_errors[].

   Just removed it since it’s now unused.

9. DrahtBot added the label CI failed on Jan 22, 2026
10. 
    billymcbip commented at 2:11 pm on January 22, 2026: contributor

    tACK 3e7c25007571592a277f44fc5d7460458c36cc0a
11. script: add SCRIPT_ERR_SCRIPTNUM error

    It will be used for errors related to CScriptNum (e.g. overflow or encoding errors).
    Currently, we simply return unknown error for these errors.

    0ca4dcd786
12. in src/script/script_error.h:17 in 3e7c250075 outdated

    13@@ -14,6 +14,7 @@ typedef enum ScriptError_t
    14     SCRIPT_ERR_UNKNOWN_ERROR,
    15     SCRIPT_ERR_EVAL_FALSE,
    16     SCRIPT_ERR_OP_RETURN,
    17+    SCRIPT_ERR_SCRIPTNUM,
    

    ---

    ---

    

    darosior commented at 2:36 pm on January 22, 2026:

    Also update ScriptErrorString() if you add a new variant?

    ---

    

    brunoerg commented at 2:49 pm on January 22, 2026:

    Yes, going to update it.

    ---

    

    brunoerg commented at 2:52 pm on January 22, 2026:

    Done, thanks.
13. brunoerg force-pushed on Jan 22, 2026
14. 
    brunoerg commented at 2:52 pm on January 22, 2026: contributor

    Force-pushed addressing #34381 (review)
15. script: use SCRIPT_ERR_SCRIPTNUM for CScriptNum errors bd31a92d67
16. test: remove UNKNOWN_ERROR from script_tests 6f7b4323cb
17. brunoerg force-pushed on Jan 22, 2026
18. 
    w0xlt commented at 4:05 pm on January 22, 2026: contributor

    ACK https://github.com/bitcoin/bitcoin/pull/34381/commits/6f7b4323cb46687ba9df7073a9cde427985d2dfc
19. DrahtBot removed the label CI failed on Jan 22, 2026
20. sedited requested review from darosior on Jan 28, 2026
21. in src/test/script_tests.cpp:55 in 6f7b4323cb

    51@@ -52,7 +52,6 @@ struct ScriptErrorDesc
    52 
    53 static ScriptErrorDesc script_errors[]={
    54     {SCRIPT_ERR_OK, "OK"},
    55-    {SCRIPT_ERR_UNKNOWN_ERROR, "UNKNOWN_ERROR"},
    

    ---

    ---

    

    darosior commented at 2:28 pm on January 28, 2026:

    Interesting, is this now unreachable?

    ---

    

    brunoerg commented at 0:12 am on January 29, 2026:

    I think so. I don’t see a explicit scenario for this unknown error.

    ---

    

    billymcbip commented at 3:18 pm on January 31, 2026:

    Me neither. I wonder if we should we log an “unexpected script error” warning here?
22. darosior approved
23. 
    darosior commented at 2:58 pm on January 28, 2026: member

    ACK 6f7b4323cb46687ba9df7073a9cde427985d2dfc
24. 
    achow101 commented at 0:12 am on January 31, 2026: member

    ACK 6f7b4323cb46687ba9df7073a9cde427985d2dfc
25. achow101 merged this on Jan 31, 2026
26. achow101 closed this on Jan 31, 2026

    ---

Contributors
brunoerg DrahtBot billymcbip darosior w0xlt achow101

Labels
Consensus