Miscellaneous comments

[JustinDrake]

Nitpicks/readability

1. "PoW main chain" => PoW chain
2. Run a spell checker (e.g. "rocess" => process, "parents block" => parent block)
3. Be consistent with American vs British English. E.g. "grey" => "gray"
4. Sometimes code formatting is missing. For example "For every shard S" => "For every shard S", "every CYCLE_LENGTH blocks" => "every CYCLE_LENGTH blocks", "into N pieces" => "into N pieces"
5. Be consistent with variable names. E.g. both withdrawal_addr and withdrawal_address are used (prefer the later), as well as bls_proof_of_possession and proof_of_possession.
6. Lint all the code for formatting. For example there are spacing inconsistencies status=0 (no spaces) vs pubkey = pubkey (with spaces).
7. Note PoW chain contract does no validation on the inputs (in particular, no validation of bls_proof_of_possession.
8. De-emphasise the comment about prioritize(block_hash, value). It's a side note (not strictly part of the Ethereum 2.0 spec) and is confusing at the very top.
9. Clean up the status states. Suggest: "pending log in", "logged in", "pending log out", "logged out", "exited"
10. Clearly label the slashing conditions as such. Searching for "slashing" only has hits in the TODO.
11. Consider adding comments in the code. For example meaty functions shuffle, get_new_shuffling, change_validators have no comments.
12. Consider replacing the half-words-half-code line-by-line assignments in "On startup" by a real on_startup function.
13. Same as above for "For each one of these attestations", "Balance recalculations related to FFG rewards", min_empty_validator, etc.
14. What does "[TODO]" mean after "For each one of these attestations" refer to?
15. Replace instances of "PoS chain" with "beacon chain" for consistency.
16. For the fork choice rule, clearly state it (in the notation of the spec). Deemphasize the ethresear.ch post.
17. Try to find a better name for penalized_in_wp, shard_and_committee_for_slots.
18. Avoid abbreviations in variable names (most variable names are ok!). For example last_state_recalc => last_state_recalculation.
19. Avoid numerical constants outside of the "Constants" section (e.g. "1024 shards").
20. Embrace the (now accepted and commonly used) word "Shasper". Better than the awkward (and inconsistent) phrasings in the spec: "PoS/sharding", "Casper+Sharding", "Casper/sharding"
21. Is the diagram "Shuffle -> Split by height -> Split by shard" still accurate? I've always been a bit confused by the different committee sizes at the end.
22. Some internal variable names are confusing, e.g. ifh_start, avs, sback, cp. Avoid one-letter variables (counters like i, j, k are fine, but things like o, m, d are harder to follow).
23. Clearly state the global clock assumption, and other relevant assumptions.
24. Be consistent between slot (e.g. in CrosslinkRecord) and slot_number (e.g. in beacon chain block).

Content

1. Clearly mention that the hash function based on BLAKE2b-512 is a placeholder that should be replaced by a STARK-friendly hash function for production.
2. Add (possibly in TODO) the double-batched Merkle accumulator for beacon chain blocks.
3. Add (possibly in TODO) the hardening of RANDAO against orphaned reveals (ethresear.ch post coming). Also flesh out the RANDAO part of the spec.
4. Replace sha3(pubkey) with hash(pubkey). Same for sha3("bye bye").
5. Should the state-transition be per-slot, not per-block? If there's a skip slot (e.g. the beacon proposer does not show up) recent_block_hashes should still be updated, right?
6. What are the rules for Merklelising the crystallized state? I would avoid a plain hash as this would not be friendly to light clients.

[djrtwo]

Thanks! I'll work through these shortly.

[JustinDrake]

Most of these are now addressed or part of the TODO list.