✅ brunoerg closed a pull request: "test: importdescriptors is not available for non-descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/31609)

💬 brunoerg commented on pull request "test: importdescriptors is not available for non-descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/31609#issuecomment-2582613729)
> Huh? Now you are adding a test for a previous release, which does not increase the test coverage, see also https://corecheck.dev/bitcoin/bitcoin/pulls/31609 .

Nevermind, my bad, I did it quickly and did not notice it. I'm gonna close it because I believe this check could be removed within bdb removal.

💬 fanquake commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1910323797)
Dropped this out as well.

📝 NicolaLS opened a pull request: "doc: merge required dependency tables"
(https://github.com/bitcoin/bitcoin/pull/31634)
I was a bit confused why there are two tables in [dependencies.md](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) please let me know if there a reason for this or if this change makes sense.

_rationale/motivation:_
Initially there was a distinction between the compiler dependencies and other required dependencies (refs #23565) but the distinction was removed (refs #24585) which is why having two distinct tables could lead to confusion now. This commit merges both tables
...

💬 maflcko commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1910358592)
this is optional

💬 maflcko commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2582680383)
lgtm ACK 08c74105fd65b6356fcdb9d01972fd19f160b588, otherwise

💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2582681441)
A silent conflict has been fixed.

💬 hodlinator commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1910360393)
(systemtap) :+1:

💬 hodlinator commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2582686766)
Would be good to move discussion/question-type commentary out of the PR summary into its own comment as it becomes part of the commit message, regarding the "I was a bit confused ... please let me know ...". Alternatively re-word it.

💬 NicolaLS commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1910364764)
should I add another sub-header (`### Tracing`) in the `## Optional` section and put it there ?

💬 fanquake commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2582707462)
Not sure. Re-arranging this to put two different compilers under "required" doesn't seem less confusing. If anything, I think the tables should be left as-is, and a "developer tools" header added, if there is any confusion.

💬 maflcko commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1910376416)
Yeah, either a new section, or merge the tables below as well into one:

| Dependency | Releases | Version used | Minimum required | Runtime |
| --- | --- | --- | --- | --- |
| [Fontconfig](../depends/packages/fontconfig.mk) (GUI) | [link](https://www.freedesktop.org/wiki/Software/fontconfig/) | [2.12.6](https://github.com/bitcoin/bitcoin/pull/23495) | 2.6 | Yes |
| [FreeType](../depends/packages/freetype.mk) (GUI) | [link](https://freetype.org) | [2.11.0](https://github.com/bitcoin/bitcoin
...

🚀 fanquake merged a pull request: "ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_*"
(https://github.com/bitcoin/bitcoin/pull/31612)

🚀 fanquake merged a pull request: "doc: Clarify min macOS and Xcode version"
(https://github.com/bitcoin/bitcoin/pull/31608)

💬 NicolaLS commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2582763764)
> Not sure. Re-arranging this to put two different compilers under "required" doesn't seem less confusing. If anything, I think the tables should be left as-is, and a "developer tools" header added, if there is any confusion.

With a "development tools" header it would still be unclear which development tools are required (`cmake`, `python`), optional (`systemtap`) or mutual substitutes (`gcc` _or_ `clang`) and since required ones would mixed with the two different compilers in the same table
...

👍 hodlinator approved a pull request: "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls"
(https://github.com/bitcoin/bitcoin/pull/31490#pullrequestreview-2542174622)
ACK 223081ece651dc616ff63d9ac447eedc5c2a28fa

Thanks for reorganizing the first commits!
Confirmed that `git -c log.showSignature=false log --oneline --follow src/bench/readwriteblock.cpp` shows 7 commits.

Cool with the sanity-check in the scripted diff, not sure I've seen that before.


### Commit message dfb2f9d004860c95fc6f0d4a016a9c038d53a475

Might add some more context:
"Similarly +to `UndoWriteToDisk` in parent commit+, `WriteBlockToDisk` wasn't really extracting"


### Com
...

💬 hodlinator commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1910364421)
Thanks for resolving the terminology @l0rinc and for the back-story @ryanofsky!

💬 hodlinator commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1910222878)
nit: Could at least remove newline, and possibly switch to more modern way of non-filterable non-categorized warning:
```suggestion
LogWarning("Failed to flush undo file %05i", pos.nFile);
```

💬 hodlinator commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#discussion_r1910226418)
nit: Could modernize logging when moving the log statement to avoid touching this line twice, resulting in git blame churn. Here and for "OpenBlockFile failed". But I guess you prefer the current commit-style.

💬 Eunovo commented on pull request "Ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r1910413909)
I created another test to check if blocks rejected in the normal IBD run get loaded during a `-reindex`. The test reveals that it does load the invalid block and it will connect it if script checks are skipped, see https://github.com/Eunovo/bitcoin/commit/1ff4e5ee78418a30ba1ea9c384b8c7ee43435ce6

IIUC During a reindex, the node does not know what blocks were connected in the previous run so it can't safely skip checks. It looks to me like we have to:
1. Sync headers before connecting blocks d
...