Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test suite names: BLS and KZG 4844 are confusingly called 'small' #3478

Closed
mratsim opened this issue Aug 13, 2023 · 1 comment
Closed

Test suite names: BLS and KZG 4844 are confusingly called 'small' #3478

mratsim opened this issue Aug 13, 2023 · 1 comment

Comments

@mratsim
Copy link
Contributor

mratsim commented Aug 13, 2023

Ages ago (in 2019 #1320), a huge refactoring introduced suite names.

At the time the BLS test suites were tagged with suite_name='small', unfortunately I do not remember the origin of the name (cc @protolambda and @asanso) as the full suite did not exist at the time (https://github.com/ethereum/bls12-381-tests).

IIRC it was a minimal set of tests that implementations should pass for interop, was agnostic to minimal/mainnet and included SSZ tests as well but I may very well completely misremember.

def cases_fn() -> Iterable[gen_typing.TestCase]:
for data in test_case_fn():
(case_name, case_content) = data
yield gen_typing.TestCase(
fork_name=fork_name,
preset_name='general',
runner_name='bls',
handler_name=handler_name,
suite_name='small',
case_name=case_name,
case_fn=lambda: [('data', 'data', case_content)]
)

The KZG-4844 suite reuses the name small which AFAIK doesn't mean anything anymore in today's codebase.

def cases_fn() -> Iterable[gen_typing.TestCase]:
for data in test_case_fn():
(case_name, case_content) = data
yield gen_typing.TestCase(
fork_name=fork_name,
preset_name='general',
runner_name='kzg',
handler_name=handler_name,
suite_name='small',
case_name=case_name,
case_fn=lambda: [('data', 'data', case_content)]
)

This is prone to confusion with the KZG minimal trusted setup as well.

I propose to change:

  • bls 'small' to just 'bls' or 'crypto-bls'
  • KZG 'small" to 'kzg-mainnet' or 'kzg-minimal' (depending on the trusted setup.
@hwwhww
Copy link
Contributor

hwwhww commented Nov 16, 2023

fixed via #3484

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants