Is this needed? This function could theoretically be used for different kinds of committees; doesn't seem wise to put a maximum designed around a specific type of committee.

It's a safety check since the size of CompactCommittee elements are set to maximum MAX_VALIDATORS_PER_COMMITTEE:

class CompactCommittee(Container):
    pubkeys: List[BLSPubkey, MAX_VALIDATORS_PER_COMMITTEE]
    compact_validators: List[uint64, MAX_VALIDATORS_PER_COMMITTEE]

It leads to one more implicit config validity condition of that MAX_VALIDATORS_PER_COMMITTEE >= TARGET_PERSISTENT_COMMITTEE_SIZE. (p.s. #407)

If we want to make this function could be used for different kinds of committees, how about refactoring this function into:

def pack_committee(state: BeaconState, committee: Sequence[ValidatorIndex]) -> Tuple[Sequence[BLSPubkey], Sequence[int]]:
    validators = [state.validators[i] for i in committee]
    compact_validators = [
        pack_compact_validator(i, v.slashed, v.effective_balance // EFFECTIVE_BALANCE_INCREMENT)
        for i, v in zip(committee, validators)
    ]
    pubkeys = [v.pubkey for v in validators]
    return pubkeys, compact_validators


def committee_to_compact_committee(state: BeaconState, committee: Sequence[ValidatorIndex]) -> CompactCommittee:
    assert len(committee) <= MAX_VALIDATORS_PER_COMMITTEE
    pubkeys, compact_validators = pack_committee(state, committee)
    return CompactCommittee(pubkeys=pubkeys, compact_validators=compact_validators)

What do you think about adding a custom type CompactValidator: uint64?

Edit: just realized that CompactValidator and unpack_compact_validator are defined in sync_protocol.md. I'd say:

1. Remove unpack_compact_validator from 1_beacon-chain-misc.md since it's not used here.
2. Move CompactValidator definition to 1_beacon-chain-misc.md.

Remove unpack_compact_validator from 1_beacon-chain-misc.md since it's not used here.

Where would it be put though? In the light client file that actually uses it? I can see how that's theoretically better in one way, but it has a big disadvantage, which is that pack and unpack are no longer beside each other. So I'd still favor putting both in the same file, to make it easier for readers to see that they are inverses.

Looks like ShardReceiptDelta is going away in #1383. Should we define the ShardReceiptDelta container here?

Going to merge this and handle the conflict after reviewing #1383. Thanks for pointing this out :)