Fix address stats when reorg-ed and confirmed in different height

[junderw]

Fixes #11

---

Requires re-index? == No

Affects API interface AND/OR response data format? == No

---

Explanation:

TxHistoryRow is just a key in rocksdb. Which means it is encoding all its information into the key and (most likely intentionally) using the property that inserting the same key twice will de-duplicate to its advantage.

In the case where txA is confirmed in orphaned block at height 10, returned to our mempool, then confirmed in new-best-chain block 15 etc. The keys are not the same (since the key contains the block height).

Other endpoints use unique() on txid etc. so there shouldn't be any problems with the other endpoints... However, this can't be properly fixed without a re-working of the index (which will require a re-index of esplora)

For now, we avoid unique() since it would just give the same result, and instead use unique_by and return a tuple of the txid + the outpoint (for spends it's the previous outpoint, for funds it's this txid+this output index. If we don't include the txid(), then it would remove the spend of each fund->spend pair)

Note: utxo_delta does not have the same problem because it uses a HashMap with the Outpoint as the key to accumulate the results, removing any duplicates.

[junderw]

(Discussing permanent solutions to myself:)

We could move the block height to the value instead of the key, that's one permanent solution, basically when it is confirmed at the later block it will just update the block height essentially.

Actually, a lot of the history APIs rely on the height being there for ordering purposes.

[mononaut]

Tested @ 8ca766e

I confirmed that this fixes incorrect balances for the addresses identified in issue #11:

however bc1qm34lsc65zpw79lxes69zkqmk6ee3ewf0j77s3h still reports an incorrect on-chain balance (i.e. chain_stats.funded_txo_sum - chain_stats.spent_txo_sum) of 30476.68434636 vs 30081.72818834 according to other explorers.

I wonder if we're still counting transactions which appeared in a stale block, but never actually confirmed in the main chain (and so wouldn't have a duplicate row)?

[junderw]

Actually, looking at that address (bc1qm34lsc65zpw79lxes69zkqmk6ee3ewf0j77s3h) I'm not sure anymore...

It might be unrelated.

[junderw]

Even fixing the key structure won't fix the orphaned but never re-confirmed case...

I also just remembered that the height being in the index gives the history its order so we can't remove it from the key.

Fixing this problem long-term will require an overhaul in the way Esplora indexes things.

[junderw]

I wonder if we're still counting transactions which appeared in a stale block, but never actually confirmed in the main chain

We are storing them in the DB, but the line self.tx_confirming_block(&history.get_txid()) will return None if the block that confirmed the tx was orphaned. (I double checked the behavior of HeadersList and it is being updated properly.

The problem with the orphan-getting-re-confirmed-at-diff-height thing was that both of them would get the updated blockhash in TxConfRow, and therefore both return Some() for self.tx_confirming_block(&history.get_txid())

bc1qm34lsc65zpw79lxes69zkqmk6ee3ewf0j77s3h being off sounds more to do with the fact that it is a very active address that sends and receives 5~10 BTC a couple times per minute etc. and has tons of unconfirmed...

Figuring out the cause of that would be hard to pin down and hard to verify any fix. lol

[mononaut]

ah ok, that makes sense, thanks. I couldn't find any other addresses with the same issue, so perhaps I just need to re-index.

in any case, this PR does solve the original issue, so it's an ACK from me.

[shesek]

Thanks for hunting down the cause for this, @junderw :)

A different way to fix this is to ensure that the block where the txid confirmed matches the height specified by the history entry. This is more efficient as it doesn't require accumulating an in-memory list of all (txid,outpoint)s in the scripthash's history.

See Blockstream/electrs#61.