remove internal collector nodes constraint

No longer require a minimum count of internal nodes; instead just report
whether enough internal nodes are present to vote for cluster QCs.

Author: tim-barry

cmd/bootstrap/cmd/clustering.go

@@ -120,7 +120,7 @@ func clusterAssignment(cmd *cobra.Command, args []string) {
 	}
 
 	log.Info().Msg("computing collection node clusters")
-	assignments, clusters, err := common.ConstructClusterAssignment(log, partnerList, internalList, int(flagCollectionClusters), clusteringPrg)
+	assignments, clusters, canConstructQCs, err := common.ConstructClusterAssignment(log, partnerList, internalList, int(flagCollectionClusters), clusteringPrg)
 	if err != nil {
 		log.Fatal().Err(err).Msg("unable to generate cluster assignment")
 	}
@@ -146,10 +146,10 @@ func clusterAssignment(cmd *cobra.Command, args []string) {
 	)
 	log.Info().Msg("")
 
-	if checkClusterConstraint(clusters, partnerNodes, internalNodes) {
-		log.Info().Msg("Enough votes for collection clusters are present - bootstrapping can continue with root block creation")
+	if canConstructQCs {
+		log.Info().Msg("enough votes for collection clusters are present - bootstrapping can continue with root block creation")
 	} else {
-		log.Info().Msg("Not enough internal votes to generate cluster QCs, need partner votes before root block creation")
+		log.Info().Msg("not enough internal votes to generate cluster QCs, need partner votes before root block creation")
 	}
 }
 

cmd/bootstrap/cmd/constraints.go

@@ -36,26 +36,3 @@ func checkConstraints(partnerNodes, internalNodes []model.NodeInfo) {
 
 	ensureUniformNodeWeightsPerRole(all)
 }
-
-// Check the ratio of internal/partner nodes in each cluster. The identities
-// in each cluster do not matter for this check.
-// Internal nodes must comprise >1/3 of each collector cluster.
-func checkClusterConstraint(clusters flow.ClusterList, partnersInfo []model.NodeInfo, internalsInfo []model.NodeInfo) bool {
-	partners := model.ToIdentityList(partnersInfo)
-	internals := model.ToIdentityList(internalsInfo)
-	for _, cluster := range clusters {
-		var clusterPartnerCount, clusterInternalCount int
-		for _, node := range cluster {
-			if _, exists := partners.ByNodeID(node.NodeID); exists {
-				clusterPartnerCount++
-			}
-			if _, exists := internals.ByNodeID(node.NodeID); exists {
-				clusterInternalCount++
-			}
-		}
-		if clusterInternalCount*2 <= clusterPartnerCount {
-			return false
-		}
-	}
-	return true
-}

cmd/bootstrap/cmd/finalize_test.go

@@ -137,15 +137,16 @@ func TestClusterAssignment(t *testing.T) {
 
 	log := zerolog.Nop()
 	// should not error
-	_, clusters, err := common.ConstructClusterAssignment(log, model.ToIdentityList(partners), model.ToIdentityList(internals), int(flagCollectionClusters), prng)
+	_, _, canConstructQCs, err := common.ConstructClusterAssignment(log, model.ToIdentityList(partners), model.ToIdentityList(internals), int(flagCollectionClusters), prng)
 	require.NoError(t, err)
-	require.True(t, checkClusterConstraint(clusters, partners, internals))
+	require.True(t, canConstructQCs)
 
 	// unhappy Path
 	internals = internals[:len(internals)-1] // reduce one internal node
-	// should error
-	_, _, err = common.ConstructClusterAssignment(log, model.ToIdentityList(partners), model.ToIdentityList(internals), int(flagCollectionClusters), prng)
-	require.Error(t, err)
+	// should no longer be able to construct QCs using only votes from internal nodes
+	_, _, canConstructQCs, err = common.ConstructClusterAssignment(log, model.ToIdentityList(partners), model.ToIdentityList(internals), int(flagCollectionClusters), prng)
+	require.NoError(t, err)
+	require.False(t, canConstructQCs)
 	// revert the flag value
 	flagCollectionClusters = tmp
 }

cmd/bootstrap/run/epochs.go

@@ -181,7 +181,7 @@ func GenerateRecoverTxArgsWithDKG(
 		return nil, fmt.Errorf("could not initialize PRNG: %w", err)
 	}
 	log.Info().Msgf("partitioning %d partners + %d internal nodes into %d collector clusters", len(partnerCollectors), len(internalCollectors), collectionClusters)
-	assignments, clusters, err := common.ConstructClusterAssignment(log, partnerCollectors, internalCollectors, collectionClusters, rng)
+	assignments, clusters, _, err := common.ConstructClusterAssignment(log, partnerCollectors, internalCollectors, collectionClusters, rng)
 	if err != nil {
 		return nil, fmt.Errorf("unable to generate cluster assignment: %w", err)
 	}

cmd/util/cmd/common/clusters.go

@@ -2,7 +2,6 @@ package common
 
 import (
 	"encoding/hex"
-	"errors"
 	"fmt"
 
 	"github.com/onflow/crypto/random"
@@ -40,8 +39,9 @@ import (
 // Returns:
 // - flow.AssignmentList: the generated assignment list.
 // - flow.ClusterList: the generate collection cluster list.
+// - bool: whether all cluster QCs can be created with only votes from internal nodes.
 // - error: if any error occurs. Any error returned from this function is irrecoverable.
-func ConstructClusterAssignment(log zerolog.Logger, partnerNodes, internalNodes flow.IdentityList, numCollectionClusters int, randomSource random.Rand) (flow.AssignmentList, flow.ClusterList, error) {
+func ConstructClusterAssignment(log zerolog.Logger, partnerNodes, internalNodes flow.IdentityList, numCollectionClusters int, randomSource random.Rand) (flow.AssignmentList, flow.ClusterList, bool, error) {
 
 	partnerCollectors := partnerNodes.Filter(filter.HasRole[flow.Identity](flow.RoleCollection))
 	internalCollectors := internalNodes.Filter(filter.HasRole[flow.Identity](flow.RoleCollection))
@@ -78,7 +78,7 @@ func ConstructClusterAssignment(log zerolog.Logger, partnerNodes, internalNodes
 	// first, round-robin internal nodes into each cluster
 	for i, node := range internalCollectors {
 		if node.InitialWeight != refWeight {
-			return nil, nil, fmt.Errorf("current implementation requires all collectors (partner & interal nodes) to have equal weight")
+			return nil, nil, false, fmt.Errorf("current implementation requires all collectors (partner & interal nodes) to have equal weight")
 		}
 		clusterIndex := i % numCollectionClusters
 		identifierLists[clusterIndex] = append(identifierLists[clusterIndex], node.NodeID)
@@ -88,17 +88,18 @@ func ConstructClusterAssignment(log zerolog.Logger, partnerNodes, internalNodes
 	// next, round-robin partner nodes into each cluster
 	for i, node := range partnerCollectors {
 		if node.InitialWeight != refWeight {
-			return nil, nil, fmt.Errorf("current implementation requires all collectors (partner & interal nodes) to have equal weight")
+			return nil, nil, false, fmt.Errorf("current implementation requires all collectors (partner & interal nodes) to have equal weight")
 		}
 		clusterIndex := i % numCollectionClusters
 		identifierLists[clusterIndex] = append(identifierLists[clusterIndex], node.NodeID)
 		constraint[clusterIndex] -= 1
 	}
 
 	// check the 2/3 constraint: for every cluster `i`, constraint[i] must be strictly positive
+	canConstructAllClusterQCs := true
 	for i := 0; i < numCollectionClusters; i++ {
 		if constraint[i] <= 0 {
-			return nil, nil, errors.New("there isn't enough internal nodes to have at least 1/3 internal nodes in each cluster")
+			canConstructAllClusterQCs = false
 		}
 	}
 
@@ -110,7 +111,7 @@ func ConstructClusterAssignment(log zerolog.Logger, partnerNodes, internalNodes
 		log.Fatal().Err(err).Msg("could not create cluster list")
 	}
 
-	return assignments, clusters, nil
+	return assignments, clusters, canConstructAllClusterQCs, nil
 }
 
 // ConvertClusterAssignmentsCdc converts golang cluster assignments type to Cadence type `[[String]]`.