Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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
💬 theStack commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1837004052)
error message nit:
```suggestion
throw std::ios_base::failure("Input musig2 participants pubkeys aggregate key is not 34 bytes");
```
(assuming we are refering to the complete size of the key here, including the compact-size type prefix; we seem to do so on other key types like PSBT_IN_TAP_SCRIPT_SIG)
👍 tdb3 approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2427905007)
re ACK fa66e0887ca1a1445d8b18ba1fadb12b2d911048
👍 tdb3 approved a pull request: "doc: correct typos"
(https://github.com/bitcoin/bitcoin/pull/31271#pullrequestreview-2427921846)
ACK 726cbee9553b25bedfef70cfd5be9f1eeec8a30d

Thanks. This scratches an itch.

Non longer saw the linter errors (present on master) when running `DOCKER_BUILDKIT=1 docker build -t bitcoin-linter --file "./ci/lint_imagefile" ./ && docker run --rm -v $(pwd):/bitcoin -it bitcoin-linter` on the PR branch.
👍 hodlinator approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2428049766)
re-ACK fa66e0887ca1a1445d8b18ba1fadb12b2d911048

`git range-diff master 80d35bc fa66e08`

### Decimal time since epoch replacing random 256-bit value

Sample of recent `now`-value is `1731341566994144855`.

According to https://www.epochconverter.com/ we won't get another digit until year 2286, so path lengths seem quite stable.

Switching to hex representation does give us constant lengths from now until unsigned 64-bit max, but only shaves off 3 chars, which is not that big of a win.
...
👍 brunoerg approved a pull request: "test: Add combinerawtransaction test to rpc_createmultisig"
(https://github.com/bitcoin/bitcoin/pull/31249#pullrequestreview-2428174474)
code review ACK 83fab3212c91d91fc5502f940c901a07772ff747
👍 Abdulkbk approved a pull request: "doc: correct typos"
(https://github.com/bitcoin/bitcoin/pull/31271#pullrequestreview-2428220989)
ACK
💬 m3dwards commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2469105561)
Pretty sure this is what you are looking for:

```diff
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index cc1d8dd905..131f350111 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -85,7 +85,7 @@ jobs:
# one for the branch push and one for the pull request.
# This can be avoided by setting SKIP_BRANCH_PUSH=true as a custom env variable
# in Github repository settings.
- if: ${{ vars.SKIP_BRANCH_PUSH != 'true' }} || github.event
...
🤔 hodlinator reviewed a pull request: "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2428114068)
Reviewed a1232973189126cfc9526713011461709685fcc8

`git range-diff master a5cad72 a123297`

The awk script in the comment in c671dd17dced0d51845dc8d2148f288c4c44ecb2 doesn't add the `'` separators...?

Care to explain the `scaling_factor` value?

`XorHistogram` claims to use 8 GB of RAM. Could be a bit much if we want to be able to also run benchmarks on low-end devices.

```diff
diff --git a/src/bench/xor.cpp b/src/bench/xor.cpp
index 2ba8b17f08..bb193c7fcf 100644
--- a/src/bench/x
...
💬 hodlinator 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#discussion_r1837215954)
```suggestion
obfuscate_key = 0; // Needed for unobfuscated Read
```