INFURA / go-ethlibs

Ethereum libraries in Go for interacting with Ethereum nodes
MIT License
161 stars 34 forks source link

fix(MarshalJSON): use value receiver for all eth types #61

Closed timmyers closed 2 years ago

ryanschneider commented 2 years ago

Ok, we pinpointed the issue, basically in the test below b1 and b2 were different values:

  log := eth.Log{ ... }
  b1, err := json.Marshall(log)
  b2, err := json.Marshal(&log)

The cause is related to the implementation of json.Marshal, basically it seems like if you pass a struct (vs. a pointer to a struct) then the MarshalJSON method on fields is only called if a) the field is a pointer or b) the embedded field implements MarshalJSON on the value receiver, not the pointer receiver.

ryanschneider commented 2 years ago

Separately seems like CircleCI is having issues go mod download-ing, this will eventually be fixed by #41.

basgys commented 2 years ago

@ryanschneider @timmyers Not directly related to the issue, but we could also simplify with the TextMarshaler and TextUnmarshaler interface, so we don't have to deal with json.Marshal/Unmarshal in our functions for the base64 encoding/decoding.

Those text marshalers would focus solely on validating and returning a canonical representation of an address, which would be compatible with other encoding formats.

Before

func (a *Address) UnmarshalJSON(data []byte) error {
    // We'll keep the checksummed string in memory so we can use it for internal representations
    str, err := unmarshalHex(data, 20, "data")
    str = ToChecksumAddress(str)
    if err != nil {
        return err
    }
    *a = Address(str)
    return nil
}

func (a Address) MarshalJSON() ([]byte, error) {
    // Seems like geth and parity both return the lower-cased string rather than the checksummed one
    s := strings.ToLower(string(a))
    return json.Marshal(&s)
}

After

func (a *Address) UnmarshalText(text []byte) error {
    str, err := validateHex(string(text), 20, "data")
    if err != nil {
        return err
    }
    str = ToChecksumAddress(str)
    *a = Address(str)
    return nil
}

func (a Address) MarshalText() ([]byte, error) {
    return bytes.ToLower([]byte(a)), nil
}
ryanschneider commented 2 years ago

Good call @basgys I created #62 to track this change.