π¬ l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2396546850)
Good point, thanks
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2396546850)
Good point, thanks
π¬ l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2396546909)
Thanks
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2396546909)
Thanks
π¬ l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2396548075)
Done, thanks
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2396548075)
Done, thanks
π¬ l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2396551204)
Done, thanks
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2396551204)
Done, thanks
π¬ l0rinc commented on pull request "[IBD] coins: increase default UTXO flush batch size to 32 MiB":
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-3358939197)
I have also measured a db-cache adjusted AssumeUTXO load to see which version performs better under the same memory requirements - as suggested by @sipa.
Reran the *before* scenario with `-dbcache=450`, it basically consumed the same *744 MiB* memory as before, finishing in *1h:46m*:
```
Command: ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -server -blocksonly -connect=0 -dbcache=450 -printtoconsole=0
Massif arguments: --time-unit=ms --massif-out-file=/mnt/
...
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-3358939197)
I have also measured a db-cache adjusted AssumeUTXO load to see which version performs better under the same memory requirements - as suggested by @sipa.
Reran the *before* scenario with `-dbcache=450`, it basically consumed the same *744 MiB* memory as before, finishing in *1h:46m*:
```
Command: ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -server -blocksonly -connect=0 -dbcache=450 -printtoconsole=0
Massif arguments: --time-unit=ms --massif-out-file=/mnt/
...
π¬ TheCharlatan commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3359134129)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3359134129)
Concept ACK
π hodlinator approved a pull request: "log: print every script verification state change"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3292830294)
re-ACK 45bd8914658a675d00aa9c83373d6903a8a9ece8
Just fixed some nits since previous review.
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3292830294)
re-ACK 45bd8914658a675d00aa9c83373d6903a8a9ece8
Just fixed some nits since previous review.
π¬ maflcko commented on issue "[29.x] guix build failure on ppc with xcb":
(https://github.com/bitcoin/bitcoin/issues/33488#issuecomment-3359477692)
I'd try to setup a fresh guix (maybe in a container) and build from there to check if something is wrong with your CPU/memory or the guix install.
It should be roughly:
```sh
docker run --privileged -it --rm ubuntu:24.04
(inside the container, assuming the guix apt package still exists:)
export DEBIAN_FRONTEND=noninteractive && apt update && apt install git vim htop guix bash curl make -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 /bitcoin-core && cd /bitcoin-core && group
...
(https://github.com/bitcoin/bitcoin/issues/33488#issuecomment-3359477692)
I'd try to setup a fresh guix (maybe in a container) and build from there to check if something is wrong with your CPU/memory or the guix install.
It should be roughly:
```sh
docker run --privileged -it --rm ubuntu:24.04
(inside the container, assuming the guix apt package still exists:)
export DEBIAN_FRONTEND=noninteractive && apt update && apt install git vim htop guix bash curl make -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 /bitcoin-core && cd /bitcoin-core && group
...
π¬ maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2397392467)
> nit: This change to ci.yml in this PR should probably be broken out to a separate commit or mentioned in the commit message as now it's just "Drop support for EOL macOS 13".
Thx, I'll adjust the commit message on the next push, if there is one. Leaving as-is for now.
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2397392467)
> nit: This change to ci.yml in this PR should probably be broken out to a separate commit or mentioned in the commit message as now it's just "Drop support for EOL macOS 13".
Thx, I'll adjust the commit message on the next push, if there is one. Leaving as-is for now.
π¬ vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3359702419)
`5b22928a8e...c76de2eea1`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3359702419)
`5b22928a8e...c76de2eea1`: rebase due to conflicts
π¬ maflcko commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3359890309)
Hmm, is the GHA cache even large enough to hold them? https://github.com/m3dwards/bitcoin/actions/caches says " Approaching total cache storage limit (9.41 GB of 10 GB Used) "
So I wonder, while this may fix the auth issue, the cache will still not be more useful.
It could be useful to know what is the sum of the size of all "normal" caches (ccache, depends, ...), and then the sum of the size of all docker caches.
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3359890309)
Hmm, is the GHA cache even large enough to hold them? https://github.com/m3dwards/bitcoin/actions/caches says " Approaching total cache storage limit (9.41 GB of 10 GB Used) "
So I wonder, while this may fix the auth issue, the cache will still not be more useful.
It could be useful to know what is the sum of the size of all "normal" caches (ccache, depends, ...), and then the sum of the size of all docker caches.
π¬ Sjors commented on pull request "init: Fix Ctrl-C shutdown hangs during wait calls":
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3359916481)
ACK 68cad90dace40f7a015ca4ff81b878fc8fdc1dd5
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3359916481)
ACK 68cad90dace40f7a015ca4ff81b878fc8fdc1dd5
π¬ Raimo33 commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3360234292)
Concept ACK
I agree with the backporting
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3360234292)
Concept ACK
I agree with the backporting
π¬ Sjors commented on pull request "init: Fix Ctrl-C shutdown hangs during wait calls":
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3360247170)
Mmm, the GUI experience is less great:
<img width="760" height="558" alt="SchermΒafbeelding 2025-10-02 om 11 55 50" src="https://github.com/user-attachments/assets/78eaf83c-b598-4fd1-abae-86aac2c47f24" />
This seems to stay stuck.
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3360247170)
Mmm, the GUI experience is less great:
<img width="760" height="558" alt="SchermΒafbeelding 2025-10-02 om 11 55 50" src="https://github.com/user-attachments/assets/78eaf83c-b598-4fd1-abae-86aac2c47f24" />
This seems to stay stuck.
π¬ Eunovo commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3360275363)
re-ACK https://github.com/bitcoin/bitcoin/pull/33336/commits/45bd8914658a675d00aa9c83373d6903a8a9ece8
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3360275363)
re-ACK https://github.com/bitcoin/bitcoin/pull/33336/commits/45bd8914658a675d00aa9c83373d6903a8a9ece8
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3360280902)
`b01d6df464...21efe8fa82`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3360280902)
`b01d6df464...21efe8fa82`: rebase due to conflicts
π€ stratospher reviewed a pull request: "net: support overriding the proxy selection in ConnectNode()"
(https://github.com/bitcoin/bitcoin/pull/33454#pullrequestreview-3293786225)
ACK c76de2e.
(non-blocking question) did you consider saving `proxy` in `ThreadPrivateBroadcast()` in `m_private_broadcast` object and then retrieving it later in `ConnectNode` instead of passing the `proxy` as a parameter through `OpenNetworkConnection`/`ConnectNode`? It would eliminate some parameter passing that's only used for private broadcast connections. but might make `proxy` lifetime unclear (so not sure about this), just curious if you considered this.
(https://github.com/bitcoin/bitcoin/pull/33454#pullrequestreview-3293786225)
ACK c76de2e.
(non-blocking question) did you consider saving `proxy` in `ThreadPrivateBroadcast()` in `m_private_broadcast` object and then retrieving it later in `ConnectNode` instead of passing the `proxy` as a parameter through `OpenNetworkConnection`/`ConnectNode`? It would eliminate some parameter passing that's only used for private broadcast connections. but might make `proxy` lifetime unclear (so not sure about this), just curious if you considered this.
π¬ ryanofsky commented on pull request "init: Fix Ctrl-C shutdown hangs during wait calls":
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3360338559)
> Mmm, the GUI experience is less great (testnet4, ctrl + c from the terminal where I launched it):
> [...]
> Doesn't happen on the first commit, so might be a regression.
Oh, thanks for testing. That does sound like a regression. Will mark as draft until this is debugged
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3360338559)
> Mmm, the GUI experience is less great (testnet4, ctrl + c from the terminal where I launched it):
> [...]
> Doesn't happen on the first commit, so might be a regression.
Oh, thanks for testing. That does sound like a regression. Will mark as draft until this is debugged
π ryanofsky converted_to_draft a pull request: "init: Fix Ctrl-C shutdown hangs during wait calls"
(https://github.com/bitcoin/bitcoin/pull/33511)
Signal `m_tip_block_cv` when Ctrl-C is pressed or `SIGTERM` is received, the same way it is currently signaled when the `stop` RPC is called. This lets RPC calls like `waitforblockheight` and IPC calls like `waitTipChanged` be interrupted, instead of waiting for their original timeouts and delaying shutdown.
This issue was reported by plebhash in #33463. These hangs have been present since #30409.
(https://github.com/bitcoin/bitcoin/pull/33511)
Signal `m_tip_block_cv` when Ctrl-C is pressed or `SIGTERM` is received, the same way it is currently signaled when the `stop` RPC is called. This lets RPC calls like `waitforblockheight` and IPC calls like `waitTipChanged` be interrupted, instead of waiting for their original timeouts and delaying shutdown.
This issue was reported by plebhash in #33463. These hangs have been present since #30409.
π€ optout21 reviewed a pull request: "net: support overriding the proxy selection in ConnectNode()"
(https://github.com/bitcoin/bitcoin/pull/33454#pullrequestreview-3293811866)
ACK c76de2eea18076f91dd80b52f66ba790f071a2b1
Changes since [last review](https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3352795379): rebase only.
(https://github.com/bitcoin/bitcoin/pull/33454#pullrequestreview-3293811866)
ACK c76de2eea18076f91dd80b52f66ba790f071a2b1
Changes since [last review](https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3352795379): rebase only.