Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 hebasto approved a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305871368)
ACK c864a4c1940d682f7eb6fdb3b91b18d638b59330.
🚀 glozow merged a pull request: "net: support overriding the proxy selection in ConnectNode()"
(https://github.com/bitcoin/bitcoin/pull/33454)
🚀 glozow merged a pull request: "build: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489)
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2407510165)
If we `wait()` or `get()` on the returned future, does that imply a release/acquire memory ordering or a relaxed memory ordering? I can't seem to find out what this means for other non-atomic memory that was written on the worker thread before the task is completed.
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-3373034901)
`8725d10df5...dd32dfaaf3`: rebase due to conflicts and fix connman fuzz test.
📝 mdfaizanaquil opened a pull request: "Update README_windows.txt"
(https://github.com/bitcoin/bitcoin/pull/33552)
looks good now

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new
...
💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3373339683)
Rebased to resolve a conflict with the merged #33489.
fanquake closed a pull request: "Update README_windows.txt"
(https://github.com/bitcoin/bitcoin/pull/33552)
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408023767)
> If we `wait()` or `get()` on the returned future, does that imply a release/acquire memory ordering or a relaxed memory ordering? I can't seem to find out what this means for other non-atomic memory that was written on the worker thread before the task is completed.

For the returned value, `wait()`/`get()` provide release/acquire semantics. All updates performed by the task should be visible after `get()` returns.
Now, if the task modifies other objects that might be accessed concurrently
...
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408056506)
Sure, so I tried updating #31132 to use the thread pool here. It makes the change simpler if we were to already have this thread pool merged. However, the threads write to non-synchronized shared vectors, which the main thread will read. We need to make sure the write happens before the read. I initially stored the futures and wait on them like this https://github.com/andrewtoth/bitcoin/commit/c256f1b457cbd5b900aa34703eb5853d2449bcde#diff-3eff97d24109f473292b38a6495c5b51e8bda9f4bff9b691cf255968b
...
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408106215)
> All updates performed by the task should be visible after get() returns.

Hmm so in that case, then the diff I linked above is correct? The main thread is the one waiting on the future, and the non-synchronized vector is modified only by the worker thread. So the change to the vector should be visible on the main thread.
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408379260)
> Sure, so I tried updating #31132 to use the thread pool here. It makes the change simpler if we were to already have this thread pool merged. However, the threads write to non-synchronized shared vectors, which the main thread will read. We need to make sure the write happens before the read. I initially stored the futures and wait on them like this [andrewtoth@c256f1b#diff-3eff97d24109f473292b38a6495c5b51e8bda9f4bff9b691cf255968ba80d85dR151-R164](https://github.com/andrewtoth/bitcoin/commit/c
...
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408391339)
Nice!
🤔 hebasto reviewed a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3307332538)
> ... even though there is not currently a CI job testing Windows libc++ builds.

Add to the nightly builds in https://github.com/hebasto/bitcoin-core-nightly/pull/74.
👍 hodlinator approved a pull request: "Clear out space on GHA jobs"
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3307376880)
ACK c01214e72b9876c59924c98e96f43906f5df3353

I'm okay with applying broadly to ease maintenance, with a very slight preference for changing it back to more surgically apply to CentOS.
👍 hodlinator approved a pull request: "Modernize use of UTF-8 in Windows code"
(https://github.com/bitcoin/bitcoin/pull/32380#pullrequestreview-3307401839)
re-ACK 53e4951a5b5b9d166d278db4240513d09b447f58
📝 mzumsande opened a pull request: "validation: Improve warnings in case of chain corruption"
(https://github.com/bitcoin/bitcoin/pull/33553)
In case of corruption that leads to a block marked as invalid that is actually valid for the rest of the network, the user currently doesn't receive good error messages, but will get be stuck in an endless headers-sync loop with no explanation (#26931).

This PR improves warnings in two ways:
- When we receive a header that is already saved in our disk, but invalid, add a warning. This will happen repeatedly during the headerssync loop (see https://github.com/bitcoin/bitcoin/issues/26391#issu
...
💬 mzumsande commented on issue "HeadersSync tracking issue":
(https://github.com/bitcoin/bitcoin/issues/33547#issuecomment-3374358704)
I opened #33553 to address the problem from https://github.com/bitcoin/bitcoin/issues/26391 .
💬 mzumsande commented on issue "Stuck in Endless Pre-Syncing Headers Loop":
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-3374369293)
> I'll work on a PR suggesting to use this check in the pre-sync header loop situation

Forgot about this completely, but opened #33553 now to improve the logging (we'll still search for peers with headers we can use, but will now repeatedly log warning suggesting to reindex if the problem persists)
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2408782353)
Let me know how it goes :)