π¬ 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
...
(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
...
(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
...
(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
...
(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
...
π¬ pinheadmz commented on issue "Adding interprocess onion access instead local ip":
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1490826301)
Ok I have a working implementation of this: https://github.com/bitcoin/bitcoin/pull/27375
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1490826301)
Ok I have a working implementation of this: https://github.com/bitcoin/bitcoin/pull/27375
π¬ theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1153728573)
This isn't clear to me either.
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1153728573)
This isn't clear to me either.
π¬ theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1490874473)
> > @TheCharlatan I see it working using e.g. the pub subcommand. You need to choose pub or bin to verify either the publicly-hosted binaries, or bin for a local binary. But you are correct the docs are now outdated...
>
> Mmh, shouldn't the sample command you posted only have verified x86_64 binaries?
Mmm, yeah, this is awkward. @achow101's original changes forced the user to specify which binaries should be checked. I requested using the contents of the SHA256SUMS file instead.
The pr
...
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1490874473)
> > @TheCharlatan I see it working using e.g. the pub subcommand. You need to choose pub or bin to verify either the publicly-hosted binaries, or bin for a local binary. But you are correct the docs are now outdated...
>
> Mmh, shouldn't the sample command you posted only have verified x86_64 binaries?
Mmm, yeah, this is awkward. @achow101's original changes forced the user to specify which binaries should be checked. I requested using the contents of the SHA256SUMS file instead.
The pr
...
π¬ red0bear commented on issue "Adding interprocess onion access instead local ip":
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1490916002)
so instead use -onion i could use -proxy ? What version this option is enabled ?
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1490916002)
so instead use -onion i could use -proxy ? What version this option is enabled ?
π¬ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1490986943)
Pushed the change to make tidy happy.
@furszy: I put my take on your suggested changes in https://github.com/Xekyo/bitcoin/commits/furszy-minmintests, but I havenβt been able to bring myself to merge it yet, because I was hoping to keep the changes small with the feature freeze on Saturday. Havenβt given up the hope completely yet. (^_^)/
If I have to touch it up a bit more, Iβm thinking to do it, but otherwise maybe in a follow-up?
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1490986943)
Pushed the change to make tidy happy.
@furszy: I put my take on your suggested changes in https://github.com/Xekyo/bitcoin/commits/furszy-minmintests, but I havenβt been able to bring myself to merge it yet, because I was hoping to keep the changes small with the feature freeze on Saturday. Havenβt given up the hope completely yet. (^_^)/
If I have to touch it up a bit more, Iβm thinking to do it, but otherwise maybe in a follow-up?
π¬ pinheadmz commented on issue "Adding interprocess onion access instead local ip":
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1491022746)
I'm just experimenting with the code. If you can review the pull request that will help get move it towards release.
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1491022746)
I'm just experimenting with the code. If you can review the pull request that will help get move it towards release.
π¬ red0bear commented on issue "Adding interprocess onion access instead local ip":
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1491023996)
OK ... i dont know how to do it.
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1491023996)
OK ... i dont know how to do it.
π¬ ChrisCho-H commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1491173572)
Need more reviewers to be merged?
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1491173572)
Need more reviewers to be merged?
π¬ vasild commented on pull request "p2p: skip netgroup diversity of new connections for tor/i2p/cjdns":
(https://github.com/bitcoin/bitcoin/pull/27374#issuecomment-1491308956)
Does this PR trick the code to behave as if we are not connected to Tor (by not inserting into `setConnected`) even if we are connected? It looks to me that trying to solve the problem of "cannot open many connections to Tor" this would create another problem: "too many connections to Tor" - it would allow all 8 outbound to be to Tor and will not try to diversify to IPv4 (or other networks).
I feel that we should lookup `-onlynet` or allow connections to Tor up to a limit to avoid "too many c
...
(https://github.com/bitcoin/bitcoin/pull/27374#issuecomment-1491308956)
Does this PR trick the code to behave as if we are not connected to Tor (by not inserting into `setConnected`) even if we are connected? It looks to me that trying to solve the problem of "cannot open many connections to Tor" this would create another problem: "too many connections to Tor" - it would allow all 8 outbound to be to Tor and will not try to diversify to IPv4 (or other networks).
I feel that we should lookup `-onlynet` or allow connections to Tor up to a limit to avoid "too many c
...
π¬ pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1491325361)
Updated changes:
- Fixed fuzz test failing.
> Since the replySent parameter of the HTTPRequest constructor is used only from the tests, I would suggest to remove it and adjust the test as needed.
@vasild I left open the possibility of passing the `replySent` arg as `true`, this way we can "build" a "safer" "invalid" request as @stickies-v mentioned above, so we can test the almost entire protocol of `HTTPRequest` if that was originally the intention of this fuzz.
Then, as part of the a
...
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1491325361)
Updated changes:
- Fixed fuzz test failing.
> Since the replySent parameter of the HTTPRequest constructor is used only from the tests, I would suggest to remove it and adjust the test as needed.
@vasild I left open the possibility of passing the `replySent` arg as `true`, this way we can "build" a "safer" "invalid" request as @stickies-v mentioned above, so we can test the almost entire protocol of `HTTPRequest` if that was originally the intention of this fuzz.
Then, as part of the a
...
π MarcoFalke opened a pull request: "ci: Remove second user account"
(https://github.com/bitcoin/bitcoin/pull/27376)
The rationale for the second (nonroot) account no longer applies. See also https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148898438
(https://github.com/bitcoin/bitcoin/pull/27376)
The rationale for the second (nonroot) account no longer applies. See also https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148898438
π¬ MarcoFalke commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT":
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1154069678)
See https://github.com/bitcoin/bitcoin/pull/27376
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1154069678)
See https://github.com/bitcoin/bitcoin/pull/27376
π josibake approved a pull request: "ci: Remove second user account"
(https://github.com/bitcoin/bitcoin/pull/27376)
utACK https://github.com/bitcoin/bitcoin/pull/27376/commits/fafe3a8e38bdb6d12b2654b066b8036ba3e9455d
(https://github.com/bitcoin/bitcoin/pull/27376)
utACK https://github.com/bitcoin/bitcoin/pull/27376/commits/fafe3a8e38bdb6d12b2654b066b8036ba3e9455d
π¬ MarcoFalke commented on pull request "test: remove `GetRNGState` lsan suppression":
(https://github.com/bitcoin/bitcoin/pull/27362#issuecomment-1491412437)
lgtm ACK 71b3e9b0ade9680f6847e93785225c5927929336
(https://github.com/bitcoin/bitcoin/pull/27362#issuecomment-1491412437)
lgtm ACK 71b3e9b0ade9680f6847e93785225c5927929336
π¬ MarcoFalke commented on pull request "ci: set docker run --ulimit to workaround Valgrind assertion":
(https://github.com/bitcoin/bitcoin/pull/27364#issuecomment-1491421754)
Have you tried podman? The `podman-docker` on Ubuntu/Debian might help (haven't tried it), but not sure if it exists on your Fedora.
(https://github.com/bitcoin/bitcoin/pull/27364#issuecomment-1491421754)
Have you tried podman? The `podman-docker` on Ubuntu/Debian might help (haven't tried it), but not sure if it exists on your Fedora.
π¬ willcl-ark commented on issue "Adding interprocess onion access instead local ip":
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1491446913)
Oh nice work @pinheadmz! I will help test #27375 too.
I have a small bash script which uses `socat` to pipe between an inet and unix socket, but native support is much better:
```bash
#!/usr/bin/env bash
# default tor sock on debian
socat -v tcp-l:9050,reuseaddr,fork unix:/run/tor/socks 2>&1/dev/null & disown
socat_pid=$!
while true; do
sleep 5 # check every 5 seconds
if ! pgrep -x "bitcoind" >/dev/null; then
# bitcoind not found, terminate socat and exit loop
...
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1491446913)
Oh nice work @pinheadmz! I will help test #27375 too.
I have a small bash script which uses `socat` to pipe between an inet and unix socket, but native support is much better:
```bash
#!/usr/bin/env bash
# default tor sock on debian
socat -v tcp-l:9050,reuseaddr,fork unix:/run/tor/socks 2>&1/dev/null & disown
socat_pid=$!
while true; do
sleep 5 # check every 5 seconds
if ! pgrep -x "bitcoind" >/dev/null; then
# bitcoind not found, terminate socat and exit loop
...
π¬ MarcoFalke commented on issue "test: `wallet_importdescriptors.py --descriptors` failure":
(https://github.com/bitcoin/bitcoin/issues/27282#issuecomment-1491468284)
Ok, this still happens: https://cirrus-ci.com/task/5541464030052352?logs=ci#L2622
However, it looks like there is another unrelated bug in the rpc proxy to dispatch the same RPC twice? There really shouldn't be any reason to blindly run the same RPC twice without the user (test developer) asking for it.
I presume running the RPC twice will also discard the earlier result, so still no luck debugging this issue.
(https://github.com/bitcoin/bitcoin/issues/27282#issuecomment-1491468284)
Ok, this still happens: https://cirrus-ci.com/task/5541464030052352?logs=ci#L2622
However, it looks like there is another unrelated bug in the rpc proxy to dispatch the same RPC twice? There really shouldn't be any reason to blindly run the same RPC twice without the user (test developer) asking for it.
I presume running the RPC twice will also discard the earlier result, so still no luck debugging this issue.