Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Can now disable the keystore #3004

Merged
merged 4 commits into from
Jul 4, 2019
Merged

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Jul 3, 2019

For #2416, the keystore obviously cannot load a key from the disk.
This PR makes it possible to disable the keystore by passing None for keystore_path.

The second change I made is change the configuration to take PathBuf instead of String. I have no idea why we were using String, and it doesn't make sense to me.

@tomaka tomaka added A0-please_review Pull request needs code review. B2-breaksapi labels Jul 3, 2019
@gavofyork gavofyork added A6-seemsok and removed A0-please_review Pull request needs code review. labels Jul 3, 2019
core/cli/src/lib.rs Outdated Show resolved Hide resolved
core/cli/src/lib.rs Outdated Show resolved Hide resolved
info!("Generated a new keypair: {:?}", public_key);

public_key
if let Some(keystore) = keystore.as_mut() {
Copy link
Member

Choose a reason for hiding this comment

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

I think putting let public_key = in front of the if is more idiomatic rust?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is extreme nit-picking, but I didn't do it this way because it makes it look like the primary objective of this block is to fetch the public key, while in reality it is to generate the key if there's none.

tomaka and others added 2 commits July 3, 2019 21:22
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
@bkchr bkchr merged commit c9a1c36 into paritytech:master Jul 4, 2019
@tomaka tomaka deleted the disablable-keystore branch July 4, 2019 08:45
MTDK1 pushed a commit to bdevux/substrate that referenced this pull request Jul 10, 2019
* Can now disable the keystore

* Fix service test

* Apply suggestions from code review

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Fix cli
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants