Decided I wanted to get this out to explain the current state of testing, and collect feedback (implementers please comment) on what you need from testing, and your feelings about BLS usage in tests.
The two pain-points to get a pretty (and large) set of test-vectors out for clients are:
And side-issue, but easily resolved:
efficient creation of a genesis state:
When BLS functionality is implemented in test-code (creation of signed deposits, and verification).
Solution would be to either cache it, or create it directly, without going through the spec functions (current temporary solution on experiment branch).
Talking about the status on spectest-deco PR 1052 here, based on the v06x branch, where we are developing 0.6 improvements. (to be merged back into dev later)
generator_mode=true to each of them, making them output a test-vector.tests/ to eth2spec/test, i.e. part of packagepytest@spec_test or similar (see PR 1052)v06x branch)operations test-gen uses test-package ability to output test-vectors for each test-casesanity tests updated and can be cleanly used for test-generation, but requires more work to define the format of the test-vectors, as they is more variety.epoch processing tests also updated, also can be used, not as complete as block-processing, lower priority.assert verify_..., just verify_..., and make it raise a special BLSVerificationError (or something like that)A work-in-progress introduction of actual full BLS usage in the pytests is started here: tests-with-sigs branch
Suggestions welcome.
I don't mind waiting a couple of weeks for state tests that don't require test-specific options to ignore failed signature check.
For sig-verification errors, we can either use a field (type?), the file name or comments to signal what should be tested, a field is probably better because you can do something like this
def my_test_case(test_cases) =
...
try:
runStateTransition(test_cases['test_empty_block_transition']
except BLSVerificationError:
if test_cases['test_empty_block_transition'].failure_type == "BLS_signature":
pass
else:
raise # reraise if a BLS error is not expected
I agree with the third option.
test-with-sigs branch, I'm inclined to have a stub for writing and verifying signatures to still be present on most cases when generator_mode=False to keep CI moving along.validate_… if the function is going to throw the error itself.sanity tests updated and can be cleanly used for test-generation, but requires more work to define the format of the test-vectors, as they is more variety.
What work is required here?
I'd bet for 3rd option though I liked first one at the beginning as it sounds like "we don't test this thing again and again", but in fact real BLS verifies not only BLS itself but logic of methods preparing inputs too, plus it's not a big overhead when running whole test in time.
@djrtwo
Few doubts here:
generator_mode to do more than it says, let's keep BLS on/off separate.verify. It's core to BLS spec, and I want it to show in the spec. Maybe we can convert the returned bool to an error, with some helper? E.g. enforce(verify_sig(...))error_type, and then assert, bls, and more in future possibly (or more precise assertion reasons). And then clients can map their own errors/reporting to these error-types, to verify it's exactly correct, and not just a happy coincident error.ii.
Plan, after discussion with @djrtwo IRL:
bls-test-deco branch)bls_active=False/True to the test, similar to generator_modebls_active=True to all tests.bls_active=False@always_bls, to force BLS to be active, regardless of what is passed as argument. For the bls-dependent tests.bls_active=True in some occasions/triggers (TBD), just to check signing works as intended.What we get:
provide test vectors with BLS ON during generation, i.e. proper signing mark tests that are dependent on BLS being ON.
I agree with this, I'd like to see all tests with valid BLS sigs/keys. Clients can choose which tests run with "fake crypto".
We're presently failing the SSZ tests because they're using fundamentally invalid signatures. Our alternative is to switch the tests to our fake crypto library, but then we're no longer testing production SSZ implementations.
We had the same problem with the v0.5.1 state tests and it forced us to run those tests with fake crypto. Our experience was that either made our build process more complex/error-prone or weakened our production code.
Most helpful comment
Plan, after discussion with @djrtwo IRL:
bls-test-decobranch)bls_active=False/Trueto the test, similar togenerator_modebls_active=Trueto all tests.bls_active=False@always_bls, to force BLS to be active, regardless of what is passed as argument. For the bls-dependent tests.bls_active=Truein some occasions/triggers (TBD), just to check signing works as intended.What we get: