script: return proper error for `CScriptNum` errors #34381

pull brunoerg wants to merge 3 commits into bitcoin:master from brunoerg:2026-01-scriptnum changing 6 files +80 −72

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.
DrahtBot added the label Consensus on Jan 22, 2026
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 <code></code> 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.
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[].
brunoerg marked this as a draft on Jan 22, 2026
brunoerg force-pushed on Jan 22, 2026
brunoerg marked this as ready for review on Jan 22, 2026
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.
DrahtBot added the label CI failed on Jan 22, 2026
billymcbip commented at 2:11 PM on January 22, 2026: contributor

tACK 3e7c25007571592a277f44fc5d7460458c36cc0a

Type	Reviewers
ACK	w0xlt, darosior, achow101
Stale ACK	billymcbip

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

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.
brunoerg force-pushed on Jan 22, 2026
brunoerg commented at 2:52 PM on January 22, 2026: contributor

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

ACK https://github.com/bitcoin/bitcoin/pull/34381/commits/6f7b4323cb46687ba9df7073a9cde427985d2dfc
DrahtBot removed the label CI failed on Jan 22, 2026
sedited requested review from darosior on Jan 28, 2026
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 12: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?
darosior approved
darosior commented at 2:58 PM on January 28, 2026: member

ACK 6f7b4323cb46687ba9df7073a9cde427985d2dfc
achow101 commented at 12:12 AM on January 31, 2026: member

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

Contributors

Labels

script: return proper error for `CScriptNum` errors #34381

Code Coverage & Benchmarks

Reviews

Conflicts