🤔 w0xlt reviewed a pull request: "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32859#pullrequestreview-2979983814)
ACK https://github.com/bitcoin/bitcoin/pull/32859/commits/f0524cda3995cf65adab3d0ca8da0dee4e31c79b

💬 tnndbtc commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3028740646)
Just for document purpose, I profiled `bitcoind` for IBD process on Mac through `gperftool`, and have chatgpt to help analyze out.txt (attached) and following is the summary:

```
LevelDB I/O & compaction ~1500s+ ~30% Block/UTXO writes, frequent flushes
leveldb::DecodeFixed32 — 241.66s (4.93%)
leveldb::DBImpl::DoCompactionWork — 1430.83s (29.2%)
leveldb::TableBuilder::Add — 1276.91s (26.06%)
leveldb::TableBuilder::Flush — 1261.79s (25.75%)

This shows:
Heavy disk write and
...

👍 pinheadmz approved a pull request: "test: allow all functional tests to be run or skipped with --usecli"
(https://github.com/bitcoin/bitcoin/pull/32290#pullrequestreview-2980079242)
re-ACK 666016e56b28b77f798dc85c767b95c1ca0abfae

Built and ran all functional tests with --usecli on macos/arm64. Reviewed all code changes again since it's been a while since last review. Coverage is really great.

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 666016e56b28b77f798dc85c767b95c1ca0abfae
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmhlbmsACgkQ5+KYS2KJ
yTp9HxAAmFHGtNyp2QfIz2EERL9n8BI/
...

💬 tnndbtc commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3028803853)
@GregTonoski you can email me with my <git user name> @ gmail.com . Then we can schedule a zoom meeting to discuss further.

💬 tnndbtc commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#issuecomment-3028819515)
@fanquake Any other thoughts on how to fix this issue, like smaller fixed data set? Currently this test is flawed as it uses random fees and missed the fact that, the randomness will go through different execution path thus the random test failure.

💬 achow101 commented on pull request "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs":
(https://github.com/bitcoin/bitcoin/pull/32618#discussion_r2180720302)
It's already false for existing descriptor wallets.

The purpose of marking it deprecated is so that it can be removed in the future. Otherwise, it may be surprising to users that this field suddenly disappeared.

💬 Sjors commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3028935370)
Shoehorning this into `SigningProvider` got ugly real fast. So instead I endup up passing `avoid_script_path` along the call stack from `SignPSBTInput` to `SignTaproot`. Which is also ugly...

Well, it compiles and works. Thoughts on how to implement this elegantly?

🤔 glozow reviewed a pull request: "cluster mempool: add TxGraph reorg functionality"
(https://github.com/bitcoin/bitcoin/pull/31553#pullrequestreview-2979497055)
ACK 2b2df98747fdb6380588991167ce2e8cb92f3bfb

Reviewed the code and the tests. I ran the bench to see that we spend a lot longer on the last commit since we are able to consider more potential clusters (almost 2x the time).

💬 glozow commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2180272606)
nit in 2b2df98747f
```suggestion
// We only need to trim the middle bottom transaction to end up with 2 clusters each within cluster limits.
```

💬 glozow commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2180717236)
I tested that `Trim()` always removes oversized singletons, even if there are no clusters to be merged, using a simple unit test

```
diff --git a/src/test/txgraph_tests.cpp b/src/test/txgraph_tests.cpp
index 9f9ad04ff37..d90afd8819d 100644
--- a/src/test/txgraph_tests.cpp
+++ b/src/test/txgraph_tests.cpp
@@ -248,4 +248,43 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_huge)
BOOST_CHECK(graph->GetTransactionCount() >= (NUM_TOP_CHAINS * NUM_TX_PER_TOP_CHAIN * 99) / 100);
}

+BOOST_AUTO_TE
...

💬 GregTonoski commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3029029658)
> @GregTonoski you can email me with my \<git user name> @ gmail.com . Then we can schedule a zoom meeting to discuss further.

I don't want to talk in private. Let's put all the information in public and solve it together with anybody interested in.

👍 hodlinator approved a pull request: "cmake: Move internal binaries from bin/ to libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2980338061)
ACK 705791cd436f237fe9bbac2cf52d63ab4b2a41c7

Fixed typos and improved PR description since my first (concept & test) review (https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2892807188).

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2180801973)
Great, incorporated. I'm assuming the txgraph.cpp changes above are to demonstrate the test works?

🤔 janb84 reviewed a pull request: "cmake: Move internal binaries from bin/ to libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2980359650)
re ACK 705791cd436f237fe9bbac2cf52d63ab4b2a41c7

Changes sinds last review:

- Fixed typos
- Changed PR description (thanks! )

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2180806848)
Done.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-3029056900)
Addressed nits, and also incorporated the benchmark in this PR.

💬 mzumsande commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2180843713)
Ah, I missed this - then it should be fine!

💬 mzumsande commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2180834640)
I still think it would be sufficient to only set it for the tip - I don't think that the edge case of `invalidateblock` justifies doing another iteration over all blocks during each init (it takes 0.1 - 0.2s on mainnet for me on a reasonably fast laptop, which is not that much, but I would imagine it might be quite a bit longer with slower hardware / longer chains such as testnet3 - unless there is another reason to set it for all blocks that I am unaware of.

👍 pinheadmz approved a pull request: "[29.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/32810#pullrequestreview-2980431502)
ACK ef2a013e31cf6fbded5735a998b4c992c176493d

Build on macos/arm64 after experiencing issues locally building a checked-out v29.0 GUI with only qt 6 installed. Can confirm these backports solve that problem and GUI runs fine. Did not test the other backports.

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK ef2a013e31cf6fbded5735a998b4c992c176493d
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmhli/0AC
...

🤔 furszy reviewed a pull request: "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs"
(https://github.com/bitcoin/bitcoin/pull/32618#pullrequestreview-2980491758)
light code review ACK b1a8ac07e91dd1d305fcbc16ea931d60e46c0055