ferranbt / fastssz

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

sszgen: regression between master and latest #78

Closed lightclient closed 2 years ago

lightclient commented 2 years ago

Hi, it seems like there is a regression against master. I am able to run sszgen@bc5fefe without error, but sszgen@fadd032 throws an error:

$ go install github.com/ferranbt/fastssz/sszgen@fadd032
$ rm -f types/builder_encoding.go types/signing_encoding.go
$ sszgen --path types --include ../go-ethereum/common/hexutil --exclude-objs ExtraData --objs Eth1Data,BeaconBlockHeader,SignedBeaconBlockHeader,ProposerSlashing,Checkpoint,AttestationData,IndexedAttestation,AttesterSlashing,Attestation,Deposit,VoluntaryExit,SyncAggregate,ExecutionPayloadHeader,VersionedExecutionPayloadHeader,BlindedBeaconBlockBody,BlindedBeaconBlock,RegisterValidatorRequestMessage,BuilderBid,SignedBuilderBid,SigningData,forkData,transactions
[ERR]: failed to encode SignedBuilderBid: type Signature not found
$ go install github.com/ferranbt/fastssz/sszgen@bc5fefe
$ sszgen --path types --include ../go-ethereum/common/hexutil --exclude-objs ExtraData --objs Eth1Data,BeaconBlockHeader,SignedBeaconBlockHeader,ProposerSlashing,Checkpoint,AttestationData,IndexedAttestation,AttesterSlashing,Attestation,Deposit,VoluntaryExit,SyncAggregate,ExecutionPayloadHeader,VersionedExecutionPayloadHeader,BlindedBeaconBlockBody,BlindedBeaconBlock,RegisterValidatorRequestMessage,BuilderBid,SignedBuilderBid,SigningData,forkData,transactions

This is running against mergemock@c7e64af.

ferranbt commented 2 years ago

Hi, most likely this commit that changed the cmd interface. I will take a look.

ferranbt commented 2 years ago

I found the issue, it was introduced as part of the fix in #75.

The generator expects all the array types (including []byte) to have tags on the size even when the size is fixed (introduced in #65). I have to address that because it is confusing to have to set the size twice (one in the [n]byte and another in the tag).

It worked in bc5fefe because you have some Signature definitions with the ssz-size:"92" already and it was able to reuse them, which it should not do.

In the meantime, until the fix is done, you can set the ssz-size tag for all the Signature objects. That fixed the issue on my side.