• Levin Zimmermann's avatar
    go/neo/proto/RowInfo: Fix representation on the wire · 7f96ad77
    Levin Zimmermann authored
    Some NEO protocol packets have the field 'RowList'. This field contains
    information about each row of a partition table. In NEO/go the information
    of each row is represented with the 'RowInfo' type [1]. This type is defined
    as a struct with the field ‘CellList’. ‘CellList’ is defined as a list of
    'CellInfo' [1] (e.g. an entry for each cell).
    
    NEO/go {en,de}codes struct types with ‘genStructHead’ (structures in
    golang are encoded as arrays in msgpack) [2]. From the 'RowList'
    definition, the msgpack decoder currently expects the following msgpack
    array structure:
    
        ArrayHeader (RowList)
    
            ArrayHeader (RowInfo)
    
                ArrayHeader (CellList)
    
                    ArrayHeader (CellInfo)
    
                        int32 (NID)
                        enum (State)
    
    However NEO/py actually sends:
    
        ArrayHeader (RowList)
    
            ArrayHeader (CellList)
    
                ArrayHeader (CellInfo)
    
                    int32 (NID)
                    enum (State)
    
    In other words, currently the NEO/go msgpack encoder expects one more
    nesting, which NEO/py doesn’t provide (and which also doesn’t seem to
    be necessary as the outer nesting would always only contain one
    element). We could adjust the msgpack {en,de}coder to introduce an
    exception for the 'RowInfo' type, however as the protocol definition in
    'proto.go' aims to transparently reflect the structure of the packets on
    the wire, it seems to be more appropriate to fix this straight in the
    protocol definition. This is also less error-prone as we don't have to
    fix all the different positions of the encoder, decoder & sizer and it's
    less code (particularly if 'RowInfo' doesn't stay the only case for such
    an issue).
    
    [1] https://lab.nexedi.com/kirr/neo/-/blob/1ad088c8/go/neo/proto/proto.go#L391-394
    [2] https://lab.nexedi.com/kirr/neo/-/blob/1ad088c8/go/neo/proto/protogen.go#L1770-1775
    
    --------
    
    kirr: I've applied the following interdiff to the original patch of
    levin.zimmermann/neoppod@c93d5dbc :
    
        --- a/go/neo/neo_test.go
        +++ b/go/neo/neo_test.go
        @@ -100,7 +100,7 @@ func _TestMasterStorage(t0 *tEnv) {
                        PTid:           1,
                        NumReplicas:    0,
                        RowList:        []proto.RowInfo{
        -                       proto.RowInfo{proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
        +                       {proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
                        },
                }))
    
        @@ -173,7 +173,7 @@ func _TestMasterStorage(t0 *tEnv) {
                        PTid:           1,
                        NumReplicas:    0,
                        RowList:        []proto.RowInfo{
        -                       proto.RowInfo{proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
        +                       {proto.CellInfo{proto.NID(proto.STORAGE, 1), proto.UP_TO_DATE}},
                        },
                }))
    
        --- a/go/neo/proto/proto_test.go
        +++ b/go/neo/proto/proto_test.go
        @@ -210,9 +210,9 @@ func TestMsgMarshal(t *testing.T) {
                                PTid:        0x0102030405060708,
                                NumReplicas: 34,
                                RowList: []RowInfo{
        -                               {CellInfo{11, UP_TO_DATE}, CellInfo{17, OUT_OF_DATE}},
        -                               {CellInfo{11, FEEDING}},
        -                               {CellInfo{11, CORRUPTED}, CellInfo{15, DISCARDED}, CellInfo{23, UP_TO_DATE}},
        +                               {{11, UP_TO_DATE}, {17, OUT_OF_DATE}},
        +                               {{11, FEEDING}},
        +                               {{11, CORRUPTED}, {15, DISCARDED}, {23, UP_TO_DATE}},
                                },
                                },
    
        @@ -229,9 +229,9 @@ func TestMsgMarshal(t *testing.T) {
                                hex("cf0102030405060708") +
                                hex("22") +
                                hex("93") +
        -                               hex("92"+"920bd40001"+"9211d40000") +
        -                               hex("91"+"920bd40002") +
        -                               hex("93"+"920bd40003"+"920fd40004"+"9217d40001"),
        +                               hex("92"+"920bd40401"+"9211d40400") +
        +                               hex("91"+"920bd40402") +
        +                               hex("93"+"920bd40403"+"920fd40404"+"9217d40401"),
                        },
    
                        // map[Oid]struct {Tid,Tid,bool}
    
    for cosmetics and because the tests were failing as
    
        --- FAIL: TestMsgMarshal (0.00s)
            proto_test.go:106: M/proto.AnswerPartitionTable: encode result unexpected:
            proto_test.go:107:  have: 93cf0102030405060708229392920bd404019211d4040091920bd4040293920bd40403920fd404049217d40401
            proto_test.go:108:  want: 93cf0102030405060708229392920bd400019211d4000091920bd4000293920bd40003920fd400049217d40001
    
    /reviewed-by @kirr
    /reviewed-on !6
    7f96ad77
proto_test.go 12.4 KB