💬 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)
```

💬 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_r2089677991)
Maybe belt and suspenders check that the output belongs to the tx:
```suggestion
{
Assume(std::ranges::find(wtx.tx->vout, output) != wtx.tx->vout.end());
}
```

💬 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_r2089882326)
style nit:

Could be made a bit shorter with:
```suggestion
auto [it, inserted] = m_txos.emplace(COutPoint{wtx.GetHash(), i}, WalletTXO{wtx, txout, ismine});
if (!inserted) {
it->second.SetIsMine(ismine);
}
```

But maybe as-is is more legible.

💬 ajtowns commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2090119662)
Personally, I think it's better to think of "deprecated" as meaning just "the devs recommend you don't use this feature".

Despite recommending against using it, a feature might be left in the software ~forever, eg because it's needed for backwards compatability (see [`gets()`](https://en.cppreference.com/w/c/io/gets) in C, deprecated in 1999, removed from the standard in 2011, but still usable with current compilers and included in [glibc](https://www.man7.org/linux/man-pages/man3/gets.3.html
...

💬 maflcko commented on pull request "test: descriptor: cover invalid multi/multi_a cases":
(https://github.com/bitcoin/bitcoin/pull/32504#issuecomment-2882604121)
Nice, but two commits for adding three lines of related tests seems like it could be squashed?