-
Notifications
You must be signed in to change notification settings - Fork 38.7k
kernel: Add block header support and validation #33822
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33822. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste ConflictsReviewers, this pull request conflicts with the following ones:
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. |
<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 |
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
641edbd to
cbf4c34
Compare
|
Fixed some lint errors. |
cbf4c34 to
86958ee
Compare
sedited
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approach ACK
This comment was marked as off-topic.
This comment was marked as off-topic.
stickies-v
left a comment
There was a problem hiding this 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?
86958ee to
bd73ce6
Compare
|
Thank you @TheCharlatan and @stickies-v for the review.
|
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
bd73ce6 to
0405b37
Compare
|
Thanks for the review @TheCharlatan
|
stringintech
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approach ACK
53eed0e to
650f046
Compare
|
Thanks for the review @stringintech.
|
10389bc to
30367c1
Compare
|
sedited
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK 30367c1
alexanderwiederin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK
|
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]>
b023f3c to
4a261e9
Compare
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
|
Thanks for the review @stringintech |
stringintech
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 4a261e9
sedited
left a comment
There was a problem hiding this 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]>
4a261e9 to
9356833
Compare
|
Fixed nit #33822 (comment). |
sedited
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-ACK 9356833
stringintech
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK 9356833
Adds a new
btck_BlockHeadertype 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
BlockValidationStateto 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_BlockHeadertype: Opaque handle for block headersHeader methods:
btck_block_header_create(): Create header from 80-byte serialized databtck_block_header_copy(): Copy block headersbtck_block_header_destroy(): Destroy header objectbtck_block_header_get_hash(): Calculate block hashbtck_block_header_get_prev_hash(): Get previous block hashbtck_block_header_get_timestamp(): Get block timestampbtck_block_header_get_bits(): Get difficulty target (compact format)btck_block_header_get_version(): Get block versionbtck_block_header_get_nonce(): Get proof-of-work noncebtck_block_get_header(): Extract header from a full blockbtck_block_tree_entry_get_block_header(): Get header associated with a block tree entryHeader 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.cppthat cover creating headers from raw data, extracting all header fields, and processing headers through the chainstate manager.CC @TheCharlatan