Deposit issues

[djrtwo]

Issue

There are some known issues in the current deposit contract and deposit processing. This issue will probably be resolved via a couple of PRs

Known issues:

• by removing assert deposit_size == DEPOSIT_SIZE for the creation of a validator, a user can now create a validator with as little as 1 ETH. This validator would be inducted and immediately be kicked out of the validator set.
• Beyond that, this allows validators to enter the validator set (and begin participating) with as little as 16 ETH. The deposit mechanism should enforce 32 ETH as initial deposit to prevent validators entering with low and risky deposits.
• The deposit contract is incorrectly using full_deposit_count to create and broadcast the leaves of the merkle tree. If a < 32 ETH deposit is made, it does not update full_deposit_count but update the merkle tree, overwriting the previously written leaf. We need to maintain deposit_count separately from full_deposit_count. deposit_count should be incremented upon every successful deposit, should be used to construct the index into the merkle tree, and should be broadcast in the HashChainValue instead of full_deposit_count.

[vbuterin]

by removing assert deposit_size == DEPOSIT_SIZE for the creation of a validator, a user can now create a validator with as little as 1 ETH. This validator would be inducted and immediately be kicked out of the validator set.

Is this that bad? Their ETH is just stuck and they'll get it out in phase 2

Beyond that, this allows validators to enter the validator set (and begin participating) with as little as 16 ETH. The deposit mechanism should enforce 32 ETH as initial deposit to prevent validators entering with low and risky deposits.

I'm inclined to agree. I don't think the deposit contract needs to be too paternalistic; it's already (unavoidably) possible to deposit with a bad randao preimage, for example; we could just make deposits fail if they have less than 32 ETH.

The deposit contract is incorrectly using full_deposit_count to create and broadcast the leaves of the merkle tree. If a < 32 ETH deposit is made, it does not update full_deposit_count but update the merkle tree, overwriting the previously written leaf. We need to maintain deposit_count separately from full_deposit_count. deposit_count should be incremented upon every successful deposit, should be used to construct the index into the merkle tree, and should be broadcast in the HashChainValue instead of full_deposit_count.

Agree!

[djrtwo]

we could just make deposits fail if they have less than 32 ETH.

I'm inclined to go this route. If less than 32 eth and no existing validator for the pubkey, then fail.

[djrtwo]

addressed in #228