💬 l0rinc commented on pull request "log: reduce excessive "rolling forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2372971637)
Agree, pushed a better solution to rate limit existing message + announcement of work ahead (as suggested by @ajtowns).
Rate limiting changes can be done in a separate PR now if needed.
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2372971637)
Agree, pushed a better solution to rate limit existing message + announcement of work ahead (as suggested by @ajtowns).
Rate limiting changes can be done in a separate PR now if needed.
💬 l0rinc commented on pull request "log: reduce excessive "rolling forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2372971804)
> Should be used with extreme care
Given that we didn't even have any rate limiting so far and as far as I can tell it wasn't weaponized so far I'm not sure this is an "extreme" danger.
But I don't mind removing the rate-limit related commit suggested by @ajtowns, since it seems to be more contentious than just reducing the rolling forward messages.
And I have extended it for the rolling forward messages as well and limit the log info to every 10k blocks to be less spammy.
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2372971804)
> Should be used with extreme care
Given that we didn't even have any rate limiting so far and as far as I can tell it wasn't weaponized so far I'm not sure this is an "extreme" danger.
But I don't mind removing the rate-limit related commit suggested by @ajtowns, since it seems to be more contentious than just reducing the rolling forward messages.
And I have extended it for the rolling forward messages as well and limit the log info to every 10k blocks to be less spammy.
💬 Sjors commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3324900365)
I'm not sure if `waitTipChanged` can handle being interrupted before its timeout yet, from either side. How does ctrl + c happens if you don't call `submitBlock`?
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3324900365)
I'm not sure if `waitTipChanged` can handle being interrupted before its timeout yet, from either side. How does ctrl + c happens if you don't call `submitBlock`?
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3324906640)
Did the same benchmark with 5000 dbcache and there is a 6% speedup :rocket: - 4:27 vs 4:44. Even with far less cache misses this change still a benefit, and will continue to improve block connection speed as the blockchain and utxo set gets bigger.
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `echo 688c03597afb0b76077f1ffc4608eef19481056e && /usr/bin/time ./build/bin/bitcoind -printtoconsole=0 -connect=192.168.2.171 -stopatheight=912683 -dbcache=5000` |
...
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3324906640)
Did the same benchmark with 5000 dbcache and there is a 6% speedup :rocket: - 4:27 vs 4:44. Even with far less cache misses this change still a benefit, and will continue to improve block connection speed as the blockchain and utxo set gets bigger.
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `echo 688c03597afb0b76077f1ffc4608eef19481056e && /usr/bin/time ./build/bin/bitcoind -printtoconsole=0 -connect=192.168.2.171 -stopatheight=912683 -dbcache=5000` |
...
📝 mzumsande opened a pull request: "p2p: Use network-dependent timers for inbound inv scheduling"
(https://github.com/bitcoin/bitcoin/pull/33464)
Currently, `NextInvToInbounds` schedules each round of `inv` at the same time for all inbound peers. It's being done this way because with a separate timer per peer (like it's done for outbounds), an attacker could do multiple connections to learn about the time a transaction arrived. (#13298).
However, having a single timer for inbounds of all networks is also an obvious fingerprinting vector: Connecting to a suspected pair of privacy-netowrk and clearnet addresses and observing the `inv` p
...
(https://github.com/bitcoin/bitcoin/pull/33464)
Currently, `NextInvToInbounds` schedules each round of `inv` at the same time for all inbound peers. It's being done this way because with a separate timer per peer (like it's done for outbounds), an attacker could do multiple connections to learn about the time a transaction arrived. (#13298).
However, having a single timer for inbounds of all networks is also an obvious fingerprinting vector: Connecting to a suspected pair of privacy-netowrk and clearnet addresses and observing the `inv` p
...
💬 l0rinc commented on pull request "ci: run s390x job":
(https://github.com/bitcoin/bitcoin/pull/33436#issuecomment-3324911767)
> The issues it finds seems to happen less than once per year
For the record a big-endian CI machine would have helped with https://github.com/bitcoin/bitcoin/pull/31144
(https://github.com/bitcoin/bitcoin/pull/33436#issuecomment-3324911767)
> The issues it finds seems to happen less than once per year
For the record a big-endian CI machine would have helped with https://github.com/bitcoin/bitcoin/pull/31144
💬 ismaelsadeeq commented on issue "RFC: Bitcoin Core Node `BlockTemplateManager`":
(https://github.com/bitcoin/bitcoin/issues/33389#issuecomment-3324920650)
I opened #33421 last week and made an update to it today, I think it is ready for for review now.
The fuzz test added in the PR also caught an issue on master that should be backported to v30 see https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3324859282
(https://github.com/bitcoin/bitcoin/issues/33389#issuecomment-3324920650)
I opened #33421 last week and made an update to it today, I think it is ready for for review now.
The fuzz test added in the PR also caught an issue on master that should be backported to v30 see https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3324859282
💬 hebasto commented on pull request "Fix dark mode detection on Linux":
(https://github.com/bitcoin-core/gui/pull/895#issuecomment-3324959004)
> > Could you please clarify which build this change is intended to affect, statically linked or shared?
> > If the latter, which system was this tested on (OS, desktop environment, Qt version)?
>
> this sadly only effects dynamically linked builds.
Bringing this functionality to statically linked (including release) binaries would require introducing new build dependencies. There is a consensus among Core developers that this should only be considered after separating the GUI build into
...
(https://github.com/bitcoin-core/gui/pull/895#issuecomment-3324959004)
> > Could you please clarify which build this change is intended to affect, statically linked or shared?
> > If the latter, which system was this tested on (OS, desktop environment, Qt version)?
>
> this sadly only effects dynamically linked builds.
Bringing this functionality to statically linked (including release) binaries would require introducing new build dependencies. There is a consensus among Core developers that this should only be considered after separating the GUI build into
...
💬 maflcko commented on pull request "ci: remove Clang build from msan fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33425#discussion_r2373034754)
yeah, but only to the extent of fixing the bug introduced in https://github.com/bitcoin/bitcoin/pull/33364/files by reverting the full and redundant clang compile it introduced.
(https://github.com/bitcoin/bitcoin/pull/33425#discussion_r2373034754)
yeah, but only to the extent of fixing the bug introduced in https://github.com/bitcoin/bitcoin/pull/33364/files by reverting the full and redundant clang compile it introduced.
📝 janb84 opened a pull request: "guix: Added guix-shasums script for gathering and formatting build output checksums"
(https://github.com/bitcoin/bitcoin/pull/33465)
When a PR requires proof of Guix builds (sha256sums), the PR author or reviewer uses a not well documented command to collect the sha256sums of build outputs or manually gathers them from files:
```sh
guix describe
uname -m
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
```
This PR introduces a guix-shasums script that gathers all the sha256sums from the output directories and either outputs them on the screen or fo
...
(https://github.com/bitcoin/bitcoin/pull/33465)
When a PR requires proof of Guix builds (sha256sums), the PR author or reviewer uses a not well documented command to collect the sha256sums of build outputs or manually gathers them from files:
```sh
guix describe
uname -m
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
```
This PR introduces a guix-shasums script that gathers all the sha256sums from the output directories and either outputs them on the screen or fo
...
💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3324991629)
> re: [#33445 (comment)](https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3315082319)
>
> > Could you please take a look at the following IPC-related warning:
>
> Thanks, created [capnproto/capnproto#2417](https://github.com/capnproto/capnproto/pull/2417) to address it
I've added a commit to temporarily silence this warning.
(https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3324991629)
> re: [#33445 (comment)](https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3315082319)
>
> > Could you please take a look at the following IPC-related warning:
>
> Thanks, created [capnproto/capnproto#2417](https://github.com/capnproto/capnproto/pull/2417) to address it
I've added a commit to temporarily silence this warning.
💬 Sjors commented on pull request "ci: remove Clang build from msan fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33425#discussion_r2373077161)
That build took over an hour on https://github.com/Sjors/sv2-tp and cherry-picking this commit made it fast again, so that's nice.
(https://github.com/bitcoin/bitcoin/pull/33425#discussion_r2373077161)
That build took over an hour on https://github.com/Sjors/sv2-tp and cherry-picking this commit made it fast again, so that's nice.
💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3325023136)
> I'm not sure if `waitTipChanged` and `waitNext` can handle being interrupted before their timeouts yet, from either side
They are supposed to be handle it because they wait on `kernel_notifications.m_tip_block_cv` and `chainman.m_interrupt` which should both get signaled on shutdown.
Client should also not need to do anything on shutdown and should just be disconnected.
From the log it seems like ctrl-c is being processed, and shutdown is proceeding all the way up to the "Shutdown down" mes
...
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3325023136)
> I'm not sure if `waitTipChanged` and `waitNext` can handle being interrupted before their timeouts yet, from either side
They are supposed to be handle it because they wait on `kernel_notifications.m_tip_block_cv` and `chainman.m_interrupt` which should both get signaled on shutdown.
Client should also not need to do anything on shutdown and should just be disconnected.
From the log it seems like ctrl-c is being processed, and shutdown is proceeding all the way up to the "Shutdown down" mes
...
💬 fanquake commented on pull request "ci: add libcpp hardening flags to macOS fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33462#issuecomment-3325033349)
Looks like this does pass with the correct Clang (https://github.com/bitcoin/bitcoin/actions/runs/17950891735/job/51049620756#step:3:16):
```bash
Apple clang version 17.0.0 (clang-1700.0.13.5)
Target: arm64-apple-darwin24.6.0
Thread model: posix
InstalledDir: /Applications/Xcode_16.4.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
```
with some slowdown. i.e on master:
```bash
tx_pool_standard: succeeded against 919 files in 22s.
utxo_total_supply: succeeded against
...
(https://github.com/bitcoin/bitcoin/pull/33462#issuecomment-3325033349)
Looks like this does pass with the correct Clang (https://github.com/bitcoin/bitcoin/actions/runs/17950891735/job/51049620756#step:3:16):
```bash
Apple clang version 17.0.0 (clang-1700.0.13.5)
Target: arm64-apple-darwin24.6.0
Thread model: posix
InstalledDir: /Applications/Xcode_16.4.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
```
with some slowdown. i.e on master:
```bash
tx_pool_standard: succeeded against 919 files in 22s.
utxo_total_supply: succeeded against
...
💬 maflcko commented on pull request "test: Fix TestFlushBehavior when coin has non-zero dynamic memory usage":
(https://github.com/bitcoin/bitcoin/pull/33381#discussion_r2373097896)
https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes
(https://github.com/bitcoin/bitcoin/pull/33381#discussion_r2373097896)
https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes
💬 plebhash commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3325050561)
I also tried with `mining` and `template` examples from https://github.com/rustaceanrob/bitcoin-ipc/tree/mining (there's also a `chain` example but it doesn't compile)
both `mining` and `template` do some brief interaction with `bitcoin-node` and then exit... in other words, they don't sustain a connection indefinitely, like my WIP code does
and killing `bitcoin-node` after these examples are finished works fine, no hanging
so this seems to be related to the fact that there's an active IPC c
...
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3325050561)
I also tried with `mining` and `template` examples from https://github.com/rustaceanrob/bitcoin-ipc/tree/mining (there's also a `chain` example but it doesn't compile)
both `mining` and `template` do some brief interaction with `bitcoin-node` and then exit... in other words, they don't sustain a connection indefinitely, like my WIP code does
and killing `bitcoin-node` after these examples are finished works fine, no hanging
so this seems to be related to the fact that there's an active IPC c
...
💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3325090308)
Makes sense an active connection or even an active call might be required to trigger this, but it's definitely not expected behavior, and there is probably a deadlock of some kind here. Its possible the patch from #33387 could fix this, and `thread apply all bt` output from GDB would also more clearly identify the problem
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3325090308)
Makes sense an active connection or even an active call might be required to trigger this, but it's definitely not expected behavior, and there is probably a deadlock of some kind here. Its possible the patch from #33387 could fix this, and `thread apply all bt` output from GDB would also more clearly identify the problem
💬 l0rinc commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3325096918)
I keep flipping between acking or nacking this change. I actually considered proposing something similar a few weeks ago but ultimately decided against it, so I understand the motivations here.
While we all want Bitcoin to succeed as the best form of money ever created, and we all understand that transactions need to generate sufficient fees to keep miners alive, I don't see how deliberately misconfiguring our nodes to reject transactions that will likely be mined anyway helps achieve these g
...
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3325096918)
I keep flipping between acking or nacking this change. I actually considered proposing something similar a few weeks ago but ultimately decided against it, so I understand the motivations here.
While we all want Bitcoin to succeed as the best form of money ever created, and we all understand that transactions need to generate sufficient fees to keep miners alive, I don't see how deliberately misconfiguring our nodes to reject transactions that will likely be mined anyway helps achieve these g
...
🤔 glozow reviewed a pull request: "[28.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/33415#pullrequestreview-3259161134)
ACK 1b1f359fc43385cb4d465b15754dac84eef06873
Could probably have someone double check the min feerate backport. The CI failure is related to #31210 / #30125. IIUC we skipped backporting #30125 as it's quite involved (also see https://github.com/bitcoin/bitcoin/pull/31422#pullrequestreview-2607160362)
(https://github.com/bitcoin/bitcoin/pull/33415#pullrequestreview-3259161134)
ACK 1b1f359fc43385cb4d465b15754dac84eef06873
Could probably have someone double check the min feerate backport. The CI failure is related to #31210 / #30125. IIUC we skipped backporting #30125 as it's quite involved (also see https://github.com/bitcoin/bitcoin/pull/31422#pullrequestreview-2607160362)
💬 fanquake commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3325105755)
Guix Build (x86_64):
```bash
cf7f86d91288477c11afdc767969d6b19b0ba1e62c73924e72b22a2527374653 guix-build-eca50854e1cb/output/aarch64-linux-gnu/SHA256SUMS.part
5afe50eb76079f96a5129b5e51e2ede657f10c5031abe24a875410f006c55511 guix-build-eca50854e1cb/output/aarch64-linux-gnu/bitcoin-eca50854e1cb-aarch64-linux-gnu-debug.tar.gz
ba6f7937c5d333e64560abcd95dbb3856ceaa43fbcde766ebcaed1f8e23f2a7e guix-build-eca50854e1cb/output/aarch64-linux-gnu/bitcoin-eca50854e1cb-aarch64-linux-gnu.tar.gz
bbdf057
...
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3325105755)
Guix Build (x86_64):
```bash
cf7f86d91288477c11afdc767969d6b19b0ba1e62c73924e72b22a2527374653 guix-build-eca50854e1cb/output/aarch64-linux-gnu/SHA256SUMS.part
5afe50eb76079f96a5129b5e51e2ede657f10c5031abe24a875410f006c55511 guix-build-eca50854e1cb/output/aarch64-linux-gnu/bitcoin-eca50854e1cb-aarch64-linux-gnu-debug.tar.gz
ba6f7937c5d333e64560abcd95dbb3856ceaa43fbcde766ebcaed1f8e23f2a7e guix-build-eca50854e1cb/output/aarch64-linux-gnu/bitcoin-eca50854e1cb-aarch64-linux-gnu.tar.gz
bbdf057
...