🤔 janb84 reviewed a pull request: "contrib: Count entry differences in asmap-tool diff summary"
(https://github.com/bitcoin/bitcoin/pull/33939#pullrequestreview-3511446986)
utACK fd4ce55121e7b0fe0e5b1ecf648dc3178ed37fd8

changes since last ACK:
- changes to use net.num_addresses

(thanks for using my suggestion! )

📝 fanquake opened a pull request: "guix: reduce allowed exported symbols"
(https://github.com/bitcoin/bitcoin/pull/33950)
Need to double-check, but pretty sure this is atl east partly from #33181.

💬 fanquake commented on pull request "guix: reduce allowed exported symbols":
(https://github.com/bitcoin/bitcoin/pull/33950#issuecomment-3581825335)
Guix Build (aarch64):
```bash
5e1eb207d68450278951fe45b8c06465beeb49af6a84ef12d037430dcd6519ad guix-build-2984690b20f7/output/aarch64-linux-gnu/SHA256SUMS.part
dd93dd1b964f0629eae5db5106ad1afc307f4317fdecd5f44223aa1942f56348 guix-build-2984690b20f7/output/aarch64-linux-gnu/bitcoin-2984690b20f7-aarch64-linux-gnu-debug.tar.gz
af0c96a7f3f1eb528e21af366d4fd6f585dd5b6620d147822f028bb4c14023f0 guix-build-2984690b20f7/output/aarch64-linux-gnu/bitcoin-2984690b20f7-aarch64-linux-gnu.tar.gz
f3653b
...

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565434827)
very cool, this also helps with retaining the if-happy-path-else-nullopt scenarios!

Nits: we can likely use `auto&` in the comparison as well and use the `it` inside the if to narrow its scope (and use `emplace_back` to avoid copy and `Now<NodeSeconds>()` for easily comparable picked time)

```suggestion
if (const auto it{std::ranges::max_element(
m_transactions,
[](const auto& a, const auto& b) { return a < b; },
[](const auto& el) { return DerivePriorit
...

📝 theStack opened a pull request: "test: check for output to stdout in `TestShell` test"
(https://github.com/bitcoin/bitcoin/pull/33951)
This is a small follow-up PR to the recently added `TestShell` test (#33546), verifying the stdout message "TestShell is already running!" when trying to instantiate a second instance.

💬 m3dwards commented on pull request "ci: clear out space on CentOS, depends, gui GHA job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3582022147)
ACK e07e57368e9fab8ecfc140d44aef7db9b23c7ce0

Like that it runs in the background, dislike that it hides any errors. It's simple enough that I don't think it's worth being more complex to catch any problems.

As an [experiment](https://github.com/m3dwards/bitcoin/commit/129850417f09cd9bbc819794b2626d82c4de39b8) I removed CMakeFiles directories after cmake ran which deletes a few GBs in under a second which was just enough to get my fork building again. I prefer the solution in this PR howeve
...

💬 Eunovo commented on pull request "fix assumevalid is ignored during reindex":
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3582057691)
Thanks for the reviews.

@l0rinc I added your test as an automated test in https://github.com/bitcoin/bitcoin/pull/33854/commits/729bf4d627f3bc3414af22990fcf67af2d50d4cf
@hodlinator I added a useful log message in https://github.com/bitcoin/bitcoin/pull/33854/commits/729bf4d627f3bc3414af22990fcf67af2d50d4cf

👍 hodlinator approved a pull request: "contrib: Count entry differences in asmap-tool diff summary"
(https://github.com/bitcoin/bitcoin/pull/33939#pullrequestreview-3511709141)
ACK fd4ce55121e7b0fe0e5b1ecf648dc3178ed37fd8

Tested by diffing 2 previously recorded[^1] and encoded[^2] Kartograf outputs:

```shell
./contrib/asmap/asmap-tool.py diff 1762865730.dat 1762878275.dat
```

#### master

```
...
# 96634298 (2^26.53) IPv4 addresses changed; 2116571614109896006051667873103669 (2^110.71) IPv6 addresses changed
```

#### fd4ce55121e7b0fe0e5b1ecf648dc3178ed37fd8

```
...
# Summary
IPv4: 35383 entries with 96634298 (2^26.53) addresses changed
IPv6: 37688 entries with 211
...

💬 hodlinator commented on pull request "contrib: Count entry differences in asmap-tool diff summary":
(https://github.com/bitcoin/bitcoin/pull/33939#discussion_r2565613005)
suggestion: Would be nice to output how many entries there are in total, something like
```
Total entries - {args.infile1.name}: {len(state1.to_entries(overlapping=False))}, {args.infile2.name}: {len(state2.to_entries(overlapping=False))}
->
Total entries - 1762865730.dat: 706503, 1762878275.dat: 698844
```
But adding that makes it run 7 seconds slower on top of the ~29 seconds I got otherwise. Maybe there's some other way to get an approximate value.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565643372)
This code is outdated. Anyway, there are still calls `NodeClock::now()`. I think passing the present time as parameter would be too cumbersome and unnecessary at the call sites. There is mock time to use from tests. I prefer to keep the `now()` call internal to the methods. The world outside of the class does not even know it deals with time internally.

💬 brunoerg commented on pull request "fuzz: wallet: add target for `TransactionCanBeBumped`":
(https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2565667203)
Done

💬 brunoerg commented on pull request "fuzz: wallet: add target for `TransactionCanBeBumped`":
(https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2565667562)
Done

💬 brunoerg commented on pull request "fuzz: wallet: add target for `TransactionCanBeBumped`":
(https://github.com/bitcoin/bitcoin/pull/33916#issuecomment-3582150488)
Force-pushed 2df9f10755ea7de94ecc9a17e6dd6dc89de0a637..52101d8809062d524393ebfde20c926d36fe5bf8:

- Rebased - and now that we have cluster mempool, I simplified the target by mocking `HasDescendants`.
- Addressed https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2564296509 and https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2564297647

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565696859)
I prefer a programming style where things are rather on separate lines instead of combining things into one line. But `try_emplace()` is a clear winner here over `emplace()`, taken.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565698576)
Reworded.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565701315)
Outdated, no more after using `max_element()`.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565703911)
Outdated, no more after using `max_element()`.

📝 fanquake opened a pull request: "depends: update freetype and document remaining `bitcoin-qt` runtime libs"
(https://github.com/bitcoin/bitcoin/pull/33952)
Update freetype to `2.11.1`.
Updating fontconfig (currently `2.12.6`) to `2.13.1` requires what looks like a hard dep on gperf; leaving that as-is for now. If someone wants to investigate,
Document expectations in `symbol-check.py`.
Closes #29977 (and changes based on discussion there).

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565710172)
Rough overview, but reworded as suggested.

💬 fanquake commented on issue "depends: Freetype and xcbproto version in depends are too new":
(https://github.com/bitcoin/bitcoin/issues/29977#issuecomment-3582195905)
See #33952.