💬 hMsats commented on issue "bitcoind 29.0 much slower than 28.0 on my system: cause found":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2940296584)
That's an impressive amount of data analyses that will take me some time to fully understand but thanks a lot!
I did some more testing but for my system 2 MiB is a clear winner and the good news is that then my server runs flawlessly but I need to let it run for more blocks. On the other hand, there are other people running version 29.0 and (with google) I haven't seen any complains yet.
> In your case (with compact blocks) an empty mempool would also introduce some variation.
During testing
...
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2940296584)
That's an impressive amount of data analyses that will take me some time to fully understand but thanks a lot!
I did some more testing but for my system 2 MiB is a clear winner and the good news is that then my server runs flawlessly but I need to let it run for more blocks. On the other hand, there are other people running version 29.0 and (with google) I haven't seen any complains yet.
> In your case (with compact blocks) an empty mempool would also introduce some variation.
During testing
...
🚀 fanquake merged a pull request: "cmake: Replace deprecated `qt6_add_translation` with `qt6_add_lrelease`"
(https://github.com/bitcoin/bitcoin/pull/32651)
(https://github.com/bitcoin/bitcoin/pull/32651)
🤔 hodlinator reviewed a pull request: "headerssync: Keep tests ahead of increasing params"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-2896798619)
Latest push (42e746096b93b8026569817d8925750089d6daf4, [compare](https://github.com/bitcoin/bitcoin/compare/bbe5609b84211c49a522da9dd5d987ab454ef912..42e746096b93b8026569817d8925750089d6daf4)):
* Relaxes the hardcoding of the header commitment period and the redownload buffer size in `HeadersSyncState`.
- This makes things more consistent with the constructor already taking `Consensus::Params`.
- Moves specification of the above constants from the top of *headerssync.cpp* into *kernel/cha
...
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-2896798619)
Latest push (42e746096b93b8026569817d8925750089d6daf4, [compare](https://github.com/bitcoin/bitcoin/compare/bbe5609b84211c49a522da9dd5d987ab454ef912..42e746096b93b8026569817d8925750089d6daf4)):
* Relaxes the hardcoding of the header commitment period and the redownload buffer size in `HeadersSyncState`.
- This makes things more consistent with the constructor already taking `Consensus::Params`.
- Moves specification of the above constants from the top of *headerssync.cpp* into *kernel/cha
...
💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2126641973)
Added generation date in latest push as it was making modifications to the Python script anyway.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2126641973)
Added generation date in latest push as it was making modifications to the Python script anyway.
💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2126653972)
Implemented in latest push.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2126653972)
Implemented in latest push.
💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2126644715)
Latest push has:
```C++
struct HeadersSyncParams {
//! Distance in blocks between header commitments.
size_t commitment_period;
...
```
in *src/kernel/chainparams.h*.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2126644715)
Latest push has:
```C++
struct HeadersSyncParams {
//! Distance in blocks between header commitments.
size_t commitment_period;
...
```
in *src/kernel/chainparams.h*.
✅ maflcko closed an issue: "Windows Python: Intermittent issue in p2p_unrequested_blocks.py", line 98, in run_test test_node.send_and_ping(msg_block(blocks_h2[0])) assert self.is_connected AssertionError / AcceptBlock FAILED (time-too-old, block's timestamp is too early) "
(https://github.com/bitcoin/bitcoin/issues/29897)
(https://github.com/bitcoin/bitcoin/issues/29897)
💬 maflcko commented on issue "Windows Python: Intermittent issue in p2p_unrequested_blocks.py", line 98, in run_test test_node.send_and_ping(msg_block(blocks_h2[0])) assert self.is_connected AssertionError / AcceptBlock FAILED (time-too-old, block's timestamp is too early) ":
(https://github.com/bitcoin/bitcoin/issues/29897#issuecomment-2940318094)
According to https://github.com/python/cpython/commit/1d95451be1f3080904c00cc4c4a6cc519efdf321 the precision on Windows of `time.time()` was only 15.6 ms! This was fixed in python3.13, which is available on GHA runners for a few months now: https://github.com/actions/runner-images/commit/c8b6f67c08223abbce0b553ad4a2346ea03bdfd5#diff-d3119a278ce9ce80fc4b5af63a1112fb3a044aec3e6002d838e8eb26d7a3a499R60
(For reference the MS STL commit that dropped support for windows 7 is https://github.com/micros
...
(https://github.com/bitcoin/bitcoin/issues/29897#issuecomment-2940318094)
According to https://github.com/python/cpython/commit/1d95451be1f3080904c00cc4c4a6cc519efdf321 the precision on Windows of `time.time()` was only 15.6 ms! This was fixed in python3.13, which is available on GHA runners for a few months now: https://github.com/actions/runner-images/commit/c8b6f67c08223abbce0b553ad4a2346ea03bdfd5#diff-d3119a278ce9ce80fc4b5af63a1112fb3a044aec3e6002d838e8eb26d7a3a499R60
(For reference the MS STL commit that dropped support for windows 7 is https://github.com/micros
...
💬 maflcko commented on issue "Networking tests fail on emulated big-endian systems":
(https://github.com/bitcoin/bitcoin/issues/31812#issuecomment-2940340520)
> > the `interface_bitcoin_cli` tests are likely also related to the IPv6 big-endian failures:
>
> What are the exact steps to reproduce? Ideally, starting from a fresh install? I guess it happens on macOS and on the Hetzner box equally?
cc @l0rinc
(https://github.com/bitcoin/bitcoin/issues/31812#issuecomment-2940340520)
> > the `interface_bitcoin_cli` tests are likely also related to the IPv6 big-endian failures:
>
> What are the exact steps to reproduce? Ideally, starting from a fresh install? I guess it happens on macOS and on the Hetzner box equally?
cc @l0rinc
💬 maflcko commented on pull request "depends: drop `ltcg` for Windows Qt":
(https://github.com/bitcoin/bitcoin/pull/32496#issuecomment-2940365526)
lgtm ACK 9653ebc053602a30aee6d8676913306d615841cf
(https://github.com/bitcoin/bitcoin/pull/32496#issuecomment-2940365526)
lgtm ACK 9653ebc053602a30aee6d8676913306d615841cf
💬 maflcko commented on pull request "fuzz: Add target for coins database":
(https://github.com/bitcoin/bitcoin/pull/32602#issuecomment-2940437069)
lgtm ACK cfc42ae5b7ef6d3892907cfe36dc3ae923495d37
(https://github.com/bitcoin/bitcoin/pull/32602#issuecomment-2940437069)
lgtm ACK cfc42ae5b7ef6d3892907cfe36dc3ae923495d37
💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#issuecomment-2940454325)
Thanks for reviewing @stickies-v! I took your suggestion for mempool's `exists`, `info`, and `info_for_relay`. These are all fairly simple functions where I agree that the separate txid/wtxid overloads make the code clearer.
I looked at `txrequest` and `txdownloadman_impl` to see where txid and wtxid could be used internally in place of the variant, but it seems to be a less straightfoward refactor. Maybe `AddTxAnnouncement` could be a good candidate? If you see something that I'm missing (or
...
(https://github.com/bitcoin/bitcoin/pull/32631#issuecomment-2940454325)
Thanks for reviewing @stickies-v! I took your suggestion for mempool's `exists`, `info`, and `info_for_relay`. These are all fairly simple functions where I agree that the separate txid/wtxid overloads make the code clearer.
I looked at `txrequest` and `txdownloadman_impl` to see where txid and wtxid could be used internally in place of the variant, but it seems to be a less straightfoward refactor. Maybe `AddTxAnnouncement` could be a good candidate? If you see something that I'm missing (or
...
💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2126881339)
done
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2126881339)
done
💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2126882401)
I gotchu
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2126882401)
I gotchu
💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2126883263)
Thank you for the patch, it helped.
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2126883263)
Thank you for the patch, it helped.
💬 RandyMcMillan commented on pull request "uint256 cxx-20 constexpr patch":
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2940472143)
@hebasto,
I would suggest adding tests to the MacOS cross compile and/or actually building for MacOS x86_64.
This PR actually enables compiling with:
```
$ clang -v
Apple clang version 14.0.3 (clang-1403.0.22.14.1)
Target: x86_64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
```

---
If you are confident NO
...
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2940472143)
@hebasto,
I would suggest adding tests to the MacOS cross compile and/or actually building for MacOS x86_64.
This PR actually enables compiling with:
```
$ clang -v
Apple clang version 14.0.3 (clang-1403.0.22.14.1)
Target: x86_64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
```

---
If you are confident NO
...
💬 fanquake commented on pull request "uint256 cxx-20 constexpr patch":
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2940488234)
> This PR actually enables compiling with:
> Apple clang version 14.0.3 (clang-1403.0.22.14.1)
Apple Clang 14.x is based on LLVM Clang 15. We currently support Clang 16 or later.
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2940488234)
> This PR actually enables compiling with:
> Apple clang version 14.0.3 (clang-1403.0.22.14.1)
Apple Clang 14.x is based on LLVM Clang 15. We currently support Clang 16 or later.
💬 RandyMcMillan commented on pull request "uint256 cxx-20 constexpr patch":
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2940489326)
https://github.com/bitcoincore-dev/bitcoin/tree/1944/899512/227275/515514a4e6/7c6d5aa000-uint256-cxx-20-constexpr-patch
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2940489326)
https://github.com/bitcoincore-dev/bitcoin/tree/1944/899512/227275/515514a4e6/7c6d5aa000-uint256-cxx-20-constexpr-patch
💬 dergoegge commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r2126904911)
It'd be better to create a fake `NetEventsInterface` for this test, as we are testing connman not peerman. Peerman is also a global here which might have unwanted side effects.
I haven't tried running the most recent version of this test but it looks like it'll run out of memory eventually as the `Peer`s allocated in peerman are never deleted?
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r2126904911)
It'd be better to create a fake `NetEventsInterface` for this test, as we are testing connman not peerman. Peerman is also a global here which might have unwanted side effects.
I haven't tried running the most recent version of this test but it looks like it'll run out of memory eventually as the `Peer`s allocated in peerman are never deleted?
💬 RandyMcMillan commented on pull request "uint256 cxx-20 constexpr patch":
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2940501114)
> > This PR actually enables compiling with:
> > Apple clang version 14.0.3 (clang-1403.0.22.14.1)
>
> Apple Clang 14.x is based on LLVM Clang 15. We currently support Clang 16 or later.
I suggest "trusting LLVM less"
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2940501114)
> > This PR actually enables compiling with:
> > Apple clang version 14.0.3 (clang-1403.0.22.14.1)
>
> Apple Clang 14.x is based on LLVM Clang 15. We currently support Clang 16 or later.
I suggest "trusting LLVM less"