From 23d1cdf1dd1ac2b68afe7249ee37b7cc929ea40b Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Tue, 17 Sep 2024 11:03:31 -0700 Subject: [PATCH 1/4] remove internal partners from root block creation --- cmd/bootstrap/cmd/rootblock.go | 6 +++++- cmd/bootstrap/cmd/rootblock_test.go | 3 +++ cmd/util/cmd/common/node_info.go | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/cmd/bootstrap/cmd/rootblock.go b/cmd/bootstrap/cmd/rootblock.go index 505d3239024..7d9f612ee10 100644 --- a/cmd/bootstrap/cmd/rootblock.go +++ b/cmd/bootstrap/cmd/rootblock.go @@ -146,7 +146,7 @@ func rootBlock(cmd *cobra.Command, args []string) { } 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 +160,10 @@ func rootBlock(cmd *cobra.Command, args []string) { log.Info().Msg("") + 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("") diff --git a/cmd/bootstrap/cmd/rootblock_test.go b/cmd/bootstrap/cmd/rootblock_test.go index 01222c0c476..8c3080d0b11 100644 --- a/cmd/bootstrap/cmd/rootblock_test.go +++ b/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` + diff --git a/cmd/util/cmd/common/node_info.go b/cmd/util/cmd/common/node_info.go index 061741d0955..5f9ebb79cd4 100644 --- a/cmd/util/cmd/common/node_info.go +++ b/cmd/util/cmd/common/node_info.go @@ -224,3 +224,19 @@ func internalWeightsByAddress(log zerolog.Logger, config string) map[string]uint return weights } + +// Reject any partner nodes that are in the internal node list. +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 +} From e87b027582805f8d3ff25260d69d51143bd514c9 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Tue, 17 Sep 2024 13:06:27 -0700 Subject: [PATCH 2/4] print nodeID when missing partner info --- cmd/util/cmd/common/node_info.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/util/cmd/common/node_info.go b/cmd/util/cmd/common/node_info.go index 5f9ebb79cd4..6f2a93a4917 100644 --- a/cmd/util/cmd/common/node_info.go +++ b/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) From 5ce965a83e0714da509a247b0f76d0bed306c2ad Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Wed, 18 Sep 2024 10:05:16 -0700 Subject: [PATCH 3/4] include mismatch number in error message --- cmd/bootstrap/run/qc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/bootstrap/run/qc.go b/cmd/bootstrap/run/qc.go index 4a31a125ff3..6f1c4dcdf29 100644 --- a/cmd/bootstrap/run/qc.go +++ b/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{} From ceb3ee2cb68ee5f6f86a8f76df9761dcec4e700c Mon Sep 17 00:00:00 2001 From: Leo Zhang Date: Thu, 19 Sep 2024 09:19:09 -0700 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Alexander Hentschel --- cmd/bootstrap/cmd/rootblock.go | 9 +++++++++ cmd/util/cmd/common/node_info.go | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/cmd/bootstrap/cmd/rootblock.go b/cmd/bootstrap/cmd/rootblock.go index 7d9f612ee10..d8e5143f477 100644 --- a/cmd/bootstrap/cmd/rootblock.go +++ b/cmd/bootstrap/cmd/rootblock.go @@ -145,6 +145,11 @@ 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") rawPartnerNodes, err := common.ReadFullPartnerNodeInfos(log, flagPartnerWeights, flagPartnerNodeInfoDir) if err != nil { @@ -160,6 +165,10 @@ 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)) diff --git a/cmd/util/cmd/common/node_info.go b/cmd/util/cmd/common/node_info.go index 6f2a93a4917..9263e5c96a3 100644 --- a/cmd/util/cmd/common/node_info.go +++ b/cmd/util/cmd/common/node_info.go @@ -225,7 +225,8 @@ func internalWeightsByAddress(log zerolog.Logger, config string) map[string]uint return weights } -// Reject any partner nodes that are in the internal node list. +// 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 {