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

Recover key endpoint #292

Closed
wants to merge 2 commits into from
Closed

Recover key endpoint #292

wants to merge 2 commits into from

Conversation

faboweb
Copy link
Contributor

@faboweb faboweb commented Dec 6, 2017

The UI needs a way to recover keys from a mnemonic. I added an endpoint copying the behavior of the CLI.

I added this, but know that there are tests missing. Could someone maybe assist me with those?

@faboweb faboweb requested a review from ethanfrey December 6, 2017 18:09
@faboweb faboweb changed the title Fabo/recover key endpoint Recover key endpoint Dec 6, 2017
@ebuchman
Copy link
Member

ebuchman commented Dec 6, 2017

Is someone working on this ? I can look tonight

@ebuchman
Copy link
Member

ebuchman commented Dec 7, 2017

Working on this now. Does it need to return the seed? It shouldn't since it's already in the request right?

I'm also doing some refactoring to clean up this code a bit

@ebuchman
Copy link
Member

ebuchman commented Dec 7, 2017

As far as I can tell there are no tests for anything in rest within this repository. Is that correct ?

@ebuchman
Copy link
Member

ebuchman commented Dec 7, 2017

See #293

Includes these commits rebased on develop, plus some refactoring and test :)

@ebuchman ebuchman closed this Dec 7, 2017
@ethanfrey
Copy link
Contributor

Yeah, rest was in a proof of concept stage. There was no strong feedback ui team was using it, bad issues, etc until October, when development has halted on Sdk.

The only tests are integration tests under rest.sh
They ensure the calls work, but only check the happy path. Better than nothing.

Rest should be overhauled as part of the rewrite. But external functionality remain the same.

@ebuchman
Copy link
Member

ebuchman commented Dec 7, 2017

Thanks frey. Seems the client/rest package is relatively self contained and shouldn't need to change much post SDK re-write. Would you agree?

All the modules/xxx/rest of course will need to change.

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.

4 participants