lbryio / lbrycrd

The blockchain that provides the digital content namespace for the LBRY protocol
https://lbry.com
MIT License
2.57k stars 178 forks source link

Block decrement can put claimtrie in erroneous state #243

Closed kaykurokawa closed 5 years ago

kaykurokawa commented 5 years ago

Issue found by @BrannonKing

When there is a claim/support in the queue (has not been activated yet, and has activation height X), it can be put into the claimtrie at some height Y less than X if a takeover is triggered (some other claim wins the name, or winning claim abandoned). This condition shall be called "early activation".

If an active block contains a claim with "early activation" and was decremented, the undo information for that claim with early activation was incorrectly applied. If correctly applied, the claim would go back into the queue with its original activation height X. There are also two queues, one that is indexed on "name" and has the activation height as one of its value, and one that is indexed on "activation height".

However, for a claim we would incorrectly put the claim back into the queue. In the "name" indexed queue we put it back with the early activation height Y and than in the "activation height" indexed queue, we put it back with the activation height X. This discrepancy would cause lbrycrdd to crash on the assert below once this claim became active again and it needed to be deleted from the queue

https://github.com/lbryio/lbrycrd/blob/e89d748b2ff4a846b1493d4ee6a9a98540ea306f/src/claimtrie.cpp#L2115

Thus if a block containing a claim with early activation was reorged, it will cause the node to eventually crash once the claim became activated again. A reindex should wipe out the incorrect state of the claimtrie, and should allow the node to continue (assuming a similar reorg does not happen again)

For a support, we would incorrectly put it back in both queues with the early activation height Y. Thus, it would incorrectly activate on the next increment. This may cause the node to get stuck as the claimtrie may end up in an incorrect state. Worse, this can be used to split consensus.

kaykurokawa commented 5 years ago

Fix here: https://github.com/lbryio/lbrycrd/pull/244