💬 glozow commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411539360)
Alternatively, both classes could implement an `IsEmpty()` function.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411539360)
Alternatively, both classes could implement an `IsEmpty()` function.
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411666783)
We're in `Cluser` so `SetType` should be available here, so I think it's redundant
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411666783)
We're in `Cluser` so `SetType` should be available here, so I think it's redundant
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411670504)
I'm also fine with other ways of deduplication - or to leave as is.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2411670504)
I'm also fine with other ways of deduplication - or to leave as is.
💬 furszy commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2411670406)
No need to add the `--fetch` option. You added a `git fetch` below.
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2411670406)
No need to add the `--fetch` option. You added a `git fetch` below.
🤔 furszy reviewed a pull request: "doc: how to update a subtree"
(https://github.com/bitcoin/bitcoin/pull/33568#pullrequestreview-3311582322)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33568#pullrequestreview-3311582322)
Concept ACK
💬 hebasto commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3378434466)
> > Added to the nightly builds in [hebasto/bitcoin-core-nightly#74](https://github.com/hebasto/bitcoin-core-nightly/pull/74).
>
> Looks like there are failing unit tests (https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18313967330/job/52151177201#step:6:401):
>
> ```shell
> test\util_string_tests.cpp(43): Entering test case "ConstevalFormatString_NumSpec"
> test/util_string_tests.cpp(40): error: in "util_string_tests/ConstevalFormatString_NumSpec": exception "const char*" r
...
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3378434466)
> > Added to the nightly builds in [hebasto/bitcoin-core-nightly#74](https://github.com/hebasto/bitcoin-core-nightly/pull/74).
>
> Looks like there are failing unit tests (https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18313967330/job/52151177201#step:6:401):
>
> ```shell
> test\util_string_tests.cpp(43): Entering test case "ConstevalFormatString_NumSpec"
> test/util_string_tests.cpp(40): error: in "util_string_tests/ConstevalFormatString_NumSpec": exception "const char*" r
...
💬 achow101 commented on pull request "doc: Add `INSTALL.md` to Linux release tarballs":
(https://github.com/bitcoin/bitcoin/pull/33451#issuecomment-3378543092)
ACK 0fe7d552ab213065b8d5807c3dd9f4e976717529
```
55ee7f209c6cd5592a87472cacaf873e4a108f681465c238b66792a422306a15 guix-build-0fe7d552ab21/output/aarch64-linux-gnu/SHA256SUMS.part
eac376bc61d39740e37283df30c04ed330d1fe2ecc66ae2fb87e484c9f5589e1 guix-build-0fe7d552ab21/output/aarch64-linux-gnu/bitcoin-0fe7d552ab21-aarch64-linux-gnu-debug.tar.gz
7a31fb9bccf15dbb439f5a1600597188d8d97f4f0f9b94ccdf78bd1e90621421 guix-build-0fe7d552ab21/output/aarch64-linux-gnu/bitcoin-0fe7d552ab21-aarch64-lin
...
(https://github.com/bitcoin/bitcoin/pull/33451#issuecomment-3378543092)
ACK 0fe7d552ab213065b8d5807c3dd9f4e976717529
```
55ee7f209c6cd5592a87472cacaf873e4a108f681465c238b66792a422306a15 guix-build-0fe7d552ab21/output/aarch64-linux-gnu/SHA256SUMS.part
eac376bc61d39740e37283df30c04ed330d1fe2ecc66ae2fb87e484c9f5589e1 guix-build-0fe7d552ab21/output/aarch64-linux-gnu/bitcoin-0fe7d552ab21-aarch64-linux-gnu-debug.tar.gz
7a31fb9bccf15dbb439f5a1600597188d8d97f4f0f9b94ccdf78bd1e90621421 guix-build-0fe7d552ab21/output/aarch64-linux-gnu/bitcoin-0fe7d552ab21-aarch64-lin
...
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411796107)
> Any reason we can't use `memory_order_relaxed` on loads and stores of `m_interrupt`?
I recall briefly thinking about it, but going down that path would mean guarding all `m_interrupt` accesses with `m_mutex` too. And that seemed like an unnecessary overhead for methods like `Submit` that should be performing a lightweight interruption check before submission.
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411796107)
> Any reason we can't use `memory_order_relaxed` on loads and stores of `m_interrupt`?
I recall briefly thinking about it, but going down that path would mean guarding all `m_interrupt` accesses with `m_mutex` too. And that seemed like an unnecessary overhead for methods like `Submit` that should be performing a lightweight interruption check before submission.
💬 mzumsande commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411794397)
I tested this for signet on two computers, and while parallel sync (5 threads) was a great speedup on an SSD, I also observed a slowdown on a HDD compared to master (by a factor 2).
Presumably that's because reading the blocks from disk is the main bottleneck on a HDD, and with parallel indexing there is a lot of jumping back and forth, increasing seek time.
Should it be mentioned in the `-indexworkers` help that it is not advisable to use this option on a HDD?
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411794397)
I tested this for signet on two computers, and while parallel sync (5 threads) was a great speedup on an SSD, I also observed a slowdown on a HDD compared to master (by a factor 2).
Presumably that's because reading the blocks from disk is the main bottleneck on a HDD, and with parallel indexing there is a lot of jumping back and forth, increasing seek time.
Should it be mentioned in the `-indexworkers` help that it is not advisable to use this option on a HDD?
💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411822083)
> 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?
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411822083)
> 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?
💬 luke-jr commented on pull request "Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found":
(https://github.com/bitcoin/bitcoin/pull/33433#discussion_r2411833402)
>The whole test is only run when sum([self.options.run_ipv4, self.options.run_ipv6, self.options.run_nonloopback]) > 1.
No, that's a failure condition (multiple options enabled)
(https://github.com/bitcoin/bitcoin/pull/33433#discussion_r2411833402)
>The whole test is only run when sum([self.options.run_ipv4, self.options.run_ipv6, self.options.run_nonloopback]) > 1.
No, that's a failure condition (multiple options enabled)
💬 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)