Revamp 1_shard-data-chains.md

[JustinDrake]

TODO:

• Add function descriptions.

PR dependencies:

1. compute_proposer_index from Improve beacon proposer selection logic #1371

Renamings:

• persistent committee -> shard committee
• phase 1 fork -> shard genesis
• block data -> block body
• attester bitfield -> aggregation bits
• basefee -> block body price
• total bytes -> block size sum
• earlier committee -> older committee
• later committee -> newer committee
• most recent block core -> latest block header
• target persistent committee size -> max period committee size

Substantive changes:

• Use FlatContainer for shard blocks and state
• Use Vectors instead of Lists in shard state to be friendly to flattening
• Make shard attestations sign ShardCheckpoints (as opposed to parent_roots)
• Make block bodies be multiples of SHARD_HEADER_SIZE (simpler and partially cachable Merkleisation)
• Shuffle indices in get_shard_proposer_index (also make it consistent with phase 0—see Improve beacon proposer selection logic #1371)
• Remove receipt_root (instead use older_committee_rewards and compute indices on the beacon chain)
• Reorder container fields into semantic groups
• Make fee and rewards logic use Gwei everywhere (and reuse get_base_reward from phase 0)
• Unify fees and rewards under _deltas
• Make body of type List[byte, SHARD_BLOCK_SIZE_LIMIT - SHARD_HEADER_SIZE] (as opposed to Bytes[SHARD_BLOCK_SIZE_LIMIT - SHARD_HEADER_SIZE])
• Added missing 2 * in range(len(persistent_committee), 2 * MAX_PERSISTENT_COMMITTEE_SIZE)
• Do not needlessly deduplicate shard committees (by construction there are no duplicates) do not sort validator indices in get_shard_committee
• Make MAX_BLOCK_SIZE_PRICE less conservative (32 ETH as opposed to 1 ETH)—fees are split across the shard committee anyway
• Added shard check in process_shard_block_header
• Added beacon_chain_root check in process_shard_block_header
• Change len(data.body) to len(data.body) + SHARD_HEADER_SIZE in process_shard_block (as for block_size_sum)
• Fix bug in process_shard_block_size_fee when computing MAX_BLOCK_SIZE_PRICE (should have been // SLOTS_PER_EPOCH as opposed to * SLOTS_PER_EPOCH)
• Do not allow block.parent_root == Bytes32() to bypass parent root check
• Possibly fixed a few bugs not listed above; most certainly added a few bugs of my own

Misc cosmetic changes:

• Flesh out table of contents
• Order containers topologically
• Add various code comments
• Remove "Beacon attestations" section and associated helpers (to go in 1_beacon-chain-misc.md—see Starting on phase 1 misc beacon changes #1323)
• Add MIN_BLOCK_SIZE_PRICE parameter
• Remove PHASE_1_FORK_SLOT, CROSSLINK_LOOKBACK, PLACEHOLDER parameters
• Remove SHARD_SLOTS_PER_BEACON_SLOT in favour of SHARD_SLOTS_PER_EPOCH
• Avoid raise Exception which is not used in phase 0
• Split monolithic shard_block_transition into three functions (header, attestations, block size fee)
• Make function signatures consistent (e.g. BeaconState parameter, then ShardState, then Shard)
• Make function parameter names consistent (e.g. use state everywhere for BeaconState, and shard_state for ShardState)
• Remove unused compute_slot_of_shard_slot

[vbuterin]

This is actually a (big) problem with this approach. With a flat container, you no longer have this nice property that an object with an entire subtree substituted with its root necessarily has the same hash tree root as the original object, so you can't have shard blocks and shard block headers in the same way. Not sure how to get around this within the FlatContainer model.

This is arguably one reason why not doing flat containers and instead taking Danny's suggestion and having "embedded vectors (and containers)" would be better. Basically, the idea would be that if you have a container as follows:

class Foo(Container):
    bar: uint64
    baz: Embedded[Vector[uint64, 4]]
    bav: Bytes32

It would be treated identically to:

class Foo(Container):
    bar: uint64
    baz_0: uint64
    baz_1: uint64
    baz_2: uint64
    baz_3: uint64
    bav: Bytes32

This way you avoid introducing a separate type of container and instead have flattening as a feature that you can use or not use for specific elements at will.

[djrtwo]

Was just about to question if we indeed get this property anymore. The embedded looks like a reasonable approach

[vbuterin]

So are we going to replace FlatContainer with Embedded here? I guess the attestations and signature would need to be part of a 2-item Embedded container where the items are themselves Embedded to get flatness while gaining the ability to use the standard signature root method?

[djrtwo]

This is awesome! super pleased with the parallels to the structure in phase 0. It makes it really easy to reason about and check for correctness.

Great work :)

I have a number of comments. I'm not 100% done reviewing, but don't want to leave you hanging as I'll be afk for much of the rest of the day. Trying to finish my review up tonight

[djrtwo]

Was just about to question if we indeed get this property anymore. The embedded looks like a reasonable approach

[terencechain]

Any reasons we not using attestation_signatures? I find attestations a bit miss-leading

[terencechain]

Suggested change

	### Slot processing

[djrtwo]

great work! still things to be done and some potential changes to discuss with crosslinking, but this is good enough to get into dev for now