Merge pull request #3134 from ethereum/eip4844-lint

Add EIP4844 pylint and Mypy checks

Makefile

@@ -105,12 +105,12 @@ install_test:
 # Testing against `minimal` config by default
 test: pyspec
 	. venv/bin/activate; cd $(PY_SPEC_DIR); \
-	python3 -m pytest -n 4 --disable-bls --cov=eth2spec.phase0.minimal --cov=eth2spec.altair.minimal --cov=eth2spec.bellatrix.minimal --cov=eth2spec.capella.minimal --cov-report="html:$(COV_HTML_OUT)" --cov-branch eth2spec
+	python3 -m pytest -n 4 --disable-bls --cov=eth2spec.phase0.minimal --cov=eth2spec.altair.minimal --cov=eth2spec.bellatrix.minimal --cov=eth2spec.capella.minimal --cov=eth2spec.eip4844.minimal --cov-report="html:$(COV_HTML_OUT)" --cov-branch eth2spec
 
 # Testing against `minimal` config by default
 find_test: pyspec
 	. venv/bin/activate; cd $(PY_SPEC_DIR); \
-	python3 -m pytest -k=$(K) --disable-bls --cov=eth2spec.phase0.minimal --cov=eth2spec.altair.minimal --cov=eth2spec.bellatrix.minimal --cov=eth2spec.capella.minimal --cov-report="html:$(COV_HTML_OUT)" --cov-branch eth2spec
+	python3 -m pytest -k=$(K) --disable-bls --cov=eth2spec.phase0.minimal --cov=eth2spec.altair.minimal --cov=eth2spec.bellatrix.minimal --cov=eth2spec.capella.minimal --cov=eth2spec.eip4844.minimal --cov-report="html:$(COV_HTML_OUT)" --cov-branch eth2spec
 
 citest: pyspec
 	mkdir -p $(TEST_REPORT_DIR);
@@ -142,8 +142,8 @@ codespell:
 lint: pyspec
 	. venv/bin/activate; cd $(PY_SPEC_DIR); \
 	flake8  --config $(LINTER_CONFIG_FILE) ./eth2spec \
-	&& pylint --disable=all --enable unused-argument ./eth2spec/phase0 ./eth2spec/altair ./eth2spec/bellatrix ./eth2spec/capella \
-	&& mypy --config-file $(LINTER_CONFIG_FILE) -p eth2spec.phase0 -p eth2spec.altair -p eth2spec.bellatrix -p eth2spec.capella
+	&& pylint --rcfile $(LINTER_CONFIG_FILE) ./eth2spec/phase0 ./eth2spec/altair ./eth2spec/bellatrix ./eth2spec/capella ./eth2spec/eip4844 \
+	&& mypy --config-file $(LINTER_CONFIG_FILE) -p eth2spec.phase0 -p eth2spec.altair -p eth2spec.bellatrix -p eth2spec.capella -p eth2spec.eip4844
 
 lint_generators: pyspec
 	. venv/bin/activate; cd $(TEST_GENERATORS_DIR); \

linter.ini

@@ -11,3 +11,8 @@ warn_unused_configs = True
 warn_redundant_casts = True
 
 ignore_missing_imports = True
+
+# pylint
+[MESSAGES CONTROL]
+disable = all
+enable = unused-argument

setup.py

@@ -588,6 +588,7 @@ def notify_forkchoice_updated(self: ExecutionEngine,
         pass
 
     def get_payload(self: ExecutionEngine, payload_id: PayloadId) -> ExecutionPayload:
+        # pylint: disable=unused-argument
         raise NotImplementedError("no default block production")
 
 
@@ -643,12 +644,14 @@ def sundry_functions(cls) -> str:
 
 
 def no_op(fn):  # type: ignore
+    # pylint: disable=unused-argument
     def wrapper(*args, **kw):  # type: ignore
         return None
     return wrapper
 
 
 def get_empty_list_result(fn):  # type: ignore
+    # pylint: disable=unused-argument
     def wrapper(*args, **kw):  # type: ignore
         return []
     return wrapper
@@ -663,7 +666,8 @@ def wrapper(*args, **kw):  # type: ignore
 # End
 #
 
-def retrieve_blobs_sidecar(slot: Slot, beacon_block_root: Root) -> Optional[BlobsSidecar]:
+def retrieve_blobs_sidecar(slot: Slot, beacon_block_root: Root) -> PyUnion[BlobsSidecar, str]:
+    # pylint: disable=unused-argument
     return "TEST"'''
 
     @classmethod
@@ -682,8 +686,8 @@ def hardcoded_custom_type_dep_constants(cls, spec_object) -> str:
 }
 
 
-def is_spec_defined_type(value: str) -> bool:
-    return value.startswith(('ByteList', 'Union', 'Vector', 'List'))
+def is_byte_vector(value: str) -> bool:
+    return value.startswith(('ByteVector'))
 
 
 def objects_to_spec(preset_name: str,
@@ -696,17 +700,8 @@ def objects_to_spec(preset_name: str,
     new_type_definitions = (
         '\n\n'.join(
             [
-                f"class {key}({value}):\n    pass\n"
+                f"class {key}({value}):\n    pass\n" if not is_byte_vector(value) else f"class {key}({value}):  # type: ignore\n    pass\n"
                 for key, value in spec_object.custom_types.items()
-                if not is_spec_defined_type(value)
-            ]
-        )
-        + ('\n\n' if len([key for key, value in spec_object.custom_types.items() if is_spec_defined_type(value)]) > 0 else '')
-        + '\n\n'.join(
-            [
-                f"{key} = {value}\n"
-                for key, value in spec_object.custom_types.items()
-                if is_spec_defined_type(value)
             ]
         )
     )

specs/eip4844/beacon-chain.md

@@ -175,10 +175,13 @@ but MUST NOT be considered valid until a valid `BlobsSidecar` has been downloade
 def is_data_available(slot: Slot, beacon_block_root: Root, blob_kzg_commitments: Sequence[KZGCommitment]) -> bool:
     # `retrieve_blobs_sidecar` is implementation dependent, raises an exception if not available.
     sidecar = retrieve_blobs_sidecar(slot, beacon_block_root)
-    if sidecar == "TEST":
-        return True  # For testing; remove once we have a way to inject `BlobsSidecar` into tests
-    validate_blobs_sidecar(slot, beacon_block_root, blob_kzg_commitments, sidecar)
 
+    # For testing, `retrieve_blobs_sidecar` returns "TEST.
+    # TODO: Remove it once we have a way to inject `BlobsSidecar` into tests.
+    if isinstance(sidecar, str):
+        return True
+
+    validate_blobs_sidecar(slot, beacon_block_root, blob_kzg_commitments, sidecar)
     return True
 ```
 
@@ -216,7 +219,7 @@ def tx_peek_blob_versioned_hashes(opaque_tx: Transaction) -> Sequence[VersionedH
 ```python
 def verify_kzg_commitments_against_transactions(transactions: Sequence[Transaction],
                                                 kzg_commitments: Sequence[KZGCommitment]) -> bool:
-    all_versioned_hashes = []
+    all_versioned_hashes: List[VersionedHash] = []
     for tx in transactions:
         if tx[0] == BLOB_TX_TYPE:
             all_versioned_hashes += tx_peek_blob_versioned_hashes(tx)
@@ -283,7 +286,8 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe
 #### Blob KZG commitments
 
 ```python
-def process_blob_kzg_commitments(state: BeaconState, body: BeaconBlockBody):
+def process_blob_kzg_commitments(state: BeaconState, body: BeaconBlockBody) -> None:
+    # pylint: disable=unused-argument
     assert verify_kzg_commitments_against_transactions(body.execution_payload.transactions, body.blob_kzg_commitments)
 ```
 

specs/eip4844/polynomial-commitments.md

@@ -144,7 +144,7 @@ def bytes_to_bls_field(b: Bytes32) -> BLSFieldElement:
     """
     Convert 32-byte value to a BLS field scalar. The output is not uniform over the BLS field.
     """
-    return int.from_bytes(b, ENDIANNESS) % BLS_MODULUS
+    return BLSFieldElement(int.from_bytes(b, ENDIANNESS) % BLS_MODULUS)
 ```
 
 #### `blob_to_polynomial`
@@ -210,7 +210,7 @@ def bls_modular_inverse(x: BLSFieldElement) -> BLSFieldElement:
     Compute the modular inverse of x
     i.e. return y such that x * y % BLS_MODULUS == 1 and return 0 for x == 0
     """
-    return pow(x, -1, BLS_MODULUS) if x != 0 else 0
+    return BLSFieldElement(pow(x, -1, BLS_MODULUS)) if x != 0 else BLSFieldElement(0)
 ```
 
 #### `div`
@@ -220,7 +220,7 @@ def div(x: BLSFieldElement, y: BLSFieldElement) -> BLSFieldElement:
     """
     Divide two field elements: ``x`` by `y``.
     """
-    return (int(x) * int(bls_modular_inverse(y))) % BLS_MODULUS
+    return BLSFieldElement((int(x) * int(bls_modular_inverse(y))) % BLS_MODULUS)
 ```
 
 #### `g1_lincomb`
@@ -251,7 +251,7 @@ def poly_lincomb(polys: Sequence[Polynomial],
     for v, s in zip(polys, scalars):
         for i, x in enumerate(v):
             result[i] = (result[i] + int(s) * int(x)) % BLS_MODULUS
-    return [BLSFieldElement(x) for x in result]
+    return Polynomial([BLSFieldElement(x) for x in result])
 ```
 
 #### `compute_powers`
@@ -284,7 +284,7 @@ def evaluate_polynomial_in_evaluation_form(polynomial: Polynomial,
     """
     width = len(polynomial)
     assert width == FIELD_ELEMENTS_PER_BLOB
-    inverse_width = bls_modular_inverse(width)
+    inverse_width = bls_modular_inverse(BLSFieldElement(width))
 
     # Make sure we won't divide by zero during division
     assert z not in ROOTS_OF_UNITY
@@ -293,9 +293,11 @@ def evaluate_polynomial_in_evaluation_form(polynomial: Polynomial,
 
     result = 0
     for i in range(width):
-        result += div(int(polynomial[i]) * int(roots_of_unity_brp[i]), (int(z) - int(roots_of_unity_brp[i])))
-    result = result * (pow(z, width, BLS_MODULUS) - 1) * inverse_width % BLS_MODULUS
-    return result
+        a = BLSFieldElement(int(polynomial[i]) * int(roots_of_unity_brp[i]) % BLS_MODULUS)
+        b = BLSFieldElement((int(BLS_MODULUS) + int(z) - int(roots_of_unity_brp[i])) % BLS_MODULUS)
+        result += int(div(a, b) % BLS_MODULUS)
+    result = result * int(pow(z, width, BLS_MODULUS) - 1) * int(inverse_width)
+    return BLSFieldElement(result % BLS_MODULUS)
 ```
 
 ### KZG
@@ -355,17 +357,13 @@ def compute_kzg_proof(polynomial: Polynomial, z: BLSFieldElement) -> KZGProof:
     Compute KZG proof at point `z` with `polynomial` being in evaluation form
     Do this by computing the quotient polynomial in evaluation form: q(x) = (p(x) - p(z)) / (x - z)
     """
-
-    # To avoid SSZ overflow/underflow, convert element into int
-    polynomial = [int(i) for i in polynomial]
-    z = int(z)
-
     y = evaluate_polynomial_in_evaluation_form(polynomial, z)
-    polynomial_shifted = [(p - int(y)) % BLS_MODULUS for p in polynomial]
+    polynomial_shifted = [BLSFieldElement((int(p) - int(y)) % BLS_MODULUS) for p in polynomial]
 
     # Make sure we won't divide by zero during division
     assert z not in ROOTS_OF_UNITY
-    denominator_poly = [(int(x) - z) % BLS_MODULUS for x in bit_reversal_permutation(ROOTS_OF_UNITY)]
+    denominator_poly = [BLSFieldElement((int(x) - int(z)) % BLS_MODULUS)
+                        for x in bit_reversal_permutation(ROOTS_OF_UNITY)]
 
     # Calculate quotient polynomial by doing point-by-point division
     quotient_polynomial = [div(a, b) for a, b in zip(polynomial_shifted, denominator_poly)]
@@ -392,7 +390,7 @@ def compute_aggregated_poly_and_commitment(
     r_powers, evaluation_challenge = compute_challenges(polynomials, kzg_commitments)
 
     # Create aggregated polynomial in evaluation form
-    aggregated_poly = Polynomial(poly_lincomb(polynomials, r_powers))
+    aggregated_poly = poly_lincomb(polynomials, r_powers)
 
     # Compute commitment to aggregated polynomial
     aggregated_poly_commitment = KZGCommitment(g1_lincomb(kzg_commitments, r_powers))

specs/eip4844/validator.md

@@ -46,6 +46,7 @@ Implementers may also retrieve blobs individually per transaction.
 
 ```python
 def get_blobs_and_kzg_commitments(payload_id: PayloadId) -> Tuple[Sequence[BLSFieldElement], Sequence[KZGCommitment]]:
+    # pylint: disable=unused-argument
     ...
 ```