Bitcoin Core Github
44 subscribers
113K links
Download Telegram
💬 Sjors commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-3266159876)
Rebased after #33283.
👍 hodlinator approved a pull request: "common: Make arith_uint256 trivially copyable"
(https://github.com/bitcoin/bitcoin/pull/33332#pullrequestreview-3196404994)
ACK 6ffc6d1140a4660e3e2a8717128d567f5557e394

Verified through:
```
static_assert(std::is_trivially_copyable_v<arith_uint256>);
```
-> Only succeeds with PR.
⚠️ marcofleon opened an issue: "ipc: crash on `submitSolution` call with invalid coinbase"
(https://github.com/bitcoin/bitcoin/issues/33341)
There seems to be an unhandled exception when deserializing the coinbase in `submitSolution`.
<details>
<summary>log</summary>

```bash
node0 2025-09-08T11:26:56.165687Z [capnp-loop] [../../../src/ipc/capnp/protocol.cpp:35] [void ipc::capnp::(anonymous namespace)::IpcLogFn(bool, std::string)] [ipc] {bitcoin-node-166995/b-capnp-loop-166997} IPC server recv request #14 BlockTemplate.submitSolution$Params (context = (thread = <external capability>), version = 0, timestamp = 0, nonce = 0, coinbase
...
💬 fanquake commented on issue "ipc: crash on `submitSolution` call with invalid coinbase":
(https://github.com/bitcoin/bitcoin/issues/33341#issuecomment-3266183002)
@Sjors @ryanofsky
👍 TheCharlatan approved a pull request: "index: Fix coinstats overflow"
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3196423523)
ACK c76797481155754329ec6a6f58e8402569043944
💬 TheCharlatan commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2330168454)
Can't we just `const COutPoint& outpoint{tx->vin[j].prevout}` and similarly on line 443?
💬 Sjors commented on issue "ipc: crash on `submitSolution` call with invalid coinbase":
(https://github.com/bitcoin/bitcoin/issues/33341#issuecomment-3266231925)
Thanks, I was able to reproduce that. Will add a guard and test.
💬 Sjors commented on issue "ipc: crash on `submitSolution` call with invalid coinbase":
(https://github.com/bitcoin/bitcoin/issues/33341#issuecomment-3266255982)
@ryanofsky looks like libmultiprocess needs to catch this earlier, because even if I make `bool submitSolution` immediately return `false` it crashes.

The node crashes with:

```
libc++abi: terminating due to uncaught exception of type std::__1::ios_base::failure: SpanReader::read(): end of data: unspecified iostream_category error
```
💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2330214192)
Looks right to me, will use it if I have to retouch.
🚀 fanquake merged a pull request: "doc: fix `LIBRARY_PATH` comment"
(https://github.com/bitcoin/bitcoin/pull/33308)
👍 instagibbs approved a pull request: "net: check for empty header before calling FillBlock"
(https://github.com/bitcoin/bitcoin/pull/33296#pullrequestreview-3196566409)
ACK 2d31c5ae72940c594ec19d745bdbf09284c257d2
💬 TheCharlatan commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-3266420083)
Approach ACK on using siphash and `FlatFilePos`.

Do you want to squash the commits? This just feels like an improvement in every regard.
💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-3266487082)
`67bc008f62...0cd86a5c70`: rebase due to conflicts
💬 fjahr commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2330441916)
Done
💬 fjahr commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2330442142)
It doesn't modify the data but the storage on disk but since there are changes I think you are right and this shouldn't be `const`. I don't think it's `noexcept` because we are doing stuff on disk there are a lot issues that could come from that.
💬 fjahr commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2330442363)
I am pretty indifferent, so making it debug. I had to add a new category since none of the existing seemed to fit.
💬 fjahr commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2330442716)
As I and others have mentioned (see the linked PR), this doesn't work for coinstatsindex and we end up with hundreds of 55kb files that seem to stay around forever or until we trigger a full compaction manually.

Benchmarking didn't seem to make sense to me because the compaction completes almost infinitely so comparing two runs with compaction and without may not have given a result that gives an accurate answer. But I figured if I time the runtime of each compaction to the millisecond and th
...
💬 fjahr commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2330443068)
DOne
💬 ryanofsky commented on issue "ipc: crash on `submitSolution` call with invalid coinbase":
(https://github.com/bitcoin/bitcoin/issues/33341#issuecomment-3266653805)
Nice find. This behavior is expected but should be improved, and shouldn't be hard to improve.

It's similar to https://github.com/bitcoin-core/libmultiprocess/issues/206 in that if a bad request is sent to the node, the node will crash instead of returning an error, when it could just as easily return an error, and it would make sense to do that for the sake of stability and to make development easier with other languages like rust and python.

Similar to https://github.com/bitcoin-core/libmult
...
🤔 fjahr reviewed a pull request: "net: Add interrupt to pcp retry loop"
(https://github.com/bitcoin/bitcoin/pull/33338#pullrequestreview-3196901800)
utACK 188de70c86414b8b2ad5143f5c607b67686526ea

Looks good to me, happy to re-review if you decide to address the nits.
💬 fjahr commented on pull request "net: Add interrupt to pcp retry loop":
(https://github.com/bitcoin/bitcoin/pull/33338#discussion_r2330502356)
nit: Could add a simple test where the interrupt is instantly called to make sure the line is covered somehow.