💬 l0rinc commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2422282858)
I have already applied your previous suggestion and added you as coauthor.

💬 GaloisField2718 commented on pull request "[30.x] Finalise v30.0":
(https://github.com/bitcoin/bitcoin/pull/33559#issuecomment-3392867370)
https://github.com/bitcoin/bitcoin/blob/v30.0/doc/release-notes.md

💬 apogio commented on pull request "Add coins (UTXOs) tab and makes it view-only":
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3393048219)
> Should probably only be available if advanced coin control is enabled in settings, and maybe only on a menu even then (if it made sense to do it as a tab, it should be within the main window, not a new window).
>
>
>
> The SVG source for the new icon should also be included.

Thanks for the comments! I will check soon!

💬 itokichi7777 commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3393221753)
tACK0f1f02995b

💬 TheCharlatan commented on pull request "Bugfix: Correct first-run free space checks":
(https://github.com/bitcoin/bitcoin/pull/29678#issuecomment-3393232862)
Concept ACK

🤔 TheCharlatan reviewed a pull request: "Policy: Report reason inputs are non standard"
(https://github.com/bitcoin/bitcoin/pull/29060#pullrequestreview-3327213980)
Concept ACK

There are also some drahtbot llm linter suggestions.

💬 TheCharlatan commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2422872962)
Since we're renaming, I would prefer `ValidateInputsStandardness` here. The `Has` prefix implies a bool return value to me.

💬 TheCharlatan commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2422900883)
I don't quite follow why we are invoking the solver here. What is this supposed to test?

💬 TheCharlatan commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2422904900)
Why is this tested here?

💬 TheCharlatan commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2422905182)
In commit 910849007151cb8868f703ed759ee31f630eaea7:

Nit: Typo in the commit message s/resin/reason/.

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2422946155)
ha, that's awesome. The queue's `std::packaged_task<void()>` change broke my mind because the task has obviously a different return value..
I see the trick now, it seems the `packaged_task<R()>` is wrapped into another `packaged_task<void()>` which forwards the call internally. That's elegant.

💬 apogio commented on pull request "Add coins (UTXOs) tab and makes it view-only":
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3393496902)
So I've made the changes.

**Specifically:**

1. I added the .svg file, like discussed.
2. I set the coins tab to be in the same window and not open a new one.
3. I set the coins tab to be visible only if "coin control features" are enabled in the advanced options.

**The way it works is:**

When coin control features are disabled, then the "Coins" tab will be hidden, like in this picture:

<img width="600" height="400" alt="new_coins_1" src="https://github.com/user-attachments/asse
...

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3393553555)
Thank you for considering the suggestions, I find the new layout a lot easier to follow.

I have left a few more nits, but I'm fine doing them in a follow-up.
I like these memory optimizations, my only concern is that we're likely doing a lot heavier copying because of the regular compactions (especially since most move constructors seem unused based on previous fuzzing reports). As mentioned, in a follow-up we can experiment with leaving a tiny buffer.

I have also added a few redundant as
...

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422338701)
This method is called from two places, one of which is dropping the pre-cacululated cluster level that we could access through `FindClusterAndLevel` - the other call doesn't seem to, I guess that's why you decided to always recalculate.
It seems to me it wouldn't complicate much to provide it when it's available and recalculate when it's not, something like:
```patch
diff --git a/src/txgraph.cpp b/src/txgraph.cpp
--- a/src/txgraph.cpp (revision 262762bbb6f7157ba8c54972909c9214448c304b)
+++ b/src
...

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422371348)
nit: for consistency:
```suggestion
Assume(!GetTxCount());
```

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422371564)
nit: likely missed because of the extra space
```suggestion
if (!GetTxCount()) return 0;
```

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422454996)
nit:
```suggestion
void GenericClusterImpl::AddDependencies(SetType parents, DepGraphIndex child) noexcept
```

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422374209)
nit: I'd put the assume before the first usage of the validated value

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422890998)
nit: it seems to me this can now be private

💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422453665)
nit: for consistency we could add the name hints here as well