💬 pythonruss commented on pull request "[30.x] Finalise v30.0":
(https://github.com/bitcoin/bitcoin/pull/33559#issuecomment-3378624908)
I would strongly suggest we back out of the PRs widening the op_return size. 20% of the network has switched to an alternative implementation just to avoid likely consequences here of having an vulnerable surface area where malicious actors can put files with illegal contents.
(https://github.com/bitcoin/bitcoin/pull/33559#issuecomment-3378624908)
I would strongly suggest we back out of the PRs widening the op_return size. 20% of the network has switched to an alternative implementation just to avoid likely consequences here of having an vulnerable surface area where malicious actors can put files with illegal contents.
💬 plebhash commented on issue "[`v30.0rc3`]`bitcoin-node` aborts with mining IPC interface usage":
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378637474)
I updated the code with the advice above and `bitcoin-node` no longer crashes during normal operation.
But if I kill my rust code, it still causes a crash on `bitcoin-node`... I wonder if I'm still doing something wrong on shutdown?
steps to reproduce:
- build `bitcoin-node` from https://github.com/bitcoin/bitcoin/pull/33519
- clone https://github.com/plebhash/sv2-bitcoin-core
- check out `2025-10-06-abort-proxy-io` branch
- launch `bitcoin-node` with `-ipc-bind=unix` (connected to `testnet4`)
...
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378637474)
I updated the code with the advice above and `bitcoin-node` no longer crashes during normal operation.
But if I kill my rust code, it still causes a crash on `bitcoin-node`... I wonder if I'm still doing something wrong on shutdown?
steps to reproduce:
- build `bitcoin-node` from https://github.com/bitcoin/bitcoin/pull/33519
- clone https://github.com/plebhash/sv2-bitcoin-core
- check out `2025-10-06-abort-proxy-io` branch
- launch `bitcoin-node` with `-ipc-bind=unix` (connected to `testnet4`)
...
💬 achow101 commented on pull request "Improve LastCommonAncestor performance + add tests":
(https://github.com/bitcoin/bitcoin/pull/33515#issuecomment-3378659388)
ACK 3635d62f5a935801e26a0d5fa2cb5e2dbbb42f9b
(https://github.com/bitcoin/bitcoin/pull/33515#issuecomment-3378659388)
ACK 3635d62f5a935801e26a0d5fa2cb5e2dbbb42f9b
🚀 achow101 merged a pull request: "Improve LastCommonAncestor performance + add tests"
(https://github.com/bitcoin/bitcoin/pull/33515)
(https://github.com/bitcoin/bitcoin/pull/33515)
💬 ryanofsky commented on issue "[`v30.0rc3`]`bitcoin-node` aborts with mining IPC interface usage":
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378693317)
> But if I kill my rust code, it still causes a crash on `bitcoin-node`... I wonder if I'm still doing something wrong on shutdown?
This seems like a new crash I haven't seen before. Probably there is something the rust code could be doing differently to prevent the crash, but that wouldn't mean it is doing anything wrong, since the node should be able to avoid crashing on unclean shutdowns. Will look into this more and try to suggest changes and fix.
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378693317)
> But if I kill my rust code, it still causes a crash on `bitcoin-node`... I wonder if I'm still doing something wrong on shutdown?
This seems like a new crash I haven't seen before. Probably there is something the rust code could be doing differently to prevent the crash, but that wouldn't mean it is doing anything wrong, since the node should be able to avoid crashing on unclean shutdowns. Will look into this more and try to suggest changes and fix.
💬 hsdredgun commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-3378693508)
Concept NACK
As a small node operator running LND for Lightning Network, this change is catastrophic.
L1 Fee Market Crisis:
Bitcoin is peer-to-peer electronic cash. By allowing 4MB OP_RETURN outputs, we're letting JPEGs and arbitrary data compete directly with legitimate payments for block space. When fees spike from data storage, Lightning operators get crushed:
Channel opens become unaffordable
Force-closes miss deadlines
Justice transactions can't confirm in time
Small routing nodes
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-3378693508)
Concept NACK
As a small node operator running LND for Lightning Network, this change is catastrophic.
L1 Fee Market Crisis:
Bitcoin is peer-to-peer electronic cash. By allowing 4MB OP_RETURN outputs, we're letting JPEGs and arbitrary data compete directly with legitimate payments for block space. When fees spike from data storage, Lightning operators get crushed:
Channel opens become unaffordable
Force-closes miss deadlines
Justice transactions can't confirm in time
Small routing nodes
...
💬 axelpale commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3378700462)
We know how it must feel **frustrating** to delay all the great hard work done by the developers towards v30. There must be a lot of good stuff in v30.0. **However**, the community seems to be deeply divided on philosophical issues and risks associated with the purpose-built ability to attach relatively large chunks of unencrypted data to transactions. The fear is that some extremely **disgusting data** end up in the chain forever for everybody to read. If it is disgusting enough, every news out
...
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3378700462)
We know how it must feel **frustrating** to delay all the great hard work done by the developers towards v30. There must be a lot of good stuff in v30.0. **However**, the community seems to be deeply divided on philosophical issues and risks associated with the purpose-built ability to attach relatively large chunks of unencrypted data to transactions. The fear is that some extremely **disgusting data** end up in the chain forever for everybody to read. If it is disgusting enough, every news out
...
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411917478)
> > going down that path would mean guarding all m_interrupt accesses with m_mutex too
>
> Hmm I don't see why we would need to do that? If we wanted to make m_interrupt a non-atomic bool we would need to do that. But if it's atomic we can just use relaxed since it's just a flag and not acting as a fence to any other non-atomic memory?
Hmm, but what about the flag updates visibility?
If all `m_interrupt` loads were relaxed, `Submit()` could read a stale false value even after `Stop()` s
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411917478)
> > going down that path would mean guarding all m_interrupt accesses with m_mutex too
>
> Hmm I don't see why we would need to do that? If we wanted to make m_interrupt a non-atomic bool we would need to do that. But if it's atomic we can just use relaxed since it's just a flag and not acting as a fence to any other non-atomic memory?
Hmm, but what about the flag updates visibility?
If all `m_interrupt` loads were relaxed, `Submit()` could read a stale false value even after `Stop()` s
...
💬 plebhash commented on issue "[`v30.0rc3`]`bitcoin-node` aborts with mining IPC interface usage":
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378715206)
to be fair, the shutdown strategy on the rust code doesn't really do anything to try to be "clean"
when `ctrl+c` is hit, we simply break the loops of every spawned task and drop everything from memory (including records to whatever `waitNext` request that was still waiting for completion)
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3378715206)
to be fair, the shutdown strategy on the rust code doesn't really do anything to try to be "clean"
when `ctrl+c` is hit, we simply break the loops of every spawned task and drop everything from memory (including records to whatever `waitNext` request that was still waiting for completion)
💬 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)