SSZ serialization comprehensiveness

[mratsim]

Keeping track of SSZ serialization test suite status.

Generic containers

Those are the container unaffected by mainnet/minimal presets

hashTreeRoot

• Valid SSZ objects
  • uints (already existed)
    • builtin types (uint8 ... uint64)
    • uint128, uint256
  • boolean (new)
  • basic vectors (new)
  • bitvectors (new)
  • bitlist (new)
    Serialization of standalone distinct types like BitList[maxLen] = distinct BitSeq is not supported
    Fails ssz_generic/bitlist/valid/.... The fix should be a branch for BitList at https://github.com/status-im/nim-beacon-chain/blob/a417edb5fff9d6610e6d149b6fd49f6b107caf44/beacon_chain/ssz/bytes_reader.nim#L61-L160
    Stack trace
    
  • complex containers (new)
    • SingleFieldTestStruct
    • SmallTestStruct
    • FixedTestStruct
    • VarTestStruct
      Some are deserialized incorrectly, the hashTreeRoot() is wrong
      Some are deserialized correctly
    • ComplexTestStruct
      semcheck issue due to Nim generics and bind-once/bind-many on line 146 https://github.com/status-im/nim-beacon-chain/blob/e2b3f0dadb7576c738c86d4f96e65c9599db7436/beacon_chain/ssz/bytes_reader.nim#L123-L147
      with added type trace:

      ComplexTestStruct
      FieldType: uint16
      SszType: uint16
      FieldType: List[system.uint16, 128]
      SszType: List[system.uint16, 1024]
      .../nim-beacon-chain/tests/official/test_fixture_ssz_generic_types.nim(228, 39) template/generic instantiation of `checkBasic` from here
      .../nim-beacon-chain/tests/official/test_fixture_ssz_generic_types.nim(79, 25) template/generic instantiation of `loadFile` from here
      .../nim-beacon-chain/vendor/nim-serialization/serialization.nim(101, 11) template/generic instantiation of `readValue` from here
      .../nim-beacon-chain/vendor/nim-serialization/serialization.nim(46, 9) template/generic instantiation of `readValue` from here
      .../nim-beacon-chain/beacon_chain/ssz.nim(285, 21) template/generic instantiation of `readSszValue` from here
      .../nim-beacon-chain/beacon_chain/ssz/bytes_reader.nim(124, 33) template/generic instantiation of `enumInstanceSerializedFields` from here
      .../nim-beacon-chain/beacon_chain/ssz/bytes_reader.nim(146, 29) Error: type mismatch: got <SszType> but expected 'List[system.uint16, 128]'
      

    • BitsStruct
      samecheck issue on line 146 as well

     .../nim-beacon-chain/beacon_chain/ssz/bytes_reader.nim(146, 29) Error: type mismatch: got <BitArray[1]> but expected 'BitArray[2]'
    

• Invalid SSZ objects are properly rejected
  • uints (already existed)
  • boolean (new)
    • No errors on overflow. Supercedes [SSZ] Decoding does not fail if the byte sequence overflows the destination type #280. Fails ssz_generic/boolean/invalid/byte_0x80.
  • basic vectors (new)
  • bitvectors (new)
  • bitlist (new)
  • complex containers (new)

Value

The value is stored in a YAML file and ideally we would need a yaml deserializer extension for nim-serialization.

The obvious ways to test the value would be:

• Converting the deserialized SSZ type to YAML and compare if output is equivalent.
  That would require playing with NimYAML formatting and may never work
• Parsing NimYAML to Nim
  We would need to implement some mappings between:
  • fixed size arrays (SSZ Vectors) and NimYAML seq
  • SSZ BitVectors/BitList to hex and comparing the YAML string
  • uint (including uint128/256) to string and compare to yaml string
  • heterogeneous container, compare field by field as string/hex string

Consensus objects

• Minimal preset
  • AggregateAndProof - Type not implemented
  • Attestation
  • AttestationData
  • AttesterSlashing
  • BeaconBlock
  • BeaconBlockBody
  • BeaconBlockHeader
  • BeaconState
  • Checkpoint
  • Deposit
  • DepositData
  • Eth1Data
  • Fork
  • HistoricalBatch
  • IndexedAttestation
  • PendingAttestation
  • ProposerSlashing
  • Validator
  • VoluntaryExit
• Mainnet preset
  Some types work on minimal but not on mainnet.
  • AggregateAndProof - Type not implemented
  • Attestation
  • AttestationData
  • AttesterSlashing
  • BeaconBlock
  • BeaconBlockBody
  • BeaconBlockHeader
  • BeaconState
  • Checkpoint
  • Deposit
  • DepositData
  • Eth1Data
  • Fork
  • HistoricalBatch
  • IndexedAttestation
  • PendingAttestation
  • ProposerSlashing
  • Validator
  • VoluntaryExit

Value

TODO: YAML deserializer

Fuzzing

TODO

[mratsim]

Update on closing criteria.

Currently we are at the "Minimum viable SSZ" state, i.e. serializing/deserializing of consensus objects.

To close this issue we need to pass atleast the full SSZ test suite, in particular the one for generic objects:

https://github.com/status-im/nim-beacon-chain/blob/9636ca3e17628d4bfbdc2a29006e5385fc398e66/tests/official/test_fixture_ssz_generic_types.nim#L190-L243

[tersec]

How much of this is resolved between:

• Fuzzing tests for SSZ #1041
• SSZ Reforms #1062
• SSZ cleanup #1073
• Ssz cleanup rebased #1082

?

[zah]

The Stint types are probably the only thing we are not testing yet

[tersec]

Stint support was removed a month ago: #1046

[zah]

I know, but it's part of the SSZ spec and there are test cases in the official test suite. I think Mamy's intention for this issue is to mainly track the tests that we are still skipping.

[arnetheduck]

stint is supported in the stand-alone library: https://github.com/status-im/nim-ssz-serialization/blob/master/ssz_serialization/types.nim#L30

tests cover it too:

nimbus-eth2/tests/consensus_spec/test_fixture_ssz_generic_types.nim

Line 106 in 710f267

let types = ["bool", "uint8", "uint16", "uint32", "uint64", "uint128", "uint256"]

Although we don't do value testing in general, we check the computed hash which is equivalent.