Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 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 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.
💬 maflcko commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#issuecomment-2346436631)
re-review ACK 23eedc5d1e24b3d31da7902ece5182b9b24672d8

only change is a comment
💬 achow101 commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2346444421)
Discussion from today's IRC meeting is that we should move forward with @davidgumberg's patch and default to false on Windows for this release.