Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 sdaftuar commented on pull request "p2p: skip netgroup diversity of new connections for tor/i2p/cjdns":
(https://github.com/bitcoin/bitcoin/pull/27374#discussion_r1153449034)
nit: If you touch this again, I think it would be worth adding a comment explaining why this code is here (and probably also add a comment to where the lookup in `setConnected` happens down below, near line 1806, so that if someone is touching the code in just one place they are aware of this issue).
💬 EthanHeilman commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#issuecomment-1490540612)
@hebasto Thats a great point. I had only been thinking of users that are in the same boat as me, doing a fresh MSVC build, but you are right, most users are likely building against pre-installed packages. I don't think my PR should be merged until it either:

* Adding the ability to define the function signature based on the actual version of libevent installed
OR
* Pinning the version of libevent in vcpkg.json so that users with old dependencies upgrade when they run build

I'm fairly al
...
💬 vasild commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1153482086)
I ran this on a mainnet `peers.dat`:
```
addrman size total: 72947
addrman size IPv4: 47252
addrman size IPv6: 11491
addrman size Tor: 13305
addrman size I2P: 893
addrman size CJDNS: 6
```

<details>
<summary>[patch] How to load /tmp/peers.dat from bench_bitcoin</summary>

```diff
diff --git i/src/bench/addrman.cpp w/src/bench/addrman.cpp
index 8a5cab443f..b8ec4a4071 100644
--- i/src/bench/addrman.cpp
+++ w/src/bench/addrman.cpp
@@ -1,17 +1,22 @@
// Copyright (c) 2020-2
...
🚀 glozow merged a pull request: "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer"
(https://github.com/bitcoin/bitcoin/pull/26140)
💬 jnewbery commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153498543)
Would it make sense to pass in a `const TxRelay&` here instead of a `const Peer&`?
💬 jnewbery commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153499627)
Can you remove the `LOCK(cs_main)` now (and the `LOCKS_EXCLUDED(cs_main)` annotation from the function declaration)?
💬 MarcoFalke commented on pull request "move-only: IsDeprecatedRPCEnabled to rpc/util to fix CI builds when called from wallet":
(https://github.com/bitcoin/bitcoin/pull/27322#issuecomment-1490625756)
Actually, on a second though. Any reason why you can't just call `rpcEnableDeprecated`?
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1490626840)
> I think that is fine because if the input is such that `HTTPRequest` constructor throws, then we need not to test those because they will never happen in the real code either.

Thanks @vasild, it makes sense what you are saying, I'm still playing with some options and I'll get back to you soon.
💬 MarcoFalke commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT":
(https://github.com/bitcoin/bitcoin/pull/27333#issuecomment-1490646824)
Meta note: The author needs to reply to the open discussions. Any reply is fine, but maintainers are likely waiting for it.
🚀 glozow merged a pull request: "test: refactor: dedup mempool_package_limits.py subtests via decorator"
(https://github.com/bitcoin/bitcoin/pull/27350)
fanquake closed an issue: "ci: Cleanup ci/test/01_base_install.sh"
(https://github.com/bitcoin/bitcoin/issues/27321)
🚀 fanquake merged a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT"
(https://github.com/bitcoin/bitcoin/pull/27333)
🚀 fanquake merged a pull request: "guix: use python-minimal (3.9)"
(https://github.com/bitcoin/bitcoin/pull/27361)
💬 dergoegge commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153606022)
Yes, seems like it. Will do this in a follow up (including your other suggestion), thanks!
💬 mzumsande commented on pull request "p2p: skip netgroup diversity of new connections for tor/i2p/cjdns":
(https://github.com/bitcoin/bitcoin/pull/27374#discussion_r1153586371)
How about putting this into a seperate function (something like `bool LimitOutboundByNetgroup(Network n)`), and using a `switch` statement there, similar to the existing `IsAddrV1Compatible()`? That way, the compiler would prevent us from forgetting about this if we ever add another network.
💬 hebasto commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#issuecomment-1490737465)
> I'm fairly allergic to the idea of build scripts inspecting function signatures and then defining macros based on that.

FWIW, the [coming](https://github.com/bitcoin/bitcoin/pull/25797) CMake-based build system will do it :)
💬 MarcoFalke commented on pull request "ci: use LLVM/clang-16 in native_asan job":
(https://github.com/bitcoin/bitcoin/pull/27360#discussion_r1153682940)
You can drop this after diff:


```diff
diff --git a/.cirrus.yml b/.cirrus.yml
index 237572f2e4..8abe2ba79b 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -261,7 +261,7 @@ task:
# Images can be found here: https://cloud.google.com/compute/docs/images/os-details
compute_engine_instance:
image_project: ubuntu-os-cloud
- image: family/ubuntu-2204-lts # when upgrading, check if we can drop "ADD_UNTRUSTED_BPFCC_PPA"
+ image: family/ubuntu-2304 # when upgrading, check if w
...
💬 jonatack commented on pull request "move-only: IsDeprecatedRPCEnabled to rpc/util to fix CI builds when called from wallet":
(https://github.com/bitcoin/bitcoin/pull/27322#issuecomment-1490798464)
> Any reason why you can't just call `rpcEnableDeprecated`?

Good question. Looking at it, maybe we can remove it? Per this comment in `src/interfaces/chain.h::L107` we would like to remove these, and this function doesn't seem constrained by the mentioned HTTP port requirement or needed if `IsDeprecatedRPCEnabled` is in common rather than node (the first commit here). @ryanofsky wdyt?

//! * The handleRpc, registerRpcs, rpcEnableDeprecated methods and other RPC
//! methods can g
...
💬 jonatack commented on pull request "move-only: IsDeprecatedRPCEnabled to rpc/util to fix CI builds when called from wallet":
(https://github.com/bitcoin/bitcoin/pull/27322#issuecomment-1490818171)
> Any reason why you can't just call `rpcEnableDeprecated`?

Good question. Looking at it, maybe we can remove it? Per this comment in `src/interfaces/chain.h::L107` from #15639 we would like to remove these, its RPC caller in that PR is gone now, and this function doesn't seem constrained by the mentioned HTTP port requirement or needed if `IsDeprecatedRPCEnabled` is in common rather than node (the first commit here). @ryanofsky wdyt?

//! * The handleRpc, registerRpcs, rpcEnableDeprec
...
📝 pinheadmz opened a pull request: "net: support unix domain sockets for -proxy"
(https://github.com/bitcoin/bitcoin/pull/27375)
Closes https://github.com/bitcoin/bitcoin/issues/27252

Adds `NET_UNIX` as another network type for `CNetAddr` and enables SOCKS5 routing through a local Tor daemon using a local unix domain socket instead of TCP. There has been work on [unix domain sockets before](https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1487455451) but for now I just wanted to start on this single use-case which is enabling unix sockets from the client side. Note that libevent has been updated since #9919
...