search

urbit / azimuth-js

JavaScript bindings for Azimuth (https://github.com/urbit/azimuth)
MIT License
19 stars 12 forks source link

Remove and replace hdkey library #88

Closed cxk280 closed 2 years ago

cxk280 commented 2 years ago

The below came up in an email conversation between myself and @Fang- .

[hdkey(https://www.npmjs.com/package/hdkey) is only used with accounts~getKeyPair. You may wish to replace it, and while @Fang- feels like its inclusion is questionable, he also notes that removing it could be a breaking change. Nonetheless, we agreed that me creating an issue so this could be tracked would likely be a good idea.

jtobin commented 2 years ago

Yeah, in particular it could probably be swapped for the bip32 library, which seems more standard. That's what was done in urbit-key-generation. The pattern of e.g.:

const hd = hdkey.fromMasterSeed(seed)
const wallet = hd.derive("m/44'/60'/0'/0/0")

with hdkey can just be replaced by

const hd = bip32.fromSeed(seed)
const wallet = hd.derivePath("m/44'/60'/0'/0/0")

with bip32.

jtobin commented 2 years ago

Although, an important point of note: hdkey or bip32 should really be specified in package.json only as a peer & dev dependency, since it isn't actually used in the library proper (rather, getKeyPair is a function operating on a hdkey object, which is to be supplied from elsewhere). Replacing the hdkey library with bip32 here is only a couple-of-lines change for us, but it will require everyone downstream to manually switch from hdkey to bip32 in their codebase as well. That's a much bigger ask.

I noticed that the newest version of bip32 has a radically different API from the version used in urbit-key-generation, and the benefits of switching to that are probably dubious. I would be inclined to save everyone downstream the headache of switching libraries, unless there were some big demand to do so, and just stick with hdkey, which is perfectly serviceable.

jtobin commented 2 years ago

Resolved by #89.