diff --git a/state/protocol/protocol_state/epochs/base_statemachine.go b/state/protocol/protocol_state/epochs/base_statemachine.go index 6b61f784ddd..171cc5da239 100644 --- a/state/protocol/protocol_state/epochs/base_statemachine.go +++ b/state/protocol/protocol_state/epochs/base_statemachine.go @@ -1,6 +1,8 @@ package epochs import ( + "fmt" + "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/state/protocol" ) @@ -95,3 +97,30 @@ func (u *baseStateMachine) EjectIdentity(nodeID flow.Identifier) error { } return nil } + +// TransitionToNextEpoch updates the notion of 'current epoch', 'previous' and 'next epoch' in the protocol +// state. An epoch transition is only allowed when _all_ of the following conditions are satisfied: +// - next epoch has been set up, +// - next epoch has been committed, +// - candidate block is in the next epoch. +// No errors are expected during normal operations. +func (u *baseStateMachine) TransitionToNextEpoch() error { + nextEpoch := u.state.NextEpoch + if nextEpoch == nil { // nextEpoch ≠ nil if and only if next epoch was already set up (on the happy path) + return fmt.Errorf("protocol state for next epoch has not yet been setup") + } + if nextEpoch.CommitID == flow.ZeroID { // nextEpoch.CommitID ≠ flow.ZeroID if and only if next epoch was already committed (on the happy path) + return fmt.Errorf("protocol state for next epoch has not yet been committed") + } + // Check if we are at the next epoch, only then a transition is allowed + if u.view < u.parentState.NextEpochSetup.FirstView { + return fmt.Errorf("epoch transition is only allowed when entering next epoch") + } + u.state = &flow.ProtocolStateEntry{ + PreviousEpoch: &u.state.CurrentEpoch, + CurrentEpoch: *u.state.NextEpoch, + InvalidEpochTransitionAttempted: u.state.InvalidEpochTransitionAttempted, + } + u.rebuildIdentityLookup() + return nil +} diff --git a/state/protocol/protocol_state/epochs/fallback_statemachine.go b/state/protocol/protocol_state/epochs/fallback_statemachine.go index 86c38374a3c..d9109d7fc35 100644 --- a/state/protocol/protocol_state/epochs/fallback_statemachine.go +++ b/state/protocol/protocol_state/epochs/fallback_statemachine.go @@ -98,10 +98,3 @@ func (m *FallbackStateMachine) ProcessEpochCommit(_ *flow.EpochCommit) (bool, er // won't process if we are in fallback mode return false, nil } - -// TransitionToNextEpoch performs transition to next epoch, in epoch fallback no transitions are possible. -// TODO for 'leaving Epoch Fallback via special service event' this might need to change. -func (m *FallbackStateMachine) TransitionToNextEpoch() error { - // won't process if we are in fallback mode - return nil -} diff --git a/state/protocol/protocol_state/epochs/fallback_statemachine_test.go b/state/protocol/protocol_state/epochs/fallback_statemachine_test.go index 5ed50283cca..0f6dfcd5ee0 100644 --- a/state/protocol/protocol_state/epochs/fallback_statemachine_test.go +++ b/state/protocol/protocol_state/epochs/fallback_statemachine_test.go @@ -61,14 +61,75 @@ func (s *EpochFallbackStateMachineSuite) TestProcessEpochCommitIsNoop() { require.Equal(s.T(), s.parentProtocolState.ID(), s.stateMachine.ParentState().ID()) } -// TestTransitionToNextEpoch ensures that transition to next epoch is not possible. +// TestTransitionToNextEpoch tests a scenario where the FallbackStateMachine processes first block from next epoch. +// It has to discard the parent state and build a new state with data from next epoch. func (s *EpochFallbackStateMachineSuite) TestTransitionToNextEpoch() { - err := s.stateMachine.TransitionToNextEpoch() - require.NoError(s.T(), err) - updatedState, updateStateID, hasChanges := s.stateMachine.Build() - require.False(s.T(), hasChanges) - require.Equal(s.T(), updatedState.ID(), updateStateID) - require.Equal(s.T(), s.parentProtocolState.ID(), updateStateID) + // update protocol state with next epoch information + unittest.WithNextEpochProtocolState()(s.parentProtocolState) + + candidate := unittest.BlockHeaderFixture( + unittest.HeaderWithView(s.parentProtocolState.CurrentEpochSetup.FinalView + 1)) + + expectedState := &flow.ProtocolStateEntry{ + PreviousEpoch: s.parentProtocolState.CurrentEpoch.Copy(), + CurrentEpoch: *s.parentProtocolState.NextEpoch.Copy(), + NextEpoch: nil, + InvalidEpochTransitionAttempted: true, + } + + // Irrespective of whether the parent state is in EFM, the FallbackStateMachine should always set + // `InvalidEpochTransitionAttempted` to true and transition the next epoch, because the candidate block + // belongs to the next epoch. + var err error + for _, parentAlreadyInEFM := range []bool{true, false} { + parentProtocolState := s.parentProtocolState.Copy() + parentProtocolState.InvalidEpochTransitionAttempted = parentAlreadyInEFM + + s.stateMachine, err = NewFallbackStateMachine(s.params, candidate.View, parentProtocolState.Copy()) + require.NoError(s.T(), err) + err = s.stateMachine.TransitionToNextEpoch() + require.NoError(s.T(), err) + updatedState, stateID, hasChanges := s.stateMachine.Build() + require.True(s.T(), hasChanges) + require.NotEqual(s.T(), parentProtocolState.ID(), updatedState.ID()) + require.Equal(s.T(), updatedState.ID(), stateID) + require.Equal(s.T(), updatedState, expectedState, "FallbackStateMachine produced unexpected Protocol State") + } +} + +// TestTransitionToNextEpochNotAllowed tests different scenarios where transition to next epoch is not allowed. +func (s *EpochFallbackStateMachineSuite) TestTransitionToNextEpochNotAllowed() { + s.Run("no next epoch protocol state", func() { + protocolState := unittest.EpochStateFixture() + candidate := unittest.BlockHeaderFixture( + unittest.HeaderWithView(protocolState.CurrentEpochSetup.FinalView + 1)) + stateMachine, err := NewFallbackStateMachine(s.params, candidate.View, protocolState) + require.NoError(s.T(), err) + err = stateMachine.TransitionToNextEpoch() + require.Error(s.T(), err, "should not allow transition to next epoch if there is no next epoch protocol state") + }) + s.Run("next epoch not committed", func() { + protocolState := unittest.EpochStateFixture(unittest.WithNextEpochProtocolState(), func(entry *flow.RichProtocolStateEntry) { + entry.NextEpoch.CommitID = flow.ZeroID + entry.NextEpochCommit = nil + entry.NextEpochIdentityTable = nil + }) + candidate := unittest.BlockHeaderFixture( + unittest.HeaderWithView(protocolState.CurrentEpochSetup.FinalView + 1)) + stateMachine, err := NewFallbackStateMachine(s.params, candidate.View, protocolState) + require.NoError(s.T(), err) + err = stateMachine.TransitionToNextEpoch() + require.Error(s.T(), err, "should not allow transition to next epoch if it is not committed") + }) + s.Run("candidate block is not from next epoch", func() { + protocolState := unittest.EpochStateFixture(unittest.WithNextEpochProtocolState()) + candidate := unittest.BlockHeaderFixture( + unittest.HeaderWithView(protocolState.CurrentEpochSetup.FinalView)) + stateMachine, err := NewFallbackStateMachine(s.params, candidate.View, protocolState) + require.NoError(s.T(), err) + err = stateMachine.TransitionToNextEpoch() + require.Error(s.T(), err, "should not allow transition to next epoch if next block is not first block from next epoch") + }) } // TestNewEpochFallbackStateMachine tests that creating epoch fallback state machine sets @@ -313,11 +374,9 @@ func (s *EpochFallbackStateMachineSuite) TestEpochFallbackStateMachineInjectsMul // TestEpochFallbackStateMachineInjectsMultipleExtensions_NextEpochCommitted tests that the state machine injects multiple extensions // as it reaches the safety threshold of the current epoch and the extensions themselves. // In this test we are simulating the scenario where the current epoch enters fallback mode when the next epoch has been committed. -// It is expected that it will transition into the next epoch (since it was committed) +// It is expected that it will transition into the next epoch (since it was committed), // then reach the safety threshold and add the extension to the next epoch, which at that point will be considered 'current'. func (s *EpochFallbackStateMachineSuite) TestEpochFallbackStateMachineInjectsMultipleExtensions_NextEpochCommitted() { - unittest.SkipUnless(s.T(), unittest.TEST_TODO, - "This test doesn't work since it misses logic to transition to the next epoch when we are in EFM.") originalParentState := s.parentProtocolState.Copy() originalParentState.InvalidEpochTransitionAttempted = false unittest.WithNextEpochProtocolState()(originalParentState) @@ -347,7 +406,6 @@ func (s *EpochFallbackStateMachineSuite) TestEpochFallbackStateMachineInjectsMul firstExtensionViewThreshold := originalParentState.NextEpochSetup.FinalView + DefaultEpochExtensionViewCount - s.params.EpochCommitSafetyThreshold() secondExtensionViewThreshold := originalParentState.NextEpochSetup.FinalView + 2*DefaultEpochExtensionViewCount - s.params.EpochCommitSafetyThreshold() thirdExtensionViewThreshold := originalParentState.NextEpochSetup.FinalView + 3*DefaultEpochExtensionViewCount - s.params.EpochCommitSafetyThreshold() - currentEpochLastView := originalParentState.CurrentEpochSetup.FinalView parentProtocolState := originalParentState.Copy() candidateView := originalParentState.CurrentEpochSetup.FirstView + 1 @@ -358,31 +416,27 @@ func (s *EpochFallbackStateMachineSuite) TestEpochFallbackStateMachineInjectsMul stateMachine, err := NewFallbackStateMachine(s.params, candidateView, parentProtocolState.Copy()) require.NoError(s.T(), err) - var ( - nextEpochSetup *flow.EpochSetup - nextEpochCommit *flow.EpochCommit - ) + previousEpochSetup, previousEpochCommit := parentProtocolState.PreviousEpochSetup, parentProtocolState.PreviousEpochCommit + currentEpochSetup, currentEpochCommit := parentProtocolState.CurrentEpochSetup, parentProtocolState.CurrentEpochCommit + nextEpochSetup, nextEpochCommit := parentProtocolState.NextEpochSetup, parentProtocolState.NextEpochCommit if candidateView > parentProtocolState.CurrentEpochFinalView() { require.NoError(s.T(), stateMachine.TransitionToNextEpoch()) - } - - updatedState, _, _ := stateMachine.Build() - if candidateView < currentEpochLastView { - // as the next epoch has been committed, we need to respect that in current epoch - // when we transition to the next epoch we will pass nil instead. - nextEpochSetup = parentProtocolState.NextEpochSetup - nextEpochCommit = parentProtocolState.NextEpochCommit + // after we have transitioned to the next epoch, we need to update the current epoch + // for the next iteration we can use parent protocol state again + previousEpochSetup, previousEpochCommit = currentEpochSetup, currentEpochCommit + currentEpochSetup, currentEpochCommit = parentProtocolState.NextEpochSetup, parentProtocolState.NextEpochCommit + nextEpochSetup, nextEpochCommit = nil, nil } + updatedState, _, _ := stateMachine.Build() parentProtocolState, err = flow.NewRichProtocolStateEntry(updatedState, - parentProtocolState.PreviousEpochSetup, - parentProtocolState.PreviousEpochCommit, - parentProtocolState.CurrentEpochSetup, - parentProtocolState.CurrentEpochCommit, + previousEpochSetup, + previousEpochCommit, + currentEpochSetup, + currentEpochCommit, nextEpochSetup, nextEpochCommit) - require.NoError(s.T(), err) } } @@ -409,11 +463,11 @@ func (s *EpochFallbackStateMachineSuite) TestEpochFallbackStateMachineInjectsMul evolveStateToView(data.TargetView) expectedState := &flow.ProtocolStateEntry{ - PreviousEpoch: originalParentState.PreviousEpoch, + PreviousEpoch: originalParentState.CurrentEpoch.Copy(), CurrentEpoch: flow.EpochStateContainer{ - SetupID: originalParentState.CurrentEpoch.SetupID, - CommitID: originalParentState.CurrentEpoch.CommitID, - ActiveIdentities: originalParentState.CurrentEpoch.ActiveIdentities, + SetupID: originalParentState.NextEpoch.SetupID, + CommitID: originalParentState.NextEpoch.CommitID, + ActiveIdentities: originalParentState.NextEpoch.ActiveIdentities, EpochExtensions: data.ExpectedExtensions, }, NextEpoch: nil, @@ -423,5 +477,4 @@ func (s *EpochFallbackStateMachineSuite) TestEpochFallbackStateMachineInjectsMul require.Greater(s.T(), parentProtocolState.CurrentEpochFinalView(), candidateView, "final view should be greater than final view of test") } - } diff --git a/state/protocol/protocol_state/epochs/happy_path_statemachine.go b/state/protocol/protocol_state/epochs/happy_path_statemachine.go index 8bbc38e3076..b73bbde1995 100644 --- a/state/protocol/protocol_state/epochs/happy_path_statemachine.go +++ b/state/protocol/protocol_state/epochs/happy_path_statemachine.go @@ -152,33 +152,3 @@ func (u *HappyPathStateMachine) ProcessEpochCommit(epochCommit *flow.EpochCommit u.state.NextEpoch.CommitID = epochCommit.ID() return true, nil } - -// TransitionToNextEpoch updates the notion of 'current epoch', 'previous' and 'next epoch' in the protocol -// state. An epoch transition is only allowed when: -// - next epoch has been set up, -// - next epoch has been committed, -// - invalid state transition has not been attempted (this is ensured by constructor), -// - candidate block is in the next epoch. -// No errors are expected during normal operations. -func (u *HappyPathStateMachine) TransitionToNextEpoch() error { - nextEpoch := u.state.NextEpoch - // Check if there is next epoch protocol state - if nextEpoch == nil { - return fmt.Errorf("protocol state has not been setup yet") - } - // Check if there is a commit event for next epoch - if nextEpoch.CommitID == flow.ZeroID { - return fmt.Errorf("protocol state has not been committed yet") - } - // Check if we are at the next epoch, only then a transition is allowed - if u.view < u.parentState.NextEpochSetup.FirstView { - return fmt.Errorf("protocol state transition is only allowed when enterring next epoch") - } - u.state = &flow.ProtocolStateEntry{ - PreviousEpoch: &u.state.CurrentEpoch, - CurrentEpoch: *u.state.NextEpoch, - InvalidEpochTransitionAttempted: false, - } - u.rebuildIdentityLookup() - return nil -} diff --git a/state/protocol/protocol_state/epochs/statemachine.go b/state/protocol/protocol_state/epochs/statemachine.go index 8f5b73ab1a5..c26051132ed 100644 --- a/state/protocol/protocol_state/epochs/statemachine.go +++ b/state/protocol/protocol_state/epochs/statemachine.go @@ -205,6 +205,11 @@ func (e *EpochStateMachine) EvolveState(sealedServiceEvents []flow.ServiceEvent) phase := parentProtocolState.EpochPhase() if phase == flow.EpochPhaseCommitted { activeSetup := parentProtocolState.CurrentEpochSetup + // TODO(efm-recovery): This logic needs to be updated when injection of EFM recovery service event is implemented. + // For now, we are only checking transition to next epoch when the next epoch is committed. + // There is no logic yet to leave EFM, but once we have it, the following logic + // will be incorrect as we need to check the epoch final view including the extensions but not + // only the final view of the setup. if e.activeStateMachine.View() > activeSetup.FinalView { err := e.activeStateMachine.TransitionToNextEpoch() if err != nil {