Fix deposit domain

[protolambda]

Forks are ignored for deposit validity: deposits are always accepted (if coming from the correct contract(s)).

Credits to @cemozerr for pointing out that the Validator and Beacon spec were inconsistent here.
Credits to @schroedingerscode for pointing out that the domain format changed some time ago (fork version and domain type swapped)

After looking into the inconsistency discussion on gitter, it occurred to me that deposits are not bound to an eth 2.0 fork, but to the eth 1.0 contract, and maybe shouldn't be signed. Vaguely remembering previous discussions about singatures with @JustinDrake in Sydney. This was confirmed to stay zeroed across forks (or previous GENESIS_FORK_VERSION), and here's the fix.

Note: I swapped back fork-version and domain type, as the change here looks unintentional, and signing/verifying with just the domain-type is more straight-forward this way.

[JustinDrake]

The current solution may be insufficient in the context of multiple deposit contracts. One approach is to assign each deposit contract a version and for deposits replace state.fork with deposit_contract.version in the calculation of domains.

[djrtwo]

I'm inclined to assume we have one deposit contract and to handle the exceptional case if we happen to change deposit contracts. Note that the rest of the eth1data machinery is not sufficient for multiple deposit contracts either

[protolambda]

Note that the rest of the eth1data machinery is not sufficient for multiple deposit contracts either

Deposit index is very much a singleton mechanism indeed.

Maybe it can be solved on the eth1 side, i.e. make the deposit contract flexible for permissioned integration of secondary contracts (for whatever reason you need a second contract to deal with deposits). However, I think the only reason for another contract is some kind of bug / unfortunate event, and in that case I suppose we fork the beacon chain for a new contract.

So what's the way forward? Create a helper function to more explicitly create a correct signature domain for deposits? I think this would be neat to just do with typing, if we had them...

A helper would be something like bls_domain(domain_type, fork_version), call bls_domain(DOMAIN_TYPE_DEPOSIT, b'\x00\x00\x00\x00')

[djrtwo]

so get_domain calls bls_domain, and when constructing a deposit domain, one calls bls_domain directly?

seems reasonable.

[djrtwo]

Made the adjustment of pulling out bls_domain into separate function and calling it directly when handling deposit domains.

Ready for review @protolambda @JustinDrake

[protolambda]

Defaulting to zero lets us avoid another pseudo-configurable constant. Generally also nicer to just read what the signature is about, i.e. the domain, and no more. And bls_domain nicely black-boxes the actual bytes being formatted into a BLS domain.

[djrtwo]

lgtm

[hwwhww]

Approving with one type hinting change request. 👍