👍 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
...

💬 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?

💬 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

💬 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.

💬 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
...

💬 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

💬 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
...

💬 casey commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2477278009)
> 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

Oh nice! I overlooked that, I was looking at the codebase, not the generated schema.

> > * I think that using JSON Schema would be ideal
>
> I agree, for the reasons given in [#29912 (comment)](https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2418963490). If the current specs don't fit within JSON Schema, i
...

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842808065)
Sorry, you're right. I've been looking at this being conditional to `m_txreconciliation->TryRemovingFromSet(peer_id, wtxid`, which is what happens with the descendants, but here it is unconditional.

It should be parsed only if `TryRemovingFromSet` succeeds.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842810444)
Not sure I follow how the first comment is an issue, but I do agree with the second. This needs to be conditional to `TryRemovingFromSet` succeeding.

💬 0xB10C commented on issue "Tracepoint Interface Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/31274#issuecomment-2477300406)
> > We could internalize the relevant macro parts of systemtap's sys/sdt.h for the Linux tracepoints. This would allow us to drop the external dependency on systemtap
>
> This still makes sense to me, it's just a few macros which haven't changed in years and are super unlikely to change significantly, because they insert ELF annotations (according to a well-known spec) and NOP instructions.
>
> Dropping a guix dependency would be good!

I just found out about https://github.com/libbpf/us
...

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842841578)
That is fair, I'll change it to bool to be explicit

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842867828)
Done

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842870647)
Mave the move dependant on `TryRemovingFromSet`

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842873615)
The whole thing from L151-L160?

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842875471)
I'm guessing this is related to https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842873615, in which case I'll drop that

📝 furszy opened a pull request: "test: add global time to place exec tests within the same dir"
(https://github.com/bitcoin/bitcoin/pull/31291)
Solving https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1836348991.

Modifies the unit/bench/fuzz tests default datadir path.

Groups all tests executed within each binary call under
a single directory prefixed by the current time.

Replicating the function test framework behavior.

Switching from:
```tmp/test_common bitcoin/test_name/current_time```
To:
```tmp/test_common bitcoin/current_time/test_name```

👍 TheCharlatan approved a pull request: "guix: scope pkg-config to Linux only"
(https://github.com/bitcoin/bitcoin/pull/31276#pullrequestreview-2437211248)
ACK bcd82b13f4649e57d7d106856aab7b2a6296d728

💬 achow101 commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1842895148)
If i need to retouch.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842897353)
I changed `ReconSetSize` to return a tuple, and updated this to read:

```
"Now the set contains %i reconcilable transactions (plus %i delayed transactions).\n",
```