💬 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.

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956359892)
```suggestion
* does exist in main, the cluster it is in is unmodified in staging.
```

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956400806)
hmm, above you mentioned "If it doesn't exist in main, it doesn't exist in staging either.".

And now there is a state where the transaction exits in staging but not in main

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956469873)
```suggestion
Assume (level - MAX_LEVELS - 1);
auto& entry = m_entries[idx];
```

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956442485)
```suggestion
int level = MAX_LEVELS - 1;

```

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956478391)
Should be set to avoid duplicates, will prevent de duplicating later on?