💬 katesalazar commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3346192356)
I think it's best when as many CI scripts as possible can be run on _any_ host. But I can agree on not cahnging this if there is no clear underlining issue being fixed.

🤔 janb84 reviewed a pull request: "CMake: Add dynamic test discovery"
(https://github.com/bitcoin/bitcoin/pull/33483#pullrequestreview-3279048407)
Approach ACK 6318195b8ecea6d4ced57c8aac97e1ca713d9822

This PR changes how (en when) we do test discovery. From configure time -> Test time and by use of a CMake module.

NIT: Please extend the PR description of the intended changed behaviour of bench sanity check tests. From single bench sanity check to one bench sanity check for each bench target.

💬 janb84 commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#discussion_r2387475262)
This comment line, is this intended as a reminder to remove this module if the code is added upstream ?

if so, Nit: please make the intent more explicit. e.g

```suggestion
#TODO remove file if fixed upstream:
# https://gitlab.kitware.com/cmake/cmake/-/issues/26920
```

🤔 janb84 reviewed a pull request: "ci: Turn CentOS config into Alpine musl config"
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3279090957)
reACK 444409ff2b78d8f3e541bd6e883af8da7adfd264

changes since last ACK:
- changed container size to md. CI still works with this size. ✅

👍 willcl-ark approved a pull request: "ci: Turn CentOS config into Alpine musl config"
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3279091535)
ACK 444409ff2b78d8f3e541bd6e883af8da7adfd264

This took 29 minutes for an alpine depends, gui build, with no docker/ccache/depends caches (as no merges to master yet) which seems in line with other jobs.

📝 AmelkaDzikwk opened a pull request: "Create based point of view"
(https://github.com/bitcoin/bitcoin/pull/33490)
#2

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that
...

💬 AmelkaDzikwk commented on pull request "Create based point of view":
(https://github.com/bitcoin/bitcoin/pull/33490#issuecomment-3346308216)
not sure what does it mean?

💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3346353073)
> Thanks for the rebase :)
>
> I updated darosior's [core_bdk_wallet](https://github.com/darosior/core_bdk_wallet?tab=readme-ov-file#generating-rust-ipc-interface-from-capnp-definition) to use this newest version and I ran into a few crashes again. This is the one I managed to get a reproducible backtrace for:

Unfortunately, I don't think this is a meaningful backtrace, since it is just showing the trace of a thread at the time it receives a SIGPIPE signal (`Thread 3 "b-capnp-loop" receiv
...

💬 TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3346397449)
Err, right. Updated the description with your suggested trace filtering.

👍 TheCharlatan approved a pull request: "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)"
(https://github.com/bitcoin/bitcoin/pull/29640#pullrequestreview-3279259103)
ACK

0465574c127907df9b764055a585e8281bae8d1d

💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3346459809)
Friendly ping @laanwj :)

💬 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_r2387660586)
There were previous discussions about the names of these methods: https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2054679774 and https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2072443916

I now renamed the method discussed above, so all 3 are changed from:
```cpp
// previous version of the PR
AlreadyConnectedToAddressPort(const std::string& addr_port);
AlreadyConnectedToAddressPort(const CService& addr_port);
AlreadyConnectedToAddress(const CNetAddr& addr);
```
to

...

💬 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-3346520156)
`8668b0c64d...c9ffb6d710`: rebase and https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2372244246

📝 AureliuszNada opened a pull request: "Create promise"
(https://github.com/bitcoin/bitcoin/pull/33491)
111

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that
...

💬 AureliuszNada commented on pull request "Create promise":
(https://github.com/bitcoin/bitcoin/pull/33491#issuecomment-3346537768)
!!! fix

📝 AureliuszNada opened a pull request: "Create no promises"
(https://github.com/bitcoin/bitcoin/pull/33492)
111

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that
...

💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3346569174)
`ec7c215345...82e46a7248`: rebase to hopefully fix CI and elaborate the return value of ``OpenNetworkConnection()` in the commit message.

💬 maflcko commented on pull request "test: Run bench sanity checks in parallel with functional tests":
(https://github.com/bitcoin/bitcoin/pull/33142#issuecomment-3346604822)
rebased after https://github.com/bitcoin/bitcoin/pull/33313 (queue refactor)

👍 hodlinator approved a pull request: "ci: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3279511439)
ACK fab9ddaf554837624fa8f388e046a30d5bf7626f

💬 hodlinator commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387762653)
I guess the amount of Mac-specific code we have that behaves differently between 14 and 15 is minimal.

If it hasn't been a problem previously I'm okay with bumping it so we don't need to re-touch in 1 year - Although if we are going to keep bumping the minimum supported version and CI version every year anyway... I would lean towards keeping them in sync. (GitHub deprecating and removing older versions would then also serve as an extra reminder if we are late to update).