Changed expected error types of ProcessEpochSetup and ProcessEpochCom…

…mit. Updated tests, error handling, docs

state/protocol/badger/mutator.go

@@ -1121,6 +1121,11 @@ func (m *FollowerState) handleEpochServiceEvents(candidate *flow.Block, updater
 
 				err = updater.ProcessEpochSetup(ev)
 				if err != nil {
+					if protocol.IsInvalidServiceEventError(err) {
+						// we have observed an invalid service event, which triggers epoch fallback mode
+						updater.SetInvalidStateTransitionAttempted()
+						return dbUpdates, nil
+					}
 					return nil, irrecoverable.NewExceptionf("could not process epoch setup event: %w", err)
 				}
 
@@ -1149,6 +1154,11 @@ func (m *FollowerState) handleEpochServiceEvents(candidate *flow.Block, updater
 
 				err = updater.ProcessEpochCommit(ev)
 				if err != nil {
+					if protocol.IsInvalidServiceEventError(err) {
+						// we have observed an invalid service event, which triggers epoch fallback mode
+						updater.SetInvalidStateTransitionAttempted()
+						return dbUpdates, nil
+					}
 					return nil, irrecoverable.NewExceptionf("could not process epoch commit event: %w", err)
 				}
 

state/protocol/badger/mutator_test.go

@@ -1404,6 +1404,28 @@ func TestExtendEpochSetupInvalid(t *testing.T) {
 			assertEpochEmergencyFallbackTriggered(t, state, true)
 		})
 	})
+
+	t.Run("participants not ordered (EECC)", func(t *testing.T) {
+		util.RunWithFullProtocolState(t, rootSnapshot, func(db *badger.DB, state *protocol.ParticipantState) {
+			block1, createSetup := setupState(t, db, state)
+
+			_, receipt, seal := createSetup(func(setup *flow.EpochSetup) {
+				var err error
+				setup.Participants, err = setup.Participants.Shuffle()
+				require.NoError(t, err)
+			})
+
+			receiptBlock, sealingBlock := unittest.SealBlock(t, state, block1, receipt, seal)
+			err := state.Finalize(context.Background(), receiptBlock.ID())
+			require.NoError(t, err)
+			// epoch fallback not triggered before finalization
+			assertEpochEmergencyFallbackTriggered(t, state, false)
+			err = state.Finalize(context.Background(), sealingBlock.ID())
+			require.NoError(t, err)
+			// epoch fallback triggered after finalization
+			assertEpochEmergencyFallbackTriggered(t, state, true)
+		})
+	})
 }
 
 // TestExtendEpochCommitInvalid tests that incorporating an invalid EpochCommit

state/protocol/protocol_state.go

@@ -79,14 +79,16 @@ type StateUpdater interface {
 	// identities from previous+current epochs and start returning identities from current+next epochs.
 	// As a result of this operation protocol state for the next epoch will be created.
 	// CAUTION: Caller must validate input event.
-	// No errors are expected during normal operations.
+	// Expected errors during normal operations:
+	// - `protocol.InvalidServiceEventError` - an invalid service event with respect to the protocol state has been supplied.
 	ProcessEpochSetup(epochSetup *flow.EpochSetup) error
 	// ProcessEpochCommit updates current protocol state with data from epoch commit event.
 	// Observing an epoch setup commit, transitions protocol state from setup to commit phase, at this point we have
 	// finished construction of the next epoch.
 	// As a result of this operation protocol state for next epoch will be committed.
 	// CAUTION: Caller must validate input event.
-	// No errors are expected during normal operations.
+	// Expected errors during normal operations:
+	// - `protocol.InvalidServiceEventError` - an invalid service event with respect to the protocol state has been supplied.
 	ProcessEpochCommit(epochCommit *flow.EpochCommit) error
 	// UpdateIdentity updates identity table with new identity entry.
 	// Should pass identity which is already present in the table, otherwise an exception will be raised.

state/protocol/protocol_state/updater.go

@@ -58,16 +58,17 @@ func (u *Updater) Build() (updatedState *flow.ProtocolStateEntry, stateID flow.I
 //   - we stop returning identities from previous+current epochs and instead returning identities from current+next epochs.
 //
 // As a result of this operation protocol state for the next epoch will be created.
-// No errors are expected during normal operations.
+// Expected errors during normal operations:
+// - `protocol.InvalidServiceEventError` - an invalid service event with respect to the protocol state has been supplied.
 func (u *Updater) ProcessEpochSetup(epochSetup *flow.EpochSetup) error {
 	if epochSetup.Counter != u.parentState.CurrentEpochSetup.Counter+1 {
-		return fmt.Errorf("invalid epoch setup counter, expecting %d got %d", u.parentState.CurrentEpochSetup.Counter+1, epochSetup.Counter)
+		return protocol.NewInvalidServiceEventErrorf("invalid epoch setup counter, expecting %d got %d", u.parentState.CurrentEpochSetup.Counter+1, epochSetup.Counter)
 	}
 	if u.state.NextEpoch != nil {
-		return fmt.Errorf("protocol state has already a setup event")
+		return protocol.NewInvalidServiceEventErrorf("protocol state has already a setup event")
 	}
 	if u.state.InvalidStateTransitionAttempted {
-		return nil // won't process new events if we are in EECC
+		return nil // won't process new events if we are in epoch fallback mode.
 	}
 
 	// When observing setup event for subsequent epoch, construct the EpochStateContainer for `ProtocolStateEntry.NextEpoch`.
@@ -94,21 +95,19 @@ func (u *Updater) ProcessEpochSetup(epochSetup *flow.EpochSetup) error {
 
 	// For collector clusters, we rely on invariants (I) and (II) holding. See `committees.Cluster` for details, specifically function
 	// `constructInitialClusterIdentities(..)`. While the system smart contract must satisfy this invariant, we run a sanity check below.
-	// TODO: In case the invariant is violated (likely bug in system smart contracts), we should go into EECC and not reject the block containing the service event.
-	//
 	activeIdentitiesLookup := u.parentState.CurrentEpoch.ActiveIdentities.Lookup() // lookup NodeID → DynamicIdentityEntry for nodes _active_ in the current epoch
 	nextEpochActiveIdentities := make(flow.DynamicIdentityEntryList, 0, len(epochSetup.Participants))
 	prevNodeID := epochSetup.Participants[0].NodeID
 	for idx, nextEpochIdentitySkeleton := range epochSetup.Participants {
 		// sanity checking invariant (I):
 		currentEpochDynamicProperties, found := activeIdentitiesLookup[nextEpochIdentitySkeleton.NodeID]
 		if found && currentEpochDynamicProperties.Dynamic.Ejected { // invariance violated
-			return fmt.Errorf("node %v is ejected in current epoch %d but readmitted by EpochSetup event for epoch %d", nextEpochIdentitySkeleton.NodeID, u.parentState.CurrentEpochSetup.Counter, epochSetup.Counter)
+			return protocol.NewInvalidServiceEventErrorf("node %v is ejected in current epoch %d but readmitted by EpochSetup event for epoch %d", nextEpochIdentitySkeleton.NodeID, u.parentState.CurrentEpochSetup.Counter, epochSetup.Counter)
 		}
 
 		// sanity checking invariant (II):
 		if idx > 0 && !order.IdentifierCanonical(prevNodeID, nextEpochIdentitySkeleton.NodeID) {
-			return fmt.Errorf("epoch setup event lists active participants not in canonical ordering")
+			return protocol.NewInvalidServiceEventErrorf("epoch setup event lists active participants not in canonical ordering")
 		}
 		prevNodeID = nextEpochIdentitySkeleton.NodeID
 
@@ -138,19 +137,20 @@ func (u *Updater) ProcessEpochSetup(epochSetup *flow.EpochSetup) error {
 // Observing an epoch setup commit, transitions protocol state from setup to commit phase, at this point we have
 // finished construction of the next epoch.
 // As a result of this operation protocol state for next epoch will be committed.
-// No errors are expected during normal operations.
+// Expected errors during normal operations:
+// - `protocol.InvalidServiceEventError` - an invalid service event with respect to the protocol state has been supplied.
 func (u *Updater) ProcessEpochCommit(epochCommit *flow.EpochCommit) error {
 	if epochCommit.Counter != u.parentState.CurrentEpochSetup.Counter+1 {
-		return fmt.Errorf("invalid epoch commit counter, expecting %d got %d", u.parentState.CurrentEpochSetup.Counter+1, epochCommit.Counter)
+		return protocol.NewInvalidServiceEventErrorf("invalid epoch commit counter, expecting %d got %d", u.parentState.CurrentEpochSetup.Counter+1, epochCommit.Counter)
 	}
 	if u.state.NextEpoch == nil {
-		return fmt.Errorf("protocol state has been setup yet")
+		return protocol.NewInvalidServiceEventErrorf("protocol state has been setup yet")
 	}
 	if u.state.NextEpoch.CommitID != flow.ZeroID {
-		return fmt.Errorf("protocol state has already a commit event")
+		return protocol.NewInvalidServiceEventErrorf("protocol state has already a commit event")
 	}
 	if u.state.InvalidStateTransitionAttempted {
-		return nil // won't process new events if we are going to enter EECC
+		return nil // won't process new events if we are going to enter epoch fallback mode
 	}
 
 	u.state.NextEpoch.CommitID = epochCommit.ID()

state/protocol/protocol_state/updater_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/model/flow/filter"
 	"github.com/onflow/flow-go/model/flow/order"
+	"github.com/onflow/flow-go/state/protocol"
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
@@ -140,13 +141,15 @@ func (s *UpdaterSuite) TestProcessEpochCommit() {
 		})
 		err := s.updater.ProcessEpochCommit(commit)
 		require.Error(s.T(), err)
+		require.True(s.T(), protocol.IsInvalidServiceEventError(err))
 	})
 	s.Run("no next epoch protocol state", func() {
 		commit := unittest.EpochCommitFixture(func(commit *flow.EpochCommit) {
 			commit.Counter = s.parentProtocolState.CurrentEpochSetup.Counter + 1
 		})
 		err := s.updater.ProcessEpochCommit(commit)
 		require.Error(s.T(), err)
+		require.True(s.T(), protocol.IsInvalidServiceEventError(err))
 	})
 	s.Run("invalid state transition attempted", func() {
 		updater := NewUpdater(s.candidate, s.parentProtocolState)
@@ -189,6 +192,7 @@ func (s *UpdaterSuite) TestProcessEpochCommit() {
 		// processing another epoch commit has to be an error since we have already processed one
 		err = updater.ProcessEpochCommit(commit)
 		require.Error(s.T(), err)
+		require.True(s.T(), protocol.IsInvalidServiceEventError(err))
 
 		newState, _, _ := updater.Build()
 		require.Equal(s.T(), commit.ID(), newState.NextEpoch.CommitID, "next epoch must be committed")
@@ -267,6 +271,7 @@ func (s *UpdaterSuite) TestProcessEpochSetupInvariants() {
 		})
 		err := s.updater.ProcessEpochSetup(setup)
 		require.Error(s.T(), err)
+		require.True(s.T(), protocol.IsInvalidServiceEventError(err))
 	})
 	s.Run("invalid state transition attempted", func() {
 		updater := NewUpdater(s.candidate, s.parentProtocolState)
@@ -290,6 +295,36 @@ func (s *UpdaterSuite) TestProcessEpochSetupInvariants() {
 
 		err = updater.ProcessEpochSetup(setup)
 		require.Error(s.T(), err)
+		require.True(s.T(), protocol.IsInvalidServiceEventError(err))
+	})
+	s.Run("participants not sorted", func() {
+		updater := NewUpdater(s.candidate, s.parentProtocolState)
+		setup := unittest.EpochSetupFixture(func(setup *flow.EpochSetup) {
+			setup.Counter = s.parentProtocolState.CurrentEpochSetup.Counter + 1
+			var err error
+			setup.Participants, err = setup.Participants.Shuffle()
+			require.NoError(s.T(), err)
+		})
+		err := updater.ProcessEpochSetup(setup)
+		require.Error(s.T(), err)
+		require.True(s.T(), protocol.IsInvalidServiceEventError(err))
+	})
+	s.Run("epoch setup state conflicts with protocol state", func() {
+		conflictingIdentity := s.parentProtocolState.ProtocolStateEntry.CurrentEpoch.ActiveIdentities[0]
+		conflictingIdentity.Dynamic.Ejected = true
+
+		updater := NewUpdater(s.candidate, s.parentProtocolState)
+		setup := unittest.EpochSetupFixture(func(setup *flow.EpochSetup) {
+			setup.Counter = s.parentProtocolState.CurrentEpochSetup.Counter + 1
+			// using same identities as in previous epoch should result in an error since
+			// we have ejected conflicting identity but it was added back in epoch setup
+			// such epoch setup event is invalid.
+			setup.Participants = s.parentProtocolState.CurrentEpochSetup.Participants
+		})
+
+		err := updater.ProcessEpochSetup(setup)
+		require.Error(s.T(), err)
+		require.True(s.T(), protocol.IsInvalidServiceEventError(err))
 	})
 }