search

romanz / electrs

An efficient re-implementation of Electrum Server in Rust
MIT License
1.05k stars 387 forks source link

Feature: revert setting index_lookup_limit to 0 #681

Open vlad-tim opened 2 years ago

vlad-tim commented 2 years ago

Is your feature request related to a problem? Please describe. Hello, I recently discovered electrs, and started to use it as a back-end to BTC RPC Explorer. Opening an address in explorer with lots of transactions makes electrs completely stuck until restart. I couldn't figure out why it's happening, and then, after spending some time digging into the code and documentation I realized the existence of index_lookup_limit parameter, and setting this parameter to something like 200 protects the server from running expensive queries that nuke it.

Describe the solution you'd like Change the default limit of index_lookup_limit to some finite value. Revert https://github.com/romanz/electrs/commit/e5047a0b106090313030e599f69924b206789876

I think that reverting index_lookup_limit to a fixed value is a more reasonable default for future new users who are not yet familiar with configuration options. My argument is that, like my anecdote above suggests, it might not obvious a person unfamiliar with electrs configuration options to figure out why their server hangs (if index_lookup_limit is unlimited by default). On the other hand, if there was a limit, the current error message like this ">200 index entries, query may take too long" would make it clear to the user about the existence of such limit and its current value. After reading this message they would be able to figure out how to increase the limit if necessary.

Additional context I no longer have a problem, it's just a suggestion how to save time of future users by setting a more reasonable default value.

Kixunil commented 2 years ago

Error message explaining the problem is a good argument. I think pagination is even better. Also I wonder if documenting this is enough.

vlad-tim commented 2 years ago

My two cents about the pagination here is that Electrum RPC protocol does not seem to define it, which means that clients like BTC RPC Explorer might not be able to work with it.

Kixunil commented 2 years ago

Yeah, it was proposed for new protocol version but there is little activity. :(