💬 l0rinc commented on pull request "refactor: Enforce readability-avoid-const-params-in-decls":
(https://github.com/bitcoin/bitcoin/pull/31650#discussion_r2018767338)
Was the `const std::string_view` kept in the `.cpp` file on purpose?
I've noticed we usually avoid consting a view...

I see we even have `const std::string_view&` - would it make sense to unify those usages here as well?

💬 l0rinc commented on pull request "refactor: Enforce readability-avoid-const-params-in-decls":
(https://github.com/bitcoin/bitcoin/pull/31650#discussion_r2019211614)
A similar one may be
```patch
diff --git a/src/netbase.h b/src/netbase.h
--- a/src/netbase.h (revision 29a5a50583920e839fc3020c08f28b2f12a64d74)
+++ b/src/netbase.h (date 1743172625447)
@@ -60,7 +60,7 @@
public:
Proxy() : m_is_unix_socket(false), m_randomize_credentials(false) {}
explicit Proxy(const CService& _proxy, bool _randomize_credentials = false) : proxy(_proxy), m_is_unix_socket(false), m_randomize_credentials(_randomize_credentials) {}
- explicit Proxy(const std:
...

📝 olaristo109 opened a pull request: "Fix legacy migration bug"
(https://github.com/bitcoin/bitcoin/pull/32161)
Fix: Prevent crash on null legacy_spkm in GetDescriptorsForLegacy

This pull request resolves a critical issue that caused Bitcoin Core to crash during legacy wallet migration. The 'GetDescriptorsForLegacy' function was not properly handling cases where 'legacy_spkm' was a null pointer, leading to a dereference error.

This PR implements the following changes:

* Adds an explicit null pointer check for 'legacy_spkm'.
* Adds detailed error logging, including the key associated with the nul
...

💬 davidgumberg commented on pull request "Fix legacy migration bug":
(https://github.com/bitcoin/bitcoin/pull/32161#discussion_r2019321308)
Why?

💬 glozow commented on issue "RPC: `getblockstats` might not return the *effective* percentile fee rate":
(https://github.com/bitcoin/bitcoin/issues/31140#issuecomment-2762446017)
I hope this doesn't sound too dismissive, but who uses this and what are they using it for? Can we delete these fields or the whole RPC? To visualize stats, I think using `getblock` to obtain the fees and sizes for each transaction is much more flexible and appropriate (though tbf I don't think this was the case when #10757 was originally opened).

Separately, linearizing and chunking transactions (whether it's a block or something else) is pretty complicated and perhaps worth exposing it or off
...

🤔 TheCharlatan reviewed a pull request: "build: Switch to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/30997#pullrequestreview-2726753659)
Guix builds:
```
aarch64
6ea4a76be3383337e57d6a12450bd589776ebb3fd0d9161347766ef845241e13 guix-build-c4861570e468/output/aarch64-linux-gnu/SHA256SUMS.part
3eb7656483dfe47fa6b7cf40bceb3decda73474c813edb224d42840adb8b49d6 guix-build-c4861570e468/output/aarch64-linux-gnu/bitcoin-c4861570e468-aarch64-linux-gnu-debug.tar.gz
0aa522010efd138d78eeac0a8ea15df469298c50afaa2451dece78c564546cac guix-build-c4861570e468/output/aarch64-linux-gnu/bitcoin-c4861570e468-aarch64-linux-gnu.tar.gz
5e1e833519
...

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019375703)
Done.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019376118)
Done.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019371904)
Added a comment.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019372075)
Done.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019372720)
No, it's being assigned to below.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019374546)
I've changed the code so that `Include()` and `Skip()` are no-ops once the end is reached.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019375780)
Done.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019374728)
Same here, made it have no effect in that case.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019375949)
Done.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019375612)
This has been rewritten.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019377271)
Done.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019379608)
That's strange, because the builder outliving the graph is definitely a problem (the observer counter in the graph, which has been destroyed, will be subtracted from). I don't see an easy way of allowing this (it'd need the graph to maintain a set of observers, which seems overkill).

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019379802)
Done.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019380171)
Done.