kernel: assert invalid buffer preconditions in `btck_*_create` functions #35312

pull stringintech wants to merge 1 commits into bitcoin:master from stringintech:2026/05/kernel-assert-buffer-preconditions changing 1 files +4 −12

stringintech commented at 6:38 AM on May 18, 2026: contributor

The kernel API appears to use nullptr returns to report failures that callers may reasonably want to recover from: malformed serialized input, object construction failures, chainstate/load failures, and similar runtime conditions.

The raw create-function buffer checks seem to be a different case. A failure of ptr == nullptr && len > 0 does not indicate malformed input data or a failure encountered while decoding, deserializing, or constructing the requested object. Returning nullptr for these checks widens the recoverable error surface with cases that are better treated as programmer errors, similar to other asserted preconditions in this API such as invalid indices and impossible enum/flag states.

This change switches those buffer argument checks from nullptr returns to assertions in btck_transaction_create, btck_script_pubkey_create, btck_block_create, and btck_block_header_create. These functions still return nullptr when the provided bytes cannot be parsed or when object creation fails during processing.

I ended up looking at this while working on the kernel-bindings-tests spec/schema for btck_script_pubkey_create, where treating this path as a regular error did not seem like the right contract: https://github.com/stringintech/kernel-bindings-tests/pull/14#discussion_r3240859568.

kernel: assert invalid buffer preconditions in `btck_*_create` functions

Switch buffer `ptr == nullptr && len > 0` checks from `nullptr` returns to assertions in `btck_transaction_create`, `btck_script_pubkey_create`, `btck_block_create`, and `btck_block_header_create`. These checks represent invalid caller preconditions, not failures encountered while decoding, deserializing, or constructing the requested object.

4c6d65317e

DrahtBot added the label Validation on May 18, 2026
DrahtBot commented at 6:38 AM on May 18, 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/35312.



Reviews

See the guideline for information on the review process.

Type Reviewers

Approach ACK stickies-v

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
in src/kernel/bitcoinkernel.cpp:1378 in 4c6d65317e
```
1374 | @@ -1381,9 +1375,7 @@ int btck_chain_contains(const btck_Chain* chain, const btck_BlockTreeEntry* entr
1375 |  
1376 |  btck_BlockHeader* btck_block_header_create(const void* raw_block_header, size_t raw_block_header_len)
1377 |  {
1378 | -    if (raw_block_header == nullptr && raw_block_header_len != 0) {
1379 | -        return nullptr;
1380 | -    }
1381 | +    assert(raw_block_header != nullptr || raw_block_header_len == 0);
```
stickies-v commented at 4:09 PM on May 18, 2026:

As you highlighted elsewhere, since the interface states we require non-null and len 80, I think it makes sense to assert that here too (and with BITCOINKERNEL_ARG_NONNULL)? In my view this is orthogonal to the changes done in #33853, where empty spans are allowed.

stringintech commented at 7:46 AM on May 19, 2026:

I had noticed that submitheader, via DecodeHexBlockHeader(), is currently lenient and accepts full block hex too, with some functional tests relying on that behavior, so I wondered whether we should mirror that here by loosening the API contract instead. But on second thought, that leniency may not be especially necessary or justified here: if the goal is just to avoid copying when starting from existing block bytes, callers could still pass a subspan over the first 80 bytes. What do you think? (Also the length check was previously discussed here: #33822 (review))

maflcko commented at 7:56 AM on May 19, 2026:

(Note if the prior GitHub link doesn't work. (it does not for me): https://mirror.b10c.me/bitcoin-bitcoin/33822#discussion_r2508035777 does)
stickies-v commented at 4:10 PM on May 18, 2026: contributor

Approach ACK
carloantinarella commented at 7:30 PM on May 19, 2026: contributor
I see there is also the function btck_chainstate_manager_options_create, which handles argument validation slightly differently and returns nullptr when, for example, data_dir == nullptr && data_dir_len > 0 (i.e., a programmer error). A suggestion could be:
```
- if (data_dir == nullptr || data_dir_len == 0 || blocks_dir == nullptr || blocks_dir_len == 0) {
+ assert(data_dir != nullptr || data_dir_len == 0);
+ assert(blocks_dir != nullptr || blocks_dir_len == 0);
+ if (data_dir_len == 0 || blocks_dir_len == 0) {
```
I don't know if it is considered out of scope for this PR.

Type	Reviewers
Approach ACK	stickies-v

Contributors

Labels

kernel: assert invalid buffer preconditions in `btck_*_create` functions #35312

Code Coverage & Benchmarks

Reviews