accounts/scwallet: Add a switch to specify path to sc daemon #19439
Conversation
|
Hmmm, doesn't this only appear for Status smart cards? I think if we have an Ethereum smart card in, this is a valid warning. If we don't, this should not appear. I'm generally against adding flags to enable features, because Geth should be smart enough to correctly handle all scenarios. Requiring user manual flags is just effectively disabling a feature. |
node/config.go
Outdated
| log.Warn(fmt.Sprintf("Failed to start smart card hub, disabling: %v", err)) | ||
| } else { | ||
| backends = append(backends, schub) | ||
| if conf.SmartCard { |
There was a problem hiding this comment.
👍 fixes the warning when running geth account list.
This warning appears each time, even if you don't have a smart card reader plugged in. That's why I only want it to be available to the user who knows that they have a smartcard reader. So yes, it's effectively disabling. I'm going to see if it's easier to detect that there is no smartcard reader before activating (basically, making the switch invisible to the user based on what |
|
Yes, that would be ideal. Btw, do the smart cards work on multiple OSes, or only linux? If only Linux, we could make the whole thing disabled for others. That's a good start. |
|
Oh right, there are other OSes besides Linux 😅 |
* disable card support in windows until tested * only activate account if pcscd socket file is present * the switch is now the path to the socket file
|
@karalabe I'm proposing an intermediate step:
|
| if fi, err := os.Stat(ctx.GlobalString(SmartCardFlag.Name)); err == nil { | ||
| cfg.SmartCard = fi.Mode()&os.ModeType == os.ModeSocket | ||
| } | ||
| } |
There was a problem hiding this comment.
Is it worth mentioning to windows-users that smart card support is disabled? I don't know, think about it, I'll leave it for you to decide.
There was a problem hiding this comment.
@holiman I have updated the command line info. Like you said this morning, let's not display a message every time we start geth. I'm also going to test it on Windows, FreeBSD and macos so it might not be limited to Linux so long anyway. I expect the PCSC daemon to behave the same on these platforms.
gballet
changed the title
gballet
force-pushed
the
scwallet-activation-switch
branch
from
8beae9b
to
e498e2d
Compare
cmd/utils/flags.go
Outdated
| @@ -127,6 +128,11 @@ var ( | |||
| Name: "nousb", | |||
| Usage: "Disables monitoring for and managing USB hardware wallets", | |||
| } | |||
| SmartCardFlag = cli.StringFlag{ | |||
| Name: "pcscd-sock", | |||
This adds a command line switch to
gethto activate the smartcard management code. This is to remove a warning that will otherwise occur atgethstartup and confuse the user:The same switch has voluntarily not been added to
clefasclefdoesn't support smartcard yet. This will be covered in the PR to add the smartcard feature toclef.