Skip to content
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

[Proposal] - Version Proposal Blocks #2439

Closed
wants to merge 17 commits into from
Closed

[Proposal] - Version Proposal Blocks #2439

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7f44ca5
wip: scaffolding
abi87 Dec 6, 2023
0632c28
wip: fixing UTs
abi87 Dec 7, 2023
69ade2b
wip: some more UTs fixing
abi87 Dec 7, 2023
41be5b9
wip: some more UTs fixing
abi87 Dec 7, 2023
d4c25dc
Merge branch 'dev' into version_proposal_blocks
abi87 Dec 7, 2023
1170933
fix metrics
abi87 Dec 7, 2023
8ddbef5
nit
abi87 Dec 7, 2023
323587f
nits
abi87 Dec 7, 2023
5eef147
nits
abi87 Dec 7, 2023
a5a25e8
nit
abi87 Dec 7, 2023
861ac12
fixed acceptor
abi87 Dec 8, 2023
48b5a81
nit
abi87 Dec 8, 2023
bbc4b58
Merge branch 'dev' into version_proposal_blocks
abi87 Dec 8, 2023
b69ec52
Merge branch 'dev' into version_proposal_blocks
abi87 Dec 9, 2023
0496520
Merge branch 'dev' into version_proposal_blocks
abi87 Dec 10, 2023
b75f688
Merge branch 'dev' into version_proposal_blocks
abi87 Dec 11, 2023
9eb48f2
Merge branch 'dev' into version_proposal_blocks
abi87 Dec 11, 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
47 changes: 32 additions & 15 deletions vms/platformvm/block/executor/acceptor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,44 +117,46 @@ func (a *acceptor) ApricotAtomicBlock(b *block.ApricotAtomicBlock) error {
}

func (a *acceptor) abortBlock(b block.Block, blockType string) error {
parentID := b.Parent()
parentState, ok := a.blkIDToState[parentID]
blkID := b.ID()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when we verified the option we copied over option's blockState the preference. Hence we can lookup the option blockState now, instead of its parent's one

blkState, ok := a.blkIDToState[blkID]
if !ok {
return fmt.Errorf("%w: %s", state.ErrMissingParentState, parentID)
return fmt.Errorf("%w %s", errMissingBlockState, blkID)
}

if a.bootstrapped.Get() {
if parentState.initiallyPreferCommit {
if blkState.initiallyPreferCommit {
a.metrics.MarkOptionVoteLost()
} else {
a.metrics.MarkOptionVoteWon()
}
}

return a.optionBlock(b, parentState.statelessBlock, blockType)
return a.optionBlock(b, blockType)
}

func (a *acceptor) commitBlock(b block.Block, blockType string) error {
parentID := b.Parent()
parentState, ok := a.blkIDToState[parentID]
blkID := b.ID()
blkState, ok := a.blkIDToState[blkID]
if !ok {
return fmt.Errorf("%w: %s", state.ErrMissingParentState, parentID)
return fmt.Errorf("%w %s", errMissingBlockState, blkID)
}

if a.bootstrapped.Get() {
if parentState.initiallyPreferCommit {
if blkState.initiallyPreferCommit {
a.metrics.MarkOptionVoteWon()
} else {
a.metrics.MarkOptionVoteLost()
}
}

return a.optionBlock(b, parentState.statelessBlock, blockType)
return a.optionBlock(b, blockType)
}

func (a *acceptor) optionBlock(b, parent block.Block, blockType string) error {
blkID := b.ID()
parentID := parent.ID()
func (a *acceptor) optionBlock(b block.Block, blockType string) error {
var (
parentID = b.Parent()
blkID = b.ID()
)

defer func() {
// Note: we assume this block's sibling doesn't
Expand All @@ -163,11 +165,25 @@ func (a *acceptor) optionBlock(b, parent block.Block, blockType string) error {
a.free(blkID)
}()

// Note that the parent must be accepted first.
if err := a.commonAccept(parent); err != nil {
// 1. Accept the proposal block
parentState, ok := a.blkIDToState[parentID]
if !ok {
return fmt.Errorf("%w: %s", state.ErrMissingParentState, parentID)
}

if err := a.commonAccept(parentState.statelessBlock); err != nil {
return err
}

parentBlkState, ok := a.blkIDToState[parentID]
if !ok {
return fmt.Errorf("%w %s", errMissingBlockState, parentID)
}
if err := parentBlkState.onAcceptState.Apply(a.state); err != nil {
return err
}

// 2. Accept the option
if err := a.commonAccept(b); err != nil {
return err
}
Expand All @@ -180,6 +196,7 @@ func (a *acceptor) optionBlock(b, parent block.Block, blockType string) error {
return err
}

// 3. Commit both option and proposal block changes
if err := a.state.Commit(); err != nil {
return err
}
Expand Down
229 changes: 102 additions & 127 deletions vms/platformvm/block/executor/acceptor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package executor

import (
"testing"
"time"

"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -135,7 +136,7 @@ func TestAcceptorVisitAtomicBlock(t *testing.T) {
childOnCommitState := state.NewMockDiff(ctrl)
childState := &blockState{
onAcceptState: childOnAcceptState,
proposalBlockState: proposalBlockState{
optionsState: optionsState{
onAbortState: childOnAbortState,
onCommitState: childOnCommitState,
},
Expand Down Expand Up @@ -224,7 +225,7 @@ func TestAcceptorVisitStandardBlock(t *testing.T) {
childOnCommitState := state.NewMockDiff(ctrl)
childState := &blockState{
onAcceptState: childOnAcceptState,
proposalBlockState: proposalBlockState{
optionsState: optionsState{
onAbortState: childOnAbortState,
onCommitState: childOnCommitState,
},
Expand Down Expand Up @@ -254,7 +255,12 @@ func TestAcceptorVisitCommitBlock(t *testing.T) {
s := state.NewMockState(ctrl)
sharedMemory := atomic.NewMockSharedMemory(ctrl)

parentID := ids.GenerateTestID()
var (
parentID = ids.GenerateTestID()
parentHeight = uint64(10)
blkTime = time.Now().Truncate(time.Second)
)

acceptor := &acceptor{
backend: &backend{
lastAccepted: parentID,
Expand All @@ -270,72 +276,55 @@ func TestAcceptorVisitCommitBlock(t *testing.T) {
bootstrapped: &utils.Atomic[bool]{},
}

blk, err := block.NewApricotCommitBlock(parentID, 1 /*height*/)
blk, err := block.NewBanffCommitBlock(blkTime, parentID, parentHeight+1)
require.NoError(err)

err = acceptor.ApricotCommitBlock(blk)
require.ErrorIs(err, state.ErrMissingParentState)

// Set [blk]'s parent in the state map.
parentOnAcceptState := state.NewMockDiff(ctrl)
parentOnAbortState := state.NewMockDiff(ctrl)
parentOnCommitState := state.NewMockDiff(ctrl)
parentStatelessBlk := block.NewMockBlock(ctrl)
parentState := &blockState{
statelessBlock: parentStatelessBlk,
onAcceptState: parentOnAcceptState,
proposalBlockState: proposalBlockState{
onAbortState: parentOnAbortState,
onCommitState: parentOnCommitState,
},
}
acceptor.backend.blkIDToState[parentID] = parentState

blkID := blk.ID()
// Set expected calls on dependencies.
// Make sure the parent is accepted first.
gomock.InOrder(
parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2),
s.EXPECT().SetLastAccepted(parentID).Times(1),
parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1),
s.EXPECT().SetHeight(blk.Height()-1).Times(1),
s.EXPECT().AddStatelessBlock(parentState.statelessBlock).Times(1),

s.EXPECT().SetLastAccepted(blkID).Times(1),
s.EXPECT().SetHeight(blk.Height()).Times(1),
s.EXPECT().AddStatelessBlock(blk).Times(1),
)

err = acceptor.ApricotCommitBlock(blk)
require.ErrorIs(err, errMissingBlockState)

// 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,
{
err = acceptor.BanffCommitBlock(blk)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to BanffCommitBlock here. There is not much difference in the logic and it's better to test options currently created on mainnet rather than deprecated versions

require.ErrorIs(err, errMissingBlockState)
}

// Set expected calls on dependencies.
// Make sure the parent is accepted first.
gomock.InOrder(
parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2),
s.EXPECT().SetLastAccepted(parentID).Times(1),
parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1),
s.EXPECT().SetHeight(blk.Height()-1).Times(1),
s.EXPECT().AddStatelessBlock(parentState.statelessBlock).Times(1),

s.EXPECT().SetLastAccepted(blkID).Times(1),
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),
s.EXPECT().Checksum().Return(ids.Empty).Times(1),
)

require.NoError(acceptor.ApricotCommitBlock(blk))
require.Equal(blk.ID(), acceptor.backend.lastAccepted)
{
// Set [blk]'s parent in the state map.
// We don't bother setting options state since
// it's not relevant when accepting the block
parentOnAcceptState := state.NewMockDiff(ctrl)
parentStatelessBlk := block.NewMockBlock(ctrl)
parentStatelessBlk.EXPECT().ID().Return(parentID).AnyTimes()
parentStatelessBlk.EXPECT().Height().Return(parentHeight).AnyTimes()

parentState := &blockState{
statelessBlock: parentStatelessBlk,
onAcceptState: parentOnAcceptState,
}
acceptor.backend.blkIDToState[parentID] = parentState

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

// Set expected calls on dependencies.
// Make sure the parent is accepted first.
gomock.InOrder(
s.EXPECT().SetLastAccepted(parentStatelessBlk.ID()).Times(1),
s.EXPECT().SetHeight(parentStatelessBlk.Height()).Times(1),
s.EXPECT().AddStatelessBlock(parentStatelessBlk).Times(1),
parentOnAcceptState.EXPECT().Apply(s).Times(1),

s.EXPECT().SetLastAccepted(blkID).Times(1),
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),
s.EXPECT().Checksum().Return(ids.Empty).Times(1),
)

require.NoError(acceptor.BanffCommitBlock(blk))
require.Equal(blk.ID(), acceptor.backend.lastAccepted)
}
}

func TestAcceptorVisitAbortBlock(t *testing.T) {
Expand All @@ -345,7 +334,11 @@ func TestAcceptorVisitAbortBlock(t *testing.T) {
s := state.NewMockState(ctrl)
sharedMemory := atomic.NewMockSharedMemory(ctrl)

parentID := ids.GenerateTestID()
var (
parentID = ids.GenerateTestID()
parentHeight = uint64(10)
blkTime = time.Now().Truncate(time.Second)
)
acceptor := &acceptor{
backend: &backend{
lastAccepted: parentID,
Expand All @@ -361,71 +354,53 @@ func TestAcceptorVisitAbortBlock(t *testing.T) {
bootstrapped: &utils.Atomic[bool]{},
}

blk, err := block.NewApricotAbortBlock(parentID, 1 /*height*/)
blk, err := block.NewBanffAbortBlock(blkTime, parentID, parentHeight+1)
require.NoError(err)

err = acceptor.ApricotAbortBlock(blk)
require.ErrorIs(err, state.ErrMissingParentState)

// Set [blk]'s parent in the state map.
parentOnAcceptState := state.NewMockDiff(ctrl)
parentOnAbortState := state.NewMockDiff(ctrl)
parentOnCommitState := state.NewMockDiff(ctrl)
parentStatelessBlk := block.NewMockBlock(ctrl)
parentState := &blockState{
statelessBlock: parentStatelessBlk,
onAcceptState: parentOnAcceptState,
proposalBlockState: proposalBlockState{
onAbortState: parentOnAbortState,
onCommitState: parentOnCommitState,
},
}
acceptor.backend.blkIDToState[parentID] = parentState

blkID := blk.ID()
// Set expected calls on dependencies.
// Make sure the parent is accepted first.
gomock.InOrder(
parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2),
s.EXPECT().SetLastAccepted(parentID).Times(1),
parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1),
s.EXPECT().SetHeight(blk.Height()-1).Times(1),
s.EXPECT().AddStatelessBlock(parentState.statelessBlock).Times(1),

s.EXPECT().SetLastAccepted(blkID).Times(1),
s.EXPECT().SetHeight(blk.Height()).Times(1),
s.EXPECT().AddStatelessBlock(blk).Times(1),
)

err = acceptor.ApricotAbortBlock(blk)
require.ErrorIs(err, errMissingBlockState)

// 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,
{
err = acceptor.BanffAbortBlock(blk)
require.ErrorIs(err, errMissingBlockState)
}

// Set expected calls on dependencies.
// Make sure the parent is accepted first.
gomock.InOrder(
parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2),
s.EXPECT().SetLastAccepted(parentID).Times(1),
parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1),
s.EXPECT().SetHeight(blk.Height()-1).Times(1),
s.EXPECT().AddStatelessBlock(parentState.statelessBlock).Times(1),

s.EXPECT().SetLastAccepted(blkID).Times(1),
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),
s.EXPECT().Checksum().Return(ids.Empty).Times(1),
)

require.NoError(acceptor.ApricotAbortBlock(blk))
require.Equal(blk.ID(), acceptor.backend.lastAccepted)
{
// Set [blk]'s parent in the state map.
// We don't bother setting options state since
// it's not relevant when accepting the block
parentOnAcceptState := state.NewMockDiff(ctrl)
parentStatelessBlk := block.NewMockBlock(ctrl)
parentStatelessBlk.EXPECT().ID().Return(parentID).AnyTimes()
parentStatelessBlk.EXPECT().Height().Return(parentHeight).AnyTimes()

parentState := &blockState{
statelessBlock: parentStatelessBlk,
onAcceptState: parentOnAcceptState,
}
acceptor.backend.blkIDToState[parentID] = parentState

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

// Set expected calls on dependencies.
// Make sure the parent is accepted first.
gomock.InOrder(
s.EXPECT().SetLastAccepted(parentStatelessBlk.ID()).Times(1),
s.EXPECT().SetHeight(parentStatelessBlk.Height()).Times(1),
s.EXPECT().AddStatelessBlock(parentStatelessBlk).Times(1),
parentOnAcceptState.EXPECT().Apply(s).Times(1),

s.EXPECT().SetLastAccepted(blkID).Times(1),
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),
s.EXPECT().Checksum().Return(ids.Empty).Times(1),
)

require.NoError(acceptor.BanffAbortBlock(blk))
require.Equal(blk.ID(), acceptor.backend.lastAccepted)
}
}
Loading
Loading