💬 hebasto commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756879893)
Thank you! Accepted.

💬 hebasto commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2346306703)
My Guix build:
```
aarch64
d4baa1d2e9ee7abeb60d115dd52c6bc7d02e8116fe4b94c0f040371321e37b80 guix-build-253a691441dd/output/aarch64-linux-gnu/SHA256SUMS.part
7c38410b9468799b74371ba195f34121a4789e917dcf611f2ccc4b81f8a63eee guix-build-253a691441dd/output/aarch64-linux-gnu/bitcoin-253a691441dd-aarch64-linux-gnu-debug.tar.gz
4a6ec8df80822e5938b1a55f2e08a945b12a9d4f8b2f1bad05ea8be90a3b6173 guix-build-253a691441dd/output/aarch64-linux-gnu/bitcoin-253a691441dd-aarch64-linux-gnu.tar.gz
48de6fea
...

💬 stickies-v commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#issuecomment-2346309176)
re-ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678
No changes except variable naming and small doc update.

💬 fanquake commented on issue "ci: failure in feature_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/issues/30878#issuecomment-2346309699)
Also seen here https://github.com/bitcoin/bitcoin/actions/runs/10828541638/job/30044139197#step:7:2918:
```bash
2024-09-12T13:21:45.762000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/Users/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-x86_64-apple-darwin/test/functional/feature_assumeutxo.py", line 738, in run_test

...

💬 fanquake commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2346312892)
Any chance this is now causing #30878? If so, would be good to fix before this is backported.

👍 hebasto approved a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2300357639)
ACK 253a691441ddb04eac2b318112064e69f4b837b7.

📝 hebasto converted_to_draft a pull request: "build: Improve `ccache` performance for different build directories"
(https://github.com/bitcoin/bitcoin/pull/30861)
This PR makes it possible to reuse the `ccache` cache populated during a build in another build directory:
```
$ cmake -B build_1 -DWITH_CCACHE=ON
$ cmake --build build_1 -t bitcoind
$ cmake -B build_2 -DWITH_CCACHE=ON
$ cmake --build build_2 -t bitcoind # 100% cache hit rate is expected.
```

The first commit has been picked from https://github.com/bitcoin/bitcoin/pull/30803.

Please refer to https://github.com/bitcoin/bitcoin/pull/20353 for historical context.

Addresses this [com
...

👍 hodlinator approved a pull request: "util: Use consteval checked format string in FatalErrorf, LogConnectFailure"
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2300417228)
ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678

`git range-diff master fae8c25 fa5bc45`

Only minor comment adjustments and clearer variable names since last reviewed commits.

Since GitHub is [troublesome](https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756561525), would prefer this change if you re-touch:
```diff
- PassFmt<129>("%129$s 999$s %2$s");
+ PassFmt<12>("%12$s 999$s %2$s");
```
Makes it more reasonable to add comparison to tinyformat in follow-up with 12 arg
...

👍 TheCharlatan approved a pull request: "code style: update .editorconfig file"
(https://github.com/bitcoin/bitcoin/pull/30877#pullrequestreview-2300418237)
ACK e118dde75ad3a8e08051b4189e1e75c172a98043

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2346382089)
macOS also seems to time out: https://github.com/bitcoin/bitcoin/actions/runs/10828462406/job/30043878794 on GHA

👍 maflcko approved a pull request: "code style: update .editorconfig file"
(https://github.com/bitcoin/bitcoin/pull/30877#pullrequestreview-2300423885)
I don't use it, so I don't care, but the changes look plausible

lgtm

💬 maflcko commented on pull request "code style: update .editorconfig file":
(https://github.com/bitcoin/bitcoin/pull/30877#discussion_r1756938195)
```suggestion
# .cirrus.yml, etc.
```

📝 willcl-ark opened a pull request: "test: re-bucket p2p_node_network_limited"
(https://github.com/bitcoin/bitcoin/pull/30879)
Locally these tests take about 20 seconds and appear in the correct bucket.

On CI runners they are taking upwards of 3 minutes which could be a contributing factor to some CI timeouts.

💬 willcl-ark commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2346397042)
I opened https://github.com/bitcoin/bitcoin/pull/30879 to re-bucket these tests which are taking a few minutes longer on CI than they are bucketed for. It's not much

👍 pablomartin4btc approved a pull request: "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD"
(https://github.com/bitcoin/bitcoin/pull/30807#pullrequestreview-2300431928)
tACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac

I was reviewing it before merge, went thru the `src` code change and the test, which are both very well documented and easy to follow.

I agree [with](https://github.com/bitcoin/bitcoin/pull/30807#discussion_r1750378079) the split of `feature_assumeutxo.py` for different scenarios required by a particular module/ area (p2p, wallet, etc.) having a base or common file too when possible.

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2346405443)
Yeah, I think we keep finding small mistakes, but there still must be a large underlying problem. In all the years, I've never seen the CI go from working fine to a large chunk of tasks timing out after 2(!) hours with no apparent reason, even across completely separate infrastructures (GHA, Cirrus workers)

💬 l0rinc commented on something "":
(https://github.com/bitcoin/bitcoin/commit/73fe7d723084653671f2178ea1177a8627edfafa#r146619899)
nit: this introduced a lint-spelling warning:
> % test/lint/lint-spelling.py
src/ipc/protocol.h:57: neccessary ==> necessary

💬 l0rinc commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1756962957)
nit: this introduced a lint-spelling warning:
> % test/lint/lint-spelling.py
src/ipc/protocol.h:57: neccessary ==> necessary

💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#issuecomment-2346432017)
ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678

The change progressed a lot from its first version and based on the enthusiasm among reviewers I'm looking forward to continuing these in follow-up PRs.

📝 fjahr opened a pull request: "test: Wait for local services to update in feature_assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/30880)
Closes #30878

It seems like there is a race where the block is stored locally and `getblock` does not error anymore, but `ActivateBestChain` has not finished yet, so the local services are not updated yet either. Fix this by waiting for the local services to update.