Skip to content

Issues related to block invalidation / re-validation #1033

Open
@ImplOfAnImpl

Description

@ImplOfAnImpl
  • If the invalidation is too deep, we should give up adding transactions back into the mempool.

  • Invalidated blocks won't be deleted immediately. We'll need to prune them later once they reach max reorg depth. (*)

  • Blocks should not be invalidated on any error. We'll need a way of distinguishing "soft" errors (such as a DB failure) from "hard" ones (when the block is definitely incorrect). Addressed in the PR Fixes related to block invalidation #1685

  • Failure flags inside BlockStatus can be reset, so that the block can be re-validated. But the re-validation will likely fail because the block data (i.e. the block itself) will be missing in the DB, either because it has been pruned or because it has never been stored there in the first place. So, we'll need a mechanism for chainstate to inform the p2p subsystem that certain blocks must be re-downloaded.
    Update: alternatively, we can just remove the previously invalid block indices instead of resetting their statuses. (*)
    Addressed in Fixes related to block invalidation #1685

  • Block invalidation and failure flag resetting use ChainstateRef::get_block_ids_by_height which can be potentially very slow (if the chain gets large enough), because it iterates over all block indices in the DB. We should optimize it or use something else instead.

(*) Certain parts of the codebase may not expect a previously existing block or block index to disappear. E.g. block sync manager in P2p doesn't currently expect this when sending blocks (see TODOs in its peer.rs). But this might be true for other components as well, so this can be a dangerous change.

(edit from gitlab): (*) At this moment the P2p component expects that blocks and block indices that once existed in the chainstate remain there forever. If this is not true, it might panic or ban the peer in certain circumstances, see the corresponding comments in the code (search the code for "#1033")
It's not an issue in p2p, provided that we only delete indices corresponding to missing blocks.
And it shouldn't be an issue for other components either.

Metadata

Metadata

Assignees

No one assigned

    Labels

    ConsensusConsensus related issuesenhancementNew feature or requestp2pp2p related issues

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions