• extended documentation with canonical ordering conventions

• minor optimization avoiding repetitive memory allocation during slice append
• minor code simplifications

model/convert/service_event.go

@@ -5,14 +5,12 @@ import (
 	"fmt"
 
 	"github.com/coreos/go-semver/semver"
-
 	"github.com/onflow/cadence"
 	"github.com/onflow/cadence/encoding/ccf"
 
 	"github.com/onflow/flow-go/crypto"
 	"github.com/onflow/flow-go/fvm/systemcontracts"
 	"github.com/onflow/flow-go/model/flow"
-	"github.com/onflow/flow-go/model/flow/assignment"
 	"github.com/onflow/flow-go/model/flow/order"
 )
 
@@ -41,8 +39,10 @@ func ServiceEvent(chainID flow.ChainID, event flow.Event) (*flow.ServiceEvent, e
 
 // convertServiceEventEpochSetup converts a service event encoded as the generic
 // flow.Event type to a ServiceEvent type for an EpochSetup event
+// CONVENTION: in the returned `EpochSetup` event,
+//   - Node identities listed in `EpochSetup.Participants` are in CANONICAL ORDER
+//   - for each cluster assignment (i.e. element in `EpochSetup.Assignments`), the nodeIDs are listed in CANONICAL ORDER
 func convertServiceEventEpochSetup(event flow.Event) (*flow.ServiceEvent, error) {
-
 	// decode bytes using ccf
 	payload, err := ccf.Decode(nil, event.Payload)
 	if err != nil {
@@ -219,13 +219,13 @@ func convertServiceEventEpochSetup(event flow.Event) (*flow.ServiceEvent, error)
 		)
 	}
 
-	// parse cluster assignments
+	// parse cluster assignments; returned assignments are in canonical order
 	setup.Assignments, err = convertClusterAssignments(cdcClusters.Values)
 	if err != nil {
 		return nil, fmt.Errorf("could not convert cluster assignments: %w", err)
 	}
 
-	// parse epoch participants
+	// parse epoch participants; returned node identities are in canonical order
 	setup.Participants, err = convertParticipants(cdcParticipants.Values)
 	if err != nil {
 		return nil, fmt.Errorf("could not convert participants: %w", err)
@@ -243,7 +243,6 @@ func convertServiceEventEpochSetup(event flow.Event) (*flow.ServiceEvent, error)
 // convertServiceEventEpochCommit converts a service event encoded as the generic
 // flow.Event type to a ServiceEvent type for an EpochCommit event
 func convertServiceEventEpochCommit(event flow.Event) (*flow.ServiceEvent, error) {
-
 	// decode bytes using ccf
 	payload, err := ccf.Decode(nil, event.Payload)
 	if err != nil {
@@ -353,15 +352,14 @@ func convertServiceEventEpochCommit(event flow.Event) (*flow.ServiceEvent, error
 // convertClusterAssignments converts the Cadence representation of cluster
 // assignments included in the EpochSetup into the protocol AssignmentList
 // representation.
+// CONVENTION: for each cluster assignment (i.e. element in `AssignmentList`), the nodeIDs are listed in CANONICAL ORDER
 func convertClusterAssignments(cdcClusters []cadence.Value) (flow.AssignmentList, error) {
-
 	// ensure we don't have duplicate cluster indices
 	indices := make(map[uint]struct{})
 
 	// parse cluster assignments to Go types
-	identifierLists := make([]flow.IdentifierList, len(cdcClusters))
+	clusterAssignments := make([]flow.IdentifierList, len(cdcClusters))
 	for _, value := range cdcClusters {
-
 		cdcCluster, ok := value.(cadence.Struct)
 		if !ok {
 			return nil, invalidCadenceTypeError("cluster", cdcCluster, cadence.Struct{})
@@ -435,8 +433,8 @@ func convertClusterAssignments(cdcClusters []cadence.Value) (flow.AssignmentList
 		}
 
 		// read weights to retrieve node IDs of cdcCluster members
+		clusterMembers := make(flow.IdentifierList, 0, len(weightsByNodeID.Pairs))
 		for _, pair := range weightsByNodeID.Pairs {
-
 			nodeIDString, ok := pair.Key.(cadence.String)
 			if !ok {
 				return nil, invalidCadenceTypeError(
@@ -452,26 +450,25 @@ func convertClusterAssignments(cdcClusters []cadence.Value) (flow.AssignmentList
 					err,
 				)
 			}
-
-			identifierLists[clusterIndex] = append(identifierLists[clusterIndex], nodeID)
+			clusterMembers = append(clusterMembers, nodeID)
 		}
-	}
 
-	// sort identifier lists in Canonical order
-	assignments := assignment.FromIdentifierLists(identifierLists)
+		// IMPORTANT: for each cluster, node IDs must be in *canonical order*
+		clusterAssignments[clusterIndex] = clusterMembers.Sort(order.IdentifierCanonical)
+	}
 
-	return assignments, nil
+	return clusterAssignments, nil
 }
 
 // convertParticipants converts the network participants specified in the
 // EpochSetup event into an IdentityList.
+// CONVENTION: returned IdentityList is in CANONICAL ORDER
 func convertParticipants(cdcParticipants []cadence.Value) (flow.IdentityList, error) {
-
 	participants := make(flow.IdentityList, 0, len(cdcParticipants))
 	var err error
 
 	for _, value := range cdcParticipants {
-
+		// checking compliance with expected format
 		cdcNodeInfoStruct, ok := value.(cadence.Struct)
 		if !ok {
 			return nil, invalidCadenceTypeError(
@@ -480,7 +477,6 @@ func convertParticipants(cdcParticipants []cadence.Value) (flow.IdentityList, er
 				cadence.Struct{},
 			)
 		}
-
 		const expectedFieldCount = 14
 		if len(cdcNodeInfoStruct.Fields) < expectedFieldCount {
 			return nil, fmt.Errorf(
@@ -489,7 +485,6 @@ func convertParticipants(cdcParticipants []cadence.Value) (flow.IdentityList, er
 				expectedFieldCount,
 			)
 		}
-
 		if cdcNodeInfoStruct.Type() == nil {
 			return nil, fmt.Errorf("nodeInfo struct doesn't have type")
 		}
@@ -640,6 +635,7 @@ func convertParticipants(cdcParticipants []cadence.Value) (flow.IdentityList, er
 		participants = append(participants, identity)
 	}
 
+	// IMPORTANT: returned identities must be in *canonical order*
 	participants = participants.Sort(order.Canonical)
 	return participants, nil
 }

model/flow/assignment/sort.go

@@ -11,7 +11,7 @@ func FromIdentifierLists(identifierLists []flow.IdentifierList) flow.AssignmentL
 	assignments := make(flow.AssignmentList, 0, len(identifierLists))
 	// in place sort to order the assignment in canonical order
 	for _, identities := range identifierLists {
-		assignment := flow.IdentifierList(identities).Sort(order.IdentifierCanonical)
+		assignment := identities.Sort(order.IdentifierCanonical)
 		assignments = append(assignments, assignment)
 	}
 	return assignments

model/flow/epoch.go

@@ -69,8 +69,8 @@ type EpochSetup struct {
 	DKGPhase2FinalView uint64         // the final view of DKG phase 2
 	DKGPhase3FinalView uint64         // the final view of DKG phase 3
 	FinalView          uint64         // the final view of the epoch
-	Participants       IdentityList   // all participants of the epoch
-	Assignments        AssignmentList // cluster assignment for the epoch
+	Participants       IdentityList   // all participants of the epoch in canonical order
+	Assignments        AssignmentList // cluster assignment for the epoch with node IDs for each cluster in canonical order
 	RandomSource       []byte         // source of randomness for epoch-specific setup tasks
 }
 

model/flow/identity.go

@@ -9,12 +9,11 @@ import (
 	"regexp"
 	"strconv"
 
-	"golang.org/x/exp/slices"
-
 	"github.com/ethereum/go-ethereum/rlp"
 	"github.com/fxamacker/cbor/v2"
 	"github.com/pkg/errors"
 	"github.com/vmihailenco/msgpack"
+	"golang.org/x/exp/slices"
 
 	"github.com/onflow/flow-go/crypto"
 	"github.com/onflow/flow-go/utils/rand"
@@ -27,7 +26,7 @@ const DefaultInitialWeight = 100
 // rxid is the regex for parsing node identity entries.
 var rxid = regexp.MustCompile(`^(collection|consensus|execution|verification|access)-([0-9a-fA-F]{64})@([\w\d]+|[\w\d][\w\d\-]*[\w\d](?:\.*[\w\d][\w\d\-]*[\w\d])*|[\w\d][\w\d\-]*[\w\d])(:[\d]+)?=(\d{1,20})$`)
 
-// IdentitySkeleton represents the static part of public identity of one network participant (node).
+// IdentitySkeleton represents the static part of a network participant's (i.e. node's) public identity.
 type IdentitySkeleton struct {
 	// NodeID uniquely identifies a particular node. A node's ID is fixed for
 	// the duration of that node's participation in the network.