💬 eval-exec commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1955435179)
Agree, I will append a commit to fix this.

💬 eval-exec commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1955435465)
Agreed, I'll refactor to avoid duplication with `GetRNDRRS()`.

⚠️ luke-jr opened an issue: "Wallet/transaction notifications: Only a single notification is displayed"
(https://github.com/bitcoin-core/gui/issues/853)
There are actually multiple issues here, at least:
1. `WalletView::processNewTransaction` only looks at the first of a batch of inserted rows.
2. `QSystemTrayIcon`'s X11 implementation (`QBalloonTip`, internal-only) only allows a single notification, destroying the previous when a new one is shown
3. Showing possibly hundreds of notifications would be terrible UX

It seems like the GUI should queue notifications at the same "instant" (eg, a new block), and if there's multiple send a summary of t
...

💬 goodthebest commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#issuecomment-2658235090)
I think it's relevant thread. Can we mine bitcoin using ```prune``` flag, what is the ideal or minimum required prune value for a pool's node to be mineable?

💬 davidgumberg commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#issuecomment-2658300687)
utreACK https://github.com/bitcoin/bitcoin/commit/851085a7f2761584402080b1767328d86cde4d94

💬 davidgumberg commented on pull request "doc: build: Fix instructions for msvc gui builds":
(https://github.com/bitcoin/bitcoin/pull/31851#issuecomment-2658310622)
I considered that, but this solution is much more sane than any of the others I managed to come up with, and the flag has been around for at least 4 years (https://github.com/microsoft/vcpkg/issues/13789#issuecomment-700140607) , and this is only documentation, if there is a better solution, especially one that doesn't require the user to intervene in any way, that would be ideal.

💬 maflcko commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#issuecomment-2658378354)
@davidgumberg Looks like you reviewed a stale commit?

💬 maflcko commented on pull request "test: deduplicates p2p_tx_download constants":
(https://github.com/bitcoin/bitcoin/pull/31758#issuecomment-2658397438)
review ACK 0a02e7fdeaca26455b3710ef76b11101ac816e53 🔖

<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: review ACK 0a02e7fdeaca
...

💬 laanwj commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#issuecomment-2658440020)
changes between 851085a7f2761584402080b1767328d86cde4d94 and current tip:
https://github.com/bitcoin/bitcoin/compare/851085a7f2761584402080b1767328d86cde4d94..b9d4d5f66a5a35c47e7abc9ec6ef5ab242b3f1e1

💬 maflcko commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2658475173)
Looks like the discussion is a bit scattered around the place. No strong opinion, but some notes (or a summary):

* A pull request is already open and I left a comment there to list the stuff I noticed that is using GCC coverage: https://github.com/bitcoin/bitcoin/pull/31394#issuecomment-2540826789
* Coverage on debuginfo-level (post-optimization) surely is different than coverage on source-code-level, but I wouldn't call it "broken".
* All of this is mostly a test-only dev-only issue, so I don'
...

📝 jirijakes opened a pull request: "doc: Fix description of byte order of hashes in ZMQ documentation"
(https://github.com/bitcoin/bitcoin/pull/31862)
ZMQ produces transaction and block hashes in the same format as RPC, that is with bytes in a reversed order from the output of hashing function. Previously, the ZMQ documentation incorrectly informed that the two are in different formats, and this changes fixes this.

Additionally, as the terms 'Big Endian' and 'Little Endian' are misleading in terms of order of bytes in an arbitrary sequence of bytes, this change also removes this notion and uses the term 'reversed byte order' instead.

Fix
...

💬 davidgumberg commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#issuecomment-2658490999)
> @davidgumberg Looks like you reviewed a stale commit?

Oops.

utreACK https://github.com/bitcoin/bitcoin/commit/b9d4d5f66a5a35c47e7abc9ec6ef5ab242b3f1e1

Recent rebase makes a comment change, and removes an unnecessary loop, addressing reviewer feedback.

💬 jirijakes commented on issue "doc/zmq: Note about endianness does not match reality":
(https://github.com/bitcoin/bitcoin/issues/31856#issuecomment-2658493762)
> current ZMQ documentation is mostly wrong describe RPC interface as using big endian format

I don't know where the notion of Big/Little Endian of hashes comes from but I have seen the RPC-style (reversed by order) to be mostly described as Big Endian, for example [learnmebitcoin](https://learnmeabitcoin.com/technical/general/byte-order/#reverse-byte-order) uses it. Anyway, it is apparently very misleading.

> Best way to fix this would be to avoid using terms little endian and big endian at a
...

👍 hebasto approved a pull request: "doc: build: Fix instructions for msvc gui builds"
(https://github.com/bitcoin/bitcoin/pull/31851#pullrequestreview-2616969253)
ACK c3fa043ae560289759b6f836ac62736d97ba91bf.

💬 maflcko commented on pull request "test: Remove stale gettime test":
(https://github.com/bitcoin/bitcoin/pull/31846#issuecomment-2658504053)
> so much is different with the `std::chrono` implementation that anything similar is unlikely to re-occur.

To give some more context, if this exact issue were to re-appear, it would now be caught by other tests (maybe).

For testing, I took the constant offset from https://github.com/bitcoin/bitcoin/issues/3494#issuecomment-31849134: `std::chrono::seconds{0x0000ffc4'00000000}` and applied it to the system clock as offset. Given that the system clock is normally in nanosecond precision, thi
...

💬 maflcko commented on pull request "[POC] cmake: Introduce LLVM's Source-based Code Coverage reports":
(https://github.com/bitcoin/bitcoin/pull/31394#issuecomment-2658549377)
> I suggest treating the broken GCC-based coverage as a 29.0 blocker, as it seems it's been effectively unusable anyway.

I don't see why this should be a 29.0 blocker at all, or why GCC is unusable, given that it is used, for example, see the previous comment above: https://github.com/bitcoin/bitcoin/pull/31394#issuecomment-2540826789

Let's continue discussion in the thread https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2658475173?

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r1955758861)
AFAIU the second flush is the original one, while the second flush was introduced in https://github.com/bitcoin/bitcoin/commit/3192975f1d177aa9f0bbd823c6387cfbfa943610 to help cleanly process all remaining callbacks. Maybe it could have removed the second flush in that commit?

That said I think it is good practice to be defensive here and keep both.

👍 TheCharlatan approved a pull request: "cmake: add a component for each binary"
(https://github.com/bitcoin/bitcoin/pull/31844#pullrequestreview-2617102083)
Re-ACK 9b033bebb18dfd609c02736292f37cc6589fcc8d

💬 hebasto commented on pull request "depends: add missing Darwin objcopy":
(https://github.com/bitcoin/bitcoin/pull/31840#issuecomment-2658646546)
> iirc one of the issues with `install-name-tool` was that it was only available version-suffixed on certain distros (even when the main llvm install was not suffixed), making it hard to find.

If it's not too much trouble, could you recall which distros those were?

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2658649883)
> Can you update the PR description to say that this is based on #31854? The convention then is to mark this PR draft until that one is merged.

Added a note at the bottom of the description of this PR. This PR was not a draft before #31854. The creation of #31854 did not change anything about this PR, so I think the existence of a chop off PR shouldn't render this one as a draft. There is no dependency between the two - either one can be merged first.

> Additionally, it's 25% easier to rev
...