Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Implement accounts command #1130

Merged
merged 1 commit into from
Jul 24, 2019
Merged

Implement accounts command #1130

merged 1 commit into from
Jul 24, 2019

Conversation

jbcarpanelli
Copy link
Contributor

@jbcarpanelli jbcarpanelli commented Jul 19, 2019

I had this in my git stash for a while... Just committed it so you can take care of it while I'm on vacations 🙂
Fixes #967?

Note: I wouldn't merge this feature for v2.5.0.

Copy link
Contributor

@ylv-io ylv-io left a comment

Choose a reason for hiding this comment

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

Pretty will break on Infura and other account-less providers.

@spalladino
Copy link
Contributor

Pretty will break on Infura and other account-less providers.

Are you sure about this @ylv-io? I understand that offline-signing providers (such as the HDWallet one) capture the eth_getAccounts call and return the ones for which they have the keys. I'd expect this to work also on those cases.

@ylv-io
Copy link
Contributor

ylv-io commented Jul 24, 2019

Nice comment @spalladino!
I just checked blueprint.network.jsand it doesn't contain HDWallet or other providers. I guess that is a separate issue so feel free to merge the PR.

Copy link
Contributor

@ylv-io ylv-io left a comment

Choose a reason for hiding this comment

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

HDWallet for the rescue 🚀

Copy link
Contributor

@spalladino spalladino left a comment

Choose a reason for hiding this comment

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

Nice work @jcarpanelli! I'm not a fan of using noSpin for outputting info (I think I now understand where #1147 came from), but the feature itself looks solid :-)

@spalladino spalladino merged commit b5c551c into master Jul 24, 2019
@spalladino spalladino deleted the feature/accounts-command branch July 24, 2019 14:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a command to query current accounts
3 participants