💬 achow101 commented on pull request "doc: Add missing 'blank=true' option in offline-signing-tutorial.md":
(https://github.com/bitcoin/bitcoin/pull/31237#issuecomment-2468501470)
Post merge -0

`disable_private_keys` already implies `blank` so there isn't really a point in specifying `blank` explicitly. I would have preferred to remove the earlier text that says both options would be set.

📝 maflcko opened a pull request: "ci: Bump valgrind tasks to clang-18"
(https://github.com/bitcoin/bitcoin/pull/31273)
This is the default, see https://packages.ubuntu.com/noble/clang, so likely more widely used. Thus, it seems better to use `clang-18`, instead of `clang-16` in the valgrind CI tasks.

💬 fanquake commented on pull request "doc: Add missing 'blank=true' option in offline-signing-tutorial.md":
(https://github.com/bitcoin/bitcoin/pull/31237#issuecomment-2468510059)
I guess if we decide to do that, we can also update the other guides that are doing this redundantly, i.e: https://github.com/bitcoin/bitcoin/blob/master/doc/multisig-tutorial.md#13-create-the-multisig-wallet.

💬 hodlinator commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2468514467)
cr-ACK ee1128ead846698db5e5633f193883837f2fbc64

`git range-diff master 0440c40 ee1128e`:

- Only [comment corrections](https://github.com/bitcoin/bitcoin/pull/31172#discussion_r1834418655.) since my last Windows 11 run.

💬 l0rinc commented on pull request "ci: Bump valgrind tasks to clang-18":
(https://github.com/bitcoin/bitcoin/pull/31273#issuecomment-2468531329)
> better to use clang-18 [...] in the valgrind CI tasks

Why only for `valgrind` tasks and not e.g. `00_setup_env_native_nowallet_libbitcoinkernel.sh`? To have a wider variety of versions?

💬 fanquake commented on pull request "ci: Bump valgrind tasks to clang-18":
(https://github.com/bitcoin/bitcoin/pull/31273#issuecomment-2468531448)
Last time I checked this, this needed additional suppressions (maybe only for aarch64)? Is that no-longer the case?

👍 tdb3 approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2427666716)
code review and light test re ACK 6c7b3bf0ce7a6238457dd44ee1d5d744b3663a52

`build/src/bench/bench_bitcoin`:
...
`/tmp/test_common bitcoin/BlockAssemblerAddPackageTxns/1731341243673972130/regtest/blocks`

💬 fanquake commented on pull request "ci: Bump valgrind tasks to clang-18":
(https://github.com/bitcoin/bitcoin/pull/31273#issuecomment-2468534148)
> Why only for valgrind tasks and not e.g. 00_setup_env_native_nowallet_libbitcoinkernel.sh?

Probably because of the reason listed in `00_setup_env_native_nowallet_libbitcoinkernel.sh`:

> Use minimum supported python3.10 (or best-effort 3.11) and clang-16, see doc/dependencies.md

👍 theStack approved a pull request: "test: report detailed msg during utf8 response decoding error"
(https://github.com/bitcoin/bitcoin/pull/31251#pullrequestreview-2427672566)
re-ACK a2c45ae5480a2ee643665d6ecaee9714a287a70e

💬 l0rinc commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2468549376)
> The test interface_usdt_utxocache.py fails

I haven't reviewed the code in detail yet (since the [build is still failing](https://github.com/bitcoin/bitcoin/pull/30610#pullrequestreview-2277942711)).

<details>
<summary>reproduced build failure</summary>

```bash

> apt install systemtap-sdt-dev
> cmake -B build -DCMAKE_BUILD_TYPE=Release -DWITH_USDT=ON && cmake --build build -j$(nproc)
> build/test/functional/interface_usdt_utxocache.py

2024-11-11T15:47:49.211000Z TestFramework
...

💬 hodlinator commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1836917699)
As `g_rng_temp_path` is no longer used, it should be removed completely from line 78-79.

💬 hodlinator commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1836915494)
nit: Could potentially move the `ToString` call up with the rest of the calculation for that value.
```suggestion
const auto now{util::ToString(TicksSinceEpoch<std::chrono::nanoseconds>(SystemClock::now()))};
m_path_root = fs::temp_directory_path() / TEST_DIR_PATH_ELEMENT / test_name / now;
```

💬 andrewtoth commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2468569514)
The tests need to be fixed per https://github.com/bitcoin/bitcoin/pull/30610#pullrequestreview-2277942711.

@l0rinc what this PR accomplishes is not wiping the dbcache if calling those RPCs. It should not affect performance of the RPCs, but performance of connecting new blocks after calling the RPCs.

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1836932900)
Yeah good, done as suggested. Thanks.

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1836936137)
I prefer the current approach to not over-extend the first line. Even with the `ToString` call, the second line is simpler to read.

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2468590367)
Updated per feedback. Thanks. Removed unused global `g_rng_temp_path` from the setup common file.

💬 fanquake commented on issue "qa: `PermissionError` in functional tests on Windows":
(https://github.com/bitcoin/bitcoin/issues/28529#issuecomment-2468597550)
Saw a failure that matches the first portion of the issue here. https://github.com/bitcoin/bitcoin/actions/runs/11781528801/job/32814548437#step:12:69. Unclear if this should be a new issue, but couldn't seem to match it to an existing issue.
```bash
269/316 - p2p_unrequested_blocks.py failed, Duration: 1 s

stdout:
2024-11-11T16:26:48.484000Z TestFramework (INFO): PRNG seed is: 2650025219029138162

2024-11-11T16:26:48.518000Z TestFramework (INFO): Initializing test directory D:\a\_temp\
...

💬 maflcko commented on pull request "ci: Bump valgrind tasks to clang-18":
(https://github.com/bitcoin/bitcoin/pull/31273#issuecomment-2468624506)
Ah, I guess that'd be https://github.com/bitcoin/bitcoin/issues/29635.

I only found on Debian Trixie:

```
# uname --machine && valgrind --version && clang-19 --version
x86_64
valgrind-3.20.0
Debian clang version 19.1.3 (1)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm-19/bin
```

```
2024-11-11T13:31:48.924322Z [tes==12188== Source and destination overlap in memcpy_chk(0x1ffeffca88, 0x1ffeffca84, 8)
==12188== at 0x484EE22: __memcpy_chk (in /usr
...

💬 brunoerg commented on issue "Wallet fuzzing tracking issue":
(https://github.com/bitcoin/bitcoin/issues/29901#issuecomment-2468632166)
Removed #30570 since it's closed.

💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#issuecomment-2468636526)
Force-pushed addressing (multisig and hd chain cover):

- https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1828420886
- https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1828423452