💬 maflcko commented on pull request "depends, doc: Add `tcl` as build dependency for `sqlite` package":
(https://github.com/bitcoin/bitcoin/pull/33975#issuecomment-3595081065)
lgtm ACK 632cf872a584e4a373a669aca5b885f63a2b7add

💬 maflcko commented on pull request "cmake: Set `WITH_ZMQ` to `ON` in Windows presets":
(https://github.com/bitcoin/bitcoin/pull/33971#issuecomment-3595102351)
lgtm ACK 49c672853503e561cd1e7fed19a32f21ad345370

💬 maflcko commented on pull request "[CI] remove `doc/release-notes.md` from lint-spelling.py":
(https://github.com/bitcoin/bitcoin/pull/33968#issuecomment-3595188944)
> (not a hypothetical example).

I tried searching for a real example in the past, but I couldn't find one: https://github.com/bitcoin/bitcoin/pulls?q=%22LLM+reason+%28%E2%9C%A8+experimental%29%22+sort%3Aupdated-desc+is%3Apr+draft%3Afalse+label%3ABackport+is%3Aclosed+

Not sure how much value there is in codespell at this point, given the LLM typo linter, so codespell could even be removed fully. It also seems fine to keep as-is and deal with true and false positives in the backport pull req
...

💬 maflcko commented on pull request "cmake: Make `BUILD_KERNEL_TEST` depend on `BUILD_KERNEL_LIB`":
(https://github.com/bitcoin/bitcoin/pull/33972#issuecomment-3595208102)
lgtm ACK fe1815d48f0cee57d2f1af50b377c7f9e462369e

💬 jsarenik commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-3595226069)
I wonder if the cluster mempool would influence the operation of [alt.signetfaucet.com](https://alt.signetfaucet.com) which currently depends on the default `limitancestorcount` of 25.

💬 maflcko 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-3595267273)
> I'm not sure how to create a reproducer for this...

For toolchain bugs, it helps to create a minimal reproducer. Often, this is easy to create in one shot, looking at the code that mis-compiled.

Sometimes, it is not so easy and you'll have to manually remove chunks from the full software, until you end up with something minimal. This may be tedious, when done manually. You can also use `creduce`, but this may not work in contexts where one can define a unique bug signature (https://github.co
...

💬 maflcko 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-3595271169)
Unrelated, you can also try different version of your gcc package, to see if this was introduced recently.

💬 l0rinc commented on pull request "[CI] remove `doc/release-notes.md` from lint-spelling.py":
(https://github.com/bitcoin/bitcoin/pull/33968#issuecomment-3595275892)
> so codespell could even be removed fully

It does seem to cause more problems than it's solving, I'd vote for that instead

💬 maflcko commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3595280357)
> 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?

💬 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());
}
```