ferranbt / fastssz

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

sszgen v0.0.1 gererates invalid code #91

Closed screwyprof closed 2 years ago

screwyprof commented 2 years ago

What version of Go are you using (go version)?

go version go1.17.10 darwin/amd64

Does this issue reproduce with the latest release?

Yes. sszgen version 0.1.1

What did you expect to see?

Correctly generated code.

What did you see instead?

Broken code.

For example given the following code

// ForkData provides data about a fork.
type ForkData struct {
    // Current version is the current fork version.
    CurrentVersion Version `ssz-size:"4"`
    // GenesisValidatorsRoot is the hash tree root of the validators at genesis.
    GenesisValidatorsRoot Root `ssz-size:"32"`
}

And running sszgen --path . --objs ForkData will produce a file which will contain an invalid function:

// MarshalSSZTo ssz marshals the ForkData object to a target array
func (f *ForkData) MarshalSSZTo(buf []byte) (dst []byte, err error) {
    dst = buf
    offset := int(4)

    return
}

The offset variable above is never used and the compiler will produce an error.

ferranbt commented 2 years ago

What is the type of Version and Root?

screwyprof commented 2 years ago
// Version is a fork version.
type Version [4]byte

// Root is a merkle root.
type Root [32]byte
ferranbt commented 2 years ago

It does work to me on the current branch here which should be the same release as 0.1.1.

How are you running the generator?

screwyprof commented 2 years ago

We use these structures in our projects. Basically just copied the folder. The newer version of fastssz introduced getTree method which is not implemented in those structures, so I'm trying to re-generate them. The generator is run via go:generate.

// see https://github.com/attestantio/go-eth2-client/blob/master/spec/phase0/generate.go
//go:generate sszgen --path . --objs AggregateAndProof,AttestationData,Attestation,AttesterSlashing,BeaconBlockBody,BeaconBlock,BeaconBlockHeader,BeaconState,Checkpoint,Deposit,DepositData,DepositMessage,ETH1Data,Fork,ForkData,IndexedAttestation,PendingAttestation,ProposerSlashing,SignedAggregateAndProof,SignedBeaconBlock,SignedBeaconBlockHeader,SignedVoluntaryExit,SigningData,Validator,VoluntaryExit
ferranbt commented 2 years ago

Ok, I will take a look at that go:generate. In the meantime, this repo also has the full set of spec tests for eth2 here in case you want to use those.

screwyprof commented 2 years ago

Thank you! I'll wait a bit if it can be solved by using the generator, if not, then I'll try the the structs in this repo, thanks!

ferranbt commented 2 years ago

Hi, I am not sure why 0.1.0 does not work. Note that go-eth2-client encodings were generated with an old version of sszgen (+14 months old) back when there were not even versions.

I tried with the latest version of sszgen 0.1.1 and it does work correctly. Here is an example with phase0 and the new generator.

screwyprof commented 2 years ago

Hi, could you please tell me how exactly were you able to run the generator? If I try to run sszgen it produces incorrect output with the exactly same error as I described. I'm using the latest version.

Screenshot 2022-07-11 at 16 43 57

screwyprof commented 2 years ago

I've figured out how to fix the problem. I removed the generated files before running sszgen and now it works. However I have another problem:

❯ ~/<project_path>pkg/eth2/spec/altair
❯ sszgen --include ~/<project_path>/pkg/eth2/spec/phase0 --path . --objs BeaconBlock
panic: create not implemented for type reference

goroutine 1 [running]:
github.com/ferranbt/fastssz/sszgen/generator.(*Value).createSlice(0xc0007adf80, 0x1)
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/generator/unmarshal.go:355 +0x4f9
github.com/ferranbt/fastssz/sszgen/generator.(*Value).unmarshalList(0xc0007adf80)
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/generator/unmarshal.go:156 +0x117
github.com/ferranbt/fastssz/sszgen/generator.(*Value).unmarshal(0xc0007adf80, {0x118410c, 0x3})
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/generator/unmarshal.go:102 +0xcc5
github.com/ferranbt/fastssz/sszgen/generator.(*Value).umarshalContainer(0xc0007adc00, 0xc0, {0x118410c, 0x4})
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/generator/unmarshal.go:312 +0x11b0
github.com/ferranbt/fastssz/sszgen/generator.(*env).unmarshal(0xc0007adc00, {0xc00001d6b0, 0xf}, 0xc000279800)
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/generator/unmarshal.go:20 +0xae
github.com/ferranbt/fastssz/sszgen/generator.(*env).print(0xc0000a1da8, {0xc000801480, 0x3, 0x1185996})
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/generator/generator.go:383 +0x5ed
github.com/ferranbt/fastssz/sszgen/generator.(*env).generateEncodings(0xc0000a1da8)
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/generator/generator.go:297 +0x1ba
github.com/ferranbt/fastssz/sszgen/generator.Encode({0x7ff7bfefeba7, 0x1}, {0xc000052350, 0x1, 0x1}, {0x0, 0x0}, {0xc000052360, 0x1, 0x1}, ...)
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/generator/generator.go:71 +0x365
main.generate()
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/main.go:51 +0x391
main.main()
    /Users/screwyprof/go/pkg/mod/github.com/ferranbt/fastssz@v0.1.1/sszgen/main.go:25 +0xb8
ferranbt commented 2 years ago

Hi, in my example there is a go:generate script that generates the sszgen output. I would try that first.

AFAIK, go-eth2-client has updated the specs with the latest sszgen version, so you could use that directly.

Besides, I also maintain go-eth-consensus with the consensus data structures and the API calls if you want to take a look. The sszgen script that generates the encoding is quite simple there (here).

screwyprof commented 2 years ago

Hi @ferranbt! Thanks for getting back. My issues is resolved. Thanks for the link to go-eth-consensus I'll see if I can replace go-eth2-client. Cheers!