ssvlabs / ssv-spec

GNU General Public License v3.0
25 stars 22 forks source link

Tests Wishlist for committee based consensus #383

Closed GalRogozinski closed 2 months ago

GalRogozinski commented 4 months ago

Runner

Consensus

Duties - New duty

Duties - Attestation Sync Committee

Post-consensus

Value check for beacon vote (committee runner's consensus data)

GalRogozinski commented 4 months ago

For duty tests we skip broadcasted consensus messages: https://github.com/ssvlabs/ssv-spec/blob/548cb4fb149852316f8d46cbf7e62a47a94fafae/ssv/spectest/tests/runner/duties/newduty/test.go#L55-L58

we shouldn't do that... we should test consensus messages as well imo... Maybe it was dont this way to seperate runner from consensus, but we run consensus instance in the test

GalRogozinski commented 4 months ago

Add ValueCheck tests

GalRogozinski commented 3 months ago

We should test the following scenarios:

  1. With invalid_quorum_then_valid_quorum we should have byzantine test where some roots have a valid root while others don't. We need to see that a 4th message fixes the invalid ones
  2. Test with validators on different committees. We need to make sure the committee indexes are not getting mixed up after post consensus stage.
  3. We need to make sure we use proper eth domains.

Currently scenario 3 is implicitly tested because the info is in the broadcasted root (so it is good enough). For 2 we are missing the input.

GalRogozinski commented 2 months ago

Done