📝 stickies-v opened a pull request: "refactor: validation: mark CheckBlockIndex as const "
(https://github.com/bitcoin/bitcoin/pull/32364)
While reviewing another PR, I [noticed](https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2056509235) that `ChainstateManager::CheckBlockIndex()` is not a `const` method. To try and assert that this method was not causing any side-effects, I modified the method to make it `const`. It did not surface any errors, but I think it would be good to merge this change regardless, even if `CheckBlockIndex` is only used in regtest.

💬 ryanofsky commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2835521038)
I'm seeing a failure in this CI job that just says `*** 3 failures are detected in the test module "Bitcoin Core Test Suite"` but doesn't seem to give a more specific indication of where the failures are happening:

https://github.com/bitcoin/bitcoin/actions/runs/14670251419/job/41176416988?pr=32345#step:5:1701

Is this expected? Am I maybe missing some clues in the output? Could I maybe add some logging to my PR or to the CI job generally that would make this failure or other failures easie
...

💬 pinheadmz commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2835535199)
@wizkid057 your comment borders on off-topic, but I'm not going to hide it because at this point in the PR thread it is new information. This will be last comment allowed with this information though -- meaning all future comments talking about the end of decentralized money as we know it will be considered redundant brigadeing. Everyone else who agrees with you can 👍 your comment instead of rewriting it in their own words. I also encourage you to take longer arguments like this to the mailing
...

👍 darosior approved a pull request: "Remove arbitrary limits on OP_Return (datacarrier) outputs"
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2799422333)
ACK cd7872ca543d31d20f419fd2203138b8301c2e68

You missed a mention of `-datacarriersize` which i think should be deleted. Although `IsStandard` is already checked for these outputs in the unit tests, i think it is worth having a functional test which exercises both cases as well as tests the bounds (here hitting the maximum standard transaction size). Here is a diff which adds those cases:
<details>

<summary>Click to see diff</summary>

```diff
diff --git a/test/functional/mempool_accep
...

💬 darosior commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2063731685)
Could remove the documented requirement in `fill_mempool()` as well. https://github.com/bitcoin/bitcoin/blob/f409444d024340d58fb3a7d6c44329e7dbdb3857/test/functional/test_framework/mempool_util.py#L44

💬 fanquake commented on pull request "depends: Fix cross-compiling `qt` package from macOS to Windows":
(https://github.com/bitcoin/bitcoin/pull/32357#issuecomment-2835570768)
Guix Build:
```bash
8ba76371ec4b297da8d29674c7ec31a01eead8131a8e0bbf9bc7e66fb4e1adff guix-build-35e57fbe336c/output/aarch64-linux-gnu/SHA256SUMS.part
d16137ebd3ca5eabb9614a6d4c322e05c43727621d1b8eca98ef4321c5964f21 guix-build-35e57fbe336c/output/aarch64-linux-gnu/bitcoin-35e57fbe336c-aarch64-linux-gnu-debug.tar.gz
3f0078d7df0a54497a52541f2252a52d5b50737b96b83f88d8f2aecd76b54cca guix-build-35e57fbe336c/output/aarch64-linux-gnu/bitcoin-35e57fbe336c-aarch64-linux-gnu.tar.gz
778792dcea8b0e82
...

💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2835578794)
> Is this expected?

The GitHub CI log handling is horrible, because you can't use normal browser workflows to search for errors in the log. You'll have to download the raw log or use the GitHub log search feature. The string you are looking for is `error: in`.

This gives: https://github.com/bitcoin/bitcoin/actions/runs/14670251419/job/41176416988?pr=32345#step:5:1004

```
./test/net_tests.cpp(707): error: in "net_tests/LimitedAndReachable_Network": check g_reachable_nets.Contains(NET_ON
...

👍 fanquake approved a pull request: "depends: Fix cross-compiling `qt` package from macOS to Windows"
(https://github.com/bitcoin/bitcoin/pull/32357#pullrequestreview-2799684867)
ACK 35e57fbe336cdcb348ff088fc04216f1f5cf2742

✅ fanquake closed an issue: "build: Windows cross Qt broken on macOS host"
(https://github.com/bitcoin/bitcoin/issues/32346)

🚀 fanquake merged a pull request: "depends: Fix cross-compiling `qt` package from macOS to Windows"
(https://github.com/bitcoin/bitcoin/pull/32357)

📝 maflcko opened a pull request: " descriptors: Reject + sign while parsing unsigned"
(https://github.com/bitcoin/bitcoin/pull/32365)
As a follow-up to https://github.com/bitcoin/bitcoin/pull/30577, reject `+` for unsigned values in key-path parsing and multi threshold parsing as well.

Both of those are using unsigned, and Bitcoin Core would never serialize a descriptor string with a stray `+`. Accepting stray `+` signs could lead to checksum mismatches, or other incompatibilities later on.

Just like https://github.com/bitcoin/bitcoin/pull/30577, both changes are breaking changes on the RPC interface, but hopefully no on
...

💬 ryanofsky commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2835616043)
Thank you! Learned something new. I did try searching for error, but I guess I used the wrong search, and somehow I missed those lines when I was scrolling through manually. Glad to see the CI job is actually providing enough information and I just didn't understand the UI. Agree it could be nice if the script made the errors more obvious, but current behavior is also reasonably good.

💬 theuni commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2835618558)
> I'd appreciate it. It would be good to cross `m_nodes_mutex` off the [list](https://github.com/bitcoin/bitcoin/issues/19303).

Yeah, agreed. That's a good justification for the change to non-recursive here.

Looking at that diff, it's not clear to me if it's safe to move that hunk out of `cs_main`, but we can review/test/argue about it in a new PR.

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

Removing long time, policy level, end user config option choice is an extreme option.

Core shouldn't be in the business of compelling speech at a policy level from it's users unless it's interested in losing users, which is already happening for related reasons.

When you remove an end users choice to configure their policy you are doing harm to that user. Please don't harm end users by nixing these config options altogether.

Thank you for the work you all do.

💬 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