ferranbt / fastssz

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

Empty uint64 array will unmarshal to nil if struct is not initialized #96

Closed jtraglia closed 2 years ago

jtraglia commented 2 years ago

Hi there. Noticed something that I believe is a bug in sszgen.

Given a structure with a uint64 array field, the field will unmarshal to a nil value if (1) the array is empty and (2) the variable that calls UnmarshalSSZ is not initialized. And if you swap uint64 for byte it works as expected.

See the following program:

package main

import "fmt"

type Foo struct {
  Bar []uint64 `json:"bar" ssz-max:"1024"`
}

func main() {
  val := Foo{Bar:[]uint64{}}
  var dec Foo // not initialized

  enc, err := val.MarshalSSZ()
  if err != nil {
    panic(err)
  }
  err = dec.UnmarshalSSZ(enc)
  if err != nil {
    panic(err)
  }

  fmt.Println("val.Bar nil?", val.Bar == nil)
  fmt.Println("dec.Bar nil?", dec.Bar == nil)
}

When executed, this will output:

$ sszgen --path main.go && go run .
val.Bar nil? false
dec.Bar nil? true

On the other hand, if dec.Bar is initialized with anything (even an empty array), it will behave as expected:

func main() {
  val := Foo{Bar:[]uint64{}}
  dec := Foo{Bar:[]uint64{}} // initialized

  enc, err := val.MarshalSSZ()
  if err != nil {
    panic(err)
  }
  err = dec.UnmarshalSSZ(enc)
  if err != nil {
    panic(err)
  }

  fmt.Println("val.Bar nil?", val.Bar == nil)
  fmt.Println("dec.Bar nil?", dec.Bar == nil)
}

When executed, this will output:

$ sszgen --path main.go && go run .
val.Bar nil? false
dec.Bar nil? false

Fixes: https://github.com/flashbots/go-boost-utils/issues/23

ferranbt commented 2 years ago

Can you share the generated go file?

ferranbt commented 2 years ago

I found the issue, it is not on the generated code but on this function that extends the uint64 array. It should be easy to fix.

ferranbt commented 2 years ago

Could you try the fix in #97 and see if it resolves the issue?

jtraglia commented 2 years ago

Yes, but I won't be able to today. On a day trip away from my workstation. Thank you for the quick fix!

jtraglia commented 2 years ago

@ferranbt Yes, it resolves the issue. Thanks!