Port kzg_4844 tests to pytest

[leolara]

It does what it says.

Comparing the generated files with diff -qr produces:

Only in consensus-spec-tests/new/general/deneb/kzg: blob_to_kzg_commitment

which make sense because that was ported already and it is only in the pytest version.

Note: I have left the non-pytest version commented out so you can check the generated files yourself. I will remove it once it is approved.

[jtraglia]

Hmm these templates feel very repetitive. Instead, could we have two templates for each test. A success template and a failure template. Then pass the test name prefix to the template as well?

Edit: better yet, a single template for each function which has a valid: bool parameter.

[jtraglia]

See my comment above.

[leolara]

Hmm these templates feel very repetitive. Instead, could we have two templates for each test. A success template and a failure template. Then pass the test name prefix to the template as well?

Here is a list of all the tests templates for ZKG 8488:

 test_compute_challenge.py:
  - _compute_challenge_case_valid

  test_compute_kzg_proof.py:
  - _compute_kzg_proof_case_valid
  - _compute_kzg_proof_case_invalid_blob
  - _compute_kzg_proof_case_invalid_z

  test_compute_blob_kzg_proof.py:
  - _compute_blob_kzg_proof_case_valid_blob
  - _compute_blob_kzg_proof_case_invalid_blob
  - _compute_blob_kzg_proof_case_invalid_commitment

  test_blob_to_kzg_commitment.py:
  - _blob_to_kzg_commitment_case_valid_blob
  - _blob_to_kzg_commitment_case_invalid_blob

  test_verify_kzg_proof.py:
  - _verify_kzg_proof_case_correct_proof
  - _verify_kzg_proof_case_incorrect_proof
  - _verify_kzg_proof_case_incorrect_proof_point_at_infinity
  - _verify_kzg_proof_case_correct_proof_point_at_infinity_for_zero
  _poly
  - _verify_kzg_proof_case_correct_proof_point_at_infinity_for_twos
  _poly
  - _verify_kzg_proof_case_invalid_commitment
  - _verify_kzg_proof_case_invalid_z
  - _verify_kzg_proof_case_invalid_y
  - _verify_kzg_proof_case_invalid_proof

  test_verify_blob_kzg_proof.py:
  - _verify_blob_kzg_proof_case_correct_proof
  - _verify_blob_kzg_proof_case_incorrect_proof
  - _verify_blob_kzg_proof_case_incorrect_proof_point_at_infinity
  - _verify_blob_kzg_proof_case_correct_proof_point_at_infinity_for
  _zero_poly
  - _verify_blob_kzg_proof_case_correct_proof_point_at_infinity_for
  _twos_poly
  - _verify_blob_kzg_proof_case_invalid_blob
  - _verify_blob_kzg_proof_case_invalid_commitment
  - _verify_blob_kzg_proof_case_invalid_proof

  test_verify_blob_kzg_proof_batch.py:
  - _verify_blob_kzg_proof_batch_case
  - _verify_blob_kzg_proof_batch_case_incorrect_proof_add_one
  - _verify_blob_kzg_proof_batch_case_incorrect_proof_point_at_infi
  nity
  - _verify_blob_kzg_proof_batch_case_invalid_blob
  - _verify_blob_kzg_proof_batch_case_invalid_commitment
  - _verify_blob_kzg_proof_batch_case_invalid_proof
  - _verify_blob_kzg_proof_batch_case_blob_length_different
  - _verify_blob_kzg_proof_batch_case_commitment_length_different
  - _verify_blob_kzg_proof_batch_case_proof_length_different

Which ones in your opinion are testing the same thing?

[jtraglia]

@leolara I'm saying that in each file, there can be a single template.

• _compute_blob_kzg_proof_case_valid_blob
• _compute_blob_kzg_proof_case_invalid_blob
• _compute_blob_kzg_proof_case_invalid_commitment

Consolidated into a single _compute_blob_kzg_proof_template function with the following params:

• test_name: str
• get_inputs: fn
• valid: bool

I'm just asking for something a bit more generic so we don't have so many template functions.

For example: I would like to see it done like this: test_compute_blob_kzg_proof.py

[jtraglia]

@leolara you've removed the templates but kept the duplicate code, which is the exact opposite of what I wanted.

[jtraglia]

This is stated at the bottom of each file:

consensus-specs/tests/formats/kzg_4844/verify_kzg_proof.md

Lines 31 to 34 in 048d382

	result should match the expected `output`. If the commitment or proof is invalid
	(e.g. not on the curve or not in the G1 subgroup of the BLS curve) or `z` or `y`
	are not a valid BLS field element, it should error, i.e. the output should be
	`null`.

I agree that this could be clearer though. Let's revert this & follow up with another PR that updates this for all KZG formats, instead of just this one.

Also, this can be simplified & we should null instead of None.

true (valid proof), false (incorrect proof), null (error)

[jtraglia]

@leolara thank you for reducing the repetitive code.

This PR looks good to me. I can confirm the outputs are the same.