🤔 janb84 reviewed a pull request: "doc: move release notes to wiki pre branch off"
(https://github.com/bitcoin/bitcoin/pull/33339#pullrequestreview-3196259513)
ACK 905c1a77f51c0b1b0d1880ad381d75194c52a12f
All cross-referenced 👍 (have added some PR numbers to the wiki)
(https://github.com/bitcoin/bitcoin/pull/33339#pullrequestreview-3196259513)
ACK 905c1a77f51c0b1b0d1880ad381d75194c52a12f
All cross-referenced 👍 (have added some PR numbers to the wiki)
👍 willcl-ark approved a pull request: "doc: move release notes to wiki pre branch off"
(https://github.com/bitcoin/bitcoin/pull/33339#pullrequestreview-3196263483)
ACK 905c1a77f51c0b1b0d1880ad381d75194c52a12f
(https://github.com/bitcoin/bitcoin/pull/33339#pullrequestreview-3196263483)
ACK 905c1a77f51c0b1b0d1880ad381d75194c52a12f
🤔 hodlinator reviewed a pull request: "index: store per-block transaction locations for efficient lookups"
(https://github.com/bitcoin/bitcoin/pull/32541#pullrequestreview-3189055673)
Higher level flow:
```mermaid
sequenceDiagram
actor User
participant C as Electrum<br/>Client
participant S as Electrum<br/>Server
participant R as bitcoind<br/>REST interface
participant L as LocationsIndex
participant B as blockstorage
User->>+C: View wallet history
C->>+S: blockchain.scripthash.get_history(scripthash) [1]
note over C,S: https://electrum-protocol.readthedocs.io<br/>/en/latest/protocol-methods.html#blockchain-scripthash-get-his
...
(https://github.com/bitcoin/bitcoin/pull/32541#pullrequestreview-3189055673)
Higher level flow:
```mermaid
sequenceDiagram
actor User
participant C as Electrum<br/>Client
participant S as Electrum<br/>Server
participant R as bitcoind<br/>REST interface
participant L as LocationsIndex
participant B as blockstorage
User->>+C: View wallet history
C->>+S: blockchain.scripthash.get_history(scripthash) [1]
note over C,S: https://electrum-protocol.readthedocs.io<br/>/en/latest/protocol-methods.html#blockchain-scripthash-get-his
...
💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2329705352)
nit: If you re-touch, could be more explicit with `block_hash` + curly initialization by default to avoid unintentional narrowing:
```suggestion
uint256 block_hash;
uint32_t row; // allow splitting one block's transactions into multiple DB rows
explicit DBKey(const uint256& block_hash_in, uint32_t row_in) : block_hash{block_hash_in}, row{row_in} {}
SERIALIZE_METHODS(DBKey, obj)
{
uint8_t prefix{DB_BLOCK_HASH};
READWRITE(prefix);
if (prefix
...
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2329705352)
nit: If you re-touch, could be more explicit with `block_hash` + curly initialization by default to avoid unintentional narrowing:
```suggestion
uint256 block_hash;
uint32_t row; // allow splitting one block's transactions into multiple DB rows
explicit DBKey(const uint256& block_hash_in, uint32_t row_in) : block_hash{block_hash_in}, row{row_in} {}
SERIALIZE_METHODS(DBKey, obj)
{
uint8_t prefix{DB_BLOCK_HASH};
READWRITE(prefix);
if (prefix
...
💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2324878782)
info: Re-parsing for every transaction in every block seems wasteful, but I couldn't measure any improvement from doing:
```diff
diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py
index 34ae549cf4..89969e50f9 100755
--- a/test/functional/interface_rest.py
+++ b/test/functional/interface_rest.py
@@ -476,6 +476,7 @@ class RESTTest (BitcoinTestFramework):
self.wait_until(lambda: self.nodes[0].getindexinfo()['locationsindex'] == expected_index_info)
...
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2324878782)
info: Re-parsing for every transaction in every block seems wasteful, but I couldn't measure any improvement from doing:
```diff
diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py
index 34ae549cf4..89969e50f9 100755
--- a/test/functional/interface_rest.py
+++ b/test/functional/interface_rest.py
@@ -476,6 +476,7 @@ class RESTTest (BitcoinTestFramework):
self.wait_until(lambda: self.nodes[0].getindexinfo()['locationsindex'] == expected_index_info)
...
🚀 fanquake merged a pull request: "doc: move release notes to wiki pre branch off"
(https://github.com/bitcoin/bitcoin/pull/33339)
(https://github.com/bitcoin/bitcoin/pull/33339)
💬 Sjors commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-3266159876)
Rebased after #33283.
(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.
(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
...
(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
(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
(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?
(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.
(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
```
(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.
(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)
(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
(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.
(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
(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
(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.
(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.