ava-labs / avalanchego

Go implementation of an Avalanche node.
https://avax.network
BSD 3-Clause "New" or "Revised" License
2.09k stars 646 forks source link

Reduce usage of `getBlock` in consensus #3151

Open StephenButtolph opened 2 days ago

StephenButtolph commented 2 days ago

Why this should be merged

getBlock can result in DB reads due to it's usage of the VM.GetBlock function. If the same behavior can be performed with in-memory checks rather than with using VM.GetBlock, we should.

This also improves TestGetProcessingAncestor.

How this works

From the VM.GetBlock specification:

// Attempt to load a block.
//
// If the block does not exist, database.ErrNotFound should be returned.
//
// It is expected that blocks that have been successfully verified should be
// returned correctly. It is also expected that blocks that have been
// accepted by the consensus engine should be able to be fetched. It is not
// required for blocks that have been rejected by the consensus engine to be
// able to be fetched.

So, we can only rely on VM.GetBlock to return Accepted and Verified but not yet Decided blocks.

Additionally, pending contains the set of blocks that are awaiting issuance due to a transitive network request. Meaning that there is a block that is not locally known between a pending block and a Processing or Decided block.

  1. In Start, getBlock can be removed to explicitly use VM.GetBlock, as pending and nonVerifiedCache are both empty.
  2. In getProcessingAncestor there are 3 cases that we need to consider: a. VM.GetBlock would have returned a Verified but not yet Decided block. This case can never occur because, we already know that the provided block is not processing. b. VM.GetBlock would have previously returned an Accepted block. If this was the case, then we would have immediately dropped it in the next check. So dropping the vote early results in the same behavior. c. pending contained the block that we are looking for. In this case, we may have been able to continue looking for a Processing ancestor. However, we know that we would have never found such an ancestor, because if we were able to locally iterate and find a processing ancestor, then this block shouldn't be pending, it should have been attempted to be issued. Additionally, we know that initialVote has already been either Fulfilled or Abandoned, so we aren't running into a situation where the pending block is currently in the process of being issued.

How this was tested