💬 skylinesales commented on pull request "wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet":
(https://github.com/bitcoin/bitcoin/pull/31423#discussion_r2181075955)
src/qt/walletcontroller.cpp

💬 davidgumberg commented on pull request "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32859#issuecomment-3029569601)
utACK https://github.com/bitcoin/bitcoin/commit/f0524cda3995cf65adab3d0ca8da0dee4e31c79b

💬 achow101 commented on pull request "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32859#issuecomment-3029573887)
ACK f0524cda3995cf65adab3d0ca8da0dee4e31c79b

💬 pinheadmz commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3029587757)
https://github.com/bitcoin/bitcoin/blob/a92e8b10a5fb1ac25484aa17787e27b4a8632f69/src/wallet/rpc/encrypt.cpp#L103-L104

🤦 yup clear as day thanks

🚀 achow101 merged a pull request: "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32859)

💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3029592937)
Rebased because of spurious CI failure fixed by #32859.

🤔 pablomartin4btc reviewed a pull request: "test: check P2SH sigop count for coinbase tx"
(https://github.com/bitcoin/bitcoin/pull/32850#pullrequestreview-2980935709)
ACK d6aaffcb11adcf47480fcc5081af9dcb732decf3

(coinbase transactions don’t refer to a real previous output and thus cannot be spending P2SH outputs, making the sigop count irrelevant for them).

🤔 marcofleon reviewed a pull request: "fuzz: Make process_message(s) more deterministic"
(https://github.com/bitcoin/bitcoin/pull/32822#pullrequestreview-2980950526)
tACK fac673d434b4d32d8c44dcc50a3655ba4a1816de

Running the stability script, there's significantly less coverage differences with this PR. My only question is why does `ResetChainman` set mock time in `process_messages` but not in `process_message`? Didn't see an obvious reason, but I could be missing something.

💬 furszy commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2181163397)
that was actually done on purpose. See https://leimao.github.io/blog/Python-Default-Argument-Mutable-Object/.

💬 furszy commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2181167049)
ups, reverting. Thanks.

💬 furszy commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2181173483)
Thanks, but I'm about a ~0 here. I don't really find that change better. I'll just leave the current code unless someone feels strongly about changing it

💬 furszy commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2181177845)
This is actually just a leftover from debugging the issue in #26966.. sorry for that. Will just remove the line.

💬 furszy commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2181170410)
> It might be good to send in the same startup_args to check_clean_start() further down in the loop?

I don't think so. All files are restored from the backup after deletion, so we know for sure the indexes are synced at that point (nothing changed).

💬 furszy commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#issuecomment-3029743647)
Updated per feedback. Thanks @hodlinator

💬 furszy commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2181204274)
We are already doing the backup file existence check at the end of `migrate_and_get_rpc`, what if you include this extra check there too?

💬 w0xlt commented on pull request "wallet: Track no-longer-spendable TXOs separately":
(https://github.com/bitcoin/bitcoin/pull/27865#discussion_r2181221787)
Why is it optional instead of false by default?

💬 pablomartin4btc commented on pull request "wallet, test: best block locator matches scan state follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32580#discussion_r2181674862)
nit: I haven't seen the format I'm proposing here but just to make it more clear if it makes sense...
```suggestion
self.log.info(
"Test that on a duplicate block disconnection event after unclean shutdown:\n"
" - wallet transactions are un-abandoned after temporarily invalidated blocks;\n"
" - wallet doesn't crash."
)
```

🤔 pablomartin4btc reviewed a pull request: "wallet, test: best block locator matches scan state follow-ups"
(https://github.com/bitcoin/bitcoin/pull/32580#pullrequestreview-2981629314)
cr-ACK 1b5c545e82fe3cf5027f16b43e2306aeb8d4ef9b

Reviewed #30221 to verify the agreed follow-ups:
- In `wallet.cpp`:
- another opportunity to use `setLastBlockProcessedInMem` - https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2090917963
- `(!loc.IsNull())` - https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2096279905
- In `wallet_reorgsrestore.py`:
- update test log - https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2091417700
- assert rescanning - http
...

👍 rkrux approved a pull request: "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs"
(https://github.com/bitcoin/bitcoin/pull/32618#pullrequestreview-2981864736)
ACK b1a8ac07e91dd1d305fcbc16ea931d60e46c0055

💬 rkrux commented on pull request "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs":
(https://github.com/bitcoin/bitcoin/pull/32618#discussion_r2181951390)
Got it. Generally how long do we wait to remove a field from the code after marking it deprecated?