[Dynamic Protocol State] EpochStateContainer stores epoch active identities

consensus/integration/epoch_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/onflow/flow-go/model/encodable"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/model/flow/filter"
-	"github.com/onflow/flow-go/model/flow/mapfunc"
 	"github.com/onflow/flow-go/model/flow/order"
 	"github.com/onflow/flow-go/state/protocol"
 	"github.com/onflow/flow-go/state/protocol/inmem"
@@ -220,7 +219,6 @@ func withNextEpoch(
 	encodableSnapshot := snapshot.Encodable()
 
 	currEpoch := &encodableSnapshot.Epochs.Current // take pointer so assignments apply
-	currentEpochIdentities, _ := snapshot.Identities(filter.Any)
 	nextEpochIdentities = nextEpochIdentities.Sort(order.Canonical[flow.Identity])
 
 	currEpoch.FinalView = currEpoch.FirstView + curEpochViews - 1 // first epoch lasts curEpochViews
@@ -249,27 +247,6 @@ func withNextEpoch(
 	// update protocol state
 	protocolState := encodableSnapshot.ProtocolState
 
-	// set identities for root snapshot to include next epoch identities,
-	// since we are in committed phase
-	protocolState.CurrentEpoch.ActiveIdentities = flow.DynamicIdentityEntryListFromIdentities(
-		append(
-			// all the current epoch identities
-			currentEpochIdentities,
-			// and all the NEW identities in next epoch, with 0 weight
-			nextEpochIdentities.
-				Filter(filter.Not(filter.In[flow.Identity](currentEpochIdentities))).
-				Map(mapfunc.WithWeight(0))...,
-		).Sort(order.Canonical[flow.Identity]))
-
-	nextEpochIdentities = append(
-		// all the next epoch identities
-		nextEpochIdentities,
-		// and all identities from current epoch, with 0 weight
-		currentEpochIdentities.
-			Filter(filter.Not(filter.In(nextEpochIdentities))).
-			Map(mapfunc.WithWeight(0))...,
-	).Sort(order.Canonical[flow.Identity])
-
 	// setup ID has changed, need to update it
 	convertedEpochSetup, _ := protocol.ToEpochSetup(inmem.NewEpoch(*currEpoch))
 	protocolState.CurrentEpoch.SetupID = convertedEpochSetup.ID()

model/flow/protocol_state.go

@@ -25,7 +25,7 @@ type DynamicIdentityEntryList []*DynamicIdentityEntry
 // TODO: https://github.com/onflow/flow-go/issues/4649
 type ProtocolStateEntry struct {
 	// Setup and commit event IDs for previous epoch.
-	PreviousEpochEventIDs EventIDs
+	PreviousEpoch EpochStateContainer
 	// Setup and commit event IDs for previous epoch. These EventIDs are ZeroID if
 	// and only if the current Epoch is the first epoch after a spork or genesis.
 	CurrentEpoch EpochStateContainer
@@ -148,16 +148,16 @@ func NewRichProtocolStateEntry(
 	}
 
 	// ensure data is consistent
-	if protocolState.PreviousEpochEventIDs.SetupID != ZeroID {
-		if protocolState.PreviousEpochEventIDs.SetupID != previousEpochSetup.ID() {
+	if protocolState.PreviousEpoch.SetupID != ZeroID {
+		if protocolState.PreviousEpoch.SetupID != previousEpochSetup.ID() {
 			return nil, fmt.Errorf("supplied previous epoch setup (%x) does not match protocol state (%x)",
 				previousEpochSetup.ID(),
-				protocolState.PreviousEpochEventIDs.SetupID)
+				protocolState.PreviousEpoch.SetupID)
 		}
-		if protocolState.PreviousEpochEventIDs.CommitID != previousEpochCommit.ID() {
+		if protocolState.PreviousEpoch.CommitID != previousEpochCommit.ID() {
 			return nil, fmt.Errorf("supplied previous epoch commit (%x) does not match protocol state (%x)",
 				previousEpochCommit.ID(),
-				protocolState.PreviousEpochEventIDs.CommitID)
+				protocolState.PreviousEpoch.CommitID)
 		}
 	}
 	if protocolState.CurrentEpoch.SetupID != currentEpochSetup.ID() {
@@ -193,6 +193,7 @@ func NewRichProtocolStateEntry(
 			protocolState.CurrentEpoch.ActiveIdentities,
 			currentEpochSetup.Participants,
 			nextEpochSetup.Participants,
+			nextEpoch.ActiveIdentities,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("could not build identity table for setup/commit phase: %w", err)
@@ -202,6 +203,7 @@ func NewRichProtocolStateEntry(
 			nextEpoch.ActiveIdentities,
 			nextEpochSetup.Participants,
 			currentEpochSetup.Participants,
+			protocolState.CurrentEpoch.ActiveIdentities,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("could not build next epoch identity table: %w", err)
@@ -217,6 +219,7 @@ func NewRichProtocolStateEntry(
 			protocolState.CurrentEpoch.ActiveIdentities,
 			currentEpochSetup.Participants,
 			previousEpochIdentities,
+			protocolState.PreviousEpoch.ActiveIdentities,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("could not build identity table for staking phase: %w", err)
@@ -232,12 +235,12 @@ func (e *ProtocolStateEntry) ID() Identifier {
 		return ZeroID
 	}
 	body := struct {
-		PreviousEpochEventIDs           EventIDs
+		PreviousEpochID                 Identifier
 		CurrentEpochID                  Identifier
 		NextEpochID                     Identifier
 		InvalidStateTransitionAttempted bool
 	}{
-		PreviousEpochEventIDs:           e.PreviousEpochEventIDs,
+		PreviousEpochID:                 e.PreviousEpoch.ID(),
 		CurrentEpochID:                  e.CurrentEpoch.ID(),
 		NextEpochID:                     e.NextEpoch.ID(),
 		InvalidStateTransitionAttempted: e.InvalidStateTransitionAttempted,
@@ -252,7 +255,7 @@ func (e *ProtocolStateEntry) Copy() *ProtocolStateEntry {
 		return nil
 	}
 	return &ProtocolStateEntry{
-		PreviousEpochEventIDs:           e.PreviousEpochEventIDs,
+		PreviousEpoch:                   e.PreviousEpoch,
 		CurrentEpoch:                    *e.CurrentEpoch.Copy(),
 		NextEpoch:                       e.NextEpoch.Copy(),
 		InvalidStateTransitionAttempted: e.InvalidStateTransitionAttempted,
@@ -282,7 +285,7 @@ func (e *RichProtocolStateEntry) Copy() *RichProtocolStateEntry {
 // EpochStatus returns epoch status for the current protocol state.
 func (e *ProtocolStateEntry) EpochStatus() *EpochStatus {
 	return &EpochStatus{
-		PreviousEpoch:                   e.PreviousEpochEventIDs,
+		PreviousEpoch:                   e.PreviousEpoch.EventIDs(),
 		CurrentEpoch:                    e.CurrentEpoch.EventIDs(),
 		NextEpoch:                       e.NextEpoch.EventIDs(),
 		InvalidServiceEventIncorporated: e.InvalidStateTransitionAttempted,
@@ -369,33 +372,30 @@ func BuildIdentityTable(
 	targetEpochDynamicIdentities DynamicIdentityEntryList,
 	targetEpochIdentitySkeletons IdentitySkeletonList,
 	adjacentEpochIdentitySkeletons IdentitySkeletonList,
+	adjacentEpochDynamicIdentities DynamicIdentityEntryList,
 ) (IdentityList, error) {
+
+	targetEpochParticipants, err := ReconstructIdentities(targetEpochIdentitySkeletons, targetEpochDynamicIdentities)
+	if err != nil {
+		return nil, fmt.Errorf("could not reconstruct participants for target epoch: %w", err)
+	}
+	adjacentEpochParticipants, err := ReconstructIdentities(adjacentEpochIdentitySkeletons, adjacentEpochDynamicIdentities)
+	if err != nil {
+		return nil, fmt.Errorf("could not reconstruct participants for adjacent epoch: %w", err)
+	}
+
 	// Combine the participants of the current and adjacent epoch. The method `GenericIdentityList.Union`
 	// already implements the following required conventions:
 	//  1. Preference for IdentitySkeleton of the target epoch:
 	//     In case an IdentitySkeleton with the same NodeID exists in the target epoch as well as
 	//     in the adjacent epoch, we use the IdentitySkeleton for the target epoch (for example,
 	//     to account for changes of keys, address, initial weight, etc).
 	//  2. Canonical ordering
-	allEpochParticipants := targetEpochIdentitySkeletons.Union(adjacentEpochIdentitySkeletons)
-	// sanity check: size of identities should be equal to previous and current epoch participants combined
-	if len(allEpochParticipants) != len(targetEpochDynamicIdentities) {
-		return nil, fmt.Errorf("invalid number of identities in protocol state: expected %d, got %d", len(allEpochParticipants), len(targetEpochDynamicIdentities))
-	}
-
-	// build full identity table for current epoch
-	var result IdentityList
-	for i, identity := range targetEpochDynamicIdentities {
-		// sanity check: identities should be sorted in canonical order
-		if identity.NodeID != allEpochParticipants[i].NodeID {
-			return nil, fmt.Errorf("identites in protocol state are not in canonical order: expected %s, got %s", allEpochParticipants[i].NodeID, identity.NodeID)
-		}
-		result = append(result, &Identity{
-			IdentitySkeleton: *allEpochParticipants[i],
-			DynamicIdentity:  identity.Dynamic,
-		})
-	}
-	return result, nil
+	allEpochParticipants := targetEpochParticipants.Union(adjacentEpochParticipants.Map(func(identity Identity) Identity {
+		identity.Weight = 0
+		return identity
+	}))
+	return allEpochParticipants, nil
 }
 
 // DynamicIdentityEntryListFromIdentities converts IdentityList to DynamicIdentityEntryList.
@@ -409,3 +409,26 @@ func DynamicIdentityEntryListFromIdentities(identities IdentityList) DynamicIden
 	}
 	return dynamicIdentities
 }
+
+// ReconstructIdentities combines identity skeletons and dynamic identities to produce a flow.IdentityList.
+// No errors are expected during normal operations.
+func ReconstructIdentities(skeletons IdentitySkeletonList, dynamics DynamicIdentityEntryList) (IdentityList, error) {
+	// sanity check: list of skeletons and dynamic should be the same
+	if len(skeletons) != len(dynamics) {
+		return nil, fmt.Errorf("invalid number of identities to reconstruct: expected %d, got %d", len(skeletons), len(dynamics))
+	}
+
+	// reconstruct identities from skeleton and dynamic parts
+	var result IdentityList
+	for i := range dynamics {
+		// sanity check: identities should be sorted in the same order
+		if dynamics[i].NodeID != skeletons[i].NodeID {
+			return nil, fmt.Errorf("identites in protocol state are not in canonical order: expected %s, got %s", skeletons[i].NodeID, dynamics[i].NodeID)
+		}
+		result = append(result, &Identity{
+			IdentitySkeleton: *skeletons[i],
+			DynamicIdentity:  dynamics[i].Dynamic,
+		})
+	}
+	return result, nil
+}

model/flow/protocol_state_test.go

@@ -34,7 +34,11 @@ func TestNewRichProtocolStateEntry(t *testing.T) {
 				CommitID:         currentEpochCommit.ID(),
 				ActiveIdentities: identities,
 			},
-			PreviousEpochEventIDs:           flow.EventIDs{},
+			PreviousEpoch: flow.EpochStateContainer{
+				SetupID:          flow.ZeroID,
+				CommitID:         flow.ZeroID,
+				ActiveIdentities: nil,
+			},
 			InvalidStateTransitionAttempted: false,
 		}
 		entry, err := flow.NewRichProtocolStateEntry(
@@ -47,7 +51,7 @@ func TestNewRichProtocolStateEntry(t *testing.T) {
 			nil,
 		)
 		assert.NoError(t, err)
-		expectedIdentities, err := flow.BuildIdentityTable(identities, setup.Participants, nil)
+		expectedIdentities, err := flow.BuildIdentityTable(identities, setup.Participants, nil, nil)
 		assert.NoError(t, err)
 		assert.Equal(t, expectedIdentities, entry.CurrentEpochIdentityTable, "should be equal to current epoch setup participants")
 	})
@@ -72,6 +76,7 @@ func TestNewRichProtocolStateEntry(t *testing.T) {
 			stateEntry.CurrentEpoch.ActiveIdentities,
 			stateEntry.CurrentEpochSetup.Participants,
 			stateEntry.PreviousEpochSetup.Participants,
+			stateEntry.PreviousEpoch.ActiveIdentities,
 		)
 		assert.NoError(t, err)
 		assert.Equal(t, expectedIdentities, richEntry.CurrentEpochIdentityTable, "should be equal to current epoch setup participants + previous epoch setup participants")
@@ -102,6 +107,7 @@ func TestNewRichProtocolStateEntry(t *testing.T) {
 			stateEntry.CurrentEpoch.ActiveIdentities,
 			stateEntry.CurrentEpochSetup.Participants,
 			stateEntry.NextEpochSetup.Participants,
+			stateEntry.NextEpoch.ActiveIdentities,
 		)
 		assert.NoError(t, err)
 		assert.Equal(t, expectedIdentities, richEntry.CurrentEpochIdentityTable, "should be equal to current epoch setup participants + next epoch setup participants")
@@ -110,6 +116,7 @@ func TestNewRichProtocolStateEntry(t *testing.T) {
 			stateEntry.NextEpoch.ActiveIdentities,
 			stateEntry.NextEpochSetup.Participants,
 			stateEntry.CurrentEpochSetup.Participants,
+			stateEntry.CurrentEpoch.ActiveIdentities,
 		)
 		assert.NoError(t, err)
 		assert.Equal(t, expectedIdentities, richEntry.NextEpochIdentityTable, "should be equal to next epoch setup participants + current epoch setup participants")
@@ -138,13 +145,15 @@ func TestNewRichProtocolStateEntry(t *testing.T) {
 			stateEntry.CurrentEpoch.ActiveIdentities,
 			stateEntry.CurrentEpochSetup.Participants,
 			stateEntry.NextEpochSetup.Participants,
+			stateEntry.NextEpoch.ActiveIdentities,
 		)
 		assert.NoError(t, err)
 		assert.Equal(t, expectedIdentities, richEntry.CurrentEpochIdentityTable, "should be equal to current epoch setup participants + next epoch setup participants")
 		expectedIdentities, err = flow.BuildIdentityTable(
 			stateEntry.NextEpoch.ActiveIdentities,
 			stateEntry.NextEpochSetup.Participants,
 			stateEntry.CurrentEpochSetup.Participants,
+			stateEntry.CurrentEpoch.ActiveIdentities,
 		)
 		assert.NoError(t, err)
 		assert.Equal(t, expectedIdentities, richEntry.NextEpochIdentityTable, "should be equal to next epoch setup participants + current epoch setup participants")
@@ -161,7 +170,7 @@ func TestProtocolStateEntry_Copy(t *testing.T) {
 	cpy := entry.Copy()
 	assert.Equal(t, entry, cpy)
 	assert.NotSame(t, entry.NextEpoch, cpy.NextEpoch)
-	assert.NotSame(t, entry.PreviousEpochEventIDs, cpy.PreviousEpochEventIDs)
+	assert.NotSame(t, entry.PreviousEpoch, cpy.PreviousEpoch)
 	assert.NotSame(t, entry.CurrentEpoch, cpy.CurrentEpoch)
 
 	cpy.InvalidStateTransitionAttempted = !entry.InvalidStateTransitionAttempted

state/protocol/inmem/convert.go

@@ -334,7 +334,11 @@ func SnapshotFromBootstrapStateWithParams(
 		})
 	}
 	protocolState := &flow.ProtocolStateEntry{
-		PreviousEpochEventIDs: flow.EventIDs{},
+		PreviousEpoch: flow.EpochStateContainer{
+			SetupID:          flow.ZeroID,
+			CommitID:         flow.ZeroID,
+			ActiveIdentities: nil,
+		},
 		CurrentEpoch: flow.EpochStateContainer{
 			SetupID:          setup.ID(),
 			CommitID:         commit.ID(),

state/protocol/inmem/dynamic_protocol_state_test.go

@@ -33,7 +33,7 @@ func TestDynamicProtocolStateAdapter(t *testing.T) {
 		entry := unittest.ProtocolStateFixture()
 		adapter := inmem.NewDynamicProtocolStateAdapter(entry, globalParams)
 		status := adapter.EpochStatus()
-		assert.Equal(t, entry.PreviousEpochEventIDs, status.PreviousEpoch)
+		assert.Equal(t, entry.PreviousEpoch.EventIDs(), status.PreviousEpoch)
 		assert.Equal(t, flow.EventIDs{
 			SetupID:  entry.CurrentEpoch.SetupID,
 			CommitID: entry.CurrentEpoch.CommitID,
@@ -48,7 +48,7 @@ func TestDynamicProtocolStateAdapter(t *testing.T) {
 
 		adapter := inmem.NewDynamicProtocolStateAdapter(entry, globalParams)
 		status := adapter.EpochStatus()
-		assert.Equal(t, entry.PreviousEpochEventIDs, status.PreviousEpoch)
+		assert.Equal(t, entry.PreviousEpoch.EventIDs(), status.PreviousEpoch)
 		assert.Equal(t, flow.EventIDs{
 			SetupID:  entry.CurrentEpoch.SetupID,
 			CommitID: entry.CurrentEpoch.CommitID,
@@ -63,7 +63,7 @@ func TestDynamicProtocolStateAdapter(t *testing.T) {
 		entry := unittest.ProtocolStateFixture(unittest.WithNextEpochProtocolState())
 		adapter := inmem.NewDynamicProtocolStateAdapter(entry, globalParams)
 		status := adapter.EpochStatus()
-		assert.Equal(t, entry.PreviousEpochEventIDs, status.PreviousEpoch)
+		assert.Equal(t, entry.PreviousEpoch.EventIDs(), status.PreviousEpoch)
 		assert.Equal(t, flow.EventIDs{
 			SetupID:  entry.CurrentEpoch.SetupID,
 			CommitID: entry.CurrentEpoch.CommitID,