Apply suggestions from @djrtwo's code review

scripts/build_spec.py

@@ -6,6 +6,7 @@ from function_puller import (
 from argparse import ArgumentParser
 from typing import (
     Dict,
+    List,
     Optional,
 )
 
@@ -122,13 +123,16 @@ def objects_to_spec(functions: Dict[str, str],
                     constants: Dict[str, str],
                     ssz_objects: Dict[str, str],
                     inserts: Dict[str, str],
-                    imports: Dict[str, str]) -> str:
+                    imports: Dict[str, str],
+                    new_types: Dict[str, str],
+                    byte_types: List[int],
+                    ) -> str:
     """
     Given all the objects that constitute a spec, combine them into a single pyfile.
     """
     new_type_definitions = \
-        '\n'.join(['''%s = NewType('%s', %s)''' % (key, key, value) for key, value in NEW_TYPES.items()])
+        '\n'.join(['''%s = NewType('%s', %s)''' % (key, key, value) for key, value in new_types.items()])
-    new_type_definitions += '\n' + '\n'.join(['Bytes%s = BytesN[%s]' % (n, n) for n in BYTE_TYPES])
+    new_type_definitions += '\n' + '\n'.join(['Bytes%s = BytesN[%s]' % (n, n) for n in byte_types])
     functions_spec = '\n\n'.join(functions.values())
     constants_spec = '\n'.join(map(lambda x: '%s = %s' % (x, constants[x]), constants))
     ssz_objects_instantiation_spec = '\n\n'.join(ssz_objects.values())
@@ -217,11 +221,10 @@ def combine_spec_objects(spec0: SpecObject, spec1: SpecObject) -> SpecObject:
 
 def build_phase0_spec(sourcefile: str, outfile: str=None) -> Optional[str]:
     functions, constants, ssz_objects, inserts = get_spec(sourcefile)
-    spec = objects_to_spec(functions, constants, ssz_objects, inserts, PHASE0_IMPORTS)
+    spec = objects_to_spec(functions, constants, ssz_objects, inserts, PHASE0_IMPORTS, NEW_TYPES, BYTE_TYPES)
     if outfile is not None:
         with open(outfile, 'w') as out:
             out.write(spec)
-    else:
     return spec
 
 
@@ -235,11 +238,10 @@ def build_phase1_spec(phase0_sourcefile: str,
     spec_objects = phase0_spec
     for value in [phase1_custody, phase1_shard_data]:
         spec_objects = combine_spec_objects(spec_objects, value)
-    spec = objects_to_spec(*spec_objects, PHASE1_IMPORTS)
+    spec = objects_to_spec(*spec_objects, PHASE1_IMPORTS, NEW_TYPES, BYTE_TYPES)
     if outfile is not None:
         with open(outfile, 'w') as out:
             out.write(spec)
-    else:
     return spec
 
 
@@ -262,7 +264,10 @@ If building phase 1:
 
     args = parser.parse_args()
     if args.phase == 0:
+        if len(args.files) == 2:
             build_phase0_spec(*args.files)
+        else:
+            print(" Phase 0 requires an output as well as an input file.")
     elif args.phase == 1:
         if len(args.files) == 4:
             build_phase1_spec(*args.files)

specs/core/0_beacon-chain.md

@@ -1246,6 +1246,8 @@ def process_slot(state: BeaconState) -> None:
 
 ### Epoch processing
 
+Note: the `# @LabelHere` lines below are placeholders to show that code will be inserted here in a future phase.
+
 ```python
 def process_epoch(state: BeaconState) -> None:
     process_justification_and_finalization(state)

test_libs/pyspec/eth2spec/test/context.py

@@ -112,11 +112,11 @@ def with_phase1(fn):
 
 def with_all_phases(fn):
     """
-    Decorator to run everything with all availible spec phases
+    Decorator to run everything with all available spec phases
     """
     def entry(*args, **kw):
         kw['spec'] = spec_phase0
         fn(*args, **kw)
         kw['spec'] = spec_phase1
-        return fn(*args, **kw)
+        fn(*args, **kw)
     return entry

test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py

@@ -1,6 +1,6 @@
 from copy import deepcopy
 
-from eth2spec.test.context import spec_state_test, expect_assertion_error, always_bls, with_all_phases
+from eth2spec.test.context import spec_state_test, expect_assertion_error, always_bls, with_all_phases, with_phase0
 from eth2spec.test.helpers.attestations import (
     get_valid_attestation,
     sign_attestation,
@@ -214,7 +214,7 @@ def test_bad_source_root(spec, state):
     yield from run_attestation_processing(spec, state, attestation, False)
 
 
-@with_all_phases
+@with_phase0
 @spec_state_test
 def test_non_zero_crosslink_data_root(spec, state):
     attestation = get_valid_attestation(spec, state)
@@ -288,7 +288,7 @@ def test_inconsistent_bitfields(spec, state):
     yield from run_attestation_processing(spec, state, attestation, False)
 
 
-@with_all_phases
+@with_phase0
 @spec_state_test
 def test_non_empty_custody_bitfield(spec, state):
     attestation = get_valid_attestation(spec, state)

test_libs/pyspec/eth2spec/test/phase_1/block_processing/test_process_early_derived_secret_reveal.py

@@ -1,6 +1,6 @@
-import pytest
-
 from eth2spec.test.helpers.custody import get_valid_early_derived_secret_reveal
+from eth2spec.test.helpers.block import apply_empty_block
+from eth2spec.test.helpers.state import next_epoch
 from eth2spec.test.context import with_phase1, spec_state_test, expect_assertion_error
 
 
@@ -56,8 +56,8 @@ def test_reveal_from_current_epoch(spec, state):
 @with_phase1
 @spec_state_test
 def test_reveal_from_past_epoch(spec, state):
-    if spec.get_current_epoch(state) < 1:
+    next_epoch(spec, state)
-        pytest.skip('testing of previous epoch requires epoch of at least 1')
+    apply_empty_block(spec, state)
     randao_key_reveal = get_valid_early_derived_secret_reveal(spec, state, spec.get_current_epoch(state) - 1)
 
     yield from run_early_derived_secret_reveal_processing(spec, state, randao_key_reveal, False)

test_libs/pyspec/eth2spec/utils/minimal_ssz.py

@@ -1,340 +0,0 @@
-from typing import Any
-
-from .hash_function import hash
-
-BYTES_PER_CHUNK = 32
-BYTES_PER_LENGTH_OFFSET = 4
-ZERO_CHUNK = b'\x00' * BYTES_PER_CHUNK
-
-
-def SSZType(fields):
-    class SSZObject():
-        def __init__(self, **kwargs):
-            for f, t in fields.items():
-                if f not in kwargs:
-                    setattr(self, f, get_zero_value(t))
-                else:
-                    setattr(self, f, kwargs[f])
-
-        def __eq__(self, other):
-            return self.fields == other.fields and self.serialize() == other.serialize()
-
-        def __hash__(self):
-            return int.from_bytes(self.hash_tree_root(), byteorder="little")
-
-        def __str__(self):
-            output = []
-            for field in self.fields:
-                output.append(f'{field}: {getattr(self, field)}')
-            return "\n".join(output)
-
-        def serialize(self):
-            return serialize_value(self, self.__class__)
-
-        def hash_tree_root(self):
-            return hash_tree_root(self, self.__class__)
-
-    SSZObject.fields = fields
-    return SSZObject
-
-
-class Vector():
-    def __init__(self, items):
-        self.items = items
-        self.length = len(items)
-
-    def __getitem__(self, key):
-        return self.items[key]
-
-    def __setitem__(self, key, value):
-        self.items[key] = value
-
-    def __iter__(self):
-        return iter(self.items)
-
-    def __len__(self):
-        return self.length
-
-
-type_of = type
-
-
-def empty(obj):
-    for field in obj.fields:
-        field = get_zero_value(field)
-    return obj
-
-
-def is_basic(typ):
-    # if not a string, it is a complex, and cannot be basic
-    if not isinstance(typ, str):
-        return False
-    # "uintN": N-bit unsigned integer (where N in [8, 16, 32, 64, 128, 256])
-    elif typ[:4] == 'uint' and typ[4:] in ['8', '16', '32', '64', '128', '256']:
-        return True
-    # "bool": True or False
-    elif typ == 'bool':
-        return True
-    # alias: "byte" -> "uint8"
-    elif typ == 'byte':
-        return True
-    # default
-    else:
-        return False
-
-
-def is_constant_sized(typ):
-    # basic objects are fixed size by definition
-    if is_basic(typ):
-        return True
-    # dynamic size array type, "list": [elem_type].
-    # Not constant size by definition.
-    elif isinstance(typ, list) and len(typ) == 1:
-        return False
-    # fixed size array type, "vector": [elem_type, length]
-    # Constant size, but only if the elements are.
-    elif isinstance(typ, list) and len(typ) == 2:
-        return is_constant_sized(typ[0])
-    # bytes array (fixed or dynamic size)
-    elif isinstance(typ, str) and typ[:5] == 'bytes':
-        # if no length suffix, it has a dynamic size
-        return typ != 'bytes'
-    # containers are only constant-size if all of the fields are constant size.
-    elif hasattr(typ, 'fields'):
-        for subtype in typ.fields.values():
-            if not is_constant_sized(subtype):
-                return False
-        return True
-    else:
-        raise Exception("Type not recognized")
-
-
-def coerce_to_bytes(x):
-    if isinstance(x, str):
-        o = x.encode('utf-8')
-        assert len(o) == len(x)
-        return o
-    elif isinstance(x, bytes):
-        return x
-    else:
-        raise Exception("Expecting bytes")
-
-
-def encode_series(values, types):
-    # Recursively serialize
-    parts = [(is_constant_sized(types[i]), serialize_value(values[i], types[i])) for i in range(len(values))]
-
-    # Compute and check lengths
-    fixed_lengths = [len(serialized) if constant_size else BYTES_PER_LENGTH_OFFSET
-                     for (constant_size, serialized) in parts]
-    variable_lengths = [len(serialized) if not constant_size else 0
-                        for (constant_size, serialized) in parts]
-
-    # Check if integer is not out of bounds (Python)
-    assert sum(fixed_lengths + variable_lengths) < 2 ** (BYTES_PER_LENGTH_OFFSET * 8)
-
-    # Interleave offsets of variable-size parts with fixed-size parts.
-    # Avoid quadratic complexity in calculation of offsets.
-    offset = sum(fixed_lengths)
-    variable_parts = []
-    fixed_parts = []
-    for (constant_size, serialized) in parts:
-        if constant_size:
-            fixed_parts.append(serialized)
-        else:
-            fixed_parts.append(offset.to_bytes(BYTES_PER_LENGTH_OFFSET, 'little'))
-            variable_parts.append(serialized)
-            offset += len(serialized)
-
-    # Return the concatenation of the fixed-size parts (offsets interleaved) with the variable-size parts
-    return b"".join(fixed_parts + variable_parts)
-
-
-def serialize_value(value, typ=None):
-    if typ is None:
-        typ = infer_type(value)
-    # "uintN"
-    if isinstance(typ, str) and typ[:4] == 'uint':
-        length = int(typ[4:])
-        assert length in (8, 16, 32, 64, 128, 256)
-        return value.to_bytes(length // 8, 'little')
-    # "bool"
-    elif isinstance(typ, str) and typ == 'bool':
-        assert value in (True, False)
-        return b'\x01' if value is True else b'\x00'
-    # Vector
-    elif isinstance(typ, list) and len(typ) == 2:
-        # (regardless of element type, sanity-check if the length reported in the vector type matches the value length)
-        assert len(value) == typ[1]
-        return encode_series(value, [typ[0]] * len(value))
-    # List
-    elif isinstance(typ, list) and len(typ) == 1:
-        return encode_series(value, [typ[0]] * len(value))
-    # "bytes" (variable size)
-    elif isinstance(typ, str) and typ == 'bytes':
-        return coerce_to_bytes(value)
-    # "bytesN" (fixed size)
-    elif isinstance(typ, str) and len(typ) > 5 and typ[:5] == 'bytes':
-        assert len(value) == int(typ[5:]), (value, int(typ[5:]))
-        return coerce_to_bytes(value)
-    # containers
-    elif hasattr(typ, 'fields'):
-        values = [getattr(value, field) for field in typ.fields.keys()]
-        types = list(typ.fields.values())
-        return encode_series(values, types)
-    else:
-        print(value, typ)
-        raise Exception("Type not recognized")
-
-
-def get_zero_value(typ: Any) -> Any:
-    if isinstance(typ, str):
-        # Bytes array
-        if typ == 'bytes':
-            return b''
-        # bytesN
-        elif typ[:5] == 'bytes' and len(typ) > 5:
-            length = int(typ[5:])
-            return b'\x00' * length
-        # Basic types
-        elif typ == 'bool':
-            return False
-        elif typ[:4] == 'uint':
-            return 0
-        elif typ == 'byte':
-            return 0x00
-        else:
-            raise ValueError("Type not recognized")
-    # Vector:
-    elif isinstance(typ, list) and len(typ) == 2:
-        return [get_zero_value(typ[0]) for _ in range(typ[1])]
-    # List:
-    elif isinstance(typ, list) and len(typ) == 1:
-        return []
-    # Container:
-    elif hasattr(typ, 'fields'):
-        return typ(**{field: get_zero_value(subtype) for field, subtype in typ.fields.items()})
-    else:
-        print(typ)
-        raise Exception("Type not recognized")
-
-
-def chunkify(bytez):
-    bytez += b'\x00' * (-len(bytez) % BYTES_PER_CHUNK)
-    return [bytez[i:i + 32] for i in range(0, len(bytez), 32)]
-
-
-def pack(values, subtype):
-    return chunkify(b''.join([serialize_value(value, subtype) for value in values]))
-
-
-def is_power_of_two(x):
-    return x > 0 and x & (x - 1) == 0
-
-
-def merkleize(chunks):
-    tree = chunks[::]
-    while not is_power_of_two(len(tree)):
-        tree.append(ZERO_CHUNK)
-    tree = [ZERO_CHUNK] * len(tree) + tree
-    for i in range(len(tree) // 2 - 1, 0, -1):
-        tree[i] = hash(tree[i * 2] + tree[i * 2 + 1])
-    return tree[1]
-
-
-def mix_in_length(root, length):
-    return hash(root + length.to_bytes(32, 'little'))
-
-
-def infer_type(value):
-    """
-    Note: defaults to uint64 for integer type inference due to lack of information.
-    Other integer sizes are still supported, see spec.
-    :param value: The value to infer a SSZ type for.
-    :return: The SSZ type.
-    """
-    if hasattr(value.__class__, 'fields'):
-        return value.__class__
-    elif isinstance(value, Vector):
-        if len(value) > 0:
-            return [infer_type(value[0]), len(value)]
-        else:
-            # Element type does not matter too much,
-            # assumed to be a basic type for size-encoding purposes, vector is empty.
-            return ['uint64']
-    elif isinstance(value, list):
-        if len(value) > 0:
-            return [infer_type(value[0])]
-        else:
-            # Element type does not matter, list-content size will be encoded regardless, list is empty.
-            return ['uint64']
-    elif isinstance(value, (bytes, str)):
-        return 'bytes'
-    elif isinstance(value, int):
-        return 'uint64'
-    else:
-        raise Exception("Failed to infer type")
-
-
-def hash_tree_root(value, typ=None):
-    if typ is None:
-        typ = infer_type(value)
-    # -------------------------------------
-    # merkleize(pack(value))
-    # basic object: merkleize packed version (merkleization pads it to 32 bytes if it is not already)
-    if is_basic(typ):
-        return merkleize(pack([value], typ))
-    # or a vector of basic objects
-    elif isinstance(typ, list) and len(typ) == 2 and is_basic(typ[0]):
-        assert len(value) == typ[1]
-        return merkleize(pack(value, typ[0]))
-    # -------------------------------------
-    # mix_in_length(merkleize(pack(value)), len(value))
-    # if value is a list of basic objects
-    elif isinstance(typ, list) and len(typ) == 1 and is_basic(typ[0]):
-        return mix_in_length(merkleize(pack(value, typ[0])), len(value))
-    # (needs some extra work for non-fixed-sized bytes array)
-    elif typ == 'bytes':
-        return mix_in_length(merkleize(chunkify(coerce_to_bytes(value))), len(value))
-    # -------------------------------------
-    # merkleize([hash_tree_root(element) for element in value])
-    # if value is a vector of composite objects
-    elif isinstance(typ, list) and len(typ) == 2 and not is_basic(typ[0]):
-        return merkleize([hash_tree_root(element, typ[0]) for element in value])
-    # (needs some extra work for fixed-sized bytes array)
-    elif isinstance(typ, str) and typ[:5] == 'bytes' and len(typ) > 5:
-        assert len(value) == int(typ[5:])
-        return merkleize(chunkify(coerce_to_bytes(value)))
-    # or a container
-    elif hasattr(typ, 'fields'):
-        return merkleize([hash_tree_root(getattr(value, field), subtype) for field, subtype in typ.fields.items()])
-    # -------------------------------------
-    # mix_in_length(merkleize([hash_tree_root(element) for element in value]), len(value))
-    # if value is a list of composite objects
-    elif isinstance(typ, list) and len(typ) == 1 and not is_basic(typ[0]):
-        return mix_in_length(merkleize([hash_tree_root(element, typ[0]) for element in value]), len(value))
-    # -------------------------------------
-    else:
-        raise Exception("Type not recognized")
-
-
-def truncate(container):
-    field_keys = list(container.fields.keys())
-    truncated_fields = {
-        key: container.fields[key]
-        for key in field_keys[:-1]
-    }
-    truncated_class = SSZType(truncated_fields)
-    kwargs = {
-        field: getattr(container, field)
-        for field in field_keys[:-1]
-    }
-    return truncated_class(**kwargs)
-
-
-def signing_root(container):
-    return hash_tree_root(truncate(container))
-
-
-def serialize(ssz_object):
-    return getattr(ssz_object, 'serialize')()