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

JavaScript native keystore #7063

Closed
aslawson opened this issue Feb 11, 2021 · 2 comments
Closed

JavaScript native keystore #7063

aslawson opened this issue Feb 11, 2021 · 2 comments
Assignees
Labels
Applications Applications circle tasks Component: ContractKit feature Feature requests Priority: P2 Major

Comments

@aslawson
Copy link
Contributor

aslawson commented Feb 11, 2021

Quick Implementation: I see this as basically a state management and persistence library that wraps the LocalWallet functionality already in ContractKit, ideally providing support for each environment we want to run it in. LocalWallet keeps the private key in memory and has support for all operations (sign, decrypt, computeSharedSecret, etc) that we're currently thinking about. In an effort to move away from Geth signing data and controlling keys we just need more people to use this LocalWallet safely.

Could also explore KeystoreManager - Basically an API for getting (and decrypting) the private key in a way all the consumers can implement.

Use Cases

  • CLI - I see an API like celocli unlock 0x... (which prompts for password), celocli lock 0x... and all other operations (e.g. sending tx's via celocli send cUSD 10 --from 0x) would simply call into the .celocli keystore file if it exists. As Amy mentioned much nicer than including the --privateKey flag
  • Valora - is able to skip touching Geth and use this keystore instead for all key. This would mean when Valora is running the light client, all it does it send signed transactions to it and read state from it. Right now when using Forno the flow is 1. Ask Geth to sign transaction 2. Get back signed transaction 3. Send to Forno.
  • Attestation Service - Right now validators need to run a separate full node with their attestation signer key unlocked in order to sign the message (attestation proof). it would be a security risk to make the request directly to a validator, but if we had a local storage for the private key, they could make the request to the validator proxy (and thus eliminate the need of running an additional full node and reduced many of our downtime issues due to node problems)here the encrypted storage might be the attestation service database or even a raw file on the VM.

Thoughts on Clef:

  • It doesn't have wide adoption
  • Bundling a second binary into Valora (Geth is the first) might cause more size bloat.
@aslawson aslawson added Component: ContractKit Applications Applications circle tasks feature Feature requests Priority: P2 Major labels Feb 11, 2021
@AlexBHarley
Copy link
Contributor

AlexBHarley commented Mar 9, 2021

Seems like the ideal scenario here is something Valora can use (ReactNative compatible), the browser can use (LocalStorage / IndexedDB compatibility) and something the CLI can use (file system access).

Where does Metamask store private keys?

Metamask uses a library called LocalForage, that acts as a thin wrapper around LocalStorage and IndexedDB. AFAICT metamask simply stores private keys with that, encrypted with a password / pin.

@aslawson
Copy link
Contributor Author

aslawson commented Jun 8, 2021

Added old description for posterity

  • Makes CIP8 very unusable
  • Getting it into CLI; don’t have to worry about browser capability, reads from file from file
  • Extend wallet interface
  • ~ 1d work
  • Wallet channel cannot access what they need to
    • RPC with geth node with native node — some keys kept in plan text (file system store); use contract kit and hook up to this store; contract kit uses this in memory wallet (DEK)
    • Do run a geth client and some things (Signing Key) native bridge
  • Would Validators like js keystore — filesystem and local store interface
  • Where does Metamask store private key?
  • .celo file

@eelanagaraj eelanagaraj self-assigned this Jun 9, 2021
mergify bot pushed a commit that referenced this issue Jun 28, 2021
### Description
- Implements most basic keystore functionality (encrypt, decrypt, change password) base class (+ file system for data persistence), based on `ethereumjs-wallet`
- `Keystore` abstraction should make it fairly straightforward to add additional IO formats (browser storage, DB, however valora wants to do this...)
- `KeystoreWalletWrapper` is a bare bones wrapper of a file keystore + the `LocalWallet` -- I expect this to change (+ maybe move into its own directory), but would wait until implementing attestation service/CLI-relevant changes before adding more here, in order to see what's necessary/makes sense

### Tested

- Unit tests
- Tested instantiating, signing, sending txs via the `KeystoreWalletWrapper` + keystore files
- Tested that keystore files created by geth can be copied into this keystore directory and decrypted with the same passphrase; same with the reverse (i.e. keystore files created + encrypted with the `FileKeystore` can be copied to the geth keystore directory, decrypted, and used to send txs)

### Related issues

- #7063

- related autogenerated docs (split PR so as to not clutter this one) [old file structure PR](#8160), [docs PR new file structure](#8185)

### Backwards compatibility

New module, no changes to existing wallets or tools.

### Remaining TODOs
- [later PR] modify KeystoreWalletWrapper as makes sense with future use cases
tkporter pushed a commit that referenced this issue Jul 8, 2021
### Description
- Implements most basic keystore functionality (encrypt, decrypt, change password) base class (+ file system for data persistence), based on `ethereumjs-wallet`
- `Keystore` abstraction should make it fairly straightforward to add additional IO formats (browser storage, DB, however valora wants to do this...)
- `KeystoreWalletWrapper` is a bare bones wrapper of a file keystore + the `LocalWallet` -- I expect this to change (+ maybe move into its own directory), but would wait until implementing attestation service/CLI-relevant changes before adding more here, in order to see what's necessary/makes sense

### Tested

- Unit tests
- Tested instantiating, signing, sending txs via the `KeystoreWalletWrapper` + keystore files
- Tested that keystore files created by geth can be copied into this keystore directory and decrypted with the same passphrase; same with the reverse (i.e. keystore files created + encrypted with the `FileKeystore` can be copied to the geth keystore directory, decrypted, and used to send txs)

### Related issues

- #7063

- related autogenerated docs (split PR so as to not clutter this one) [old file structure PR](#8160), [docs PR new file structure](#8185)

### Backwards compatibility

New module, no changes to existing wallets or tools.

### Remaining TODOs
- [later PR] modify KeystoreWalletWrapper as makes sense with future use cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Applications Applications circle tasks Component: ContractKit feature Feature requests Priority: P2 Major
Projects
None yet
Development

No branches or pull requests

3 participants