Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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
💬 maflcko commented on issue "qa: `PermissionError` in functional tests on Windows":
(https://github.com/bitcoin/bitcoin/issues/28529#issuecomment-2468646570)
Has anyone ever seen any of the warnings or errors locally?
📝 maflcko converted_to_draft 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.
💬 polespinasa commented on issue "wallet: rpc: `settxfee` sets the wallet feerate not fee":
(https://github.com/bitcoin/bitcoin/issues/31088#issuecomment-2468682651)
@maflcko do you know where can I find info about creating those aliases?

I have a proposal (https://github.com/polespinasa/bitcoin/commit/47fea6e17c9cfb8c2590fd83fe691d6c09d1c2c2) implemented to do what it's requested by using ``return settxfee().HandleRequest(adjusted_request);``. Where ``adjusted_request`` is the request adjusted to the correct format ``sat/vB or B/KvB`` and it is forwarded to the original function.
Actually, the main workflow happens always in ``settxfee`` and if ``settxf
...
⚠️ 0xB10C opened an issue: "Tracepoint Interface Tracking Issue"
(https://github.com/bitcoin/bitcoin/issues/31274)
### Please describe the feature you'd like to see added.

With https://github.com/bitcoin/bitcoin/pull/26593 merged, a few **ideas** for the Bitcoin Core tracepoint interface that _could_ be next steps. I posted these as a comment in https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2468000391 but surfacing them here for more visibility.

These ideas are all up for discussion and aren't TODOs. The numbers don't indicate order or priority but make it easier reference them.

## Depe
...
👍 theStack approved a pull request: "psbt: MuSig2 Fields"
(https://github.com/bitcoin/bitcoin/pull/31247#pullrequestreview-2427834909)
Code-review ACK 8da5ab60b58b808d693d251c8605d53ae54ba617

I've prepared a branch for adding the MuSig2 key types to the test framework and adding a functional test that verifies the `decodepsbt` results on these, feel free to either pull in or ignore (will just open a separate PR after this one is merged then): https://github.com/theStack/bitcoin/tree/202411-test-add_musig2_decodepsbt_test