💬 vasild commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2098000122)
_Repeat comment https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2090912262 here:_

Is the intention here to send 8MB of data?

💬 vasild commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2098002185)
_Repeat comment https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2090917556 here:_

Here and elsewhere in the added tests, `assert_equal()` produces a better error message:

`assert` (value of `out1` is not printed):
```
assert out1 == b'{"result":"high-hash","error":null}\n'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError
```

vs

`assert_equal()`:
```
AssertionError: not(b'{"result":null,"error":{"code":-32700,"message":"Parse error"},"id"
...

💬 hebasto commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2894473878)
> What I don't understand is why the CentOS task seemingly passed before, but started failing after [af65fd1](https://github.com/bitcoin/bitcoin/commit/af65fd1a333011137dafd3df9a51704fd319feb4), even though the task wasn't modified apart from a comment.

`-g0` -> `-g2`?

💬 willcl-ark commented on pull request "depends: use "mkdir -p" when installing xproto":
(https://github.com/bitcoin/bitcoin/pull/32568#issuecomment-2894484089)
Concept ACK.

Tested successfully @ d730249 using

```bash
$ docker run -it alpine:latest

# On the guest
$ apk update
$ apk add git
$ git clone https://github.com/bitcoin/bitcoin --depth=1
$ git fetch origin pull/32568/head:pr-32568
$ git checkout pr-32568
$ apk add bash build-base cmake curl m4 make patch
$ apk add bison linux-headers ninja-build pkgconf python3 xz
$ make -C depends -j14
```

No strong opinion from me on `MKDIRPROG` vs `MKDIR_P`.

💬 maflcko commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2894494499)
I wonder if there is a platform without `posix_fallocate` (outside of macos and windows). If not, the fallback can probably be fully removed, along with the detection and it can just be assumed to be always present?

💬 willcl-ark commented on issue "Coin Selection tracepoint overreports use of APS":
(https://github.com/bitcoin/bitcoin/issues/25150#issuecomment-2894497168)
@0xB10C do you think anything should be done with this tracepoint?

Perhaps if @murchandamus is recommending removal then that could be the best course of action?

💬 TheCharlatan commented on pull request "test: fix pushdata scripts":
(https://github.com/bitcoin/bitcoin/pull/32270#issuecomment-2894504215)
Would ACK once rebased.

👍 brunoerg approved a pull request: "Remove legacy `Parse(U)Int*`"
(https://github.com/bitcoin/bitcoin/pull/32520#pullrequestreview-2854297782)
code review ACK faf55fc80b11f3d9b0b12c1b26a9612ea9ce9b40

Did a grep to check if anything is missing, seems fine.

💬 hebasto commented on pull request "Use subprocess library for notifications":
(https://github.com/bitcoin/bitcoin/pull/32566#discussion_r2098055653)
3e2b4660b84af872d114afc1556649f3dd93a1af:

`#include <codecvt>` might be removed now.

💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-2894562974)
Changes overlapping with https://github.com/bitcoin/bitcoin/pull/32566 have been dropped.

📝 hebasto converted_to_draft a pull request: "cmake, guix: Skip building tests in subtrees for releases"
(https://github.com/bitcoin/bitcoin/pull/32054)
The `BUILD_TESTS` variable has a broad scope, controlling:
- Building `test_bitcoin`
- Building `test_bitcoin-qt`
- Building tests in subtrees, such as `secp256k1` and `univalue`
- Creating CTest's tests

However, for release builds, only the first is necessary.

To address this, this PR introduces the new `BUILD_TEST_BINARY` variable, which allows building only the `test_bitcoin` binary without enabling other tests.

---

As an alternative, an explicit list of build targets can be s
...

📝 vicjuma opened a pull request: "refactor: remove dump.h/cpp, wallet_bdb_parser.cpp"
(https://github.com/bitcoin/bitcoin/pull/32569)
This PR removes the following deprecated legacy wallet files:

- `src/wallet/dump.cpp`
- `src/wallet/dump.h`
- `src/wallet/test/fuzz/wallet_bdb_parse.cpp`
- `Their associated references and usages`

These files were originally part of the legacy BerkeleyDB wallet backend and are no longer used in current functionality. They were retained for backward compatibility but are now fully obsolete due to ongoing deprecation tracked in #28710.

### Motivation

- These files are no longer comp
...

💬 maflcko commented on pull request "refactor: remove dump.h/cpp, wallet_bdb_parser.cpp":
(https://github.com/bitcoin/bitcoin/pull/32569#issuecomment-2894610004)
Please no AI slop

✅ maflcko closed a pull request: "refactor: remove dump.h/cpp, wallet_bdb_parser.cpp"
(https://github.com/bitcoin/bitcoin/pull/32569)

🤔 hebasto reviewed a pull request: "refactor: remove dump.h/cpp, wallet_bdb_parser.cpp"
(https://github.com/bitcoin/bitcoin/pull/32569#pullrequestreview-2854410943)
Please do not touch automatically generated `src/qt/bitcoinstrings.cpp`.

👍 hebasto approved a pull request: "qt: drop unused watch-only functionality"
(https://github.com/bitcoin/bitcoin/pull/32459#pullrequestreview-2854442158)
ACK e8661aac752eb08fee318eb8f56e599578d78f9f, I have reviewed the code and it looks OK. The `src/qt/forms/overviewpage.ui` form was reviewed in Qt Designer.

Tested on Ubuntu 25.04.

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2098146539)
The problem with this is the test becomes flakey because the timer may not start in sync with the server, so it may record less time than the server actually waited: https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2081584805

note also that libevent doesn't even try to test the lower bound: https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2080431221

💬 instagibbs commented on pull request "test: fix pushdata scripts":
(https://github.com/bitcoin/bitcoin/pull/32270#issuecomment-2894683573)
@TheCharlatan rebased

💬 luke-jr commented on issue "rpc method removeprunedfunds should take an array of txids":
(https://github.com/bitcoin/bitcoin/issues/29466#issuecomment-2894701844)
Can't you just make a batch RPC request?

💬 polespinasa commented on pull request "rpc: generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2098176485)
> not sure about forcing an array, when all tests mostly just want to provide a single dummy value.

I don't think this should be a problem.
Same happens with `sendall`. Most tests just provide a single value in a list. And it actually doesn't affect many tests.

I think it keeps the code simpler and avoids the confusion that could appear if we use multiple possible arguments.

Also, even if this is not a backward compatible change it's not something critical as it is the test code.

B
...