💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-2898773591)
Rebased, ready for review.

👋 achow101's pull request is ready for review: "wallet: Remove watchonly behavior and isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523)

👋 achow101's pull request is ready for review: "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet"
(https://github.com/bitcoin/bitcoin/pull/32489)

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#issuecomment-2898781091)
Rebased, ready for review

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2100870613)
Done

💬 pinheadmz commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2100887156)
fa53098472521de9d5b3cc42983043c240b7c321

Why is this here and not in the next block that is already guarded by `pindex_was_in_chain`? `to_mark_failed` and `m_chainman` are still in scope.

💬 pinheadmz commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2898819643)
One thing that's a little weird about this is that when I open regtest after a few days...

```
--> bccli -regtest -getinfo
Chain: regtest
Blocks: 202
Headers: 202
Verification progress: ▒▒▒▒▒▒░░░░░░░░░░░░░░░ 27.7195%
```

Of course after generating 1 block at time=now, it snaps to 100%

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2100880884)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2099737369

execvp is documented to return -1 if there is an error (https://linux.die.net/man/3/execvp and https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/execvp-wexecvp) so if it returns something else I think it is better to abort and not just keep going.

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2100888510)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2099742506

Yes I used to refer to them as NUL terminated strings, until I came across https://en.wikipedia.org/wiki/Null-terminated_string and decided there wasn't any real ambiguity and null was probably easier to read. But would be ok changing this if others would prefer

🤔 ryanofsky reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2858683133)
Thanks for reviewing again!

💬 davidgumberg commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2100898080)
Am I correct in understanding that `handleNotifyBlockTip` takes a lock now where it didn't before?

💬 rkrux commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-2898830691)
This PR comes at a right time. I'm prioritising reviewing this one because it removes `isminetype`.

#27286: The new wallet unspents model in memory is using `isminetype` currently and I'd prefer to not have it checked into master only to be removed subsequently. This diff should also ease the review of the other PR.

https://github.com/bitcoin/bitcoin/pull/27286/commits/d637af8a9786e38f1ba140ed828a3d5009e62926#diff-d41d68c5a65d67956c91b33ca86da7df1981d84a0b15b4a186deea566563fed5R372

💬 pinheadmz commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2898832559)
Also this is somewhat pathalogical but...

```

--> bccli -regtest setmocktime 1719861124 # July 1, 2024
--> bccli -regtest -getinfo
Chain: regtest
Blocks: 203
Headers: 203
Verification progress: -0.7451%

```

👍 ryanofsky approved a pull request: "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage"
(https://github.com/bitcoin/bitcoin/pull/32467#pullrequestreview-2858731450)
Code review ACK fd290730f530a8b76a9607392f49830697cdd7c5, just removing assert since last review. Thanks for considering all the comments and feedback!

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

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100993968)
I have changed the variable names to `max_cluster_count` and `max_cluster_size` everywhere.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100995536)
`GetTotalTxSize` is a nice color.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100996101)
Done, but also removed in a later commit.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100995144)
Do you mean in the TxGraph::AddTransaction definition in txgraph.h? I have expanded the comments in the fuzz test around this a bit.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2100993169)
I have expended the commit message of both Trim commits.