🚀 ryanofsky merged a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660)

👍 ryanofsky approved a pull request: "doc: warn that CheckBlock() underestimates sigops"
(https://github.com/bitcoin/bitcoin/pull/31624#pullrequestreview-2826389039)
Code review ACK a04f17a1882407db09b0a07338e12877ac1d9e92

Thanks for digging up the history! At least it's clear how the check get here, though not exactly what it was trying to do.

💬 ryanofsky commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2080480454)
In commit "doc: warn that CheckBlock() underestimates sigops" (a04f17a1882407db09b0a07338e12877ac1d9e92)

This comment is good to have but would be nice to clarify:

1. Why it is not a problem to underestimate?
2. What this check accomplishes if isn't accurate?
3. If it might be better to remove the check or update it?

Might suggest expanding comment to something like "This check underestimates the number of sigops because it does not count witness and p2sh sigops. This is not a problem
...

🤔 w0xlt reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2826415285)
reACK https://github.com/bitcoin/bitcoin/pull/27286/commits/51d4846f648e79e3e098f1f1bea7d5e823ebdea8

🤔 w0xlt reviewed a pull request: "test: refactor: negate signature-s using libsecp256k1"
(https://github.com/bitcoin/bitcoin/pull/32436#pullrequestreview-2826428063)
ACK https://github.com/bitcoin/bitcoin/pull/32436/commits/1ee698fde2e8652d8eb083749edb8029c003b8db

💬 w0xlt commented on pull request "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`":
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-2864385970)
Thanks for the review and suggestion @musaHaruna . Added that text to the PR's description.

🤔 pablomartin4btc reviewed a pull request: "contrib: remove bdb exception from FORTIFY check"
(https://github.com/bitcoin/bitcoin/pull/32448#pullrequestreview-2826445260)
cr ACK f9dfe8d5e0d3f628659702ab781b7919505c829f

FWIW: Tried to run the check on Ubuntu and it seems some lief symbols have [changed](https://github.com/lief-project/LIEF/blob/main/doc/sphinx/changelog.rst) previously to the latest released version (< 0.16.5 - eg `lief.EXE_FORMATS.ELF` => `lief.ELF.Binary`), corrected it locally and ran succesfully, then saw that the CI has 0.13.2, this works well too.

💬 murchandamus commented on pull request "coinselection: Optimize BnB exploration":
(https://github.com/bitcoin/bitcoin/pull/32150#issuecomment-2864494772)
The preceding PR https://github.com/bitcoin/bitcoin/pull/29532 got merged, so this is now ready for review.

👍 hodlinator approved a pull request: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396#pullrequestreview-2826464805)
ACK 477b69ad19d7fe37de9ca23926bcf007b309876d

### Concept

Paves the way for more intentional Windows manifest usage. [Linked documentation](https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests) confirms that it can be used to control codepage and specify supported Windows versions.

### Approach

Not something I could have easily written from scratch but makes logical sense in how the new CMake function `add_windows_application_manifest` generates a manifest file
...

📝 achow101 opened a pull request: "test: Remove legacy wallet RPC overloads"
(https://github.com/bitcoin/bitcoin/pull/32452)
`RPCOverloadWrapper` implemented overloads for legacy wallet only RPCs so that the same function call could be used within tests for both legacy wallets and descriptor wallets. With legacy wallets now removed, there is no need to continue to have these overloads.

For `importaddress`, `addmultisigaddress`, and `importpubkey`, the uses of these are converted to `importdescriptors`.

For `importprivkey`, a new helper function `wallet_imporprivkey` is introduced that does what the overload did.
...

📝 JeremyRubin opened a pull request: "[Policy] Discourage Unsigned Annexes"
(https://github.com/bitcoin/bitcoin/pull/32453)
This patch adds a "redundant" rule with the annex discouragement, to discourage Annexes that are present but do not get signed.

This PR does not include tests, as the code paths are entirely redundant (and small!) with existing annex discouragement. If desired, tests could be made.

This new policy would come into play if someone somewhere for some reason decided that they were going to modify standard policy on their node to allow annexes **even though it is abundantly clear that annex pol
...

💬 achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2080598493)
I could not figure out the CI failure, so I've reverted this change.

💬 achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2864638615)
> Simplest fix would probably be to make the CWallet object a static variable in the function like the TestingSetup variable is, instead of a global variable.

It can't be static because the `FUZZ_TARGET` below needs to access it. However, the wallet could be created in there instead of in the setup, as the other wallet fuzz tests do.

But, with moving `WriteBestBlock` back to `RemoveWallet`, this is not necessary either.

👍 theStack approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2826561409)
Concept and code-review ACK 8c360d78b13fa7136db8d6e1e0af5d05f5141a64

💬 theStack commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2080597980)
Seeing this constant also tricked me into thinking that the script size limit might play a role here. Agree that a comment would be nice, or maybe just bump it up to something much higher like e.g. 50000.

💬 achow101 commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#issuecomment-2864640606)
I think we could have a test for this in `wallet_backwards_compatibility.py`.

🤔 w0xlt reviewed a pull request: "fuzz: doc: add info about `afl-system-config` for macOS"
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2826580160)
ACK https://github.com/bitcoin/bitcoin/pull/32175/commits/61ea5f348da71b886807c0492587835dd7e57499

Good clarification.

💬 rot13maxi commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2864661542)
Im surprised we haven’t seen any weird annex-use yet. This seems like a good safety policy to have in here until the annex’s use gets consensus. Could someone remove this? Sure, but at least theres sort of a warning sign that they might not want to.

🤔 w0xlt reviewed a pull request: "build: let CMake determine the year"
(https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2826582868)
ACK https://github.com/bitcoin/bitcoin/pull/32445/files

💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2864794113)
I definitely opened this PR prematurely, thinking I could get away with just bumping the version and fixing the python scripts 😂.

Working on getting guix builds working right, I referenced prior art by @willcl-ark here (https://github.com/willcl-ark/bitcoin/commit/ede9aa93fe6339fe6285c954dc8fbd9ae8623916).

The python build is convinced that a `build` option is set in `config-settings`, even when I override `configure-flags`, which is where `config-settings` is supposed to get it's value f
...