search

livepeer / go-livepeer

Official Go implementation of the Livepeer protocol
http://livepeer.org
MIT License
538 stars 169 forks source link

Usage for `-ethKeystorePath` option is confusing #2691

Closed joegraviton closed 1 year ago

joegraviton commented 1 year ago

Describe the bug

The name and cli help for this option is unclear and misleading.

To Reproduce Steps to reproduce the behavior:

  1. In cli help:
  -ethKeystorePath string
        Path for the Eth Key
  1. When I read this, I am not sure what I should provide.

From the name ethKeystorePath, it looks like I should provide a dir path like:

/root/.lpData/keystore

But, from the help msg, it looks like I should provide a file path, like:

/root/.lpData/keystore/UTC--2022-12-03T10-31-30.841732443Z--6710ff8ab1d776097a6892c181964683442507b2`

This is too lengthy, I believe most users would rather not to provide it.

  1. actual behavior

If I provide /root/.lpData/keystore, the actually dir code will look into will be /root/.lpData/, which is unexpcted. To get this right, I must provide the path with trailing /: /root/.lpData/keystore/.

If I provide the lengthy key file path, code will look into the parent dir, the actual key file name is ignored. If you have multiple keys in that dir, you will need to specify it again, with -ethAcctAddr. Also unexpected.

related source code:

https://github.com/livepeer/go-livepeer/blob/7be4c3b518544f6cf29f79872e545b1a34cf72d9/cmd/livepeer/starter/starter.go#L525-L530

If the path exists, either it's dir or file, the path is split, which is not reasonable.

Expected behavior

This option should be a path to a keystore dir, no matter it has trailing / or not.

eliteprox commented 1 year ago

@joegraviton Let me know what you think of this solution https://github.com/livepeer/go-livepeer/pull/2713

ethKeystorePath will now accept either a directory or file without issue.

When a file is provided, ethAcctAddr will be set automatically so the extra flag is no longer needed. The parent directory is used as the "keystore path" for additional accounts.

When a folder is provided, the exact directory is used for keystore path and ethAcctAddr can be used to select an account for backwards compatibility.

joegraviton commented 1 year ago

@eliteprox That's perfect, thank you !