💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411934291)
Updated, thanks for testing!
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411934291)
Updated, thanks for testing!
💬 plebhash commented on issue "[`v30.0rc3`]`bitcoin-node` aborts with mining IPC interface usage":
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378729024)
also worth mentioning that the steps listed above are not able to reproduce it deterministically
sometimes it happens, sometimes it doesn't happen
probably related to the order in which things are dropped from memory, which I assume `capnp` and `capnp-rpc` crates take as triggers for IPC signals for resource cleanup on the server side
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378729024)
also worth mentioning that the steps listed above are not able to reproduce it deterministically
sometimes it happens, sometimes it doesn't happen
probably related to the order in which things are dropped from memory, which I assume `capnp` and `capnp-rpc` crates take as triggers for IPC signals for resource cleanup on the server side
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411949367)
I don't see how memory ordering solves that? Consider this exact line where the `Submit` thread is suspended:
```
auto Submit(T task) -> std::future<decltype(task())>
{
if (m_workers.empty() || m_interrupt.load()) throw std::runtime_error("No active workers; cannot accept new tasks");
---> Stop() is called right here on another thread, setting m_interrupt to true and all threads exit before this thread continues execution
using TaskType = std::packaged_task<
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411949367)
I don't see how memory ordering solves that? Consider this exact line where the `Submit` thread is suspended:
```
auto Submit(T task) -> std::future<decltype(task())>
{
if (m_workers.empty() || m_interrupt.load()) throw std::runtime_error("No active workers; cannot accept new tasks");
---> Stop() is called right here on another thread, setting m_interrupt to true and all threads exit before this thread continues execution
using TaskType = std::packaged_task<
...
💬 achow101 commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3378806623)
ACK 652424ad162b63d73ecb6bd65bd26946e90c617f
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3378806623)
ACK 652424ad162b63d73ecb6bd65bd26946e90c617f
🚀 achow101 merged a pull request: "Bump SCRIPT_VERIFY flags to 64 bit"
(https://github.com/bitcoin/bitcoin/pull/32998)
(https://github.com/bitcoin/bitcoin/pull/32998)
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2412093040)
> If all m_interrupt loads were relaxed, Submit() could read a stale false value even after Stop() set it to true, right?
I don't think it could? Once a write to true becomes visible to a thread, it cannot be read again as false until something writes false to it again. This is independent of memory ordering. Relaxed memory ordering can reorder when writes to different data become visible to a thread. For instance:
```
atomic<bool> x{false}, y{false};
// thread 1 stores true to x first the
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2412093040)
> If all m_interrupt loads were relaxed, Submit() could read a stale false value even after Stop() set it to true, right?
I don't think it could? Once a write to true becomes visible to a thread, it cannot be read again as false until something writes false to it again. This is independent of memory ordering. Relaxed memory ordering can reorder when writes to different data become visible to a thread. For instance:
```
atomic<bool> x{false}, y{false};
// thread 1 stores true to x first the
...
💬 l0rinc commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3378978210)
I'm on it, it seems related to the discussion in the original PR which already had some Windows build problems: https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1745687847
I have reproduced the error in https://github.com/l0rinc/bitcoin-core-nightly/pull/2 with @hebasto's nightly clone redirected to `l0rinc/master`.
I pushed a potential fix that adds a `const char *` overload to `HasReason` in https://github.com/l0rinc/bitcoin/pull/43 - tested through https://github.com/l0rinc/bit
...
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3378978210)
I'm on it, it seems related to the discussion in the original PR which already had some Windows build problems: https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1745687847
I have reproduced the error in https://github.com/l0rinc/bitcoin-core-nightly/pull/2 with @hebasto's nightly clone redirected to `l0rinc/master`.
I pushed a potential fix that adds a `const char *` overload to `HasReason` in https://github.com/l0rinc/bitcoin/pull/43 - tested through https://github.com/l0rinc/bit
...
💬 hebasto commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3379021879)
I've submitted an [issue](https://github.com/mstorsjo/llvm-mingw/issues/522) to https://github.com/mstorsjo/llvm-mingw.
@l0rinc
> I have reproduced the error in [l0rinc/bitcoin-core-nightly#2](https://github.com/l0rinc/bitcoin-core-nightly/pull/2) with @hebasto's nightly clone redirected to `l0rinc/master`.
>
> I pushed a potential fix that adds a `const char *` overload to `HasReason` in [l0rinc#43](https://github.com/l0rinc/bitcoin/pull/43) - tested through [l0rinc/bitcoin-core-night
...
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3379021879)
I've submitted an [issue](https://github.com/mstorsjo/llvm-mingw/issues/522) to https://github.com/mstorsjo/llvm-mingw.
@l0rinc
> I have reproduced the error in [l0rinc/bitcoin-core-nightly#2](https://github.com/l0rinc/bitcoin-core-nightly/pull/2) with @hebasto's nightly clone redirected to `l0rinc/master`.
>
> I pushed a potential fix that adds a `const char *` overload to `HasReason` in [l0rinc#43](https://github.com/l0rinc/bitcoin/pull/43) - tested through [l0rinc/bitcoin-core-night
...
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2412161898)
Thinking about this more, since `m_workers` is not synchronized it implies `Stop` and `Submit` must be called on the same thread. Therefore `m_workers.empty()` is sufficient to test if the pool is stopped since `Stop()` must complete before a subsequent `Submit()`. So `m_interrupt` is redundant in `if (m_workers.empty() || m_interrupt.load())`, and `m_interrupt` can just be made a regular bool guarded by `m_mutex`. `Start` would have call it with the mutex locked, but that is infrequent compared
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2412161898)
Thinking about this more, since `m_workers` is not synchronized it implies `Stop` and `Submit` must be called on the same thread. Therefore `m_workers.empty()` is sufficient to test if the pool is stopped since `Stop()` must complete before a subsequent `Submit()`. So `m_interrupt` is redundant in `if (m_workers.empty() || m_interrupt.load())`, and `m_interrupt` can just be made a regular bool guarded by `m_mutex`. `Start` would have call it with the mutex locked, but that is infrequent compared
...
✅ hebasto closed an issue: "ci: remove third-party javascript usage from Windows CI"
(https://github.com/bitcoin/bitcoin/issues/32508)
(https://github.com/bitcoin/bitcoin/issues/32508)
🚀 hebasto merged a pull request: "ci: remove 3rd party js from windows dll gha job"
(https://github.com/bitcoin/bitcoin/pull/32513)
(https://github.com/bitcoin/bitcoin/pull/32513)
💬 achow101 commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3379064266)
ACK 93a70a42d30fa2f9404b76d5bbdb5ea316fc1032
```
ff3ab842fd36d8c51e76fa87d80e460e3a26fca8641543f01be1b7b62eea99d8 guix-build-93a70a42d30f/output/aarch64-linux-gnu/SHA256SUMS.part
d8e022a8f8794df818cf682e75da7ca2f633e5954cd306cf14cd06f62bc3405d guix-build-93a70a42d30f/output/aarch64-linux-gnu/bitcoin-93a70a42d30f-aarch64-linux-gnu-debug.tar.gz
c7ade0382153220a35b2d59a04f4c6578e40957898c468e0040555c49508039d guix-build-93a70a42d30f/output/aarch64-linux-gnu/bitcoin-93a70a42d30f-aarch64-lin
...
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3379064266)
ACK 93a70a42d30fa2f9404b76d5bbdb5ea316fc1032
```
ff3ab842fd36d8c51e76fa87d80e460e3a26fca8641543f01be1b7b62eea99d8 guix-build-93a70a42d30f/output/aarch64-linux-gnu/SHA256SUMS.part
d8e022a8f8794df818cf682e75da7ca2f633e5954cd306cf14cd06f62bc3405d guix-build-93a70a42d30f/output/aarch64-linux-gnu/bitcoin-93a70a42d30f-aarch64-linux-gnu-debug.tar.gz
c7ade0382153220a35b2d59a04f4c6578e40957898c468e0040555c49508039d guix-build-93a70a42d30f/output/aarch64-linux-gnu/bitcoin-93a70a42d30f-aarch64-lin
...
🚀 achow101 merged a pull request: "depends: Update URL for `qrencode` package source tarball"
(https://github.com/bitcoin/bitcoin/pull/33494)
(https://github.com/bitcoin/bitcoin/pull/33494)
💬 ryanofsky commented on issue "[`v30.0rc3`]`bitcoin-node` aborts with mining IPC interface usage":
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3379176513)
Thanks @plebhash. The stack trace from https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378637474 shows the problem pretty clearly, and I created an issue to track it: https://github.com/bitcoin-core/libmultiprocess/issues/226.
It looks to me like there's not way great way for the rust client to avoid this problem, and this is something `bitcoin-node` should be changed to fix by (1) fixing the abort described in https://github.com/bitcoin-core/libmultiprocess/issues/226 and (2) imp
...
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3379176513)
Thanks @plebhash. The stack trace from https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378637474 shows the problem pretty clearly, and I created an issue to track it: https://github.com/bitcoin-core/libmultiprocess/issues/226.
It looks to me like there's not way great way for the rust client to avoid this problem, and this is something `bitcoin-node` should be changed to fix by (1) fixing the abort described in https://github.com/bitcoin-core/libmultiprocess/issues/226 and (2) imp
...
📝 l0rinc opened a pull request: "refactor: replace `const char*` exceptions with `std::runtime_error`"
(https://github.com/bitcoin/bitcoin/pull/33569)
Related to https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3378978210
Fixes MinGW/libc++ test failures where `BOOST_CHECK_EXCEPTION` with `const char*` exceptions failed due to platform-specific exception handling differences.
Following [C++ Core Guidelines E.15](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e15-throw-by-value-catch-exceptions-from-a-hierarchy-by-reference) "Throw by value, catch exceptions from a hierarchy by reference", throw `std::runtime_error
...
(https://github.com/bitcoin/bitcoin/pull/33569)
Related to https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3378978210
Fixes MinGW/libc++ test failures where `BOOST_CHECK_EXCEPTION` with `const char*` exceptions failed due to platform-specific exception handling differences.
Following [C++ Core Guidelines E.15](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e15-throw-by-value-catch-exceptions-from-a-hierarchy-by-reference) "Throw by value, catch exceptions from a hierarchy by reference", throw `std::runtime_error
...
💬 l0rinc commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3379213071)
> I'd lean to the second option you mentioned
Thanks, closed the other ones and opened https://github.com/bitcoin/bitcoin/pull/33569 - the dedicated nightly build passed: https://github.com/l0rinc/bitcoin-core-nightly/pull/3
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3379213071)
> I'd lean to the second option you mentioned
Thanks, closed the other ones and opened https://github.com/bitcoin/bitcoin/pull/33569 - the dedicated nightly build passed: https://github.com/l0rinc/bitcoin-core-nightly/pull/3
📝 l0rinc opened a pull request: "randomenv: Fix MinGW dllimport warning for `environ`"
(https://github.com/bitcoin/bitcoin/pull/33570)
Related to https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3378978210
Extends 7703884a to guard environ declaration on all Windows builds, not just MSVC.
MinGW also defines environ as a macro that expands to a dllimport function, causing the same inconsistent linkage warning.
Use WIN32 instead of _MSC_VER to match the platform-specific guards already used throughout the file.
----
The error was reproduced by adding a temporary [nightly build](https://github.com/l0rinc/bit
...
(https://github.com/bitcoin/bitcoin/pull/33570)
Related to https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3378978210
Extends 7703884a to guard environ declaration on all Windows builds, not just MSVC.
MinGW also defines environ as a macro that expands to a dllimport function, causing the same inconsistent linkage warning.
Use WIN32 instead of _MSC_VER to match the platform-specific guards already used throughout the file.
----
The error was reproduced by adding a temporary [nightly build](https://github.com/l0rinc/bit
...
💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#issuecomment-3379397164)
Added an AssumeUTXO fuzz test here that was requested in https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2404014654 - but it needs the fixes here to work.
(https://github.com/bitcoin/bitcoin/pull/32313#issuecomment-3379397164)
Added an AssumeUTXO fuzz test here that was requested in https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2404014654 - but it needs the fixes here to work.
💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2412387188)
Update: had to remove the fuzz test, the accounting is broken here, but it's fixed in another PR, see: https://github.com/bitcoin/bitcoin/pull/32313#issuecomment-3379397164
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2412387188)
Update: had to remove the fuzz test, the accounting is broken here, but it's fixed in another PR, see: https://github.com/bitcoin/bitcoin/pull/32313#issuecomment-3379397164
💬 Sjors commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2412728343)
You only need to add it as a remote once, but each subsequent update requires a fetch. I'll drop `--fetch` from the first command though.
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2412728343)
You only need to add it as a remote once, but each subsequent update requires a fetch. I'll drop `--fetch` from the first command though.