Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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
💬 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
💬 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
💬 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
💬 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
💬 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
👍 darosior approved a pull request: "[29.x] Backports"
(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.
💬 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
👍 darosior approved a pull request: "[30.0] rc2 backports"
(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?
🤔 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.
💬 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.
💬 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.
💬 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.
👍 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.
🚀 glozow merged a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33344)
🚀 glozow merged a pull request: "[30.0] rc2 backports"
(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
...
💬 enirox001 commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2356250657)
The current error handling only checks for `LOAD_OK` and ignores the error string provided by the new overload. This may result in losing useful error details. It might be worth updating the handling to make use of the message