ferranbt / fastssz

Fast Ethereum2.0 SSZ encoder/decoder
MIT License
74 stars 44 forks source link

BeaconState does not pass spectests #61

Closed kasey closed 3 years ago

kasey commented 3 years ago

To reproduce, add BeaconState to the list of "codecs" and remove the special case logic skipping it. Then run spectests to observe that the generated code does not currently produce the correct result.

Prysm is side-stepping this issue for now because we use a hand-written HTR function for BeaconState, but this bug poses a risk in the event that some code accidentally calls the fastssz-generated HTR code. We're working on an additional work-around to prevent that from happening, but we're hoping that you can identify the issue so that the generated code is correct. Thank you!

$ git diff
diff --git a/spectests/structs_test.go b/spectests/structs_test.go
index b9fcddd..ed06cbb 100644
--- a/spectests/structs_test.go
+++ b/spectests/structs_test.go
@@ -38,6 +38,7 @@ var codecs = map[string]testCallback{
        "AggregateAndProof": func(config string) codec { return new(AggregateAndProof) },
        "Attestation":       func(config string) codec { return new(Attestation) },
        "AttesterSlashing":  func(config string) codec { return new(AttesterSlashing) },
+       "BeaconState": func(config string) codec { return new(BeaconState) },
        "BeaconBlock": func(config string) codec {
                if config == "minimal" {
                        return new(BeaconBlockMinimal)
@@ -306,7 +307,7 @@ func TestSpecMainnet(t *testing.T) {
                spl := strings.Split(f, "/")
                name := spl[len(spl)-1]

-               if name == "BeaconState" || name == "HistoricalBatch" {
+               if name == "HistoricalBatch" {
                        continue
                }
                base, ok := codecs[name]

$ go test -v ./spectests/...
=== RUN   TestFuzzMarshalWithWrongSizes
    structs_test.go:121: Fuzz testing not enabled, skipping
--- SKIP: TestFuzzMarshalWithWrongSizes (0.00s)
=== RUN   TestErrorResponse
--- PASS: TestErrorResponse (0.02s)
=== RUN   TestFuzzEncoding
    structs_test.go:121: Fuzz testing not enabled, skipping
--- SKIP: TestFuzzEncoding (0.00s)
=== RUN   TestFuzzUnmarshalAppend
    structs_test.go:121: Fuzz testing not enabled, skipping
--- SKIP: TestFuzzUnmarshalAppend (0.00s)
=== RUN   TestFuzzUnmarshalShuffle
    structs_test.go:121: Fuzz testing not enabled, skipping
--- SKIP: TestFuzzUnmarshalShuffle (0.00s)
=== RUN   TestSpecMinimal
    structs_test.go:297: Process AggregateAndProof ../eth2.0-spec-tests/tests/minimal/altair/ssz_static/AggregateAndProof
    structs_test.go:297: Process Attestation ../eth2.0-spec-tests/tests/minimal/altair/ssz_static/Attestation
    structs_test.go:297: Process AttestationData ../eth2.0-spec-tests/tests/minimal/altair/ssz_static/AttestationData
    structs_test.go:297: Process AttesterSlashing ../eth2.0-spec-tests/tests/minimal/altair/ssz_static/AttesterSlashing
    structs_test.go:297: Process BeaconBlock ../eth2.0-spec-tests/tests/minimal/altair/ssz_static/BeaconBlock
    structs_test.go:297: Process BeaconBlockBody ../eth2.0-spec-tests/tests/minimal/altair/ssz_static/BeaconBlockBody
    structs_test.go:297: Process BeaconBlockHeader ../eth2.0-spec-tests/tests/minimal/altair/ssz_static/BeaconBlockHeader
    structs_test.go:297: Process BeaconState ../eth2.0-spec-tests/tests/minimal/altair/ssz_static/BeaconState
    structs_test.go:339: vector does not have the correct length
--- FAIL: TestSpecMinimal (2.27s)
=== RUN   TestSpecMainnet
    structs_test.go:319: Process AggregateAndProof ../eth2.0-spec-tests/tests/mainnet/altair/ssz_static/AggregateAndProof
    structs_test.go:319: Process Attestation ../eth2.0-spec-tests/tests/mainnet/altair/ssz_static/Attestation
    structs_test.go:319: Process AttestationData ../eth2.0-spec-tests/tests/mainnet/altair/ssz_static/AttestationData
    structs_test.go:319: Process AttesterSlashing ../eth2.0-spec-tests/tests/mainnet/altair/ssz_static/AttesterSlashing
    structs_test.go:319: Process BeaconBlock ../eth2.0-spec-tests/tests/mainnet/altair/ssz_static/BeaconBlock
    structs_test.go:319: Process BeaconBlockBody ../eth2.0-spec-tests/tests/mainnet/altair/ssz_static/BeaconBlockBody
    structs_test.go:319: Process BeaconBlockHeader ../eth2.0-spec-tests/tests/mainnet/altair/ssz_static/BeaconBlockHeader
    structs_test.go:319: Process BeaconState ../eth2.0-spec-tests/tests/mainnet/altair/ssz_static/BeaconState
    structs_test.go:509: 1 error(s) decoding:

        * 'block_roots': expected source data to have length less or equal to 64, got 8192
--- FAIL: TestSpecMainnet (1.97s)
FAIL
FAIL github.com/ferranbt/fastssz/spectests  4.269s
?   github.com/ferranbt/fastssz/spectests/external  [no test files]
?   github.com/ferranbt/fastssz/spectests/external2 [no test files]
FAIL
ferranbt commented 3 years ago

I did not implement BeaconState because I was told you guys were using custom code for that instead of fastssz.