👍 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

💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826869399)
Good point! Resolved in latest push.

👍 tdb3 approved a pull request: "test: Shut down framework cleanly on RPC connection failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2411752016)
re ACK 042cee7073b338d79b5176d4157cf08bbd079b9f

👍 jasonribble approved a pull request: "Refactor BnB tests"
(https://github.com/bitcoin/bitcoin/pull/29532#pullrequestreview-2411753208)
ACK

I went through each of the commits. It is an improvement of the branch and bound selection tests.