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

Validator address checksums #1183

Closed
JustinDrake opened this issue Jun 15, 2019 · 15 comments
Closed

Validator address checksums #1183

JustinDrake opened this issue Jun 15, 2019 · 15 comments

Comments

@JustinDrake
Copy link
Collaborator

Both @benjaminion and @cdetrio (see here) argue for consensus-level checksums for validator addresses.

Creating this issue for discussion :)

@JustinDrake
Copy link
Collaborator Author

Vitalik suggests pushing for a base58check similar to Bitcoin, outside of the consensus layer. This standard would be socially enforced from day 1, instead of being retro-fitted like EIP55.

@vbuterin
Copy link
Contributor

vbuterin commented Jun 16, 2019

I actually quite aesthetically dislike base58check 😄

My favorite choices would be:

  1. Stick with mixed-case hex like in ETH1, but make sure all clients verify it from day 1. With the length increase from 20 to 32 bytes, it becomes significantly more robust.
  2. zBase32 (with checksums). Better than base58 as you can evaluate it 5 bytes at a time so you don't need bigints.

@JustinDrake
Copy link
Collaborator Author

Stick with mixed-case hex like in ETH1 [...] With the length increase from 20 to 32 byte

so you don't need bigints

Does EIP55 also use bigints with utils.big_endian_to_int(utils.sha3(addr.hex()))? It also uses big-endian encoding which I guess is another argument to consider in #1046.

@vbuterin
Copy link
Contributor

vbuterin commented Jun 17, 2019

That's just using the hash as a bitfield (the only use of v is o += c.upper() if (v & (2**(255 - i))) else c.lower()); it's completely doable without converting to ints at any step of the process, I just wrote it that way because it was the simplest to write in python.

@dankrad
Copy link
Contributor

dankrad commented Jun 19, 2019

I actually quite aesthetically dislike base58check

What do you dislike about it in particular? Surely having to use a bigint is rather marginal
(Shorter addresses seem like a plus to me)

@vbuterin
Copy link
Contributor

vbuterin commented Jun 21, 2019

Copying from bitcoin documentation (bitcoin is trying to switch from base58 to a base32 variant):

  • Base58 needs a lot of space in QR codes, as it cannot use the alphanumeric mode.
  • The mixed case in base58 makes it inconvenient to reliably write down, type on mobile keyboards, or read out loud.
  • Most of the research on error-detecting codes only applies to character-set sizes that are a prime power, which 58 is not.
  • Base58 decoding is complicated and relatively slow.

@vbuterin
Copy link
Contributor

We could potentially even use bech32 as is.

@mcdee
Copy link
Contributor

mcdee commented Sep 19, 2019

As mentioned at https://ethresear.ch/t/ethereum-2-address-format/6138 bech32 seems like a decent option.

What would the process for public key to address be, though? Seem to be that there are three main options:

  • use the public key directly
  • use the sha256() of the public key
  • use the last 20 bytes of the sha256() of the public key

which would give, as an example, the following:

  • a99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c
  • dbfad2a6bfb0e7f1f0f45460944fbd8dfa7f37da06a4d13b3983cc90bb46963b
  • 944fbd8dfa7f37da06a4d13b3983cc90bb46963b

with bech32 encoding of these being (assuming an "eth" prefix):

  • eth14xd8dmthjmmmugk4kl59mm4hc4nhaz89z8stxdmp37xyadsnfx6t7tg48ajf776nxk073w228rjycmj83h0
  • eth1m0ad9f4lkrnlru8523sfgnaa3ha87d76q6jdzwees0xfpw6xjcas7ucq5e
  • eth1j38mmr060uma5p4y6yannq7vjza5d93md9du2k

Personally I think the first one is too long, but either of the latter two could work.

@djrtwo
Copy link
Contributor

djrtwo commented Nov 5, 2019

@mcdee I think addressing via a hash generally makes sense here. As for the hash vs the 20 bytes, I think we have generally been thinking that there is no need to reduce the security by using 20 bytes like eth1 and should instead use the full 32

@JustinDrake
Copy link
Collaborator Author

JustinDrake commented Dec 15, 2019

@CarlBeek What is the status of checksums? (I understood using bech32 out of the box doesn't work.) Are you pursuing EIP 2336 (a closed PR)?

@protolambda
Copy link
Collaborator

Careful there with new Bech32 use-cases. Recently there was a (small, but oversight) issue found in Bech32: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-December/017521.html

@sgryphon
Copy link
Contributor

+1 to having checksums from day 1.

While trying to process some Eth1 payments recently, I ran into some annoying lack of checksums, and thought of the Eth2 implementation. Before raising a new issue, I thought I would check if it had already been discussed and found it it still an open issue.

There are two places I see checksums being useful:
(1) withdrawal credentials - 32 bytes, kind of like an address (is a hash with bytes truncated)
(2) validator public key - 48 bytes, and you need the private key to sign the deposit message so is probably going to be derived (not typed in)

The eth2 values are likely going to be more specialised users than general eth1 address, but even if they are derived values, I still think there are benefits of having a checksum:

  • even they are moved from one computer to another manually (e.g. typed from a mobile message into a PC browser), it provides a check on typos
  • setting a precedent from day 1 is probably a good idea, to avoid tacking something on later.

I would like to see this issue implemented.

@JustinDrake
Copy link
Collaborator Author

cc @CarlBeek

@sgryphon
Copy link
Contributor

For validator public key:

A kind of weird thought, if you don't want to change the data size, and have a checksum that is completely invisible to the current implementation, would be to specify valid keys as only those whose public key where the last 1 or 2 bytes are a valid checksum for the other bytes.

i.e. add a simple CRC-8 or CRC-16 check by simply restricting the domain of valid values.

This may mean you need to calculate ~256 private keys before you randomly get one who public key happens that happens to meet the criteria, so would add time to key generation. (If it was CRC-16 then that would be ~16,000 generations.)

It would also be reducing the effective key size by at least 8 bits (or maybe 16 bits as the private key is 2x public). I don't know what other cryptographic effects it might have, and it would need to be analyzed theoretically to see if such a constraint causes issues, e.g. something about the curve makes CRC values more rare than expected.

(It should be easy enough to check if there is any correlation, by seeing if the actual rate of randomly meeting the criteria matches the expected rate or not).


For withdrawal credentials it is even easier, as it is already just a hash value that we trim down to 31 bytes... easy enough to trim the hash down to 29 bytes, so:

withdrawal credentials = version byte + hash bytes 2-30 + 2 byte CRC-16

This doesn't have any impact on the keys, just the withdrawal "address" becomes only 29 bytes.

There would be no impact on any of the code either; at least not in phase 0... it would only be relevant to validate withdrawals. And could even be made optional by having withdrawal type 1 (checks 31 bytes match) and a new type 2 (checks 29 bytes match and the last 2 are the CRC).


Note that adding a CRC to the bytes representation is different from having an encoding for addresses (e.g. Base 32) and/or have an error detection scheme specifically designed to detect transpositions, repetitions, etc. You could even have both if you wanted.

@JustinDrake
Copy link
Collaborator Author

The rough consensus from the EF research team seems to be that checksums for validator pubkeys don't have a well-defined use case. The key actions involving validator pubkeys (namely deposits and transfers) have some in-built protections. For example, the deposit interface will check the proof of possession, the deposit contract has a deposit data root check, and transfers can only be made to an existing validator.

Closing for now but feel free to open another issue if you feel otherwise :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants