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

JS Keystore Docs (with updated file structure) #8185

Merged
merged 32 commits into from
Jul 8, 2021

Conversation

eelanagaraj
Copy link
Contributor

@eelanagaraj eelanagaraj commented Jun 24, 2021

(Up to date with new file structure)

  • Added autogenerated docs for keystore
  • Added a walkthrough with code/usage examples in using-js-keystores.md (note: will not work until first version is published); this is what it will look like on gitbook

Split so as to not clutter the first PR (#8096)

(apologies for the messy commits here -- wanted to stack them to make the differences cleaner initially but didn't want to wait to land docs with the rest of the PR!)

@eelanagaraj eelanagaraj mentioned this pull request Jun 24, 2021
@eelanagaraj eelanagaraj changed the base branch from master to eelanagaraj/js-keystore June 24, 2021 18:02
@eelanagaraj eelanagaraj requested review from aslawson and a team June 24, 2021 18:18
Copy link
Contributor

@AlexBHarley AlexBHarley left a comment

Choose a reason for hiding this comment

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

Looks good!

Btw @eelanagaraj what is this file packages/docs/developer-resources/keystores/reference/README.md?

Is that supposed to be there?

mergify bot pushed a commit that referenced this pull request 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
@eelanagaraj
Copy link
Contributor Author

@AlexBHarley that file is autogenerated by yarn docs, but yeah doesn't show up anywhere except in the repo if you click into the package!

@eelanagaraj eelanagaraj changed the base branch from eelanagaraj/js-keystore to master June 28, 2021 15:46
@eelanagaraj eelanagaraj force-pushed the eelanagaraj/js-keystore-docs-new branch from b973a46 to 91fb0fe Compare June 28, 2021 15:56
Copy link
Contributor

@AlexBHarley AlexBHarley left a comment

Choose a reason for hiding this comment

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

LGTM!

tkporter pushed a commit that referenced this pull request 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
@eelanagaraj eelanagaraj added the automerge Have PR merge automatically when checks pass label Jul 8, 2021
@mergify mergify bot merged commit fe288ae into master Jul 8, 2021
@mergify mergify bot deleted the eelanagaraj/js-keystore-docs-new branch July 8, 2021 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants