Skip to content

Revert "Make sure inner state summary accept is called (#3831)" #3953

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.

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

Draft
wants to merge 1 commit into
base: coreth-atomic-refactor
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 2 additions & 8 deletions snow/engine/snowman/block/state_summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,8 @@ type StateSummary interface {
// Bytes returns a byte slice than can be used to reconstruct this summary.
Bytes() []byte

// Accept notifies the VM that this is a canonically accepted state summary.
// The VM is expected to return how it plans on handling this summary via
// the [StateSyncMode].
// Accept triggers the VM to start state syncing to this summary.
//
// Invariant: The VM must be able to correctly handle the acceptance of a
// state summary that is older than its current state.
//
// See [StateSyncSkipped], [StateSyncStatic], and [StateSyncDynamic] for the
// expected invariants the VM must maintain based on the return value.
// It returns the state sync mode selected by the VM.
Accept(context.Context) (StateSyncMode, error)
}
9 changes: 6 additions & 3 deletions vms/proposervm/state_summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ func (s *stateSummary) Height() uint64 {
}

func (s *stateSummary) Accept(ctx context.Context) (block.StateSyncMode, error) {
// If we have already synced up to or past this state summary, we do not
// want to sync to it.
if s.vm.lastAcceptedHeight >= s.Height() {
return block.StateSyncSkipped, nil
}

// set fork height first, before accepting proposerVM full block
// which updates height index (among other indices)
if err := s.vm.State.SetForkHeight(s.StateSummary.ForkHeight()); err != nil {
Expand All @@ -56,8 +62,5 @@ func (s *stateSummary) Accept(ctx context.Context) (block.StateSyncMode, error)
// innerSummary.Accept may fail with the proposerVM block and index already
// updated. The error would be treated as fatal and the chain would then be
// repaired upon the VM restart.
// After the inner summary is accepted, the engine transitions to bootstrapping
// and SetState is responsible for re-aligning the ProposerVM to the height reported
// by the inner VM after handling state sync.
return s.innerSummary.Accept(ctx)
}
21 changes: 2 additions & 19 deletions vms/proposervm/state_syncable_vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ func helperBuildStateSyncTestObjects(t *testing.T) (*fullVM, *VM) {
nil,
nil,
))
require.NoError(vm.SetState(context.Background(), snow.StateSyncing))

return innerVM, vm
}
Expand Down Expand Up @@ -565,28 +564,12 @@ func TestStateSummaryAcceptOlderBlock(t *testing.T) {

summary, err := vm.GetStateSummary(context.Background(), reqHeight)
require.NoError(err)
require.Equal(summary.Height(), reqHeight)

// test Accept summary invokes innerVM
calledInnerAccept := false
// test Accept skipped
innerSummary.AcceptF = func(context.Context) (block.StateSyncMode, error) {
innerVM.LastAcceptedF = func(context.Context) (ids.ID, error) {
return innerSummary.ID(), nil
}
innerVM.GetBlockF = func(context.Context, ids.ID) (snowman.Block, error) {
return innerBlk, nil
}
calledInnerAccept = true
return block.StateSyncStatic, nil
}
status, err := summary.Accept(context.Background())
require.NoError(err)
require.Equal(block.StateSyncStatic, status)
require.True(calledInnerAccept)

require.NoError(vm.SetState(context.Background(), snow.Bootstrapping))
require.Equal(summary.Height(), vm.lastAcceptedHeight)
lastAcceptedID, err := vm.LastAccepted(context.Background())
require.NoError(err)
require.Equal(proBlk.ID(), lastAcceptedID)
require.Equal(block.StateSyncSkipped, status)
}