This adds serialization for btck_BlockHeader API. Also, updated the CheckHandle to compare the byte content instead of size.
The changes here is done in two commits. First commit adds the SpanWriter class and next one moves the block header serialization to SpanWriter. See commit message for more details.
Follow-up to #33822 .
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34401.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | stickies-v, alexanderwiederin, musaHaruna |
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
No conflicts as of last run.
Possible typos and grammar issues:
2026-03-09 13:37:47
1767@@ -1768,6 +1768,20 @@ BITCOINKERNEL_API int32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_header_get
1768 BITCOINKERNEL_API uint32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_header_get_nonce(
1769 const btck_BlockHeader* header) BITCOINKERNEL_ARG_NONNULL(1);
1770
1771+/**
1772+ * @brief Serializes the btck_BlockHeader through the passed in callback to bytes.
For consistency with other serialization function documentation, would be good to add
This is consensus serialization that is also used for the P2P network.
1775+ * @param[in] writer Non-null, callback to a write bytes function.
1776+ * @param[in] user_data Holds a user-defined opaque structure that will be
1777+ * passed back through the writer callback.
1778+ * @return 0 on success.
1779+ */
1780+BITCOINKERNEL_API int btck_block_header_to_bytes(
Headers are fixed 80 byte structures, so I think we can just return a fixed-length buffer instead of using a writer - see e.g. btck_block_hash_to_bytes.
0diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
1index 525d861827..2fd5a51873 100644
2--- a/src/kernel/bitcoinkernel.cpp
3+++ b/src/kernel/bitcoinkernel.cpp
4@@ -1389,15 +1389,11 @@ uint32_t btck_block_header_get_nonce(const btck_BlockHeader* header)
5 return btck_BlockHeader::get(header).nNonce;
6 }
7
8-int btck_block_header_to_bytes(const btck_BlockHeader* header, btck_WriteBytes writer, void* user_data)
9+void btck_block_header_to_bytes(const btck_BlockHeader* header, unsigned char output[80])
10 {
11- try {
12- WriterStream ws{writer, user_data};
13- ws << btck_BlockHeader::get(header);
14- return 0;
15- } catch (...) {
16- return -1;
17- }
18+ DataStream stream{};
19+ stream << btck_BlockHeader::get(header);
20+ std::memcpy(output, stream.data(), 80);
21 }
22
23 void btck_block_header_destroy(btck_BlockHeader* header)
24diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h
25index 53491ea69e..4f6ae41c61 100644
26--- a/src/kernel/bitcoinkernel.h
27+++ b/src/kernel/bitcoinkernel.h
28@@ -1769,18 +1769,13 @@ BITCOINKERNEL_API uint32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_header_ge
29 const btck_BlockHeader* header) BITCOINKERNEL_ARG_NONNULL(1);
30
31 /**
32- * [@brief](/bitcoin-bitcoin/contributor/brief/) Serializes the btck_BlockHeader through the passed in callback to bytes.
33+ * [@brief](/bitcoin-bitcoin/contributor/brief/) Serializes the btck_BlockHeader to bytes.
34 *
35 * [@param](/bitcoin-bitcoin/contributor/param/)[in] header Non-null.
36- * [@param](/bitcoin-bitcoin/contributor/param/)[in] writer Non-null, callback to a write bytes function.
37- * [@param](/bitcoin-bitcoin/contributor/param/)[in] user_data Holds a user-defined opaque structure that will be
38- * passed back through the writer callback.
39- * [@return](/bitcoin-bitcoin/contributor/return/) 0 on success.
40+ * [@param](/bitcoin-bitcoin/contributor/param/)[out] output The serialized block header (80 bytes).
41 */
42-BITCOINKERNEL_API int btck_block_header_to_bytes(
43- const btck_BlockHeader* header,
44- btck_WriteBytes writer,
45- void* user_data) BITCOINKERNEL_ARG_NONNULL(1, 2);
46+BITCOINKERNEL_API void btck_block_header_to_bytes(
47+ const btck_BlockHeader* header, unsigned char output[80]) BITCOINKERNEL_ARG_NONNULL(1, 2);
48
49 /**
50 * Destroy the btck_BlockHeader.
51diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h
52index c23ad8ba3b..95afd90480 100644
53--- a/src/kernel/bitcoinkernel_wrapper.h
54+++ b/src/kernel/bitcoinkernel_wrapper.h
55@@ -747,9 +747,11 @@ public:
56 return btck_block_header_get_nonce(impl());
57 }
58
59- std::vector<std::byte> ToBytes() const
60+ std::array<std::byte, 80> ToBytes() const
61 {
62- return write_bytes(impl(), btck_block_header_to_bytes);
63+ std::array<std::byte, 80> header;
64+ btck_block_header_to_bytes(impl(), reinterpret_cast<unsigned char*>(header.data()));
65+ return header;
66 }
67 };
68
69diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
70index 86c2e25057..79c88c94c7 100644
71--- a/src/test/kernel/test_kernel.cpp
72+++ b/src/test/kernel/test_kernel.cpp
73@@ -266,7 +266,7 @@ void run_verify_test(
74
75 template <typename T>
76 concept HasToBytes = requires(T t) {
77- { t.ToBytes() } -> std::convertible_to<std::vector<std::byte>>;
78+ { t.ToBytes() } -> std::convertible_to<std::span<const std::byte>>;
79 };
80
81 template <typename T>
🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/actions/runs/21541544208/job/62076715283
LLM reason (✨ experimental): Linting failed due to include-usage warnings (lint-includes.py) causing the all_python_linters check to fail.
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.
Thanks for review @stickies-v.
1390@@ -1391,6 +1391,13 @@ uint32_t btck_block_header_get_nonce(const btck_BlockHeader* header)
1391 return btck_BlockHeader::get(header).nNonce;
1392 }
1393
1394+void btck_block_header_to_bytes(const btck_BlockHeader* header, unsigned char output[80])
1395+{
1396+ DataStream stream{};
1397+ stream << btck_BlockHeader::get(header);
1398+ std::memcpy(output, stream.data(), 80);
1399+}
Sorry, my previous suggestion wasn’t entirely sound. This function can throw an exception that crosses the C boundary (e.g. if memory allocation fails), so we should better handle this.
Some options are:
WriterStream approach, which adds some unnecessary overheadSpanWriter to avoid allocating memory. I’m not sure about the guarantees we have that serializing the block header will never throw, though, so this might be in addition to Option 1 (but still has the benefit of avoiding allocation)Possible SpanWriter approach:
0diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
1index 2b129f2952..cdac10a05d 100644
2--- a/src/kernel/bitcoinkernel.cpp
3+++ b/src/kernel/bitcoinkernel.cpp
4@@ -1393,9 +1393,7 @@ uint32_t btck_block_header_get_nonce(const btck_BlockHeader* header)
5
6 void btck_block_header_to_bytes(const btck_BlockHeader* header, unsigned char output[80])
7 {
8- DataStream stream{};
9- stream << btck_BlockHeader::get(header);
10- std::memcpy(output, stream.data(), 80);
11+ SpanWriter{std::span{output, 80}} << btck_BlockHeader::get(header);
12 }
13
14 void btck_block_header_destroy(btck_BlockHeader* header)
15diff --git a/src/streams.h b/src/streams.h
16index 466084e9fa..3358fa994a 100644
17--- a/src/streams.h
18+++ b/src/streams.h
19@@ -77,6 +77,38 @@ private:
20 size_t nPos;
21 };
22
23+/** Minimal stream for writing to an existing span of bytes.
24+ */
25+class SpanWriter
26+{
27+private:
28+ std::span<std::byte> m_data;
29+ size_t m_pos{0};
30+
31+public:
32+ explicit SpanWriter(std::span<unsigned char> data) : m_data{std::as_writable_bytes(data)} {}
33+ explicit SpanWriter(std::span<std::byte> data) : m_data{data} {}
34+ template <typename... Args>
35+ SpanWriter(std::span<unsigned char> data, Args&&... args) : SpanWriter{data}
36+ {
37+ ::SerializeMany(*this, std::forward<Args>(args)...);
38+ }
39+
40+ void write(std::span<const std::byte> src)
41+ {
42+ assert(m_pos + src.size() <= m_data.size());
43+ memcpy(m_data.data() + m_pos, src.data(), src.size());
44+ m_pos += src.size();
45+ }
46+
47+ template <typename T>
48+ SpanWriter& operator<<(const T& obj)
49+ {
50+ ::Serialize(*this, obj);
51+ return *this;
52+ }
53+};
54+
55 /** Minimal stream for reading from an existing byte array by std::span.
56 */
57 class SpanReader
58diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp
59index af75ee987a..29fce0612d 100644
60--- a/src/test/streams_tests.cpp
61+++ b/src/test/streams_tests.cpp
62@@ -207,6 +207,31 @@ BOOST_AUTO_TEST_CASE(streams_vector_writer)
63 vch.clear();
64 }
65
66+BOOST_AUTO_TEST_CASE(streams_span_writer)
67+{
68+ unsigned char a(1);
69+ unsigned char b(2);
70+ unsigned char bytes[] = {3, 4, 5, 6};
71+ std::array<unsigned char, 8> arr{};
72+
73+ // Test operator<<.
74+ SpanWriter writer{arr};
75+ writer << a << b;
76+ std::array<unsigned char, 8> expected1{{1, 2, 0, 0, 0, 0, 0, 0}};
77+ BOOST_CHECK_EQUAL_COLLECTIONS(arr.begin(), arr.end(), expected1.begin(), expected1.end());
78+
79+ // Use variadic constructor and write to subspan.
80+ SpanWriter{std::span{arr}.subspan(2), a, bytes, b};
81+ std::array<unsigned char, 8> expected2{{1, 2, 1, 3, 4, 5, 6, 2}};
82+ BOOST_CHECK_EQUAL_COLLECTIONS(arr.begin(), arr.end(), expected2.begin(), expected2.end());
83+
84+ // Test std::byte span constructor.
85+ std::array<std::byte, 2> byte_arr{};
86+ SpanWriter{std::span{byte_arr}} << a << b;
87+ std::array<std::byte, 2> expected3{{std::byte{1}, std::byte{2}}};
88+ BOOST_CHECK_EQUAL_COLLECTIONS(byte_arr.begin(), byte_arr.end(), expected3.begin(), expected3.end());
89+}
90+
91 BOOST_AUTO_TEST_CASE(streams_vector_reader)
92 {
93 std::vector<unsigned char> vch = {1, 255, 3, 4, 5, 6};
SpanWriter approach as suggested by @stickies-v and also split into two commits. First commit adds the SpanWriter class and next one moves the block header serialization to SpanWriter. See commit message for more details. Also updated the PR description.
Concept ACK.
I recommend we wrap the span serialisation in a try/catch, as you suggested earlier, and return an int like the other to_bytes methods.
The commit messages can be more descriptive and I would also split the second commit into two: one for the method and the other for the HasToBytes change.
Concept ACK
Still familiarizing myself with the Kernel API, but still looked at the code through a newbie lens. In 77c36df, the SpanWriter class provides a minimal serialization stream that writes bytes into a pre-allocated memory buffer represented by a std::span. It maintains an internal write position (m_pos) and appends serialized data sequentially into the span.
One design detail I noticed is the bounds check inside write():
assert(m_pos + src.size() <= m_data.size());
Based on my understanding, the use of assert here instead of throwing an exception means that this condition is a programming invariant rather than a recoverable runtime error. The class assumes that callers are responsible for providing a buffer that is large enough for the serialized data.
So in case of an incorrect input, the caller will be the one to handle the error at runtime? IIUC, this is what this comment below is suggesting if such case of incorrect inputs occurs.
I recommend we wrap the span serialisation in a try/catch, as you suggested earlier, and return an
intlike the otherto_bytesmethods.
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Also fixes `CheckHandle` to compare exact byte content instead of size comparisons.
🚧 At least one of the CI tasks failed.
Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/22855674050/job/66295237030
LLM reason (✨ experimental): CI failure due to a build error: the cmake/gmake compilation of src/kernel/bitcoinkernel.cpp failed with a non-zero exit status.
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.
1767@@ -1768,6 +1768,16 @@ BITCOINKERNEL_API int32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_header_get
1768 BITCOINKERNEL_API uint32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_header_get_nonce(
1769 const btck_BlockHeader* header) BITCOINKERNEL_ARG_NONNULL(1);
1770
1771+/**
1772+ * @brief Serializes the btck_BlockHeader through the passed in callback to bytes.
I would suggest @brief Serializes the block header to bytes. - or similar.
there is no callback.
1767@@ -1768,6 +1768,16 @@ BITCOINKERNEL_API int32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_header_get
1768 BITCOINKERNEL_API uint32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_header_get_nonce(
1769 const btck_BlockHeader* header) BITCOINKERNEL_ARG_NONNULL(1);
1770
1771+/**
1772+ * @brief Serializes the btck_BlockHeader through the passed in callback to bytes.
1773+ * This is consensus serialization that is also used for the P2P network.
1774+ *
1775+ * @param[in] header Non-null.
1776+ * @param[out] output The serialized block header (80 bytes).
To align with the others:
0 * [@param](/bitcoin-bitcoin/contributor/param/)[out] output Non-null, 80-byte buffer to write the serialized header into.
1 * [@return](/bitcoin-bitcoin/contributor/return/) 0 on success.
Thanks.
I still think we can improve the commit messages. I use this here for guidance: https://cbea.ms/git-commit/