Bitcoin Core Github
43 subscribers
123K links
Download Telegram
📝 furszy opened a pull request: "wallet: bugfix, 'AvailableCoins' invalid output type set for raw PK outputs"
(https://github.com/bitcoin/bitcoin/pull/27478)
`AvailableCoins` sets raw pubkey outputs with an `OutputType::UNKNOWN` type instead of `OutputType::LEGACY`.

Gladly, no type unknown outputs are skipped during coin selection, nor anything different is done with them.
It is just an inconsistency.
💬 achow101 commented on pull request "wallet: bugfix, 'AvailableCoins' invalid output type set for raw PK outputs":
(https://github.com/bitcoin/bitcoin/pull/27478#issuecomment-1512107265)
I'm not sure if that's really a bug. 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.

Additionally, in descriptors, `pk()` descriptors are treated as having no output type, rather than `unknown` or `legacy`.
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#issuecomment-1512128741)
thanks for the reviews! I'll address the outstanding comments in a followup
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1169300130)
🤦‍♀️ yup, agreed. will do in follow-up
📝 sipa opened a pull request: "BIP324: ElligatorSwift integrations"
(https://github.com/bitcoin/bitcoin/pull/27479)
This replaces #23432 and part of #23561.

This PR introduces all of the ElligatorSwift-related changes (libsecp256k1 updates, generation, decoding, ECDH, tests, fuzzing, benchmarks) in. These were previously split over two PRs, but as the libsecp256k1 code has ECDH operations implemented directly on the ellswift encoded public keys, it feels more natural to merge them.

Some of the code is structured a bit differently, as some further simplifications were possible.
💬 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
...