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

Update LCD keys endpoint for crypto/keybase API updates #1442

Closed
cwgoes opened this issue Jun 28, 2018 · 5 comments
Closed

Update LCD keys endpoint for crypto/keybase API updates #1442

cwgoes opened this issue Jun 28, 2018 · 5 comments

Comments

@cwgoes
Copy link
Contributor

cwgoes commented Jun 28, 2018

Do we need separate endpoints to return a seed and derive a key from that seed?

Ref 3533fc5

(we definitely want the ability to derive new keys from an existing seed; Derive will do that with an HD path)

@cwgoes cwgoes changed the title Update LCD keys endpoint for crypto/keybase API updates? Update LCD keys endpoint for crypto/keybase API updates Jun 28, 2018
@fedekunze fedekunze added lite and removed lcd labels Sep 13, 2018
@fedekunze
Copy link
Collaborator

fedekunze commented Sep 13, 2018

@cwgoes are you talking about the keys/seed endpoint ? we should refactor the handler as well, as it doesn't return any type of errors

// function to just a new seed to display in the UI before actually persisting it in the keybase
func getSeed(algo keys.SigningAlgo) string {
kb := client.MockKeyBase()
pass := "throwing-this-key-away"
name := "inmemorykey"
_, seed, _ := kb.CreateMnemonic(name, keys.English, pass, algo)
return seed
}
// Seed REST request handler
func SeedRequestHandler(w http.ResponseWriter, r *http.Request) {
vars := mux.Vars(r)
algoType := vars["type"]
// algo type defaults to secp256k1
if algoType == "" {
algoType = "secp256k1"
}
algo := keys.SigningAlgo(algoType)
seed := getSeed(algo)
w.Write([]byte(seed))
}

@jackzampolin
Copy link
Member

Is this issue still accurate? Do we need to do any additional work in this area cc @fedekunze

@cwgoes
Copy link
Contributor Author

cwgoes commented Oct 12, 2018

I'm not sure what the current state is - also there's a PR in progress: #2090.

We should definitely evaluate the endpoints to ensure DRY and minimize the likelihood that a caller will do something they didn't intend, especially around key derivation.

@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 16, 2018

There's a bug in the current design:

  1. POST /keys a new key with name Chris.
  2. GET /keys/Chris returns the new key.
  3. POST /keys a new key with name seed.
  4. GET /keys/seed does not return the new key, violating the REST abstraction of keys as a collection.

This confused a contract test I wrote and I had to add extra code to work around the bug.

@fedekunze
Copy link
Collaborator

closing as we don't have keys exposed to the REST API anymore

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

No branches or pull requests

4 participants