Handle endianess in client implementation

[dangerousfood]

This design seems like it would have a high gas cost. An alternative might be to expect the client implementers to provide little endian values for the contract.

https://github.com/ethereum/deposit_contract/blob/a251fa8bf27f2b0ac74182db98bc311046b08d54/deposit_contract/contracts/validator_registration.v.py#L28-L44

[dangerousfood]

Another alternative would be to make this function pure and then have the clients call to_little_endian_64 before sending the deposit transaction.

[nisdas]

@dangerousfood Endianess of the deposit data is not an issue because it isnt processed by the contract, it is simply appended with the deposit_amount and timestamp and then logged out in the deposit log.

It does become an issue though when processing the logs from the contract where all the integers are by default big endian, so if we dont handle endianess in the contract, we would have to handle it on the client's end. Its trivial for a client to handle the endianess .

Agree on not handling it in the contract , if we want to optimize for gas efficiency we shouldn't be converting byte arrays in the contract.

[dangerousfood]

Agree on not handling it in the contract , if we want to optimize for gas efficiency we shouldn't be converting byte arrays in the contract.

So would you agree on my second approach? The clients calling to_little_endian_64 locally and providing little endian values to the deposit transaction. And the deposit method expecting little endian values rather than expending gas for the invocation of to_little_endian_64?

[djrtwo]

we are only calling to_little_endian_64 on two values -- the sent eth amount and the timestamp. Both of these can't be trustlessly sent as little endian data in the tx. The only two paths are to convert inside the contract or convert on the 2.0 client side

[nisdas]

@dangerousfood Wouldn't the deposit_data be little endian by default ? We would serialize this container using SSZ, ex:

{
    # BLS pubkey
    'pubkey': 'bytes48',
    # Withdrawal credentials
    'withdrawal_credentials': 'bytes32',
    # A BLS signature of this `DepositInput`
    'proof_of_possession': 'bytes96',
}

since the spec for ssz uses little-endian we wouldn't need to change anything.

Deposit Flow

1. Create deposit data using pubkey,withrawal_credentials and proof_of_possession of the validator.
2. Serialize this using SSZ.
3. Send a deposit to the contract with the serialized data.
4. The serialized data, along with the timestamp and deposit amount is then logged by the contract.
5. The client then processes these logs.

In my opinion, where clients would have to handle endianess on would be at step 5. Like @djrtwo said,the only two paths to convert would be in the contract or on the client's end. If we are optimizing it for gas efficiency then it should be handled on the client-side. We don't need to call to_little_endian_64 locally, since most languages have native libraries which handle conversion from little-endian to big-endian and vice versa

[djrtwo]

I imagine calling to_little_endian_64 twice is a small percentage of the overall gas cost of calling deposit and that it is worth keeping within the contract for cleanliness of handling the incoming deposit object in the 2.0 client.

[JustinDrake]

Calling to_little_endian_64 on deposit_amount is not optional because we are calculating the hash_tree_root of a DepositData object. (Separate from this issue we may change to big-endian—see #1046.)