kernel: Add script evaluation tracer #35641

sedited commented at 3:00 PM on July 2, 2026: contributor

This adds a callback hook to the kernel library C header for getting the current script evaluation state on each script evaluation step. This can be used for debugging scripts and gives external applications a view into how Bitcoin Core does script evaluation.

The initial sketch for this work was prepared on rust-bitcoinkernel: https://github.com/sedited/rust-bitcoinkernel/tree/feature_script_debug . That branch also includes a very rudimentary script debugger showcasing the use case for this patch. Clankers seems to do a decent job at coding up prototypes for more featureful debuggers, which seems like a fun project to hack on if anybody else wants to give it a try.

This feature is gated behind the ENABLE_SCRIPT_TRACE flag. To compile with the feature enabled, pass -DENABLE_SCRIPT_TRACE=ON. If the feature is not manually enabled, the trace hooks are stubbed out.

The traces are divided into separate frames. RAII is used to guarantee that every script evaluation has at least its start and end state captured. Each instruction generates a unique frame as well.

The ScriptError in the VerifyScript invocation run when calling script_pubkey_verify is added in order to be able to surface the error code in the trace frames.

Another use case could also be using the frames to ensure that the script interpreter is actually doing what we think it is doing during the script tests. This might prevent unit test bugs in the future, i.e. when error conditions shadow actual behaviour, and could also be interesting for fuzzing the script interpreter.

DrahtBot added the label Validation on Jul 2, 2026

DrahtBot commented at 3:00 PM on July 2, 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/35641.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	furszy, w0xlt, theStack
Approach ACK	haanhvu
Stale ACK	alexanderwiederin

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:

#35511 (RFC: consensus: Make CAmount a class by hodlinator)
#30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)

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.

sedited added this to a project on Jul 2, 2026

github-project-automation[bot] changed the project status on Jul 2, 2026

sedited changed the project status on Jul 2, 2026

furszy commented at 4:04 PM on July 2, 2026: member

Concept ACK, haven't checked the implementation. But this is something that would benefit testing greatly.

w0xlt commented at 5:57 PM on July 2, 2026: contributor

Strong Concept ACK

sedited force-pushed on Jul 2, 2026

DrahtBot added the label CI failed on Jul 2, 2026

sedited marked this as ready for review on Jul 2, 2026

DrahtBot removed the label CI failed on Jul 2, 2026

sedited renamed this:
~~kernel: Add script execution tracer~~
kernel: Add script evaluation tracer
on Jul 4, 2026

alexanderwiederin commented at 9:31 AM on July 6, 2026: contributor

Does this support multi-threaded applications? If not, do we want it to?

sedited commented at 9:37 AM on July 6, 2026: contributor

Does this support multi-threaded applications? If not, do we want it to?

It should be thread-safe, but I don't think this is particularly interesting to run in a multi-threaded context. I was thinking of adding an id to the trace frames, but didn't because I felt like the use case is pretty slim.

in src/kernel/bitcoinkernel.h:2025 in 31a5c9869b outdated

2020 | + *
2021 | + * @param[in] callback                   The callback function to register.
2022 | + * @param[in] user_data                  User-defined opaque pointer passed to the callback.
2023 | + * @param[in] user_data_destroy_callback Nullable, function for freeing the user data.
2024 | + * @return                               0 if the script trace feature is available.
2025 | + */

alexanderwiederin commented at 7:38 AM on July 8, 2026:

I suggest we add something like: "The callback fires once per instruction, after the opcode is decoded and before it is dispatched/executed."

sedited commented at 3:26 PM on July 8, 2026:

Taken.

in src/kernel/bitcoinkernel.h:2035 in 31a5c9869b

2030 | +
2031 | +/**
2032 | + * @brief Unregister the global script trace callback.
2033 | + *
2034 | + * After calling this function, no new script trace callbacks will be invoked
2035 | + * once the last invocation returns.

alexanderwiederin commented at 7:42 AM on July 8, 2026:

Is this accurate? Couldn't a thread that copied the callback still begin an invocation?

I think it's safe, but the doc comment overpromises if I am not wrong.

sedited commented at 1:46 PM on July 8, 2026:

Mmh, not sure how to reword this. Can you suggest something?

alexanderwiederin commented at 2:20 PM on July 8, 2026:

Maybe: "Unregistration is not synchronized with callback execution. Pending invocations on other threads may still run to completion after this returns, but no invocations begin thereafter."

What do you think?

sedited commented at 3:26 PM on July 8, 2026:

Taken.

sedited force-pushed on Jul 8, 2026

theStack commented at 8:22 PM on July 8, 2026: contributor

Concept ACK

in src/kernel/bitcoinkernel.h:1999 in 2df591d0b8 outdated

1994 | +    uint8_t opcode;                             //!< The current opcode under evaluation. Only meaningful in step frames.
1995 | +    int op_count;                               //!< Counter towards the ops per script limit.
1996 | +    btck_SigVersion sig_version;                //!< Signature version.
1997 | +    const unsigned char* tapleaf_hash;          //!< Either null if not evaluating a tapleaf, or points to exactly 32 bytes (and sig_version is TAPSCRIPT).
1998 | +    uint32_t codeseparator_pos;                 //!< Opcode position of the last evaluated OP_CODESEPARATOR. 0xFFFFFFFF if none.
1999 | +    int32_t script_error;                       //!< Script error code. Only meaningful in end frames.

alexanderwiederin commented at 3:23 PM on July 10, 2026:

I think we have to define what each code means. What do you think?

sedited commented at 3:40 PM on July 10, 2026:

I think we should leave this here - it's fairly straight forward to get the error code header from somewhere. I'll try to open another pull request for exposing the entire error list in the kernel header, since we'd also want to wire it through the script verification function.

alexanderwiederin commented at 3:36 PM on July 10, 2026: contributor

Do we need a CI job to run with ENABLE_SCRIPT_TRACE=ON?

sedited commented at 3:42 PM on July 10, 2026: contributor

Do we need a CI job to run with ENABLE_SCRIPT_TRACE=ON?

I wasn't sure where to put such a job, so I didn't introduce it here. Maybe we can add a kernel-lib-only job that sets it, but not sure if that is worthwhile enough yet.

in src/script/trace.cpp:18 in 2df591d0b8

  13 | +static ScriptTraceCallback g_script_trace_callback GUARDED_BY(g_script_trace_mutex){nullptr};
  14 | +
  15 | +void TraceScript(const ScriptTraceFrame& trace_frame)
  16 | +{
  17 | +    // Copy the callback in case another thread attempts to de-register
  18 | +    ScriptTraceCallback callback{WITH_LOCK(g_script_trace_mutex, return g_script_trace_callback)};

alexanderwiederin commented at 4:14 PM on July 10, 2026:

Could ScriptTraceScope snapshot the callback in its constructor? That takes both the mutex acquisition and the std::function copy out of the per-opcode path.

sedited commented at 8:19 PM on July 13, 2026:

Done.

sedited force-pushed on Jul 13, 2026

alexanderwiederin commented at 10:32 AM on July 14, 2026: contributor

Thanks! Can we rebase, so we can pull in the changes to rust-bitcoinkernel without reverting commits?

sedited force-pushed on Jul 14, 2026

sedited commented at 10:47 AM on July 14, 2026: contributor

Thanks! Can we rebase, so we can pull in the changes to rust-bitcoinkernel without reverting commits?

Done.

alexanderwiederin commented at 7:20 AM on July 15, 2026: contributor

ACK 114e48e968a087f74c1ab611ac2a31a9266812e1

DrahtBot requested review from furszy on Jul 15, 2026

DrahtBot requested review from theStack on Jul 15, 2026

elmeriniemela commented at 9:44 AM on July 22, 2026: none

I had some fun and tested this with a clanker.

Added python bindings: https://github.com/elmeriniemela/pybitcoinkernel/commit/a3017b277fa1f8ca877218869049f8a35233d69e#diff-203e11f27272eb568dbda09906ba8c8aa6718c4816d32310c279cd1e521e35e8

Made a visualizer using the traces: https://eniemela.fi/bitcoin/tx/8e2ee13d2a19951c2777bb3a54f0cb69a2f76dae8baa954cd86149ed1138cb6c

FWIW, found no issues :)

elmeriniemela commented at 10:10 AM on July 24, 2026: none

I kept playing with this, and one weirdness that I found is that a trace can be invalid, but the related script error code is still OK. My clanker added a condition for this + its own logic to figure out what the real error is: https://github.com/elmeriniemela/pybitcoinkernel/blob/f67613ce47dbf142ba1d6f9746153d9e3e39878d/src/pybitcoinkernel/debugger.py#L809

I can reach the if condition above with this command:

$ python scripts/manual_debug.py 02000000013f7cebd65c27431a90bba7f796914fe8cc2ddfc3f2cbd6f7e5f2fc854534da95000000006b483045022100de1ac3bcdfb0332207c4a91f3832bd2c2915840165f876ab47c5f8996b971c3602201c6c053d750fadde599e6f5c4e1963df0f01fc0d97815e8157e3d59fe09ca30d012103699b464d1d8bc9e47d4fb1cdaa89a1c5783d68363c4dbc4b524ed3d857148617feffffff02836d3c01000000001976a914fc25d6d5c94003bf5b0c7b640a248e2c637fcfb088ac7ada8202000000001976a914fbed3d9b11183209a57999d54d59f67c019e756c88ac6acb0700 --spent 00 0
transaction script verification: INVALID (1 input(s))

########## input 0 ##########
script verification: INVALID  (error: OK)

=== script [#0](/bitcoin-bitcoin/0/): input script (scriptSig) · BASE · 107 bytes ===
    483045022100de1ac3bcdfb0332207c4a91f3832bd2c2915840165f876ab47c5f8996b971c3602201c6c053d750fadde599e6f5c4e1963df0f01fc0d97815e8157e3d59fe09ca30d012103699b464d1d8bc9e47d4fb1cdaa89a1c5783d68363c4dbc4b524ed3d857148617
  [#0000](/bitcoin-bitcoin/0000/)  OP_PUSHBYTES_72        Push the next 72 bytes onto the stack.
         stack: []
  [#0001](/bitcoin-bitcoin/0001/)  OP_PUSHBYTES_33        Push the next 33 bytes onto the stack.
         stack: [3045022100de1ac3bcdfb0332207c4a9...(72 bytes)]
  result: [3045022100de1ac3bcdfb0332207c4a9...(72 bytes), 03699b464d1d8bc9e47d4fb1cdaa89a1...(33 bytes)]  -> OK

=== script [#1](/bitcoin-bitcoin/1/): output script (scriptPubkey) · BASE · 1 bytes ===
    00
  [#0000](/bitcoin-bitcoin/0000/)  OP_0                   Push an empty byte vector (represents false / zero).
         stack: [3045022100de1ac3bcdfb0332207c4a9...(72 bytes), 03699b464d1d8bc9e47d4fb1cdaa89a1...(33 bytes)]
  result: [3045022100de1ac3bcdfb0332207c4a9...(72 bytes), 03699b464d1d8bc9e47d4fb1cdaa89a1...(33 bytes), 0x]  -> OK

  note: scripts evaluated without error, but verification failed (top of stack is false).

I'm not competent enough to say if this is how it should be or not (still learning about bitcoin), but I just thought it might be useful to let you know.

in src/kernel/bitcoinkernel.cpp:1512 in 114e48e968 outdated

1503 | @@ -1499,3 +1504,67 @@ int btck_transaction_check(const btck_Transaction* tx, btck_TxValidationState* v
1504 |      const bool ok = CheckTransaction(*btck_Transaction::get(tx), state);
1505 |      return ok ? 1 : 0;
1506 |  }
1507 | +
1508 | +int btck_script_trace_register_callback(btck_ScriptTraceCallback callback, void* user_data, btck_DestroyCallback user_data_destroy_callback)
1509 | +{
1510 | +#ifndef ENABLE_SCRIPT_TRACE
1511 | +    (void)callback;
1512 | +    if (user_data_destroy_callback) user_data_destroy_callback(user_data);

haanhvu commented at 10:31 AM on July 24, 2026:

Should we clearly comment that user data is destroyed in any case, whether tracing enabled/succeeds or not, to avoid problems like double free?

sedited commented at 8:53 PM on August 28, 2026:

I don't think this needs an extra comment. To me it seems self-explanatory.

sedited commented at 2:26 PM on July 25, 2026: contributor

I'm not competent enough to say if this is how it should be or not (still learning about bitcoin), but I just thought it might be useful to let you know.

What you are seeing is the intended behaviour here. Your LLM seems to already have inferred the correct scenario from the context. The verification of the full script fails, because the remaining stack element (OP_0) is false. VerifyScript (which is what we call here), checks this after script evaluation script/interpreter.cpp:2092. The tracer however is limited to the EvalScript function, which does not enforce this final stack condition. This should be communicated by exposing the full script verification error codes when calling btck_script_pubkey_verify, but as I said here #35641 (review) , I'd prefer doing that in a separate pull request.

in src/test/kernel/test_kernel.cpp:429 in 114e48e968

 424 | +        status));
 425 | +    BOOST_CHECK(status == ScriptVerifyStatus::OK);
 426 | +
 427 | +    ScriptTraceUnsetCallback();
 428 | +
 429 | +    BOOST_CHECK_EQUAL(states.size(), 11);

haanhvu commented at 3:11 PM on July 29, 2026:

BOOST_CHECK_EQUAL(states.size(), 11) is non-fatal, but the test checks states[0] through states[10] after this. So if the size check fails, the test still continues and may access out-of-bound items, resulting in an undefined behavior instead of a clean test failure. Would it make sense to use BOOST_REQUIRE_EQUAL(states.size(), 11) instead?

in src/test/kernel/test_kernel.cpp:427 in 114e48e968 outdated

 422 | +        /*input_index=*/0,
 423 | +        VERIFY_ALL_PRE_TAPROOT,
 424 | +        status));
 425 | +    BOOST_CHECK(status == ScriptVerifyStatus::OK);
 426 | +
 427 | +    ScriptTraceUnsetCallback();

haanhvu commented at 10:36 AM on July 30, 2026:

Should we verify unregistering actually stops tracing?

Something like:

// Before unregistering
const auto frame_count = states.size();

// After unregistering
BOOST_CHECK(legacy_spent_script_pubkey.Verify(...));
BOOST_CHECK_EQUAL(states.size(), frame_count);

in src/test/kernel/test_kernel.cpp:399 in 114e48e968

 394 | +    std::vector<ScriptTraceFrame>& m_state;
 395 | +
 396 | +    void ScriptTrace(ScriptTraceFrame state)
 397 | +    {
 398 | +        m_state.emplace_back(state);
 399 | +    }

haanhvu commented at 8:59 AM on August 1, 2026:

Nit: state is already passed by value here, so m_state.emplace_back(state) performs an extra copy of the ScriptTraceFrame. Would it make sense to move it instead? m_state.emplace_back(std::move(state));

sedited commented at 8:54 PM on August 28, 2026:

I don't think this matters in a test. Besides, the new type is hardly worth the move now.

in src/kernel/bitcoinkernel.h:2031 in 114e48e968 outdated

2026 | +
2027 | +/**
2028 | + * @brief Register a global script trace callback.
2029 | + *
2030 | + * Only one callback can be registered at a time. Registering a new callback
2031 | + * replaces the previous one. The callback fires on entry of the script

haanhvu commented at 10:14 AM on August 1, 2026:

Should we extend the test to cover this callback replacement contract? For example, register tracer A, then register tracer B, perform a new script evaluation, and verify that only tracer B receives frames.

in src/kernel/bitcoinkernel.h:1994 in 114e48e968

1989 | +#define btck_SigVersion_TAPSCRIPT  ((btck_SigVersion)(3))
1990 | +
1991 | +/**
1992 | + * Snapshot of script execution state passed to the trace callback.
1993 | + */
1994 | +typedef struct {

stringintech commented at 1:20 PM on August 11, 2026:

I noticed kernel is zero-copy where possible for frames (e.g. the stack item bytes are borrowed; only the pointer and size arrays are built per frame), and then the C++ wrapper deep-copies every frame unconditionally. If an owning copy is what consumers need anyway, why not do it in the library instead?

Also, would it make sense to make the frame opaque like other btck_* types, with getters, instead of a public struct (perhaps an opaque pointer to a shared_ptr, so copying a frame is just a refcount bump)? The C++ wrapper could then use the existing View/Handle pattern.

in src/kernel/bitcoinkernel.cpp:1539 in 114e48e968

1534 | +            altstack_ptrs.push_back(item.data());
1535 | +            altstack_sizes.push_back(item.size());
1536 | +        }
1537 | +
1538 | +        btck_ScriptTraceFrame btck_frame;
1539 | +        btck_frame.kind = static_cast<btck_ScriptTraceFrameKind>(frame.kind);

stringintech commented at 1:20 PM on August 11, 2026:

Perhaps it is better to use an exhaustive switch for kind and sig_version as well, in both directions (bitcoinkernel.cpp and the C++ wrapper), like the existing enum conversions in the kernel?

in src/script/trace.h:73 in 114e48e968

  68 | +        m_sig_version{sigversion},
  69 | +        m_tapleaf_hash{tapleaf_hash},
  70 | +        m_codeseparator_pos{codeseparator_pos},
  71 | +        m_error{error}
  72 | +    {
  73 | +        Emit(ScriptTraceFrameKind::Begin, m_exec_fn(), /*opcode=*/0, SCRIPT_ERR_OK);

stringintech commented at 1:20 PM on August 11, 2026:

My understanding is that f_exec is only meaningful in step frames, where it says whether the current opcode is about to be evaluated. If so, can we drop the closure and give it a fixed dummy in begin and end frames?

in src/script/trace.h:83 in 114e48e968

  78 | +        Emit(ScriptTraceFrameKind::Step, exec, opcode, SCRIPT_ERR_OK);
  79 | +    }
  80 | +
  81 | +    ~ScriptTraceScope()
  82 | +    {
  83 | +        Emit(ScriptTraceFrameKind::End, m_exec_fn(), /*opcode=*/0, m_error ? *m_error : SCRIPT_ERR_UNKNOWN_ERROR);

stringintech commented at 1:20 PM on August 11, 2026:

ScriptTraceScope learns the outcome only from m_error. set_error in EvalScript also communicates through the returned bool, but the end frame only has access to the error object, so a null m_error makes a successful eval report SCRIPT_ERR_UNKNOWN_ERROR. The kernel path always passes &error, but in case any non-kernel callers want to use the tracer, would it make sense to assert it in the ctor?

<details> <summary>Suggested change</summary>

@@
         m_error{error}
     {
+        assert(!m_callback || error);
         Emit(ScriptTraceFrameKind::Begin, m_exec_fn(), /*opcode=*/0, SCRIPT_ERR_OK);
     }

</details>

DrahtBot added the label Needs rebase on Aug 11, 2026

in src/test/kernel/test_kernel.cpp:438 in 114e48e968 outdated

 433 | +    BOOST_CHECK(states[10].m_kind == ScriptTraceFrameKind::END);
 434 | +    for (int i : {1, 2, 5, 6, 7, 8, 9}) {
 435 | +        BOOST_CHECK(states[i].m_kind == ScriptTraceFrameKind::STEP);
 436 | +    }
 437 | +    BOOST_CHECK_EQUAL(states[3].m_script_error, 0);
 438 | +    BOOST_CHECK_EQUAL(states[10].m_script_error, 0);

haanhvu commented at 9:22 AM on August 14, 2026:

Should we also verify other fields besides m_kind and m_script_error to see if the deep-copy conversion works fully correctly? For example, checking at least one STEP frame with a non-empty stack and verifying that stack items are copied correctly would help catch pointer/size conversion bugs (if there's any).

sedited commented at 8:54 PM on August 28, 2026:

I'm not sure if this is still relevant after the last push. In any case, I don't want the tests here to be exhaustive.

haanhvu commented at 10:07 AM on August 14, 2026: none

Approach ACK

sedited force-pushed on Aug 28, 2026

sedited commented at 8:39 PM on August 28, 2026: contributor

Thank you for the review @stringintech and @haanhvu,

Rebased and updated 114e48e968a087f74c1ab611ac2a31a9266812e1 -> 20fb03490d1f38dde67f058acd68e816595d9222 (kernel_script_tracer_0 -> kernel_script_tracer_1, compare)

Addressed @stringintech's comment, wrapped the frame into nested opaque types and getters.
Addressed @stringintech's comment, use exhaustive enum conversions.
Addressed @stringintech's comment, dropped f_exec pre-loading with a lambda. As suggested, the begin and end frames now contain dummy values that are documented as such.
Addressed @stringintech's comment, added the suggested assertion for passing in the error.
Addressed @haanhvu's comment, made the boost size check a hard requirement.
Addressed @haanhvu's comment, added a quick verification that unregistering the tracer actually works.
Addressed @haanhvu's comment, added a verification that re-registering the tracer works.

sedited force-pushed on Aug 28, 2026

DrahtBot added the label CI failed on Aug 28, 2026

DrahtBot removed the label Needs rebase on Aug 28, 2026

DrahtBot removed the label CI failed on Aug 28, 2026

in src/kernel/bitcoinkernel.h:2222 in 20fb03490d outdated

2217 | + * evaluations already in progress complete with the previously registered
2218 | + * callback. Script evaluations started after this call won't invoke the
2219 | + * callback anymore.
2220 | + */
2221 | +BITCOINKERNEL_API void btck_script_trace_unregister_callback();
2222 | +

alexanderwiederin commented at 8:23 AM on August 31, 2026:

Needs a ///@}

kernel: Add script tracer

This adds a callback hook to the kernel library C header for getting the
current script execution state on each script evaluation step. This can
be used for debugging scripts and gives external applications a view
into how Bitcoin Core does script evaluation.

This feature is gated behind the ENABLE_SCRIPT_TRACE flag. To compile
with the feature enabled, pass `-DENABLE_SCRIPT_TRACE=ON`. If the feature
is not manually enabled, the trace hooks are stubbed out.

The traces are divided into separate frames. RAII is used to guarantee
that every script execution has at least its start and end state
captured. Each instruction generates a unique frame as well.

The ScriptError in the VerifyScript invocation run when calling
script_pubkey_verify is added in order to be able to surface the error
code in the trace frames.

97462c683d

in src/test/kernel/test_kernel.cpp:464 in 20fb03490d outdated

 459 | +        /*tx_to=*/legacy_spending_tx,
 460 | +        /*precomputed_txdata=*/nullptr,
 461 | +        /*input_index=*/0,
 462 | +        VERIFY_ALL_PRE_TAPROOT,
 463 | +        status));
 464 | +    BOOST_CHECK_EQUAL(states.size(), 22);

alexanderwiederin commented at 10:39 AM on August 31, 2026:

Add ScriptTraceUnsetCallback() to avoid the next test case push into a destroyed vector. Test suite currently does not pass with the feature flag.

sedited force-pushed on Aug 31, 2026

sedited commented at 3:46 PM on August 31, 2026: contributor

Updated 20fb03490d1f38dde67f058acd68e816595d9222 -> 97462c683d49792a58ba954723a205cda86a5e0b (kernel_script_tracer_1 -> kernel_script_tracer_2, compare)

Addressed @alexanderwiederin's comment, added close tag for new doxygen section.
Addressed @alexanderwiederin's comment, unsetting the callback after the trace tests are complete.

w0xlt commented at 7:40 PM on September 1, 2026: contributor

The tracing code assumes serror is non-null whenever a callback is registered, but EvalScript permits nullptr for example, CheckSignetBlockSolution() calls VerifyScript() without an error pointer.

This triggers the assertion assert(!m_callback || error); when tracing is enabled.

<details> <summary>Suggestion:</summary>

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 45e7f48cd0..a7d2ce7440 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -417,6 +417,12 @@ static bool EvalChecksig(const valtype& sig, const valtype& pubkey, CScript::con
 
 bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, script_verify_flags flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptExecutionData& execdata, ScriptError* serror)
 {
+#ifdef ENABLE_SCRIPT_TRACE
+    // END frames report the result even when the caller does not request it.
+    ScriptError trace_error{SCRIPT_ERR_UNKNOWN_ERROR};
+    if (serror == nullptr) serror = &trace_error;
+#endif
+
     static const CScriptNum bnZero(0);
     static const CScriptNum bnOne(1);
     // static const CScriptNum bnFalse(0);
diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
index 5b97321367..cbb7464197 100644
--- a/src/test/kernel/test_kernel.cpp
+++ b/src/test/kernel/test_kernel.cpp
@@ -463,6 +463,22 @@ BOOST_AUTO_TEST_CASE(btck_script_trace_tests)
         status));
     BOOST_CHECK_EQUAL(states.size(), 22);
     ScriptTraceUnsetCallback();
+
+    // Script errors are optional for internal EvalScript callers. Exercise the
+    // signet validation path, which intentionally does not request one.
+    states.clear();
+    ChainParams signet_params{ChainType::SIGNET};
+    Block signet_block{hex_string_to_byte_vec(
+        "00000020f61eee3b63a380a477a063af32b2bbc97c9ff9f01f2c4225e973988108000000f575c83235984e7dc4afc1f30944c170462e84437ab6f2d52e16878a79e4678bd1914d5fae77031eccf4070001010000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff025151feffffff0200f2052a010000001600149243f727dd5343293eb83174324019ec16c2630f0000000000000000776a24aa21a9ede2f61c3f71d1defd3fa999dfa36953755c690689799962b48bebd836974e8cf94c4fecc7daa2490047304402205e423a8754336ca99dbe16509b877ef1bf98d008836c725005b3c787c41ebe46022047246e4467ad7cc7f1ad98662afcaf14c115e0095a227c7b05c5182591c23e7e01000120000000000000000000000000000000000000000000000000000000000000000000000000")};
+    BlockValidationState validation_state;
+    ScriptTraceSetCallback(std::make_unique<ScriptTracer>(states));
+    const bool valid{signet_block.Check(signet_params.GetConsensusParams(), BlockCheckFlags::ALL, validation_state)};
+    ScriptTraceUnsetCallback();
+
+    BOOST_CHECK(valid);
+    BOOST_REQUIRE(!states.empty());
+    BOOST_CHECK(states.back().m_kind == ScriptTraceFrameKind::END);
+    BOOST_CHECK_EQUAL(states.back().m_script_error, 0);
 #endif
 }

</details>

w0xlt commented at 8:05 PM on September 1, 2026: contributor

EvalScript rejects oversized scripts before constructing ScriptTraceScope, so a registered callback receives neither BEGIN nor END. Moving the size check below scope construction produces:

create scope → BEGIN
size check → set SCRIPT_ERR_SCRIPT_SIZE → return
scope destructor → END with SCRIPT_ERR_SCRIPT_SIZE

Non-tracing behavior remains the same.

<details> <summary>Suggestion:</summary>

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 45e7f48cd0..1116ffd9c2 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -436,9 +436,6 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
     ConditionStack vfExec;
     std::vector<valtype> altstack;
     set_error(serror, SCRIPT_ERR_UNKNOWN_ERROR);
-    if ((sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0) && script.size() > MAX_SCRIPT_SIZE) {
-        return set_error(serror, SCRIPT_ERR_SCRIPT_SIZE);
-    }
     int nOpCount = 0;
     bool fRequireMinimal = (flags & SCRIPT_VERIFY_MINIMALDATA) != 0;
     uint32_t opcode_pos = 0;
@@ -447,6 +444,10 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
 
     SCRIPT_TRACE_SCOPE(stack, script, opcode_pos, altstack, nOpCount, sigversion, execdata.m_tapleaf_hash_init ? execdata.m_tapleaf_hash.data() : nullptr, execdata.m_codeseparator_pos, serror);
 
+    if ((sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0) && script.size() > MAX_SCRIPT_SIZE) {
+        return set_error(serror, SCRIPT_ERR_SCRIPT_SIZE);
+    }
+
     try
     {
         for (; pc < pend; ++opcode_pos) {
diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
index 5b97321367..999199bb3b 100644
--- a/src/test/kernel/test_kernel.cpp
+++ b/src/test/kernel/test_kernel.cpp
@@ -4,6 +4,7 @@
 
 #include <kernel/bitcoinkernel.h>
 #include <kernel/bitcoinkernel_wrapper.h>
+#include <script/script_error.h>
 #include <util/byte_units.h>
 #include <util/fs.h>
 
@@ -463,6 +464,23 @@ BOOST_AUTO_TEST_CASE(btck_script_trace_tests)
         status));
     BOOST_CHECK_EQUAL(states.size(), 22);
     ScriptTraceUnsetCallback();
+
+    std::vector<ScriptTracer::Snapshot> oversized_states;
+    ScriptPubkey oversized_script_pubkey{std::vector<std::byte>(10'001)};
+    ScriptTraceSetCallback(std::make_unique<ScriptTracer>(oversized_states));
+    BOOST_CHECK(!oversized_script_pubkey.Verify(
+        /*amount=*/0,
+        /*tx_to=*/legacy_spending_tx,
+        /*precomputed_txdata=*/nullptr,
+        /*input_index=*/0,
+        VERIFY_ALL_PRE_TAPROOT,
+        status));
+    ScriptTraceUnsetCallback();
+
+    BOOST_REQUIRE(oversized_states.size() >= 2);
+    BOOST_CHECK(oversized_states[oversized_states.size() - 2].m_kind == ScriptTraceFrameKind::BEGIN);
+    BOOST_CHECK(oversized_states.back().m_kind == ScriptTraceFrameKind::END);
+    BOOST_CHECK_EQUAL(oversized_states.back().m_script_error, SCRIPT_ERR_SCRIPT_SIZE);
 #endif
 }

</details>

w0xlt commented at 8:37 PM on September 1, 2026: contributor

Is there a specific reason these need to be function-like macros? Expanding them at their sole call site makes the object lifetime and Step() call explicit and easier to debug, while the #ifdefs still prevent argument evaluation when tracing is disabled.

The trade-off is some additional preprocessor noise and repetition if more step sites are added, but with only one scope and one Step() call, this seems worthwhile.

<details> <summary>Possible version without the tracing helper macros:</summary>

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 45e7f48cd0..f23673007f 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -445,7 +445,19 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
     execdata.m_codeseparator_pos = 0xFFFFFFFFUL;
     execdata.m_codeseparator_pos_init = true;
 
-    SCRIPT_TRACE_SCOPE(stack, script, opcode_pos, altstack, nOpCount, sigversion, execdata.m_tapleaf_hash_init ? execdata.m_tapleaf_hash.data() : nullptr, execdata.m_codeseparator_pos, serror);
+#ifdef ENABLE_SCRIPT_TRACE
+    ScriptTraceScope script_trace_scope{
+        stack,
+        script,
+        opcode_pos,
+        altstack,
+        nOpCount,
+        static_cast<uint8_t>(sigversion),
+        execdata.m_tapleaf_hash_init ? execdata.m_tapleaf_hash.data() : nullptr,
+        execdata.m_codeseparator_pos,
+        serror,
+    };
+#endif
 
     try
     {
@@ -459,7 +471,9 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                 return set_error(serror, SCRIPT_ERR_BAD_OPCODE);
             }
 
-            SCRIPT_TRACE_STEP(fExec, opcode);
+#ifdef ENABLE_SCRIPT_TRACE
+            script_trace_scope.Step(fExec, static_cast<uint8_t>(opcode));
+#endif
 
             if (vchPushValue.size() > MAX_SCRIPT_ELEMENT_SIZE)
                 return set_error(serror, SCRIPT_ERR_PUSH_SIZE);
diff --git a/src/script/trace.h b/src/script/trace.h
index 7cd63fffdd..1f1e6aec8e 100644
--- a/src/script/trace.h
+++ b/src/script/trace.h
@@ -104,15 +104,6 @@ private:
     }
 };
 
-#define SCRIPT_TRACE_SCOPE(stack, script, opcode_pos, altstack, nOpCount, sigversion, tapleaf_hash, codeseparator_pos, error) \
-    ScriptTraceScope script_trace_scope { stack, script, opcode_pos, altstack, nOpCount, static_cast<uint8_t>(sigversion), tapleaf_hash, codeseparator_pos, error }
-
-#define SCRIPT_TRACE_STEP(fExec, opcode) \
-    script_trace_scope.Step(fExec, static_cast<uint8_t>(opcode))
-
-#else
-#define SCRIPT_TRACE_SCOPE(stack, script, opcode_pos, altstack, nOpCount, sigversion, tapleaf_hash, codeseparator_pos, error) static_assert(true)
-#define SCRIPT_TRACE_STEP(fExec, opcode) static_assert(true)
 #endif // ENABLE_SCRIPT_TRACE
 
 #endif // BITCOIN_SCRIPT_TRACE_H

</details>