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

Clef: USB hw wallet support #17756

Merged
merged 2 commits into from
Sep 28, 2018
Merged

Clef: USB hw wallet support #17756

merged 2 commits into from
Sep 28, 2018

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Sep 25, 2018

This PR ports USB hardware wallets like Trezor over to Clef. Since Clef does not have access to state, it auto-derives a number of accounts (currently hardcoded to 10).
It also adds a new internal API-call to ask the UI for a trezor PIN.

Copy link

@nagydani nagydani left a comment

Choose a reason for hiding this comment

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

This is a fairly straightforward port of functionality from geth. My only concern is the hard-wired magic number 10 for the number of accounts to derive. It should be part of configuration, but this is no reason not to merge.

@holiman
Copy link
Contributor Author

holiman commented Sep 27, 2018

I agree, the future plan is to either make it a flag, but the real fix is to make it configurable from the UI. That requires a bidirectional commnication channel, which our current RPC implementation does not support (the json-rpc protocol supports it though)

I did it this way for now, since it's easy and I didn't want to add another flag that will be removed soon:ish anyway

@holiman
Copy link
Contributor Author

holiman commented Sep 27, 2018

Rebased to get travis working again...

@holiman holiman merged commit dcaabfe into ethereum:master Sep 28, 2018
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