Make AVM implement block.HeightIndexedChainVM
#1699
Conversation
dhrubabasu
requested review from
danlaine,
StephenButtolph and
coffeeavax
as code owners
dhrubabasu
force-pushed
the
avm-standardize-getblockidatheight
branch
from
7558d33 to
4bc89d1
Compare
dhrubabasu
changed the title
GetBlockID -> GetBlockIDAtHeightblock.HeightIndexedChainVM
StephenButtolph
dismissed stale reviews from coffeeavax, abi87, and darioush
stale
| stopVertexID ids.ID | ||
| } | ||
|
|
||
| func NewLinearizeOnInitializeVM(vm vertex.LinearizableVMWithEngine) *linearizeOnInitializeVM { | ||
| hVM, _ := vm.(block.HeightIndexedChainVM) |
There was a problem hiding this comment.
Can this ever not be ok?
There was a problem hiding this comment.
I guess it would just be nil then and we handle that below...
There was a problem hiding this comment.
Not in our usage but it could not be okay by somebody implementing a linearizable vm (definitely not advised haha).
| return vm.state.GetBlockIDAtHeight(height) | ||
| } | ||
|
|
||
| func (*VM) VerifyHeightIndex(context.Context) error { |
There was a problem hiding this comment.
This is never called because it is wrapped by the HeightIndexedVM?
(if so, may make sense to clarify that)
There was a problem hiding this comment.
The vm itself implements HeightIndexedVM, it doesn't wrap it. This function is called here when we're generating the proposervm height index.
Reason why it's nil is because the AVM has always had a height index so we don't need to verify it.
There was a problem hiding this comment.
How does it always have it if it is performed async (and could be interrupted)?
There was a problem hiding this comment.
The AVM height index is not performed async. Since we linearized, each block was included in the height index on accept.
This is different than the P-chain where there is a period of time where both the inner vm and the proposervm height indexes are incomplete so we cannot simply return nil there.
Signed-off-by: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com>
Why this should be merged
We use both
GetBlockIDandGetBlockIDAtHeightin different places in the repo. This standardizes all of these calls toGetBlockIDAtHeight.Additionally, implements
block.HeightIndexedChainVMon the AVM to kick off the X-chain proposervm height index (https://github.com/ava-labs/avalanchego/blob/master/vms/proposervm/vm.go#L341)How this works
Find + replace + implement interface
How this was tested
CI