urbit / urbit-key-generation

Key derivation and HD wallet generation functions for Urbit
MIT License
15 stars 8 forks source link

Rename and restructure how wallet keys are organized in wallet output #15

Closed g-a-v-i-n closed 5 years ago

g-a-v-i-n commented 5 years ago

To clearly communicate what the values of these actually are, I propose we rename and restructure this group of subwallet data.

Current:

keys: {
   public: '...',
   private: '...',
   chain: '...'
}

new:

private: {
   key: '...' // same as 'private'
   chainCode: '...' // same as 'chain'
}
public {
   address: '...' //an ethereum address
   key: '...' // same as 'public'
}

public.key is very similar to public address, but I hesitate to suggest removing it before we're certain it can and should be removed.

jtobin commented 5 years ago

Maybe. I think the existing structure is simpler. The Ethereum address can be retrieved very easily from the keys, so I probably wouldn't bother rendering it here, and instead delegate that job to whatever is consuming the output. I'll come back to it.

g-a-v-i-n commented 5 years ago

up to you

Fang- commented 5 years ago

I actually feel sort of strongly about displaying the Ethereum addresses? They're more directly useful than the raw public keys.

g-a-v-i-n commented 5 years ago

The other thing to consider is that we will be using keygen-js in contexts where it would be nice to not have to do downstream work to do the conversion across an entire wallet object, like generating planet wallets to give away at Escape or otherwise.

flowerornament commented 5 years ago

I'd suggest something like this – only the public and private key are "keys" per se:

"owner": {
    "keys": {
      "public": "",
      "private": ""
    },
    "chain_code": "",
    "address": "",
    "seed": ""
  },
Fang- commented 5 years ago

Not that it matters all that much for the hoon code, but for now I'm going to assume @msutherl's suggestion is what we're going with here. (Though it seems a bit pedantic about the meaning of "keys".)

For completeness, you get the Ethereum address by keccak-256 hashing the public key and taking the last 20 bytes. Not sure if any of our dependencies already provide keccak-256 (sometimes incorrectly named sha3 by Ethereum code), or if we need to pull in something new.

jtobin commented 5 years ago

I'll consider this to be closed by #34 -- the output doesn't exactly match what was suggested here, but it does include additional information, etc.