🤔 Copilot reviewed a pull request: "Replace cluster linearization algorithm with SFL"
(https://github.com/bitcoin/bitcoin/pull/32545#pullrequestreview-3354761107)
## Pull Request Overview

This PR replaces the existing cluster linearization algorithm (LIMO with candidate-set search) with a new spanning-forest linearization (SFL) algorithm that offers significantly better performance for complex clusters. The new algorithm uses a fundamentally different approach based on maintaining spanning trees within chunks and performing split/merge operations.

**Key Changes:**
- Replaces search-based linearization with spanning-forest based approach
- Simplifies qua
...

💬 Copilot commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2443583986)
The new `is_topological` parameter defaults to `true`, but when `old_linearization` is empty, this parameter is ignored. This could be confusing for API consumers. Consider documenting this behavior more explicitly in the function comment, or validate that when `old_linearization` is empty, `is_topological` is not set to `false`.

💬 Copilot commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2443583989)
[nitpick] The function signature changed from accepting `const std::array<uint8_t, N>&` to `const std::vector<uint8_t>&`. This removes compile-time size checking. While this makes the function more flexible, it also removes a safety guarantee. Consider if the added flexibility is worth the loss of compile-time size verification.
```suggestion
template <size_t N>
void BenchLinearizeOptimally(benchmark::Bench& bench, const std::array<uint8_t, N>& serialized)
```

💬 l0rinc commented on pull request "test: Fix TestFlushBehavior when coin has non-zero dynamic memory usage":
(https://github.com/bitcoin/bitcoin/pull/33381#issuecomment-3420329732)
@PiRK, can you please check if this was already fixed by https://github.com/bitcoin/bitcoin/pull/32313?

💬 yuvicc commented on pull request "test: [move-only] binary utils to utils.py":
(https://github.com/bitcoin/bitcoin/pull/33633#issuecomment-3420744948)
LGTM ACK fa75ef4328f638221bcf85fcbefa885122084622

💬 PiRK commented on pull request "test: Fix TestFlushBehavior when coin has non-zero dynamic memory usage":
(https://github.com/bitcoin/bitcoin/pull/33381#issuecomment-3420788196)
@l0rinc Yes, #32313 fixes it.

Tested with this patch:
```
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index a1152d245f..a3ee441c42 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -880,6 +880,7 @@ Coin MakeCoin()
{
Coin coin;
coin.out.nValue = m_rng.rand32();
+ coin.out.scriptPubKey.assign(m_rng.randbits(6), 0);
coin.nHeight = m_rng.randrange(4096);
coin.fCoinBase = 0;
return coin;
```
coins_tests fails as
...

💬 PiRK commented on pull request "test: Fix TestFlushBehavior when coin has non-zero dynamic memory usage":
(https://github.com/bitcoin/bitcoin/pull/33381#issuecomment-3420846050)
AFAICT the issue my patch raised was only reachable after suppressing a `std::logic_error`, which shouldn't happen in production code.

💬 ubbabeck commented on pull request "test: multisig verify spend from 100 of 999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/33658#issuecomment-3420984051)
Closing this as it's rather trivial to add once the https://github.com/bitcoin/bitcoin/issues/29098 and rather focus on testing and reviewing it.

✅ ubbabeck closed a pull request: "test: multisig verify spend from 100 of 999 taproot multisig wallet"
(https://github.com/bitcoin/bitcoin/pull/33658)

📝 halilyucel opened a pull request: "Improve fatal error message for block read failures in BaseIndex::ProcessBlock"
(https://github.com/bitcoin/bitcoin/pull/33659)
When a block read operation fails in BaseIndex::ProcessBlock, the current error message only reports:
Failed to read block %s from disk

This message doesn't provide enough context to help developers or node operators diagnose the problem.

What changed?
The fatal error message now includes:
The block hash (already existed)
The block height
A clearer description of the issue
A hint to check whether the block exists in the database

Before:
```
FatalErrorf("Failed to read block %s f
...

📝 NadiaDjarbo opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/33660)
<br>## Summary<br>As a new contributor to Bitcoin Core, I added a beginner-friendly welcome note to the README.md to highlight node setup for the original cryptocurrency. This helps newcomers understand the peer-to-peer electronic cash system! <br><br>## Motivation<br>New users often struggle with initial node sync in issues—this note provides a quick intro to reduce barriers for Bitcoin enthusiasts.<br><br>## Changes<br>- Added a new subsection "## Welcome to Bitcoin Node Setup" at the end of R
...

💬 optout21 commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2444741664)
nit: Declaration differs from definition, `block` vs `data` parameter name

💬 optout21 commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2444750382)
This way both `block` and `blockpart` api's take the same paramaters, including `"offset"` and `"size"`, right? Is that intended?

📝 optout21 opened a pull request: "test: Add test on skip heights in CBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/33661)
The skip height values, used by `CBlockIndex::BuildSkip()` and `GetSkipHeight` are not tested and not well documented. (noticed while reviewing #33515, recently merged).

The motivation is to document the skip value computation through a test.

The first commit adds a test for the properties of the distribution of skip values, namely that they have non-uniform distribution: most values are small but there are some large ones as well.

The second commit adds low-level tests to the `GetSkipH
...

🤔 maflcko reviewed a pull request: "fix: invalid plist format and update values to meet macOS 1.0 standard"
(https://github.com/bitcoin/bitcoin/pull/33654#pullrequestreview-3356260624)
is this llm generated?

💬 maflcko commented on pull request "fix: invalid plist format and update values to meet macOS 1.0 standard":
(https://github.com/bitcoin/bitcoin/pull/33654#discussion_r2444851592)
Isn't 10.14 different from 14.0?

💬 stickies-v commented on pull request "Improve fatal error message for block read failures in BaseIndex::ProcessBlock":
(https://github.com/bitcoin/bitcoin/pull/33659#issuecomment-3421841236)
NACK, feels like busywork, don't see how this improves things.

✅ halilyucel closed a pull request: "Improve fatal error message for block read failures in BaseIndex::ProcessBlock"
(https://github.com/bitcoin/bitcoin/pull/33659)

💬 famouswizard commented on pull request "fix: invalid plist format and update values to meet macOS 1.0 standard":
(https://github.com/bitcoin/bitcoin/pull/33654#discussion_r2444875498)
[maflcko](https://github.com/maflcko), yes, exactly - the previous value `14` was incorrect because it refers to macOS 14 (Sonoma). the correct minimum version is `10.14.0` (Mojave), using the proper X.Y.Z format required by LSMinimumSystemVersion.

🤔 naiyoma reviewed a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3356353069)
utACK 84b2ad03349bf6af3d6057f3dbcdf84c17f11bcb