💬 maflcko commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756594931)
Thanks, adjusted commit message.
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756594931)
Thanks, adjusted commit message.
💬 maflcko commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756595040)
Thanks, adjusted commit message.
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756595040)
Thanks, adjusted commit message.
💬 maflcko commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756595333)
Yes.
I do not know. I presume this is just the initial value that was set when there were no debug builds or builds with sanitizers (which are larger).
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756595333)
Yes.
I do not know. I presume this is just the initial value that was set when there were no debug builds or builds with sanitizers (which are larger).
💬 maflcko commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756595452)
Nah, it was just copy-pasted from above. Fixed the style here.
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756595452)
Nah, it was just copy-pasted from above. Fixed the style here.
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#issuecomment-2345968106)
utACK 2222b44d35c092b5aaefed12bc5ca608e6e78760
(https://github.com/bitcoin/bitcoin/pull/30869#issuecomment-2345968106)
utACK 2222b44d35c092b5aaefed12bc5ca608e6e78760
💬 fanquake commented on pull request "test: remove unused src_dir param from run_tests after CMake migration":
(https://github.com/bitcoin/bitcoin/pull/30733#issuecomment-2345997775)
@davidgumberg fyi you've ack'd the previous commit has here.
(https://github.com/bitcoin/bitcoin/pull/30733#issuecomment-2345997775)
@davidgumberg fyi you've ack'd the previous commit has here.
🚀 fanquake merged a pull request: "test: remove unused src_dir param from run_tests after CMake migration"
(https://github.com/bitcoin/bitcoin/pull/30733)
(https://github.com/bitcoin/bitcoin/pull/30733)
💬 fanquake commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2346016603)
Could you rebase this, now that guix builds are reproducible again.
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2346016603)
Could you rebase this, now that guix builds are reproducible again.
💬 l0rinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1756663411)
It seems to me that production code never actually calls `AddFlags` with `0` value - and we're always setting `DIRTY`, even when `FRESH` is set.
So it would make sense to change `AddFlags` from having a custom `uint8_t flags` parameter which is always `DIRTY` (and sometimes `FRESH`) in reality (and is only called with `0` flag in tests) to have a freshness parameter like `inline void SetDirtyAndFresh(CoinsCachePair& self, CoinsCachePair& sentinel, bool is_fresh = false) noexcept` instead.
This
...
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1756663411)
It seems to me that production code never actually calls `AddFlags` with `0` value - and we're always setting `DIRTY`, even when `FRESH` is set.
So it would make sense to change `AddFlags` from having a custom `uint8_t flags` parameter which is always `DIRTY` (and sometimes `FRESH`) in reality (and is only called with `0` flag in tests) to have a freshness parameter like `inline void SetDirtyAndFresh(CoinsCachePair& self, CoinsCachePair& sentinel, bool is_fresh = false) noexcept` instead.
This
...
💬 hebasto commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1754576656)
> I think I'm missing something here. Isn't it a bug that us already setting `SECP256K1_BUILD_CTIME_TESTS=OFF` is sometimes overridden later on? Otherwise, this change is unclear, because the ctime tests (along with the other secp256k1 tests) are already supposedly controlled by `BUILD_TESTS`, but now the ctime tests are being wrapped in another `BUILD_TESTS` conditional? (adding a comment would be good, otherwise I assume anyone reading this code will assume the second conditional is redundant)
...
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1754576656)
> I think I'm missing something here. Isn't it a bug that us already setting `SECP256K1_BUILD_CTIME_TESTS=OFF` is sometimes overridden later on? Otherwise, this change is unclear, because the ctime tests (along with the other secp256k1 tests) are already supposedly controlled by `BUILD_TESTS`, but now the ctime tests are being wrapped in another `BUILD_TESTS` conditional? (adding a comment would be good, otherwise I assume anyone reading this code will assume the second conditional is redundant)
...
💬 willcl-ark commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2346027116)
> There are a bunch of places where `CCACHE_MAXSIZE` is hardcoded, making it impossible to set a larger size. This needs to be fixed first, so I did that in #30869 (among other stuff).
Awesome!
> Assuming .5GB of caches per task, 10 tasks per push, 100 pushes per day, and a cache duration of 20 days, means that 10TB of storage should be sufficient.
Not sure I understand this; the idea is to have "one ccache per task" surely, not one per push?
I was thinking we should set up with:
...
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2346027116)
> There are a bunch of places where `CCACHE_MAXSIZE` is hardcoded, making it impossible to set a larger size. This needs to be fixed first, so I did that in #30869 (among other stuff).
Awesome!
> Assuming .5GB of caches per task, 10 tasks per push, 100 pushes per day, and a cache duration of 20 days, means that 10TB of storage should be sufficient.
Not sure I understand this; the idea is to have "one ccache per task" surely, not one per push?
I was thinking we should set up with:
...
💬 hebasto commented on issue "GUI event loop should be block free":
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2346033798)
> What is the status of the QML GUI? IIRC it was put on hold, because the Android APK in depends couldn't be updated for some reason, but I may be misremembering.
The development continues, but the code base is not synced with this repository until re-introducing ability to build for Android.
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2346033798)
> What is the status of the QML GUI? IIRC it was put on hold, because the Android APK in depends couldn't be updated for some reason, but I may be misremembering.
The development continues, but the code base is not synced with this repository until re-introducing ability to build for Android.
💬 fanquake commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756673070)
I'm not sure, because in the case being fixed here (`-DBUILD_FOR_FUZZING`), `BUILD_TESTS=OFF`, so we aren't forcing it to `ON`?
It also doesn't explain why our setting (of `OFF`) is being overriden later. The same thing doesn't happen if you pass `SECP256K1_BUILD_CTIME_TESTS=OFF` when invoking cmake.
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756673070)
I'm not sure, because in the case being fixed here (`-DBUILD_FOR_FUZZING`), `BUILD_TESTS=OFF`, so we aren't forcing it to `ON`?
It also doesn't explain why our setting (of `OFF`) is being overriden later. The same thing doesn't happen if you pass `SECP256K1_BUILD_CTIME_TESTS=OFF` when invoking cmake.
💬 theStack commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2346043306)
> Could you rebase this, now that guix builds are reproducible again.
Sure, done.
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2346043306)
> Could you rebase this, now that guix builds are reproducible again.
Sure, done.
💬 hebasto commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756681136)
> I'm not sure, because in the case being fixed here (`-DBUILD_FOR_FUZZING`), `BUILD_TESTS=OFF`, so we aren't forcing it to `ON`?
We cannot force it to `ON` because the build will requires the installed Valgrind, which is not always the case.
> It also doesn't explain why our setting (of `OFF`) is being overriden later.
When?
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756681136)
> I'm not sure, because in the case being fixed here (`-DBUILD_FOR_FUZZING`), `BUILD_TESTS=OFF`, so we aren't forcing it to `ON`?
We cannot force it to `ON` because the build will requires the installed Valgrind, which is not always the case.
> It also doesn't explain why our setting (of `OFF`) is being overriden later.
When?
💬 maflcko commented on pull request "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`":
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2346074452)
Not sure why, but for me the command took almost an hour:
```
$ ps -p 816447 --format time,rss,cmd
TIME RSS CMD
00:51:12 117640 /usr/bin/cmake -DRAW_SOURCE_PATH=_/src/bench/data/block413567.raw -DHEADER_PATH=_/bld-cmake/src/bench/data/block413567.raw.h -DRAW_NAMESPACE=benchmark::data -P _/cmake/script/GenerateHeaderFromRaw.cmake
```
Previously it took only a few seconds.
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2346074452)
Not sure why, but for me the command took almost an hour:
```
$ ps -p 816447 --format time,rss,cmd
TIME RSS CMD
00:51:12 117640 /usr/bin/cmake -DRAW_SOURCE_PATH=_/src/bench/data/block413567.raw -DHEADER_PATH=_/bld-cmake/src/bench/data/block413567.raw.h -DRAW_NAMESPACE=benchmark::data -P _/cmake/script/GenerateHeaderFromRaw.cmake
```
Previously it took only a few seconds.
💬 pablomartin4btc commented on pull request "rpc: add `revelant_blocks` to `scanblocks status`":
(https://github.com/bitcoin/bitcoin/pull/30713#issuecomment-2346078539)
> Any ideas on how to slow down `scanblocks()` enough to allow a `status` call to consistently return status of a scan in progress?
I've been playing a bit around the `invalidateblock` as I had present in my mind the recent [issue](https://github.com/bitcoin/bitcoin/pull/30808#issue-2504050692) from the assume-utxo testing but that wouldn't make the trick either. I went thru the original PRs of `scanblocks` (#23549 and #20664) but it seems there were no questions around this 'status' call te
...
(https://github.com/bitcoin/bitcoin/pull/30713#issuecomment-2346078539)
> Any ideas on how to slow down `scanblocks()` enough to allow a `status` call to consistently return status of a scan in progress?
I've been playing a bit around the `invalidateblock` as I had present in my mind the recent [issue](https://github.com/bitcoin/bitcoin/pull/30808#issue-2504050692) from the assume-utxo testing but that wouldn't make the trick either. I went thru the original PRs of `scanblocks` (#23549 and #20664) but it seems there were no questions around this 'status' call te
...
💬 hebasto commented on pull request "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`":
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2346084391)
> Not sure why, but for me the command took almost an hour:
>
>
>
>
>
> ```
>
> $ ps -p 816447 --format time,rss,cmd
>
> TIME RSS CMD
>
> 00:51:12 117640 /usr/bin/cmake -DRAW_SOURCE_PATH=_/src/bench/data/block413567.raw -DHEADER_PATH=_/bld-cmake/src/bench/data/block413567.raw.h -DRAW_NAMESPACE=benchmark::data -P _/cmake/script/GenerateHeaderFromRaw.cmake
>
> ```
>
>
>
> Previously it took only a few seconds.
What's your system?
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2346084391)
> Not sure why, but for me the command took almost an hour:
>
>
>
>
>
> ```
>
> $ ps -p 816447 --format time,rss,cmd
>
> TIME RSS CMD
>
> 00:51:12 117640 /usr/bin/cmake -DRAW_SOURCE_PATH=_/src/bench/data/block413567.raw -DHEADER_PATH=_/bld-cmake/src/bench/data/block413567.raw.h -DRAW_NAMESPACE=benchmark::data -P _/cmake/script/GenerateHeaderFromRaw.cmake
>
> ```
>
>
>
> Previously it took only a few seconds.
What's your system?
💬 maflcko commented on issue "GUI event loop should be block free":
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2346089167)
Ok, and Android support was blocked on cmake or qt6, or was it both? (Sorry for using this thread, but I couldn't find the tracking issue).
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2346089167)
Ok, and Android support was blocked on cmake or qt6, or was it both? (Sorry for using this thread, but I couldn't find the tracking issue).
💬 fjahr commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2346092294)
post-merge utACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2346092294)
post-merge utACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac