search

urbit / urbit-key-generation

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

Ethereum addresses need to be checksummed #27

Closed Fang- closed 5 years ago

Fang- commented 5 years ago

They shouldn't be rendered as just plain hex strings, but should instead have capitalization as is standard for "checksummed Ethereum addresses" as per EIP-55.

Web3 supports generating and working with such addresses, but we may not want to pull that in as a dependency. Taking just the relevant code from it should be doable.

(This doesn't apply to keygen-hoon because that just gives you the number value and doesn't do any rendering. We may or may not (honestly, probably not) want a @uxe to literal/render checksummed Ethereum addresses.

flowerornament commented 5 years ago

I'm not sure I really understand EIP55, but I'd just like to mention that "all-lower" format is much easier to read than "all-upper," and that matters for Bridge UX.

Fang- commented 5 years ago

Checksumming does mixed lower and upper. I really don't think we should care about readability of hex strings of all things. Tiny security gain of doing checksumming > tiny ux gain of not doing it.

eglaysher commented 5 years ago

This is a pretty important safety feature and we should do it.

flowerornament commented 5 years ago

@Fang- I'm on a roll – I read this completely backwards as a necessitating mixed case → all-upper/lower vs all-upper → mixed case. I only meant that given a choice between all-upper and all-lower, we should do all-lower, but that doesn't make any sense in this context. 100% in favor.

jtobin commented 5 years ago

Closed in #34.