💬 stutxo commented on pull request "add ignore_incoming_blocks option to ProcessMessage and SendMessages":
(https://github.com/bitcoin/bitcoin/pull/32366#issuecomment-2835737982)
oops opened this PR to the wrong place soz, ignore pls

💬 hebasto 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-2835789138)
> @hebasto I have upstreamed the changes as requested: [arun11299/cpp-subprocess#117](https://github.com/arun11299/cpp-subprocess/pull/117).

Thank you!

I've backported the merged upstream PR in https://github.com/bitcoin/bitcoin/pull/32358.

So this one can be closed, I think.

💬 hebasto commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2835793736)
Added https://github.com/arun11299/cpp-subprocess/pull/117, which is upstreamed https://github.com/bitcoin/bitcoin/pull/32342.

💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-2835804766)
Updated a56468ab0bf2154159919a2c3feba9974486d10e -> 0b72656ecf363d9fc4820a9e3e22afb2c98d4e75 ([`pr/ipc-stop.2`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.2) -> [`pr/ipc-stop.3`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-stop.2..pr/ipc-stop.3)) to try to fix init test CI failures https://github.com/bitcoin/bitcoin/actions/runs/14670251419/job/41176416988?pr=32345 and https://cirrus-ci.com/task/60942682
...

💬 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