💬 ryanofsky commented on pull request "thread-safety: fix annotations with REVERSE_LOCK":
(https://github.com/bitcoin/bitcoin/pull/32465#issuecomment-2936288218)
To be clear, I all think the changes in this PR are improvements, even the last commit annotating REVERSE_LOCK, so I'd be happy to merge see this PR merged.

I'm just saying the last commit changing and annotating REVERSE_LOCK could be unnecessary if we decide to remove it later, and think we should remove it for reasons discussed above.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2936303614)
Rebased.

All feedback from @maflcko and @fanquake has been addressed.

The IWYU-related workarounds are now documented with references to the relevant upstream issues.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2124428421)
> Doesn't matter much, but the comment could also say "... so skip the patch because it is not needed and to avoid issues ...".

Thanks! [Adjusted](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2936303614).

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2124430164)
[Reworked](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2936303614) into a local pragma.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2124443576)
Sure, yes. I'll add a commit to the beginning to refactor to this structure + change all clients to do `MakeTxOrphanage`, and then swap out the `Impl`s basically.

💬 l3x3l commented on issue "Unusual "Wallet requires newer version" Error with wallet.dat on macOS, Even with Older Client":
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2936382014)
@maflcko Remove the data corruption label.

📝 brunoerg opened a pull request: "test: wallet: cover wallet passphrase with a null char"
(https://github.com/bitcoin/bitcoin/pull/32675)
This PR adds test coverage for the `walletpassphrase` RPC when the passphrase is incorrect due to a null character.

For reference: https://github.com/bitcoin/bitcoin/pull/27068 introduced the usage of `SecureString` to allow null characters.

💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2124510957)
done

💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2124511972)
completed

📝 mzumsande opened a pull request: "test: apply microsecond precision to test framework logging"
(https://github.com/bitcoin/bitcoin/pull/32676)
When analyzing functional test logs (produced with `combine_logs.py`), entries sometimes sort slightly out of order because even though python prints 6 digits for microsecond precision, it fills up the last 3 digits up with zeroes. For example, it may look like a message was received by the test framework before it was sent by the node.

Change this to actually use microsecond precision - this should make combined logs a little bit easier to analyze.

🤔 maflcko reviewed a pull request: "clang-tidy: Apply modernize-deprecated-headers"
(https://github.com/bitcoin/bitcoin/pull/32673#pullrequestreview-2893488292)
pushed a commit to sort

💬 maflcko commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124554243)
thx, pushed a commit

💬 maflcko commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124518776)
It is needed, see:

[09:40:31.692] The full include-list for /ci_container_base/src/bech32.h:
[09:40:31.692] #include <cstdint> // for uint8_t

💬 maflcko commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124554639)
thx, sorted

💬 maflcko commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124553705)
there are several, so maybe a separate pull request or linter could be added for this? Although, I don't see a risk here.

```
src/base58.h:14:#ifndef BITCOIN_BASE58_H
src/bech32.h:14:#ifndef BITCOIN_BECH32_H
src/common/messages.h:11:#ifndef BITCOIN_COMMON_MESSAGES_H
src/common/types.h:13:#ifndef BITCOIN_COMMON_TYPES_H
src/net_permissions.h:12:#ifndef BITCOIN_NET_PERMISSIONS_H
src/node/types.h:13:#ifndef BITCOIN_NODE_TYPES_H
src/txgraph.h:14:#ifndef BITCOIN_TXGRAPH_H
src/wallet/types.h
...

🤔 polespinasa reviewed a pull request: "tests: Expand HTTP coverage to assert libevent behavior"
(https://github.com/bitcoin/bitcoin/pull/32408#pullrequestreview-2893647989)
LGTM friendly acknowledging this PR :)
ACK f16c8c67bf137e64fbeea1242431baaa915a5c53

Agree on the 5sec timeout change, we avoid weird errors if the CI is lazy.

I locally rebased this on top of master (e872a566f251c73908de8b6d243c94a6679c2eac) to make sure there aren't silent merging conflicts that make the test fail. It works correctly :)

📝 stringintech opened a pull request: "test: headers sync timeout"
(https://github.com/bitcoin/bitcoin/pull/32677)
When reviewing PR #32051 and considering which functional tests might need to be adapted/extended accordingly, I noticed there appears to be limited functional test coverage for header sync timeouts and thought it might be helpful to add one.

This test attempts to cover two scenarios:

1. **Normal peer timeout behavior:** When a peer fails to respond to initial getheaders requests within the timeout period, it should be disconnected and the node should attempt to sync headers from the next
...

🤔 maflcko reviewed a pull request: "test: apply microsecond precision to test framework logging"
(https://github.com/bitcoin/bitcoin/pull/32676#pullrequestreview-2893719748)
nice!

review ACK ed179e0a6528c39b3bca76365f256716f917e19b 🗳

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK ed1
...

💬 achow101 commented on issue "Unusual "Wallet requires newer version" Error with wallet.dat on macOS, Even with Older Client":
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2936749799)
As this is the Bitcoin Core repo, you probably won't get any suggestions to use other software. Most of the contributors here use Bitcoin Core, and obviously our expertise is in this particular software. Personally, I suggest using Bitcoin Core as I will also be able to assist you if you run into further issues.

💬 achow101 commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2124675098)
I've always used my own guix builds, I don't use the script. However, I would prefer to not drop functionality.