🤔 naumenkogs reviewed a pull request: "TxDownloadManager followups"
(https://github.com/bitcoin/bitcoin/pull/31190#pullrequestreview-2417664548)
ACK 8351562bec6081eda2a600bfe4edeb264a9dee0b

I haven't verified what happens if `time` goes below 0

🚀 fanquake merged a pull request: "ci: Use clang-19 from apt.llvm.org"
(https://github.com/bitcoin/bitcoin/pull/30634)

👍 fanquake approved a pull request: "build: Unify `-logsourcelocations` format"
(https://github.com/bitcoin/bitcoin/pull/30811#pullrequestreview-2417738480)
ACK 788c1324f3d840f7a39b8bc3537dcff26ca0b552

✅ fanquake closed an issue: "Unify -logsourcelocations format"
(https://github.com/bitcoin/bitcoin/issues/30799)

🚀 fanquake merged a pull request: "build: Unify `-logsourcelocations` format"
(https://github.com/bitcoin/bitcoin/pull/30811)

💬 laanwj commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#issuecomment-2459213686)
Concept ACK

💬 dergoegge commented on pull request "TxDownloadManager followups":
(https://github.com/bitcoin/bitcoin/pull/31190#issuecomment-2459317676)
oss-fuzz found a crash in txdownloadman_impl, perhaps worth including the fix here or in another followup:

```
$ FUZZ=txdownloadman_impl out/libfuzzer/fuzz clusterfuzz-testcase-minimized-txdownloadman_impl-5180010463297536.txt
src/test/fuzz/txdownloadman.cpp:290 CheckInvariants: Assertion `txdownload_impl.m_txrequest.CountInFlight(peer) <= node::MAX_PEER_TX_REQUEST_IN_FLIGHT' failed.
```

[clusterfuzz-testcase-minimized-txdownloadman_impl-5180010463297536.txt](https://github.com/user-att
...

💬 dergoegge commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#issuecomment-2459379945)
Perhaps relevant for the discussion around removing `testBlockValidity`: https://github.com/bitcoin/bitcoin/issues/31136#issuecomment-2453055947

💬 laanwj commented on pull request "tests: Handle BDB dynamic pagesize":
(https://github.com/bitcoin/bitcoin/pull/31222#discussion_r1830802184)
Might want to do some basic checks on the pagesize; eg
```python
MIN_PAGESIZE = 512
MAX_PAGESIZE = 65536
⋮
assert pagesize >= MIN_PAGESIZE and pagesize <= MAX_PAGESIZE and (pagesize & (pagesize-1) == 0):
```

💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-2459428224)
`99b1f88fe8...cf83f0c14c`: rebase due to conflicts and mock the sleeps

> > I guess it should be possible to mock CConnman::interruptNet so that its sleep_for() method returns immediately.

> Sounds good to me.

Done. `CThreadInterrupt` can now be mocked in other tests as well. Thanks for the suggestion, @dergoegge!

💬 dergoegge commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r1830823046)
Perhaps in a follow up we can make mocktime accurate to the millisecond and then advance it here to simulate an actual sleep.

👍 laanwj approved a pull request: "Update secp256k1 subtree to v0.6.0"
(https://github.com/bitcoin/bitcoin/pull/31216#pullrequestreview-2417975546)
ACK 97235c446e9986ecca09c2a4b78d6c6239853fdb97235c446e9986ecca09c2a4b78d6c6239853fdb
good to have the secp256k1_memclear commits

💬 fanquake commented on pull request "Update secp256k1 subtree to v0.6.0":
(https://github.com/bitcoin/bitcoin/pull/31216#issuecomment-2459458747)
Guix Build:
```bash
df8b274bbadabebfc2ec3d28591f6f66a983f2500779fb95016aaef8c728ad8d guix-build-97235c446e99/output/aarch64-linux-gnu/SHA256SUMS.part
26aeeb60bf02b1d6e1c072a5c42a3ec14165f208efb5f17992f1716c2db7b826 guix-build-97235c446e99/output/aarch64-linux-gnu/bitcoin-97235c446e99-aarch64-linux-gnu-debug.tar.gz
043e16b13eed1ff5b3a1c4ce67309d595fffbdbbd0cd6800221a9eabb84a35eb guix-build-97235c446e99/output/aarch64-linux-gnu/bitcoin-97235c446e99-aarch64-linux-gnu.tar.gz
3f2b606211c543d8
...

💬 vasild commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1830836157)
I think this is worth a followup. If you agree, I would open one, or just leave it as it is? What to reword it to? "Ending handling of connection to ..."? Or log different messages based on `self.serv.keep_alive`?

📝 fanquake opened a pull request: "ci: `add second_deadlock_stack=1` to TSAN options"
(https://github.com/bitcoin/bitcoin/pull/31232)
This is mentioned in the developer notes, but isn't present in `TSAN_OPTIONS`, resulting in:
```bash
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=60508)
Cycle in lock order graph: M0 (0xffff98e02208) => M1 (0xffff98e0cbe8) => M2 (0xffff98e0cd98) => M0
<snip>

Hint: use TSAN_OPTIONS=second_deadlock_stack=1 to get more informative warning message
```

Add it, for (potentially) more informative output, when failures occur. Checked that adding does output mor
...

🚀 fanquake merged a pull request: "Update secp256k1 subtree to v0.6.0"
(https://github.com/bitcoin/bitcoin/pull/31216)

💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2459516090)
The current implementation seems to be using the aggregate pubkey (before the tweaks) inside the key of the `PSBT_IN_MUSIG2_PUB_NONCE` (and I'd assume `PSBT_IN_MUSIG2_PARTIAL_SIGNATURE`, but I didn't reach there, yet); instead, [BIP-373](https://github.com/bitcoin/bips/blob/master/bip-0373.mediawiki) says that it must be _the key found in the script and not the aggregate public key that it was derived from, if it was derived from an aggregate key_. Therefore, I interpreted it as the taproot pubk
...

🤔 rkrux reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2417722869)
Successful make and functional tests at 131bed19bdfc922328cad9781fa9122b6810a8fd.

I've gone through only the src/policy/*, src/rpc/*, src/validation.cpp yet. My comments are mostly around code structure. I plan to review the functional tests and src/tests/* very soon.

💬 rkrux commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830689563)
Nit: This function is independent of the ephemeral nature and is generic enough to lie outside this file, near where `IsDust()` is.

💬 rkrux commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830700460)
Although it seems correct to run the loop for the outputs of the parent transaction but as per the definition of Ephemeral Dust, we can break (and end) the loop here after the dust is found because only one is allowed per parent transaction?