Merge pull request #6472 from onflow/leo/fix-root-block-creation

Remove internal partners from root block creation

cmd/bootstrap/cmd/rootblock.go

@@ -145,8 +145,13 @@ func rootBlock(cmd *cobra.Command, args []string) {
 		log.Fatal().Err(err).Msg("invalid epoch timing config")
 	}
 
+	// Read partner node's information and internal node's information.
+	// With "internal nodes" we reference nodes, whose private keys we have. In comparison,
+	// for "partner nodes" we generally do not have their keys. However, we allow some overlap,
+	// in that we tolerate a configuration where information about an "internal node" is also
+	// duplicated in the list of "partner nodes".
 	log.Info().Msg("collecting partner network and staking keys")
-	partnerNodes, err := common.ReadFullPartnerNodeInfos(log, flagPartnerWeights, flagPartnerNodeInfoDir)
+	rawPartnerNodes, err := common.ReadFullPartnerNodeInfos(log, flagPartnerWeights, flagPartnerNodeInfoDir)
 	if err != nil {
 		log.Fatal().Err(err).Msg("failed to read full partner node infos")
 	}
@@ -160,6 +165,14 @@ func rootBlock(cmd *cobra.Command, args []string) {
 
 	log.Info().Msg("")
 
+	// we now convert to the strict meaning of: "internal nodes" vs "partner nodes"
+	//  • "internal nodes" we have they private keys for
+	//  • "partner nodes" we don't have the keys for
+	//  • both sets are disjoint (no common nodes)
+	log.Info().Msg("remove internal partner nodes")
+	partnerNodes := common.FilterInternalPartners(rawPartnerNodes, internalNodes)
+	log.Info().Msgf("removed %d internal partner nodes", len(rawPartnerNodes)-len(partnerNodes))
+
 	log.Info().Msg("checking constraints on consensus nodes")
 	checkConstraints(partnerNodes, internalNodes)
 	log.Info().Msg("")

cmd/bootstrap/cmd/rootblock_test.go

@@ -21,7 +21,10 @@ const rootBlockHappyPathLogs = "collecting partner network and staking keys" +
 	`read \d+ internal private node-info files` +
 	`read internal node configurations` +
 	`read \d+ weights for internal nodes` +
+	`remove internal partner nodes` +
+	`removed 0 internal partner nodes` +
 	`checking constraints on consensus nodes` +
+
 	`assembling network and staking keys` +
 	`wrote file \S+/node-infos.pub.json` +
 	`running DKG for consensus nodes` +

cmd/bootstrap/run/qc.go

@@ -169,7 +169,7 @@ func GenerateQCParticipantData(allNodes, internalNodes []bootstrap.NodeInfo, dkg
 
 	// length of DKG participants needs to match stakingNodes, since we run DKG for external and internal validators
 	if len(allNodes) != len(dkgData.PrivKeyShares) {
-		return nil, fmt.Errorf("need exactly the same number of staking public keys as DKG private participants")
+		return nil, fmt.Errorf("need exactly the same number of staking public keys as DKG private participants, (all=%d, dkg=%d)", len(allNodes), len(dkgData.PrivKeyShares))
 	}
 
 	qcData := &ParticipantData{}

cmd/util/cmd/common/node_info.go

@@ -50,7 +50,7 @@ func ReadFullPartnerNodeInfos(log zerolog.Logger, partnerWeightsPath, partnerNod
 
 		weight := weights[partner.NodeID]
 		if valid := ValidateWeight(weight); !valid {
-			return nil, fmt.Errorf(fmt.Sprintf("invalid partner weight: %d", weight))
+			return nil, fmt.Errorf(fmt.Sprintf("invalid partner weight %v: %d", partner.NodeID, weight))
 		}
 
 		if weight != flow.DefaultInitialWeight {
@@ -148,7 +148,7 @@ func ReadFullInternalNodeInfos(log zerolog.Logger, internalNodePrivInfoDir, inte
 		weight := weights[internal.Address]
 
 		if valid := ValidateWeight(weight); !valid {
-			return nil, fmt.Errorf(fmt.Sprintf("invalid partner weight: %d", weight))
+			return nil, fmt.Errorf(fmt.Sprintf("invalid partner weight %v: %d", internal.NodeID, weight))
 		}
 		if weight != flow.DefaultInitialWeight {
 			log.Warn().Msgf("internal node (id=%x) has non-default weight (%d != %d)", internal.NodeID, weight, flow.DefaultInitialWeight)
@@ -224,3 +224,20 @@ func internalWeightsByAddress(log zerolog.Logger, config string) map[string]uint
 
 	return weights
 }
+
+// FilterInternalPartners returns the `partners`, dropping any entries that are also in `internal`
+// Formally, this function implements the set difference `partners \ internal`.
+func FilterInternalPartners(partners []bootstrap.NodeInfo, internal []bootstrap.NodeInfo) []bootstrap.NodeInfo {
+	lookup := make(map[flow.Identifier]struct{})
+	for _, node := range internal {
+		lookup[node.NodeID] = struct{}{}
+	}
+
+	var filtered []bootstrap.NodeInfo
+	for _, node := range partners {
+		if _, ok := lookup[node.NodeID]; !ok {
+			filtered = append(filtered, node)
+		}
+	}
+	return filtered
+}