💬 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?

💬 davidgumberg commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2882622785)
Rebased to fix merge conflict.

👍 hebasto approved a pull request: "doc: remove Carls substitute server from Guix docs"
(https://github.com/bitcoin/bitcoin/pull/32498#pullrequestreview-2842400855)
ACK 3b824169c7766460794bb445028ac55a7111ee3e, the listed substitute servers are the same as in https://guix.gnu.org/manual/en/html_node/Official-Substitute-Servers.html.

💬 v479038280 commented on issue "Anticipation quantum attack ; post quantum security":
(https://github.com/bitcoin/bitcoin/issues/31799#issuecomment-2882696213)
mark

💬 i-am-yuvi commented on pull request "init: drop `-upnp`":
(https://github.com/bitcoin/bitcoin/pull/32500#issuecomment-2882719488)
ACK 301993ebf7f8ec23050e91377e0fd05823bb372a

🤔 i-am-yuvi reviewed a pull request: "build: Fix `macdeployqtplus` after switching to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/32287#pullrequestreview-2842440076)
re-ACK 84de8c93e7d4979575161a2bb8f7eb64e1317b89