Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#issuecomment-1512175200)
I'm in the process of taking over the BIP324 PRs from @dhruv.

Note for reviewers:
* Previously, the EllSwift encoding/decoding/creation was in #23432, while the ECDH logic was in #23561. I've opted here to merge all EllSwift-related functionality into 1 PR, as the integration is now tighter at the libsecp256k1 too compared to when the PRs were first opened (libsecp256k1 has functions for directly performing ECDH on the EllSwift-encoded keys).
* `EllSwiftPubKey` is now a proper class that ex
...
💬 furszy commented on pull request "wallet: bugfix, 'AvailableCoins' invalid output type set for raw PK outputs":
(https://github.com/bitcoin/bitcoin/pull/27478#issuecomment-1512179805)
> I'm not sure if that's really a bug so long as we aren't excluding them from coin selection. I don't think we actually classify p2pk outputs as being legacy since they don't have an address. Really legacy is talking about address type.

Yeah, so far we aren't excluding them from coin selection but hmm, the more I think about this, the odder is turns for me.
Like, we mix this "legacy" output type usage for the address encoding and also to create and handle the legacy spkm. Because, in parall
...
💬 RandyMcMillan commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1512186089)
utACK d8434da
💬 mzumsande commented on pull request "refactor: Introduce EvictionManager and use it for the inbound eviction logic":
(https://github.com/bitcoin/bitcoin/pull/25572#discussion_r1169360367)
hmm, thinking more about this, I didn't see the whole picture, because everything happens in `ThreadSocketHandler` - after ` CConnman::SocketHandler() -> CConnman::SocketHandlerListening()`, `CConnman::DisconnectNodes()` is called, so it's not possible to accept new connections in between - however, `CConnman::SocketHandlerListening()` calls `AcceptConnection()` in a loop for multiple `ListenSocket`s, so if we have multiple listening sockets and an attacker would connect to more than one at the
...
💬 fanquake commented on pull request "BIP324: CKey encode/decode to elligator-swift":
(https://github.com/bitcoin/bitcoin/pull/23432#issuecomment-1512280285)
Closing for now. See #27479.
fanquake closed a pull request: "BIP324: CKey encode/decode to elligator-swift"
(https://github.com/bitcoin/bitcoin/pull/23432)
👍 vasild approved a pull request: "p2p: skip netgroup diversity follow-up"
(https://github.com/bitcoin/bitcoin/pull/27467#pullrequestreview-1389339459)
ACK 11bb31c1c43b5da36ca8509b5747abfb3278ffcd
📝 willcl-ark opened a pull request: "doc: Improve documentation of rpcallowip"
(https://github.com/bitcoin/bitcoin/pull/27480)
Closes #21070

v21.0 introduced a behaviour changed noted in #21070 where using a config value `rpcallowip=::0` no longer also permitted ipv4 ip addresses.

The rpc_bind.py functional test covers this new behaviour already by checking that the list of bind addresses exactly matches what is expected so this commit only updates the documentation.
💬 ajtowns commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#issuecomment-1512510817)
Maybe move most of the text in the PR description into a comment? "Review this first" and "Alternatives considered" stuff is useful here, but doesn't seem useful for the merge commit message?
⚠️ adityachoubeyy opened an issue: "UD"
(https://github.com/bitcoin/bitcoin/issues/27481)
### Motivation

UNIDENTIFIED SIMULATION DISTINCTION

### Possible solution

N/A

### Useful Skills

Unique Unidentified Distinction

### Guidance for new contributors

Want to work on this issue?

For guidance on contributing, please read [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md) before opening your pull request.
:lock: fanquake locked an issue: "UD"
(https://github.com/bitcoin/bitcoin/issues/27481)
👍 vasild approved a pull request: "test: add regression tests for #27468 (invalid URI segfaults)"
(https://github.com/bitcoin/bitcoin/pull/27477#pullrequestreview-1389480071)
ACK 6a77d290da589bd5620585def5bfc019e242e189
💬 fanquake commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#issuecomment-1512672126)
This will be merged after branch-off. It would be good to either get the follow up opened in advance, so we can avoid any time between merging, and the potential for intermittent CI failures, or, if that change is straight-forward enough, you could push an additional commit here, leaving the rest of the branch as-is.
💬 vasild commented on issue "Add per message stats to getnettotals rpc":
(https://github.com/bitcoin/bitcoin/issues/26337#issuecomment-1512696265)
@satsie, thanks, all looks good except the semantic of the argument (I have not looked at the code yet).

To me "aggregate by" means to "sum by" those fields, similarly to the `GROUP BY` clause in SQL. So, if I aggregate by `connection_type` and `message_type` I would expect to see only `network` and `direction` in the output. I do not have a strong opinion whether the argument should act that way or in the inverse way, but the name of the argument should be chosen in such a way as to avoid co
...
💬 fanquake commented on pull request "BIP324: Handshake prerequisites":
(https://github.com/bitcoin/bitcoin/pull/23561#issuecomment-1512705307)
Closing this for now, as it's partly included in #27479.
fanquake closed a pull request: "BIP324: Handshake prerequisites"
(https://github.com/bitcoin/bitcoin/pull/23561)
💬 fanquake commented on pull request "test: add regression tests for #27468 (invalid URI segfaults)":
(https://github.com/bitcoin/bitcoin/pull/27477#issuecomment-1512724993)
This has been merged.
fanquake closed a pull request: "test: add regression tests for #27468 (invalid URI segfaults)"
(https://github.com/bitcoin/bitcoin/pull/27477)
💬 TheCharlatan commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1169737011)
I checked this carefully again and don't think anymore that the `+ 1` is necessary.