💬 hebasto commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#discussion_r1956420649)
> @hebasto Looking at this again, is there any reason [this needs to be an object library](https://github.com/bitcoin/bitcoin/blob/master/cmake/minisketch.cmake#L43)?

I don't thin so.

> It triggers the bug that necessitates the workaround here. I'm wondering if rather than using our hack or the (better) workaround suggested upstream, if we should just make it an archive library and avoid the issue altogether?

From [Professional CMake: A Practical Guide 20th Edition](https://discourse.cm
...

👍 TheCharlatan approved a pull request: "build: move `rpc/external_signer` to node library"
(https://github.com/bitcoin/bitcoin/pull/31865#pullrequestreview-2618309064)
ACK e501246e77cc36cff222fb07aed2fd1316e11e19

💬 ajtowns commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2659802629)
ACK 4080b66cbec2b6fc2fcfd7356941236f65d508e3 - light review

Did you know 48% of utxos are under 1000 sats ($1)? With this PR you can find that out, and also how recent those utxos are:

```
sqlite> select count(*) from utxos;
178306838
sqlite> select count(*)/1783068 from utxos where value < 1000;
48
sqlite> select count(*)/1783068, (height/50000)*50000 from utxos where value < 1000 group by height/50000;
0|50000
0|100000
0|150000
0|200000
0|250000
0|300000
0|350000
0|400000
0
...

📝 Syed-AbdurRahaman2006 opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/31867)
Line 6:
Before: For an immediately usable, binary version of the Bitcoin Core software, see https://bitcoincore.org/en/download/. After: For an immediately usable binary version of the Bitcoin Core software, visit [Bitcoin Core Downloads](https://bitcoincore.org/en/download/). Removed the comma after "usable" for better flow, replaced "see" with "visit" for action-oriented language, and turned the URL into a clickable link. Line 16:
Before: Further information about Bitcoin Core is available i
...

📝 l0rinc opened a pull request: "optimization: speed up block serialization"
(https://github.com/bitcoin/bitcoin/pull/31868)
This PR contain a few different optimization I found by profiling IBD and the newly added block seralization benchmarks.

The commits merge similar (de)serialization methods and separates them internally with `if constexpr` - similarly to how it has been [done here before](https://github.com/bitcoin/bitcoin/pull/28203). This enabled further `SizeComputer` otimizations as well.

Other than these, since single byte writes are used very often (used for every `(u)int8_t` or `std::byte` or `bool
...

💬 theuni commented on pull request "cmake: add a component for each binary":
(https://github.com/bitcoin/bitcoin/pull/31844#issuecomment-2659817050)
> > @stickies-v Thanks for pointing that out, that was accidental! I actually didn't notice the difference. I think we should fix it so that they're aligned instead of having a single outlier.
> > ~Though.. would it make sense to rename the target to `libbitcoinkernel` instead of changing the component to `bitcoinkernel`? I think the former would be more obvious?~
> > Edit: nm, can't do that as CMake will rename the resulting file to liblibbitcoinkernel.so.
>
> But we can:
>
> ```cmake

...

💬 l0rinc commented on pull request "doc: Fix description of byte order of hashes in ZMQ documentation":
(https://github.com/bitcoin/bitcoin/pull/31862#issuecomment-2659824080)
Concept ACK

💬 l0rinc commented on pull request "Update README.md":
(https://github.com/bitcoin/bitcoin/pull/31867#issuecomment-2659840197)
NACK - opinionated random changes

✅ fanquake closed an issue: "CMake `CheckPIESupported` doesn't always work"
(https://github.com/bitcoin/bitcoin/issues/30771)

🚀 fanquake merged a pull request: "cmake: Add `CheckLinkerSupportsPIE` module"
(https://github.com/bitcoin/bitcoin/pull/31359)

✅ fanquake closed a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/31867)

💬 mzumsande commented on pull request "wallet: fix rescanning inconsistency":
(https://github.com/bitcoin/bitcoin/pull/31629#issuecomment-2659846075)
> I think we could solve the first case here, so it can get inside the upcoming release, and leave the second case for a follow-up. Also, I'm not really against the fix proposed in this PR, it works. It just feels a bit hacky.

See https://github.com/mzumsande/bitcoin/tree/202502_rescan_initfix for this alternative. Happy to change it to that if people like it better. This fixes the issue during `AttachChain()` and is admittedly nicer, but it wouldn't fix the following race:
1) User runs `res
...

📝 hebasto opened a pull request: "cmake: Add `libbitcoinkernel` target"
(https://github.com/bitcoin/bitcoin/pull/31869)
This PR amends https://github.com/bitcoin/bitcoin/pull/31844 by:
1. Adding a convenience `libbitcoinkernel` target as a synonym for `bitcoinkernel`.
2. Renaming the `bitcoinkernel` component to `libbitcoinkernel`, as initially intended in https://github.com/bitcoin/bitcoin/pull/31844

💬 hebasto commented on pull request "cmake: add a component for each binary":
(https://github.com/bitcoin/bitcoin/pull/31844#issuecomment-2659856484)
> Want to PR that along with a comment along the lines of "Add a convenience libbitcoinkernel target as a synonym for bitcoinkernel" and I'll give it a quick ACK?

https://github.com/bitcoin/bitcoin/pull/31869.

💬 theuni commented on pull request "optimization: speed up block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#discussion_r1956480589)
These are nice optims, but [as I mentoned here](https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1929225185):

> In the future we could potentially add specializations for types with compile-time-known sizes via concepts.
>
> Presumably some of the streams could benefit from static extents, but that's waaay overkill for here.

I think it makes sense to wait for the `std::span` replacement (#31519) to do this, that way we can specialize for any static extent instead which should c
...

💬 theuni commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#issuecomment-2659867175)
@fanquake do you have an opinion on this for v29? Getting it in would make future backports easier. Could also see waiting until after branch-off to be less disruptive though.

💬 hebasto commented on pull request "cmake: add a component for each binary":
(https://github.com/bitcoin/bitcoin/pull/31844#issuecomment-2659879903)
> ACK [9b033be](https://github.com/bitcoin/bitcoin/commit/9b033bebb18dfd609c02736292f37cc6589fcc8d).

My apologies not very solid testing, but this branch seems not always working:
```
$ cmake -B build -G Ninja -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_SHARED_LIBS=OFF
$ cmake --build build --target bitcoinkernel
$ cmake --install build --component bitcoinkernel --prefix /home/hebasto/INSTALL
-- Install configuration: "RelWithDebInfo"
CMake Error at build/src/kernel/cmake_install.cmake:46 (file)
...

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: introduce TxGraph"
(https://github.com/bitcoin/bitcoin/pull/31363#pullrequestreview-2618143580)
Code Review e19bbc328236f64716034277857951184309cd14

It seems to me that this is designed with the flexibility to accommodate more levels, which explains the use of `std::vector` for `ClusterSet`

If yes then current approach is perfect else would it be better for `TxGraph` to maintain a `std::array` of two `ClusterSet? `main` and `staging and store their indexes in `TxGraph` as enums? This way, `Cluster` would only need to maintain that enum.

In `ClusterSet`, we could introduce a bool
...

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956343988)
What do you mean by implicitly

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956385389)
Should just refer to main and staging only instead of level-1, main, staging and top level?
It will be consistent this way.