💬 achow101 commented on issue "Load PSBT error: Unable to decode PSBT":
(https://github.com/bitcoin/bitcoin/issues/30070#issuecomment-3554041212)
We could instead defer signing of the transaction until after "Send" is clicked.

> Edit: Why is the loading of PSBT upset if there are signatures? In other words, why `throw` here:

A (v0) PSBT contains the entire unsigned transaction in its own field, with signatures and scripts placed in the PSBT fields. The transaction must be unsigned in order to make further signing and finalizing possible to reason about generically. Trying to make a PSBT from a transaction that already contained signatur
...

👍 l0rinc approved a pull request: "qa: Account for unset errno in ConnectionResetError"
(https://github.com/bitcoin/bitcoin/pull/33875#pullrequestreview-3484132142)
untested code review ACK 76e0e6087d0310ec31f43d751de60adf0c0a2faa

This is a continuation of https://github.com/bitcoin/bitcoin/pull/30660/files#diff-7f22a082e3a80ca2f212e36193f87dd80237035afedb7f15b116ef7fa265f3eeR326, separating the existing `TimeoutError` to add extra details for `ConnectionResetError` as well.

💬 fjahr commented on pull request "test: Fix race condition in IPC interface block progation test":
(https://github.com/bitcoin/bitcoin/pull/33880#issuecomment-3554086620)
> Good catch. The test needs to check two things:

Thanks for explaining, I have switched the check to node0 and kept it above the sync because that feels like the most natural flow given my improved understanding of the test now. I have also added some short comments based on your explanation.

💬 D33r-Gee commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#issuecomment-3554088845)
with [36ae2ef](https://github.com/bitcoin/bitcoin/commit/36ae2effba30f6806a6171fa6a8018fde5215302) rebased over master.

No changes to commits...

💬 ryanofsky commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#issuecomment-3554122176)
re: https://github.com/bitcoin/bitcoin/pull/32061#issuecomment-3553506888

> This push is mainly for a CI sanity check, then I'm going to put it in draft while I refactor for @theuni main feedback which will be removing the sockman abstraction and implementing the I/O loop directly in `HTTPServer`

I agree with theuni it should make this PR easier to review and simpler if current sockman functionality ([sockman.cpp](https://github.com/pinheadmz/bitcoin/blob/8c81bdf2532aa1c61e25d367a480ce3aa7
...

💬 hebasto commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3554161151)
> > minimum GCC version for Windows cross-compilation to [13.1](https://doc.qt.io/qt-6.8/supported-platforms.html#windows).
>
> Have you checked that it doesn't work with 12?

This [branch](https://github.com/hebasto/bitcoin/tree/251119-qt6.9) fails to cross-compile on Debian Bookworm with GCC 12.2.0:
```
/home/hebasto/bitcoin/depends/work/build/x86_64-w64-mingw32/qt/6.9.3-d5ebe8f681b/qtbase/src/3rdparty/pcre2/src/pcre2_compile.c: In function ‘pcre2_compile_16’:
/home/hebasto/bitcoin/dep
...

💬 mzumsande commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2543210150)
Done. Since the `net_processing` warning is now harder to understand (half of the logged text comes from validation) I added a comment there.

💬 mzumsande commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#issuecomment-3554211685)
[6db2551](https://github.com/bitcoin/bitcoin/commit/6db2551dc27c4a9b989e8814054c93dd9d8f1b36) to [7f28483](https://github.com/bitcoin/bitcoin/commit/7f284835be87c4d1a8a56804992043e12dae1ea1):
- addressed https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2539190197
- reworked the second `CheckForkWarningConditions` commit to unify the log message with the alernotify warning. I noticed the discrepancy due to #33893

🤔 mzumsande reviewed a pull request: "test: add `-alertnotify` test for large work invalid chain warning"
(https://github.com/bitcoin/bitcoin/pull/33893#pullrequestreview-3484323098)
Concept ACK

FYI I proposed to reword the warning message in #33553 (7f284835be87c4d1a8a56804992043e12dae1ea1).

💬 Sjors commented on pull request "test: Fix race condition in IPC interface block progation test":
(https://github.com/bitcoin/bitcoin/pull/33880#issuecomment-3554275116)
utACK 944502628a532469b0d3c9a3af04fa2d9c18f849

💬 mzumsande commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2543275564)
@polespinasa Not sure (#15141 seems relevant), but I think it's to avoid that the network splits into two parts in case of bugs leading to an temporary consensus incompability between clients, that may self-correct because the invalid block may be reorged out.

💬 maflcko commented on pull request "test: Fix race condition in IPC interface block progation test":
(https://github.com/bitcoin/bitcoin/pull/33880#discussion_r2543309283)

it own -> its own [possessive pronoun "its" required; "it" is incorrect here]

drahtbot_id_5_m

💬 maflcko commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3554424316)
I'd say it is fine to bump the minimum required to 13 for win-cross compilation:

```diff
diff --git a/doc/build-windows.md b/doc/build-windows.md
index ce822dd0dc..9d3ec9cd86 100644
--- a/doc/build-windows.md
+++ b/doc/build-windows.md
@@ -15,6 +15,7 @@ Other options which may work, but which have not been extensively tested are (pl

The instructions below work on Ubuntu and Debian. Make sure the distribution's `g++-mingw-w64-x86-64-posix`
package meets the minimum required `g++` v
...

💬 hebasto commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3554447029)
> Either way is fine.

We should also bear in mind that the fix for the original issue needs to be backported to 30.x.

⚠️ TheCharlatan opened an issue: "RFC: Add versioning to the kernel header"
(https://github.com/bitcoin/bitcoin/issues/33911)
### Please describe the feature you'd like to see added.

The kernel header installed alongside its dynamic library should be versioned. Versioning best practices around providing a stable API are well established, e.g. through semantic versioning. However the library presents challenges to versioning beyond API stability. Internal features changing, i.e. a new soft fork rule, or a new validation cache, might not have a direct impact on the API, but won't necessarily be desirable to run by all u
...

💬 Christewart commented on pull request "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`":
(https://github.com/bitcoin/bitcoin/pull/31823#issuecomment-3554637246)
I think this is good for merge?

💬 theStack commented on pull request "test: add `-alertnotify` test for large work invalid chain warning":
(https://github.com/bitcoin/bitcoin/pull/33893#issuecomment-3554648564)
@brunoerg @maflcko: Thanks for reviewing, I addressed all the nits.

> FYI I proposed to reword the warning message in #33553 ([7f28483](https://github.com/bitcoin/bitcoin/commit/7f284835be87c4d1a8a56804992043e12dae1ea1)).

Ah nice, makes sense to unify, was already wondering why there are two different messages, each one only mentioning one possible cause.

💬 theStack commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#issuecomment-3554651834)
Concept ACK

💬 TheCharlatan commented on pull request "test: add `-alertnotify` test for large work invalid chain warning":
(https://github.com/bitcoin/bitcoin/pull/33893#issuecomment-3554692728)
Nice, Concept ACK

💬 achow101 commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-3554721014)
Finally got around to debugging the actual issue.

The issue has to do with how `std::filesystem::recursive_directory_iterator` handles symlinks. The [docs](https://en.cppreference.com/w/cpp/filesystem/directory_options.html) for the default options that we use say that symlinks are not recursed into for searching. Instead, when we find symlink directories, we look for a wallet.dat file, but we won't search any of the subdirectories.

The problem is that GCC does not implement symlink detect
...