diff --git a/state/protocol/badger/mutator.go b/state/protocol/badger/mutator.go index 2f24168fea6..46f90d0bf2f 100644 --- a/state/protocol/badger/mutator.go +++ b/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) } diff --git a/state/protocol/badger/mutator_test.go b/state/protocol/badger/mutator_test.go index 71b2fcbdde6..17209205c7d 100644 --- a/state/protocol/badger/mutator_test.go +++ b/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 diff --git a/state/protocol/protocol_state.go b/state/protocol/protocol_state.go index 2a6a7ad0530..d1cdb9a19a2 100644 --- a/state/protocol/protocol_state.go +++ b/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. diff --git a/state/protocol/protocol_state/updater.go b/state/protocol/protocol_state/updater.go index d69a4edda1b..6329919ee5d 100644 --- a/state/protocol/protocol_state/updater.go +++ b/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,8 +95,6 @@ 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 @@ -103,12 +102,12 @@ func (u *Updater) ProcessEpochSetup(epochSetup *flow.EpochSetup) error { // 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() diff --git a/state/protocol/protocol_state/updater_test.go b/state/protocol/protocol_state/updater_test.go index 97c0de05c05..5ea301d66d0 100644 --- a/state/protocol/protocol_state/updater_test.go +++ b/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,6 +141,7 @@ 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) { @@ -147,6 +149,7 @@ func (s *UpdaterSuite) TestProcessEpochCommit() { }) 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)) }) }