🤔 glozow reviewed a pull request: "test: fix anti-fee-sniping off-by-one error"
(https://github.com/bitcoin/bitcoin/pull/33118#pullrequestreview-3081219214)
utACK e07e2532b4d7b4a549722e56cb2913a6081ccbaf

💬 hebasto commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3146719966)
Apparently, after the failed run:
```
$ rm -rf /home/hebasto/.bitcoin/regtest/blocks/index
$ ./build/bin/bitcoind -regtest -daemon
Bitcoin Core starting
$ tail -6 /home/hebasto/.bitcoin/regtest/debug.log
2025-08-02T15:02:00Z : Error initializing block database.
Please restart with -reindex or -reindex-chainstate to recover.
2025-08-02T15:02:00Z Shutdown: In progress...
2025-08-02T15:02:00Z scheduler thread exit
2025-08-02T15:02:00Z Flushed fee estimates to fee_estimates.dat.
2025-08-02T15:02:00Z
...

💬 jesterhodl commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2249392438)
transifex updated

💬 hebasto commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3146725108)
@theStack

Can you confirm the bug?

🤔 hodlinator reviewed a pull request: "index: store per-block transaction locations for efficient lookups"
(https://github.com/bitcoin/bitcoin/pull/32541#pullrequestreview-3081181690)
Concept ACK 1b928f58fc78f4727cef988902075abc05c372b2

Cool way of compressing transaction metadata to significantly reduce on-disk footprint compared to `TxIndex` and make it more likely this one fits in RAM. (Fine as long as external code can fetch by block hash + tx index instead of by txid).

Optimization as I understand it: All transactions within the same block have the same `FlatFilePos` base, which is compressed by a factor of 128 thanks to `TXS_PER_ROW`. Instead of storing an individ
...

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249347002)
Unused variable. Was this supposed to be used to skip forward in the stream instead of deserializing the header?

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249356202)
nit:
```suggestion
return RESTERR(req, HTTP_NOT_FOUND, strprintf("Failed to read transaction #%d from block %s", index, block_hash->ToString()));
```

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249355515)
We can skip this work if `LocationsIndex` is enabled as it doesn't use `block_pos`.

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249370232)
nit: Might be less confusing to use "row" everywhere here instead of "part"?

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249371490)
nit: Should use snake_case naming convention as per developer-notes.md, `n_tx`/`num_tx`/`tx_count`?

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249348139)
Would prefer it be moved to function-local scope so we don't add any startup time for those who never enable -locationsindex or use the REST interface.

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249374257)
nit: Unused header.

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249370654)
nit: Could use local variable with descriptive name rather than mutating in-param:
```suggestion
const auto column = i % TXS_PER_ROW; // index within a single DB row
```

💬 hodlinator commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249349229)
nit:
* Improved order?
* Clarify that wasted work is being done.
```suggestion
By default, this endpoint will also deserialize the leading transactions, before reading and returning the requested one.
```

💬 luke-jr commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2249411442)
same

💬 luke-jr commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2249411415)
This should come (ultimately) from /CMakeLists.txt, not have to be re-specified here

💬 luke-jr commented on pull request "Adding alert for failure to prevent dead-end user crash":
(https://github.com/bitcoin/bitcoin/pull/33127#discussion_r2249412471)
Adding command line options is non-trivial for GUI users. We should use the existing "data corrupt" message/prompt, so they can click a button to reindex.

💬 luke-jr commented on pull request "Adding alert for failure to prevent dead-end user crash":
(https://github.com/bitcoin/bitcoin/pull/33127#discussion_r2249412210)
Should probably just replace the assert

💬 Ataraxia009 commented on pull request "Adding alert for failure to prevent dead-end user crash":
(https://github.com/bitcoin/bitcoin/pull/33127#discussion_r2249504606)
The alert doesn't terminate the app. We need the assert there for termination.

💬 Ataraxia009 commented on pull request "Adding alert for failure to prevent dead-end user crash":
(https://github.com/bitcoin/bitcoin/pull/33127#discussion_r2249504661)
The alert doesn't terminate the app. We need the assert there for termination.