💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2476779538)
I pulled in the changes from https://github.com/Sjors/bitcoin/pull/65 to enable multiprocess for non-depends builds. But I disabled it for various jobs, see TODO in PR description.
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2476779538)
I pulled in the changes from https://github.com/Sjors/bitcoin/pull/65 to enable multiprocess for non-depends builds. But I disabled it for various jobs, see TODO in PR description.
💬 andrewtoth commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2476798087)
@l0rinc a way to test that this PR is working properly is to first run master and sync a few blocks. In the `UpdateTip` logs you will see a `cache=` value that increases. Running either of these RPCs should make the next `UpdateTip` reset a few MB at most.
When doing the same test on this branch, the next `UpdateTip` value should be the same or higher than the previous.
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2476798087)
@l0rinc a way to test that this PR is working properly is to first run master and sync a few blocks. In the `UpdateTip` logs you will see a `cache=` value that increases. Running either of these RPCs should make the next `UpdateTip` reset a few MB at most.
When doing the same test on this branch, the next `UpdateTip` value should be the same or higher than the previous.
💬 maflcko commented on issue "bitcoin-qt failed assertion on startup":
(https://github.com/bitcoin/bitcoin/issues/31289#issuecomment-2476813033)
Thanks for the report. This is a GUI issue. The GUI is running in a separate thread during init, so the two threads can race. It is possible to reproduce by pressing `q` at the right time. See this diff to reproduce:
```diff
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index 34f19df256..e8e58e7631 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -280,6 +280,8 @@ bool CRPCTable::removeCommand(const std::string& name, const CRPCCommand* pcmd)
void StartRPC()
{
...
(https://github.com/bitcoin/bitcoin/issues/31289#issuecomment-2476813033)
Thanks for the report. This is a GUI issue. The GUI is running in a separate thread during init, so the two threads can race. It is possible to reproduce by pressing `q` at the right time. See this diff to reproduce:
```diff
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index 34f19df256..e8e58e7631 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -280,6 +280,8 @@ bool CRPCTable::removeCommand(const std::string& name, const CRPCCommand* pcmd)
void StartRPC()
{
...
💬 TheCharlatan commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2476846187)
Guix build:
```
780670d401a6f110231c6a941c3e044065624f4c60fe1ce4340992c8bf8513a7 guix-build-bcd82b13f464/output/aarch64-linux-gnu/SHA256SUMS.part
2ecebfbbf8f557d371d7eecb312bc6a17d8ce1d38db94acf84f7571202f8a688 guix-build-bcd82b13f464/output/aarch64-linux-gnu/bitcoin-bcd82b13f464-aarch64-linux-gnu-debug.tar.gz
2dd1837f044308c53ec5a2cb76b5f93497e08561c1b7c1c7efeb3af5e0a3afdf guix-build-bcd82b13f464/output/aarch64-linux-gnu/bitcoin-bcd82b13f464-aarch64-linux-gnu.tar.gz
b2b6a2de895dcb68ea5e
...
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2476846187)
Guix build:
```
780670d401a6f110231c6a941c3e044065624f4c60fe1ce4340992c8bf8513a7 guix-build-bcd82b13f464/output/aarch64-linux-gnu/SHA256SUMS.part
2ecebfbbf8f557d371d7eecb312bc6a17d8ce1d38db94acf84f7571202f8a688 guix-build-bcd82b13f464/output/aarch64-linux-gnu/bitcoin-bcd82b13f464-aarch64-linux-gnu-debug.tar.gz
2dd1837f044308c53ec5a2cb76b5f93497e08561c1b7c1c7efeb3af5e0a3afdf guix-build-bcd82b13f464/output/aarch64-linux-gnu/bitcoin-bcd82b13f464-aarch64-linux-gnu.tar.gz
b2b6a2de895dcb68ea5e
...
💬 maflcko commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2476847004)
I think the issue in the fuzz tests still persists.
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2476847004)
I think the issue in the fuzz tests still persists.
💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1842526635)
The spec: https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md#451-handshake-act-1-nx-handshake-part-1---e
Which says `Noise_NX_Secp256k1+EllSwift_ChaChaPoly_SHA256`. I adjusted the second comment.
I also briefly sanity checked the numerical values with some online tools.
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1842526635)
The spec: https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md#451-handshake-act-1-nx-handshake-part-1---e
Which says `Noise_NX_Secp256k1+EllSwift_ChaChaPoly_SHA256`. I adjusted the second comment.
I also briefly sanity checked the numerical values with some online tools.
💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1842538464)
Looks like I dropped this in a recent update. `WITH_SV2` is now on by default and not turned off by the fuzzer CI. I think...
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1842538464)
Looks like I dropped this in a recent update. `WITH_SV2` is now on by default and not turned off by the fuzzer CI. I think...
💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1842541209)
It's an edge case, but `valid_from == now` and `valid_to == now` should be allowed.
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1842541209)
It's an edge case, but `valid_from == now` and `valid_to == now` should be allowed.
💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2476925664)
This needs a rebase. Otherwise it's non-trivial for me to keep https://github.com/Sjors/bitcoin/pull/67 rebased.
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2476925664)
This needs a rebase. Otherwise it's non-trivial for me to keep https://github.com/Sjors/bitcoin/pull/67 rebased.
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2476951015)
`70c2f13f83...c4f51c7f61`: rebase due to conflicts, next I will look at @pinheadmz feedback above and implement changes from coredev discussions:
* Store the node id -> CNode map in SockMan (then the SockMan class becomes templated)
* SockMan <-> Connman calls based on CNode, not node id
* Make the virtual methods in SockMan private, since they are called only from SockMan
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2476951015)
`70c2f13f83...c4f51c7f61`: rebase due to conflicts, next I will look at @pinheadmz feedback above and implement changes from coredev discussions:
* Store the node id -> CNode map in SockMan (then the SockMan class becomes templated)
* SockMan <-> Connman calls based on CNode, not node id
* Make the virtual methods in SockMan private, since they are called only from SockMan
👍 dergoegge approved a pull request: "fuzz: Fix difficulty target generation in `p2p_headers_presync`"
(https://github.com/bitcoin/bitcoin/pull/31213#pullrequestreview-2436723824)
Code review ACK a6ca8f324396522e9748c9a7bbefb3bf1c74a436
(https://github.com/bitcoin/bitcoin/pull/31213#pullrequestreview-2436723824)
Code review ACK a6ca8f324396522e9748c9a7bbefb3bf1c74a436
💬 laanwj commented on issue "guix: Linux and macOS builds are not cross-arch reproducible with powerpc64le build arch":
(https://github.com/bitcoin/bitcoin/issues/31207#issuecomment-2476982448)
> Last I was looking at this, if you guix built (master) for powerpc64le-linux-gnu, once on x86_64 and once on aarch64
It might be, but it's unclear to me if that is the same issue, this OP talks about building *on* PPC64.
(https://github.com/bitcoin/bitcoin/issues/31207#issuecomment-2476982448)
> Last I was looking at this, if you guix built (master) for powerpc64le-linux-gnu, once on x86_64 and once on aarch64
It might be, but it's unclear to me if that is the same issue, this OP talks about building *on* PPC64.
💬 fanquake commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2477072548)
Guix Build (x86_64):
```bash
780670d401a6f110231c6a941c3e044065624f4c60fe1ce4340992c8bf8513a7 guix-build-bcd82b13f464/output/aarch64-linux-gnu/SHA256SUMS.part
2ecebfbbf8f557d371d7eecb312bc6a17d8ce1d38db94acf84f7571202f8a688 guix-build-bcd82b13f464/output/aarch64-linux-gnu/bitcoin-bcd82b13f464-aarch64-linux-gnu-debug.tar.gz
2dd1837f044308c53ec5a2cb76b5f93497e08561c1b7c1c7efeb3af5e0a3afdf guix-build-bcd82b13f464/output/aarch64-linux-gnu/bitcoin-bcd82b13f464-aarch64-linux-gnu.tar.gz
b2b6a2d
...
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2477072548)
Guix Build (x86_64):
```bash
780670d401a6f110231c6a941c3e044065624f4c60fe1ce4340992c8bf8513a7 guix-build-bcd82b13f464/output/aarch64-linux-gnu/SHA256SUMS.part
2ecebfbbf8f557d371d7eecb312bc6a17d8ce1d38db94acf84f7571202f8a688 guix-build-bcd82b13f464/output/aarch64-linux-gnu/bitcoin-bcd82b13f464-aarch64-linux-gnu-debug.tar.gz
2dd1837f044308c53ec5a2cb76b5f93497e08561c1b7c1c7efeb3af5e0a3afdf guix-build-bcd82b13f464/output/aarch64-linux-gnu/bitcoin-bcd82b13f464-aarch64-linux-gnu.tar.gz
b2b6a2d
...
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842711569)
I've reworked the docs for this
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842711569)
I've reworked the docs for this
🚀 fanquake merged a pull request: "guix: remove `util-linux`"
(https://github.com/bitcoin/bitcoin/pull/31285)
(https://github.com/bitcoin/bitcoin/pull/31285)
💬 mzumsande commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1842752832)
took the suggestion now!
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1842752832)
took the suggestion now!
👍 maflcko approved a pull request: "test: Rework wallet_migration.py to use previous releases"
(https://github.com/bitcoin/bitcoin/pull/31248#pullrequestreview-2436993322)
Nothing blocking
ACK a76ad56a80d9c9a60352bb98b363131e359a383b 🌆
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK a76ad5
...
(https://github.com/bitcoin/bitcoin/pull/31248#pullrequestreview-2436993322)
Nothing blocking
ACK a76ad56a80d9c9a60352bb98b363131e359a383b 🌆
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK a76ad5
...
💬 maflcko commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1842764705)
nit in a76ad56a80d9c9a60352bb98b363131e359a383b: Forgot to use the node alias?
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1842764705)
nit in a76ad56a80d9c9a60352bb98b363131e359a383b: Forgot to use the node alias?
💬 mzumsande commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#issuecomment-2477220525)
Updated PR (took much longer than expected due to a medical issue, sorry) and I still plan on adding a functional test, hopefully tomorrow.
- took vasild's suggestion, so that OnionServiceTargetPort is removed from `chainparamsbase`
- expanded release note
(https://github.com/bitcoin/bitcoin/pull/31223#issuecomment-2477220525)
Updated PR (took much longer than expected due to a medical issue, sorry) and I still plan on adding a functional test, hopefully tomorrow.
- took vasild's suggestion, so that OnionServiceTargetPort is removed from `chainparamsbase`
- expanded release note
💬 mzumsande commented on issue "net: Tor service target port collides when running multiple nodes, making bitcoind error out":
(https://github.com/bitcoin/bitcoin/issues/31133#issuecomment-2477231557)
> Also, this has another flaw: `bind_on_any` will be `false` if `-bind=0.0.0.0:5555` is used, but it should really be `true` :-/
I agree, setting `bind_on_any` (and thus making `Discover()` possible unless disabled) if the user specified bind-on-all explicitly via `-bind=0.0.0.0` seems like a bug that should be fixed in master, regardless of this issue.
(https://github.com/bitcoin/bitcoin/issues/31133#issuecomment-2477231557)
> Also, this has another flaw: `bind_on_any` will be `false` if `-bind=0.0.0.0:5555` is used, but it should really be `true` :-/
I agree, setting `bind_on_any` (and thus making `Discover()` possible unless disabled) if the user specified bind-on-all explicitly via `-bind=0.0.0.0` seems like a bug that should be fixed in master, regardless of this issue.