The spec-test coverage hunt

[protolambda]

The spec tests currently get clients to about ~72% coverage of their (optimized) spec code. I'm taking ZRNT (the Go-spec) as reference here, which is tested based on the same yaml spec tests.

Note that the spec itself has much more coverage (over 90%), since we have finalization tests which are not part of the spec-test-vectors (maybe later, or alternative more direct tests).

For full spec-test coverage, we need to cover the following remaining cases:

exceed max-operations:

• attestations
• attester slashings
• deposits
• proposer slashings
• transfers
• voluntary exits

Need for exceeding-max-operations tests is abstracted away with #1180, where the limit is naturally imposed by the SSZ data type. Yay. Just need SSZ spec-tests for limit-enforcement later.

attestations:

• BAD: out of range shard in crosslink
• BAD: target old epoch
• BAD: target epoch in future
• BAD: parent crosslink End-epoch + MAX_EPOCHS_PER_CROSSLINK does not equal crosslink end-epoch when parent epoch is lower than target

attester slashing

• BAD: test invalid attestation, e.g. unsorted (nr 1)
• BAD: test invalid attestation, e.g. unsorted (nr 2)

deposits

• BAD: not expected count of deposits (eth 1)
• OK: for a normal non-maxed out deposit, it may get an effective balance lower than the normal balance, due to EFFECTIVE_BALANCE_INCREMENT.
• OK: deposit large amount, over effective balance, get limited at just effective balance.

eth1 data

• OK: majority vote, update latest eth1 data
• OK: no majority vote (but close), do not update latest eth1 data

transfers

• BAD: duplicate transfer TODO; need config change for this.
• BAD: sender does not exist
• BAD: recipient does not exist
• BAD: invalid transfer pubkey (old, line was missed in go spec because of other old transfer reason)
• BAD: results in dust on sender address
• Super BAD: duplicate balance by sending all in both fee and amount
  • x 9: cover different combinations

deltas

• matching penalties/rewards
  • matching head: penalty, reward, none, both
  • matching target: penalty, reward, none, both
  • matching source: penalty, reward, none, both
• proposer inclusion delay
• inactivity penalty
• crosslink committee:
  • penalty if not in committee, reward if in committee
  • no crosslink, empty participation, everyone penalty
    Postponed to post-freeze. Due to the nesting of functions rewards computation, and multiple functions affecting the same state (balances), it's too time consuming to test right before freeze. But also not breaking or strictly limited by deadline, so we prioritize test-vector polishing and features (recent genesis rework, among others), for the best spec-freeze :)

epoch final updates:

• OK: validator does well, and hits the MAX_EFFECTIVE_BALANCE ceiling through hysteresis
  • and a lot more hysteresis / rounding edge cases
• OK: eth1 votes reset, at end of voting period
• OK: eth1 votes no reset, when not at end of voting period

justification/finalization

• OK(x 4) participation, actual attestations. (credits to @djrtwo for bugfixes on initial testing setup, and @CarlBeek for fixing the remaining tests). Edit: fixed it to also work on mainnet config.
  • finalize on 234, yes/no case
  • finalize on 23, yes/no case
  • finalize on 123, yes/no case
  • finalize on 12, yes/no case
• sum(balances) is enough to justify but sum(effective_balances) is not. Ensure no justification. (picked up by @CarlBeek) Edit: postponed, difficult setup.

registry updates:

• validator is activated (ActivationEligibilityEpoch == FAR_FUTURE_EPOCH && v.EffectiveBalance >= MAX_EFFECTIVE_BALANCE)
• activation queue sorting
• hit activation churn limit
• check delayed activation of epochs (get_delayed_activation_exit_epoch) Edit; test_activation already covers it.

process slashings:

• relative to total slashed balance, and with non-uniform balances start
• maximum slashing
• minimum slashing

genesis:

• test genesis state creation (picked up by @hwwhww)

indexed attestations:

• BAD: too many (bit0, bit1, or sum)
• BAD: not sorted (bit1 is for later phase 1 testing)
• BAD: intersecting bit0 and bit1 indices
• BAD: invalid indices (bit0, bit1)

fork:

• state has fork set, and fork slot has past. sig with new fork Irregular state transition. To be tested with future fork work.

block processing:

• BAD: block with old slot on state

• BAD: block with invalid state root

• resolve test_empty_epoch_transition_not_finalizing

• close PR skip transfer tests and enable the commented tests #1181, with updates from this PR + current state of ssz-list-limit checking making the 0 max hard to deal with, better for future point when transfers get enabled in a new configuration.

[hwwhww]

I can take the genesis task.

[protolambda]

@hwwhww Great! Steps to implement:

• use the bare spec-test decorator, the one that doesn't provide a pre-state (but we do want the phase0/phase1 spec to be passed). See context + util in test root folder.
• yield a list of deposits
• yield genesis time
• compute deposits root
• yield eth1 data, with root in it
• construct genesis state using spec
• yield genesis state

And then I'll write a small generator for it (really just a main file + a few calls + dependency to base-generator and eth2spec). And we can output it as test-vectors for clients, in addition to testing the spec :)

[protolambda]

I will start now, and go from top to bottom. If anyone wants to contribute, you are welcome, but let's avoid double work: go over the list in reverse. Or just share what you are working on, I'll monitor this issue.

[hwwhww]

@protolambda Thanks for the guide! I opened a draft PR: #1202
Two questions:

1. "we do want the phase0/phase1 spec to be passed" - I think the genesis test case only needs to be tested on phase 0?
2. Somehow I can't just remove @spec_state_test and the given pre-state. Do you mean there's another "bare spec-test decorator" or I need to write one?

[protolambda]

@hwwhww

1. Unsure, we may want to start test-chains from later forks. Just phase-0 is ok for now
2. See context.py:

# shorthand for decorating @with_state @spectest()
def spec_state_test(fn):
    return with_state(bls_switch(spectest()(fn)))

We just need bls_switch and spectest() here to be applied, no pre-state from with_state.

[JustinDrake]

Addressed in #1206 🎉, genesis tests on their way in a different PR :)