search

ethereum / pyethapp

MIT License
1.28k stars 605 forks source link

Question: `mk_random_privkey` Entropy #251

Open dychen opened 7 years ago

dychen commented 7 years ago

Just scanning the code (nice work here!) and had a quick question:

In https://github.com/ethereum/pyethapp/blob/1f97f98b332f2a03cb58bd6bc78daa45181a3fb1/pyethapp/accounts.py#L22

I am no expert, but is there a reason to use this implementation instead of

os.urandom(32).encode('hex')

Both implementations use system-defined randomness, and I'm a bit worried about the loss of entropy of using zfill.

Also quick question: is there any reason you're storing the raw address returned by privtopub instead of the hex-encoded version? It looks like there's quite a bit of representation conversion back and forth, and I'm wondering if this could be avoided by just storing the hex.

dychen commented 7 years ago

@joeykrug thoughts?

joeykrug commented 7 years ago

I would use a non zfill version, no idea re raw address.

Would merge a PR w/ non zfill'd version [the loss of entropy isn't significant imo, but 10 years from now perhaps it will be so better to be safe]

dychen commented 7 years ago

Yeah, my main thought is it's easier and more maintainable to go with the simple version. Relatively trivial and maybe not worth a PR, but seems preferable in all respects to the existing implementation.