kernel: expose witness stack and scriptSig for btck

pzafonte commented at 12:53 PM on May 26, 2026: contributor

Silent payments scanning needs the public key from every input. For SegWit inputs it is in the witness stack. For P2PKH inputs it is in scriptSig. Without these new functions, callers must deserialize the raw transaction themselves to reach that data, which is difficult and error-prone.

Adds three functions:

btck_transaction_input_count_witness_stack_items item count
btck_transaction_input_get_witness_stack_item single item by index via btck_WriteBytes
btck_transaction_input_get_script_sig full scriptSig via btck_WriteBytes

All three are exposed in the C++ wrapper via GetWitnessStackNumItems(), GetWitnessStackItem(), WitnessStack() (generated by MAKE_RANGE_METHOD), and GetScriptSig().

The scriptSig commit was previously in #35382, bundled here per stickies-v's suggestion.

DrahtBot added the label Validation on May 26, 2026

DrahtBot commented at 12:53 PM on May 26, 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/35380.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	sedited
Concept ACK	stickies-v, theStack, w0xlt, yuvicc

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

sedited added this to a project on May 26, 2026

github-project-automation[bot] changed the project status on May 26, 2026

sedited changed the project status on May 26, 2026

in src/kernel/bitcoinkernel.h:1689 in f09656e96a

1684 | + * @brief Return the number of witness stack items of a transaction input.
1685 | + *
1686 | + * @param[in] transaction_input Non-null.
1687 | + * @return                      The number of witness stack items.
1688 | + */
1689 | +BITCOINKERNEL_API size_t btck_transaction_input_get_witness_stack_num_items(

stickies-v commented at 1:19 PM on May 26, 2026:

nit: we use _count for similar functions, so would stay consistent with btck_transaction_input_count_witness_stack_items

pzafonte commented at 2:53 PM on May 26, 2026:

Done. Updated in 34897f3

in src/test/kernel/test_kernel.cpp:524 in f09656e96a

 519 | +
 520 | +    // P2WSH input: OP_0, sig, sig, redeem_script (0, 71, 71, 105 bytes).
 521 | +    Transaction segwit_tx{hex_string_to_byte_vec("010000000001011f97548fbbe7a0db7588a66e18d803d0089315aa7d4cc28360b6ec50ef36718a0100000000ffffffff02df1776000000000017a9146c002a686959067f4866b8fb493ad7970290ab728757d29f0000000000220020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d04004730440220565d170eed95ff95027a69b313758450ba84a01224e1f7f130dda46e94d13f8602207bdd20e307f062594022f12ed5017bbf4a055a06aea91c10110a0e3bb23117fc014730440220647d2dc5b15f60bc37dc42618a370b2a1490293f9e5c8464f53ec4fe1dfe067302203598773895b4b16d37485cbe21b337f4e4b650739880098c592553add7dd4355016952210375e00eb72e29da82b89367947f29ef34afb75e8654f6ea368e0acdfd92976b7c2103a1b26313f430c4b15bb1fdce663207659d8cac749a0e53d70eff01874496feff2103c96d495bfdd5ba4145e3e046fee45e84a8a48ad05bd8dbb395c011a32cf9f88053ae00000000")};
 522 | +    TransactionInputView segwit_input = segwit_tx.GetInput(0);
 523 | +    BOOST_CHECK_EQUAL(segwit_input.GetWitnessStackNumItems(), 4);
 524 | +    BOOST_CHECK_EQUAL(segwit_input.GetWitnessStackItem(0).size(), 0);

stickies-v commented at 1:19 PM on May 26, 2026:

Would be better to test full equality rather than just the size

pzafonte commented at 2:37 PM on May 26, 2026:

Updated the tests in 34897f3

in src/kernel/bitcoinkernel.h:1700 in f09656e96a outdated

1695 | + *
1696 | + * @param[in] transaction_input Non-null.
1697 | + * @param[in] index             Index of the witness stack item.
1698 | + * @param[in] writer            Non-null, function pointer for writing bytes.
1699 | + * @param[in] user_data         Nullable, passed back through the writer callback.
1700 | + * @return                      0 on success, -1 if the index is out of bounds.

stickies-v commented at 1:35 PM on May 26, 2026:

Depending on the writer impl, -1 can also indicate that writing failed. As a general pattern, I wonder if it makes sense to wholesale return the writer error code. Alternatively we could catch the writer error code, log it, and then returning a pre-defined writer error code, so we get e.g. 0 on success, -1 on out of bounds, and -2 on write failure?

DrahtBot added the label CI failed on May 26, 2026

DrahtBot commented at 1:39 PM on May 26, 2026: contributor

🚧 At least one of the CI tasks failed. Task Alpine (musl): https://github.com/bitcoin/bitcoin/actions/runs/26449143818/job/77864724232 LLM reason (✨ experimental): CI failed because compilation of test_kernel errored out: bitcoinkernel_wrapper.h used assert(...) without including it (so assert was not declared).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

stickies-v commented at 1:41 PM on May 26, 2026: contributor

Concept ACK, seems useful to expose.

Small motivation nit:

Without access to it, kernel API callers cannot extract the sender key from these input types which is a requirement for silent payments scanning.

Callers can, but they have to parse the entire transaction, so exposing these functions obviously makes that easier and less error-prone.

pzafonte force-pushed on May 26, 2026

pzafonte renamed this:
~~kernel: expose witness stack items for btck_TransactionInput~~
kernel: expose witness stack and scriptSig for btck_TransactionInput
on May 26, 2026

pzafonte commented at 2:49 PM on May 26, 2026: contributor

Callers can, but they have to parse the entire transaction, so exposing these functions obviously makes that easier and less error-prone.

Updated the title and description to clarify motivation and reflect both commits.

theStack commented at 3:23 PM on May 26, 2026: contributor

Concept ACK

DrahtBot removed the label CI failed on May 26, 2026

w0xlt commented at 9:40 PM on May 26, 2026: contributor

Concept ACK

in src/kernel/bitcoinkernel_wrapper.h:596 in 34897f32eb outdated

 591 | +    size_t GetWitnessStackNumItems() const
 592 | +    {
 593 | +        return btck_transaction_input_count_witness_stack_items(impl());
 594 | +    }
 595 | +
 596 | +    std::vector<std::byte> GetWitnessStackItem(size_t index) const

yuvicc commented at 10:10 AM on May 27, 2026:

I think this could crash the program when an invalid index is passed?

BOOST_CHECK_THROW(segwit_input.GetWitnessStackItem(4), std::out_of_range);

</details>

<details> <summary>output Log</summary>

unknown location:0: fatal error: in "btck_transaction_input": signal: SIGABRT (application abort requested)

</details>

I think the cause might be here at L318 - L320:

if (to_bytes(object, write, &user_data) != 0) {
    std::rethrow_exception(user_data.exception);
}

</details>

When passing an index out of bounds, the C API returns -1, and the user_data.exception remains nullptr. Passing nullptr to std::rethrow_exception results in undefined behaviour (UB). Probably add a bounds check here?

  if (to_bytes(object, write, &user_data) != 0) {
      if (user_data.exception) std::rethrow_exception(user_data.exception);
      throw std::runtime_error("Kernel API call failed");
  }

</details>

yuvicc commented at 10:11 AM on May 27, 2026: contributor

Concept ACK

pzafonte force-pushed on May 29, 2026

pzafonte commented at 4:21 PM on May 29, 2026: contributor

Double checking myself.

Looking at src/validation.cpp:3869-L3871 const auto& witness_stack{block.vtx[0]->vin[0].scriptWitness.stack}; if (witness_stack.size() != 1 || witness_stack[0].size() != 32) {

And src/core_io.cpp:463-464 txinwitness.reserve(tx.vin[i].scriptWitness.stack.size()); for (const auto& item : tx.vin[i].scriptWitness.stack) {

I don't think valid usage would ever reach an unverified index given the count function is available, so assert feels more appropriate here?

in src/kernel/bitcoinkernel_wrapper.h:11 in a398cd8405

   7 | @@ -8,6 +8,7 @@
   8 |  #include <kernel/bitcoinkernel.h>
   9 |  
  10 |  #include <array>
  11 | +#include <cassert>

sedited commented at 2:22 PM on June 4, 2026:

I think this include must be from a prior revision. Can you remove it?

in src/kernel/bitcoinkernel_wrapper.h:591 in a398cd8405

 586 | @@ -586,6 +587,27 @@ class TransactionInputApi
 587 |      {
 588 |          return btck_transaction_input_get_sequence(impl());
 589 |      }
 590 | +
 591 | +    size_t GetWitnessStackNumItems() const

sedited commented at 2:25 PM on June 4, 2026:

I think this should be called CountWitnessStackItems.

sedited commented at 2:37 PM on June 4, 2026: contributor

I would drop the description in the commit message of what the witness data is needed for. There are many use cases for this, and I don't think enumerating them is useful.

pzafonte force-pushed on Jun 4, 2026

DrahtBot added the label CI failed on Jun 4, 2026

DrahtBot commented at 5:06 PM on June 4, 2026: contributor

🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/26962501443/job/79556335400 LLM reason (✨ experimental): CI failed due to a C++ build error: btck_transaction_input_get_script_sig is an undeclared identifier in bitcoinkernel_wrapper.h (compiling bitcoin-chainstate.cpp).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

kernel: expose witness stack items for btck_TransactionInput c6d5324144

pzafonte force-pushed on Jun 4, 2026

kernel: expose scriptSig for btck_TransactionInput 269e39b2dc

pzafonte force-pushed on Jun 4, 2026

DrahtBot removed the label CI failed on Jun 4, 2026

sedited approved

sedited commented at 7:59 AM on June 11, 2026: contributor

ACK 269e39b2dc41f99f6184120f648f5ab3c1c93b7d

DrahtBot requested review from theStack on Jun 11, 2026

DrahtBot requested review from stickies-v on Jun 11, 2026

DrahtBot requested review from yuvicc on Jun 11, 2026

in src/kernel/bitcoinkernel.h:1697 in 269e39b2dc

1692 | +/**
1693 | + * @brief Serialize a witness stack item at a given index through the passed in
1694 | + * callback.
1695 | + *
1696 | + * @param[in] transaction_input Non-null.
1697 | + * @param[in] index             Index of the witness stack item.

stickies-v commented at 1:30 PM on June 12, 2026:

nit: index is a transaction input propery

 * [@param](/bitcoin-bitcoin/contributor/param/)[in] index             Index of the witness stack item in the transaction input.

in src/kernel/bitcoinkernel.h:1702 in 269e39b2dc

1697 | + * @param[in] index             Index of the witness stack item.
1698 | + * @param[in] writer            Non-null, function pointer for writing bytes.
1699 | + * @param[in] user_data         Nullable, passed back through the writer callback.
1700 | + * @return                      The return value of the writer. Aborts if index is out of bounds.
1701 | + */
1702 | +BITCOINKERNEL_API int BITCOINKERNEL_WARN_UNUSED_RESULT btck_transaction_input_get_witness_stack_item(

stickies-v commented at 10:24 AM on June 15, 2026:

I think it makes sense to add a btck_WitnessStack type here. We're mapping a getter onto its parent, and the witness stack is not an implementation detail. If in the future we want to extend functionality (e.g. expose the taproot annex), it will also be much cleaner to do so this way.

Would be curious to hear more opinions on this. We don't need to overly complicate things either, it just makes sense to me.

const btck_WitnessStack* btck_transaction_input_get_witness_stack(const btck_TransactionInput*);

size_t btck_witness_stack_count(const btck_WitnessStack*);
int btck_witness_stack_item_to_bytes(const btck_WitnessStack*, size_t index, btck_WriteBytes, void*);
btck_WitnessStack* btck_witness_stack_copy(const btck_WitnessStack*);
void btck_witness_stack_destroy(btck_WitnessStack*);

alexanderwiederin commented at 7:27 PM on June 16, 2026:

Not an easy decision. When do you expect further functionality, like the annex, to be needed?

stickies-v commented at 11:09 AM on June 18, 2026:

I'm not sure when users who want this will show up, but I think it's a very natural extension of the API so I suspect eventually we will.

Personally, I think even without adding more functionality a separate btck_WitnessStack makes sense from a layering perspective.

DrahtBot requested review from stickies-v on Jun 15, 2026

kernel: expose witness stack and scriptSig for btck_TransactionInput #35380

Code Coverage & Benchmarks

Reviews