kernel: Add script evaluation tracer #35641

pull sedited wants to merge 1 commits into bitcoin:master from sedited:kernel_script_tracer changing 9 files +458 −2
  1. 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.

  2. DrahtBot added the label Validation on Jul 2, 2026
  3. DrahtBot commented at 3:00 PM on July 2, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35641.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

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

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35511 (RFC: consensus: Make CAmount a class by hodlinator)
    • #35496 (kernel: add btck_set_mock_time for testing time-dependent paths by stringintech)
    • #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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. sedited added this to a project on Jul 2, 2026
  5. github-project-automation[bot] changed the project status on Jul 2, 2026
  6. sedited changed the project status on Jul 2, 2026
  7. 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.

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

    Strong Concept ACK

  9. sedited force-pushed on Jul 2, 2026
  10. sedited force-pushed on Jul 2, 2026
  11. DrahtBot added the label CI failed on Jul 2, 2026
  12. sedited marked this as ready for review on Jul 2, 2026
  13. DrahtBot removed the label CI failed on Jul 2, 2026
  14. sedited renamed this:
    kernel: Add script execution tracer
    kernel: Add script evaluation tracer
    on Jul 4, 2026
  15. alexanderwiederin commented at 9:31 AM on July 6, 2026: contributor

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

  16. 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.

  17. 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.

  18. 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.

  19. sedited force-pushed on Jul 8, 2026
  20. theStack commented at 8:22 PM on July 8, 2026: contributor

    Concept ACK

  21. 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.

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

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

  23. 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.

  24. 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.

  25. sedited force-pushed on Jul 13, 2026
  26. 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?

  27. 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.
    114e48e968
  28. sedited force-pushed on Jul 14, 2026
  29. 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.

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

    ACK 114e48e968a087f74c1ab611ac2a31a9266812e1

  31. DrahtBot requested review from furszy on Jul 15, 2026
  32. DrahtBot requested review from theStack on Jul 15, 2026
  33. 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.

  34. in src/kernel/bitcoinkernel.cpp:1512 in 114e48e968
    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?

  35. 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.

  36. 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?

  37. in src/test/kernel/test_kernel.cpp:427 in 114e48e968
     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);
    
  38. 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));

  39. in src/kernel/bitcoinkernel.h:2031 in 114e48e968
    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.

  40. 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.

  41. 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?

  42. 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?

  43. 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>

  44. DrahtBot added the label Needs rebase on Aug 11, 2026
  45. DrahtBot commented at 9:12 PM on August 11, 2026: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->

    🐙 This pull request conflicts with the target branch and needs rebase.

  46. in src/test/kernel/test_kernel.cpp:438 in 114e48e968
     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).

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

    Concept ACK


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-08-19 11:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me