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.

Sign up for GitHub

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

Jump to bottom

[R4R] Update testnet to use canonical genesis time #2692

Merged
merged 13 commits into from
Nov 7, 2018
Merged

[R4R] Update testnet to use canonical genesis time #2692

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
486be46
Update testnet to use canonical genesis time
Nov 5, 2018
3379790
Fix linting in genesis test
Nov 5, 2018
b8334c1
Update dep lock
Nov 5, 2018
2b297c4
Add pending log entry
Nov 5, 2018
74c7dac
Reduce usage of viper
Nov 5, 2018
144907c
Revert viper changes and cleanup flag vars
Nov 5, 2018
7499782
Cleanup return values
Nov 6, 2018
9d48722
Address PR comments on viper usage
Nov 6, 2018
06c9d2f
Attempt CI broken depedencies fix
Nov 6, 2018
b95a42f
Attempt CI broken depedencies fix...again
Nov 6, 2018
de04649
Fix TM dep
jackzampolin Nov 7, 2018
816bff8
Update lockfile
jackzampolin Nov 7, 2018
07e4e60
Merge branch 'develop' into bez/2691-testnet-validator-gen-time-fix-2
jaekwon Nov 7, 2018
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
Prev Previous commit
Next Next commit
Revert viper changes and cleanup flag vars
  • Loading branch information
Aleksandr Bezobchuk committed Nov 5, 2018
commit 144907c48bf8ac09883475f216e3106cb283ecb4
60 changes: 29 additions & 31 deletions cmd/gaia/init/testnet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ import (
)

var (
nodeDirPrefix = "node-dir-prefix"
nValidators = "v"
outputDir = "output-dir"
nodeDaemonHome = "node-daemon-home"
nodeCliHome = "node-cli-home"

startingIPAddress = "starting-ip-address"
flagNodeDirPrefix = "node-dir-prefix"
flagNumValidators = "v"
flagOutputDir = "output-dir"
flagNodeDaemonHome = "node-daemon-home"
flagNodeCliHome = "node-cli-home"
flagStartingIPAddress = "starting-ip-address"
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
)

const nodeDirPerm = 0755
Expand All @@ -58,30 +57,30 @@ Example:
},
}

cmd.Flags().Int(nValidators, 4,
cmd.Flags().Int(flagNumValidators, 4,
"Number of validators to initialize the testnet with",
)
cmd.Flags().StringP(outputDir, "o", "./mytestnet",
cmd.Flags().StringP(flagOutputDir, "o", "./mytestnet",
"Directory to store initialization data for the testnet",
)
cmd.Flags().String(nodeDirPrefix, "node",
cmd.Flags().String(flagNodeDirPrefix, "node",
"Prefix the directory name for each node with (node results in node0, node1, ...)",
)
cmd.Flags().String(nodeDaemonHome, "gaiad",
cmd.Flags().String(flagNodeDaemonHome, "gaiad",
"Home directory of the node's daemon configuration",
)
cmd.Flags().String(nodeCliHome, "gaiacli",
cmd.Flags().String(flagNodeCliHome, "gaiacli",
"Home directory of the node's cli configuration",
)
cmd.Flags().String(startingIPAddress, "192.168.0.1",
cmd.Flags().String(flagStartingIPAddress, "192.168.0.1",
"Starting IP address (192.168.0.1 results in persistent peers list ID0@192.168.0.1:46656, ID1@192.168.0.2:46656, ...)")

return cmd
}

func initTestnet(config *cfg.Config, cdc *codec.Codec) error {
outDir := viper.GetString(outputDir)
numValidators := viper.GetInt(nValidators)
outDir := viper.GetString(flagOutputDir)
numValidators := viper.GetInt(flagNumValidators)

chainID := "chain-" + cmn.RandStr(6)

Expand All @@ -96,9 +95,9 @@ func initTestnet(config *cfg.Config, cdc *codec.Codec) error {

// generate private keys, node IDs, and initial transactions
for i := 0; i < numValidators; i++ {
nodeDirName := fmt.Sprintf("%s%d", viper.GetString(nodeDirPrefix), i)
nodeDaemonHomeName := viper.GetString(nodeDaemonHome)
nodeCliHomeName := viper.GetString(nodeCliHome)
nodeDirName := fmt.Sprintf("%s%d", viper.GetString(flagNodeDirPrefix), i)
nodeDaemonHomeName := viper.GetString(flagNodeDaemonHome)
nodeCliHomeName := viper.GetString(flagNodeCliHome)
nodeDir := filepath.Join(outDir, nodeDirName, nodeDaemonHomeName)
clientDir := filepath.Join(outDir, nodeDirName, nodeCliHomeName)
gentxsDir := filepath.Join(outDir, "gentxs")
Expand Down Expand Up @@ -209,14 +208,11 @@ func initTestnet(config *cfg.Config, cdc *codec.Codec) error {
}
}

if err := initGenFiles(cdc, chainID, accs, genFiles, numValidators); err != nil {
if err := initGenFiles(cdc, chainID, accs, genFiles); err != nil {
return err
}

err := collectGenFiles(
cdc, config, chainID, monikers, nodeIDs, valPubKeys, numValidators, outputDir,
)
if err != nil {
if err := collectGenFiles(cdc, config, chainID, monikers, nodeIDs, valPubKeys); err != nil {
return err
}

Expand All @@ -225,10 +221,10 @@ func initTestnet(config *cfg.Config, cdc *codec.Codec) error {
}

func initGenFiles(
cdc *codec.Codec, chainID string, accs []app.GenesisAccount,
genFiles []string, numValidators int,
cdc *codec.Codec, chainID string, accs []app.GenesisAccount, genFiles []string,
) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a weird way to split up the func header, I'd suggest something along the lines of:

func initGenFiles(cdc *codec.Codec, chainID string,
     accs []app.GenesisAccount, genFiles []string) error {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ughhh I find that so much less elegant and harder to read imho. I'll be happy to change it if it's our standard though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's prettier:

a.

foo := []string{"foo",
	"bar", "cat"}

b.

foo := []string{
	"foo", "bar", "cat",
}

Copy link
Contributor

@rigelrozanski rigelrozanski Nov 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k - yeah so-far the style I've suggested is used everywhere, so I think you should change it - we could open up a stylistic issue to discuss further (but post-launch change obv.).

I think functions are unique from struct definition, I don't find it adequate to draw the comparison (but ftr obv. b.). Part of what's nice about keeping func wrapping as I've suggested is that it's unique from how structs are defined - it becomes a visual cue as you're skimming through the code to notice that this a function not a struct

[if you create an issue should link it to this comment]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'm vetoed here haha. I'll change it. Will open up an issue later to discuss this more openly.


numValidators := viper.GetInt(flagNumValidators)
rigelrozanski marked this conversation as resolved.
Show resolved Hide resolved
appGenState := app.NewDefaultGenesisState()
appGenState.Accounts = accs

Expand Down Expand Up @@ -256,17 +252,19 @@ func initGenFiles(
func collectGenFiles(
cdc *codec.Codec, config *cfg.Config, chainID string,
monikers, nodeIDs []string, valPubKeys []crypto.PubKey,
numValidators int, outputDir string,
) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, feel weird about this style, suggest:

func collectGenFiles(cdc *codec.Codec, config *cfg.Config, chainID string,
	monikers, nodeIDs []string, valPubKeys []crypto.PubKey,) error {


outDir := viper.GetString(flagOutputDir)
numValidators := viper.GetInt(flagNumValidators)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same, let's not use viper this way, these should be inputs to the function and viper should be used only in the cobra commands

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k just realized you're only re-organizing here, and it was already this way - so probably okay to just leave for this PR - however it would still be great if you wanted to address this


var appState json.RawMessage
genTime := tmtime.Now()

for i := 0; i < numValidators; i++ {
nodeDirName := fmt.Sprintf("%s%d", viper.GetString(nodeDirPrefix), i)
nodeDaemonHomeName := viper.GetString(nodeDaemonHome)
nodeDir := filepath.Join(outputDir, nodeDirName, nodeDaemonHomeName)
gentxsDir := filepath.Join(outputDir, "gentxs")
nodeDirName := fmt.Sprintf("%s%d", viper.GetString(flagNodeDirPrefix), i)
nodeDaemonHomeName := viper.GetString(flagNodeDaemonHome)
nodeDir := filepath.Join(outDir, nodeDirName, nodeDaemonHomeName)
gentxsDir := filepath.Join(outDir, "gentxs")
moniker := monikers[i]
config.Moniker = nodeDirName

Expand Down Expand Up @@ -309,7 +307,7 @@ func collectGenFiles(
}

func getIP(i int) (ip string, err error) {
ip = viper.GetString(startingIPAddress)
ip = viper.GetString(flagStartingIPAddress)

if len(ip) == 0 {
ip, err = server.ExternalIP()
Expand Down