Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC813bdb38c28a: [interpreter] Return `INTEGER_OVERFLOW` or `BAD_INTEGER_ENCODING` instead of…

Summary

Currently, the Script interpreter returns UNKNOWN when it encountered an integer overflow or a non-minimally encoded integer.

This has been a nuisance for new Script devs for a while, because "unknown error" sounds like the interpreter is broken, but it's actually expected behavior.

Also, this makes script number tests more brittle.

Instead, we return INTEGER_OVERFLOW if there's been any kind of integer overflow, e.g. when parsing operands, or when executing an operation like OP_ADD, OP_BIN2NUM, etc.

Test Plan

ninja check && ninja check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Branch

interpreter-better-scriptnum-errors

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 34129
Build 67726: Build Diff	lint-circular-dependencies · build-without-wallet · build-fuzzer · build-diff · build-debug · build-clang-tidy · build-clang
Build 67725: arc lint + arc unit

Event Timeline

tobias_ruck created this revision.Aug 12 2025, 12:01

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 12 2025, 12:01

Harbormaster completed remote builds in B34128: Diff 55208.Aug 12 2025, 12:19

rebase onto master, fix comment

tobias_ruck mentioned this in D18469: [64-bit ints] Add activation logic.Aug 12 2025, 12:30

tobias_ruck published this revision for review.Aug 12 2025, 12:36

Fabien requested changes to this revision.Aug 12 2025, 13:27

Fabien added inline comments.

src/script/interpreter.cpp
212	This ensures we reset any previous script_error in case of success
src/script/interpreter.h
145–148	If it can't throw, let's mark it noexcept
src/script/script.h
215–225

This revision now requires changes to proceed.Aug 12 2025, 13:27

tobias_ruck added inline comments.Aug 12 2025, 13:34

src/script/interpreter.cpp
212	but that changes the behavior of RunNextOp, it previously did not do that. And a Script is not successful after just one opcode, it must run all of them to be successful

address review

Harbormaster completed remote builds in B34129: Diff 55211.Aug 12 2025, 13:37

tobias_ruck added a child revision: D18469: [64-bit ints] Add activation logic.Aug 12 2025, 13:42

Harbormaster completed remote builds in B34131: Diff 55214.Aug 12 2025, 13:49

tobias_ruck added inline comments.Aug 12 2025, 14:01

src/script/interpreter.h
146 ↗	(On Diff #55214)

remove redundant "will not throw" comment

Harbormaster completed remote builds in B34132: Diff 55215.Aug 12 2025, 14:17

Fabien accepted this revision.Aug 12 2025, 15:41

This revision is now accepted and ready to land.Aug 12 2025, 15:41

Closed by commit rABC813bdb38c28a: [interpreter] Return `INTEGER_OVERFLOW` or `BAD_INTEGER_ENCODING` instead of… (authored by tobias_ruck). · Explain WhyAug 12 2025, 21:08

This revision was automatically updated to reflect the committed changes.

tobias_ruck added a commit: rABC813bdb38c28a: [interpreter] Return `INTEGER_OVERFLOW` or `BAD_INTEGER_ENCODING` instead of….

[interpreter] Return `INTEGER_OVERFLOW` or `BAD_INTEGER_ENCODING` instead of UNKNOWN
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 55211

src/iguana/tests/test_iguana.py

src/script/interpreter.h

src/script/interpreter.cpp

src/script/script.h

src/script/script_error.h

src/script/script_error.cpp

src/test/data/script_tests.json

src/test/fuzz/string.cpp

src/test/intmath_tests.cpp

src/test/monolith_opcodes_tests.cpp

src/test/script_tests.cpp

src/test/scriptinterpreter_tests.cpp

src/test/scriptnum_63bit_tests.cpp

src/test/scriptnum_tests.cpp

[interpreter] Return `INTEGER_OVERFLOW` or `BAD_INTEGER_ENCODING` instead of UNKNOWNClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 55211

src/iguana/tests/test_iguana.py

src/script/interpreter.h

src/script/interpreter.cpp

src/script/script.h

src/script/script_error.h

src/script/script_error.cpp

src/test/data/script_tests.json

src/test/fuzz/string.cpp

src/test/intmath_tests.cpp

src/test/monolith_opcodes_tests.cpp

src/test/script_tests.cpp

src/test/scriptinterpreter_tests.cpp

src/test/scriptnum_63bit_tests.cpp

src/test/scriptnum_tests.cpp

[interpreter] Return `INTEGER_OVERFLOW` or `BAD_INTEGER_ENCODING` instead of UNKNOWN
ClosedPublic
Actions

Revision Contents
Changeset List