ferranbt / fastssz

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

sszgen: max size shadowed by first occurence of custom type def #75

Closed lightclient closed 2 years ago

lightclient commented 2 years ago
Bytes []byte

type A struct {
        Foo Bytes `ssz-max:"2048"`
}

type B struct {
        Bar Bytes `ssz-max:"32"`
}

If you run sszgen on these structs, both A and B will use 2048 as the max size for the byte list. In the case of B, this causes an incorrect hash-tree-root to be calculated, because the mix in should be based on 32, not `2048.

lightclient commented 2 years ago

It's also making a MarshalSSZ method with an offset that is never used:

func (b *Bytes) MarshalSSZTo(buf []byte) (dst []byte, err error) {
    dst = buf
    offset := int(4)

    return
}
ferranbt commented 2 years ago

Which commit of sszgen are you using? I am trying with the latest sszgen but I get a different error.

lightclient commented 2 years ago

@ferranbt I'm using the sszgen given by go install github.com/ferranbt/fastssz/sszgen@latest

edit: compiled from 780dd73 and was able to replicate the issue? What error are you seeing that differs?

ferranbt commented 2 years ago

Fixed in #76. There are two things here:

lightclient commented 2 years ago

Great - thank you @ferranbt. Will --exclude-objs also resolve the following issue:

import "github.com/ethereum/go-ethereum/common/hexutil"

type A struct {
        Foo hexutil.Bytes `ssz-max:"2048"`
}

type B struct {
        Bar hexutil.Bytes `ssz-max:"32"`
}

and sszgen --include ../go-ethereum/common/hexutil --path . --objs A,B causes the resulting encoding file to have the error

./main_encoding.go:6:2: imported and not used: "github.com/ethereum/go-ethereum/common/hexutil"
ferranbt commented 2 years ago

Ok, now this is a different thing hahah. I was wondering if that Bytes object would come from a different place. In this case, you do not need to use --exclude-objs since it is not part of the main source. Do you mind opening another issue regarding this?

lightclient commented 2 years ago

Yes I will