-
Notifications
You must be signed in to change notification settings - Fork 116
[Test Coverage Improvement] Validator Manager Flags #2690
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
Changes from 33 commits
74082f2
8c8d665
96d874c
aaca645
0bd0dbe
6138faa
4ddcc73
09520e3
9b37e4b
02d80b4
de14ae6
b3aceaf
db5a674
a5d6789
7902334
225bb19
3a31816
217a03e
5befbb6
9782ca1
d523864
ac6ee98
8334125
e377c10
c09efdc
143512e
631591c
0b23ce4
9c03b43
d1ab0ea
aac4455
bcd19e3
4a3ce8a
817ad66
7daf566
b1436fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,8 @@ import ( | |
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/ava-labs/avalanche-cli/cmd/flags" | ||
|
|
||
| "github.com/ava-labs/avalanche-cli/pkg/blockchain" | ||
| "github.com/ava-labs/avalanche-cli/pkg/cobrautils" | ||
| "github.com/ava-labs/avalanche-cli/pkg/constants" | ||
|
|
@@ -52,25 +54,26 @@ var ( | |
| pop string | ||
| remainingBalanceOwnerAddr string | ||
| disableOwnerAddr string | ||
| rpcURL string | ||
| aggregatorLogLevel string | ||
| aggregatorLogToStdout bool | ||
| delegationFee uint16 | ||
| errNoSubnetID = errors.New("failed to find the subnet ID for this subnet, has it been deployed/created on this network?") | ||
| errMutuallyExclusiveDurationOptions = errors.New("--use-default-duration/--use-default-validator-params and --staking-period are mutually exclusive") | ||
| errMutuallyExclusiveStartOptions = errors.New("--use-default-start-time/--use-default-validator-params and --start-time are mutually exclusive") | ||
| errMutuallyExclusiveWeightOptions = errors.New("--use-default-validator-params and --weight are mutually exclusive") | ||
| ErrNotPermissionedSubnet = errors.New("subnet is not permissioned") | ||
| aggregatorExtraEndpoints []string | ||
| aggregatorAllowPrivatePeers bool | ||
| clusterNameFlagValue string | ||
| createLocalValidator bool | ||
| externalValidatorManagerOwner bool | ||
| validatorManagerOwner string | ||
| httpPort uint32 | ||
| stakingPort uint32 | ||
| addValidatorFlags BlockchainAddValidatorFlags | ||
| ) | ||
|
|
||
| type BlockchainAddValidatorFlags struct { | ||
| RPC string | ||
| SigAggFlags flags.SignatureAggregatorFlags | ||
| } | ||
|
|
||
| const ( | ||
| validatorWeightFlag = "weight" | ||
| ) | ||
|
|
@@ -92,7 +95,8 @@ Testnet or Mainnet.`, | |
| Args: cobrautils.MaximumNArgs(1), | ||
| } | ||
| networkoptions.AddNetworkFlagsToCmd(cmd, &globalNetworkFlags, true, networkoptions.DefaultSupportedNetworkOptions) | ||
|
|
||
| flags.AddRPCFlagToCmd(cmd, app, &addValidatorFlags.RPC) | ||
| flags.AddSignatureAggregatorFlagsToCmd(cmd, &addValidatorFlags.SigAggFlags) | ||
| cmd.Flags().StringVarP(&keyName, "key", "k", "", "select the key to use [fuji/devnet only]") | ||
| cmd.Flags().Float64Var( | ||
| &balanceAVAX, | ||
|
|
@@ -111,11 +115,6 @@ Testnet or Mainnet.`, | |
| cmd.Flags().BoolVar(&createLocalValidator, "create-local-validator", false, "create additional local validator and add it to existing running local node") | ||
| cmd.Flags().BoolVar(&partialSync, "partial-sync", true, "set primary network partial sync for new validators") | ||
| cmd.Flags().StringVar(&nodeEndpoint, "node-endpoint", "", "gather node id/bls from publicly available avalanchego apis on the given endpoint") | ||
| cmd.Flags().StringSliceVar(&aggregatorExtraEndpoints, "aggregator-extra-endpoints", nil, "endpoints for extra nodes that are needed in signature aggregation") | ||
| cmd.Flags().BoolVar(&aggregatorAllowPrivatePeers, "aggregator-allow-private-peers", true, "allow the signature aggregator to connect to peers with private IP") | ||
| cmd.Flags().StringVar(&rpcURL, "rpc", "", "connect to validator manager at the given rpc endpoint") | ||
| cmd.Flags().StringVar(&aggregatorLogLevel, "aggregator-log-level", constants.DefaultAggregatorLogLevel, "log level to use with signature aggregator") | ||
| cmd.Flags().BoolVar(&aggregatorLogToStdout, "aggregator-log-to-stdout", false, "use stdout for signature aggregator logs") | ||
| cmd.Flags().DurationVar(&duration, "staking-period", 0, "how long this validator will be staking") | ||
| cmd.Flags().BoolVar(&useDefaultStartTime, "default-start-time", false, "(for Subnets, not L1s) use default start time for subnet validator (5 minutes later for fuji & mainnet, 30 seconds later for devnet)") | ||
| cmd.Flags().StringVar(&startTimeStr, "start-time", "", "(for Subnets, not L1s) UTC start time when this validator starts validating, in 'YYYY-MM-DD HH:MM:SS' format") | ||
|
|
@@ -186,13 +185,7 @@ func addValidator(cmd *cobra.Command, args []string) error { | |
| } | ||
|
|
||
| if len(args) == 0 { | ||
| if rpcURL == "" { | ||
| rpcURL, err = app.Prompt.CaptureURL("What is the RPC endpoint?", false) | ||
felipemadero marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| sc, err = importL1(blockchainIDStr, rpcURL, network) | ||
| sc, err = importL1(blockchainIDStr, addValidatorFlags.RPC, network) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -293,8 +286,6 @@ func addValidator(cmd *cobra.Command, args []string) error { | |
| if err != nil { | ||
| return err | ||
| } | ||
| // make sure extra validator endpoint added for the new node | ||
| aggregatorExtraEndpoints = append(aggregatorExtraEndpoints, constants.LocalAPIEndpoint) | ||
| } | ||
|
|
||
| if nodeIDStr == "" { | ||
|
|
@@ -340,6 +331,7 @@ func addValidator(cmd *cobra.Command, args []string) error { | |
| remainingBalanceOwnerAddr, | ||
| disableOwnerAddr, | ||
| sc, | ||
| addValidatorFlags.RPC, | ||
| ); err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -372,6 +364,7 @@ func CallAddValidator( | |
| remainingBalanceOwnerAddr string, | ||
| disableOwnerAddr string, | ||
| sc models.Sidecar, | ||
| rpcURL string, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the point of passing this and other global variables (flags in this case) to this function as arguments as opposed to just using them directly?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the goal is to be able to call this command from another command, and pass to it only the needed values. this concept is best represented in the tokentransferrer deploy cmd, here the implementation is partial. |
||
| ) error { | ||
| nodeID, err := ids.NodeIDFromString(nodeIDStr) | ||
| if err != nil { | ||
|
|
@@ -514,18 +507,15 @@ func CallAddValidator( | |
| Threshold: 1, | ||
| Addresses: disableOwnerAddrID, | ||
| } | ||
|
|
||
| extraAggregatorPeers, err := blockchain.GetAggregatorExtraPeers(app, clusterNameFlagValue, aggregatorExtraEndpoints) | ||
| extraAggregatorPeers, err := blockchain.GetAggregatorExtraPeers(app, clusterNameFlagValue) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| aggregatorLogger, err := utils.NewLogger( | ||
| constants.SignatureAggregatorLogName, | ||
| aggregatorLogLevel, | ||
| constants.DefaultAggregatorLogLevel, | ||
| addValidatorFlags.SigAggFlags.AggregatorLogLevel, | ||
| addValidatorFlags.SigAggFlags.AggregatorLogToStdout, | ||
| app.GetAggregatorLogDir(clusterNameFlagValue), | ||
| aggregatorLogToStdout, | ||
| ux.Logger.PrintToUser, | ||
| ) | ||
| if err != nil { | ||
| return err | ||
|
|
@@ -548,7 +538,6 @@ func CallAddValidator( | |
| disableOwners, | ||
| weight, | ||
| extraAggregatorPeers, | ||
| aggregatorAllowPrivatePeers, | ||
| aggregatorLogger, | ||
| pos, | ||
| delegationFee, | ||
|
|
@@ -593,7 +582,6 @@ func CallAddValidator( | |
| ownerPrivateKey, | ||
| validationID, | ||
| extraAggregatorPeers, | ||
| aggregatorAllowPrivatePeers, | ||
| aggregatorLogger, | ||
| validatorManagerAddress, | ||
| ) | ||
|
|
||
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.
What's the point of grouping these flags here?
Uh oh!
There was an error while loading. Please reload this page.
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 variable retains only the value of the command that was last initialized (a conflict for command that have different initialization values)
the idea is for this struct to contain all command flags in the future