👍 TheCharlatan approved a pull request: "miner: timelock the coinbase to the mined block's height"
(https://github.com/bitcoin/bitcoin/pull/32155#pullrequestreview-2814660224)
Re-ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2850863863)
`fe2d1955f6...2c1a8ff0c0`: address suggestions

> AutoFile::write_buffer should also mark the file as dirty

Done.

> the current fclose returns a fake error code...

Replied to the discussion thread at https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2048792563

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2073366652)
Thanks! Added.

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2850870900)
> i've added a check for the application manifest to the symbol checker here: https://github.com/laanwj/bitcoin/tree/2025-05-manifest-check, feel free to cherry-pick.
>
> (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 #32380, it's really important for them to be in the release too)

Thanks! Cherry-picked.

💬 hebasto commented on pull request "build: replace header checks with `__has_include`":
(https://github.com/bitcoin/bitcoin/pull/32405#issuecomment-2850873641)
My Guix build:
```
aarch64
66d71c866bd111ffe65bc03b9e1653a95eb678f0b04451759c56af868bfc03d5 guix-build-e1f543823b30/output/aarch64-linux-gnu/SHA256SUMS.part
1a4130d801620a63d86c3069b1fbca39ebc963e610101451d3f48b1c191ca4b3 guix-build-e1f543823b30/output/aarch64-linux-gnu/bitcoin-e1f543823b30-aarch64-linux-gnu-debug.tar.gz
745adbc7767344a8cd0ebe1e7592239614d89f949558c9b6a2ae58f7b2602a32 guix-build-e1f543823b30/output/aarch64-linux-gnu/bitcoin-e1f543823b30-aarch64-linux-gnu.tar.gz
cb69d205
...

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2073373828)
The interface of `AutoFile::fclose()` is not changed by this PR and is not the purpose of this PR to change it. Thus I will leave it as it is.

I think that the interface of the current `AutoFile::fclose()` is just fine. It mimics 1:1 the libc [fclose(3)](https://www.man7.org/linux/man-pages/man3/fclose.3.html) which is in line with the majority of the other system functions which:

> return the value 0 if successful; otherwise the value -1 is returned and the global variable errno is set to
...

👍 hebasto approved a pull request: "build: replace header checks with `__has_include`"
(https://github.com/bitcoin/bitcoin/pull/32405#pullrequestreview-2814684426)
ACK e1f543823b300b28c9edaf5d1a3e1e9badde471b, I have reviewed the code and it looks OK.

💬 purpleKarrot commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#issuecomment-2850897602)
> This results in behaviour where every configuration step using `-DCMAKE_BUILD_TYPE=Release` with the default flags ends in an error, which is a terrible UX.

Then don't use `-DCMAKE_BUILD_TYPE=Release`. You get better UX with less typing.

For local development, just use the default build type. CI and package builds are done with scripts that allow enough ways to specify compile flags.

Distro packagers want to build all packages with the same compile flags. They will patch out all proje
...

👍 rkrux approved a pull request: "scripted-diff: adapt script error constant names in feature_taproot.py"
(https://github.com/bitcoin/bitcoin/pull/32415#pullrequestreview-2814701995)
tACK b5f580c580257d28d295cae3f787b55eb1863f16

Having gone through `feature_taproot.py` file previously that can take a while to understand fully, this seems like a reasonable change to make it easier to follow the file by making the error constant names congruent with the ones in the cpp code.

+1 on using the scripted diff so that the reviewers need not manually verify each change in the file.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2073385113)
Done

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2073386754)
I see. Done. I also moved the `m_was_written = true` to be just after the `fwrite()` calls.

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2850907232)
@laanwj

```
[12:43:30.811] contrib/devtools/symbol-check.py:308: error: No overload variant of "int" matches argument type "RESOURCE_TYPES" [call-overload]
[12:43:30.811] contrib/devtools/symbol-check.py:308: note: Possible overload variants:
[12:43:30.811] contrib/devtools/symbol-check.py:308: note: def __new__(cls, str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc = ..., /) -> int
[12:43:30.811] contrib/devtools/symbol-check.py:308: note: def __new__(cls, str | bytes
...

💬 Sjors commented on pull request "miner: don't needlessly append dummy OP_0 to bip34":
(https://github.com/bitcoin/bitcoin/pull/32420#issuecomment-2850943519)
I limited the exception to regtest. I also found two tests that for some reason implement their own coinbase construction code. I adjusted those for consistently with the first commit.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2850944121)
`2c1a8ff0c0...61ba3fc494`: fix CI

💬 ajtowns commented on pull request "miner: don't needlessly append dummy OP_0 to bip34":
(https://github.com/bitcoin/bitcoin/pull/32420#discussion_r2073418791)
Match the comment to the updated code too?

📝 hebasto converted_to_draft a pull request: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396)
Windows [application manifests ](https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests) provide several benefits—such as enhanced security settings, and the ability to set a process-wide code page (required for https://github.com/bitcoin/bitcoin/pull/32380), as well as granular control over supported Windows versions. Most of these benefits lie beyond the scope of this PR and will be evaluated separately.

On the current master branch @ fc6346dbc8dc3db40aad4079210332b5f8b
...

💬 Sjors commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2850994834)
@l0rinc ah, I did have an `addnode` with a local peer in my config file for this otherwise fresh node. Let me try again without that.

💬 laanwj commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2073451336)
Without the `int()` this doesn't work at all. Maybe just harcode `24`.

💬 l0rinc commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2851026294)
`write_buffer` was also marked as dirty in latest push.

untested ACK 61ba3fc494f2f5e60b668316c84cecbdc123e5fa

Q: shouldn't `Truncate` and `Commit` also set `m_was_written = true`?

💬 Sjors commented on pull request "miner: don't needlessly append dummy OP_0 to bip34":
(https://github.com/bitcoin/bitcoin/pull/32420#discussion_r2073461584)
Fixed