From eb5725b80e20f851cf34850d3e0712542a25f5e3 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Mon, 18 Jul 2022 18:33:09 +0000 Subject: [PATCH 1/2] lib/trie: add `Version` type - `String()` method - `ParseVersion` function - Unit tests --- lib/trie/version.go | 66 +++++++++++++++++++++ lib/trie/version_test.go | 124 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+) create mode 100644 lib/trie/version.go create mode 100644 lib/trie/version_test.go diff --git a/lib/trie/version.go b/lib/trie/version.go new file mode 100644 index 0000000000..99dac62c0e --- /dev/null +++ b/lib/trie/version.go @@ -0,0 +1,66 @@ +// Copyright 2022 ChainSafe Systems (ON) +// SPDX-License-Identifier: LGPL-3.0-only + +package trie + +import ( + "errors" + "fmt" + "strings" +) + +// Version is the state trie version which dictates how a +// Merkle root should be constructed. It is defined in +// https://spec.polkadot.network/#defn-state-version +type Version uint8 + +const ( + // V0 is the state trie version 0 where the values of the keys are + // inserted into the trie directly. + // TODO set to iota once CI passes + V0 Version = 1 +) + +func (v Version) String() string { + switch v { + case V0: + return "v0" + default: + panic(fmt.Sprintf("unknown version %d", v)) + } +} + +var ErrParseVersion = errors.New("parsing version failed") + +// ParseVersion parses a state trie version string. +func ParseVersion(s string) (version Version, err error) { + switch { + case strings.EqualFold(s, V0.String()): + return V0, nil + default: + choices := orStrings([]string{V0.String()}) + return version, fmt.Errorf("%w: %q must be %s", + ErrParseVersion, s, choices) + } +} + +func orStrings(strings []string) (result string) { + return joinStrings(strings, "or") +} + +func joinStrings(strings []string, lastJoin string) (result string) { + if len(strings) == 0 { + return "" + } + + result = strings[0] + for i := 1; i < len(strings); i++ { + if i < len(strings)-1 { + result += ", " + strings[i] + } else { + result += " " + lastJoin + " " + strings[i] + } + } + + return result +} diff --git a/lib/trie/version_test.go b/lib/trie/version_test.go new file mode 100644 index 0000000000..0c410df729 --- /dev/null +++ b/lib/trie/version_test.go @@ -0,0 +1,124 @@ +// Copyright 2022 ChainSafe Systems (ON) +// SPDX-License-Identifier: LGPL-3.0-only + +package trie + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_Version_String(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + version Version + versionString string + panicMessage string + }{ + "v0": { + version: V0, + versionString: "v0", + }, + "invalid": { + version: Version(99), + panicMessage: "unknown version 99", + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + if testCase.panicMessage != "" { + assert.PanicsWithValue(t, testCase.panicMessage, func() { + _ = testCase.version.String() + }) + return + } + + versionString := testCase.version.String() + assert.Equal(t, testCase.versionString, versionString) + }) + } +} + +func Test_ParseVersion(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + s string + version Version + errWrapped error + errMessage string + }{ + "v0": { + s: "v0", + version: V0, + }, + "V0": { + s: "V0", + version: V0, + }, + "invalid": { + s: "xyz", + errWrapped: ErrParseVersion, + errMessage: "parsing version failed: \"xyz\" must be v0", + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + version, err := ParseVersion(testCase.s) + + assert.Equal(t, testCase.version, version) + assert.ErrorIs(t, err, testCase.errWrapped) + if testCase.errWrapped != nil { + assert.EqualError(t, err, testCase.errMessage) + } + }) + } +} + +func Test_orStrings(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + strings []string + s string + }{ + "nil": {}, + "v0": { + strings: []string{"v0"}, + s: "v0", + }, + "v0 or v1": { + strings: []string{"v0", "v1"}, + s: "v0 or v1", + }, + "v0, v1 or v2": { + strings: []string{"v0", "v1", "v2"}, + s: "v0, v1 or v2", + }, + "v0, v1, v2 or v3": { + strings: []string{"v0", "v1", "v2", "v3"}, + s: "v0, v1, v2 or v3", + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + s := orStrings(testCase.strings) + + assert.Equal(t, testCase.s, s) + }) + } +} From ddf17e3b6e7ac0eafaf31eca69e353946ae544b0 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Wed, 10 Aug 2022 22:40:28 +0000 Subject: [PATCH 2/2] Remove `orStrings` and just make it simple! --- lib/trie/version.go | 24 +----------------------- lib/trie/version_test.go | 38 -------------------------------------- 2 files changed, 1 insertion(+), 61 deletions(-) diff --git a/lib/trie/version.go b/lib/trie/version.go index 99dac62c0e..23527890c8 100644 --- a/lib/trie/version.go +++ b/lib/trie/version.go @@ -38,29 +38,7 @@ func ParseVersion(s string) (version Version, err error) { case strings.EqualFold(s, V0.String()): return V0, nil default: - choices := orStrings([]string{V0.String()}) return version, fmt.Errorf("%w: %q must be %s", - ErrParseVersion, s, choices) + ErrParseVersion, s, V0) } } - -func orStrings(strings []string) (result string) { - return joinStrings(strings, "or") -} - -func joinStrings(strings []string, lastJoin string) (result string) { - if len(strings) == 0 { - return "" - } - - result = strings[0] - for i := 1; i < len(strings); i++ { - if i < len(strings)-1 { - result += ", " + strings[i] - } else { - result += " " + lastJoin + " " + strings[i] - } - } - - return result -} diff --git a/lib/trie/version_test.go b/lib/trie/version_test.go index 0c410df729..ab2ac03ebe 100644 --- a/lib/trie/version_test.go +++ b/lib/trie/version_test.go @@ -84,41 +84,3 @@ func Test_ParseVersion(t *testing.T) { }) } } - -func Test_orStrings(t *testing.T) { - t.Parallel() - - testCases := map[string]struct { - strings []string - s string - }{ - "nil": {}, - "v0": { - strings: []string{"v0"}, - s: "v0", - }, - "v0 or v1": { - strings: []string{"v0", "v1"}, - s: "v0 or v1", - }, - "v0, v1 or v2": { - strings: []string{"v0", "v1", "v2"}, - s: "v0, v1 or v2", - }, - "v0, v1, v2 or v3": { - strings: []string{"v0", "v1", "v2", "v3"}, - s: "v0, v1, v2 or v3", - }, - } - - for name, testCase := range testCases { - testCase := testCase - t.Run(name, func(t *testing.T) { - t.Parallel() - - s := orStrings(testCase.strings) - - assert.Equal(t, testCase.s, s) - }) - } -}