Bitcoin Core Github
42 subscribers
127K links
Download Telegram
💬 arejula27 commented on pull request "build: Fix source paths for debugging in CMake":
(https://github.com/bitcoin/bitcoin/pull/34081#issuecomment-3662249544)
Just tested what you mean, removed:
```
-try_append_cxx_flags("-fdebug-prefix-map=A=B" TARGET core_interface SKIP_LINK
- IF_CHECK_PASSED "-fdebug-prefix-map=${PROJECT_SOURCE_DIR}/src=."
-)
```
It had the same effect, yes, make more sense removing code that makes an incorrect behaviour than adding more above it. I will try to research if it has any advantages, but in other cas,e the correct approach should be to remove the incorrect mapping
💬 maflcko commented on pull request "build: Fix source paths for debugging in CMake":
(https://github.com/bitcoin/bitcoin/pull/34081#issuecomment-3662304656)
> Edit: @hebasto pointed out that removing it might resurface an issue (#30799.) I will try to look into it [#31957 (comment)](https://github.com/bitcoin/bitcoin/issues/31957#issuecomment-3567810672)

This may be a macro, and not the debug info, so may be fine, but I haven't checked this.
📝 theuni opened a pull request: "Add initial vectorized chacha20 implementation for 2-3x speedup"
(https://github.com/bitcoin/bitcoin/pull/34083)
Exploit modern simd to calculate 2/4/6/8/16 states at a time depending on the size of the input.

This demonstrates a 2x speedup on x86-64 and 3x for arm+neon. Platforms which require runtime detection (avx2/avx512) improve performance even further, and will come as a follow-up.

Rather than hand-writing assembly or using arch-specific intrinsics, this is written using compiler built-ins understood by gcc and clang.

In practice (at least on x86_64 and armv8), the compilers are able to pro
...
💬 theuni commented on pull request "Add initial vectorized chacha20 implementation for 2-3x speedup":
(https://github.com/bitcoin/bitcoin/pull/34083#issuecomment-3662325588)
Adding pings for a few people I've discussed this with: @sipa @ajtowns @l0rinc
🤔 ryanofsky reviewed a pull request: "qa: Avoid knock-on exception in assert_start_raises_init_error"
(https://github.com/bitcoin/bitcoin/pull/32929#pullrequestreview-3584856747)
Code review ACK 6b0eac750f3aecb29446d70c039559143e43a011. Since last review, just tweaking the last commit and adding two initial commits after some apparent conflicts with other PRs.

I do think this PR could benefit from being organized better and appearing to have more focus. I would definitely move the third commit d7ff1d0fa572ec36ff1eb10c0a1c04ef5e5be072 first since it's the main commit and probably the one most useful for other developers.

I also think it would be good to distinguish impr
...
💬 ryanofsky commented on pull request "qa: Avoid knock-on exception in assert_start_raises_init_error":
(https://github.com/bitcoin/bitcoin/pull/32929#discussion_r2624666464)
In commit "qa: Add test for init error timeout behavior" (c06a8ee18699800a0fa06898251ac7088ea8ef70)

I was pretty confused by this test because I thought it was supposed to be "waiting for init errors" but didn't see where there were any init errors. Would suggest replacing "waiting for init errors" with something like "waiting for init errors that do not occur" or something like that. Would be good to update this string, but also s/TestInitErrorTimeout/TestMissingInitErrorTimeout/ or similar.
💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3662401541)
I spun up a Warnet simulation with 100 nodes running this branch, along with a private, internal Tor DA. The scenarios fund all the nodes then randomly send transactions between them, choosing randomly to send with the wallet or private broadcast:

https://github.com/pinheadmz/private-broadcast-warnet

There are a few screenshots there showing the mechanism in action, and essentially stable memory usage over time even though the onion node count is wildly dynamic ;-)

Reviewers let me know
...
📝 maflcko opened a pull request: "scripted-diff: [doc] Unify stale copyright headers"
(https://github.com/bitcoin/bitcoin/pull/34084)
Historically, the upper year range in file headers was bumped manually
or with a script.

This has many issues:

* The script is causing churn. See for example commit 306ccd4, or
drive-by first-time contributions bumping them one-by-one. (A few from
this year: https://github.com/bitcoin/bitcoin/pull/32008,
https://github.com/bitcoin/bitcoin/pull/31642,
https://github.com/bitcoin/bitcoin/pull/32963, ...)
* Some, or likely most, upper year values were wrong. Reasons for
incor
...
📝 sipa opened a pull request: "cluster mempool: exploit SFL properties in txgraph"
(https://github.com/bitcoin/bitcoin/pull/34085)
Part of #30289, follow-up to #32545.

This gets rid of `FixLinearization()` by integrating the functionality into `Linearize()`, and makes txgraph exploit that (by delaying fixing of clusters until their first re-linearization). It also reduces (but does not eliminate) the number of calls to `PostLinearize`, as the SFL linearization effectively performs something very similar to postlinearization when loading in an existing linearization already.
💬 sipa commented on pull request "Optimized SFL cluster linearization":
(https://github.com/bitcoin/bitcoin/pull/34023#issuecomment-3662433317)
I have moved the changes to TxGraph here to a separate PR, #34085, as it's not actually dependent on the optimizations here.
💬 ryanofsky commented on pull request "depends: Do not consider `CC` environment variable for detecting system":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-3662618510)
Current version of this PR is ok but I do think fanquake's suggested change https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2079037412 passing along the requested compiler to `config.guess` would be an improvement.

I think his change had a typo because it suggests using `CC_FOR_BUILD=$CC` when this should probably be `CC_FOR_BUILD=$(build_CC)` (or `CC=$(build_CC)` which is [equivalent](https://github.com/bitcoin/bitcoin/blob/13891a8a685d255cb13dd5018e3d5ccc18b07c34/depends/config.g
...
👍 hebasto approved a pull request: "kernel: Remove non-kernel module includes"
(https://github.com/bitcoin/bitcoin/pull/34079#pullrequestreview-3585190785)
ACK 065639200505035428df01584ff43172c4b2dd90, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/34079#pullrequestreview-3582699579).
💬 maflcko commented on pull request "A few followups after introducing `/rest/blockpart/` endpoint":
(https://github.com/bitcoin/bitcoin/pull/34074#issuecomment-3662702660)
review ACK 584f0cbf7932b4e5cf629c90fc7453d94a890abe 🐟

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 584f0cbf7932
...
💬 boxtob commented on pull request "cmake: Add fail pattern to `try_append_cxx_flags`":
(https://github.com/bitcoin/bitcoin/pull/34047#discussion_r2624973413)
Technically it does negate the switch but the result/outcome is the same as already mentioned by @fanquake and @maflcko.

Expected behavior:
```
$ env CXX=clang++ CC=clang CXXFLAGS="-Wno-error=unknown-warning-option" cmake -B build 2>&1 | grep WREALLY_UNKNOWN_WARNING
-- Performing Test CXX_SUPPORTS__WREALLY_UNKNOWN_WARNING
-- Performing Test CXX_SUPPORTS__WREALLY_UNKNOWN_WARNING - Success
```

Non expected behavior:
```
$ env CXX=clang++ CC=clang CXXFLAGS="-Wno-error=unknown-warning-o
...
💬 davidgumberg commented on pull request "test: sock: Enable socket pair tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/33506#issuecomment-3662730391)
> > My gut reaction is that it would not be worth the extra code to do this, but I don't feel very strongly and am happy to add this if you think it's better.
>
> Not 100% sure. As long as this is only used for the tests, like now, i don't mind it being like this. But if it were to be used for anything in the actual code, i think a socket pair should be a socket pair. Using a fallback work-around for functionality that's plainly supported seems strange.

100% agree that if we were using `so
...
💬 l0rinc commented on pull request "A few followups after introducing `/rest/blockpart/` endpoint":
(https://github.com/bitcoin/bitcoin/pull/34074#discussion_r2625015165)
I think this was supposed to be a `BIN` to explain it's not a json failure
```suggestion
res = self.test_rest_request(f"/blockpart/{blockhash}", status=400, req_type=ReqType.BIN, ret_type=RetType.OBJ)
```
💬 romanz commented on pull request "A few followups after introducing `/rest/blockpart/` endpoint":
(https://github.com/bitcoin/bitcoin/pull/34074#discussion_r2625055321)
Updated 7fe94a0493
💬 l0rinc commented on pull request "A few followups after introducing `/rest/blockpart/` endpoint":
(https://github.com/bitcoin/bitcoin/pull/34074#issuecomment-3662873909)
ACK 59b93f11e8600d5224359f4d05619c0f56aef1e6

[Since last ack](https://github.com/bitcoin/bitcoin/compare/823d4d8a5b71b7c740a6552f72fbfdb9b2813ca6..59b93f11e8600d5224359f4d05619c0f56aef1e6) the reason was added for status failure and non-json test was added for missing offset
💬 achow101 commented on issue "sqlite legacy descriptor wallet migration fails":
(https://github.com/bitcoin/bitcoin/issues/33468#issuecomment-3663281115)
> it doesn't let me :(

Open the dump file, change the `bdb` at the top to `sqlite` and do `createfromdump`. You'll get a checksum error that gives you the correct checksum. Copy that checksum, open the dump file again, go to the bottom and replace the checksum. `createfromdump` should now work.