👍 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.
💬 fjahr commented on pull request "wallet, desc spkm: Return SigningProvider only if we have the privkey":
(https://github.com/bitcoin/bitcoin/pull/31242#issuecomment-2565453438)
utACK 493656763f73e5ef1cfb979a513c12983dca99dd
Test coverage would be nice but I suppose it might be skipped if this get coverage as part of wider coverage of the musig2 code paths.
(https://github.com/bitcoin/bitcoin/pull/31242#issuecomment-2565453438)
utACK 493656763f73e5ef1cfb979a513c12983dca99dd
Test coverage would be nice but I suppose it might be skipped if this get coverage as part of wider coverage of the musig2 code paths.
💬 fjahr commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2565454379)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2565454379)
Concept ACK
🤔 furszy reviewed a pull request: "test: descriptor: fix test for `MaxSatisfactionWeight`"
(https://github.com/bitcoin/bitcoin/pull/31570#pullrequestreview-2525657833)
utACK b29d68f942e
(https://github.com/bitcoin/bitcoin/pull/31570#pullrequestreview-2525657833)
utACK b29d68f942e
💬 furszy commented on pull request "test: descriptor: fix test for `MaxSatisfactionWeight`":
(https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1899587183)
nit: could use `BOOST_CHECK_LE`.
(https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1899587183)
nit: could use `BOOST_CHECK_LE`.
👍 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-2525711019)
re-ACK 6983d82c84d4ca351a7cd9d1e0e20ab878da6475
Responding to my previous feedback + testing negative max return values.
Retested modified targets.
(https://github.com/bitcoin/bitcoin/pull/30746#pullrequestreview-2525711019)
re-ACK 6983d82c84d4ca351a7cd9d1e0e20ab878da6475
Responding to my previous feedback + testing negative max return values.
Retested modified targets.
📝 hebasto opened a pull request: "test: Remove non-portable IPv6 test"
(https://github.com/bitcoin/bitcoin/pull/31580)
On Illumos-based systems, such as OpenIndiana and SmartOS, the assumption that "the default zone ID of 0 can be omitted for the default scope" is incorrect. As a result, `getaddrinfo("fe80::1%0", ...)` returns the `EAI_NONAME` error instead of resolving to "fe80::1".
See: https://www.illumos.org/man/3SOCKET/getaddrinfo.
This PR removes the problematic code introduced in https://github.com/bitcoin/bitcoin/pull/19951.
(https://github.com/bitcoin/bitcoin/pull/31580)
On Illumos-based systems, such as OpenIndiana and SmartOS, the assumption that "the default zone ID of 0 can be omitted for the default scope" is incorrect. As a result, `getaddrinfo("fe80::1%0", ...)` returns the `EAI_NONAME` error instead of resolving to "fe80::1".
See: https://www.illumos.org/man/3SOCKET/getaddrinfo.
This PR removes the problematic code introduced in https://github.com/bitcoin/bitcoin/pull/19951.
💬 hebasto commented on pull request "test: Remove non-portable IPv6 test":
(https://github.com/bitcoin/bitcoin/pull/31580#issuecomment-2565627902)
cc @jonatack @laanwj @ryanofsky @vasild
(https://github.com/bitcoin/bitcoin/pull/31580#issuecomment-2565627902)
cc @jonatack @laanwj @ryanofsky @vasild
💬 hebasto commented on pull request "net, test: CNetAddr scoped ipv6 test coverage, rename scopeId to m_scope_id":
(https://github.com/bitcoin/bitcoin/pull/19951#discussion_r1899626430)
Apparently, this test is not portable. See: https://github.com/bitcoin/bitcoin/pull/31580.
(https://github.com/bitcoin/bitcoin/pull/19951#discussion_r1899626430)
Apparently, this test is not portable. See: https://github.com/bitcoin/bitcoin/pull/31580.
💬 fjahr commented on pull request "Add `contrib/justfile` containing useful development workflow commands.":
(https://github.com/bitcoin/bitcoin/pull/31292#issuecomment-2565635915)
This would require some additional documentation in order to be merged but I also think this doesn't belong into this repo per the rationale given by @achow101 . Such files would probably add a lot of noise to the repo even from well meaning contributors. The approach @willcl-ark suggests sounds like a good compromise. Maybe we could have a page in the wiki that maintains a list of productivity tools and resources like this. I wasn't aware of Will's repo myself until now.
(https://github.com/bitcoin/bitcoin/pull/31292#issuecomment-2565635915)
This would require some additional documentation in order to be merged but I also think this doesn't belong into this repo per the rationale given by @achow101 . Such files would probably add a lot of noise to the repo even from well meaning contributors. The approach @willcl-ark suggests sounds like a good compromise. Maybe we could have a page in the wiki that maintains a list of productivity tools and resources like this. I wasn't aware of Will's repo myself until now.