💬 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.
💬 casey commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2477233215)
I had no idea this issue existed! Thanks @0xB10C for sharing and for the ping.
My motivation was the current situation with Rust Bitcoin Core JSON RPC client crates, which is not great. There's one which is mostly complete, [rust-bitcoincore-rpc](https://github.com/rust-bitcoin/rust-bitcoincore-rpc), but which is now unmaintained, and a successor project [rust-bitcoind-json-rpc](https://github.com/rust-bitcoin/rust-bitcoind-json-rpc/), which is incomplete, but looks like it's a little more pr
...
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2477233215)
I had no idea this issue existed! Thanks @0xB10C for sharing and for the ping.
My motivation was the current situation with Rust Bitcoin Core JSON RPC client crates, which is not great. There's one which is mostly complete, [rust-bitcoincore-rpc](https://github.com/rust-bitcoin/rust-bitcoincore-rpc), but which is now unmaintained, and a successor project [rust-bitcoind-json-rpc](https://github.com/rust-bitcoin/rust-bitcoind-json-rpc/), which is incomplete, but looks like it's a little more pr
...
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842776153)
I misunderstood the use of ForceRelay here. Will amend it so it applied to both
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842776153)
I misunderstood the use of ForceRelay here. Will amend it so it applied to both
💬 maflcko commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2477250414)
> * `RPCArg::m_names` has this comment: `can contain multiple aliases separated by | for named request arguments`. As a result, the generated JSON for an RPCArg has `names` key, whose value is a list of names. However, I could not find an instance of this being used in the code.
Are you sure? I checked for this in your generated json and found it https://github.com/casey/bitcoin/blob/f2f32b6cc44ef88e4c57e5b0a75935aa912e1862/api.json#L6669-L6674
> * I think that using JSON Schema would
...
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2477250414)
> * `RPCArg::m_names` has this comment: `can contain multiple aliases separated by | for named request arguments`. As a result, the generated JSON for an RPCArg has `names` key, whose value is a list of names. However, I could not find an instance of this being used in the code.
Are you sure? I checked for this in your generated json and found it https://github.com/casey/bitcoin/blob/f2f32b6cc44ef88e4c57e5b0a75935aa912e1862/api.json#L6669-L6674
> * I think that using JSON Schema would
...