Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sszgen: regression between master and latest #78

Closed
lightclient opened this issue May 24, 2022 · 2 comments
Closed

sszgen: regression between master and latest #78

lightclient opened this issue May 24, 2022 · 2 comments

Comments

@lightclient
Copy link

lightclient commented May 24, 2022

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
Copy link
Owner

ferranbt commented May 25, 2022

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

@ferranbt
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants