Skip to content

Make AVM implement block.HeightIndexedChainVM #1699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jul 13, 2023
Merged

Make AVM implement block.HeightIndexedChainVM #1699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4bc89d1
push
dhrubabasu Jul 5, 2023
efd4dff
Merge branch 'dev' into avm-standardize-getblockidatheight
dhrubabasu Jul 5, 2023
495b7c1
wip
dhrubabasu Jul 7, 2023
a79b86b
nit
dhrubabasu Jul 7, 2023
e6e77a6
Merge branch 'dev' into avm-standardize-getblockidatheight
dhrubabasu Jul 7, 2023
55f7278
wip
dhrubabasu Jul 11, 2023
fd95c16
nits
dhrubabasu Jul 11, 2023
7478712
Merge branch 'dev' into avm-standardize-getblockidatheight
dhrubabasu Jul 11, 2023
041e891
fix interface
StephenButtolph Jul 12, 2023
7b1ef34
revert metervm change
StephenButtolph Jul 12, 2023
9d3ed63
Update chains/manager.go
dhrubabasu Jul 13, 2023
d70a4fc
Merge branch 'dev' into avm-standardize-getblockidatheight
StephenButtolph Jul 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions chains/linearizable_vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import (
)

var (
_ vertex.LinearizableVM = (*initializeOnLinearizeVM)(nil)
_ block.ChainVM = (*linearizeOnInitializeVM)(nil)
_ vertex.LinearizableVM = (*initializeOnLinearizeVM)(nil)
_ block.ChainVM = (*linearizeOnInitializeVM)(nil)
_ block.HeightIndexedChainVM = (*linearizeOnInitializeVM)(nil)
)

// initializeOnLinearizeVM transforms the consensus engine's call to Linearize
Expand Down Expand Up @@ -62,9 +63,18 @@ func (vm *initializeOnLinearizeVM) Linearize(ctx context.Context, stopVertexID i
// channel to the VM that is being linearized.
type linearizeOnInitializeVM struct {
vertex.LinearizableVMWithEngine
hVM block.HeightIndexedChainVM
stopVertexID ids.ID
}

func NewLinearizeOnInitializeVM(vm vertex.LinearizableVMWithEngine) *linearizeOnInitializeVM {
hVM, _ := vm.(block.HeightIndexedChainVM)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this ever not be ok?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would just be nil then and we handle that below...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in our usage but it could not be okay by somebody implementing a linearizable vm (definitely not advised haha).

return &linearizeOnInitializeVM{
LinearizableVMWithEngine: vm,
hVM: hVM,
}
}

func (vm *linearizeOnInitializeVM) Initialize(
ctx context.Context,
_ *snow.Context,
Expand All @@ -78,3 +88,19 @@ func (vm *linearizeOnInitializeVM) Initialize(
) error {
return vm.Linearize(ctx, vm.stopVertexID, toEngine)
}

func (vm *linearizeOnInitializeVM) VerifyHeightIndex(ctx context.Context) error {
if vm.hVM == nil {
return block.ErrHeightIndexedVMNotImplemented
}

return vm.hVM.VerifyHeightIndex(ctx)
}

func (vm *linearizeOnInitializeVM) GetBlockIDAtHeight(ctx context.Context, height uint64) (ids.ID, error) {
if vm.hVM == nil {
return ids.Empty, block.ErrHeightIndexedVMNotImplemented
}

return vm.hVM.GetBlockIDAtHeight(ctx, height)
}
18 changes: 9 additions & 9 deletions chains/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,19 +683,20 @@ func (m *manager) createAvalancheChain(
return nil, fmt.Errorf("error while fetching chain config: %w", err)
}

dagVM := vm
if m.MeterVMEnabled {
vm = metervm.NewVertexVM(vm)
dagVM = metervm.NewVertexVM(dagVM)
}
if m.TracingEnabled {
vm = tracedvm.NewVertexVM(vm, m.Tracer)
dagVM = tracedvm.NewVertexVM(dagVM, m.Tracer)
}

// Handles serialization/deserialization of vertices and also the
// persistence of vertices
vtxManager := state.NewSerializer(
state.SerializerConfig{
ChainID: ctx.ChainID,
VM: vm,
VM: dagVM,
DB: vertexDB,
Log: ctx.Log,
CortinaTime: version.GetCortinaTime(ctx.NetworkID),
Expand Down Expand Up @@ -726,7 +727,7 @@ func (m *manager) createAvalancheChain(
// snowmanMessageSender here is where the metrics will be placed. Because we
// end up using this sender after the linearization, we pass in
// snowmanMessageSender here.
err = vm.Initialize(
err = dagVM.Initialize(
context.TODO(),
ctx.Context,
vmDBManager,
Expand Down Expand Up @@ -754,9 +755,8 @@ func (m *manager) createAvalancheChain(

chainAlias := m.PrimaryAliasOrDefault(ctx.ChainID)

untracedVMWrappedInsideProposerVM := &linearizeOnInitializeVM{
LinearizableVMWithEngine: vm,
}
// Note: this does not use [dagVM] to ensure we use the [vm]'s height index.
untracedVMWrappedInsideProposerVM := NewLinearizeOnInitializeVM(vm)

var vmWrappedInsideProposerVM block.ChainVM = untracedVMWrappedInsideProposerVM
if m.TracingEnabled {
Expand Down Expand Up @@ -784,7 +784,7 @@ func (m *manager) createAvalancheChain(
// Note: linearizableVM is the VM that the Avalanche engines should be
// using.
linearizableVM := &initializeOnLinearizeVM{
DAGVM: vm,
DAGVM: dagVM,
vmToInitialize: vmWrappingProposerVM,
vmToLinearize: untracedVMWrappedInsideProposerVM,

Expand Down Expand Up @@ -972,7 +972,7 @@ func (m *manager) createAvalancheChain(
return &chain{
Name: chainAlias,
Context: ctx,
VM: vm,
VM: dagVM,
Handler: h,
}, nil
}
Expand Down
2 changes: 1 addition & 1 deletion vms/avm/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (s *Service) GetBlockByHeight(_ *http.Request, args *api.GetBlockByHeightAr
}
reply.Encoding = args.Encoding

blockID, err := s.vm.state.GetBlockID(uint64(args.Height))
blockID, err := s.vm.state.GetBlockIDAtHeight(uint64(args.Height))
if err != nil {
return fmt.Errorf("couldn't get block at height %d: %w", args.Height, err)
}
Expand Down
12 changes: 6 additions & 6 deletions vms/avm/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2150,7 +2150,7 @@ func TestServiceGetBlockByHeight(t *testing.T) {
name: "block height not found",
serviceAndExpectedBlockFunc: func(_ *testing.T, ctrl *gomock.Controller) (*Service, interface{}) {
state := states.NewMockState(ctrl)
state.EXPECT().GetBlockID(blockHeight).Return(ids.Empty, database.ErrNotFound)
state.EXPECT().GetBlockIDAtHeight(blockHeight).Return(ids.Empty, database.ErrNotFound)

manager := executor.NewMockManager(ctrl)
return &Service{
Expand All @@ -2170,7 +2170,7 @@ func TestServiceGetBlockByHeight(t *testing.T) {
name: "block not found",
serviceAndExpectedBlockFunc: func(_ *testing.T, ctrl *gomock.Controller) (*Service, interface{}) {
state := states.NewMockState(ctrl)
state.EXPECT().GetBlockID(blockHeight).Return(blockID, nil)
state.EXPECT().GetBlockIDAtHeight(blockHeight).Return(blockID, nil)

manager := executor.NewMockManager(ctrl)
manager.EXPECT().GetStatelessBlock(blockID).Return(nil, database.ErrNotFound)
Expand All @@ -2195,7 +2195,7 @@ func TestServiceGetBlockByHeight(t *testing.T) {
block.EXPECT().Txs().Return(nil)

state := states.NewMockState(ctrl)
state.EXPECT().GetBlockID(blockHeight).Return(blockID, nil)
state.EXPECT().GetBlockIDAtHeight(blockHeight).Return(blockID, nil)

manager := executor.NewMockManager(ctrl)
manager.EXPECT().GetStatelessBlock(blockID).Return(block, nil)
Expand All @@ -2220,7 +2220,7 @@ func TestServiceGetBlockByHeight(t *testing.T) {
block.EXPECT().Bytes().Return(blockBytes)

state := states.NewMockState(ctrl)
state.EXPECT().GetBlockID(blockHeight).Return(blockID, nil)
state.EXPECT().GetBlockIDAtHeight(blockHeight).Return(blockID, nil)

expected, err := formatting.Encode(formatting.Hex, blockBytes)
require.NoError(t, err)
Expand Down Expand Up @@ -2248,7 +2248,7 @@ func TestServiceGetBlockByHeight(t *testing.T) {
block.EXPECT().Bytes().Return(blockBytes)

state := states.NewMockState(ctrl)
state.EXPECT().GetBlockID(blockHeight).Return(blockID, nil)
state.EXPECT().GetBlockIDAtHeight(blockHeight).Return(blockID, nil)

expected, err := formatting.Encode(formatting.HexC, blockBytes)
require.NoError(t, err)
Expand Down Expand Up @@ -2276,7 +2276,7 @@ func TestServiceGetBlockByHeight(t *testing.T) {
block.EXPECT().Bytes().Return(blockBytes)

state := states.NewMockState(ctrl)
state.EXPECT().GetBlockID(blockHeight).Return(blockID, nil)
state.EXPECT().GetBlockIDAtHeight(blockHeight).Return(blockID, nil)

expected, err := formatting.Encode(formatting.HexNC, blockBytes)
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions vms/avm/states/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (d *diff) AddTx(tx *txs.Tx) {
d.addedTxs[tx.ID()] = tx
}

func (d *diff) GetBlockID(height uint64) (ids.ID, error) {
func (d *diff) GetBlockIDAtHeight(height uint64) (ids.ID, error) {
if blkID, exists := d.addedBlockIDs[height]; exists {
return blkID, nil
}
Expand All @@ -109,7 +109,7 @@ func (d *diff) GetBlockID(height uint64) (ids.ID, error) {
if !ok {
return ids.Empty, fmt.Errorf("%w: %s", ErrMissingParentState, d.parentID)
}
return parentState.GetBlockID(height)
return parentState.GetBlockIDAtHeight(height)
}

func (d *diff) GetBlock(blkID ids.ID) (blocks.Block, error) {
Expand Down
36 changes: 18 additions & 18 deletions vms/avm/states/mock_states.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vms/avm/states/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ type ReadOnlyChain interface {
avax.UTXOGetter

GetTx(txID ids.ID) (*txs.Tx, error)
GetBlockID(height uint64) (ids.ID, error)
GetBlockIDAtHeight(height uint64) (ids.ID, error)
GetBlock(blkID ids.ID) (blocks.Block, error)
GetLastAccepted() ids.ID
GetTimestamp() time.Time
Expand Down Expand Up @@ -382,7 +382,7 @@ func (s *state) AddTx(tx *txs.Tx) {
s.addedTxs[txID] = tx
}

func (s *state) GetBlockID(height uint64) (ids.ID, error) {
func (s *state) GetBlockIDAtHeight(height uint64) (ids.ID, error) {
if blkID, exists := s.addedBlockIDs[height]; exists {
return blkID, nil
}
Expand Down
10 changes: 5 additions & 5 deletions vms/avm/states/state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func ChainTxTest(t *testing.T, c Chain) {
func ChainBlockTest(t *testing.T, c Chain) {
require := require.New(t)

fetchedBlkID, err := c.GetBlockID(populatedBlkHeight)
fetchedBlkID, err := c.GetBlockIDAtHeight(populatedBlkHeight)
require.NoError(err)
require.Equal(populatedBlkID, fetchedBlkID)

Expand All @@ -226,7 +226,7 @@ func ChainBlockTest(t *testing.T, c Chain) {
require.Equal(populatedBlk.ID(), fetchedBlk.ID())

// Pull again for the cached path
fetchedBlkID, err = c.GetBlockID(populatedBlkHeight)
fetchedBlkID, err = c.GetBlockIDAtHeight(populatedBlkHeight)
require.NoError(err)
require.Equal(populatedBlkID, fetchedBlkID)

Expand All @@ -253,22 +253,22 @@ func ChainBlockTest(t *testing.T, c Chain) {
blkID := blk.ID()
blkHeight := blk.Height()

_, err = c.GetBlockID(blkHeight)
_, err = c.GetBlockIDAtHeight(blkHeight)
require.ErrorIs(err, database.ErrNotFound)

_, err = c.GetBlock(blkID)
require.ErrorIs(err, database.ErrNotFound)

// Pull again for the cached path
_, err = c.GetBlockID(blkHeight)
_, err = c.GetBlockIDAtHeight(blkHeight)
require.ErrorIs(err, database.ErrNotFound)

_, err = c.GetBlock(blkID)
require.ErrorIs(err, database.ErrNotFound)

c.AddBlock(blk)

fetchedBlkID, err = c.GetBlockID(blkHeight)
fetchedBlkID, err = c.GetBlockIDAtHeight(blkHeight)
require.NoError(err)
require.Equal(blkID, fetchedBlkID)

Expand Down
10 changes: 10 additions & 0 deletions vms/avm/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/ava-labs/avalanchego/snow/consensus/snowstorm"
"github.com/ava-labs/avalanchego/snow/engine/avalanche/vertex"
"github.com/ava-labs/avalanchego/snow/engine/common"
"github.com/ava-labs/avalanchego/snow/engine/snowman/block"
"github.com/ava-labs/avalanchego/utils/json"
"github.com/ava-labs/avalanchego/utils/linkedhashmap"
"github.com/ava-labs/avalanchego/utils/set"
Expand Down Expand Up @@ -62,6 +63,7 @@ var (
errBootstrapping = errors.New("chain is currently bootstrapping")

_ vertex.LinearizableVMWithEngine = (*VM)(nil)
_ block.HeightIndexedChainVM = (*VM)(nil)
)

type VM struct {
Expand Down Expand Up @@ -387,6 +389,14 @@ func (vm *VM) LastAccepted(context.Context) (ids.ID, error) {
return vm.chainManager.LastAccepted(), nil
}

func (vm *VM) GetBlockIDAtHeight(_ context.Context, height uint64) (ids.ID, error) {
return vm.state.GetBlockIDAtHeight(height)
}

func (*VM) VerifyHeightIndex(context.Context) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is never called because it is wrapped by the HeightIndexedVM?

(if so, may make sense to clarify that)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@patrick-ogrady patrick-ogrady Jul 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it always have it if it is performed async (and could be interrupted)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return nil
}

/*
******************************************************************************
*********************************** DAG VM ***********************************
Expand Down