💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2452927225)
> Only found a download path for the QREncode lib in _depends/packages/qrencode.mk_, **could not spot** how the new _cmake/module/FindQRencode.cmake_ script hooks into Depends after the PkgConfig check fails? 🤔

Depends are not involved when building on Windows.

The [toolchain file provided by `vcpkg`](https://github.com/bitcoin/bitcoin/blob/f1bcf3edc5027b501616670db33d8be1f2cb5a11/CMakePresets.json#L32) sets the [`CMAKE_FIND_ROOT_PATH`](https://cmake.org/cmake/help/latest/variable/CMAKE_F
...

⚠️ maflcko reopened an issue: "Gracefully handle dropped UPnP support "
(https://github.com/bitcoin-core/gui/issues/843)
https://github.com/bitcoin/bitcoin/pull/31130 dropped UPnP support. It's now recommended that
users use PCP (with NATPMP fallback).

But this is not explained in a friendly manner:

<img width="496" alt="error" src="https://github.com/user-attachments/assets/d35891ac-82e0-4f61-8e9b-671aaa1d0d75">

A user would have to figure out that they need to manually edit settings.json or delete it and redo all their settings.

We should probably automatically delete it from settings.json. And the
...

💬 maflcko commented on issue "Gracefully handle dropped UPnP support ":
(https://github.com/bitcoin-core/gui/issues/843#issuecomment-2452961465)
I don't think this was fixed fully for the gui

👍 i-am-yuvi approved a pull request: "net: Use actual memory size in receive buffer accounting"
(https://github.com/bitcoin/bitcoin/pull/31164#pullrequestreview-2411278339)
Tested ACK ad5c012157c5f261503022cfa22d7124bfda5765

> Add a method `CNetMessage::GetMemoryUsage` and use this for accounting of the size of the process receive queue instead of the raw message size (like we already do for the send buffer and `CSerializedNetMsg`).
>
> This ensures that allocation and deserialization overhead is better taken into account.
>
> On average, this counts about ~100 extra bytes per packet on x86_64:
>
> ```
> 2024-10-27T09:50:12Z [net] 24 bytes -> 112 bytes
...

✅ TheBlueMatt closed an issue: "Support validating a PoW-free block over over RPC"
(https://github.com/bitcoin/bitcoin/issues/31136)

💬 TheBlueMatt commented on issue "Support validating a PoW-free block over over RPC":
(https://github.com/bitcoin/bitcoin/issues/31136#issuecomment-2453055656)
Ah, thanks. I always forget about `getblocktemplate`'s options (and the BIP isn't always useful as IIRC only part of it was ever implemented).

💬 TheBlueMatt commented on issue "Support validating a PoW-free block over over RPC":
(https://github.com/bitcoin/bitcoin/issues/31136#issuecomment-2453055947)
It'd be nice to have this in the mining protocol.

💬 tdb3 commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#issuecomment-2453085686)
> > > Does bitcoind consider this to be a breaking change? The new error message may be "more accurate", but for clients that were matching on this error to figure out why a transaction was rejected, this breaks old behavior.
>
> I've added a release notes for this

The release note helps a lot. To err on the side of caution, it seems appropriate to include a `deprecatedrpc=` option, to enable a period of deprecation for users.

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2453089942)
ACK 51c2394eb8158113e73659f4ae65c237813bd5d2

🤔 tdb3 reviewed a pull request: "fees: document non-monotonic estimation edge case"
(https://github.com/bitcoin/bitcoin/pull/31080#pullrequestreview-2411707754)
Approach ACK

The update suggested in https://github.com/bitcoin/bitcoin/pull/31080#discussion_r1810430953 seems to increase clarity.

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2453108430)
Thanks for the hints @maflcko, I was under the impression that big-endian tests were run automatically.

### Fix

It seem that that `std::rotr` doesn't take endianness into account, so the fix basically looks like:
```C++
size_t key_rotation = 8 * key_offset;
if constexpr (std::endian::native == std::endian::big) key_rotation *= -1;
return std::rotr(key, key_rotation);
```
I've emulated the s390x behavior locally like this:

<details>
<summary>Details</summary>

```bash
brew inst
...

👍 tdb3 approved a pull request: "test: Fix intermittent issue in wallet_backwards_compatibility.py"
(https://github.com/bitcoin/bitcoin/pull/29982#pullrequestreview-2411713750)
ACK ec777917d6eba0b417dbc90b9b891240a44b7ec4

With a similar delay (150ms) induced with https://github.com/Randy808/bitcoin/commit/f39144d808975c016c1a94022a2c4251950afb7d,
Saw test failure without the `sync_mempools()` line added in this PR. After adding the line, saw consistent passing (20x runs).

🤔 hodlinator reviewed a pull request: "test: Shut down framework cleanly on RPC connection failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2411714494)
Thanks for the review @tdb3!

Implemented feedback from that review, made the code a bit more Pythonic and improved comments slightly.

💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826774806)
Tried using `update()`, but it resulted in things like:
```
AssertionError: [node 0] Unable to connect to bitcoind after 10s (ignored errors: {'m': 40, 'i': 120, 's': 120, 'n': 80, 'g': 40, '_': 40, 'c': 40, 'r': 40, 'e': 80, 'd': 40, 't': 40, 'a': 40, 'l': 40}, latest error: ValueError('No RPC credentials'))
```
i.e. it was iterating over the string and incrementing an entry for each unique character, which seems to match the [docs](https://docs.python.org/3/library/collections.html#collect
...

💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826772195)
(Same issue with `update()`).

💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826774077)
Had to add a commit (2a0c0e410e9f32dbf7af7229457889b09553c438) that refurbishes `assert_raises_message()`, but test is much cleaner now!

👋 l0rinc's pull request is ready for review: "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`"
(https://github.com/bitcoin/bitcoin/pull/31144)

💬 hodlinator commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2453170834)
> The [toolchain file provided by `vcpkg`](https://github.com/bitcoin/bitcoin/blob/f1bcf3edc5027b501616670db33d8be1f2cb5a11/CMakePresets.json#L32) sets the [`CMAKE_FIND_ROOT_PATH`](https://cmake.org/cmake/help/latest/variable/CMAKE_FIND_ROOT_PATH.html) variable, directing the `find_path()` and `find_library()` commands to the appropriate paths.

Okay, the toolchain file seems to be *C:\Program Files\Microsoft Visual Studio\2022\Community\VC\vcpkg\scripts\buildsystems\vcpkg.cmake* in my case. D
...

👍 tdb3 approved a pull request: "test: Shut down framework cleanly on RPC connection failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2411750030)
Code Review re ACK 1148c4a53295e7635a813aadfde53ba5d973395c

Nice work cleaning up the existing code around the core changes. Increases readability.

💬 tdb3 commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826867112)
non-blocking pico-nit:
If this file gets touched again, could use `f` string instead of `format`.
https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#style-guidelines