tendermint / go-amino

Protobuf3 with Interface support - Designed for blockchains (deterministic, upgradeable, fast, and compact)
Other
260 stars 78 forks source link

Empty slices round-trip as nil #275

Open michaelfig opened 5 years ago

michaelfig commented 5 years ago

Description

I'm using go-amino as the codec in the cosmos-sdk. I expect that when a zero-length slice is marshalled then unmarshalled, it would be returned as a zero-length slice (just as encoding/json does).

However, what amino actually returns is a nil slice.

Running the test program

The below program shows this output:

$ go build -o aminotest .
$ ./aminotest
Sent: []int{}
json: 00000000  7b 22 53 6c 69 63 65 22  3a 5b 5d 7d              |{"Slice":[]}|
JSON Received: []int{}
amino: 00000000  24 d1 85 73                                       |$..s|
Amino Received: []int(nil)
$

As you see, JSON roundtrips, but Amino does not.

Test program sources

Here is amino-empty-slice-roundtrip.go:

package main

import (
    "encoding/hex"
    "encoding/json"
    "fmt"

    amino "github.com/tendermint/go-amino"
)

type MySlice struct {
    Slice []int
}

// RegisterCodec registers concrete types on the Amino codec
func main() {
    cdc := amino.NewCodec()
    cdc.RegisterConcrete(MySlice{}, "test/MySlice", nil)

    var sin, aout, jout MySlice

    // Make an empty slice.
    sin.Slice = []int{}

    // Prints "Sent: []int{}", which is an empty slice.
    fmt.Printf("Sent: %#v\n", sin.Slice)

    // JSON Marshal and Unmarshal.
    bz, _ := json.Marshal(sin)
    fmt.Print("json: ", hex.Dump(bz))
    json.Unmarshal(bz, &jout)

    // Prints "JSON Received: []int{}", which is an empty slice.
    fmt.Printf("JSON Received: %#v\n", jout.Slice)

    // Amino Marshal and Unmarshal.
    bz2 := cdc.MustMarshalBinaryBare(sin)
    fmt.Print("amino: ", hex.Dump(bz2))
    cdc.MustUnmarshalBinaryBare(bz2, &aout)

    // Prints "Amino Received: []int(nil)", which is *not* an empty slice.
    fmt.Printf("Amino Received: %#v\n", aout.Slice)
}

Thanks for any help you can offer.

liamsi commented 5 years ago

Thanks! A similar problem was noticed a while ago regarding byte slices: https://github.com/tendermint/go-amino/pull/209#discussion_r205491486

This is probably a rather simple change: we need to initialize slices differently (on decoding).

Note to myself: Investigate if this is relevant for https://github.com/tendermint/go-amino/milestone/1

liamsi commented 5 years ago

Actually, given this protobuf message:

message IntSlice {
    repeated int64 Slice = 1;
}

and this test:

func TestEmptySlicesCompat(t *testing.T) {
    var slin, slout p3.IntSlice

    // Make an empty proto3 slice:
    slin.Slice = []int64{}

    pb, err := proto.Marshal(&slin)
    require.NoError(t, err)

    err = proto.Unmarshal(pb, &slout)
    require.NoError(t, err)

    assert.Equal(t, slin, slout)
}

reveals that this is exactly the same behaviour as protobuf:

--- FAIL: TestEmptySlicesCompat (0.00s)
    proto3_compat_test.go:403: 
            Error Trace:    proto3_compat_test.go:403
            Error:          Not equal: 
                            expected: proto3tests.IntSlice{Slice:[]int64{}, XXX_NoUnkeyedLiteral:struct {}{}, XXX_unrecognized:[]uint8(nil), XXX_sizecache:0}
                            actual  : proto3tests.IntSlice{Slice:[]int64(nil), XXX_NoUnkeyedLiteral:struct {}{}, XXX_unrecognized:[]uint8(nil), XXX_sizecache:0}

                            Diff:
            Test:           TestEmptySlicesCompat

In other words: gogoproto, given an encoded []int64{} returns a []int64(nil) when decoding.

The only difference here is that the byte encoding differs but not semantically (amino returns []byte(nil) while protobuf returns []byte{}).