{"action":"closed","issue":{"url":"https://api.github.com/repos/ethereum/eth2.0-specs/issues/21","repository_url":"https://api.github.com/repos/ethereum/eth2.0-specs","labels_url":"https://api.github.com/repos/ethereum/eth2.0-specs/issues/21/labels{/name}","comments_url":"https://api.github.com/repos/ethereum/eth2.0-specs/issues/21/comments","events_url":"https://api.github.com/repos/ethereum/eth2.0-specs/issues/21/events","html_url":"https://github.com/ethereum/eth2.0-specs/issues/21","id":365852408,"node_id":"MDU6SXNzdWUzNjU4NTI0MDg=","number":21,"title":"Miscellaneous comments","user":{"login":"JustinDrake","id":731743,"node_id":"MDQ6VXNlcjczMTc0Mw==","avatar_url":"https://avatars1.githubusercontent.com/u/731743?v=4","gravatar_id":"","url":"https://api.github.com/users/JustinDrake","html_url":"https://github.com/JustinDrake","followers_url":"https://api.github.com/users/JustinDrake/followers","following_url":"https://api.github.com/users/JustinDrake/following{/other_user}","gists_url":"https://api.github.com/users/JustinDrake/gists{/gist_id}","starred_url":"https://api.github.com/users/JustinDrake/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JustinDrake/subscriptions","organizations_url":"https://api.github.com/users/JustinDrake/orgs","repos_url":"https://api.github.com/users/JustinDrake/repos","events_url":"https://api.github.com/users/JustinDrake/events{/privacy}","received_events_url":"https://api.github.com/users/JustinDrake/received_events","type":"User","site_admin":false},"labels":[{"id":1065467474,"node_id":"MDU6TGFiZWwxMDY1NDY3NDc0","url":"https://api.github.com/repos/ethereum/eth2.0-specs/labels/proofread","name":"proofread","color":"81dd5f","default":false}],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-10-02T11:22:48Z","updated_at":"2018-10-15T18:57:52Z","closed_at":"2018-10-15T18:57:52Z","author_association":"COLLABORATOR","body":"**Nitpicks/readability**\r\n\r\n1. \"PoW main chain\" => PoW chain\r\n2. Run a spell checker (e.g. \"rocess\" => process, \"parents block\" => parent block)\r\n3. Be consistent with American vs British English. E.g. \"grey\" => \"gray\"\r\n4. 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\"\r\n5. 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`.\r\n6. Lint all the code for formatting. For example there are spacing inconsistencies `status=0` (no spaces) vs `pubkey = pubkey` (with spaces).\r\n7. Note PoW chain contract does no validation on the inputs (in particular, no validation of `bls_proof_of_possession`.\r\n8. 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.\r\n9. Clean up the `status` states. Suggest: \"pending log in\", \"logged in\", \"pending log out\", \"logged out\", \"exited\"\r\n10. Clearly label the slashing conditions as such. Searching for \"slashing\" only has hits in the TODO.\r\n11. Consider adding comments in the code. For example meaty functions `shuffle`, `get_new_shuffling`, `change_validators` have no comments.\r\n12. Consider replacing the half-words-half-code line-by-line assignments in \"On startup\" by a real `on_startup` function.\r\n13. Same as above for \"For each one of these attestations\", \"Balance recalculations related to FFG rewards\", `min_empty_validator`, etc.\r\n14. What does \"[TODO]\" mean after \"For each one of these attestations\" refer to?\r\n15. Replace instances of \"PoS chain\" with \"beacon chain\" for consistency.\r\n16. For the fork choice rule, clearly state it (in the notation of the spec). Deemphasize the ethresear.ch post.\r\n17. Try to find a better name for `penalized_in_wp`, `shard_and_committee_for_slots`.\r\n18. Avoid abbreviations in variable names (most variable names are ok!). For example `last_state_recalc` => `last_state_recalculation`.\r\n19. Avoid numerical constants outside of the \"Constants\" section (e.g. \"1024 shards\").\r\n20. 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\"\r\n21. 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.\r\n22. 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).\r\n23. Clearly state the global clock assumption, and other relevant assumptions.\r\n24. Be consistent between `slot` (e.g. in `CrosslinkRecord`) and `slot_number` (e.g. in beacon chain block).\r\n\r\n**Content**\r\n\r\n1. 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.\r\n2. Add (possibly in TODO) the double-batched Merkle accumulator for beacon chain blocks.\r\n3. Add (possibly in TODO) the hardening of RANDAO against orphaned reveals (ethresear.ch post coming). Also flesh out the RANDAO part of the spec.\r\n4. Replace `sha3(pubkey)` with `hash(pubkey)`. Same for `sha3(\"bye bye\")`.\r\n5. 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?\r\n6. What are the rules for Merklelising the crystallized state? I would avoid a plain hash as this would not be friendly to light clients.\r\n"}}
{"actor":{"display_login":"JustinDrake"}}