Block Serialization round trip test fails with empty Tx or RawRootsList struct(s)

[jbowen93]

Error

Diff:
                                --- Expected
                                +++ Actual
                                @@ -38,7 +38,5 @@
                                  Data: (types.Data) {
                                -  Txs: (types.Txs) {
                                -  },
                                +  Txs: (types.Txs) <nil>,
                                   IntermediateStateRoots: (types.IntermediateStateRoots) {
                                -   RawRootsList: ([][]uint8) {
                                -   }
                                +   RawRootsList: ([][]uint8) <nil>
                                   },

Repro: add this test In types/serialization_test.go Line 60

{"No Txs", &Block{
            Header: Header{
                Version: Version{
                    Block: 1,
                    App:   2,
                },
                NamespaceID:     [8]byte{0, 1, 2, 3, 4, 5, 6, 7},
                Height:          3,
                Time:            4567,
                LastHeaderHash:  h[0],
                LastCommitHash:  h[1],
                DataHash:        h[2],
                ConsensusHash:   h[3],
                AppHash:         h[4],
                LastResultsHash: h[5],
                ProposerAddress: []byte{4, 3, 2, 1},
            },
            Data: Data{
                Txs:                    Txs{},
                IntermediateStateRoots: IntermediateStateRoots{RawRootsList: [][]byte{[]byte{0x1}}},
                // TODO(tzdybal): update when we have actual evidence types
                Evidence: EvidenceData{Evidence: nil},
            },
            LastCommit: Commit{
                Height:     8,
                HeaderHash: h[6],
                Signatures: []Signature{Signature([]byte{1, 1, 1}), Signature([]byte{2, 2, 2})},
            },
        }},

It gives

Diff:
                                --- Expected
                                +++ Actual
                                @@ -40,4 +40,3 @@
                                  Data: (types.Data) {
                                -  Txs: (types.Txs) {
                                -  },
                                +  Txs: (types.Txs) <nil>,
                                   IntermediateStateRoots: (types.IntermediateStateRoots) {
Test:           TestBlockSerializationRoundTrip/No_Txs

I ran into this in the getRandomBlock function of mock_test.go

func getRandomBlock(height uint64, nTxs int) *types.Block {
	block := &types.Block{
		Header: types.Header{
			Height: height,
		},
		Data: types.Data{
			Txs: make(types.Txs, nTxs),
			IntermediateStateRoots: types.IntermediateStateRoots{
				RawRootsList: make([][]byte, nTxs),
			},
		},
	}
	copy(block.Header.AppHash[:], getRandomBytes(32))

	for i := 0; i < nTxs; i++ {
		block.Data.Txs[i] = getRandomTx()
		block.Data.IntermediateStateRoots.RawRootsList[i] = getRandomBytes(32)
	}

	return block
}

I think make(types.Txs, nTxs) and make([][]byte, nTxs) give empty structs when nTxs is 0

The incompatibility is due to how the (b *Block) UnmarshalBinary works in types/serialization.go

func (b *Block) UnmarshalBinary(data []byte) error {
	var pBlock pb.Block
	err := pBlock.Unmarshal(data)
	if err != nil {
		return err
	}
	err = b.FromProto(&pBlock)
	return err
}

[Raneet10]

Does modifying txs :

message Data {
	repeated bytes txs = 1 [(gogoproto.nullable) = false];
	repeated bytes intermediate_state_roots = 2;
	repeated tendermint.abci.Evidence evidence = 3;
}

help in anyway? I could be missing something fundamental here but maybe the omitempty tag is responsible ?

[jbowen93]

Looks like this doesn't help

WARNING: field Data.Txs is a repeated non-nullable native type, nullable=false has no effect

[Manav-Aggarwal]

This seems like an issue with protobuf treating empty slices and nil values the same when serializing. This is because protobuf automatically uses the omitempty tag when doing buf generate. This seems like it can be fixed by using a custom json marshaller for protobuf. Relevant comment/issue in tendermint: https://github.com/tendermint/tendermint/issues/3882#issuecomment-527374037