Skip to content

Commit 901376b

Browse files
qdm12qdm12
committed
Core_Version codec changes
- Always encode using all latest fields - Decode required fields then optional fields - Remove `legacyData` struct - Update `Encode` and `DecodeVersion` comments - Update unit tests
1 parent 30b61b6 commit 901376b

File tree

2 files changed

+121
-103
lines changed

2 files changed

+121
-103
lines changed

‎lib/runtime/version.go

Lines changed: 46 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
package runtime
55

66
import (
7+
"bytes"
78
"errors"
89
"fmt"
9-
"strings"
1010

1111
"github.com/ChainSafe/gossamer/pkg/scale"
1212
)
@@ -26,79 +26,65 @@ type Version struct {
2626
ImplVersion uint32
2727
APIItems []APIItem
2828
TransactionVersion uint32
29-
legacy bool
30-
}
31-
32-
type legacyData struct {
33-
SpecName []byte
34-
ImplName []byte
35-
AuthoringVersion uint32
36-
SpecVersion uint32
37-
ImplVersion uint32
38-
APIItems []APIItem
3929
}
4030

4131
// Encode returns the scale encoding of the version.
32+
// Note the encoding contains all the latest Core_version fields as defined in
33+
// https://spec.polkadot.network/#defn-rt-core-version
34+
// In other words, decoding older version data with missing fields
35+
// and then encoding it will result in a longer encoding due to the
36+
// extra version fields. This however remains compatible since the
37+
// version fields are still encoded in the same order and an older
38+
// decoder would succeed with the longer encoding.
4239
func (v *Version) Encode() (encoded []byte, err error) {
43-
if !v.legacy {
44-
return scale.Marshal(*v)
45-
}
46-
47-
toEncode := legacyData{
48-
SpecName: v.SpecName,
49-
ImplName: v.ImplName,
50-
AuthoringVersion: v.AuthoringVersion,
51-
SpecVersion: v.SpecVersion,
52-
ImplVersion: v.ImplVersion,
53-
APIItems: v.APIItems,
54-
}
55-
return scale.Marshal(toEncode)
40+
return scale.Marshal(*v)
5641
}
5742

5843
var (
59-
ErrDecodingVersion = errors.New("decoding version")
60-
ErrDecodingLegacyVersion = errors.New("decoding legacy version")
44+
ErrDecodingVersionField = errors.New("decoding version field")
6145
)
6246

63-
// DecodeVersion scale decodes the encoded version data and returns an error.
64-
// It first tries to decode the data using the current version format.
65-
// If that fails with an EOF error, it then tries to decode the data
66-
// using the legacy version format (for Kusama).
47+
// DecodeVersion scale decodes the encoded version data.
48+
// For older version data with missing fields (such as `transaction_version`)
49+
// the missing field is set to its zero value (such as `0`).
6750
func DecodeVersion(encoded []byte) (version Version, err error) {
68-
err = scale.Unmarshal(encoded, &version)
69-
if err == nil {
70-
return version, nil
71-
}
51+
reader := bytes.NewReader(encoded)
52+
decoder := scale.NewDecoder(reader)
7253

73-
if !strings.Contains(err.Error(), "EOF") {
74-
// TODO io.EOF should be wrapped in scale and
75-
// ErrDecodingVersion should be removed once this is done.
76-
return version, fmt.Errorf("%w: %s", ErrDecodingVersion, err)
54+
type namedValue struct {
55+
// name is the field name used to wrap eventual codec errors
56+
name string
57+
// value is the field value to handle
58+
value interface{}
7759
}
7860

79-
// TODO: kusama seems to use the legacy version format
80-
var legacy legacyData
81-
err = scale.Unmarshal(encoded, &legacy)
82-
if err != nil {
83-
// TODO sentinel error should be wrapped in scale and
84-
// ErrDecodingLegacyVersion should be removed once this is done.
85-
return version, fmt.Errorf("%w: %s", ErrDecodingLegacyVersion, err)
61+
requiredFields := [...]namedValue{
62+
{name: "spec name", value: &version.SpecName},
63+
{name: "impl name", value: &version.ImplName},
64+
{name: "authoring version", value: &version.AuthoringVersion},
65+
{name: "spec version", value: &version.SpecVersion},
66+
{name: "impl version", value: &version.ImplVersion},
67+
{name: "API items", value: &version.APIItems},
68+
}
69+
for _, requiredField := range requiredFields {
70+
err = decoder.Decode(requiredField.value)
71+
if err != nil {
72+
return Version{}, fmt.Errorf("%w %s: %s", ErrDecodingVersionField, requiredField.name, err)
73+
}
8674
}
8775

88-
return Version{
89-
SpecName: legacy.SpecName,
90-
ImplName: legacy.ImplName,
91-
AuthoringVersion: legacy.AuthoringVersion,
92-
SpecVersion: legacy.SpecVersion,
93-
ImplVersion: legacy.ImplVersion,
94-
APIItems: legacy.APIItems,
95-
legacy: true,
96-
}, nil
97-
}
76+
optionalFields := [...]namedValue{
77+
{name: "transaction version", value: &version.TransactionVersion},
78+
}
79+
for _, optionalField := range optionalFields {
80+
err = decoder.Decode(optionalField.value)
81+
if err != nil {
82+
if err.Error() == "EOF" {
83+
return version, nil
84+
}
85+
return Version{}, fmt.Errorf("%w %s: %s", ErrDecodingVersionField, optionalField.name, err)
86+
}
87+
}
9888

99-
// WithLegacy sets the legacy boolean (for Kusama)
100-
// and is only used for tests.
101-
func (v Version) WithLegacy() Version {
102-
v.legacy = true //skipcq: RVV-B0006
103-
return v
89+
return version, nil
10490
}

‎lib/runtime/version_test.go

Lines changed: 75 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,45 +6,67 @@ package runtime
66
import (
77
"testing"
88

9+
"github.com/ChainSafe/gossamer/pkg/scale"
910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
1112
)
1213

13-
func Test_Version_Encode(t *testing.T) {
14-
t.Parallel()
14+
func scaleEncode(t *testing.T, x any) []byte {
15+
encoded, err := scale.Marshal(x)
16+
require.NoError(t, err)
17+
return encoded
18+
}
1519

16-
someVersion := Version{
17-
SpecName: []byte{1},
18-
ImplName: []byte{2},
19-
AuthoringVersion: 3,
20-
SpecVersion: 4,
21-
ImplVersion: 5,
22-
APIItems: []APIItem{{
23-
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
24-
Ver: 6,
25-
}},
26-
TransactionVersion: 7,
20+
func concatBytes(slices [][]byte) (concatenated []byte) {
21+
for _, slice := range slices {
22+
concatenated = append(concatenated, slice...)
2723
}
24+
return concatenated
25+
}
26+
27+
func Test_Version_Encode(t *testing.T) {
28+
t.Parallel()
2829

2930
testCases := map[string]struct {
3031
version Version
3132
encoding []byte
3233
errWrapped error
3334
errMessage string
3435
}{
35-
"not legacy": {
36-
version: someVersion,
36+
"no optional field set": {
37+
version: Version{
38+
SpecName: []byte{1},
39+
ImplName: []byte{2},
40+
AuthoringVersion: 3,
41+
SpecVersion: 4,
42+
ImplVersion: 5,
43+
APIItems: []APIItem{{
44+
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
45+
Ver: 6,
46+
}},
47+
},
3748
encoding: []byte{
3849
0x4, 0x1, 0x4, 0x2, 0x3, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0,
3950
0x5, 0x0, 0x0, 0x0, 0x4, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
40-
0x8, 0x6, 0x0, 0x0, 0x0, 0x7, 0x0, 0x0, 0x0},
51+
0x8, 0x6, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0},
4152
},
42-
"legacy": {
43-
version: someVersion.WithLegacy(),
53+
"all optional fields set": {
54+
version: Version{
55+
SpecName: []byte{1},
56+
ImplName: []byte{2},
57+
AuthoringVersion: 3,
58+
SpecVersion: 4,
59+
ImplVersion: 5,
60+
APIItems: []APIItem{{
61+
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
62+
Ver: 6,
63+
}},
64+
TransactionVersion: 7,
65+
},
4466
encoding: []byte{
4567
0x4, 0x1, 0x4, 0x2, 0x3, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0,
4668
0x5, 0x0, 0x0, 0x0, 0x4, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
47-
0x8, 0x6, 0x0, 0x0, 0x0},
69+
0x8, 0x6, 0x0, 0x0, 0x0, 0x7, 0x0, 0x0, 0x0},
4870
},
4971
}
5072

@@ -73,11 +95,35 @@ func Test_DecodeVersion(t *testing.T) {
7395
errWrapped error
7496
errMessage string
7597
}{
76-
"unmarshal success": {
98+
"required field decode error": {
99+
encoded: concatBytes([][]byte{
100+
scaleEncode(t, []byte{1, 2}),
101+
{255, 255}, // error
102+
}),
103+
errWrapped: ErrDecodingVersionField,
104+
errMessage: "decoding version field impl name: could not decode invalid integer",
105+
},
106+
// TODO add transaction version decode error once
107+
// https://github.com/ChainSafe/gossamer/pull/2683
108+
// is merged.
109+
// "transaction version decode error": {
110+
// encoded: concatBytes([][]byte{
111+
// scaleEncode(t, []byte("a")), // spec name
112+
// scaleEncode(t, []byte("b")), // impl name
113+
// scaleEncode(t, uint32(1)), // authoring version
114+
// scaleEncode(t, uint32(2)), // spec version
115+
// scaleEncode(t, uint32(3)), // impl version
116+
// scaleEncode(t, []APIItem{{}}), // api items
117+
// {1, 2, 3}, // transaction version
118+
// }),
119+
// errWrapped: ErrDecoding,
120+
// errMessage: "decoding transaction version: could not decode invalid integer",
121+
// },
122+
"no optional field set": {
77123
encoded: []byte{
78124
0x4, 0x1, 0x4, 0x2, 0x3, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0,
79125
0x5, 0x0, 0x0, 0x0, 0x4, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
80-
0x8, 0x6, 0x0, 0x0, 0x0, 0x7, 0x0, 0x0, 0x0},
126+
0x8, 0x6, 0x0, 0x0, 0x0},
81127
version: Version{
82128
SpecName: []byte{1},
83129
ImplName: []byte{2},
@@ -88,24 +134,13 @@ func Test_DecodeVersion(t *testing.T) {
88134
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
89135
Ver: 6,
90136
}},
91-
TransactionVersion: 7,
92137
},
93138
},
94-
"unmarshal error": {
95-
encoded: []byte{255, 255},
96-
errWrapped: ErrDecodingVersion,
97-
errMessage: "decoding version: could not decode invalid integer, field: []",
98-
},
99-
"legacy unmarshal error": {
100-
encoded: []byte{0},
101-
errWrapped: ErrDecodingLegacyVersion,
102-
errMessage: "decoding legacy version: EOF, field: []",
103-
},
104-
"legacy unmarshal success": {
139+
"transaction version set": {
105140
encoded: []byte{
106141
0x4, 0x1, 0x4, 0x2, 0x3, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0,
107142
0x5, 0x0, 0x0, 0x0, 0x4, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
108-
0x8, 0x6, 0x0, 0x0, 0x0},
143+
0x8, 0x6, 0x0, 0x0, 0x0, 0x7, 0x0, 0x0, 0x0},
109144
version: Version{
110145
SpecName: []byte{1},
111146
ImplName: []byte{2},
@@ -116,7 +151,7 @@ func Test_DecodeVersion(t *testing.T) {
116151
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
117152
Ver: 6,
118153
}},
119-
legacy: true,
154+
TransactionVersion: 7,
120155
},
121156
},
122157
}
@@ -145,7 +180,7 @@ func Test_Version_Scale(t *testing.T) {
145180
encoding []byte
146181
decoded Version
147182
}{
148-
"current version": {
183+
"no optional field set": {
149184
version: Version{
150185
SpecName: []byte{1},
151186
ImplName: []byte{2},
@@ -156,12 +191,11 @@ func Test_Version_Scale(t *testing.T) {
156191
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
157192
Ver: 6,
158193
}},
159-
TransactionVersion: 7,
160194
},
161195
encoding: []byte{
162196
0x4, 0x1, 0x4, 0x2, 0x3, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0,
163197
0x5, 0x0, 0x0, 0x0, 0x4, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
164-
0x8, 0x6, 0x0, 0x0, 0x0, 0x7, 0x0, 0x0, 0x0},
198+
0x8, 0x6, 0x0, 0x0, 0x0},
165199
decoded: Version{
166200
SpecName: []byte{1},
167201
ImplName: []byte{2},
@@ -172,10 +206,9 @@ func Test_Version_Scale(t *testing.T) {
172206
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
173207
Ver: 6,
174208
}},
175-
TransactionVersion: 7,
176209
},
177210
},
178-
"legacy version": {
211+
"transaction version set": {
179212
version: Version{
180213
SpecName: []byte{1},
181214
ImplName: []byte{2},
@@ -187,12 +220,11 @@ func Test_Version_Scale(t *testing.T) {
187220
Ver: 6,
188221
}},
189222
TransactionVersion: 7,
190-
legacy: true,
191223
},
192224
encoding: []byte{
193225
0x4, 0x1, 0x4, 0x2, 0x3, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0,
194226
0x5, 0x0, 0x0, 0x0, 0x4, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
195-
0x8, 0x6, 0x0, 0x0, 0x0},
227+
0x8, 0x6, 0x0, 0x0, 0x0, 0x7, 0x0, 0x0, 0x0},
196228
decoded: Version{
197229
SpecName: []byte{1},
198230
ImplName: []byte{2},
@@ -203,7 +235,7 @@ func Test_Version_Scale(t *testing.T) {
203235
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
204236
Ver: 6,
205237
}},
206-
legacy: true,
238+
TransactionVersion: 7,
207239
},
208240
},
209241
}

0 commit comments

Comments
 (0)