🤔 sipa reviewed a pull request: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-3289213901)
utACK c9ffb6d7104f016ab89057bd57cd128ac03fd1fc

💬 sipa commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2394630697)
In commit "net: avoid recursive m_nodes_mutex lock in DisconnectNode()"

Nit: the `auto` here made me worried this was making a copy of the `CNode` data structure. Maybe make the `CNode*` type explicit here?

💬 willcl-ark commented on issue "[29.x] guix build failure on ppc with xcb":
(https://github.com/bitcoin/bitcoin/issues/33488#issuecomment-3356499067)
Ah I see.

FWIW I am using the same guix version via nix but can't reproduce :(

```
[100%] Built target check-symbols
-- Install configuration: "RelWithDebInfo"
-- Installing: /distsrc-base/distsrc-f6d49d0a0995-powerpc64-linux-gnu/installed/bitcoin-f6d49d0a0995/bin/bitcoin-wallet
-- Installing: /distsrc-base/distsrc-f6d49d0a0995-powerpc64-linux-gnu/installed/bitcoin-f6d49d0a0995/share/man/man1/bitcoin-wallet.1
-- Installing: /distsrc-base/distsrc-f6d49d0a0995-powerpc64-linux-gnu/installed/bitco
...

📝 willcl-ark opened a pull request: "Clear out space on centos job"
(https://github.com/bitcoin/bitcoin/pull/33514)
Fixes #33293

Clear out space on the centos job be deleteing unnecessary files.

Raised in #33293 which pointed to a solution like https://github.com/google/oss-fuzz/commit/b7f04d782277638a67bc44865de445977eed4708 which is adapted slightly here.

Only runs when cache provider (runner) is `gha`, and on the CentOS job.

A run on my fork can be seen here: https://github.com/willcl-ark/bitcoin/actions/runs/18130629028/job/51596196163#step:6:2

💬 willcl-ark commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3356522227)
Maintaining an action in this repo which only caters to forks is not ideal, but unless GH increases the space on runners, or provides a "slim" image variant, then it might be the best there is for now.

As it doesn't run in this repo it should at least never break our CI.

💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3356581222)
`0c718da693...73071b0cdb`: rebase due to conflicts

💬 katesalazar commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3356590750)
Which one is the chain needing this change?

💬 janb84 commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3356595768)
So how does this relate to #33480 (remove centos for alpine) . Is this a fix until that PR gets merged ?

💬 Raimo33 commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3356601737)
Concept ACK

💬 willcl-ark commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3356601782)
> Which one is the chain needing this change?

Ah I was slightly unclear; this actually applies to "personal forks" (e.g. my own, your own) as well as actual coin-forks. Anywhere where Cirrus runners, which have more space, are not being used.

> So how does this relate to #33480 (remove centos for alpine) . Is this a fix until that PR gets merged ?

I had thought the alpine job would use less space, but I think they are ~ equivalent, so this fix is still needed on that job for runs on for
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3356612313)
`3aa74b5176...22fadecc14`: rebase due to conflicts and pick changes from https://github.com/bitcoin/bitcoin/pull/33454

💬 Crypt-iQ commented on issue "compact blocks in IBD resets m_stalling_since":
(https://github.com/bitcoin/bitcoin/issues/31962#issuecomment-3356659498)
I am (still) curious why the `!already_in_flight && !CanDirectFetch()` conditional was introduced and I wasn't able to find an explanation in https://github.com/bitcoin/bitcoin/pull/8068 (because I don't understand it, I'm not advocating for changing it). The check to see if we've requested the block doesn't distinguish between if we requested it with `MSG_CMPCT_BLOCK` or `MSG_BLOCK` in the GETDATA. It also doesn't tell us if we requested the block from the peer sending the compact block.

Ther
...

💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3356712363)
`73071b0cdb...5b22928a8e`: fix silent merge conflict with a newly added call to `OpenNetworkConnection()` which upset tidy with `/*strDest=*/` vs `/*pszDest=*/`.

💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-3356756655)
`c9ffb6d710...87e7f37918`: do https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2394630697

💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2394864828)
Done.

💬 sipa commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2394877881)
It occurs to me that it's possible the fork point between the active tip and `pindexBestKnownBlock` is ahead of `pindexLastCommonBlock`, but not quite equal to the active tip itself.

Would something like this, unconditionally, not be more general even?

```c++
auto fork_point = LastCommonAncestor(state->pindexBestKnownBlock, m_chainman.ActiveTip());
if (fork_point->nChainWork > state->pindexLastCommonBlock->nChainWork) {
state->pindexLastCommonBlock = fork_point;
}
```

If perfor
...

🤔 janb84 reviewed a pull request: "Clear out space on centos job"
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3289659139)
ACK 014b9f6268390b78e66e1be24813d3f069588442

PR introduces new CI action. This action will remove unnecessary packages to reduce space. This action will only run on `GHA` runners and will save about 22gb.

Kinda surprised about the space savings or why are the packages are there in the first place. Anyhow this will help to resolve the GHA space problems for Centos job. (resolves #33293)

🤔 furszy reviewed a pull request: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-3289688389)
Code review ACK 87e7f37918d42c28033e9f684db52f94eeed617b

💬 ryanofsky commented on pull request "init: Signal m_tip_block_cv on Ctrl-C":
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3356907361)
I'm confused by windows CI failures: [1](https://github.com/bitcoin/bitcoin/actions/runs/18140884308/job/51631167802?pr=33511) [2](https://github.com/bitcoin/bitcoin/actions/runs/18140884308/job/51631960886?pr=33511)

They seem to show py -3 test_runner.py being called, and this just with failing with no output after around 10 minutes, with CI reporting `Process completed with exit code -1073741510.` where -1073741510 is 0xc000013a or [STATUS_CONTROL_C_EXIT](https://learn.microsoft.com/en-us/o
...

💬 l0rinc commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2395019568)
This is meant to avoid a partial initialization, `.value()` throws if we forget to call `FillShortTxIDSelector` (while previously we would have just salted with 0)