💬 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.
(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;
```
(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.
(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.
(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.
(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.
(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\
...
(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
...
(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.
(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
(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?
(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.
(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
...
(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
...
(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
(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)
(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
(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.
(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.
...
(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
(https://github.com/bitcoin/bitcoin/pull/31249#pullrequestreview-2428174474)
code review ACK 83fab3212c91d91fc5502f940c901a07772ff747