Change subnet/blockchain creation order #1375
Conversation
then to create blockchain.
| @@ -103,7 +101,10 @@ func syncSubnet(_ *cobra.Command, args []string) error { | |||
| return fmt.Errorf("node(s) %s failed to sync with subnet %s", untrackedNodes, subnetName) | |||
| } | |||
| ux.Logger.PrintToUser("Node(s) successfully started syncing with Subnet!") | |||
| ux.Logger.PrintToUser(fmt.Sprintf("Check node subnet syncing status with avalanche node status %s --subnet %s", clusterName, subnetName)) | |||
| blockchainID := sc.Networks[network.Kind.String()].BlockchainID | |||
There was a problem hiding this comment.
we need to check if sc.Networks[network.Kind.String()] is not nil before trying to access BlockchainID as it can crash
| ux.Logger.PrintToUser(fmt.Sprintf("Check node subnet syncing status with avalanche node status %s --subnet %s", clusterName, subnetName)) | ||
| blockchainID := sc.Networks[network.Kind.String()].BlockchainID | ||
| if blockchainID != ids.Empty { | ||
| ux.Logger.PrintToUser(fmt.Sprintf("Check node subnet syncing status with avalanche node status %s --subnet %s", clusterName, subnetName)) |
There was a problem hiding this comment.
fmt.Sprintf is not need as PrintToUser is a wrapper around Sprintf itself
| } | ||
| return nil | ||
| return clustersConfig.Clusters[clusterName].Network, nil |
There was a problem hiding this comment.
check if clustersConfig.Clusters[clusterName] is not nil before accessing Network
| if err != nil { | ||
| return nil, err | ||
| } | ||
| subnetID := sc.Networks[network.Kind.String()].SubnetID |
There was a problem hiding this comment.
check if sc.Networks[network.Kind.String()] is not nil
| } | ||
| nonValidators := []string{} | ||
| for i, nodeID := range nodeIDs { | ||
| isValidator, err := subnet.IsSubnetValidator(subnetID, nodeID, network) |
There was a problem hiding this comment.
suggestion: can be done in parallel to increase performance
| ux.Logger.PrintToUser(" " + nonValidator) | ||
| } | ||
| ux.Logger.PrintToUser("") | ||
| return fmt.Errorf("failed to verify all nodes are subnet validators after %d seconds", uint32(timeout.Seconds())) |
There was a problem hiding this comment.
can it be replaced with return fmt.Errorf("failed to verify all nodes as subnet validators after %f seconds", timeout.Seconds())?
| return err | ||
| } | ||
| if len(nonValidators) == 0 { | ||
| ux.Logger.PrintToUser("All nodes are subnet validators after %d seconds", uint32(time.Since(startTime).Seconds())) |
There was a problem hiding this comment.
can it be %f s or %0.2f?
| fee += network.GenesisParams().CreateSubnetTxFee | ||
| fee = network.GenesisParams().CreateSubnetTxFee | ||
| } else { | ||
| fee = network.GenesisParams().CreateBlockchainTxFee |
There was a problem hiding this comment.
nit: can be simplified with
fee := network.GenesisParams().CreateBlockchainTxFee
if createSubnet {
fee = network.GenesisParams().CreateSubnetTxFee
}
|
Ok so seems that currently with this PR, user is expected to run the deploy command ONCE, call addValidator command, and then call deploy command again -> need to call 3 commands to deploy a subnet in public network. I think it's better for user experience if we add prompts required for adding Validators in the deploy command so that we can deploy subnet in public network only with 1 command |
|
Also are we changing the order here? Because seems that we are already creating subnet first and then creating blockchain anyways |
Closes #1279
With this,
subnet deploywill create a subnet id on first execution, and create a blockchain on second execution. In the middle, it is expected for the user to add validators to the subnet.subnet deploywill check for that condition when creating the blockchain.