Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 zaidmstrr commented on issue "Test interface_ipc.py fails with Duplicate ID error when libmultiprocess is installed system-wide":
(https://github.com/bitcoin/bitcoin/issues/33417#issuecomment-3303796570)
I tested with your suggested change `imports = [str(mp_dir), str(capnp_dir), str(src_dir)]` and this fixes the issue. As now `mp_dir` was in the first place of the `imports` list so when `capnp.load()` parses the `imports` it finds `proxy.capnp` early, so no further lookups in the global directory were taken.
🤔 zaidmstrr reviewed a pull request: "test: Avoid interface_ipc.py Duplicate ID errors"
(https://github.com/bitcoin/bitcoin/pull/33420#pullrequestreview-3235350017)
Tested ACK [e9c5227](https://github.com/bitcoin/bitcoin/pull/33420/commits/e9c52272ebd78d01882ac9b32b1aee8e12d87bec)

This fixes the issue and is also mentioned [here](https://github.com/bitcoin/bitcoin/issues/33417#issuecomment-3303796570).
💬 ArmchairCryptologist commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3303852998)
> 96% nodes are using 1sat/vB: https://delvingbitcoin.org/t/measuring-minrelaytxfee-across-the-bitcoin-network/

Not saying their numbers do not have better coverage, but I'm basing my stats off nodes that connect to my public nodes, where as of right now, 79 out of 794 connected nodes that report a feefilter have it set to 0.1 sat/vB or less, which is just about 10%. The ratio is pretty similar across all nodes, but I suppose it could be skewed by nodes that are specifically trying to connect
...
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2356170332)
Hmm, so there is a sort-of dependency between this invocation of `CheckMemPoolPolicyLimits()` and `ReplacementChecks()` -- the cluster size limit check won't be correct if we invoke it before we've added potential conflicts for removal to the changeset as well. However, if there are conflicts for removal, then cluster size limits are checked within `ReplacementChecks()` anyway, so there's not really an issue, except that this slightly confusing and perhaps this relationship could be overlooked
...
💬 Sjors commented on pull request "test: Avoid interface_ipc.py Duplicate ID errors":
(https://github.com/bitcoin/bitcoin/pull/33420#issuecomment-3303876135)
Concept ACK
💬 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_r2356183335)
The order is intentional, we want to always fill in data that we know about first.
💬 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.
💬 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.