💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912175146)
nit: Give a hint at why?
```suggestion
throw std::overflow_error("Too many mebibytes to fit into size_t bytes");
```
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912175146)
nit: Give a hint at why?
```suggestion
throw std::overflow_error("Too many mebibytes to fit into size_t bytes");
```
💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912185410)
nit: Could modernize cast.
```suggestion
if (std::make_unsigned_t<Input>(i) > Output{std::numeric_limits<Output>::max() >> shift}) {
```
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912185410)
nit: Could modernize cast.
```suggestion
if (std::make_unsigned_t<Input>(i) > Output{std::numeric_limits<Output>::max() >> shift}) {
```
💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912181339)
nit: Could have less churn on the line by making `InitAndLoadChainstate()` take `const` in the same commit where this line is first touched.
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912181339)
nit: Could have less churn on the line by making `InitAndLoadChainstate()` take `const` in the same commit where this line is first touched.
💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912179390)
nit: Typo
```suggestion
// Convert -dbcache from MiB units to bytes. The total cache is floored by MIN_DB_CACHE and capped by max size_t value.
```
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912179390)
nit: Typo
```suggestion
// Convert -dbcache from MiB units to bytes. The total cache is floored by MIN_DB_CACHE and capped by max size_t value.
```
💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912212037)
nit in commit 4eebb638916ecf43cfd9237c7e8bb8da7bc397d7:
Seems like we could use `std::min` without the template parameter here from the start (it's removed later without parameter types changing, resulting in line churn).
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912212037)
nit in commit 4eebb638916ecf43cfd9237c7e8bb8da7bc397d7:
Seems like we could use `std::min` without the template parameter here from the start (it's removed later without parameter types changing, resulting in line churn).
💬 andrewtoth commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2585403343)
I benchmarked this PR rebased onto 35bf426e02210c1bbb04926f4ca2e0285fbfcd11 up to block 878k two times each, and I saw a 9% speedup. This was using `dbcache=30000`.
```
hyperfine --export-markdown ~/bench.md --show-output --parameter-list commit 1298bae74a1f690fd6cc0e029e490537cbeb301b,35bf426e02210c1bbb04926f4ca2e0285fbfcd11 --prepare 'git checkout {commit} && \
cmake --build build -j $(nproc); \
rm -rf /home/user/.bitcoin; \
sync; sudo /sbin/sysctl vm.drop_caches=3;' -M 2 'echo '{commit
...
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2585403343)
I benchmarked this PR rebased onto 35bf426e02210c1bbb04926f4ca2e0285fbfcd11 up to block 878k two times each, and I saw a 9% speedup. This was using `dbcache=30000`.
```
hyperfine --export-markdown ~/bench.md --show-output --parameter-list commit 1298bae74a1f690fd6cc0e029e490537cbeb301b,35bf426e02210c1bbb04926f4ca2e0285fbfcd11 --prepare 'git checkout {commit} && \
cmake --build build -j $(nproc); \
rm -rf /home/user/.bitcoin; \
sync; sudo /sbin/sysctl vm.drop_caches=3;' -M 2 'echo '{commit
...
💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912234840)
Using non-narrowing conversion should give you a compiler error. Allowing type narrowing here should be safe.
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912234840)
Using non-narrowing conversion should give you a compiler error. Allowing type narrowing here should be safe.
👍 hodlinator approved a pull request: "doc: improve dependencies documentation"
(https://github.com/bitcoin/bitcoin/pull/31634#pullrequestreview-2545217904)
ACK d0ca5c53635916868e7d5da913faf3d9a8d706b7
Good to clarify that only one of the compilers is required, and re-introduce that section!
Thanks for updating the PR summary.
### Commit message
Suggest removing repetition of initial sentence and fixing grammar of last line, + less important other changes.
```diff
-doc: improve dependencies documentation
+doc: Improve dependencies documentation
-This commit improves the dependencies documentation by:
-- moving "Clang" and "GCC"
...
(https://github.com/bitcoin/bitcoin/pull/31634#pullrequestreview-2545217904)
ACK d0ca5c53635916868e7d5da913faf3d9a8d706b7
Good to clarify that only one of the compilers is required, and re-introduce that section!
Thanks for updating the PR summary.
### Commit message
Suggest removing repetition of initial sentence and fixing grammar of last line, + less important other changes.
```diff
-doc: improve dependencies documentation
+doc: Improve dependencies documentation
-This commit improves the dependencies documentation by:
-- moving "Clang" and "GCC"
...
💬 hodlinator commented on pull request "doc: improve dependencies documentation":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1912232236)
nit: Not yet touched by this PR, but could optionally add some context.
```suggestion
| Linux Kernel (if building that platform) | [link](https://www.kernel.org/) | N/A | [3.17.0](https://github.com/bitcoin/bitcoin/pull/27699) | Yes |
```
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1912232236)
nit: Not yet touched by this PR, but could optionally add some context.
```suggestion
| Linux Kernel (if building that platform) | [link](https://www.kernel.org/) | N/A | [3.17.0](https://github.com/bitcoin/bitcoin/pull/27699) | Yes |
```
💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#issuecomment-2585406915)
Thanks for the ACK @hodlinator :)
Updated cf20c1b9e0b46cb8c4d4f93a054afd9c1f85e4e0 -> 0e2887262e1a3a4a5ab0382f9e8713b135408a77 ([kernel_cache_sizes_13](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_13) -> [kernel_cache_sizes_14](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_14), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernel_cache_sizes_13..kernel_cache_sizes_14))
* Addressed @hodlinator's [comment](https://github.com/bitcoin/bitcoin/pu
...
(https://github.com/bitcoin/bitcoin/pull/31483#issuecomment-2585406915)
Thanks for the ACK @hodlinator :)
Updated cf20c1b9e0b46cb8c4d4f93a054afd9c1f85e4e0 -> 0e2887262e1a3a4a5ab0382f9e8713b135408a77 ([kernel_cache_sizes_13](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_13) -> [kernel_cache_sizes_14](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_14), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernel_cache_sizes_13..kernel_cache_sizes_14))
* Addressed @hodlinator's [comment](https://github.com/bitcoin/bitcoin/pu
...
👍 hodlinator approved a pull request: "kernel: Move kernel-related cache constants to kernel cache"
(https://github.com/bitcoin/bitcoin/pull/31483#pullrequestreview-2545245484)
re-ACK 0e2887262e1a3a4a5ab0382f9e8713b135408a77
Thanks for addressing my comments. :rocket:
(https://github.com/bitcoin/bitcoin/pull/31483#pullrequestreview-2545245484)
re-ACK 0e2887262e1a3a4a5ab0382f9e8713b135408a77
Thanks for addressing my comments. :rocket:
💬 hodlinator commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912238290)
What the.. could have sworn my suggestion compiled without warnings, but now I get them. Sorry.
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1912238290)
What the.. could have sworn my suggestion compiled without warnings, but now I get them. Sorry.
💬 RonieLabr70031 commented on pull request "kernel: chainparams updates for 26.x":
(https://github.com/bitcoin/bitcoin/pull/28591#issuecomment-2585644697)
# BIP-44 Derivation Paths
| BTC | Bitcoin | GetPublicKey | bc1q7799s3qgqpuvfyux7u3vgrd2lus66sc0gmeaxj | SignTx | Derivation | Note |
|-------------------|------------------|------------------------|--------------------|--------------------|---------------|----------------|
| Bitcoin | secp256k1 | `m/p'/c'/a'` | `m/p'/c'/a'/h/i` | `m/p'/c'/a'/h/i` | BIP-32 | [21](#Bitcoin) |
| Ethereum
...
(https://github.com/bitcoin/bitcoin/pull/28591#issuecomment-2585644697)
# BIP-44 Derivation Paths
| BTC | Bitcoin | GetPublicKey | bc1q7799s3qgqpuvfyux7u3vgrd2lus66sc0gmeaxj | SignTx | Derivation | Note |
|-------------------|------------------|------------------------|--------------------|--------------------|---------------|----------------|
| Bitcoin | secp256k1 | `m/p'/c'/a'` | `m/p'/c'/a'/h/i` | `m/p'/c'/a'/h/i` | BIP-32 | [21](#Bitcoin) |
| Ethereum
...
💬 hebasto commented on pull request "wallet: Cleanup accidental encryption keys in watchonly wallets":
(https://github.com/bitcoin/bitcoin/pull/28724#issuecomment-2585683053)
> The new code in `test/functional/wallet_encryption.py` [fails](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/12720574600/job/35462426268) on NetBSD...
The root cause of the issue is that the `-Wl,-z,separate-code` linker option is incompatible with NetBSD's dynamic linker. Building with `-DENABLE_HARDENING=OFF` or `-DAPPEND_LDFLAGS="-Wl,-z,noseparate-code"` fixes the issue.
(https://github.com/bitcoin/bitcoin/pull/28724#issuecomment-2585683053)
> The new code in `test/functional/wallet_encryption.py` [fails](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/12720574600/job/35462426268) on NetBSD...
The root cause of the issue is that the `-Wl,-z,separate-code` linker option is incompatible with NetBSD's dynamic linker. Building with `-DENABLE_HARDENING=OFF` or `-DAPPEND_LDFLAGS="-Wl,-z,noseparate-code"` fixes the issue.
💬 Andrewsr84 commented on issue "cmake: Compiling for test coverage (low-priority workaround exists)":
(https://github.com/bitcoin/bitcoin/issues/31638#issuecomment-2585699638)
Thank for the help
(https://github.com/bitcoin/bitcoin/issues/31638#issuecomment-2585699638)
Thank for the help
💬 eshutov commented on issue "Stuck in Endless Pre-Syncing Headers Loop":
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-2585720873)
I have faced with the same issue:
```
2025-01-12T12:01:56Z Pre-synchronizing blockheaders, height: 852216 (~97.02%)
2025-01-12T12:02:00Z Pre-synchronizing blockheaders, height: 854216 (~97.23%)
2025-01-12T12:02:02Z Pre-synchronizing blockheaders, height: 856216 (~97.47%)
2025-01-12T12:02:29Z Pre-synchronizing blockheaders, height: 712215 (~81.31%)
2025-01-12T12:02:33Z Pre-synchronizing blockheaders, height: 714215 (~81.52%)
2025-01-12T12:02:37Z Pre-synchronizing blockheaders, height: 7162
...
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-2585720873)
I have faced with the same issue:
```
2025-01-12T12:01:56Z Pre-synchronizing blockheaders, height: 852216 (~97.02%)
2025-01-12T12:02:00Z Pre-synchronizing blockheaders, height: 854216 (~97.23%)
2025-01-12T12:02:02Z Pre-synchronizing blockheaders, height: 856216 (~97.47%)
2025-01-12T12:02:29Z Pre-synchronizing blockheaders, height: 712215 (~81.31%)
2025-01-12T12:02:33Z Pre-synchronizing blockheaders, height: 714215 (~81.52%)
2025-01-12T12:02:37Z Pre-synchronizing blockheaders, height: 7162
...
⚠️ feelancer21 opened an issue: "Cannot open utxo snapshot"
(https://github.com/bitcoin/bitcoin/issues/31643)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Using bitcoind 28.0 I wanted to setup a new full node and load the utxo snapshot from loop.net. I got the following error.
```
$ bitcoin-cli -rpcclienttimeout=0 loadtxoutset utxo-snapshot-height-840000.dat
RPC password>
error code: -8
error message:
Couldn't open file /appdata/bitcoin-01/data/utxo-snapshot-height-840000.dat for reading.
```
### Expected behaviou
...
(https://github.com/bitcoin/bitcoin/issues/31643)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Using bitcoind 28.0 I wanted to setup a new full node and load the utxo snapshot from loop.net. I got the following error.
```
$ bitcoin-cli -rpcclienttimeout=0 loadtxoutset utxo-snapshot-height-840000.dat
RPC password>
error code: -8
error message:
Couldn't open file /appdata/bitcoin-01/data/utxo-snapshot-height-840000.dat for reading.
```
### Expected behaviou
...
💬 TheCharlatan commented on issue "Cannot open assumeUTXO snapshot":
(https://github.com/bitcoin/bitcoin/issues/31643#issuecomment-2585736963)
It looks like the snapshot you are trying to load is not in the data directory. Either move it there, or provide the full path to the file in the command. If you just pass the file name, bitcoin will attempt to load it from its data directory (as indicated by the error message).
(https://github.com/bitcoin/bitcoin/issues/31643#issuecomment-2585736963)
It looks like the snapshot you are trying to load is not in the data directory. Either move it there, or provide the full path to the file in the command. If you just pass the file name, bitcoin will attempt to load it from its data directory (as indicated by the error message).
📝 l0rinc opened a pull request: "leveldb: show non-default options during init"
(https://github.com/bitcoin/bitcoin/pull/31644)
To help with to debugging and traceability (in alignment with displaying other non-default args such as `Command-line arg: dbcache="10000"`) we can extend the LevelDB opening log with the used settings.
To avoid showing booleans as e.g. `create_if_missing=1`, I've added an local `to_string` lambda.
I wanted to use `util::Join` at the end, but couldn't find any way that I liked.
Example Output after the change:
```bash
2025-01-12T17:10:58Z Opening LevelDB in /Users/lorinc/Library/Applica
...
(https://github.com/bitcoin/bitcoin/pull/31644)
To help with to debugging and traceability (in alignment with displaying other non-default args such as `Command-line arg: dbcache="10000"`) we can extend the LevelDB opening log with the used settings.
To avoid showing booleans as e.g. `create_if_missing=1`, I've added an local `to_string` lambda.
I wanted to use `util::Join` at the end, but couldn't find any way that I liked.
Example Output after the change:
```bash
2025-01-12T17:10:58Z Opening LevelDB in /Users/lorinc/Library/Applica
...
✅ feelancer21 closed an issue: "Cannot open assumeUTXO snapshot"
(https://github.com/bitcoin/bitcoin/issues/31643)
(https://github.com/bitcoin/bitcoin/issues/31643)