💬 maflcko commented on pull request "feature_taproot: sample tx version border values more":
(https://github.com/bitcoin/bitcoin/pull/32841#discussion_r2180240391)
i guess it is not enough and the truc rules need to be checked as well. Otherwise:

```
[10:00:08.436] �[0;36m test_framework.authproxy.JSONRPCException: TRUC-violation, version=3 tx 18cf284e19b7df3927bb95ea6a15d7d710a180bb62ccf18ad05948915c85171f (wtxid=3a4704c5da033ee18e46813915c5bad5747277e40f9684689bb66ed91ac85256) is too big: 25250 > 10000 virtual bytes (-26)�[0m
```

https://cirrus-ci.com/task/6341200170450944?logs=ci#L2708

🤔 rkrux reviewed a pull request: "wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet"
(https://github.com/bitcoin/bitcoin/pull/31423#pullrequestreview-2979454191)
LGTM ACK b78990734621b8fe46c68a6e7edaf1fbd2f7d351

I verified from `MigrateToDescriptor` that `data.desc_spkms` can contain only spendable SPKMs as the watch-only ones are filtered out in `data.watch_descs` when the descriptor can't be converted to a private string. Rest two cases of HDChains and NonHD-Keys push SPKMs in `data.desc_spkms` and they both have access to the corresponding key material. So, I believe the following condition seems sufficient to check for spendable material.

```cp
...

💬 instagibbs commented on pull request "feature_taproot: sample tx version border values more":
(https://github.com/bitcoin/bitcoin/pull/32841#discussion_r2180249602)
There are no in-mempool deps afaik, so checking the vsize of `tx` should suffice?

👍 theStack approved a pull request: "test: fix an incorrect `feature_fee_estimation.py` subtest"
(https://github.com/bitcoin/bitcoin/pull/32463#pullrequestreview-2979460112)
Light ACK 9b75cfda4d62a0a3bde402503244dd57e1621a12

I'm not familiar with the inner workings of our fee estimation code, but only considering how the "feerate" result is constructed in the `estimatesmartfee` RPC, the changes look good to me. Increasing the max block weight params by 4000 to compensate for https://github.com/bitcoin/bitcoin/pull/31384 also makes sense.

> Nice work finding the issue. For some reason, I couldn't reproduce using `--random=3450808900320758527`. But `--random=413
...

📝 hebasto opened a pull request: "doc: Add workaround for vcpkg issue with paths with embedded spaces"
(https://github.com/bitcoin/bitcoin/pull/32858)
This PR is an alternative to https://github.com/bitcoin/bitcoin/pull/32397 suggested in https://github.com/bitcoin/bitcoin/pull/32397#issuecomment-2883438040.

💬 hebasto commented on pull request "doc: Add hint about avoiding spaces in paths when building on Windows":
(https://github.com/bitcoin/bitcoin/pull/32397#issuecomment-3028163852)
> Closing for now. @hebasto maybe you can followup with your own suggestion.

Sure. https://github.com/bitcoin/bitcoin/pull/32858.

💬 pinheadmz commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3028164813)
@achow101 I started working on calling `WalletContext.scheduler` directly from `walletpassphrase()` but there is one nice extra feature we get from using `RPCRunLater` and `RPCTimerBase` which is that timers are mapped by name and can be removed by name if, for example, the user calls the RPC again with a different timeout value. This might result in a change of behavior where the shorter timer will still lock the wallet, and the longer timer will sit in the queue and eventually do nothing. Curr
...

💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2180262495)
actually ended up being pretty easy to remove for a time-based timelock

```
diff --git a/test/functional/mempool_reorg.py b/test/functional/mempool_reorg.py
index 3906117e8b..0f6124f9ab 100755
--- a/test/functional/mempool_reorg.py
+++ b/test/functional/mempool_reorg.py
@@ -28,5 +28,6 @@ from test_framework.blocktools import (

# Number of blocks to create in temporary blockchain branch for reorg testing
-FORK_LENGTH = 10
+# Needs to be long enough to allow MTP to move arbitrarily
...

🤔 ryanofsky reviewed a pull request: "cmake: Move internal binaries from bin/ to libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2979400065)
Thanks for the reviews!

Updated d253fa9ebe4305118417a02aa72cc06810662ac6 -> 705791cd436f237fe9bbac2cf52d63ab4b2a41c7 ([`pr/libexec.6`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.6) -> [`pr/libexec.7`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.7), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/libexec.6..pr/libexec.7)) just fixing typos pointed out in reviews.

---

re: TheCharlatan https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2807539249
...

💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2180213722)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2124090430

Thanks, fixed

💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2180213943)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2120285755

Thanks, fixed

💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2180213170)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2127330034

Rewrote the PR description to hopefully make this clearer, but this change is just adding a missing install rule. If you don't go out of your way to enable the BUILD_UTIL_CHAINSTATE option, the binary won't be installed, but if you do enable it, it will be installed. This is consistent with how we treat other build options like BUILD_GUI_TESTS with test_bitcoin-qt and BUILD_BENCH with bench_bitcoin.


but it doesn't
...

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2180297022)
I'm aware. "Where" refers to the scripts. This is in opposition to the current sigops counting which, as you know, counts sigops in unexecuted scripts.

I think the wording is clear as such, but if you have a better one please suggest it.

💬 luke-jr commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2180298687)
```suggestion
// Unlike the existing block wide sigop limit which counts sigops present in the block itself (including the scriptPubKey which is not executed until spending later), BIP54 counts sigops in the block where they are potentially executed (only).
```

📝 instagibbs opened a pull request: "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32859)
Resolves https://github.com/bitcoin/bitcoin/pull/32841#discussion_r2180240391

💬 instagibbs commented on pull request "feature_taproot: sample tx version border values more":
(https://github.com/bitcoin/bitcoin/pull/32841#discussion_r2180310249)
opened potential resolution at https://github.com/bitcoin/bitcoin/pull/32859

was able to reproduce the issue and not run into it anymore

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2180310701)
Taken, thanks.

👍 maflcko approved a pull request: "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32859#pullrequestreview-2979572797)
lgtm

💬 maflcko commented on pull request "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32859#discussion_r2180318357)
```suggestion
and not (tx.version == 3 and tx.get_vsize() > TRUC_MAX_VSIZE) # Topological standardness rules must be followed
)
```

nit: Could move the `)` on the next line, to avoid having to touch the line again next time a rule is added below?

💬 instagibbs commented on pull request "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32859#discussion_r2180355177)
taken, thanks