Rename 'slot' to 'slot_number'

[tarasbob]

I think this makes it clearer and explicit. The word "slot" alone intuitively feels like we are referring to some kind of object instead of a number.

[hwwhww]

@tarasbob could you change to against dev branch?

I remember that we discussed it when we renamed GENESIS_SLOT_NUMBER to GENESIS_SLOT. (/cc @JustinDrake) Personally, I'm fine with slot. 😅

[tarasbob]

I was just reading the spec and trying to understand it. It feels to me that adding the explicit word "number" makes it easier to understand for people who are not familiar with the spec. (For example, it reminds the reader that there are many slots, one after another. Especially since "slot" is a new term, I don't think it's used in Eth 1.0 or other blockchains afaik.)

The same reasoning could apply to renaming shard to shard_number.

[paulhauner]

I respectfully disagree with this change:

1. It's overly verbose. The definition of slot in BeaconChain states clearly it is a uint64. I think it's fair to assume that a uint64 is a number, unless it's being used as a bitfield (see justification_bitfield).
2. It's a slippery slope. One could start to argue that total_balance should be total_balance_number to ensure you're not refering to a TotalBalance struct. Futhermore, beacon_block_root could become beacon_block_root_bytes.
3. It's going to require 8+ implementation teams to refactor their code bases.

With regards to shard, I find it unlikely that there's going to be an object called Shard.

[JustinDrake]

We're going the other direction 😂and removing Number from the SlotNumber, EpochNumber, ShardNumber types #534

It's going to require 8+ implementation teams to refactor their code bases.

The good news is that naming is not technically part of the spec. The spec merely tries to make a sane suggestion. Implementers can use slot_number internally if they want!

[paulhauner]

The good news is that naming is not technically part of the spec. The spec merely tries to make a sane suggestion. Implementers can use slot_number internally if they want!

Is the ordering of fields in the SSZ list still lexicographic? If so, spec name changes can potentially affect implementations.

EDIT: Additionally, keeping naming the same as the spec really helps with cross-referencing whenever the spec changes. Personally, I would change our variable names to match the spec. I speak from experience because my first beacon chain impl used different function names and it was very difficult to track changes. But you're right, 8+ teams aren't "required" to refactor.

[djrtwo]

@paulhauner Ordering of ssz fields is based upon ordering in defined datastructure rather than lexicographical. Implementations can name whatever they want (although following spec a decent amount will aid in readability).

[paulhauner]

@paulhauner Ordering of ssz fields is based upon ordering in defined datastructure rather than lexicographical. Implementations can name whatever they want (although following spec a decent amount will aid in readability).

Great, I think this is a good call. I remember it being lexicographic at one stage, but I might be making that up. Lighthouse is using the order they're defined.

[djrtwo]

It did use to be lexicographic!
New way is better for sure.

[djrtwo]

Closing. I think using Slot as one of the custom types (seed #534) will make this clear enough and not add verbosity.