💬 TheCharlatan commented on pull request "validation: Send correct notification during snapshot completion":
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1898917833)
Should the docs be updated to mention the added node?
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1898917833)
Should the docs be updated to mention the added node?
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1898942778)
Adjusted, thanks:
> // It's been a while since we wrote the block index and chain state to disk. Do this frequently, so we don't need to redownload or reindex after a crash.
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1898942778)
Adjusted, thanks:
> // It's been a while since we wrote the block index and chain state to disk. Do this frequently, so we don't need to redownload or reindex after a crash.
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2564363281)
Rebased due to conflicts from #31534.
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2564363281)
Rebased due to conflicts from #31534.
💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2564375695)
ACK b37452ca437856ed5d8effce2cef737d9df479c1
The differences since last ACK:
* the conflicting new logging - which had to be reindented;
* `chain state` and `reindex` mentioned in the comment of `fPeriodicWrite`.
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2564375695)
ACK b37452ca437856ed5d8effce2cef737d9df479c1
The differences since last ACK:
* the conflicting new logging - which had to be reindented;
* `chain state` and `reindex` mentioned in the comment of `fPeriodicWrite`.
👍 tdb3 approved a pull request: "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script"
(https://github.com/bitcoin/bitcoin/pull/31560#pullrequestreview-2524765817)
ACK 59df8480be77a8e3618c9422536c4f8aed82467e
Also re-ran tests in https://github.com/bitcoin/bitcoin/pull/31560#pullrequestreview-2522042088
(https://github.com/bitcoin/bitcoin/pull/31560#pullrequestreview-2524765817)
ACK 59df8480be77a8e3618c9422536c4f8aed82467e
Also re-ran tests in https://github.com/bitcoin/bitcoin/pull/31560#pullrequestreview-2522042088
👍 tdb3 approved a pull request: "test: descriptor: fix test for `MaxSatisfactionWeight`"
(https://github.com/bitcoin/bitcoin/pull/31570#pullrequestreview-2524768462)
re ACK b29d68f942e333d2cfdd6be8d49fe484e0d15e11
(https://github.com/bitcoin/bitcoin/pull/31570#pullrequestreview-2524768462)
re ACK b29d68f942e333d2cfdd6be8d49fe484e0d15e11
🤔 tdb3 reviewed a pull request: "contrib: add tool to convert compact-serialized UTXO set to SQLite database"
(https://github.com/bitcoin/bitcoin/pull/27432#pullrequestreview-2524872630)
Approach ACK
Providing some manual test results. Planning to review `utxo_to_sqlite.py` in more detail.
Created a utxo dump (for 876819), created the sqlite dump from it, then ran `calc_utxo_hash.go` against it. muhash values matched.
```
$ bitcoin-cli dumptxoutset ~/utxos876819.dat
$ bitcoin-cli gettxoutsetinfo muhash 876819
...
"muhash": "ffb3cbccf78eba08c13f5486b05562f399058023f4f10b593973c448966826f3",
$ contrib/utxo-tools/utxo_to_sqlite.py ~/utxos876819.dat ~/utxos876819.sql
...
(https://github.com/bitcoin/bitcoin/pull/27432#pullrequestreview-2524872630)
Approach ACK
Providing some manual test results. Planning to review `utxo_to_sqlite.py` in more detail.
Created a utxo dump (for 876819), created the sqlite dump from it, then ran `calc_utxo_hash.go` against it. muhash values matched.
```
$ bitcoin-cli dumptxoutset ~/utxos876819.dat
$ bitcoin-cli gettxoutsetinfo muhash 876819
...
"muhash": "ffb3cbccf78eba08c13f5486b05562f399058023f4f10b593973c448966826f3",
$ contrib/utxo-tools/utxo_to_sqlite.py ~/utxos876819.dat ~/utxos876819.sql
...
💬 JeremiahR commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1899052947)
typo
```suggestion
// Return even if the peer isn't an orphan resolution candidate. This would be caught by AlreadyHaveTx.
```
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1899052947)
typo
```suggestion
// Return even if the peer isn't an orphan resolution candidate. This would be caught by AlreadyHaveTx.
```
💬 Eunovo commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1899085059)
It looks like you have resolved this in https://github.com/bitcoin/bitcoin/pull/31404/commits/e1d6179b61a2d4766c5eb6183a839fcaa6e566fa. Mark this as resolved?
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1899085059)
It looks like you have resolved this in https://github.com/bitcoin/bitcoin/pull/31404/commits/e1d6179b61a2d4766c5eb6183a839fcaa6e566fa. Mark this as resolved?
👍 romanz approved a pull request: "contrib: add tool to convert compact-serialized UTXO set to SQLite database"
(https://github.com/bitcoin/bitcoin/pull/27432#pullrequestreview-2524929410)
tACK 4080b66cbe on signet (using [calc_utxo_hash](https://github.com/theStack/utxo_dump_tools/blob/8981aa3e85efac046f0f3b6a1f99d3f4a273cdd1/calc_utxo_hash/calc_utxo_hash.go)):
```
$ bitcoin-cli -signet dumptxoutset signet.utxo latest
{
"coins_written": 5898081,
"base_hash": "0000007e74079165369e130c7df56bcfa8100f64049d82d69b8fdce6fe95644f",
"base_height": 228463,
"path": "/home/user/.bitcoin/signet/signet.utxo",
"txoutset_hash": "3ce62cf97d0acbb35e3e8d822760df08b8eb7bcc98886c8b
...
(https://github.com/bitcoin/bitcoin/pull/27432#pullrequestreview-2524929410)
tACK 4080b66cbe on signet (using [calc_utxo_hash](https://github.com/theStack/utxo_dump_tools/blob/8981aa3e85efac046f0f3b6a1f99d3f4a273cdd1/calc_utxo_hash/calc_utxo_hash.go)):
```
$ bitcoin-cli -signet dumptxoutset signet.utxo latest
{
"coins_written": 5898081,
"base_hash": "0000007e74079165369e130c7df56bcfa8100f64049d82d69b8fdce6fe95644f",
"base_height": 228463,
"path": "/home/user/.bitcoin/signet/signet.utxo",
"txoutset_hash": "3ce62cf97d0acbb35e3e8d822760df08b8eb7bcc98886c8b
...
💬 jafpri1967 commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2564733016)
>
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2564733016)
>
💬 rebroad commented on issue "verification progress dropped 11% on bitcoin core update from 22 to 28":
(https://github.com/bitcoin/bitcoin/issues/31473#issuecomment-2564817598)
> Sync progress as reported by Bitcoin Core is really only accurate up to the height of the chain at the time of release. The number of transactions in the blockchain at that time are hard coded:
>
> https://github.com/bitcoin/bitcoin/blob/d6b225f1652526cb053ec32c8ff09160d5a759c5/src/kernel/chainparams.cpp#L195-L200
>
> These data are updated with every new release:
>
> https://github.com/bitcoin/bitcoin/blob/28.x/doc/release-process.md#before-branch-off
>
> After the hard-coded heig
...
(https://github.com/bitcoin/bitcoin/issues/31473#issuecomment-2564817598)
> Sync progress as reported by Bitcoin Core is really only accurate up to the height of the chain at the time of release. The number of transactions in the blockchain at that time are hard coded:
>
> https://github.com/bitcoin/bitcoin/blob/d6b225f1652526cb053ec32c8ff09160d5a759c5/src/kernel/chainparams.cpp#L195-L200
>
> These data are updated with every new release:
>
> https://github.com/bitcoin/bitcoin/blob/28.x/doc/release-process.md#before-branch-off
>
> After the hard-coded heig
...
💬 pinheadmz commented on issue "verification progress dropped 11% on bitcoin core update from 22 to 28":
(https://github.com/bitcoin/bitcoin/issues/31473#issuecomment-2564873934)
> Are you implying that block production can vary by as much as 11% from predicted rates in 3 years?
It's about total number of transactions. Created by humans. Very unpredictable.
(https://github.com/bitcoin/bitcoin/issues/31473#issuecomment-2564873934)
> Are you implying that block production can vary by as much as 11% from predicted rates in 3 years?
It's about total number of transactions. Created by humans. Very unpredictable.
⚠️ kelvinator07 opened an issue: " Cannot figure out how to use std::atomic error for MacOS Sequoia 15.2"
(https://github.com/bitcoin/bitcoin/issues/31579)
Fails to compile when running `cmake -B build -DWITH_BDB=ON`
Attempted the build process on two different laptops with identical specifications and encountered the same error.
Build Output:
```
-- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC
-- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC - Failed
-- Performing Test STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC
-- Performing Test STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC - Failed
CMake Error at cmake/module/TestAppendRequiredLibraries.cmake:
...
(https://github.com/bitcoin/bitcoin/issues/31579)
Fails to compile when running `cmake -B build -DWITH_BDB=ON`
Attempted the build process on two different laptops with identical specifications and encountered the same error.
Build Output:
```
-- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC
-- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC - Failed
-- Performing Test STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC
-- Performing Test STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC - Failed
CMake Error at cmake/module/TestAppendRequiredLibraries.cmake:
...
💬 araujo0608 commented on issue " (bitcoin core warning: unknown new rules activated versionbit 2) ":
(https://github.com/bitcoin/bitcoin/issues/31536#issuecomment-2564980074)
I'm new to this whole Bitcoin thing. But since it's software, I would look at the log file. Look for errors or warnings related to the wallet or blockchain synchronization.
(https://github.com/bitcoin/bitcoin/issues/31536#issuecomment-2564980074)
I'm new to this whole Bitcoin thing. But since it's software, I would look at the log file. Look for errors or warnings related to the wallet or blockchain synchronization.
👍 hodlinator approved a pull request: "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests"
(https://github.com/bitcoin/bitcoin/pull/30746#pullrequestreview-2525417419)
re-ACK a32002f2d5cff72639c9782d70ae52ec162d9ef8
- Only added commit that fuzzes the max length parameter of `DecodeBase58`&`DecodeBase58Check`.
Re-ran the 2 modified fuzz-targets 30 seconds each.
(https://github.com/bitcoin/bitcoin/pull/30746#pullrequestreview-2525417419)
re-ACK a32002f2d5cff72639c9782d70ae52ec162d9ef8
- Only added commit that fuzzes the max length parameter of `DecodeBase58`&`DecodeBase58Check`.
Re-ran the 2 modified fuzz-targets 30 seconds each.
💬 hodlinator commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1899441448)
nit: As `random_string` is currently `0-1000` long, and `max_ret_len` is `1-300`, most of the time `DecodeBase58()` is just returning `false` since `max_ret_len` is too small. But maybe that is intentional to stress that aspect?
Might be better to have more similar lengths, and also to include `0` as `max_ret_len` since it might occur in places where that parameter is calculated.
```suggestion
const std::string random_string{provider.ConsumeRandomLengthString(1000)};
const auto m
...
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1899441448)
nit: As `random_string` is currently `0-1000` long, and `max_ret_len` is `1-300`, most of the time `DecodeBase58()` is just returning `false` since `max_ret_len` is too small. But maybe that is intentional to stress that aspect?
Might be better to have more similar lengths, and also to include `0` as `max_ret_len` since it might occur in places where that parameter is calculated.
```suggestion
const std::string random_string{provider.ConsumeRandomLengthString(1000)};
const auto m
...
💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1899492703)
Good point, and even negative values theoretically - thanks, [pushed](https://github.com/bitcoin/bitcoin/compare/a32002f2d5cff72639c9782d70ae52ec162d9ef8..6983d82c84d4ca351a7cd9d1e0e20ab878da6475)
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1899492703)
Good point, and even negative values theoretically - thanks, [pushed](https://github.com/bitcoin/bitcoin/compare/a32002f2d5cff72639c9782d70ae52ec162d9ef8..6983d82c84d4ca351a7cd9d1e0e20ab878da6475)
💬 fjahr commented on pull request "refactor: std::span compat fixes":
(https://github.com/bitcoin/bitcoin/pull/31540#issuecomment-2565392387)
Code review ACK fa494a1d53f3f030fafe7b533d72b2200428a0fd
(https://github.com/bitcoin/bitcoin/pull/31540#issuecomment-2565392387)
Code review ACK fa494a1d53f3f030fafe7b533d72b2200428a0fd
💬 l0rinc commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2565453331)
@ismaelsadeeq https://github.com/bitcoin/bitcoin/pull/20999 by @martinus also contains a few optimization attempts (a lot of them were already applied in other PRs since, but a few of the changes can still be done).
If RPC speed is important to you, the above suggestions can result in a measurable speedups.
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2565453331)
@ismaelsadeeq https://github.com/bitcoin/bitcoin/pull/20999 by @martinus also contains a few optimization attempts (a lot of them were already applied in other PRs since, but a few of the changes can still be done).
If RPC speed is important to you, the above suggestions can result in a measurable speedups.