Skip to content

accounts/keystore: prevent key-loading in geth account new#15522

Closed
holiman wants to merge 2 commits into
ethereum:masterfrom
holiman:fix_slow_create_account
Closed

accounts/keystore: prevent key-loading in geth account new#15522
holiman wants to merge 2 commits into
ethereum:masterfrom
holiman:fix_slow_create_account

Conversation

@holiman
Copy link
Copy Markdown
Contributor

@holiman holiman commented Nov 19, 2017

The changes introduced in #15171 made an initial scan of the filesystem performed upon startup. This had an adverse effect in the case where geth was not actually intended to run in normal 'daemon'-mode, but was only meant for brief geth account new.

This PR makes the geth account new not configure a complete full node, but instead initalize a keystore directly. When doing so, it can avoind to load all existing files from the keystore directory.

Fixes #15511.

On commit 88b1db728826efd499ea407579f41e8b683d6b53 (with 55k accounts):

#time yes | build/bin/geth --datadir ~/tmp/account_gen account  new
Your new account is locked with a password. Please give a password. Do not forget this password.
!! Unsupported terminal, password will be echoed.
Passphrase: 
Repeat passphrase: 
Address: {b3d522cbff4c47cfeee5252e29729b52ada68b3a}

real	0m46.971s
user	0m49.480s
sys	0m1.524s

With this PR:

#time yes | build/bin/geth --datadir ~/tmp/account_gen account  new
Your new account is locked with a password. Please give a password. Do not forget this password.
!! Unsupported terminal, password will be echoed.
Passphrase: 
Repeat passphrase: 
Address: {a9486ada158b8dc2e2008dbbc20351f81f7af95f}

real	0m0.743s
user	0m0.712s
sys	0m0.040s

@holiman holiman requested a review from karalabe November 19, 2017 11:59
@holiman holiman force-pushed the fix_slow_create_account branch from 8172c36 to 5120a06 Compare November 19, 2017 12:01
@holiman holiman changed the title accounts/keystore: prevent key-loading from #15511 accounts/keystore: prevent key-loading in geth account new Nov 19, 2017
@karalabe
Copy link
Copy Markdown
Member

karalabe commented Nov 19, 2017

I'm not sure this is actually correct.

https://github.com/ethereum/go-ethereum/blob/master/node/config.go#L72

	// KeyStoreDir is the file system folder that contains private keys. The directory can
	// be specified as a relative path, in which case it is resolved relative to the
	// current directory.
	//
	// If KeyStoreDir is empty, the default location is the "keystore" subdirectory of
	// DataDir. If DataDir is unspecified and KeyStoreDir is empty, an ephemeral directory
	// is created by New and destroyed when the node is stopped.
	KeyStoreDir string `toml:",omitempty"`

I.e. The keystore path given as a flag, config or otherwise can be resolved to a lot of different locations. You PR essentially removes this resolution altogether.

// NewKeyStore creates a keystore for the given directory.
func NewKeyStore(keydir string, scryptN, scryptP int) *KeyStore {
// NewUninitializedKeyStore creates a keystore for the given directory, but does not load the existing keys
func NewUninitializedKeyStore(keydir string, scryptN, scryptP int) *KeyStore {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is a very ugly API hack :P

@holiman
Copy link
Copy Markdown
Contributor Author

holiman commented Nov 19, 2017

@karalabe you were totally right. I pushed some changes, so that it now correctly uses the configuration. The code is on the level of 'it works but it aint pretty'. If I get the time, I'll try to make another stab at a nicer fix.

@holiman
Copy link
Copy Markdown
Contributor Author

holiman commented Nov 20, 2017

Closing this in favour of #15529

@holiman holiman closed this Nov 20, 2017
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