clarify config/preset separation, address review comments by @djrtwo

configs/README.md

@@ -1,20 +1,26 @@
-# Presets & Configurations
+# Configurations
 
-This directory contains a set of presets and configurations used for testing, testnets, and mainnet.
+This directory contains a set of configurations used for testing, testnets, and mainnet.
+A client binary may be compiled for a specific `PRESET_BASE`, 
+and then load different configurations around that preset to participate in different networks or tests.
 
-**Presets** are for deeper customization for different modes of operation, compile-time changes.
+Standard configs:
-**Configurations** are intended for different network configurations, fully runtime-configurable.
+- [`mainnet.yaml`](./mainnet.yaml): Mainnet configuration
-
+- [`minimal.yaml`](./minimal.yaml): Minimal configuration, used in spec-testing along with the [`minimal`](../presets/minimal) preset.
-Later-fork variables can be ignored, e.g. ignore Sharding variables as a client that only supports Phase 0 currently.
 
+Not all network configurations are in scope for the specification,
+see [`github.com/eth2-clients/eth2-networks`](https://github.com/eth2-clients/eth2-networks) for common networks,
+and additional testnet assets.
 
 ## Forking
 
 Variables are not replaced, but extended with forks. This is to support syncing from one state to the other over a fork boundary, without hot-swapping a config.
-Instead, for forks that introduce changes in a variable, the variable name is prefixed with a short abbreviation of the fork.
+Instead, for forks that introduce changes in a variable, the variable name is suffixed with the fork name, e.g. `INACTIVITY_PENALTY_QUOTIENT_ALTAIR`.
+
+Future-fork variables can be ignored, e.g. ignore Sharding variables as a client that only supports Phase 0 currently.
 
 Over time, the need to sync an older state may be deprecated.
-In this case, the prefix on the new variable may be removed, and the old variable will keep a special name before completely being removed.
+In this case, the suffix on the new variable may be removed, and the old variable will keep a special name before completely being removed.
 
 A previous iteration of forking made use of "timelines", but this collides with the definitions used in the spec (variables for special forking slots, etc.), and was not integrated sufficiently in any of the spec tools or implementations.
 Instead, the config essentially doubles as fork definition now, e.g. changing the value for `ALTAIR_FORK_EPOCH` changes the fork.
@@ -31,19 +37,3 @@ Each preset and configuration is a key-value mapping.
 
 This format is fully YAML compatible.
 The presets and configurations may contain comments to describe the values.
-
-## Presets
-
-Presets are more extensive than runtime configurations, and generally only applicable during compile-time.
-Each preset is defined as a directory, with YAML files per fork.
-Configurations can extend a preset by setting the `PRESET_BASE` variable.
-An implementation may choose to only support 1 preset per build-target and should validate this `PRESET_BASE` variable.
-
-See: [`mainnet_preset/`](./mainnet_preset) and [`minimal_preset/`](./minimal_preset).
-
-## Configuration
-
-Configurations are more minimal, singular YAML files, to define different network definitions.
-Besides different (test-)network definitions, implementations also apply these during runtime for spec-tests.
-
-See: [`mainnet_config.yaml`](./mainnet_config.yaml) and [`minimal_config.yaml`](./minimal_config.yaml).

configs/mainnet.yaml

@@ -18,8 +18,8 @@ GENESIS_DELAY: 604800
 # Forking
 # ---------------------------------------------------------------
 # Some forks are disabled for now:
-#  - They may be re-assigned another fork-version later
+#  - These may be re-assigned to another fork-version later
-#  - They set a temporarily max uint64 value: 2**64 - 1
+#  - Temporarily set to max uint64 value: 2**64 - 1
 
 # Altair
 ALTAIR_FORK_VERSION: 0x01000000
@@ -49,16 +49,14 @@ SHARD_COMMITTEE_PERIOD: 256
 ETH1_FOLLOW_DISTANCE: 2048
 
 
-# Inactivity penalties
+# Validator cycle
 # ---------------------------------------------------------------
 # 2**2 (= 4)
 INACTIVITY_SCORE_BIAS: 4
 # 2**4 (= 16)
 INACTIVITY_SCORE_RECOVERY_RATE: 16
-
+# 2**4 * 10**9 (= 16,000,000,000) Gwei
-
+EJECTION_BALANCE: 16000000000
-# Validator Churn
-# ---------------------------------------------------------------
 # 2**2 (= 4)
 MIN_PER_EPOCH_CHURN_LIMIT: 4
 # 2**16 (= 65,536)

configs/minimal.yaml

@@ -48,16 +48,14 @@ SHARD_COMMITTEE_PERIOD: 64
 ETH1_FOLLOW_DISTANCE: 16
 
 
-# Inactivity penalties
+# Validator cycle
 # ---------------------------------------------------------------
 # 2**2 (= 4)
 INACTIVITY_SCORE_BIAS: 4
 # 2**4 (= 16)
 INACTIVITY_SCORE_RECOVERY_RATE: 16
-
+# 2**4 * 10**9 (= 16,000,000,000) Gwei
-
+EJECTION_BALANCE: 16000000000
-# Validator Churn
-# ---------------------------------------------------------------
 # 2**2 (= 4)
 MIN_PER_EPOCH_CHURN_LIMIT: 4
 # 2**16 (= 65,536)

presets/README.md

@@ -0,0 +1,26 @@
+## Presets
+
+Presets are more extensive than runtime configurations, and generally only applicable during compile-time.
+Each preset is defined as a directory, with YAML files per fork.
+
+Configurations can extend a preset by setting the `PRESET_BASE` variable.
+An implementation may choose to only support 1 preset per build-target and should validate
+the `PRESET_BASE` variable in the config matches the running build.
+
+Standard presets:
+- [`mainnet/`](./mainnet): Used in mainnet, mainnet-like testnets (e.g. Prater), and spec-testing
+- [`minimal/`](./minimal): Used in low-resource local dev testnets, and spec-testing
+
+Client implementers may opt to support additional presets, e.g. for extra large beacon states for benchmarking.
+See [`/configs/`](../configs) for run-time configuration, e.g. to configure a new testnet.
+
+## Forking
+
+Like the [config forking](../configs/README.md#forking),
+the preset extends with every fork, instead of overwriting previous values.
+An implementation can ignore preset files as a whole for future forks,
+and can thus implement stricter compile-time warnings on unrecognized or missing variables in current forks.
+
+## Format
+
+The preset format matches the [config format](../configs/README.md#format).

presets/mainnet/phase0.yaml

@@ -30,8 +30,6 @@ SAFE_SLOTS_TO_UPDATE_JUSTIFIED: 8
 MIN_DEPOSIT_AMOUNT: 1000000000
 # 2**5 * 10**9 (= 32,000,000,000) Gwei
 MAX_EFFECTIVE_BALANCE: 32000000000
-# 2**4 * 10**9 (= 16,000,000,000) Gwei
-EJECTION_BALANCE: 16000000000
 # 2**0 * 10**9 (= 1,000,000,000) Gwei
 EFFECTIVE_BALANCE_INCREMENT: 1000000000
 

presets/minimal/phase0.yaml

@@ -30,8 +30,6 @@ SAFE_SLOTS_TO_UPDATE_JUSTIFIED: 2
 MIN_DEPOSIT_AMOUNT: 1000000000
 # 2**5 * 10**9 (= 32,000,000,000) Gwei
 MAX_EFFECTIVE_BALANCE: 32000000000
-# 2**4 * 10**9 (= 16,000,000,000) Gwei
-EJECTION_BALANCE: 16000000000
 # 2**0 * 10**9 (= 1,000,000,000) Gwei
 EFFECTIVE_BALANCE_INCREMENT: 1000000000
 

setup.py

@@ -141,7 +141,7 @@ def _get_eth2_spec_comment(child: LinkRefDef) -> Optional[str]:
 def _parse_value(name: str, typed_value: str) -> VariableDefinition:
     comment = None
     if name == "BLS12_381_Q":
-        comment = "  # noqa: E501"
+        comment = "noqa: E501"
 
     typed_value = typed_value.strip()
     if '(' not in typed_value:
@@ -842,8 +842,8 @@ class PySpecCommand(Command):
         self.md_doc_paths = ''
         self.out_dir = 'pyspec_output'
         self.build_targets = """
-                minimal:configs/minimal_preset:configs/minimal_config.yaml
+                minimal:presets/minimal:configs/minimal.yaml
-                mainnet:configs/mainnet_preset:configs/mainnet_config.yaml        
+                mainnet:presets/mainnet:configs/mainnet.yaml        
         """
 
     def finalize_options(self):
@@ -1003,11 +1003,13 @@ setup(
     url="https://github.com/ethereum/eth2.0-specs",
     include_package_data=False,
     package_data={'configs': ['*.yaml'],
+                  'presets': ['*.yaml'],
                   'specs': ['**/*.md'],
                   'eth2spec': ['VERSION.txt']},
     package_dir={
         "eth2spec": "tests/core/pyspec/eth2spec",
         "configs": "configs",
+        "presets": "presets",
         "specs": "specs",
     },
     packages=find_packages(where='tests/core/pyspec') + ['configs', 'specs'],

specs/phase0/beacon-chain.md

@@ -22,7 +22,7 @@
 - [Configuration](#configuration)
   - [Genesis settings](#genesis-settings)
   - [Time parameters](#time-parameters-1)
-  - [Validator Churn](#validator-churn)
+  - [Validator cycle](#validator-cycle)
 - [Containers](#containers)
   - [Misc dependencies](#misc-dependencies)
     - [`Fork`](#fork)
@@ -231,7 +231,6 @@ Additional preset configurations can be found in the [`configs`](../../configs)
 | - | - |
 | `MIN_DEPOSIT_AMOUNT` | `Gwei(2**0 * 10**9)` (= 1,000,000,000) |
 | `MAX_EFFECTIVE_BALANCE` | `Gwei(2**5 * 10**9)` (= 32,000,000,000) |
-| `EJECTION_BALANCE` | `Gwei(2**4 * 10**9)` (= 16,000,000,000) |
 | `EFFECTIVE_BALANCE_INCREMENT` | `Gwei(2**0 * 10**9)` (= 1,000,000,000) |
 
 ### Time parameters
@@ -305,10 +304,11 @@ Testnets and other types of chain instances may use a different configuration.
 | `SHARD_COMMITTEE_PERIOD` | `uint64(2**8)` (= 256) | epochs | ~27 hours |
 | `ETH1_FOLLOW_DISTANCE` | `uint64(2**11)` (= 2,048) | Eth1 blocks | ~8 hours |
 
-### Validator Churn
+### Validator cycle
 
 | Name | Value |
 | - | - |
+| `EJECTION_BALANCE` | `Gwei(2**4 * 10**9)` (= 16,000,000,000) |
 | `MIN_PER_EPOCH_CHURN_LIMIT` | `uint64(2**2)` (= 4) |
 | `CHURN_LIMIT_QUOTIENT` | `uint64(2**16)` (= 65,536) |
 

tests/core/pyspec/eth2spec/config/README.md

@@ -1,8 +1,11 @@
 # Eth2 config util
 
-For configuration, see [Configs documentation](../../../../../configs/README.md).
+For run-time configuration, see [Configs documentation](../../../../../configs/README.md).
 
-## Usage:
+For compile-time presets, see [Presets documentation](../../../../../presets/README.md)
+and the `build-targets` flag for the `pyspec` distutils command.
+
+## Config usage:
 
 ```python
 from eth2spec.config import config_util
@@ -17,4 +20,4 @@ spec.config = spec.Configuration(**config_util.load_config_file(Path('mytestnet.
 
 Note: previously the testnet config files included both preset and runtime-configuration data.
 The new config loader is compatible with this: all config vars are loaded from the file, 
-but those that have become presets will be ignored. 
+but those that have become presets can be ignored. 

tests/core/pyspec/eth2spec/config/config_util.py

@@ -56,8 +56,8 @@ loaded_defaults = False
 def load_defaults(spec_configs_path: Path) -> None:
     global mainnet_config_data, minimal_config_data
 
-    mainnet_config_data = load_config_file(spec_configs_path / 'mainnet_config.yaml')
+    mainnet_config_data = load_config_file(spec_configs_path / 'mainnet.yaml')
-    minimal_config_data = load_config_file(spec_configs_path / 'minimal_config.yaml')
+    minimal_config_data = load_config_file(spec_configs_path / 'minimal.yaml')
 
     global loaded_defaults
     loaded_defaults = True

tests/core/pyspec/eth2spec/utils/hash_function.py

@@ -6,8 +6,4 @@ ZERO_BYTES32 = b'\x00' * 32
 
 
 def hash(x: Union[bytes, bytearray, memoryview]) -> Bytes32:
-    try:
+    return Bytes32(sha256(x).digest())
-        return Bytes32(sha256(x).digest())
-    except TypeError:
-        print(x)
-        raise Exception("bad")