📝 yuvicc opened a pull request: "[kernel] Add block header support and validation"
(https://github.com/bitcoin/bitcoin/pull/33822)
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.
#### New Block Header API
- **`btck_BlockHeader` type**: Opaque handle for block headers
- **Header methods**:
...
(https://github.com/bitcoin/bitcoin/pull/33822)
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.
#### New Block Header API
- **`btck_BlockHeader` type**: Opaque handle for block headers
- **Header methods**:
...
💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2504684591)
The old methods are redundant, but they're not conflicting. Clients can use either and both have test coverage.
From this projects perspective there's hardly any additional review burden when we prune deprecated methods in a later PR, that should be trivial.
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2504684591)
The old methods are redundant, but they're not conflicting. Clients can use either and both have test coverage.
From this projects perspective there's hardly any additional review burden when we prune deprecated methods in a later PR, that should be trivial.
💬 maflcko commented on pull request "cmake: Create subdirectories in build tree in advance":
(https://github.com/bitcoin/bitcoin/pull/32773#issuecomment-3503780491)
I know, but `-B` does not help to fix the other problems: (1) They are not run via ctest, and (2) they are stand-alone scripts, where the user/dev would have to specify it every time, or set the env var.
(https://github.com/bitcoin/bitcoin/pull/32773#issuecomment-3503780491)
I know, but `-B` does not help to fix the other problems: (1) They are not run via ctest, and (2) they are stand-alone scripts, where the user/dev would have to specify it every time, or set the env var.
💬 fanquake commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#issuecomment-3503803553)
https://github.com/bitcoin/bitcoin/actions/runs/19175802541/job/54820162776?pr=33822#step:5:179:
```bash
<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
```
(https://github.com/bitcoin/bitcoin/pull/33822#issuecomment-3503803553)
https://github.com/bitcoin/bitcoin/actions/runs/19175802541/job/54820162776?pr=33822#step:5:179:
```bash
<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
```
💬 hebasto commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#discussion_r2504727853)
The `CMAKE_<TYPE>_LINKER_FLAGS` variables are quite invasive. They override any initial values set either in a toolchain file via `CMAKE_<TYPE>_LINKER_FLAGS_INIT` or in the `LDFLAGS` environment variable.
Fortunately, there are no linker flags set for Linux hosts in depends.
Still, I suggest documenting this behaviour for our future selves.
(https://github.com/bitcoin/bitcoin/pull/33181#discussion_r2504727853)
The `CMAKE_<TYPE>_LINKER_FLAGS` variables are quite invasive. They override any initial values set either in a toolchain file via `CMAKE_<TYPE>_LINKER_FLAGS_INIT` or in the `LDFLAGS` environment variable.
Fortunately, there are no linker flags set for Linux hosts in depends.
Still, I suggest documenting this behaviour for our future selves.
💬 fanquake commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#discussion_r2504749571)
Ok, given the behaviour here, we can add some documentation if we end up taking this approach in future.
(https://github.com/bitcoin/bitcoin/pull/33181#discussion_r2504749571)
Ok, given the behaviour here, we can add some documentation if we end up taking this approach in future.
💬 maflcko commented on pull request "ci: Add IWYU job":
(https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2504767099)
Maybe:
```sh
echo "^^^ ⚠️ Failure generated from IWYU"
echo
echo "Some adjustments to the diff may be needed:"
echo
echo "* Use the C++ headers: E.g. <cerrno> over <errno.h>"
echo '* Use #include <__.h> over #include "__.h" for all headers'
echo "* Sort the headers, so that they are in the right pre-existing section, if"
echo " there are any."
(https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2504767099)
Maybe:
```sh
echo "^^^ ⚠️ Failure generated from IWYU"
echo
echo "Some adjustments to the diff may be needed:"
echo
echo "* Use the C++ headers: E.g. <cerrno> over <errno.h>"
echo '* Use #include <__.h> over #include "__.h" for all headers'
echo "* Sort the headers, so that they are in the right pre-existing section, if"
echo " there are any."
🤔 hebasto reviewed a pull request: "guix: build for Linux HOSTS with `-static-libgcc`"
(https://github.com/bitcoin/bitcoin/pull/33181#pullrequestreview-3435466193)
I have reviewed the code and it looks OK.
Still waiting for my Guix build on RISC-V machine.
(https://github.com/bitcoin/bitcoin/pull/33181#pullrequestreview-3435466193)
I have reviewed the code and it looks OK.
Still waiting for my Guix build on RISC-V machine.
💬 hebasto commented on pull request "ci: Add IWYU job":
(https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2504805636)
Thanks! Added.
(https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2504805636)
Thanks! Added.
💬 marcofleon commented on issue "RFC: Add multiprocess fuzz target":
(https://github.com/bitcoin/bitcoin/issues/23015#issuecomment-3503990755)
> dreading the crashes
While we are looking forward to them :)
> Are there changes that would make it more maintainable?
@dergoegge could speak more to this, but an initial step would probably be to enable fuzzamoto to more easily build the multiprocess binaries. Not having to manually change Dockerfiles and `Cargo.toml`. And then also, we could have the various clients separated out from the scenarios themselves (or one general IPC client that wraps the others maybe).
Then it would just be
...
(https://github.com/bitcoin/bitcoin/issues/23015#issuecomment-3503990755)
> dreading the crashes
While we are looking forward to them :)
> Are there changes that would make it more maintainable?
@dergoegge could speak more to this, but an initial step would probably be to enable fuzzamoto to more easily build the multiprocess binaries. Not having to manually change Dockerfiles and `Cargo.toml`. And then also, we could have the various clients separated out from the scenarios themselves (or one general IPC client that wraps the others maybe).
Then it would just be
...
📝 portlandhodl converted_to_draft a pull request: "Enhanced error messages for invalid network prefix during address parsing."
(https://github.com/bitcoin/bitcoin/pull/27260)
### **Issue:**
Simply `DecodeDestination` does not handle errors where the user inputs a valid address for the wrong network. _e.x. testnet while running client on mainnet_
The current error `not a valid Bech32 or Base58 encoding` for a valid address on a different network is entirely incorrect. This is because of the is_bech32 variable used at the core of `DecodeDestination` logic only checks that the prefix matches. If it doesn't it just starts running everything as `DecodeBase58Check
...
(https://github.com/bitcoin/bitcoin/pull/27260)
### **Issue:**
Simply `DecodeDestination` does not handle errors where the user inputs a valid address for the wrong network. _e.x. testnet while running client on mainnet_
The current error `not a valid Bech32 or Base58 encoding` for a valid address on a different network is entirely incorrect. This is because of the is_bech32 variable used at the core of `DecodeDestination` logic only checks that the prefix matches. If it doesn't it just starts running everything as `DecodeBase58Check
...
💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2504862983)
To sum up what needs to happen if we delete instead of deprecate these old methods:
1. `sv2-tp` needs a new release that includes https://github.com/stratum-mining/sv2-tp/pull/59 (no problem, will happen anyway)
2. the SRI TP (draft) https://github.com/stratum-mining/sv2-apps/pull/59 needs to be updated to support both the old and new method (like `sv2-tp` does), sometime before v31 comes out (support for the old method can be dropped after v31 comes out, at the cost of dropping support for
...
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2504862983)
To sum up what needs to happen if we delete instead of deprecate these old methods:
1. `sv2-tp` needs a new release that includes https://github.com/stratum-mining/sv2-tp/pull/59 (no problem, will happen anyway)
2. the SRI TP (draft) https://github.com/stratum-mining/sv2-apps/pull/59 needs to be updated to support both the old and new method (like `sv2-tp` does), sometime before v31 comes out (support for the old method can be dropped after v31 comes out, at the cost of dropping support for
...
💬 ismaelsadeeq commented on issue "Header-only support for waitNext()":
(https://github.com/bitcoin/bitcoin/issues/33756#issuecomment-3504051962)
Thanks for the context.
> This provides miners with additional revenue because they are less likely to mine a stale block [0]
Before doing this, I think we should benchmark in practice the usual latency between knowing about a valid new block header, downloading, validating, and switching the chain tip.
As you mentioned, for a node with a good connection and reasonable policy rules, compact block reconstruction should prevent any measurable latency. Also, we are likely going to have `SENDTEMPL
...
(https://github.com/bitcoin/bitcoin/issues/33756#issuecomment-3504051962)
Thanks for the context.
> This provides miners with additional revenue because they are less likely to mine a stale block [0]
Before doing this, I think we should benchmark in practice the usual latency between knowing about a valid new block header, downloading, validating, and switching the chain tip.
As you mentioned, for a node with a good connection and reasonable policy rules, compact block reconstruction should prevent any measurable latency. Also, we are likely going to have `SENDTEMPL
...
👍 maflcko approved a pull request: "ci: Add IWYU job"
(https://github.com/bitcoin/bitcoin/pull/33810#pullrequestreview-3435561658)
lgtm
(https://github.com/bitcoin/bitcoin/pull/33810#pullrequestreview-3435561658)
lgtm
💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3504056653)
@plebhash can you try to see if it's easy for you to support both `getCoinbase()` and `getCoinbaseTx()` in https://github.com/stratum-mining/sv2-apps/pull/59, preferring the first and falling back to the latter if it doesn't exist?
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3504056653)
@plebhash can you try to see if it's easy for you to support both `getCoinbase()` and `getCoinbaseTx()` in https://github.com/stratum-mining/sv2-apps/pull/59, preferring the first and falling back to the latter if it doesn't exist?
💬 Sjors commented on issue "Header-only support for waitNext()":
(https://github.com/bitcoin/bitcoin/issues/33756#issuecomment-3504084601)
> we should benchmark in practice the usual latency between knowing about a valid new block header, downloading, validating, and switching the chain tip
Agreed, ideally I'd like to see data from actual miners.
> compact block reconstruction should prevent any measurable latency
Yes, but with the recent trend of people using incompatible policies, we should prepare for a scenario where compact block relay (often) doesn't work.
I don't know if `SENDTEMPLATE` can fully compensate for broken com
...
(https://github.com/bitcoin/bitcoin/issues/33756#issuecomment-3504084601)
> we should benchmark in practice the usual latency between knowing about a valid new block header, downloading, validating, and switching the chain tip
Agreed, ideally I'd like to see data from actual miners.
> compact block reconstruction should prevent any measurable latency
Yes, but with the recent trend of people using incompatible policies, we should prepare for a scenario where compact block relay (often) doesn't work.
I don't know if `SENDTEMPLATE` can fully compensate for broken com
...
🤔 marcofleon reviewed a pull request: "doc: reference fuzz coverage steps in quick-start"
(https://github.com/bitcoin/bitcoin/pull/33536#pullrequestreview-3435606541)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33536#pullrequestreview-3435606541)
Concept ACK
💬 marcofleon commented on pull request "doc: reference fuzz coverage steps in quick-start":
(https://github.com/bitcoin/bitcoin/pull/33536#discussion_r2504902827)
nit: Not sure if "coverage reports" needs the backticks here?
(https://github.com/bitcoin/bitcoin/pull/33536#discussion_r2504902827)
nit: Not sure if "coverage reports" needs the backticks here?
💬 l0rinc commented on issue "malloc: Failed to allocate segment from range group - out of space":
(https://github.com/bitcoin/bitcoin/issues/33806#issuecomment-3504302105)
### 29
Seems v29 already had this problem:
<details>
<summary>-dbcache=45000 OOM @ height=500515, cache=8553.2MiB</summary>
```
2025-11-07T16:03:59Z UpdateTip: new best=0000000000000000008abb306f6c51b4dfd3aed47a3e46099a92dc164d4d6059 height=500515 version=0x20000000 log2_work=87.707859 tx=284945541 date='2017-12-22T09:02:59Z' progress=0.225917 cache=8553.2MiB(63023361txo)
Process 74025 launched: '/Users/lorinc/IdeaProjects/bitcoin/build/bin/bitcoind' (arm64)
Process 74025 stopped
* thread #35
...
(https://github.com/bitcoin/bitcoin/issues/33806#issuecomment-3504302105)
### 29
Seems v29 already had this problem:
<details>
<summary>-dbcache=45000 OOM @ height=500515, cache=8553.2MiB</summary>
```
2025-11-07T16:03:59Z UpdateTip: new best=0000000000000000008abb306f6c51b4dfd3aed47a3e46099a92dc164d4d6059 height=500515 version=0x20000000 log2_work=87.707859 tx=284945541 date='2017-12-22T09:02:59Z' progress=0.225917 cache=8553.2MiB(63023361txo)
Process 74025 launched: '/Users/lorinc/IdeaProjects/bitcoin/build/bin/bitcoind' (arm64)
Process 74025 stopped
* thread #35
...
💬 l0rinc commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3504370470)
reACK 854269a785fc51602cc5ae018c666fc1d2659776
`git range-diff b271b34...854269a` indicates it's a simple rebase.
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3504370470)
reACK 854269a785fc51602cc5ae018c666fc1d2659776
`git range-diff b271b34...854269a` indicates it's a simple rebase.