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
Reduce usage of viper
  • Loading branch information
Aleksandr Bezobchuk committed Nov 5, 2018
commit 74c7dacd81e9582c769904476d5194bc3ac0d077
21 changes: 11 additions & 10 deletions cmd/gaia/init/testnet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,23 +209,26 @@ func initTestnet(config *cfg.Config, cdc *codec.Codec) error {
}
}

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

if err := collectGenFiles(cdc, config, chainID, monikers, nodeIDs, valPubKeys); err != nil {
err := collectGenFiles(
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned below, there are several things weird here... (partially due to the weirdness of the original logic). We shouldn't be init'ing 4 times and then "collecting" (misnomer) 4 times.

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

fmt.Printf("Successfully initialized %v node directories\n", viper.GetInt(nValidators))
fmt.Printf("Successfully initialized %d node directories\n", numValidators)
return nil
}

func initGenFiles(
cdc *codec.Codec, chainID string, accs []app.GenesisAccount, genFiles []string,
cdc *codec.Codec, chainID string, accs []app.GenesisAccount,
genFiles []string, numValidators int,
) 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(nValidators)
appGenState := app.NewDefaultGenesisState()
appGenState.Accounts = accs

Expand Down Expand Up @@ -253,19 +256,17 @@ 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(outputDir)
numValidators := viper.GetInt(nValidators)

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(outDir, nodeDirName, nodeDaemonHomeName)
gentxsDir := filepath.Join(outDir, "gentxs")
nodeDir := filepath.Join(outputDir, nodeDirName, nodeDaemonHomeName)
gentxsDir := filepath.Join(outputDir, "gentxs")
moniker := monikers[i]
config.Moniker = nodeDirName

Expand Down