Skip to content

Commit

Permalink
Merge pull request onflow#6484 from onflow/leo/fix-finalization
Browse files Browse the repository at this point in the history
[Bootstrap] Fix duplication detection in root block finalization
  • Loading branch information
zhangchiqing authored Oct 1, 2024
2 parents ec4ad1c + 6872106 commit 7d782c8
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 8 deletions.
7 changes: 5 additions & 2 deletions cmd/bootstrap/cmd/final_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,16 @@ func finalList(cmd *cobra.Command, args []string) {
registeredNodes := readStakingContractDetails()

// merge internal and partner node infos (from local files)
localNodes := mergeNodeInfos(internalNodes, partnerNodes)
localNodes, err := mergeNodeInfos(internalNodes, partnerNodes)
if err != nil {
log.Fatal().Err(err).Msg("failed to merge node infos")
}

// reconcile nodes from staking contract nodes
validateNodes(localNodes, registeredNodes)

// write node-config.json with the new list of nodes to be used for the `finalize` command
err := common.WriteJSON(model.PathFinallist, flagOutdir, model.ToPublicNodeInfoList(localNodes))
err = common.WriteJSON(model.PathFinallist, flagOutdir, model.ToPublicNodeInfoList(localNodes))
if err != nil {
log.Fatal().Err(err).Msg("failed to write json")
}
Expand Down
15 changes: 10 additions & 5 deletions cmd/bootstrap/cmd/finalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ func finalize(cmd *cobra.Command, args []string) {
log.Info().Msg("")

log.Info().Msg("assembling network and staking keys")
stakingNodes := mergeNodeInfos(internalNodes, partnerNodes)
stakingNodes, err := mergeNodeInfos(internalNodes, partnerNodes)
if err != nil {
log.Fatal().Err(err).Msgf("failed to merge internal and partner nodes: %v", err)
}
log.Info().Msg("")

// create flow.IdentityList representation of participant set
Expand Down Expand Up @@ -312,29 +315,31 @@ func readRootBlockVotes() []*hotstuff.Vote {
//
// IMPORTANT: node infos are returned in the canonical ordering, meaning this
// is safe to use as the input to the DKG and protocol state.
func mergeNodeInfos(internalNodes, partnerNodes []model.NodeInfo) []model.NodeInfo {
func mergeNodeInfos(internalNodes, partnerNodes []model.NodeInfo) ([]model.NodeInfo, error) {
nodes := append(internalNodes, partnerNodes...)

// test for duplicate Addresses
addressLookup := make(map[string]struct{})
for _, node := range nodes {
if _, ok := addressLookup[node.Address]; ok {
log.Fatal().Str("address", node.Address).Msg("duplicate node address")
return nil, fmt.Errorf("duplicate node address: %v", node.Address)
}
addressLookup[node.Address] = struct{}{}
}

// test for duplicate node IDs
idLookup := make(map[flow.Identifier]struct{})
for _, node := range nodes {
if _, ok := idLookup[node.NodeID]; ok {
log.Fatal().Str("NodeID", node.NodeID.String()).Msg("duplicate node ID")
return nil, fmt.Errorf("duplicate node ID: %v", node.NodeID.String())
}
idLookup[node.NodeID] = struct{}{}
}

// sort nodes using the canonical ordering
nodes = model.Sort(nodes, flow.Canonical[flow.Identity])

return nodes
return nodes, nil
}

// readRootBlock reads root block data from disc, this file needs to be prepared with
Expand Down
27 changes: 27 additions & 0 deletions cmd/bootstrap/cmd/finalize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,3 +190,30 @@ func checkClusterConstraint(clusters flow.ClusterList, partnersInfo []model.Node
}
return true
}

func TestMergeNodeInfos(t *testing.T) {
partnersLen := 7
internalLen := 22
partners := unittest.NodeInfosFixture(partnersLen, unittest.WithRole(flow.RoleCollection))
internals := unittest.NodeInfosFixture(internalLen, unittest.WithRole(flow.RoleCollection))

// Check if there is no overlap, then should pass
merged, err := mergeNodeInfos(partners, internals)
require.NoError(t, err)
require.Len(t, merged, partnersLen+internalLen)

// Check if internals and partners have overlap, then should fail
internalAndPartnersHaveOverlap := append(partners, internals[0])
_, err = mergeNodeInfos(internalAndPartnersHaveOverlap, internals)
require.Error(t, err)

// Check if partners have overlap, then should fail
partnersHaveOverlap := append(partners, partners[0])
_, err = mergeNodeInfos(partnersHaveOverlap, internals)
require.Error(t, err)

// Check if internals have overlap, then should fail
internalsHaveOverlap := append(internals, internals[0])
_, err = mergeNodeInfos(partners, internalsHaveOverlap)
require.Error(t, err)
}
5 changes: 4 additions & 1 deletion cmd/bootstrap/cmd/rootblock.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,10 @@ func rootBlock(cmd *cobra.Command, args []string) {
log.Info().Msg("")

log.Info().Msg("assembling network and staking keys")
stakingNodes := mergeNodeInfos(internalNodes, partnerNodes)
stakingNodes, err := mergeNodeInfos(internalNodes, partnerNodes)
if err != nil {
log.Fatal().Err(err).Msgf("failed to merge node infos")
}
err = common.WriteJSON(model.PathNodeInfosPub, flagOutdir, model.ToPublicNodeInfoList(stakingNodes))
if err != nil {
log.Fatal().Err(err).Msg("failed to write json")
Expand Down

0 comments on commit 7d782c8

Please sign in to comment.