Speeding up getrawtransaction when txindex=1

[Pantamis]

The txindex of Core stores the exact location of a transaction in the blockfiles. This allows to retrieve the full transaction from its transaction ID in one short disk read with getrawtransaction. This RPC call also allows to search for a transaction in a block by specifying the blockhash of where it should be as argument, txindex enable or not. That's how electrs is able to work without txindex: it stores the blockheight of any transaction in its index (and the txid is the key).

But there is an issue: if the blockhash is given, Core will read the whole block and search the transaction inside, no mater if txindex is enable or not !!

This means that giving the blockhash to getrawtransaction in fact slower transaction retrieval if txndex is enable ! Indeed, the txindex is used only if the transaction wasn't found in the block so adding the blockhash will result in at least one full block disk read (much longer !). This was pointed in bitcoin/bitcoin#22382 and recently solved by bitcoin/bitcoin#22383, merged two weeks ago, well after feature freeze.

This means the issue is still present in Core 22.0 release and will ultimately be solved in 23.0 release. In the meantime, each time Electrs retrieves a ransaction in a block, it suffers from lower perf because it always fills the blockhash argument of getrawtransaction so Core doesn't use txindex, this increases time to retrieves a transaction by a factor of 5 in average.

It could be nice to detect if txindex=1 in Core or add a config flag so that when set ElectRS always call getrawtransaction without adding the blockhash, to benefit from the higher perfs with txindex. This would be extremly beneficial to current branch. (less for p2p branch because it indexes blockheight, so the whole block are always read no matter what) Once 23.0 is out, this flag can be removed.

The other solution is to implement that in rust-bitcoincore-rpc instead...

[Kixunil]

detect if txindex=1 in Core or add a config flag

Please detect - much more reliable than people setting flags. I'd also wait with removing it to not create dependency hell.

[Pantamis]

The RPC command getindexinfo "txindex" returns nothing if txindex=0 and

[
    {
        "txindex": {
            "synced": true,
            "best_block_height": ...
        }
    }
]

if txindex=1.

But this call is not supported in rust-bitcoincore-rpc lib. Maybe I should first open a PR there first ?

[Kixunil]

We should only enable it if synced. If we want to be perfectionists we could enable it based on best_block_height if we know the height of the hash we have. I'm fine with avoiding this complication though - it's mostly edge-case anyway.

But this call is not supported in rust-bitcoincore-rpc lib. Maybe I should first open a PR there first ?

We do not require direct support because there's generic call() method but it's definitely nicer to have support there. So yes, make a PR and we will see how fast it gets merged.

[Pantamis]

I started working on it in #452, should be quick to review

This PR could be used for a proposal in rust-bitcoincore-rpc (I had to define an IndexInfo type)

[romanz]

Let's wait for next Bitcoin Core release :) https://github.com/romanz/electrs/pull/452#issuecomment-922250599