Skip to content

Conversation

@yuvicc
Copy link
Contributor

@yuvicc yuvicc commented Nov 7, 2025

Adds a new btck_BlockHeader type and associated functions to create, access, and validate block headers. Block headers will have their own type (btck_BlockHeader) that can be created from raw data, copied, and queried for all the standard header fields (hash, prev hash, timestamp, bits, version, nonce). We can also extract headers from full blocks or block tree entries.

The first commit here refactors BlockValidationState to use Handle/View pattern so external code can own them, which is required for the header processing in the API.

New Block Header API

  • btck_BlockHeader type: Opaque handle for block headers

  • Header methods:

    • btck_block_header_create(): Create header from 80-byte serialized data

    • btck_block_header_copy(): Copy block headers

    • btck_block_header_destroy(): Destroy header object

    • btck_block_header_get_hash(): Calculate block hash

    • btck_block_header_get_prev_hash(): Get previous block hash

    • btck_block_header_get_timestamp(): Get block timestamp

    • btck_block_header_get_bits(): Get difficulty target (compact format)

    • btck_block_header_get_version(): Get block version

    • btck_block_header_get_nonce(): Get proof-of-work nonce

    • btck_block_get_header(): Extract header from a full block

    • btck_block_tree_entry_get_block_header(): Get header associated with a block tree entry

  • Header Processing Methods:

    • btck_chainstate_manager_process_block_header(): Validates and processes a block header without requiring the full block. This performs proof-of-work verification, timestamp validation, and updates the internal chain state.
    • btck_chainstate_manager_get_best_entry(): Returns the block tree entry with the most cumulative proof-of-work.

Why btck_chainstate_manager_get_best_entry() is included alongside header validation? Just as we have logic to get the tip for block validation (so you can request more blocks extending your best from your peers), we need the equivalent for header validation. To make header validation worthwhile, knowing what the best current header is seems useful—it tells you what headers to request next from peers.

Testing

Added tests in test_kernel.cpp that cover creating headers from raw data, extracting all header fields, and processing headers through the chainstate manager.

CC @TheCharlatan

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 7, 2025

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/33822.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK sedited, stringintech
Approach ACK stickies-v
Stale ACK alexanderwiederin

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #33856 (kernel, validation: Refactor ProcessNewBlockHeaders to return BlockValidationState by yuvicc)

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.

@fanquake fanquake changed the title [kernel] Add block header support and validation kernel: Add block header support and validation Nov 7, 2025
@fanquake
Copy link
Member

fanquake commented Nov 7, 2025

https://github.com/bitcoin/bitcoin/actions/runs/19175802541/job/54820162776?pr=33822#step:5:179:

<snip>
src/kernel/bitcoinkernel.h:1726: * 
src/kernel/bitcoinkernel_wrapper.h:759:    
src/kernel/bitcoinkernel_wrapper.h:762:    
^^^
Trailing whitespace (including Windows line endings [CR LF]) is problematic

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 7, 2025

🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/actions/runs/19175802541/job/54820162776
LLM reason (✨ experimental): Trailing whitespace detected by lint, causing the CI to fail.

Hints

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.

@yuvicc
Copy link
Contributor Author

yuvicc commented Nov 8, 2025

Fixed some lint errors.

@yuvicc yuvicc force-pushed the 2025-11-kernelApi_add_blockheaders branch from cbf4c34 to 86958ee Compare November 8, 2025 05:51
@DrahtBot DrahtBot removed the CI failed label Nov 8, 2025
Copy link
Contributor

@sedited sedited left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach ACK

@Aa777263100

This comment was marked as off-topic.

Copy link
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach ACK 86958ee668a950a03d08ef188f2de27137e89b33

I think the commit can be broken up a bit further, e.g. validationstate logic and chainman logic can be separate I think?

@yuvicc
Copy link
Contributor Author

yuvicc commented Nov 12, 2025

Thank you @TheCharlatan and @stickies-v for the review.

fanquake added a commit that referenced this pull request Nov 12, 2025
40dcbf5 build: add -Wtrailing-whitespace=any (fanquake)
d7659cd build: add -Wleading-whitespace=spaces (fanquake)
d866502 cmake: Disable `-Wtrailing-whitespace` warnings for RCC-generated files (Hennadii Stepanov)
aabc5ca cmake: Switch from AUTORCC to `qt6_add_resources` (Hennadii Stepanov)
25ae14c subprocess: replace tab with space (fanquake)
0c2b9da scripted-diff: remove whitespace in sha256_sse4.cpp (fanquake)
4da084f scripted-diff: change whitespace to spaces in univalue (fanquake)
e6caf15 ci: add moreutils to lint job (fanquake)

Pull request description:

  GCC 15 now has options to turn leading & trailing whitespace into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable `-Wleading-whitespace` and `-Wtrailing-whitespace`.

  We currently get PRs that are opened with various whitespace, i.e #33822, so turning that into compile-time failure where possible, seems useful, to avoid a CI roundtrip.

ACKs for top commit:
  ajtowns:
    utACK 40dcbf5
  hebasto:
    re-ACK 40dcbf5.

Tree-SHA512: a128001ab2abb41cd6d249dcf46be4167ebd608d6b0f1452212a3ec9a383747bea623ab0382ec7bc0ac7a232a47cca5174e1cd73d4eda6751aa3cb2365ad2ede
@yuvicc yuvicc force-pushed the 2025-11-kernelApi_add_blockheaders branch from bd73ce6 to 0405b37 Compare November 16, 2025 05:58
@yuvicc
Copy link
Contributor Author

yuvicc commented Nov 16, 2025

Thanks for the review @TheCharlatan

  • Removed some unnecessary comments and indentation
  • Added inline doc comment

Copy link
Contributor

@stringintech stringintech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach ACK

@yuvicc yuvicc force-pushed the 2025-11-kernelApi_add_blockheaders branch 2 times, most recently from 53eed0e to 650f046 Compare November 17, 2025 06:07
@yuvicc
Copy link
Contributor Author

yuvicc commented Nov 17, 2025

Thanks for the review @stringintech.

  • Fixed some indentations and removed some extra comments to maintain consistency.
  • Simplified BlockValidationState create method by @stringintech comment

@yuvicc yuvicc force-pushed the 2025-11-kernelApi_add_blockheaders branch from 10389bc to 30367c1 Compare November 27, 2025 18:00
@yuvicc
Copy link
Contributor Author

yuvicc commented Nov 27, 2025

  • Addressed some clang-format nits
  • Updated the comments to use struct name instead of raw names as suggested by @stickies-v.

Copy link
Contributor

@sedited sedited left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-ACK 30367c1

Copy link

@alexanderwiederin alexanderwiederin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

@alexanderwiederin
Copy link

ACK 30367c1

Add C API functions for managing BlockValidationState lifecycle:
  - btck_block_validation_state_create()
  - btck_block_validation_state_copy()
  - btck_block_validation_state_destroy()

Introduce BlockValidationStateApi<> template to share common getter methods between BlockValidationState (Handle) and BlockValidationStateView (View) classes in the C++ wrapper. This enables external code to create and own BlockValidationState objects needed for the new process_block_header() API.

Co-authored-by: TheCharlatan <[email protected]>
@yuvicc yuvicc force-pushed the 2025-11-kernelApi_add_blockheaders branch 2 times, most recently from b023f3c to 4a261e9 Compare December 29, 2025 09:08
@DrahtBot
Copy link
Contributor

🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/actions/runs/20568596692/job/59071182413
LLM reason (✨ experimental): Lint failure: tabs used for whitespace (tabs_whitespace) in src/kernel/bitcoinkernel.cpp, causing CI to fail.

Hints

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.

@yuvicc
Copy link
Contributor Author

yuvicc commented Dec 29, 2025

Thanks for the review @stringintech

  • Addressed nits related to opaque struct naming
  • Addressed comment on allowing null args and adding a check as well as test case for it
  • Removed const-ness from BlockChecked methods for BlockValidationStateView comment
  • Rebased

Copy link
Contributor

@stringintech stringintech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 4a261e9

Copy link
Contributor

@sedited sedited left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-ACK 4a261e9

Introduces btck_BlockHeader type with accessor methods and btck_chainstate_manager_process_block_header() for validating headers without full blocks. Also, adds btck_chainstate_manager_get_best_entry() to query the header with most cumulative proof-of-work.

Co-authored-by: TheCharlatan <[email protected]>
@yuvicc yuvicc force-pushed the 2025-11-kernelApi_add_blockheaders branch from 4a261e9 to 9356833 Compare January 3, 2026 05:56
@yuvicc
Copy link
Contributor Author

yuvicc commented Jan 3, 2026

Fixed nit #33822 (comment).

Copy link
Contributor

@sedited sedited left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-ACK 9356833

@DrahtBot DrahtBot requested a review from stringintech January 3, 2026 07:54
Copy link
Contributor

@stringintech stringintech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-ACK 9356833

@DrahtBot DrahtBot removed the CI failed label Jan 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: API Development

Development

Successfully merging this pull request may close these issues.

9 participants