🚀 achow101 merged a pull request: "doc: clarify that the "-j N" goes after the "--build build" part"
(https://github.com/bitcoin/bitcoin/pull/32846)

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2178357373)
... it would make the code clearer... and we can handle the errors within that function instead... Thanks! Checking it...

💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3025223286)
Rebased a67f3b07d4a1ea9289d4f7d5d3536fe2033b67b4 -> 0a9a1940782fad8dc3494c4aacee0a06c030eb5c ([`pr/ipc-stop.15`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.15) -> [`pr/ipc-stop.16`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.16), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-stop.15-rebase..pr/ipc-stop.16)) after https://github.com/bitcoin-core/libmultiprocess/pull/186 merge. Marking ready for review again

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

🤔 w0xlt reviewed a pull request: "wallet: Replace CWalletTx::mapValue and vOrderForm with explicit class members"
(https://github.com/bitcoin/bitcoin/pull/32763#pullrequestreview-2976772441)
ACK https://github.com/bitcoin/bitcoin/pull/32763/commits/9a8fd16e67755e336de683c0158d4fdd2b4ddfe8.
`value_map` and `order_form` (and therefore `WalletOrderForm` and `WalletValueMap`) lack clarity, making replacing with well-defined fields a much better approach.

🤔 ismaelsadeeq reviewed a pull request: "refactor: CFeeRate encapsulates FeeFrac internally"
(https://github.com/bitcoin/bitcoin/pull/32750#pullrequestreview-2976133077)
The current iteration is looking better, I will do another round of review and testing tomorrow.

Just a few nits after a short glance

💬 ismaelsadeeq commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2178028452)
nit: be less ambiguous?
```suggestion
const CAmount feerate_per_kvb = GetFeePerK();
```

💬 ismaelsadeeq commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2178446483)
nit:
includes are alphabetical

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2178453208)
Also, RPC `migratewallet` calls the same function so I could refactor that instance too.

💬 achow101 commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3025413663)
> The approach I suggested of just backing up all wallets as `{prefix}_{timestamp}.legacy.bak` files in the top-level wallets directory seems like it should make the backup files easier to find, identify, and clean up.

That seems reasonable to me.

💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-3025430544)
> Would there be interest in adding a benchmark for the Trim() operation here, or rather in a follow-up?

That will be nice, I don't mind adding it here. I plan to do another round of review of the added tests and diff after my last review so if added will review it as well.

I try running the bench on the linked commit. `MakeTxGraph` seems to takes two parameters, but the bench call provided three arguments.
I removed `NUM_ACCEPTABLE_ITERS` arg this is the result.


| ns/o
...

💬 achow101 commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#issuecomment-3025443426)
ACK 8cc9845b8ddf4f93a02c622e7df8d1095dc1a640

💬 achow101 commented on pull request "wallet, rpc: Use `OUTPUT_TYPES` to describe the output types instead of hardcoding them":
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-3025445225)
ACK 8cc9845b8ddf4f93a02c622e7df8d1095dc1a640

💬 achow101 commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#issuecomment-3025474575)
ACK 941b8f54c0d35d3243bb6083f3b52681d1b9a555

🚀 achow101 merged a pull request: "wallet, rpc: Use `OUTPUT_TYPES` to describe the output types instead of hardcoding them"
(https://github.com/bitcoin/bitcoin/pull/32432)

💬 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_r2178494285)
It can move backwards in height if reorg goes across difficulty adustments (impossible on regtest) or if we mine blocks with varying MTP and use time-based timelock on the tx

🚀 achow101 merged a pull request: "test: enabling wallet migration functional test on windows"
(https://github.com/bitcoin/bitcoin/pull/32219)

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-3025529620)
@ismaelsadeeq Yeah, the benchmark is in my "all txgraph" branch which also includes #32263. Removing the `NUM_ACCEPTABLE_ITERS` variable should be fine.

💬 achow101 commented on pull request "Wallet: "listreceivedby*" fix":
(https://github.com/bitcoin/bitcoin/pull/30972#issuecomment-3025543790)
Concept ACK

💬 achow101 commented on pull request "Refactor: Redefine CTransaction equality to include witness data":
(https://github.com/bitcoin/bitcoin/pull/32723#issuecomment-3025611532)
ACK 6efbd1e1dcdfbe9eae2d5c22abab3ee616a75ff2