go/neo/proto/RowInfo: Fix representation on the wire
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 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 kirr/neo!6
Showing
This diff is collapsed.
Please register or sign in to comment