💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2582770939)
Why not simply call `NumToOpenAdd(num_for_rebroadcast)`. Given that we only reattempt to reconnect every 12.5 minutes (and we add just 1 more attempt per tx, compared to the initial 3 attempts), which seem both quite conservative choices, wouldn't it be better to overshoot? If we are currently doing other (non-stale) private broadcasts (which will have fewer attempts and therefore higher priority), these could otherwise prevent the rebroadcast of stale transactions.

💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2582800611)
I think it would be better to have a bunch randomly created addresses (obviously with a valid checksum) instead of addrs that at sometime belonged to actual node runners on mainnet - less confusing (node runners may search the internet for their node) and in case of a future bug that leads to actual connections, it's less likely that actual nodes are contacted.

💬 pablomartin4btc commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#discussion_r2583352655)
Done.

💬 pablomartin4btc commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#discussion_r2583352810)
Done.

💬 pablomartin4btc commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#discussion_r2583553569)
Done in 2nd. commit: _"gui: Update QMessageBox::Icon with chaintype image"_.

💬 Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3605610816)
Here's a slightly more realistic plot from last night on a well connected node running on an Intel i5-8400:

<img width="3842" height="1369" alt="getmemoryload-scatter" src="https://github.com/user-attachments/assets/f1ae2d88-73fa-4d2c-bd5f-bce5a168eeee" />

It's connected to DMND pool, declaring custom templates and getting them approved, but not actually mining. Due to their rate limiting I set `-sv2interval=20`, so if fees go up, it waits at least 20 seconds before generating a new templa
...

💬 willcl-ark commented on pull request "guix: reduce allowed exported symbols":
(https://github.com/bitcoin/bitcoin/pull/33950#issuecomment-3605815880)
Guix hashes:

```
x86_64
b507c965417a1adcd6b10000f9a8bae29e18ba9a1e76060c38af7fe1fbbcd0e4 guix-build-7b90b4f5bb10/output/aarch64-linux-gnu/SHA256SUMS.part
c60c33e4ececbc650bbeda06c7aa7dad714ace312a08c804afd750fc0a0c3d68 guix-build-7b90b4f5bb10/output/aarch64-linux-gnu/bitcoin-7b90b4f5bb10-aarch64-linux-gnu-debug.tar.gz
77bdfc199c87064ee166c6c124aa9d586fb24b144d8408bdd3d0e2e0065577a5 guix-build-7b90b4f5bb10/output/aarch64-linux-gnu/bitcoin-7b90b4f5bb10-aarch64-linux-gnu.tar.gz
5b6c18817
...

👍 rkrux approved a pull request: "doc: improvements to doc/descriptors.md"
(https://github.com/bitcoin/bitcoin/pull/33986#pullrequestreview-3534021842)
lgtm ACK fc33626f66e7f93f39989fbbabfbf00222762071

💬 Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#discussion_r2584340016)
That's useful. So far `interface_ipc.py` never had a transaction appear in multiple templates, so the destructor would always remove the last reference.

I adjusted the test so that it does. Now your mutation causes a crash during this test.

👍 rkrux approved a pull request: "contrib: fix manpage generation"
(https://github.com/bitcoin/bitcoin/pull/33996#pullrequestreview-3534104599)
tACK 10e6417b3b33a7df18a61d657153f3d90b89dbf2

The regex seems a little loose but ok.

👍 rkrux approved a pull request: "init: point out -stopatheight may be imprecise"
(https://github.com/bitcoin/bitcoin/pull/33993#pullrequestreview-3534134158)
crACK e4703863655135856d0d6ad54a7021d3326bf071

Helpful note.

🤔 janb84 reviewed a pull request: "contrib: fix manpage generation"
(https://github.com/bitcoin/bitcoin/pull/33996#pullrequestreview-3534147771)
ACK 10e6417b3b33a7df18a61d657153f3d90b89dbf2

This PR fixes gen-manpages to process the version in a different way.

Master, breaks:
<details>

```sh
./contrib/devtools/gen-manpages.py
Traceback (most recent call last):
File "/Users/arjan/Projects/bitcoin/./contrib/devtools/gen-manpages.py", line 65, in <module>
assert verstr.startswith('v')
~~~~~~~~~~~~~~~~~^^^^^
AssertionError
```
</details>

This PR, works:
<details>

```sh
./contrib/devtools/gen-manpag
...

💬 janb84 commented on pull request "contrib: fix manpage generation":
(https://github.com/bitcoin/bitcoin/pull/33996#discussion_r2584390842)
Non-blocking NIT: maybe use raw strings version of regex for clarity and following [best practices](https://mimo.org/glossary/python/regex-regular-expressions)

```suggestion
search = re.search(r"v[0-9]\S+", output)
```

💬 vasild commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3605988525)
> should I drop the last commit

I am fine either way. Would be interested to see what other reviewers think.

🚀 fanquake merged a pull request: "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO"
(https://github.com/bitcoin/bitcoin/pull/33702)

👍 hodlinator approved a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3534148441)
re-ACK f07c765aa6bdba2511ceec56aa7f9755fa29a81e

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2584406304)
I'm fine with both 1 commit or breaking apart as suggested.

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2584396172)
meganit: double newline

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2584393520)
nit: I prefer using `std::variant` instead of out-parameters. See ab11b4bd7a5844d6ce654b39fca034d73defe973 / https://github.com/bitcoin/bitcoin/compare/master...hodlinator:bitcoin:pr/33657_suggestions2

👍 sedited approved a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3534235283)
Re-ACK f07c765aa6bdba2511ceec56aa7f9755fa29a81e