✅ achow101 closed an issue: "test: functional test failures under `--usecli`"
(https://github.com/bitcoin/bitcoin/issues/32264)

🚀 achow101 merged a pull request: "test: Handle empty string returned by CLI as None in RPC tests"
(https://github.com/bitcoin/bitcoin/pull/32286)

⚠️ achow101 reopened an issue: "test: functional test failures under `--usecli`"
(https://github.com/bitcoin/bitcoin/issues/32264)
Ubuntu 24.04.2 LTS
Python 3.12.3
master @ 817edfb21e4fa96289d69d4e1dbabb6b9ef9d8f5
```bash
cmake -B build
cmake --build build
./build/test/functional/test_runner.py --usecli
```

```bash
feature_assumeutxo.py | ✖ Failed | 11 s
feature_bip68_sequence.py | ✖ Failed | 58 s
feature_fastprune.py | ✖ Failed | 1 s
feature_fee_estimation.py
...

🚀 achow101 merged a pull request: "test: Add test coverage for rpcwhitelistdefault when unset"
(https://github.com/bitcoin/bitcoin/pull/32079)

✅ achow101 closed an issue: "doc/zmq: Note about endianness does not match reality"
(https://github.com/bitcoin/bitcoin/issues/31856)

🚀 achow101 merged a pull request: "doc: Fix and clarify description of ZMQ message format"
(https://github.com/bitcoin/bitcoin/pull/31862)

💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-2811266661)
> Can you add a test for for failing to migrate a wallet specified by relative path? It's possible that restoration won't work as we expect it to.

Added a test that the 'automatic' restoration from a backup after a failed migration works, and added a test that restoring a backup made from an external relative wallet works.

> Also IIUC, there wasn't an issue with relative paths generally like "a/b/c", only an issue with relative paths containing `..` pointing outside of the wallets director
...

💬 davidgumberg commented on pull request "bench: Fix WalletMigration benchmark":
(https://github.com/bitcoin/bitcoin/pull/32281#discussion_r2047985458)
For context on my comment in https://github.com/bitcoin/bitcoin/issues/32277#issuecomment-2807698798, `SetAddressBook()` includes a [write](https://github.com/bitcoin/bitcoin/blob/e66e30c9e5383a467789574e61114b57536193b9/src/wallet/wallet.cpp#L2496-L2500) to the database, and so does [`AddToWallet(tx)`](https://github.com/bitcoin/bitcoin/blob/e66e30c9e5383a467789574e61114b57536193b9/src/wallet/wallet.cpp#L1160)

This is the reason why some records were persisted, but none that were strictly
...

💬 w0xlt commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r2047995904)
nit: Since `coinselection_tests.cpp` is introduced in the first commit, this change may be there as well.

💬 w0xlt commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r2048001478)
nit: Just for clarity
```suggestion
static std::string InputAmountsToString(const SelectionResult& selection)
```

💬 w0xlt commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r2048003665)
nit: This function is added in the commit https://github.com/bitcoin/bitcoin/pull/29532/commits/66200b3ffa21605fc3234ccbda7b424381f3319a and then modified in the commit https://github.com/bitcoin/bitcoin/pull/29532/commits/afd4b807ff1300e4f74ceab6a683f3ff1376369d.
It can be added directly as it is in the latest commit.

🤔 w0xlt reviewed a pull request: "Refactor BnB tests"
(https://github.com/bitcoin/bitcoin/pull/29532#pullrequestreview-2774237655)
nit: Perhaps the commit descriptions ("BnB rate sensitivity tests", "simple BnB failure tests", and "BnB iteration exhaustion test", for example) could become functions for clarity (not necessarily new test cases).

💬 davidgumberg commented on pull request "bench: Fix WalletMigration benchmark":
(https://github.com/bitcoin/bitcoin/pull/32281#issuecomment-2811361979)
Tested ACK 7912cd41258d59e4

Either of the two DB writes added here seems to fix the issue in my testing, but it's better to be consistent and write both.

---

I am no longer confident the [change to `success`](https://github.com/bitcoin/bitcoin/pull/32149/commits/0f602c5693ef5d0c63b1e5b7dc0990dced3655d6#diff-1f2db0e4d5c12d109c7f0962333c245b49b696cb39ff432da048e9d6c08944d8L4537-R4543) introduced in #32149 makes sense:

https://github.com/bitcoin/bitcoin/blob/e66e30c9e5383a467789574e61
...

🤔 w0xlt reviewed a pull request: "Refactor BnB tests"
(https://github.com/bitcoin/bitcoin/pull/29532#pullrequestreview-2774238544)
nit: Some commits can be squashed to avoid warnings like "unused function".

💬 w0xlt commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2811380510)
Approach ACK

💬 davidgumberg commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2811396408)
Another interesting user of the functional test framework is the warnet tool: https://github.com/bitcoin-dev-project/warnet.

I suspect one challenge/tradeoff with an additional common framework/interface is that this would increase maintenance burden today with only a hypothetical future benefit, the utility framework might languish featureless and unmaintained and/or duplicating things more first-class external interfaces like the kernel do, and doing things the other way around trades a fut
...

💬 pseudoramdom commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-2811571133)
Tested ACK https://github.com/bitcoin/bitcoin/pull/32273/commits/db4ea2632f0c1a5a5d6885526545cbddae7305d7

The patch works for both paths containing `"../"` and `"a/b/c/d"`
> I thought so too, but checked after reading your comment and this is also broken on master:

+1. `master` does fail `migrate` for `"a/b/c"` style.

<details>
<summary>master</summary>

Migrating with `..` style
```
2025-04-17T01:49:47Z copied /Library/Application Support/Bitcoin/regtest/wallets/../../../myrelat
...

🤔 furszy reviewed a pull request: "bench: Fix WalletMigration benchmark"
(https://github.com/bitcoin/bitcoin/pull/32281#pullrequestreview-2774369418)
utACK 7912cd41258d5

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2811720979)
`8939947449...aa88c6dfb6`: rebase due to conflicts

💬 achow101 commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2811880334)
A user has reported their router is (probably) incompatible: https://bitcointalk.org/index.php?topic=5538256.msg65285896#msg65285896