👋 ryanofsky's pull request is ready for review: "Update libmultiprocess subtree in 30.x branch"
(https://github.com/bitcoin/bitcoin/pull/33519)
(https://github.com/bitcoin/bitcoin/pull/33519)
💬 stringintech commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2410849029)
When fuzzing on macOS (Apple Silicon), I ran into lots of crashes when copying the cached dir into the destination:
```
libc++abi: terminating due to uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in copy_file: File exists ["/var/folders/wc/3l6l_0b92zzg54v6rmvqqn300000gn/T/test_common bitcoin/cmpctblock/6c5e38c6dcfbc2607321/regtest/blocks/xor.dat"] ["/var/folders/wc/3l6l_0b92zzg54v6rmvqqn300000gn/T/cmpctblock_cachedca12974312651650d6b0/datadir/regt
...
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2410849029)
When fuzzing on macOS (Apple Silicon), I ran into lots of crashes when copying the cached dir into the destination:
```
libc++abi: terminating due to uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in copy_file: File exists ["/var/folders/wc/3l6l_0b92zzg54v6rmvqqn300000gn/T/test_common bitcoin/cmpctblock/6c5e38c6dcfbc2607321/regtest/blocks/xor.dat"] ["/var/folders/wc/3l6l_0b92zzg54v6rmvqqn300000gn/T/cmpctblock_cachedca12974312651650d6b0/datadir/regt
...
📝 vasild opened a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565)
Rename `PeerManager::RelayTransaction()` to
`PeerManager::ScheduleTxForBroadcastToAll()`. The transaction is not relayed when the method returns. It is only scheduled for broadcasting at a later time. Also, there will be another method which only schedules for broadcast to Tor or I2P peers.
---
This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull/29415). Putting it in its own PR to reduce the size of #29415
...
(https://github.com/bitcoin/bitcoin/pull/33565)
Rename `PeerManager::RelayTransaction()` to
`PeerManager::ScheduleTxForBroadcastToAll()`. The transaction is not relayed when the method returns. It is only scheduled for broadcasting at a later time. Also, there will be another method which only schedules for broadcast to Tor or I2P peers.
---
This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull/29415). Putting it in its own PR to reduce the size of #29415
...
💬 mzumsande commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2410894036)
done
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2410894036)
done
💬 m3dwards commented on pull request "Clear out space on GHA jobs":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3377259935)
I wondered if it could be sped up and it looks like deleting `/usr/local/lib/android` is what takes the time. It's 9.8gb so probably not worth skipping the deletion. I tried a few tricks like rsyncing an empty directory over it but it looks like `rm -rf` is king in terms of speed.
So I concur, perhaps best to just apply to the job that needs it.
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3377259935)
I wondered if it could be sped up and it looks like deleting `/usr/local/lib/android` is what takes the time. It's 9.8gb so probably not worth skipping the deletion. I tried a few tricks like rsyncing an empty directory over it but it looks like `rm -rf` is king in terms of speed.
So I concur, perhaps best to just apply to the job that needs it.
🤔 janb84 reviewed a pull request: "validation: Improve warnings in case of chain corruption"
(https://github.com/bitcoin/bitcoin/pull/33553#pullrequestreview-3310548122)
Concept ACK
This PR introduces a informative warning for the user in the case that the users has an invalid block (header) due to corruption or otherwise.
NIT: why keep looping ? is there a path where the node can recover from this ?
partial code review ✅
<details><summary> this PR </summary>
Gives warning in case of chain corruption
```sh
2025-10-07T14:24:52Z Pre-synchronizing blockheaders, height: 266146 (~97.53%)
2025-10-07T14:24:55Z [warning] Received header for a bl
...
(https://github.com/bitcoin/bitcoin/pull/33553#pullrequestreview-3310548122)
Concept ACK
This PR introduces a informative warning for the user in the case that the users has an invalid block (header) due to corruption or otherwise.
NIT: why keep looping ? is there a path where the node can recover from this ?
partial code review ✅
<details><summary> this PR </summary>
Gives warning in case of chain corruption
```sh
2025-10-07T14:24:52Z Pre-synchronizing blockheaders, height: 266146 (~97.53%)
2025-10-07T14:24:55Z [warning] Received header for a bl
...
📝 Sjors opened a pull request: "miner: empty mempool special case for waitNext()"
(https://github.com/bitcoin/bitcoin/pull/33566)
Block template fees are calculated by looping over new_tmpl->vTxFees and return (early) once the fee_threshold is exceeded.
This left an edge case when the mempool is empty, which this commit fixes and adds a test for.
Fixes https://github.com/Sjors/sv2-tp/issues/9
(https://github.com/bitcoin/bitcoin/pull/33566)
Block template fees are calculated by looping over new_tmpl->vTxFees and return (early) once the fee_threshold is exceeded.
This left an edge case when the mempool is empty, which this commit fixes and adds a test for.
Fixes https://github.com/Sjors/sv2-tp/issues/9
💬 Sjors commented on pull request "miner: empty mempool special case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3377304798)
This is not worth the effort to backport imo.
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3377304798)
This is not worth the effort to backport imo.
💬 theuni commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3377330552)
Rebased on #33518 temporarily to make sure ci is happy with everything combined.
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3377330552)
Rebased on #33518 temporarily to make sure ci is happy with everything combined.
💬 theuni commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2410969953)
I only did it this way because it's more readable to me that way, and because I figured there may be more logging options for us to use in the future that would make the in-place init more complicated.
Happy to change it if you feel strongly.
(https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2410969953)
I only did it this way because it's more readable to me that way, and because I figured there may be more logging options for us to use in the future that would make the in-place init more complicated.
Happy to change it if you feel strongly.
💬 theuni commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2410980251)
I'm torn about what to do here, so feedback welcome. Adding another value upstream at any point would cause a crash here. But because we don't know what that future value would be, we can't predict how we should react to it now.
Just commenting because this effectively freezes `mp::Log`, and would require adding new fields to `LogMessage` instead. I'm ok with that, but @ryanofsky might not be.
(https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2410980251)
I'm torn about what to do here, so feedback welcome. Adding another value upstream at any point would cause a crash here. But because we don't know what that future value would be, we can't predict how we should react to it now.
Just commenting because this effectively freezes `mp::Log`, and would require adding new fields to `LogMessage` instead. I'm ok with that, but @ryanofsky might not be.
💬 Sjors commented on pull request "Update libmultiprocess subtree to support reduced logging":
(https://github.com/bitcoin/bitcoin/pull/33518#issuecomment-3377368922)
utACK eda91b07fd9f2a6af3c31659d51f51aacf8989c4
Just checked the subtree update validity. Will do some testing with #33517 and https://github.com/Sjors/sv2-tp
(https://github.com/bitcoin/bitcoin/pull/33518#issuecomment-3377368922)
utACK eda91b07fd9f2a6af3c31659d51f51aacf8989c4
Just checked the subtree update validity. Will do some testing with #33517 and https://github.com/Sjors/sv2-tp
📝 vasild opened a pull request: "node: change a tx-relay on/off flag to enum"
(https://github.com/bitcoin/bitcoin/pull/33567)
Previously the `bool relay` argument to `BroadcastTransaction()` designated:
```
relay=true: add to the mempool and broadcast to all peers
relay=false: add to the mempool
```
Change this to an `enum`, so it is more readable and easier to extend with a 3rd option. Consider these example call sites:
```cpp
Paint(true);
// Or
Paint(/*is_red=*/true);
```
vs
```cpp
Paint(RED);
```
The idea for putting `TxBroadcastMethod` into `node/types.h` by Ryan.
---
This is part o
...
(https://github.com/bitcoin/bitcoin/pull/33567)
Previously the `bool relay` argument to `BroadcastTransaction()` designated:
```
relay=true: add to the mempool and broadcast to all peers
relay=false: add to the mempool
```
Change this to an `enum`, so it is more readable and easier to extend with a 3rd option. Consider these example call sites:
```cpp
Paint(true);
// Or
Paint(/*is_red=*/true);
```
vs
```cpp
Paint(RED);
```
The idea for putting `TxBroadcastMethod` into `node/types.h` by Ryan.
---
This is part o
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3377395591)
`7d837fef28...a9d697c0e1`: minor changes to facilitate chopping off two pieces of this into smaller independent PRs:
https://github.com/bitcoin/bitcoin/pull/33565
https://github.com/bitcoin/bitcoin/pull/33567
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3377395591)
`7d837fef28...a9d697c0e1`: minor changes to facilitate chopping off two pieces of this into smaller independent PRs:
https://github.com/bitcoin/bitcoin/pull/33565
https://github.com/bitcoin/bitcoin/pull/33567
📝 Sjors opened a pull request: "doc: how to update a subtree"
(https://github.com/bitcoin/bitcoin/pull/33568)
We have instructions on how to verify a subtree update, but not on how to perform one.
(https://github.com/bitcoin/bitcoin/pull/33568)
We have instructions on how to verify a subtree update, but not on how to perform one.
💬 mzumsande commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#issuecomment-3377423901)
> NIT: why keep looping ? is there a path where the node can recover from this ?
Not if we are actually corrupted, but we can't be 100% sure. Maybe we are right, and the next peer would extend the chain that we see as valid.
There is the possibility of an an attack where a miner creates an invalid block with valid PoW (which is costly), and has some sybil nodes feed that chain to others - it wouldn't be good if this could be used to shutdown nodes. Or there could be some kind of chaotic ch
...
(https://github.com/bitcoin/bitcoin/pull/33553#issuecomment-3377423901)
> NIT: why keep looping ? is there a path where the node can recover from this ?
Not if we are actually corrupted, but we can't be 100% sure. Maybe we are right, and the next peer would extend the chain that we see as valid.
There is the possibility of an an attack where a miner creates an invalid block with valid PoW (which is costly), and has some sybil nodes feed that chain to others - it wouldn't be good if this could be used to shutdown nodes. Or there could be some kind of chaotic ch
...
💬 Sjors commented on pull request "miner: empty mempool special case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3377434108)
@sipa I ended up updating `test/functional/interface_ipc.py` to reflect the new behavior, but I'm not sure what the original test tried to do.
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3377434108)
@sipa I ended up updating `test/functional/interface_ipc.py` to reflect the new behavior, but I'm not sure what the original test tried to do.
💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3377440627)
Reviewers may find #33566 interesting.
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3377440627)
Reviewers may find #33566 interesting.
💬 fanquake commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3377516698)
> Added to the nightly builds in https://github.com/hebasto/bitcoin-core-nightly/pull/74.
Looks like there are failing unit tests (https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18313967330/job/52151177201#step:6:401):
```bash
test\util_string_tests.cpp(43): Entering test case "ConstevalFormatString_NumSpec"
test/util_string_tests.cpp(40): error: in "util_string_tests/ConstevalFormatString_NumSpec": exception "const char*" raised as expected: validation on the raised exceptio
...
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3377516698)
> Added to the nightly builds in https://github.com/hebasto/bitcoin-core-nightly/pull/74.
Looks like there are failing unit tests (https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18313967330/job/52151177201#step:6:401):
```bash
test\util_string_tests.cpp(43): Entering test case "ConstevalFormatString_NumSpec"
test/util_string_tests.cpp(40): error: in "util_string_tests/ConstevalFormatString_NumSpec": exception "const char*" raised as expected: validation on the raised exceptio
...
💬 hebasto commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3377526162)
> > Added to the nightly builds in https://github.com/hebasto/bitcoin-core-nightly/pull/74.
>
>
>
> Looks like there are failing unit tests (https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18313967330/job/52151177201#step:6:401):
>
> ```bash
>
> test\util_string_tests.cpp(43): Entering test case "ConstevalFormatString_NumSpec"
>
> test/util_string_tests.cpp(40): error: in "util_string_tests/ConstevalFormatString_NumSpec": exception "const char*" raised as expected: validation
...
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3377526162)
> > Added to the nightly builds in https://github.com/hebasto/bitcoin-core-nightly/pull/74.
>
>
>
> Looks like there are failing unit tests (https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18313967330/job/52151177201#step:6:401):
>
> ```bash
>
> test\util_string_tests.cpp(43): Entering test case "ConstevalFormatString_NumSpec"
>
> test/util_string_tests.cpp(40): error: in "util_string_tests/ConstevalFormatString_NumSpec": exception "const char*" raised as expected: validation
...