Skip to content

Commit

Permalink
vms/platformvm: Process atomicRequests and onAcceptFunc in opti…
Browse files Browse the repository at this point in the history
…on blocks (ava-labs#2459)

Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
dhrubabasu and StephenButtolph authored Dec 11, 2023
1 parent 2fd8931 commit f5266fb
Showing 5 changed files with 98 additions and 27 deletions.
19 changes: 17 additions & 2 deletions vms/platformvm/block/executor/acceptor.go
Original file line number Diff line number Diff line change
@@ -180,8 +180,23 @@ func (a *acceptor) optionBlock(b, parent block.Block, blockType string) error {
return err
}

if err := a.state.Commit(); err != nil {
return err
defer a.state.Abort()
batch, err := a.state.CommitBatch()
if err != nil {
return fmt.Errorf(
"failed to commit VM's database for block %s: %w",
blkID,
err,
)
}

// Note that this method writes [batch] to the database.
if err := a.ctx.SharedMemory.Apply(blkState.atomicRequests, batch); err != nil {
return fmt.Errorf("failed to apply vm's state to shared memory: %w", err)
}

if onAcceptFunc := blkState.onAcceptFunc; onAcceptFunc != nil {
onAcceptFunc()
}

a.ctx.Log.Trace(
68 changes: 53 additions & 15 deletions vms/platformvm/block/executor/acceptor_test.go
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ package executor

import (
"testing"
"time"

"github.com/stretchr/testify/require"

@@ -124,7 +125,7 @@ func TestAcceptorVisitAtomicBlock(t *testing.T) {
// Set [blk]'s state in the map as though it had been verified.
onAcceptState := state.NewMockDiff(ctrl)
childID := ids.GenerateTestID()
atomicRequests := map[ids.ID]*atomic.Requests{ids.GenerateTestID(): nil}
atomicRequests := make(map[ids.ID]*atomic.Requests)
acceptor.backend.blkIDToState[blk.ID()] = &blockState{
onAcceptState: onAcceptState,
atomicRequests: atomicRequests,
@@ -207,7 +208,7 @@ func TestAcceptorVisitStandardBlock(t *testing.T) {
// Set [blk]'s state in the map as though it had been verified.
onAcceptState := state.NewMockDiff(ctrl)
childID := ids.GenerateTestID()
atomicRequests := map[ids.ID]*atomic.Requests{ids.GenerateTestID(): nil}
atomicRequests := make(map[ids.ID]*atomic.Requests)
calledOnAcceptFunc := false
acceptor.backend.blkIDToState[blk.ID()] = &blockState{
onAcceptState: onAcceptState,
@@ -280,13 +281,21 @@ func TestAcceptorVisitCommitBlock(t *testing.T) {
parentOnAbortState := state.NewMockDiff(ctrl)
parentOnCommitState := state.NewMockDiff(ctrl)
parentStatelessBlk := block.NewMockBlock(ctrl)
calledOnAcceptFunc := false
atomicRequests := make(map[ids.ID]*atomic.Requests)
parentState := &blockState{
statelessBlock: parentStatelessBlk,
onAcceptState: parentOnAcceptState,
proposalBlockState: proposalBlockState{
onAbortState: parentOnAbortState,
onCommitState: parentOnCommitState,
},
statelessBlock: parentStatelessBlk,

onAcceptState: parentOnAcceptState,
onAcceptFunc: func() {
calledOnAcceptFunc = true
},

atomicRequests: atomicRequests,
}
acceptor.backend.blkIDToState[parentID] = parentState

@@ -308,13 +317,21 @@ func TestAcceptorVisitCommitBlock(t *testing.T) {
err = acceptor.ApricotCommitBlock(blk)
require.ErrorIs(err, errMissingBlockState)

parentOnCommitState.EXPECT().GetTimestamp().Return(time.Unix(0, 0))

// Set [blk]'s state in the map as though it had been verified.
acceptor.backend.blkIDToState[parentID] = parentState
onAcceptState := state.NewMockDiff(ctrl)
acceptor.backend.blkIDToState[blkID] = &blockState{
onAcceptState: onAcceptState,
onAcceptState: parentState.onCommitState,
onAcceptFunc: parentState.onAcceptFunc,

inputs: parentState.inputs,
timestamp: parentOnCommitState.GetTimestamp(),
atomicRequests: parentState.atomicRequests,
}

batch := database.NewMockBatch(ctrl)

// Set expected calls on dependencies.
// Make sure the parent is accepted first.
gomock.InOrder(
@@ -328,12 +345,15 @@ func TestAcceptorVisitCommitBlock(t *testing.T) {
s.EXPECT().SetHeight(blk.Height()).Times(1),
s.EXPECT().AddStatelessBlock(blk).Times(1),

onAcceptState.EXPECT().Apply(s).Times(1),
s.EXPECT().Commit().Return(nil).Times(1),
parentOnCommitState.EXPECT().Apply(s).Times(1),
s.EXPECT().CommitBatch().Return(batch, nil).Times(1),
sharedMemory.EXPECT().Apply(atomicRequests, batch).Return(nil).Times(1),
s.EXPECT().Checksum().Return(ids.Empty).Times(1),
s.EXPECT().Abort().Times(1),
)

require.NoError(acceptor.ApricotCommitBlock(blk))
require.True(calledOnAcceptFunc)
require.Equal(blk.ID(), acceptor.backend.lastAccepted)
}

@@ -371,13 +391,21 @@ func TestAcceptorVisitAbortBlock(t *testing.T) {
parentOnAbortState := state.NewMockDiff(ctrl)
parentOnCommitState := state.NewMockDiff(ctrl)
parentStatelessBlk := block.NewMockBlock(ctrl)
calledOnAcceptFunc := false
atomicRequests := make(map[ids.ID]*atomic.Requests)
parentState := &blockState{
statelessBlock: parentStatelessBlk,
onAcceptState: parentOnAcceptState,
proposalBlockState: proposalBlockState{
onAbortState: parentOnAbortState,
onCommitState: parentOnCommitState,
},
statelessBlock: parentStatelessBlk,

onAcceptState: parentOnAcceptState,
onAcceptFunc: func() {
calledOnAcceptFunc = true
},

atomicRequests: atomicRequests,
}
acceptor.backend.blkIDToState[parentID] = parentState

@@ -399,14 +427,21 @@ func TestAcceptorVisitAbortBlock(t *testing.T) {
err = acceptor.ApricotAbortBlock(blk)
require.ErrorIs(err, errMissingBlockState)

parentOnAbortState.EXPECT().GetTimestamp().Return(time.Unix(0, 0))

// Set [blk]'s state in the map as though it had been verified.
acceptor.backend.blkIDToState[parentID] = parentState

onAcceptState := state.NewMockDiff(ctrl)
acceptor.backend.blkIDToState[blkID] = &blockState{
onAcceptState: onAcceptState,
onAcceptState: parentState.onAbortState,
onAcceptFunc: parentState.onAcceptFunc,

inputs: parentState.inputs,
timestamp: parentOnAbortState.GetTimestamp(),
atomicRequests: parentState.atomicRequests,
}

batch := database.NewMockBatch(ctrl)

// Set expected calls on dependencies.
// Make sure the parent is accepted first.
gomock.InOrder(
@@ -420,11 +455,14 @@ func TestAcceptorVisitAbortBlock(t *testing.T) {
s.EXPECT().SetHeight(blk.Height()).Times(1),
s.EXPECT().AddStatelessBlock(blk).Times(1),

onAcceptState.EXPECT().Apply(s).Times(1),
s.EXPECT().Commit().Return(nil).Times(1),
parentOnAbortState.EXPECT().Apply(s).Times(1),
s.EXPECT().CommitBatch().Return(batch, nil).Times(1),
sharedMemory.EXPECT().Apply(atomicRequests, batch).Return(nil).Times(1),
s.EXPECT().Checksum().Return(ids.Empty).Times(1),
s.EXPECT().Abort().Times(1),
)

require.NoError(acceptor.ApricotAbortBlock(blk))
require.True(calledOnAcceptFunc)
require.Equal(blk.ID(), acceptor.backend.lastAccepted)
}
8 changes: 4 additions & 4 deletions vms/platformvm/block/executor/backend.go
Original file line number Diff line number Diff line change
@@ -51,20 +51,20 @@ func (b *backend) GetState(blkID ids.ID) (state.Chain, bool) {
return b.state, blkID == b.state.GetLastAccepted()
}

func (b *backend) getOnAbortState(blkID ids.ID) (state.Diff, bool) {
func (b *backend) getBlkWithOnAbortState(blkID ids.ID) (*blockState, bool) {
state, ok := b.blkIDToState[blkID]
if !ok || state.onAbortState == nil {
return nil, false
}
return state.onAbortState, true
return state, true
}

func (b *backend) getOnCommitState(blkID ids.ID) (state.Diff, bool) {
func (b *backend) getBlkWithOnCommitState(blkID ids.ID) (*blockState, bool) {
state, ok := b.blkIDToState[blkID]
if !ok || state.onCommitState == nil {
return nil, false
}
return state.onCommitState, true
return state, true
}

func (b *backend) GetBlock(blkID ids.ID) (block.Block, error) {
22 changes: 16 additions & 6 deletions vms/platformvm/block/executor/verifier.go
Original file line number Diff line number Diff line change
@@ -319,33 +319,43 @@ func (v *verifier) commonBlock(b block.Block) error {
// abortBlock populates the state of this block if [nil] is returned
func (v *verifier) abortBlock(b block.Block) error {
parentID := b.Parent()
onAcceptState, ok := v.getOnAbortState(parentID)
parentState, ok := v.getBlkWithOnAbortState(parentID)
if !ok {
return fmt.Errorf("%w: %s", state.ErrMissingParentState, parentID)
}

blkID := b.ID()
v.blkIDToState[blkID] = &blockState{
statelessBlock: b,
onAcceptState: onAcceptState,
timestamp: onAcceptState.GetTimestamp(),

onAcceptState: parentState.onAbortState,
onAcceptFunc: parentState.onAcceptFunc,

inputs: parentState.inputs,
timestamp: parentState.onAbortState.GetTimestamp(),
atomicRequests: parentState.atomicRequests,
}
return nil
}

// commitBlock populates the state of this block if [nil] is returned
func (v *verifier) commitBlock(b block.Block) error {
parentID := b.Parent()
onAcceptState, ok := v.getOnCommitState(parentID)
parentState, ok := v.getBlkWithOnCommitState(parentID)
if !ok {
return fmt.Errorf("%w: %s", state.ErrMissingParentState, parentID)
}

blkID := b.ID()
v.blkIDToState[blkID] = &blockState{
statelessBlock: b,
onAcceptState: onAcceptState,
timestamp: onAcceptState.GetTimestamp(),

onAcceptState: parentState.onCommitState,
onAcceptFunc: parentState.onAcceptFunc,

inputs: parentState.inputs,
timestamp: parentState.onCommitState.GetTimestamp(),
atomicRequests: parentState.atomicRequests,
}
return nil
}
8 changes: 8 additions & 0 deletions vms/platformvm/vm_test.go
Original file line number Diff line number Diff line change
@@ -1873,6 +1873,10 @@ func TestUptimeDisallowedWithRestart(t *testing.T) {
secondCtx.Lock.Unlock()
}()

atomicDB := prefixdb.New([]byte{1}, db)
m := atomic.NewMemory(atomicDB)
secondCtx.SharedMemory = m.NewSharedMemory(secondCtx.ChainID)

secondMsgChan := make(chan common.Message, 1)
require.NoError(secondVM.Initialize(
context.Background(),
@@ -1962,6 +1966,10 @@ func TestUptimeDisallowedAfterNeverConnecting(t *testing.T) {
ctx := defaultContext(t)
ctx.Lock.Lock()

atomicDB := prefixdb.New([]byte{1}, db)
m := atomic.NewMemory(atomicDB)
ctx.SharedMemory = m.NewSharedMemory(ctx.ChainID)

msgChan := make(chan common.Message, 1)
appSender := &common.SenderTest{T: t}
require.NoError(vm.Initialize(

0 comments on commit f5266fb

Please sign in to comment.