💬 yancyribbens commented on issue "coin-grinder missing test for TOTAL_TRIES":
(https://github.com/bitcoin/bitcoin/issues/33419#issuecomment-3303748477)
cc @murchandamus
(https://github.com/bitcoin/bitcoin/issues/33419#issuecomment-3303748477)
cc @murchandamus
📝 ryanofsky opened a pull request: "test: Avoid interface_ipc.py Duplicate ID errors"
(https://github.com/bitcoin/bitcoin/pull/33420)
This change should fix issue https://github.com/bitcoin/bitcoin/issues/33417 reported by zaidmstrr. It's possible to reproduce the `mp/proxy.capnp:0: failed: Duplicate ID @0xcc316e3f71a040fb` error by installing libmultiprocess system-wide, or to one of the locations listed in the python test's `imports` list before the local libmultiprocess subtree, and then running the test.
(https://github.com/bitcoin/bitcoin/pull/33420)
This change should fix issue https://github.com/bitcoin/bitcoin/issues/33417 reported by zaidmstrr. It's possible to reproduce the `mp/proxy.capnp:0: failed: Duplicate ID @0xcc316e3f71a040fb` error by installing libmultiprocess system-wide, or to one of the locations listed in the python test's `imports` list before the local libmultiprocess subtree, and then running the test.
💬 ryanofsky 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-3303794531)
I was able to reproduce the error by modifying the test to point at an external libmultiprocess installation and I think #33420 should fix it
(https://github.com/bitcoin/bitcoin/issues/33417#issuecomment-3303794531)
I was able to reproduce the error by modifying the test to point at an external libmultiprocess installation and I think #33420 should fix it
💬 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.
(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).
(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
...
(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
...
(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
(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.
(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.
(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