Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Support importing one or more private keys with indexing #38

Merged
merged 7 commits into from
Oct 23, 2018

Conversation

emkman
Copy link
Contributor

@emkman emkman commented May 21, 2018

This is inspired by some previous open PRs: #2, #25, and #32. It enables support for importing one or more wallet addresses using private keys instead of a mnemonic. It supports a single key as a string, or an array of keys. The difference between this PRs and previous is that this fully implements the current HookedSubprovider interface and supports both the address_index and num_addresses options when passed an array. I have added updated documentation, both of this feature, and num_addresses generally, which is still undocumented in master. Also includes new and updated unit tests.

@julientregoat
Copy link

this would be really awesome

@cgewecke
Copy link
Contributor

cgewecke commented Jun 17, 2018

@emkman Agree with @julientregoat, this looks really nice. Will circle back here this week and pull this down for a more a careful review.

Sincere apologies for the delay in getting to it and thank you for opening.

Material to resolving the underlying problem in truffle 1003

@jsvisa
Copy link

jsvisa commented Sep 6, 2018

any update on this PR?

@emkman
Copy link
Contributor Author

emkman commented Sep 6, 2018

I don't think @cgewecke had time to review it over the last couple of months. Now it looks like master has moved forward with new nonce provider functionality so I would have to resolve those conflicts and retest. I could possibly do that this weekend but I would want to hear from @cgewecke first to make sure the update could get looked at. Otherwise, you can pull from my branch in your package.json, I can confirm it has been running in production since May.

@cgewecke
Copy link
Contributor

cgewecke commented Sep 6, 2018

Hi @emkman, @jsvisa. Apologies, really dropped the ball on this. This module gets treated like a step-child and needs more concentrated engineering resources. At the moment we're really only able address issues that are the source of chronic bugs at Truffle main repo, hence the nonce PR. . .

It's always possible to use someone's fork of the wallet if it adds features that are helpful. But I feel like we need to add some infrastructure here (testing and review wise) to really process PRs that extend the wallet's functionality in part because this is a sensitive piece of the stack. For example we should have tests that actually send transactions to Infura and run through a variety of scenarios.

I will talk to the rest of the team about this.

@emkman
Copy link
Contributor Author

emkman commented Sep 6, 2018

Agreed, I added some tests in my branch but they are still thin overall. Totally understand the need to focus resources. Let me know if I can help in any way!

@cgewecke
Copy link
Contributor

cgewecke commented Sep 6, 2018

@emkman Thanks so much, really sweet of you.

@benjamincburns benjamincburns merged commit 0f91b88 into trufflesuite:master Oct 23, 2018
@benjamincburns
Copy link
Contributor

Thanks for the awesome work @emkman!

@slyfox42
Copy link

Hi guys! When will these changes be published? Really looking forward to them 😁

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.

6 participants