💬 Sjors commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#issuecomment-2497685083)
I added a comment based on https://github.com/bitcoin/bitcoin/pull/31346#discussion_r1855741271 to clarify under which circumstances `m_tip_block` is unset at this init stage.

I find it quite confusing that `ActivateBestChain` is called by `ImportBlocks` even if no blocks are imported, but haven't dared to refactor it.

💬 maflcko commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#issuecomment-2497727233)
> I find it quite confusing that `ActivateBestChain` is called by `ImportBlocks` even if no blocks are imported, but haven't dared to refactor it.

It is explained in the code comment above the line that does it. It was added as a fix for https://github.com/bitcoin/bitcoin/issues/2239

💬 fanquake commented on pull request "depends, refactor: Avoid hardcoding `host_prefix` in toolchain file":
(https://github.com/bitcoin/bitcoin/pull/31358#issuecomment-2497736026)
Is there any particular motivation (to do this now), given it doesn't work/can't be tested?

🤔 rkrux reviewed a pull request: "include verbose "reject-details" field in testmempoolaccept response"
(https://github.com/bitcoin/bitcoin/pull/28121#pullrequestreview-2458147883)
I want to test this out, do you mind rebasing it so that the `CMake` build changes are in this as well?

```
➜ bitcoin git:(tma-debug) ✗ bitcoinmakeall
CMake Error: The source directory "/Users/rkrux/projects/bitcoin" does not appear to contain CMakeLists.txt.
Specify --help for usage, or press the help button on the CMake GUI.
make: *** [cmake_check_build_system] Error 1
```

💬 rkrux commented on pull request "include verbose "reject-details" field in testmempoolaccept response":
(https://github.com/bitcoin/bitcoin/pull/28121#discussion_r1856436682)
Forgot to push?

💬 hebasto commented on pull request "depends, refactor: Avoid hardcoding `host_prefix` in toolchain file":
(https://github.com/bitcoin/bitcoin/pull/31358#issuecomment-2497746776)
> Is there any particular motivation (to do this now), given it doesn't work/can't be tested?

To achieve the same quality in `toolchain.cmake` as it was with `config.site` in Autotools.

💬 maflcko commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2497769121)
re-lgtm ACK b031b7910d62819443813b91705211c466933a53

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856458151)
Thanks! Switched to use `APT_LLVM_V`.

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856459919)
This change has been removed from this PR.

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856460150)
This change has been removed from this PR.

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856462103)
This change has been removed from this PR.

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#issuecomment-2497780660)
@maflcko @l0rinc

Thank you for your reviews. The PR has been updated per your feedback.

💬 maflcko commented on issue "Tracepoint Interface Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/31274#issuecomment-2497785934)
There is also a `TRACE_RAII` idea from https://github.com/bitcoin/bitcoin/pull/28226, mostly for performance/timing tracking in that example.

💬 vasild commented on issue "Discover() will not run if listening on any address with an explicit bind=0.0.0.0":
(https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2497792487)
@andremralves good observation! `Discover()` seems to be ok, but the test has rotten because it is not run regularly in CI (it was clear upfront that this may happen, but still it is better to have it than not, maybe it can be made to be run by CI if we can `ifconfig lo:0 1.1.1.1/32 up` in the CI?).

At some point the test framework was changed to add some `-bind`s:

https://github.com/bitcoin/bitcoin/blob/2638fdb4f934be96b7c798dbac38ea5ab8a6374a/test/functional/test_framework/test_node.py#L
...

💬 maflcko commented on pull request "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors":
(https://github.com/bitcoin/bitcoin/pull/31364#issuecomment-2497801969)
I don't think you need to repeat the full diff (or a variation of it) in the commit message. If someone was interested in the detailed diff, they could just look at the diff.

💬 fanquake commented on issue "Discover() will not run if listening on any address with an explicit bind=0.0.0.0":
(https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2497815300)
> At some point the test framework was changed to add some -binds:

That was done in #22729.

💬 hebasto commented on pull request "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors":
(https://github.com/bitcoin/bitcoin/pull/31364#issuecomment-2497820760)
Concept ACK.

I'd prefer if this PR was based on https://github.com/bitcoin/bitcoin/pull/31306 to ensure that CI checks that all cases are addressed.

💬 hebasto commented on pull request "[POC] ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1856504656)
FWIW, we do not use the installed packages cache in the "test-each-commit" job.

💬 l0rinc commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856545513)
Thank you!

💬 hebasto commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2497915410)
From the [macOS 14 arm64 (20241125) Image Update](https://github.com/actions/runner-images/releases/tag/macos-14-arm64%2F20241125.556):
> Added: `pkgconf` 2.3.0
> Deleted: `pkg-config` 0.29.2