💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356190597)
No, signing does not log.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356190597)
No, signing does not log.
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192082)
Fixed
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192082)
Fixed
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192320)
Done
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192320)
Done
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192456)
Done
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192456)
Done
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192604)
Done
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192604)
Done
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192801)
Done
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356192801)
Done
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356193704)
Done
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2356193704)
Done
👍 darosior approved a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33344#pullrequestreview-3235483971)
utACK f2bd79f80c74a2b77f14954ac65679417697a332
(https://github.com/bitcoin/bitcoin/pull/33344#pullrequestreview-3235483971)
utACK f2bd79f80c74a2b77f14954ac65679417697a332
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2356226958)
Thanks, I'm taking this change.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2356226958)
Thanks, I'm taking this change.
💬 Raimo33 commented on pull request "crypto: optimize SipHash Write() method with chunked processing":
(https://github.com/bitcoin/bitcoin/pull/33325#issuecomment-3303939761)
> In the meantime, please check if there's any way to split this into even smaller commits to make it even easier for reviewers
I've just split the PR into 3 commits, diff should now be simpler
(https://github.com/bitcoin/bitcoin/pull/33325#issuecomment-3303939761)
> In the meantime, please check if there's any way to split this into even smaller commits to make it even easier for reviewers
I've just split the PR into 3 commits, diff should now be simpler
👍 darosior approved a pull request: "[30.0] rc2 backports"
(https://github.com/bitcoin/bitcoin/pull/33356#pullrequestreview-3235516279)
utACK c9f751090cb638ad8fff600133349446bf426e15
(https://github.com/bitcoin/bitcoin/pull/33356#pullrequestreview-3235516279)
utACK c9f751090cb638ad8fff600133349446bf426e15
💬 Raimo33 commented on pull request "coinstats: avoid unnecessary Coin copy in ApplyHash":
(https://github.com/bitcoin/bitcoin/pull/33410#issuecomment-3303956055)
Concept ACK, I want to expand on l0rinc suggestion by saying that, given the above warning, I think this should only be merged if it results in a significant performance improvement. @sashass1315 are you able to run some before/after benchmarks?
(https://github.com/bitcoin/bitcoin/pull/33410#issuecomment-3303956055)
Concept ACK, I want to expand on l0rinc suggestion by saying that, given the above warning, I think this should only be merged if it results in a significant performance improvement. @sashass1315 are you able to run some before/after benchmarks?
🤔 mzumsande reviewed a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33344#pullrequestreview-3235527352)
utACK f2bd79f80c74a2b77f14954ac65679417697a332
I mostly looked at the p2p backports (#32646, #33296, #33395), the rest looked correct too but I didn't check very deeply.
(https://github.com/bitcoin/bitcoin/pull/33344#pullrequestreview-3235527352)
utACK f2bd79f80c74a2b77f14954ac65679417697a332
I mostly looked at the p2p backports (#32646, #33296, #33395), the rest looked correct too but I didn't check very deeply.
💬 mstampfer commented on pull request "contrib: Add zsh completion scripts":
(https://github.com/bitcoin/bitcoin/pull/33402#issuecomment-3303976362)
I have opened an [Issue requesting zsh completion support](https://github.com/bitcoin/bitcoin/issues/33404). Please reopen this PR or please comment with any concerns why this feature should not be merged.
(https://github.com/bitcoin/bitcoin/pull/33402#issuecomment-3303976362)
I have opened an [Issue requesting zsh completion support](https://github.com/bitcoin/bitcoin/issues/33404). Please reopen this PR or please comment with any concerns why this feature should not be merged.
💬 achow101 commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2356282381)
>1 TB of RAM is entirely feasible on high end workstations and servers, I think we could add a couple more 0's to this check.
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2356282381)
>1 TB of RAM is entirely feasible on high end workstations and servers, I think we could add a couple more 0's to this check.
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2356303156)
I wanted this to be a lower-end maximum - otherwise we can just delete it. But I don't mind adding another 0 at the end.
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2356303156)
I wanted this to be a lower-end maximum - otherwise we can just delete it. But I don't mind adding another 0 at the end.
👍 hebasto approved a pull request: "[30.0] rc2 backports"
(https://github.com/bitcoin/bitcoin/pull/33356#pullrequestreview-3235615679)
ACK c9f751090cb638ad8fff600133349446bf426e15, I applied all backports locally without conflicts and obtained a zero diff with this PR branch.
https://github.com/bitcoin/bitcoin/pull/33395 is missing from the PR description.
(https://github.com/bitcoin/bitcoin/pull/33356#pullrequestreview-3235615679)
ACK c9f751090cb638ad8fff600133349446bf426e15, I applied all backports locally without conflicts and obtained a zero diff with this PR branch.
https://github.com/bitcoin/bitcoin/pull/33395 is missing from the PR description.
🚀 glozow merged a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33344)
(https://github.com/bitcoin/bitcoin/pull/33344)
🚀 glozow merged a pull request: "[30.0] rc2 backports"
(https://github.com/bitcoin/bitcoin/pull/33356)
(https://github.com/bitcoin/bitcoin/pull/33356)
🤔 enirox001 reviewed a pull request: "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`"
(https://github.com/bitcoin/bitcoin/pull/32636#pullrequestreview-3235533295)
Concept ACK, overall the refactor is a good attempt, and would modularize the code better. Here are review comments on a commit by commit basis. The commits are self contained and atomic, which I commend, makes review very efficient.
f92031e3b0: Good rename from LoadWallet() to PopulateWalletFromDB() with proper scripted-diff.
8b6a8a04f9: Good separation of error handling logic into PopulateWalletFromDB() with overloaded method for detailed error reporting.
f6c4454b87: Good conversion from
...
(https://github.com/bitcoin/bitcoin/pull/32636#pullrequestreview-3235533295)
Concept ACK, overall the refactor is a good attempt, and would modularize the code better. Here are review comments on a commit by commit basis. The commits are self contained and atomic, which I commend, makes review very efficient.
f92031e3b0: Good rename from LoadWallet() to PopulateWalletFromDB() with proper scripted-diff.
8b6a8a04f9: Good separation of error handling logic into PopulateWalletFromDB() with overloaded method for detailed error reporting.
f6c4454b87: Good conversion from
...