From c9087fc2aad07f619df41d419bea033b61c33dbe Mon Sep 17 00:00:00 2001 From: Alexander Hentschel Date: Mon, 21 Aug 2023 16:50:17 -0700 Subject: [PATCH] =?UTF-8?q?=E2=80=A2=20extended=20documentation=20with=20c?= =?UTF-8?q?anonical=20ordering=20conventions=20=E2=80=A2=20minor=20optimiz?= =?UTF-8?q?ation=20avoiding=20repetitive=20memory=20allocation=20during=20?= =?UTF-8?q?slice=20append=20=E2=80=A2=20minor=20code=20simplifications?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- model/convert/service_event.go | 36 +++++++++++++++------------------- model/flow/assignment/sort.go | 2 +- model/flow/epoch.go | 4 ++-- model/flow/identity.go | 5 ++--- 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/model/convert/service_event.go b/model/convert/service_event.go index 4db354eaf24..d91316d755c 100644 --- a/model/convert/service_event.go +++ b/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 } diff --git a/model/flow/assignment/sort.go b/model/flow/assignment/sort.go index 3b135d91152..a942c559489 100644 --- a/model/flow/assignment/sort.go +++ b/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 diff --git a/model/flow/epoch.go b/model/flow/epoch.go index 3f27586f2a2..6d075ff4826 100644 --- a/model/flow/epoch.go +++ b/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 } diff --git a/model/flow/identity.go b/model/flow/identity.go index 2dc3d4dccd3..67999343df6 100644 --- a/model/flow/identity.go +++ b/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.