Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 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.
💬 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.
```