highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-14 00:30:17 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#35312: kernel: assert invalid buffer preconditions in btck_*_create functions

Browse Source 
570a627640 kernel: assert invalid buffer preconditions in `btck_*_create` functions (stringintech)

Pull request description:

  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 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`, `btck_block_header_create`, and `btck_chainstate_manager_options_create`. `btck_block_header_create` additionally asserts the pre-existing documented length contract (must be 80 bytes). 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.

ACKs for top commit:
  stickies-v:
    ACK 570a627640
  janb84:
    ACK 570a627640
  w0xlt:
    ACK 570a627640
  sedited:
    ACK 570a627640

Tree-SHA512: 064d834abe0c27245a144e5290bbeeb510daf9e4d50bb3a8e50bd8a0bf897b3dcf6ad5acfcabf1d8110da120e5e014ee3aea0241c0f181a21c6f3c14dc452ade
This commit is contained in:
merge-script
2026-06-01 21:47:38 +02:00
parent fbe628756c 570a627640
commit 19e45334bc
3 changed files with 13 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
src/kernel/bitcoinkernel.cpp
View File

@@ -507,9 +507,7 @@ struct btck_ConsensusParams: Handle<btck_ConsensusParams, Consensus::Params> {};
btck_Transaction* btck_transaction_create(const void* raw_transaction, size_t raw_transaction_len)
{
if (raw_transaction == nullptr && raw_transaction_len != 0) {
return nullptr;
}
assert(raw_transaction != nullptr || raw_transaction_len == 0);
try {
SpanReader stream{std::span{reinterpret_cast<const std::byte*>(raw_transaction), raw_transaction_len}};
return btck_Transaction::create(std::make_shared<const CTransaction>(deserialize, TX_WITH_WITNESS, stream));
@@ -574,9 +572,7 @@ void btck_transaction_destroy(btck_Transaction* transaction)
btck_ScriptPubkey* btck_script_pubkey_create(const void* script_pubkey, size_t script_pubkey_len)
{
if (script_pubkey == nullptr && script_pubkey_len != 0) {
return nullptr;
}
assert(script_pubkey != nullptr || script_pubkey_len == 0);
auto data = std::span{reinterpret_cast<const uint8_t*>(script_pubkey), script_pubkey_len};
return btck_ScriptPubkey::create(data.begin(), data.end());
}
@@ -966,7 +962,9 @@ btck_BlockValidationResult btck_block_validation_state_get_block_validation_resu
btck_ChainstateManagerOptions* btck_chainstate_manager_options_create(const btck_Context* context, const char* data_dir, size_t data_dir_len, const char* blocks_dir, size_t blocks_dir_len)
{
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) {
LogError("Failed to create chainstate manager options: dir must be non-null and non-empty");
return nullptr;
}
@@ -1117,9 +1115,7 @@ int btck_chainstate_manager_import_blocks(btck_ChainstateManager* chainman, cons
btck_Block* btck_block_create(const void* raw_block, size_t raw_block_length)
{
if (raw_block == nullptr && raw_block_length != 0) {
return nullptr;
}
assert(raw_block != nullptr || raw_block_length == 0);
auto block{std::make_shared<CBlock>()};
SpanReader stream{std::span{reinterpret_cast<const std::byte*>(raw_block), raw_block_length}};
@@ -1383,9 +1379,7 @@ int btck_chain_contains(const btck_Chain* chain, const btck_BlockTreeEntry* entr
btck_BlockHeader* btck_block_header_create(const void* raw_block_header, size_t raw_block_header_len)
{
if (raw_block_header == nullptr && raw_block_header_len != 0) {
return nullptr;
}
assert(raw_block_header != nullptr && raw_block_header_len == 80);
auto header{std::make_unique<CBlockHeader>()};
SpanReader stream{std::span{reinterpret_cast<const std::byte*>(raw_block_header), raw_block_header_len}};

13
src/kernel/bitcoinkernel.h
View File

@@ -1151,12 +1151,11 @@ BITCOINKERNEL_API const btck_BlockTreeEntry* btck_block_tree_entry_get_ancestor(
*
* @param[in] context Non-null, the created options and through it the chainstate manager will
* associate with this kernel context for the duration of their lifetimes.
* @param[in] data_directory Non-null, non-empty path string of the directory containing the
* chainstate data. If the directory does not exist yet, it will be
* created.
* @param[in] blocks_directory Non-null, non-empty path string of the directory containing the block
* data. If the directory does not exist yet, it will be created.
* @return The allocated chainstate manager options, or null on error.
* @param[in] data_directory Path string of the directory containing the chainstate data. If the directory
* does not exist yet, it will be created.
* @param[in] blocks_directory Path string of the directory containing the block data. If the directory
* does not exist yet, it will be created.
* @return The allocated chainstate manager options, or null on error (e.g. if a path is invalid).
*/
BITCOINKERNEL_API btck_ChainstateManagerOptions* BITCOINKERNEL_WARN_UNUSED_RESULT btck_chainstate_manager_options_create(
const btck_Context* context,
@@ -1877,7 +1876,7 @@ BITCOINKERNEL_API void btck_block_hash_destroy(btck_BlockHash* block_hash);
* @return btck_BlockHeader, or null on error.
*/
BITCOINKERNEL_API btck_BlockHeader* BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_header_create(
const void* raw_block_header, size_t raw_block_header_len);
const void* raw_block_header, size_t raw_block_header_len) BITCOINKERNEL_ARG_NONNULL(1);
/**
* @brief Copy a btck_BlockHeader.

4
src/test/kernel/test_kernel.cpp
View File

@@ -683,10 +683,6 @@ BOOST_AUTO_TEST_CASE(btck_block_header_tests)
BlockHeader header_1{hex_string_to_byte_vec("00c00020e7cb7b4de21d26d55bd384017b8bb9333ac3b2b55bed00000000000000000000d91b4484f801b99f03d36b9d26cfa83420b67f81da12d7e6c1e7f364e743c5ba9946e268b4dd011799c8533d")};
CheckHandle(header_0, header_1);
// Test error handling for invalid data
BOOST_CHECK_THROW(BlockHeader{hex_string_to_byte_vec("00")}, std::runtime_error);
BOOST_CHECK_THROW(BlockHeader{hex_string_to_byte_vec("")}, std::runtime_error);
// Test all header field accessors using mainnet block 1
auto mainnet_block_1_header = hex_string_to_byte_vec("010000006fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000982051fd1e4ba744bbbe680e1fee14677ba1a3c3540bf7b1cdb606e857233e0e61bc6649ffff001d01e36299");
BlockHeader header{mainnet_block_1_header};