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

Generate PubKey from SecKey if only SecKey is set in Visor config #256

Merged
merged 5 commits into from
Mar 26, 2020

Conversation

nkryuchkov
Copy link
Contributor

@nkryuchkov nkryuchkov commented Mar 26, 2020

Did you run make format && make check? Yes

Fixes #255

Depends on https://github.com/SkycoinPro/skywire-services/pull/128

Changes:

  • Generate PubKey from SecKey if only SecKey is set in Visor config
  • Change static_secret_key to secret_key in Visor JSON config.
  • Change static_public_key to public_key in Visor JSON config.
  • If a necessary config field is not set and default config value is generated, then the value is updated in the JSON config.

How to test this PR:

  • Run generic integration env and check if Visor PK is generated correctly for the following conditions:
    • If both PK and SK are set in config, PK from config should be shown.
    • If only SK is set in config, the same PK should be shown.
    • If only PK is set in config, a newly generated PK should be shown.
    • If no keys are set in config, a newly generated PK should be shown.

@jdknives
Copy link
Member

Should the PubKey be added to the config itself when it is generated? @nkryuchkov

@jdknives
Copy link
Member

Also, when the secret key is empty and public key is empty, I get the following error:

FATAL [skywire]: Failed to decode &{%!s(*os.file=&{{{0 0 0} 7 {0} <nil> 0 1 true true true} /Users/rudikleine/go/src/github.com/SkycoinProject/skywire-mainnet/skywire-config.json <nil> false false false})}: Invalid secret key length

Config looks like this:

	"key_pair": {
		"static_public_key": "",
		"static_secret_key": ""
	}

@nkryuchkov
Copy link
Contributor Author

@jdknives

Should the PubKey be added to the config itself when it is generated?

Yes, because if it is not set, it will be generated from SecKey next time Keys() is called. Also, cipher.GenerateKeyPair() generates both keys, so I think it's better to fill both at one time.

@nkryuchkov
Copy link
Contributor Author

nkryuchkov commented Mar 26, 2020

@jdknives

Also, when the secret key is empty and public key is empty, I get the following error:

In this case, keys are considered malformed, because the key_pair section is present in the config. However, I think it might be a good idea to generate new keys if they are malformed.

@jdknives jdknives merged commit 59026ea into skycoin:develop Mar 26, 2020
@nkryuchkov nkryuchkov deleted the feature/visor-pubkey-from-seckey branch March 27, 2020 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants