💬 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.

💬 Ataraxia009 commented on pull request "Adding alert for failure to prevent dead-end user crash":
(https://github.com/bitcoin/bitcoin/pull/33127#discussion_r2249504887)
We need to terminate the app after the alert. Keeping it going to lead to unsafe behaviour given that the data is corrupt.

💬 Ataraxia009 commented on pull request "Adding alert for failure to prevent dead-end user crash":
(https://github.com/bitcoin/bitcoin/pull/33127#discussion_r2249505556)
I agree this is a bad experience for non technical users.

But it seems like this is the standard right now in the code, to give this kind of alert on the code base (ie tell the the user to reindex instead of a direct action button)

If there is somewhere that actually has a button that reindexes, show me, I'll use it. If not, adding this functionality is work for another pull request.

💬 Ataraxia009 commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2249554357)
Yeah thats better, is there a way to pipe it now? @luke-jr

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249608455)
You're right, thanks - removed the unneeded code and renamed `tip` to `block_index`.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249617802)
Removed.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249617820)
Good catch, thanks!

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249617848)
Thanks - renamed to `tx_count`.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249617854)
Thanks - fixed.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249617938)
Thanks - renamed to `row`.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249618007)
Thanks - fixed.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249618092)
Good catch - fixed.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249618203)
Removed.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249618256)
Thanks - fixed.