🤔 fanquake requested changes to a pull request: "doc: Document compiler configuration for native depends packages"
(https://github.com/bitcoin/bitcoin/pull/33902#pullrequestreview-3492766868)
See https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2549756641.

✅ diegoviola closed a pull request: "Fix bitcoin-qt visual glitches on Wayland"
(https://github.com/bitcoin-core/gui/pull/904)

💬 diegoviola commented on pull request "Fix bitcoin-qt visual glitches on Wayland":
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3563050872)
Closing in favor of [#904.](https://github.com/bitcoin-core/gui/pull/914).

💬 apogio commented on pull request "Add coins (UTXOs) tab and makes it view-only":
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3563051500)
I'm not sure what the comment about motivation means, but I appreciate your time.

💬 diegoviola commented on pull request "Revert "gui, qt: brintToFront workaround for Wayland"":
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3563080669)
@hebasto ACK. Thanks for looking into this and for the revert. :+1:

💬 frankomosh commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#discussion_r2549829849)
```suggestion
bool mutated = fuzzed_data_provider.ConsumeBool(); // output param, initial value ignored
```

🤔 l0rinc reviewed a pull request: "coins: use number of dirty cache entries in flush warnings/logs"
(https://github.com/bitcoin/bitcoin/pull/33512#pullrequestreview-3492784897)
Thanks for the comments @optout21, since I wasn't invalidating any ACKs, I have applied your suggestions (with rebase) - let me know if this is what you meant.

> keeping a count like the dirty count is very efficient, but prone to going out of sync

yeah, we have a few of these - but the sanitizers and sanity checks and fuzzers usually reveal the problems (sanitizers break on underflow and the sanity checks recalculate everything during testing).

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2549809335)
I have grouped dirty setter and dirty count updates deliberately:
```
% git grep -A1 'CCoinsCacheEntry::SetDirty' src/coins.cpp
src/coins.cpp: CCoinsCacheEntry::SetDirty(*it, m_sentinel);
src/coins.cpp- ++m_dirty_count;
--
src/coins.cpp: CCoinsCacheEntry::SetDirty(*it, m_sentinel);
src/coins.cpp- ++m_dirty_count;
--
src/coins.cpp: CCoinsCacheEntry::SetDirty(*it, m_sentinel);
src/coins.cpp- ++m_dirty_count;
--
src/coins.cpp: CCoinsCache
...

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2549782860)
Fair, changed both:
```C++
//! Size of the cache (in number of transaction outputs)
unsigned int GetCacheSize() const;

//! Number of dirty cache entries (transaction outputs)
size_t GetDirtyCount() const noexcept { return m_dirty_count; }
```

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2549775651)
You mean like:
```C++
BOOST_CHECK_EQUAL(dirty_count, 0U);
```

💬 hebasto commented on pull request "Add coins (UTXOs) tab and makes it view-only":
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3563118321)
> I'm not sure what the comment about motivation means...

It means that the following only describes the feature itself, without providing the goals, use cases, or rationale behind it:
> GUI: Add coins tab and sets view-only mode
>
> Adds Coins (UTXOs) tab at the top and:
>
> * Removes selection checkboxes from the dialog
>
> * Disables coin selection functionality
>
> * Maintains UTXO viewing capabilities
>
> * Removes all the information from the top area of the
...

💬 l0rinc commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#discussion_r2549840441)
Why is that better?

💬 apogio commented on pull request "Add coins (UTXOs) tab and makes it view-only":
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3563134954)
> > I'm not sure what the comment about motivation means...
>
>
>
> It means that the following only describes the feature itself, without providing the goals, use cases, or rationale behind it:
>
> > GUI: Add coins tab and sets view-only mode
>
> >
>
> > Adds Coins (UTXOs) tab at the top and:
>
> >
>
> > * Removes selection checkboxes from the dialog
>
> >
>
> > * Disables coin selection functionality
>
> >
>
> > * Maintains UTXO viewing capabilities
>
> >
>
> > *
...

💬 frankomosh commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#discussion_r2549851020)
nit

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549873306)
My motivation for the if-init version was just to make the “happy path” a single block, to me that reads as “if we have a node for this id and a matching tx, return it, otherwise return nullopt”, which matches how I’d describe the code in English. The previous version is more verbose in English and focuses on the unlikely scenarios which I find distracting.

On the first/second bit: those can also be avoided with destructuring when they start to hurt readability, e.g.:
```C++
const auto [nod
...

👋 hebasto's pull request is ready for review: "ci: Add Windows + UCRT jobs for cross-compiling and native testing"
(https://github.com/bitcoin/bitcoin/pull/33764)

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549880323)
I think the same applies here as in https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549873306, the suggested version focuses on the happy path (which is the purpose of the method - and it's also easer to say out loud), which I found helps with comprehension.

💬 l0rinc commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#discussion_r2549881417)
Why is that better?

💬 hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3563180629)
> > > Not sure I understand the dependence on #33775?
> > > CI is using it's own build, not the guix build right? Also MinGW brings it's own GCC doesn't it?
> >
> >
> > The CI jobs are more useful when using toolchains with versions similar to those in the Guix script.
>
> I think this is just a doc question. I think the `trixie/g++-mingw-w64-ucrt64` task can be left as-is, without adding a dependency here. In the future, #33593 could simply update the comment to clarify which one is ru
...

💬 frankomosh commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#discussion_r2549888077)
> Why is that better?

"ignored" clarifies the value is not read at all, while "shouldn't matter" could mean read-but-no-effect. Very minor preference though, I think either can works just fine