Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2382524112)
in commit e54d27d0f81c7a9c8991516f1ed06e86d52d6c79: consistency nit: same as in `CreateMuSig2AggregateSig`, could also check that the pubnonces and pubkey count match here, i.e.
```
// Check if enough pubnonces
if (pubnonces.size() != pubkeys.size()) return std::nullopt;

```
💬 brunoerg commented on issue "Ability to retrieve peer info by peer id":
(https://github.com/bitcoin/bitcoin/issues/33478#issuecomment-3338963332)
> for reference, this seems related to [#32741](https://github.com/bitcoin/bitcoin/pull/32741)

Should #32741 be up for graps?
💬 glozow commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3338979025)
> Tangibly, that would mean closing this PR and a new one to change the sentence
> Let me know @glozow.

It looks like you have conceptual support from folks to change the string, but strong preferences for one of two approaches. Your call whether you want to change your PR, leave it as is, open a new one, etc. Not everyone will agree.
💬 glozow commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#discussion_r2382612607)
fixed in #33476
💬 glozow commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2382621427)
fixed
💬 glozow commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#issuecomment-3339002781)
backported to 28.x in #33476
💬 glozow commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3339005617)
backported to 29.x in #33226
backported to 28.x in #33476
👋 glozow's pull request is ready for review: "[28.x] backports + 28.3rc1"
(https://github.com/bitcoin/bitcoin/pull/33476)
💬 glozow commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#issuecomment-3339030472)
Ready for review, though I think the tidy job tripped somehow?
🤔 rkrux reviewed a pull request: "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys"
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3272643818)
Code review 02994c2cbe2f051b868f49e65fac042feace2edf

Thanks for addressing the previous comments.

I'm about to wrap up my review by taking a look at the `libsecp` specific functions used in the `CreateMuSig2*` functions.
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2382674470)
In 1181568816e32f378f3f9a50a0658d96541771de "sign: Create MuSig2 signatures for known MuSig2 aggregate keys" (but also in the 3 previous commits)

The original impetus of this suggestion was for the reader to avoid reading redundant code, but having the `sighash` calculated only once during MuSig2 signing flow for a `script_pubkey` seems better from performance POV as well, even though this redundant calculation is inside wallet and not in the more time sensitive operations of the node. This a
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2382697115)
In 02994c2cbe2f051b868f49e65fac042feace2edf "test: Test MuSig2 in the wallet"

In continuation to the previous suggestion of handling the cases where the `musig` descriptor is not known to the wallet and instead only few keys are that could sign - https://github.com/bitcoin/bitcoin/pull/29675/#discussion_r2368738533 (thanks for adding the fix btw).

When I was debugging this case earlier, 2 tests failed because of incorrect number of nonces added - earlier `send` RPC was used instead of `wal
...
💬 HowHsu commented on pull request "index: handle case where pindex_prev equals chain tip in NextSyncBlock()":
(https://github.com/bitcoin/bitcoin/pull/32875#discussion_r2382744473)
> Not sure what that means, we write tests to tell us when the behavior of the application changed. You have changed the behavior and no test broke, so there's a lack of test coverage that we should fix to explain why this change is

this PR doesn't change the behavior of `NextSyncBlock()`, it just tweaks the code to make the function clearer.
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3339174566)
Rebased. And addressed the changes done in [PR #33230](https://github.com/bitcoin/bitcoin/pull/33230).
💬 adam3us commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#discussion_r2382752997)
The warning "deprecated & discouraged" doesn't sound consistent with @bitschmidty's use case for miners wanting to set this parameter

> Some miners want to use Bitcoin Core for block template building but dont want to include large op_returns.

https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3320079835

I didn't see any commentary disagreeing with it being reasonable for miners to want that, and in fact miners are the node type uniquely in a position to actually curb large dat
...
💬 adam3us commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3339197218)
> @ajtowns wrote:
>
> > For comparison, the difficulty increase from early May to now (given the same hashrate) implies a decrease in revenue of 19.5%, more than 100x as much. I think it makes sense to set defaults to maximise even small amounts of revenue, but that's a "picking up pennies" saving, not a "small miners will be put out of business" situation.
>
> Is the argument that poor compact block reconstruction rates leads to miner centralization overblown then if the loss in revenue d
...
🚀 glozow merged a pull request: "key: use static context for libsecp256k1 calls where applicable"
(https://github.com/bitcoin/bitcoin/pull/33399)
💬 maflcko commented on issue "use -loadblock to load blk*****.dat files, but the blocks in it are not recognized":
(https://github.com/bitcoin/bitcoin/issues/33280#issuecomment-3339296357)
> I don't know if that was discussed when block obfuscation was added (FYI [@maflcko](https://github.com/maflcko)).

I wasn't aware of a use-case to load raw block files without first linearizing them. As suggested, the workaround here would be `-blocksxor=0` for the node that generates block files to import. You could also hack together a trivial python script to do the xor for you, quickly. (The linearize script may also work, if you omit the unneeded block hashes, but I haven't tried this).
🚀 glozow merged a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313)
💬 0xB10C commented on pull request "p2p: Use network-dependent timers for inbound inv scheduling":
(https://github.com/bitcoin/bitcoin/pull/33464#issuecomment-3339341600)
Approach ACK.

Nice. I had noticed that we announce INVs to everyone at the same time, but it didn't occur to me that this is a fingerprinting leak. To test this change, I got creative and compared a 30.0 release candidate node with a node running this PR using my [p2p-circle](https://github.com/0xB10C/peer-observer/blob/master/tools/websocket/www/p2p-circle.html) peer-observer tool.

In the middle is our node and around it are our peers. Here, colored by the network which we are connected t
...