💬 wizkid057 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2835638919)
> @wizkid057 your comment borders on off-topic, but I'm not going to hide it

Thanks. As a general note I want to briefly point out that I've personally been a long time contributor to the Bitcoin ecosystem as a whole. Longer than many/most active devs here. While my contributions haven't been directly to Bitcoin Core code to-date (although there is certainly code of mine passed through to it from me through others from long before I had any need for recognition for such things), there's 10,00
...

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2063932237)
> any reason why the verification flags don't also include SCRIPT_VERIFY_LOW_S?

I had been wondering about this as well. I think this flag should be included as the core wallet always produces low R/S values ECDSA signatures already ever since this change: #13666.

If I am not missing anything, might as well the wallet reject such PSBTs while unserializing?

📝 D33r-Gee opened a pull request: "interface: Expose load utxo snapshot functionality"
(https://github.com/bitcoin-core/gui/pull/869)
Expose load/activate AssumeUTXO snapshot functionaility so that it can be laoded through the GUI.

This can be tested and viewed in action in the qml repository with legacy code (https://github.com/bitcoin-core/gui-qml/pull/424)

💬 D33r-Gee commented on pull request "interface: Expose load utxo snapshot functionality":
(https://github.com/bitcoin-core/gui/pull/869#issuecomment-2835661664)
friendly ping @Sjors

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2063944522)
Also, I realise that it would be cool to pass a reference to the last argument to `CheckSignatureEncoding` that captures the error so that we can relay it forward to the user; the current error message (or its variation) can remain as a prefix.

🤔 jarolrod reviewed a pull request: "interface: Expose load utxo snapshot functionality"
(https://github.com/bitcoin-core/gui/pull/869#pullrequestreview-2799797924)
There's no consumer of this on the bitcoin-core/gui side, can you show this works with some sort of POC on the bitcoin-core/gui side?

How do we know this works

📝 stutxo opened a pull request: "add ignore_incoming_blocks option to ProcessMessage and SendMessages"
(https://github.com/bitcoin/bitcoin/pull/32366)

✅ stutxo closed a pull request: "add ignore_incoming_blocks option to ProcessMessage and SendMessages"
(https://github.com/bitcoin/bitcoin/pull/32366)

👍 hodlinator approved a pull request: "test: avoid stack overflow in `FindChallenges` via manual iteration"
(https://github.com/bitcoin/bitcoin/pull/32351#pullrequestreview-2799813040)
re-ACK 7e8ef959d0637ca5543ed33d3919937e0d053e70

nit: Would switch order of:
* e400ac53524d143467740e2f59698a7c94644c21 refactor: simplify repeated comparisons in `FindChallenges`
* f670836112c01feb3cb71618192e9c0c2e55767f test: remove old recursive `FindChallenges_recursive` implementation

This way CI should compare the `switch`-statement iterative version against the recursive one before it is removed. (Something I liked about what I last reviewed).

💬 hodlinator commented on pull request "test: avoid stack overflow in `FindChallenges` via manual iteration":
(https://github.com/bitcoin/bitcoin/pull/32351#discussion_r2063969451)
nit:
If you re-touch, it would be nicer to abide by the Sonar suggestion "...or a reference" with:
```suggestion
const auto challenges{FindChallenges(*node)}; // Find all challenges in the generated miniscript.
```
and
```C++
std::set<Challenge> FindChallenges(const Node& root)
```
to signify `nullptr` being unsupported.

💬 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
...