💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2857767885)
Rebased after #32086.

💬 hebasto commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2857794327)
> Am I correctly understanding therefore that you are suggesting for the following changes as a fix for this?:
>
> 1. Remove `CMAKE_FIND_USE_CMAKE_ENVIRONMENT_PATH`

Building our own depends implies that we fully rely on them, ideally skipping all other search paths used by CMake's package, library, and header search mechanisms. Therefore, I believe we should retain as many restrictions as possible.

> 2. Explicitly set `CMAKE_PREFIX_PATH` using an if-guard, as per: [437a4da](https://gith
...

💬 l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#issuecomment-2857833155)
> I'll compare the reindex-chainstate performance next, before & after, let's see if there's a regression (we do expect a 4% slowdown for max cache, like in IBD).

IBD seems to be basically the same as before, this change indeed only affects `reindex-chainstate`.

<details>
<summary>Details</summary>

```bash
COMMITS="baa848b8d38187ce6b24a57cfadf1ea180209711 c822dd1f355b6f403191072cdd3c9c3e234bbcaa"; \
STOP_HEIGHT=888888; DBCACHE=45000; \
CC=gcc; CXX=g++; \
BASE_DIR="/mnt/my_storage";
...

💬 sipsorcery commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2857899480)
tACK b074ae1b4f52471a9d71e9431deebae5ec3d603e native Windows build.

```
c:\dev\github\bitcoin>mt.exe -nologo -inputresource:build\bin\Release\bitcoind.exe -validate_manifest
Parsing of manifest successful.
```

👍 l0rinc approved a pull request: "test: Add and use ElapseTime helper"
(https://github.com/bitcoin/bitcoin/pull/32430#pullrequestreview-2821125017)
Concept ACK - was thinking of something similar when reviewing https://github.com/bitcoin/bitcoin/pull/32414/commits/41479ed1d23ea752d0ce14c2cf5627f43bceb722#diff-0d33078447a7be03c4ca7c0c86e3f4b64f77afae719c7a353e86d688932bff3eR38

💬 maflcko commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#issuecomment-2857909160)
Sorry for the force-pushes, but this uncovered pre-existing issues in the fuzz tests that were not using mocktime during init. So I went ahead and fixed those as well in a separate commit. Should be good now, hopefully :sweat_smile:

💬 fanquake commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2077252899)
Why do we need to use a third-party repository, that runs some Javascript, to configure a Windows Command Prompt?

💬 fanquake commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2077259998)
Why do we need this check? Regex over a copy-pasted blob of XML is pretty meh. We aren't really checking anything important here either?

💬 laanwj commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2077268171)
Currently, no, but after #32380 it becomes critical because it determines the process code page.

> (not too important yet now as we're just adding some metadata, but when we add critical things in there like "use UTF-8 codepage" as in https://github.com/bitcoin/bitcoin/pull/32380, it's really important for them to be in the release too)

This is a quick and dirty check that the manifest is added and is what we expect, feel free to add detailed XML parsing.

💬 laanwj commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2077273311)
It also already found one bug in this PR: the manifest was not added to `test_bitcoin.exe` .

💬 fanquake commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2077274476)
I think for now we could just assert `binary.resources_manager.has_manifest`, if we care that this is being added? Could save anything else for if/when it's actually being used?

💬 laanwj commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2077279482)
i don't see the problem with having it right now like this, but okay.

💬 laanwj commented on pull request "lint: Remove string exclusion from locale check":
(https://github.com/bitcoin/bitcoin/pull/32434#issuecomment-2858004369)
> The exclusion isn't needed. In fact, it prevents detection of "bla" + wrong().

i wonder if the same is true for the comment exclusion. Would it detect `/* bla */ wrong()`.

💬 laanwj commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2077314447)
One thing i did think about is using the input file (`make/windows-app.manifest.in`) as template instead of hardcoding it here, but having the check depend on files scattered throughout the repository is probably not that great, either.

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2077317447)
Can be marked as resolved, thanks! I think we were talking past each other a bit but resolved offline.

⚠️ maflcko opened an issue: "intermittent issue in wallet_listsinceblock.py ( test_framework.test_node.FailedToStartError: [node 3] bitcoind exited with status -6 during initialization.)"
(https://github.com/bitcoin/bitcoin/issues/32435)
-6 seems to indicate an abort. However, the node did not print anything at all to the log, nor to the stderr:

https://github.com/bitcoin/bitcoin/actions/runs/14879952737/job/41785504218?pr=32430#step:7:3527

```
186/266 - wallet_listsinceblock.py failed, Duration: 1 s

stdout:
2025-05-07T09:47:50.041000Z TestFramework (INFO): PRNG seed is: 2493267510557761438
2025-05-07T09:47:50.044000Z TestFramework (INFO): Initializing test directory /Users/runner/work/bitcoin/bitcoin/ci/scratch/test_runner/t
...

✅ maflcko closed an issue: "intermittent issue in wallet_listsinceblock.py ( test_framework.test_node.FailedToStartError: [node 3] bitcoind exited with status -6 during initialization.)"
(https://github.com/bitcoin/bitcoin/issues/32435)

💬 maflcko commented on issue "intermittent issue in wallet_listsinceblock.py ( test_framework.test_node.FailedToStartError: [node 3] bitcoind exited with status -6 during initialization.)":
(https://github.com/bitcoin/bitcoin/issues/32435#issuecomment-2858030829)
Not sure how to debug this further, so closing for now. However, further input is still very welcome.

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2858043588)
Re https://github.com/bitcoin/bitcoin/pull/32317#pullrequestreview-2785006540

> nit: The name SpendBlock may not be clear (unless this concept is already known and I'm missing the point).
Maybe BIP30Validation, something like that, would be clearer.


It is doing more than BIP-30 validation, so I don't think that would accurate either. I thought the name was pretty clear for what it does, but I'd be open to other suggestions.

📝 theStack opened a pull request: "test: refactor: negate signature-s using libsecp256k1"
(https://github.com/bitcoin/bitcoin/pull/32436)
This small PR gets rid of manual mod-n inversion of the ECDSA signature-s part in unit tests (introduced a long time ago in #5256, triggered by https://github.com/bitcoin-core/secp256k1/pull/69) by using secp256k1 instead. The function wasn't available at that time, but was introduced about three years later, see https://github.com/bitcoin-core/secp256k1/pull/408. Note that as the name suggests, `secp256k1_ec_seckey_negate` is meant to be used for secret keys, but it obviously works in general f
...