Skip to content
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

tidy up usage and flags #149

Merged
merged 5 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions cmd/horcrux/cmd/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,23 @@ func addressCmd() *cobra.Command {

var pubKey crypto.PubKey

chainID := args[0]

switch config.Config.SignMode {
case signer.SignModeThreshold:
err := config.Config.ValidateThresholdModeConfig()
if err != nil {
return err
}

keyFile, err := config.KeyFileExistsCosigner(args[0])
keyFile, err := config.KeyFileExistsCosigner(chainID)
if err != nil {
return err
}

key, err := signer.LoadCosignerEd25519Key(keyFile)
if err != nil {
return fmt.Errorf("error reading cosigner key: %s", err)
return fmt.Errorf("error reading cosigner key: %w, check that key is present for chain ID: %s", err, chainID)
}

pubKey = key.PubKey
Expand All @@ -54,9 +56,9 @@ func addressCmd() *cobra.Command {
if err != nil {
return err
}
keyFile, err := config.KeyFileExistsSingleSigner(args[0])
keyFile, err := config.KeyFileExistsSingleSigner(chainID)
if err != nil {
return err
return fmt.Errorf("error reading priv-validator key: %w, check that key is present for chain ID: %s", err, chainID)
}

filePV := cometprivval.LoadFilePVEmptyState(keyFile, "")
Expand Down
58 changes: 38 additions & 20 deletions cmd/horcrux/cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const (
flagRaftTimeout = "raft-timeout"
flagGRPCTimeout = "grpc-timeout"
flagOverwrite = "overwrite"
flagBare = "bare"
)

func configCmd() *cobra.Command {
Expand All @@ -33,16 +34,17 @@ func configCmd() *cobra.Command {

func initCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "init [chain-nodes]",
Use: "init",
DavidNix marked this conversation as resolved.
Show resolved Hide resolved
Aliases: []string{"i"},
Short: "initialize configuration file and home directory if one doesn't already exist",
Long: "initialize configuration file, use flags for cosigner configuration.\n\n" +
"[chain-nodes] is a comma separated array of chain node addresses i.e.\n" +
"tcp://chain-node-1:1234,tcp://chain-node-2:1234",
Long: `initialize configuration file.
for threshold signer mode, --cosigner flags and --threshold flag are required.
`,
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) (err error) {
cmdFlags := cmd.Flags()

bare, _ := cmdFlags.GetBool(flagBare)
nodes, _ := cmdFlags.GetStringSlice(flagNode)

cn, err := signer.ChainNodesFromFlag(nodes)
Expand Down Expand Up @@ -89,8 +91,11 @@ func initCmd() *cobra.Command {
ChainNodes: cn,
DebugAddr: debugAddr,
}
if err = cfg.ValidateThresholdModeConfig(); err != nil {
return err

if !bare {
if err = cfg.ValidateThresholdModeConfig(); err != nil {
return err
}
}
} else {
// Single Signer Config
Expand All @@ -100,8 +105,10 @@ func initCmd() *cobra.Command {
ChainNodes: cn,
DebugAddr: debugAddr,
}
if err = cfg.ValidateSingleSignerConfig(); err != nil {
return err
if !bare {
if err = cfg.ValidateSingleSignerConfig(); err != nil {
return err
}
}
}

Expand All @@ -122,23 +129,34 @@ func initCmd() *cobra.Command {
return nil
},
}
cmd.Flags().StringP(flagSignMode, "m", string(signer.SignModeThreshold),
`sign mode, "threshold" (recommended) or "single" (unsupported). threshold mode requires --cosigners and --threshold`,

f := cmd.Flags()
f.StringP(flagSignMode, "m", string(signer.SignModeThreshold),
`sign mode, "threshold" (recommended) or "single" (unsupported). threshold mode requires --cosigner (multiple) and --threshold`, //nolint
)
cmd.Flags().StringSliceP(flagNode, "n", []string{}, "chain nodes in format tcp://{p2p-addr}:{port}")
cmd.Flags().StringSliceP(flagCosigner, "c", []string{},
"cosigners in format tcp://{p2p-addr}:{port}|{shard-id} \n"+
"(i.e. \"tcp://node-1:2222|1,tcp://node-2:2222|2,tcp://node-3:2222|3\")")
cmd.Flags().IntP(flagThreshold, "t", 0, "number of shards required for threshold signature")
cmd.Flags().StringP(
f.StringSliceP(flagNode, "n", []string{}, "chain nodes in format tcp://{node-addr}:{privval-port} \n"+
"(e.g. --node tcp://sentry-1:1234 --node tcp://sentry-2:1234 --node tcp://sentry-3:1234 )")
agouin marked this conversation as resolved.
Show resolved Hide resolved

f.StringSliceP(flagCosigner, "c", []string{},
`cosigners in format tcp://{cosigner-addr}:{p2p-port}
(e.g. --cosigner tcp://horcrux-1:2222 --cosigner tcp://horcrux-2:2222 --cosigner tcp://horcrux-3:2222)`)

f.IntP(flagThreshold, "t", 0, "number of shards required for threshold signature")

f.StringP(
flagDebugAddr, "d", "",
"listen address for debug server and prometheus metrics in format localhost:8543",
)
cmd.Flags().StringP(flagKeyDir, "k", "", "key directory if other than home directory")
cmd.Flags().String(flagRaftTimeout, "1500ms", "cosigner raft timeout value, \n"+
f.StringP(flagKeyDir, "k", "", "key directory if other than home directory")
f.String(flagRaftTimeout, "1500ms", "cosigner raft timeout value, \n"+
"accepts valid duration strings for Go's time.ParseDuration() e.g. 1s, 1000ms, 1.5m")
cmd.Flags().String(flagGRPCTimeout, "1500ms", "cosigner grpc timeout value, \n"+
f.String(flagGRPCTimeout, "1500ms", "cosigner grpc timeout value, \n"+
"accepts valid duration strings for Go's time.ParseDuration() e.g. 1s, 1000ms, 1.5m")
cmd.Flags().BoolP(flagOverwrite, "o", false, "overwrite an existing config.yaml")
f.BoolP(flagOverwrite, "o", false, "overwrite an existing config.yaml")
f.Bool(
flagBare,
false,
"allows initialization without providing any flags. If flags are provided, will not perform final validation",
)
return cmd
}
2 changes: 2 additions & 0 deletions cmd/horcrux/cmd/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ thresholdMode:
chainNodes:
- privValAddr: tcp://10.168.0.1:1234
- privValAddr: tcp://10.168.0.2:1234
debugAddr: ""
`,
},
{
Expand All @@ -60,6 +61,7 @@ chainNodes:
chainNodes:
- privValAddr: tcp://10.168.0.1:1234
- privValAddr: tcp://10.168.0.2:1234
debugAddr: ""
`,
},
{
Expand Down
34 changes: 19 additions & 15 deletions cmd/horcrux/cmd/shards.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,9 @@ func addOutputDirFlag(cmd *cobra.Command) {
cmd.Flags().StringP(flagOutputDir, "", "", "output directory")
}

func addShardsFlag(cmd *cobra.Command) {
func addTotalShardsFlag(cmd *cobra.Command) {
cmd.Flags().Uint8(flagShards, 0, "total key shards")
}

func addShardFlags(cmd *cobra.Command) {
addShardsFlag(cmd)
cmd.Flags().Uint8(flagThreshold, 0, "threshold number of shards required to successfully sign")
cmd.Flags().String(flagKeyFile, "", "priv_validator_key.json file to shard")
cmd.Flags().String(flagChainID, "", "key shards will sign for this chain ID")
_ = cmd.MarkFlagRequired(flagShards)
}

// createCosignerEd25519ShardsCmd is a cobra command for creating
Expand All @@ -84,19 +78,19 @@ func createCosignerEd25519ShardsCmd() *cobra.Command {
var errs []error

if keyFile == "" {
errs = append(errs, fmt.Errorf("key-file flag must be provided and non-empty"))
errs = append(errs, fmt.Errorf("key-file flag must not be empty"))
}

if chainID == "" {
errs = append(errs, fmt.Errorf("chain-id flag must be provided and non-empty"))
errs = append(errs, fmt.Errorf("chain-id flag must not be empty"))
}

if threshold == 0 {
errs = append(errs, fmt.Errorf("threshold flag must be provided and non-zero"))
errs = append(errs, fmt.Errorf("threshold flag must be > 0, <= --shards, and > --shards/2"))
}

if shards == 0 {
errs = append(errs, fmt.Errorf("shards flag must be provided and non-zero"))
errs = append(errs, fmt.Errorf("shards flag must be greater than zero"))
}

if _, err := os.Stat(keyFile); err != nil {
Expand Down Expand Up @@ -148,15 +142,25 @@ func createCosignerEd25519ShardsCmd() *cobra.Command {
return nil
},
}
addShardFlags(cmd)

addOutputDirFlag(cmd)
addTotalShardsFlag(cmd)

f := cmd.Flags()
f.Uint8(flagThreshold, 0, "threshold number of shards required to successfully sign")
_ = cmd.MarkFlagRequired(flagThreshold)
f.String(flagKeyFile, "", "priv_validator_key.json file to shard")
_ = cmd.MarkFlagRequired(flagKeyFile)
f.String(flagChainID, "", "key shards will sign for this chain ID")
_ = cmd.MarkFlagRequired(flagChainID)

return cmd
}

// createCosignerRSAShardsCmd is a cobra command for creating cosigner shards from a priv validator
func createCosignerRSAShardsCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "create-rsa-shards shards",
Use: "create-rsa-shards",
Args: cobra.NoArgs,
Short: "Create cosigner RSA shards",

Expand Down Expand Up @@ -196,7 +200,7 @@ func createCosignerRSAShardsCmd() *cobra.Command {
return nil
},
}
addShardsFlag(cmd)
addTotalShardsFlag(cmd)
addOutputDirFlag(cmd)
return cmd
}
4 changes: 2 additions & 2 deletions cmd/horcrux/cmd/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func showStateCmd() *cobra.Command {

func setStateCmd() *cobra.Command {
return &cobra.Command{
Use: "set [chain-id] [height]",
Use: "set chain-id height",
Aliases: []string{"s"},
Short: "Set the height for the sign state of a specific chain-id",
Args: cobra.ExactArgs(2),
Expand Down Expand Up @@ -136,7 +136,7 @@ func setStateCmd() *cobra.Command {

func importStateCmd() *cobra.Command {
return &cobra.Command{
Use: "import [chain-id]",
Use: "import chain-id",
Aliases: []string{"i"},
Short: "Read the old priv_validator_state.json and set the height, round and step" +
"(good for migrations but NOT shared state update)",
Expand Down
1 change: 1 addition & 0 deletions cmd/horcrux/cmd/testdata/config-migrated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ chainNodes:
- privValAddr: tcp://127.0.0.1:1234
- privValAddr: tcp://127.0.0.1:2345
- privValAddr: tcp://127.0.0.1:3456
debugAddr: ""
11 changes: 6 additions & 5 deletions signer/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ type Config struct {
PrivValKeyDir *string `yaml:"keyDir,omitempty"`
SignMode SignMode `yaml:"signMode"`
ThresholdModeConfig *ThresholdModeConfig `yaml:"thresholdMode,omitempty"`
ChainNodes ChainNodes `yaml:"chainNodes,omitempty"`
DebugAddr string `yaml:"debugAddr,omitempty"`
ChainNodes ChainNodes `yaml:"chainNodes"`
DebugAddr string `yaml:"debugAddr"`
}

func (c *Config) Nodes() (out []string) {
Expand Down Expand Up @@ -304,10 +304,11 @@ func (cns ChainNodes) Validate() error {
return errors.Join(errs...)
}

func ChainNodesFromFlag(nodes []string) (out ChainNodes, err error) {
for _, n := range nodes {
func ChainNodesFromFlag(nodes []string) (ChainNodes, error) {
out := make(ChainNodes, len(nodes))
for i, n := range nodes {
cn := ChainNode{PrivValAddr: n}
out = append(out, cn)
out[i] = cn
}
if err := out.Validate(); err != nil {
return nil, err
Expand Down
1 change: 1 addition & 0 deletions signer/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ chainNodes:
- privValAddr: tcp://127.0.0.1:1234
- privValAddr: tcp://127.0.0.1:2345
- privValAddr: tcp://127.0.0.1:3456
debugAddr: ""
`, string(configYamlBz))
}

Expand Down