💬 theuni commented on pull request "net: remove unnecessary check from AlreadyConnectedToAddress()":
(https://github.com/bitcoin/bitcoin/pull/32338#discussion_r2064058927)
I agree this is overkill.

📝 hebasto opened a pull request: "cmake: Check user-defined `APPEND_*FLAGS` variables early"
(https://github.com/bitcoin/bitcoin/pull/32367)
For the purpose of checks performed by the build system, we strive to handle user-defined `APPEND_*FLAGS` in the same way as flags provided by other standard means. In particular, they are considered when the `try_compile()` command is used.

However, these flags are not considered during `enable_language()` command invocation due to design limitations, which might cause issues, such as https://github.com/bitcoin/bitcoin/issues/32323.

This PR addresses the issue by introducing an additional
...

👍 theuni approved a pull request: "net: remove unnecessary check from AlreadyConnectedToAddress()"
(https://github.com/bitcoin/bitcoin/pull/32338#pullrequestreview-2799968661)
utACK f1b142856a4ecd0a0d90bc3d73ef5137997b14ff

As I mentioned above, I believe this was needed at one point when connecting to an IP through a proxy, or when using addnode for an IP, as those were stored as strings rather than resolved addresses.

But now that we always attempt to resolve ips, `m_addr_name` is only interesting for non-ip connections. So the check here should be redundant and unnecessary.

💬 rstmsn commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2835842668)
Concept NACK.

- Between blocks 877325 (01 Jan 2025) and 894289 (today) there's 30 non-std OP_RETURN txs out of 7 million, or 0.004246903% of the total.

- That's a 99.995753097% success rate for the 83 byte limit spam filter in 2025.

💬 hebasto commented on issue "build: CMake caching failing PIE check":
(https://github.com/bitcoin/bitcoin/issues/32323#issuecomment-2835842776)
After further consideration, I believe this issue exposes a bug that is fixed in https://github.com/bitcoin/bitcoin/pull/32367.

📝 hebasto converted_to_draft a pull request: "cmake: Check user-defined `APPEND_*FLAGS` variables early"
(https://github.com/bitcoin/bitcoin/pull/32367)
For the purpose of checks performed by the build system, we strive to handle user-defined `APPEND_*FLAGS` in the same way as flags provided by other standard means. In particular, they are considered when the `try_compile()` command is used.

However, these flags are not considered during `enable_language()` command invocation due to design limitations, which might cause issues, such as https://github.com/bitcoin/bitcoin/issues/32323.

This PR addresses the issue by introducing an additional
...

👋 hebasto's pull request is ready for review: "cmake: Check user-defined `APPEND_*FLAGS` variables early"
(https://github.com/bitcoin/bitcoin/pull/32367)

💬 Sjors commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2835862040)
@rstmsn that's not a sensible metric of "success", which you haven't defined either. If I were to define it as "percentage of attempted OP_RETURN transactions included in a block", and if I then consider the lack evidence of a single attempt, the success rate is 100%. The fact that one can find such wildly different metrics and definitions, implies there's all useless.

💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#discussion_r2064099087)
Thanks, yes, that would be betterr.

💬 wizkid057 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2835923552)
> @rstmsn that's not a sensible metric of "success", which you haven't defined either. If I were to define "success" as "percentage of attempted OP_RETURN transactions permanently excluded from the blockchain", and if I then consider the lack of evidence for a single failed attempt, the success rate is 0%. The fact that one can find such wildly different metrics and definitions, implies they're all useless.
>
> (updated to be more in line with your sense that more filtering means more success
...

🤔 brunoerg reviewed a pull request: "descriptors: Reject + sign while parsing unsigned"
(https://github.com/bitcoin/bitcoin/pull/32365#pullrequestreview-2800083002)
Concept ACK

💬 theuni commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2064139694)
> Hmm, in `CConnman::OpenNetworkConnection()` the code does:
>
> * if `pszDest` is not set then use `AlreadyConnectedToAddress(addrConnect)` which ignores the port
>
> * if `pszDest` is set then `OutboundConnectedToStr(pszDest)` which compares the port as well.
>
>
> Changing that logic is, I guess, out of the scope of this non-functional refactor PR. If to be done it would better be assessed in isolation in its own PR.

Also, later on in `ConnectNode()`:

```c++
if (
...

✅ tomasandroil closed a pull request: "Fix missing error check in `set_clo_on_exec` for FD_CLOEXEC handling"
(https://github.com/bitcoin/bitcoin/pull/32342)

💬 tomasandroil commented on pull request "Fix missing error check in `set_clo_on_exec` for FD_CLOEXEC handling":
(https://github.com/bitcoin/bitcoin/pull/32342#issuecomment-2835987967)
Closing as requested — changes were upstreamed and backported in #32358. Thanks everyone

💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2835990608)
[3acd934311477ac88e1c3176aaeaec2b3ad35425 → bdf634918c32d3c0da2c2262252470d1755bb085](https://github.com/bitcoin/bitcoin/compare/3acd934311477ac88e1c3176aaeaec2b3ad35425..bdf634918c32d3c0da2c2262252470d1755bb085)
- Use ToIntegral instead of ParseInt

💬 theStack commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2835996184)
Concept ACK

💬 D33r-Gee commented on pull request "interface: Expose load utxo snapshot functionality":
(https://github.com/bitcoin-core/gui/pull/869#issuecomment-2836004928)
> can you show this works with some sort of POC on the bitcoin-core/gui side?

Great idea! Just updated the description with a link to a POC and screenshots...

💬 brunoerg commented on pull request "fuzz: doc: add info about `afl-system-config` for macOS":
(https://github.com/bitcoin/bitcoin/pull/32175#issuecomment-2836044421)
Pushed 66572c27454e1464173dc318d62fdfc11d4b7832..6e026606f368d8d1139b266c382076685e76d0b2 to address https://github.com/bitcoin/bitcoin/pull/32175#discussion_r2042642262

💬 brunoerg commented on pull request "fuzz: doc: add info about `afl-system-config` for macOS":
(https://github.com/bitcoin/bitcoin/pull/32175#discussion_r2064193303)
Done. Thanks.

🤔 janb84 reviewed a pull request: "fuzz: doc: add info about `afl-system-config` for macOS"
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2800214659)
re ACK [6e02660](https://github.com/bitcoin/bitcoin/pull/32175/commits/6e026606f368d8d1139b266c382076685e76d0b2)

Changes sinds last ACK:
- Minor style change in comments to align with rest of comment style.