👍 laanwj approved a pull request: "crypto: disable ASan for sha256_sse4 with Clang"
(https://github.com/bitcoin/bitcoin/pull/32437#pullrequestreview-2821466599)
Code review ACK 4e8ab5e00fa72016a7ec0e0505ca025d4e59e4d8
Have not tested.

🤔 rkrux reviewed a pull request: "Remove the legacy wallet and BDB dependency"
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2821415265)
Couple of points in 8ede6dea0c55bb4afefa790b83dd4da48a2f84da, can be done in follow up if no more changes being in this PR.

💬 rkrux commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2077427167)
Besides the typo `s/an/a` above, the comment "Import a private key" doesn't go well with `rescanblockchain`.

💬 rkrux commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2077459615)
In #31243, `IsSingleKey` was added with a TODO to remove it after removing legacy wallets.
I checked that no other usage of `IsSingleKey` is left, it can be removed.

https://github.com/bitcoin/bitcoin/blob/6d5edfcc585bab3374ae14aa7918b3e178e016aa/src/script/descriptor.h#L114-L117

💬 instagibbs commented on pull request "test: refactor: overhaul (w)txid determination for `CTransaction` objects":
(https://github.com/bitcoin/bitcoin/pull/32421#issuecomment-2858353344)
concept ACK either way; we lose mental cycles every time we need to figure this out

💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2858366754)
re-ACK de054df6dc32cbd8b127c6761d9c65d95025e08 🔗

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK de054df6dc32cbd8b127c
...

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2077497713)
Done.

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2077498609)
Done and also used `<` and `>` for the other fields.

💬 1440000bytes commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2858379376)
> but not allowing a node runner to configure limits on their mempool to make it feasible to run a node (in terms of RAM) goes against our core value of making nodes easy to run for personal use

https://github.com/bitcoin/bitcoin/pull/32406

💬 maflcko commented on pull request "crypto: disable ASan for sha256_sse4 with Clang":
(https://github.com/bitcoin/bitcoin/pull/32437#issuecomment-2858381159)
lgtm ACK 4e8ab5e00fa72016a7ec0e0505ca025d4e59e4d8

I haven't tested this, but the diff looks plausible

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#issuecomment-2858386981)
`6a9f04688f...2059eaa692`: address suggestions

> If the user sets `-proxy=addr:port=tor` then `-onion=` shouldn't be allowed, no? Otherwise one could set 2 diff proxies for Tor(?).

I think that is ok, assuming the expectation is that later options override earlier ones. For example, without this PR, `-proxy=x -onion=y` is supposed to set the Tor proxy to `y`, overriding the earlier `x`. Updated `tor.md` to mention that.

> So, shouldn't `-proxy=0=cjdns` be the default if `-cjdnsreachable
...

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2077521444)
What alternatives are you suggesting for configuring the Developer Command Prompt?

FWIW, we've been using this action since https://github.com/bitcoin/bitcoin/pull/28173.

💬 pinheadmz commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2858416006)
> > but not allowing a node runner to configure limits on their mempool to make it feasible to run a node (in terms of RAM) goes against our core value of making nodes easy to run for personal use
>
>
>
> https://github.com/bitcoin/bitcoin/pull/32406

There's also `-maxmempool` specifically for this. And if you still have memory problems running a full node open an issue.

💬 Sjors commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2858435213)
ACK 8b73630c6fe64158bf123da5e22bd0a71ab305c9.

> Windows 11 against HWI 3.1.0 on testnet4.
>
> It crashes immediately after creating a new wallet from a Trezor model T.

This still happens, but it seems only on testnet4, e.g. not on mainnet and not on signet (didn't try testnet3). It also happens on macOS 15.4.1, so it's clearly unrelated (will open an issue).

On Windows 11 I was able to create a new wallet with the GUI, verify an address and make a transaction.

I did not test handl
...

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2077537805)
done

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2077538094)
done

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2077538389)
done

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2858452233)
@Sjors

> ACK [8b73630](https://github.com/bitcoin/bitcoin/commit/8b73630c6fe64158bf123da5e22bd0a71ab305c9).

Thank you for your review!

Could you please double-check the commit hash in your comment?

💬 fanquake commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2077545934)
> What alternatives are you suggesting for configuring the Developer Command Prompt?

Directly running the command that somebody who owns a Windows computer would run, that doesn't require installing NPM, and then executing somebody else's JavaScript.

💬 Sjors commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2858457623)
@hebasto fixed, that was from a different PR, but I checked that I used the right commit for my Windows guix build.