👍 willcl-ark approved a pull request: "test: add skip_if_running_under_valgrind()"
(https://github.com/bitcoin/bitcoin/pull/32492#pullrequestreview-2841515552)
ACK 75a185ea3db3177e8e479ee61a39bcb51e08d9a6

Very lightly tested. Code changes look correct to me too.

<details>
<summary>Details</summary>

```bash
bitcoin on  functional_tracepoints_skip_valgrind:refs/pull/32492/head [$!] via △ v3.30.5 via 🐍 v3.12.7 via ❄️ impure (nix-shell-env) took 6s
❯ build/test/functional/interface_usdt_coinselection.py --valgrind
2025-05-14T21:03:58.758000Z TestFramework (INFO): PRNG seed is: 5177280005936620294
2025-05-14T21:03:58.758000Z TestFramework (
...

💬 hebasto commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2881576093)
> utACK 677943032785253ab268e51c9d37fbacc1483568.
>
>
>
> 17.14 is still in preview so can't test yet (would rather avoid the >10GB download and install for the sake of a week or two).

From https://learn.microsoft.com/en-us/visualstudio/releases/2022/release-notes:

> 17.14 ... was released on May 13, 2025.

💬 ismaelsadeeq commented on pull request "RPC: removeprunedfunds should take an array of txids":
(https://github.com/bitcoin/bitcoin/pull/32501#issuecomment-2881584536)
@BrandonOdiwuor
I don't think it's productive to open a draft PR without first addressing the comments.
If you'd like to continue working on this but are facing blockers, it would be better to communicate your approach and explain where you're stuck in the tracking issue.

I suggest closing this for now and reopening it once the concerns raised about the previous approach have been addressed.

💬 achow101 commented on pull request "refactor: Remove UB in prevector reverse iterators":
(https://github.com/bitcoin/bitcoin/pull/32490#issuecomment-2881607692)
ACK faf9082a5f689e2e51a474bf654e4e9b6ca29685

Checked dead code removal, did not test for UB.

💬 sipsorcery commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2881609695)
Just checked again (1 hour later) and installing 17.14 now.

Seems it was just my VS Installer instance being crap and telling me there were no new updates and offering 17.14 as a preview.

🚀 achow101 merged a pull request: "refactor: Remove UB in prevector reverse iterators"
(https://github.com/bitcoin/bitcoin/pull/32490)

💬 jonatack commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2089805607)
Concept ACK (people have been asking me about this deprecation warning for at least 6-7 years)

💬 achow101 commented on pull request "wallet, refactor: Remove Legacy wallet unused warnings and errors":
(https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2089806374)
In 5111176aec91396320c7a337c0613389c1a5f4c0 "wallet, refactor: Remove Legacy warnings and errors"

This `if` is not needed anymore with the above `Assert`.

💬 achow101 commented on pull request "doc: remove Carls substitute server from Guix docs":
(https://github.com/bitcoin/bitcoin/pull/32498#issuecomment-2881677010)
ACK 3b824169c7766460794bb445028ac55a7111ee3e

Carl's substitute server is indeed down, IIRC I ran into this a couple months ago.

The replacement matches the Guix defaults.

💬 achow101 commented on pull request "wallet: Drop unused fFromMe from CWalletTx":
(https://github.com/bitcoin/bitcoin/pull/32502#issuecomment-2881681061)
ACK 5bf91ba8800d23402536d758f02198eac0fd7d61

🚀 achow101 merged a pull request: "wallet: Drop unused fFromMe from CWalletTx"
(https://github.com/bitcoin/bitcoin/pull/32502)

💬 D33r-Gee commented on pull request "[DRAFT] Expose AssumeUTXO Load Snapshot Functionality To The GUI":
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-2881745240)
> Seems like this would be more appropriate for the first-run intro page?

yep that's a great idea! I will update soon.

📝 theStack opened a pull request: "depends: bump to latest config.guess and config.sub"
(https://github.com/bitcoin/bitcoin/pull/32505)
Noticed that these files were last updated from [upstream](https://git.savannah.gnu.org/gitweb/?p=config.git) quite a while ago (in 2023, see #28781), so bump them again.

Can be verified via e.g.
```
$ git clone https://git.savannah.gnu.org/git/config.git /tmp/config.git
$ diff /tmp/config.git/config.guess ./depends/config.guess
$ diff /tmp/config.git/config.sub ./depends/config.sub
```

💬 achow101 commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2881791126)
ACK a0eed55398f882d9390e50582b10272d18f2b836

Tested manually with a modified mock signer. Prior to this change, several fds exit referring to various files and sockets opened by the node. With this PR, fds 0, 1, and 2 remain open as pipes, and the rest are for files opened by the mock signer itself.

🚀 achow101 merged a pull request: "common: Close non-std fds before exec in RunCommandJSON"
(https://github.com/bitcoin/bitcoin/pull/32343)

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

💬 murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2089924798)
In "wallet: Recalculate the wallet's txos after any imports" (850de8343e36a17a5c875b1be974153bf06efaca):
I’m surprised that the prior commit did not have failing tests if the addition of `RefreshAllTXOs()` is necessary here. Is there maybe test coverage missing here?

💬 murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2089928193)
I think it would make more sense to me if "wallet: Change balance calculation to use m_txos" (ff73562fb5df406da95d6a6327e3d22b8d3e12f2) came after "test: Test for balance update due to untracked output becoming spendable" (f062a438620625bfa0d20022e76cad1705587737).

First the fix, then the test, then the refactor passes the test.

💬 murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2082398496)
In wallet: Keep track of transaction outputs owned by the wallet (e82763903b5e0723abf09cb46874953c7b523d24):
I’m also not excited about "TxTXOs". `RefreshTXOsFromSingleTx(…)` would be just slightly longer and easier to parse. Alternatively `CacheOwnTXOsFromTx(…)`?

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2089860593)
millinit: Maybe the function signature is sufficient to communicate that it's just for a single TX?

```suggestion
void CWallet::RefreshTXOs(const CWalletTx& wtx)
```