Bitcoin Core Github
42 subscribers
124K links
Download Telegram
💬 maflcko commented on pull request "[CI] remove `doc/release-notes.md` from lint-spelling.py":
(https://github.com/bitcoin/bitcoin/pull/33968#issuecomment-3595293116)
> will fail CI if there are spelling errors

I don't think this is true either. The comment in the file literally says:

> Note: Will exit successfully regardless of spelling errors.
👍 sedited approved a pull request: "cmake: Make `BUILD_KERNEL_TEST` depend on `BUILD_KERNEL_LIB`"
(https://github.com/bitcoin/bitcoin/pull/33972#pullrequestreview-3523828047)
ACK fe1815d48f0cee57d2f1af50b377c7f9e462369e
💬 sedited commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-3595384275)
This has been repeatedly close to merging, and I am a bit puzzled it didn't make it over the line a few times already (though I have myself to blame for that too by not being consistent on the review). At this point I wonder though if using `std::expected` instead would have more success. Afaict our minimal compiler versions are close to supporting c++23 and `std::expected` in particular.
📝 codomposer opened a pull request: "test: add test for coin write failure"
(https://github.com/bitcoin/bitcoin/pull/33980)
# Coin Write Failure Test Implementation Summary

## Overview

This implementation adds comprehensive test coverage for coin database write failures in Bitcoin Core. Previously, there were **no tests** for this critical failure scenario.

## What Was Added

### 1. Unit Test Suite (`src/test/coins_write_failure_tests.cpp`)

A complete unit test suite with **8 test cases** covering:

- Flush failure handling
- Sync failure handling
- Recovery after write failures
- Partial
...
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3595490396)
> > The UCRT CI job is up [now](https://github.com/bitcoin/bitcoin/pull/33764).
>
> You'll have to rebase this one and adjust the ci comments in the ci config files, no?

Thanks! Done.
💬 laanwj commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3595491237)
> Seeing the following difference:

That's such a strange difference. Just register selection.

We should probably aim to create a minimum reproduction, to be able to report and troubleshoot this.
🤔 l0rinc reviewed a pull request: "validation: fetch block inputs on parallel threads >40% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132#pullrequestreview-3523358497)
Redid he measurements on a Mac with AppleClang for different sizes to check why there's such a massive speedup for lowe memory:
```bash
for DBCACHE in 450 4500 45000; do \
COMMITS="d5ed4ba9d8627f1897322ce7eb5b34e08e4f73ac b1a791db1c75a47569b690baf7b074b78e08ca5a"; \
STOP=921129; \
DATA_DIR="$HOME/Library/Application Support/Bitcoin"; LOG_DIR="$HOME/bitcoin-reindex-logs"; \
mkdir -p "$LOG_DIR"; \
COMMA_COMMITS=${COMMITS// /,}; \
(echo ""; echo "$COMMITS" | tr ' ' '\n' | wh
...
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2575931153)
`ConnectBlock` calls `AccessCoin` for every input, shouldn't the test do the same? Aren't we cheating by only calling it when we already know it's not an internal spend?
```C++
for (const auto& tx : block.vtx) {
if (tx->IsCoinBase()) {
BOOST_CHECK(!cache.GetPossiblySpentCoinFromCache(tx->vin[0].prevout));
} else {
for (const auto& outpoint : tx->vin | std::views::transform(&CTxIn::prevout)) {
const auto external{!txids.contains(o
...
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2576139456)
we're only using `100` here, we might as well:

```suggestion
static constexpr auto num_txs{100};
CBlock CreateBlock() const noexcept
```
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2575987031)
why are coinbases and internal spends added to the cache, that's not what happens in reality, right?
It should represent the state before the block is connected, and it should populate backed and db caches as well, so maybe something like:

```suggestion
void PopulateCache(const CBlock& block, CCoinsView& view, bool spent = false)
{
CCoinsViewCache cache{&view};
cache.SetBestBlock(uint256::ONE);

std::unordered_set<Txid, SaltedTxidHasher> txids{};
txids.reserve(block.vt
...
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2576160496)
we should do *something* with the result:

```suggestion
for (const auto& in : tx->vin) {
const auto& c{view.AccessCoin(in.prevout)};
BOOST_CHECK(c.IsSpent());
}
```
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2576140637)
seems unused
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2576143396)
we could add comments for these cases as well:
```C++
if (i % 3 == 0) {
// External input
txid = Txid::FromUint256(uint256(i));
} else if (i % 3 == 1) {
// Internal spend (prev tx)
txid = prevhash;
} else {
// Test shortid collisions (looks internal, but is external)
```
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2576158123)
we could also add an in-memory leveldb here to simplify the code and make it more realistic:
```suggestion
CCoinsViewDB db{DBParams{.path = "", .cache_bytes = 1_MiB, .memory_only = true, .wipe_data = true}, CoinsViewOptions{}};
CCoinsViewCache main_cache{&db};
CoinsViewCacheAsync view{main_cache, db};
```
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2575797791)
I prefer simple but fully functioning chunks that converge towards a feature (as someone illustrated: "skateboard -> bicycle -> scooter -> motorcycle -> car" instead of "left wheels -> right wheels -> doors -> wipers -> radio antenna -> windows -> etc").
So if we can carve out chunks (such as the internal spend guard, or a single-threaded fetcher first), we could guide the reviewer instead of having a big-bang change that's really hard to fully comprehend as such.
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2575840858)
wouldn't that percolate to the database layer?
l0rinc closed an issue: "Startup crash on macOS with GCC 15.2: `std::source_location::file_name()` is `nullptr`"
(https://github.com/bitcoin/bitcoin/issues/33964)
💬 l0rinc commented on issue "Startup crash on macOS with GCC 15.2: `std::source_location::file_name()` is `nullptr`":
(https://github.com/bitcoin/bitcoin/issues/33964#issuecomment-3595607653)
Not sure how to create a reliable reproducer for this, closing for now
🚀 fanquake merged a pull request: "cmake: Set `WITH_ZMQ` to `ON` in Windows presets"
(https://github.com/bitcoin/bitcoin/pull/33971)
⚠️ maflcko opened an issue: "integer overflow in FUZZ=package_rbf"
(https://github.com/bitcoin/bitcoin/issues/33981)
Oss-Fuzz: https://issues.oss-fuzz.com/issues/464820824

Input: https://github.com/user-attachments/files/23850907/clusterfuzz-testcase-minimized-package_rbf-5766839826448384.bin.not.txt

Reproduce:

`UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=package_rbf ./bld-cmake/bin/fuzz -runs=1 /tmp/tmp.hLXaXiiFIP/clusterfuzz-testcase-minimized-package_rbf-5766839826448384.bin.not.txt`

```
src/util/feefrac.h:128:14: runt
...