💬 ishaanam commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r2167076582)
Done

💬 l0rinc commented on pull request "clang-format: modernize and realign clang-format configuration":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2167081920)
Many of these keys aren't even listed in the documentation and the IDE shows that many are invalid since it doesn't match the allowed schema.

💬 maflcko commented on pull request "clang-format: modernize and realign clang-format configuration":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2167114881)
I'd say the others are probably edge cases to not matter much, but no strong opinion. Seems fine to add them as well to avoid silent changes. However, I don't think the config is forward compatible, so it can probably only contain keys of the minimum required clang version.

💬 vasild commented on pull request "doc: add release notes for #32425":
(https://github.com/bitcoin/bitcoin/pull/32727#issuecomment-3005398388)
`a4cfaa5eab...b9a2e8ee96`: take suggestions

💬 vasild commented on pull request "doc: add release notes for #32425":
(https://github.com/bitcoin/bitcoin/pull/32727#discussion_r2167135148)
Done, plus some more rewording.

💬 hebasto commented on pull request "cmake: Use `HINTS` instead of `PATHS` in `find_*` commands":
(https://github.com/bitcoin/bitcoin/pull/32805#issuecomment-3005421352)
@purpleKarrot

Thank you for the review!

> Nit: Is there a problem on master?

In terms of build failure -- no.
In terms of needlessly including overly broad paths like `/usr/local/include` or `/opt/homebrew/include` on macOS -- yes.

> It looks like the result from `libqrencode.pc` is ignored...

It depends on the platform used for building. On macOS, that's the case on the master branch, but not with this PR.

> ... if that is not causing actual problems, maybe `pkg_check_module
...

🤔 mzumsande reviewed a pull request: "test: fix catchup loop in outbound eviction functional test"
(https://github.com/bitcoin/bitcoin/pull/32742#pullrequestreview-2958894994)
Code Review ACK dd8447f70faf6419b4617da3c1b57098e9cd66a6

I also think that this part of the test would benefit from additional cleanups / more comments, it's pretty hard to understand what's going on.

💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2167155199)
The reviewers of the PR are only ACKing that the actual data from the collaborative run was used and that the data was encoded correctly by reproducing the file hash. This is the part I am referring to a little further below:

```
Files will not be merged to the repository
without at least two additional reviewers confirming that the process described
above was followed as expected and that the encoding step yielded the same
file hash.
```

The 5 participants is the number I am looking
...

👍 pablomartin4btc approved a pull request: "doc: add release notes for #32425"
(https://github.com/bitcoin/bitcoin/pull/32727#pullrequestreview-2958946450)
> a4cfaa5eab...b9a2e8ee96: take suggestions

re-ACK b9a2e8ee96

👍 hebasto approved a pull request: "depends: Bump boost to 1.88.0 and use new CMake buildsystem"
(https://github.com/bitcoin/bitcoin/pull/32665#pullrequestreview-2958947519)
re-ACK 6c2538d5bfeafe6f234e5382c151d173826d78f0, only rebased since my previous [review](https://github.com/bitcoin/bitcoin/pull/32665#pullrequestreview-2891203225).

💬 vasild commented on pull request "doc: add release notes for #32425":
(https://github.com/bitcoin/bitcoin/pull/32727#discussion_r2167192118)
Done.

"for example `-proxy=127.0.0.1:5555=ipv6` would configure a proxy only for IPv6"

> it looks like it's talking about the past behavior

I guess I used "5: The second conditional" from https://www.perfect-english-grammar.com/would.html:
> If I had enough money, I would travel around the world.
> If we lived in Madrid, we would study Spanish.

🤔 janb84 reviewed a pull request: "doc: add release notes for #32425"
(https://github.com/bitcoin/bitcoin/pull/32727#pullrequestreview-2959000718)
reACK b9a2e8ee965daffe2bc618f58b21f0ddebadeb23

Changes sinds last ACK;
- div textual changes

(still find it weird that a non-localhost ipv4 address for ipv6 proxy can be used, but agree to disagree on that )

💬 stringintech commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#discussion_r2167253212)
I think the +1 offset would still be needed in a corner case when the input of the ceil function is already an integer. In this case, if +1 is removed, the timeout will not be triggered because the check `current_time > peer->m_headers_sync_timeout` ([here](https://github.com/bitcoin/bitcoin/blob/7d5a6d17398aae3cea6d63564d33e7e49da3c71c/src/net_processing.cpp#L5888)) would evaluate to `timeout > timeout` (false) instead of `timeout + 1 > timeout` (true).

💬 stringintech commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#discussion_r2167267966)
Aside from the +1 offset though, maybe it is good to apply your suggestion and use the ceil function so that `calculate_headers_timeout` returns a time that is >= the real timeout calculated by core, ensuring it doesn't return a lower value due to precision loss.

👋 ishaanam's pull request is ready for review: "wallet, rpc: add anti-fee-sniping to `send` and `sendall`"
(https://github.com/bitcoin/bitcoin/pull/28944)

📝 hebasto opened a pull request: "cmake: Explicitly specify `Boost_ROOT` for Homebrew's package"
(https://github.com/bitcoin/bitcoin/pull/32814)
On macOS, this change ensures that the Boost package is located at its real path rather than via the symlink in the default prefix.

A backport to 29.x is required for https://github.com/bitcoin/bitcoin/pull/32804, as this change prevents contamination of include directories by broad locations such as `/usr/local/include` or `/opt/homebrew/include`, which take precedence over Qt’s `-iframework` flags.

Below is the relevant change in the configuration logs on my macOS 15.5 `x64`:
- master b
...

💬 hebasto commented on pull request "Fix build on macOS when `qt@6` is installed":
(https://github.com/bitcoin/bitcoin/pull/32804#issuecomment-3005666028)
> Please review #32805 first.

Also https://github.com/bitcoin/bitcoin/pull/32814.

💬 stratospher commented on pull request "test: refactor out same-txid-diff-wtxid tx to reuse in other tests":
(https://github.com/bitcoin/bitcoin/pull/32385#discussion_r2167312793)
I like how the interface is simpler! but what about this comment though? - https://github.com/bitcoin/bitcoin/pull/32385#discussion_r2074738962

transactions constructed using this method would be non-standard. so when we try to `sendrawtransaction` a high-S signature transaction, mempool accept fails and an error would be returned [here](https://github.com/bitcoin/bitcoin/blob/7d5a6d17398aae3cea6d63564d33e7e49da3c71c/src/node/transaction.cpp#L75).

```
test_framework.authproxy.JSONRPCExcep
...

🤔 hebasto reviewed a pull request: "depends: Bump boost to 1.88.0 and use new CMake buildsystem"
(https://github.com/bitcoin/bitcoin/pull/32665#pullrequestreview-2959223968)
My Guix build:
```
aarch64
dcedbdafa5857dbbb7748e27bbbfadc57a5dc70eff9b565c1d30061e64027ca4 guix-build-6c2538d5bfea/output/aarch64-linux-gnu/SHA256SUMS.part
ccf56c678924e933ef46f306d0be509251d1920f17529e11f007a5c0b74a5282 guix-build-6c2538d5bfea/output/aarch64-linux-gnu/bitcoin-6c2538d5bfea-aarch64-linux-gnu-debug.tar.gz
550fa33f9e19c06cf4651abd4e69d813db6702461b6b59b717a719821c8b6495 guix-build-6c2538d5bfea/output/aarch64-linux-gnu/bitcoin-6c2538d5bfea-aarch64-linux-gnu.tar.gz
aab708c3
...

💬 glozow commented on pull request "descriptors: Allow `H` as a hardened indicator":
(https://github.com/bitcoin/bitcoin/pull/32788#issuecomment-3005754433)
Not merging for now - IIUC if the approach mentioned in https://groups.google.com/u/1/g/bitcoindev/c/IAYEx4zUhHA is taken, this would be closed?