Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graceful degradation for pruned and txindex-less nodes #279

Merged
merged 11 commits into from
Feb 25, 2021

Conversation

shesek
Copy link
Contributor

@shesek shesek commented Feb 4, 2021

This is an attempt to enable as much of the functionality as possible for pruned nodes and for nodes without txindex.

txindex related changes:

  • Fetch block transactions with getrawtransaction <txid> <blockhash>. This works without txindex.
  • Tolerate missing previous output information and render transactions without it.
  • Support /tx/<txid>?height=<height> and link to it from block pages.

Pruning related changes:

  • Fallback to getblockheader instead of getblock for pruned blocks. The txid list and weight/size information will be missing.
  • Tolerate missing block stats.
  • Extract the Bitcoin whitepaper from the UTXO set.

Tested functionality:

  • Homepage: Fully functional (recent blocks are unpruned, extracting the coinbase tx no longer depends on txindex)
  • Blocks list: Functional except the "Avg Fee", "Fees" and "% Full" columns, which are unavailable for pruned blocks
  • Single block page:
    Without txindex: Functional except not showing previous output information (address/amount) and the mining fee.
    With pruning: Shows only the summary for pruned blocks, without the tx list.
  • Single transaction page: Only works for non-pruned txs, when the height query string parameter is specified. Does not show previous outputs info. Edit: Also works with wallet and recently confirmed transactions.
  • Mining Summary: Works for recent unpruned blocks, shows (very) partial information for pruned blocks
  • Block Stats/Analysis: Does not work for pruned blocks
  • Difficulty History: Fully functional

@shesek
Copy link
Contributor Author

shesek commented Feb 4, 2021

I left some if (true) in a bunch of places to have saner diffs, I can remove them and re-indent the code once its reviewed.

@shesek
Copy link
Contributor Author

shesek commented Feb 4, 2021

Hmm, I just realized that 4aafc4e would prevent querying for mempool transactions... I guess I could just remove that commit?

Edit: fixed by 445fb06

`txindex` related changes:

- Fetch block transactions with `getrawtransaction <txid> <blockhash>`. This works without `txindex`.

- Tolerate missing previous output information and render transactions without it.

Pruning related changes:

- Fallback to `getblockheader` instead of `getblock` for pruned blocks. The txid list and weight/size information will be missing.

- Tolerate missing block stats.
i.e. `/tx/<txid>?height=<height>`. This can work without txindex.

When txindex is disabled, the block page links to the transactions with
this parameter set.
…mempool

But still avoid unnecessarily loading prevouts
Also:
- Use nicer-looking /tx/<txid>@<height> URLs
- Simplify by removing some unnecessary code
- Fix a bug where searching for invalid block height hanged
Also changed session.userMessage to render as markdown,
hopefully that's okay?
@shesek
Copy link
Contributor Author

shesek commented Feb 14, 2021

I rebased on master and added a few enhancements:

  • Try looking up txids in wallet transactions. This makes it possible to display wallet transactions even after the block containing them gets pruned.

  • Try looking up txids in recent blocks. This makes looking up for recently confirmed transactions by their txid work, where "recently" is determined by BTCEXP_NOTXINDEX_SEARCH_DEPTH.

  • Support <txid>@<height> in the search box.

  • A better error message explaining what lookups are possible without txindex.

  • Explain that the fee is unavailable instead of showing it as 0.

I would love some feedback @janoside, is this something you'd be willing to consider merging?

@janoside
Copy link
Owner

@shesek Feedback: I love this! This change is so well suited to the philosophy of this project and I'm excited to get it merged. It's just a matter of finding time to review and test. I'll try to prioritize this over the next few days. I don't currently run any pruning nodes so there's a bit more overhead than some other PRs, but I'll make it happen soon.

@Kixunil
Copy link
Contributor

Kixunil commented Feb 14, 2021

@shesek big thank you! I was actually thinking about this too but never found the time to improve it or open an issue. This will make btc-rpc-explorer more accessible to the users of my repository.

Using the information from `getmempooletry`.
@janoside
Copy link
Owner

@shesek Update - Reviewing/testing is marching forward. No major issues yet and I've covered a lot of this.

I have a branch feature-prune-support where I'm testing/tweaking. Hoping to wrap up by tomorrow.

@shesek
Copy link
Contributor Author

shesek commented Feb 19, 2021

@janoside The work on your branch looks great, thanks for pushing this forward!

@shesek
Copy link
Contributor Author

shesek commented Feb 19, 2021

I wrote a summary of the limited functionality for a project I'm working on that integrates btc-rpc-explorer, maybe useful to also mention this on (or link to this comment from) the readme?


If pruning is enabled or if txindex is disabled, some functionality will be limited:

  • You will only be able to search for mempool, recently confirmed and wallet transactions by their txid. Searching for non-wallet transactions that were confirmed over 3 blocks ago is only possible if you provide the confirmed block height in addition to the txid.
  • Pruned blocks will display basic header information, without the list of transactions. Transactions in pruned blocks will not be available, unless they're wallet-related. Block stats will only work for unpruned blocks.
  • The address and amount of previous transaction outputs will not be shown, only the txid:vout.
  • The mining fee will only be available for unconfirmed transactions.

@Kixunil
Copy link
Contributor

Kixunil commented Feb 19, 2021

Would it make sense to attempt to use a special blockchain.transaction.get_confirmed_blockhash method available in electrs as a fallback if txindex is not available?

I originally wanted to write an RPC proxy to do that (btc-rpc-explorer was actually my motivating case) but didn't have the time yet and I suspect it could be easy to add the feature here. It'd also make it easier for users who don't use automated systems (one less daemon).

This only works with Electrs and requires enabling BTCEXP_ELECTRUM_TXINDEX.

See: romanz/electrs@a0a3d4f
@shesek
Copy link
Contributor Author

shesek commented Feb 19, 2021

@Kixunil Your wish is granted: bea32a6 :)

This requires setting BTCEXP_ELECTRUM_TXINDEX, since most Electrum servers won't have it.

Kinda strange to have this as part of the electrumAddressApi, but I didn't want to start moving everything around for this on my own. What do you think @janoside?

@Kixunil
Copy link
Contributor

Kixunil commented Feb 19, 2021

@shesek awesome! That's the best kind of answer I could get. :D

@shesek
Copy link
Contributor Author

shesek commented Feb 21, 2021

I'm seeing this weird behavior where pruned and unpruned blocks are interleaved throughout the history. I confirmed that this is what gets reported by bitcoind. Any idea what could be causing this?

image

@janoside
Copy link
Owner

@shesek Hmm, interesting. I haven't encountered that, but reading the help info for getblockchaininfo and its pruneheight value I suppose I can imagine that blocks could be partially pruned? I was using the totalFees value to determine the point to insert the note which was obviously flimsy. This change is cleaner and should fix this, I think. Let me know if it doesn't.

@janoside
Copy link
Owner

After the changes I just pushed in the branch, I consider this PR feature complete. I'll be doing a bit more testing over the next couple of days. If anyone wants to review any of the recent changes or offer their own testing results that'd be great. After that, on to master.

@shesek
Copy link
Contributor Author

shesek commented Feb 24, 2021

I've been using your branch (minus the changes from today) and its been working great. Will update and test some more. Awesome work! Excited to get this released :-)

@janoside janoside merged commit 8604466 into janoside:master Feb 25, 2021
@shesek
Copy link
Contributor Author

shesek commented Feb 25, 2021

🚀

@shesek
Copy link
Contributor Author

shesek commented Mar 25, 2021

The project I was integrating btc-rpc-explorer into is eznode, which got released just now.

With it, you can get a pruned node and btc-rpc-explorer running using docker run -it -v ~/eznode:/data eznode/eznode. It also has support for a personal electrum server, specter, and several options for secure remote access.

@janoside
Copy link
Owner

@shesek That looks awesome. Congrats on the launch!

@Kixunil
Copy link
Contributor

Kixunil commented Mar 27, 2021

Oh, sorry @shesek I was overwhelmed when I got your reply and then forgot about it. It indeed looks great! Perhaps the only thing I'd like to see (in Bitcoin community in general) is avoiding the situation in which each different project has its own Bitcoind. In your case you have manual override, but I'd love it to not be manual. :)

@shesek
Copy link
Contributor Author

shesek commented Mar 27, 2021

@Kixunil How would you like this to work?

Right now you can use an existing bitcoind by starting docker with -v ~/.bitcoin:/bitcoin --add-host host.docker.internal:host-gateway if its a local one running on the same machine, or by setting BITCOIND_URL/BITCOIND_AUTH if its remote.

@Kixunil
Copy link
Contributor

Kixunil commented Mar 27, 2021

Didn't want to spam here, so I opened ez-org/eznode#8

Copy link

@0racl3z 0racl3z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You guys R.O.C.K!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants