genesis_time in relation to GENESIS_EPOCH and GENESIS_SLOT

[ericsson49]

Beacon chain specification defines GENESIS_SLOT and GENESIS_EPOCH constants., but doesn't specify how they are related to genesis_time.
Intuitively, genesis_time should be equal to the GENESIS_SLOT start time (and GENESIS_EPOCH should be the epoch of GENESIS_SLOT).
However, fork choice calculates time ignoring GENESIS_SLOT.
It could be possible though that get_current_slot actually specifies the right way to convert time to a slot value (so that genesis_time means start time of Slot(0)).

There is no problem, if GENESIS_SLOT and GENESIS_EPOCH are zeros (and always will be), as they currently are. However, if it's admissible that GENESIS_SLOT/GENESIS_EPOCH change in future, then there can be problems with the beacon chain spec and/or with its implementations.
Overall, it's not clear now what is the right way to convert time to a slot number, i.e. should GENESIS_SLOT be added or not. While, it's trivial to implement, if 'GENESIS_SLOT` changes, it can be quite troublesome to verify that all the necessary updates are really performed.

Because of the above, we propose to clarify explicitly in the specification:

• whether GENESIS_SLOT/GENESIS_EPOCH are expected to change in future, or they are just symbolic names for 0 and never going to have a different value
• if it is admissible that GENESIS_SLOT can change for any reason, then the specification should clarify whether genesis_time corresponds to the start of Slot(0) or to the start of GENESIS_SLOT

Additionally, we propose to modify the fork choice spec, so that it uses slot instead of time. I.e. replace Store.time with Store.slot and on_tick(store, time) with on_tick(store, slot) (and other related changes). Basically, the Store.time is only used to detect whether a block/attestation is too early/too late or in-time, and current slot can be used for the same purpose.
This will get rid of the problem with GENESIS_SLOT entirely, in the context of the fork choice (sub)spec. But the problem remains for the purpose of time-to-slot conversion, of course. Additionally, it will simplify the on_block/on_attestation code and make it a bit more readable.
We can provide a Pull Request with the proposed fork choice modification, if needed.

[ericsson49]

@mkalinin also found a bug related to slot/time conversion, so I add as a comment here instead of opening a new issue.
on_attestation methods doesn't add genesis_time at one point, when calculated slot time:

# Attestations can only affect the fork choice of subsequent slots.
# Delay consideration in the fork choice until their slot is in the past.
assert store.time >= (attestation.data.slot + 1) * SECONDS_PER_SLOT

[djrtwo]

addressing this. Agree that assuming GENESIS_SLOT is zero here is slightly dangerous depending on network configurations in the future

[protolambda]

Closed by #1510