-
Notifications
You must be signed in to change notification settings - Fork 197
Add cluster bootstrapping commands #8013
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/collector-decentralization-bootstrapping
Are you sure you want to change the base?
Add cluster bootstrapping commands #8013
Conversation
Creation of cluster assignment moved from `rootblock.go` to `clustering.go`.
The new commands are to be used by collector nodes. Following creation of the cluster assignment by `cmd/bootstrap/run/clustering.go`, Collector nodes retrieve the assignment, create a vote for the root block of the collection cluster they are assigned to, and upload that vote. Once enough votes have been received to construct QCs for all the cluster root blocks, bootstrapping can continue to the next step (the consensus root block).
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good.
Could you make sure to also update:
- the Benchnet2 bootstrapping logic
- the Localnet bootstrapping logic
- the bootstrap CLI README
- the general documentation should be updated to include the new command
- the examples there should be updated to include the new command
@@ -0,0 +1,187 @@ | |||
package cmd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bootstrap CLI readme has some general documentation about this process. It also has some example commands which can be used to test the full bootstrapping flow. Could you update these example commands and the relevant documentation in the README?
cmd.MarkFlagRequired(rootBlockCmd, "root-height") | ||
cmd.MarkFlagRequired(rootBlockCmd, "root-view") | ||
cmd.MarkFlagRequired(rootBlockCmd, "random-seed") | ||
cmd.MarkFlagRequired(rootBlockCmd, "intermediary-clustering-data") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is fine to make this non-required and set the default value to the default write path from the prior command
return nil | ||
} | ||
|
||
// readIntermediaryBootstrappingData reads intermediary clustering data file from disk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// readIntermediaryBootstrappingData reads intermediary clustering data file from disk. | |
// readIntermediaryClusteringData reads intermediary clustering data file from disk. |
} | ||
} | ||
if !found { | ||
log.Warn().Msgf("ignoring vote for block %v from signerID %v not part of the assignment", vote.BlockID, vote.SignerID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log.Warn().Msgf("ignoring vote for block %v from signerID %v not part of the assignment", vote.BlockID, vote.SignerID) | |
log.Fatal().Msgf("Halting because found vote for block %v from signerID %v not part of the assignment", vote.BlockID, vote.SignerID) |
I would suggest failing more loudly here. We don't expect this to happen in normal circumstances, so I think it's reasonable to force an operator to investigate. They will be able to get past the error by removing/moving the problematic file.
} | ||
|
||
// readClusterBlockVotes reads votes for root cluster blocks. | ||
// It sorts the votes into the appropriate clusters according to the given assignment list. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// It sorts the votes into the appropriate clusters according to the given assignment list. | |
// It sorts the votes into the appropriate clusters according to the given assignment list. | |
// The returned list of votes is in cluster index order (first list of votes is for cluster 0, etc.) |
|
||
var pullClusteringCmd = &cobra.Command{ | ||
Use: "pull-clustering", | ||
Short: "Pull epoch clustering", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Short: "Pull epoch clustering", | |
Short: "Pull clustering assignment for the first epoch of this spork. This is used to generate a root cluster block vote for this node's assigned cluster.", |
Adds the new bootstrapping command
clustering.go
to generate the collector clusters for the first epoch.Adds new transit script commands for collector nodes to vote on their cluster's root block.
Updates existing tests to use the new commands so that they can finish successfully.
The constraint that 2/3rds of collector nodes must be internal has not yet been removed.
Closes: #7846, #7847, #7848