Skip to content

Commit

Permalink
enhancement(pkg/scale): remove sorting logic from encodeMap method (#…
Browse files Browse the repository at this point in the history
…2904)

In go it's understood that the order of the keys is non-deterministic. In this case, we should be testing that we can encode a map and decode into a map, and assert that the maps are equal. 
This commit ensures that decoding into a rust BTreeMap via the parity-scale-codec works as expected. To make the tests more deterministic, we have taken a map with a single key or a map with two keys, and asserted that it's equal to one of the two encoding and removed the sorting logic altogether.
  • Loading branch information
axaysagathiya authored Nov 2, 2022
1 parent 30f903b commit c331361
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 34 deletions.
15 changes: 4 additions & 11 deletions pkg/scale/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"io"
"math/big"
"reflect"
"sort"
)

// Encoder scale encodes to a given io.Writer.
Expand Down Expand Up @@ -233,21 +232,15 @@ func (es *encodeState) encodeMap(in interface{}) (err error) {
return fmt.Errorf("encoding length: %w", err)
}

mapKeys := v.MapKeys()

sort.Slice(mapKeys, func(i, j int) bool {
keyByteOfI, _ := Marshal(mapKeys[i].Interface())
keyByteOfJ, _ := Marshal(mapKeys[j].Interface())
return bytes.Compare(keyByteOfI, keyByteOfJ) < 0
})

for _, key := range mapKeys {
iterator := v.MapRange()
for iterator.Next() {
key := iterator.Key()
err = es.marshal(key.Interface())
if err != nil {
return fmt.Errorf("encoding map key: %w", err)
}

mapValue := v.MapIndex(key)
mapValue := iterator.Value()
if !mapValue.CanInterface() {
continue
}
Expand Down
51 changes: 28 additions & 23 deletions pkg/scale/encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -909,28 +909,9 @@ var (
},
}

mapTests = tests{
{
name: "testMap1",
in: map[int8][]byte{2: []byte("some string")},
want: []byte{4, 2, 44, 115, 111, 109, 101, 32, 115, 116, 114, 105, 110, 103},
},
{
name: "testMap2",
in: map[int8][]byte{
2: []byte("some string"),
16: []byte("lorem ipsum"),
},
want: []byte{
8, 2, 44, 115, 111, 109, 101, 32, 115, 116, 114, 105, 110, 103, 16, 44, 108, 111, 114, 101, 109, 32,
105, 112, 115, 117, 109,
},
},
}

allTests = newTests(
fixedWidthIntegerTests, variableWidthIntegerTests, stringTests,
boolTests, structTests, sliceTests, arrayTests, mapTests,
boolTests, structTests, sliceTests, arrayTests,
varyingDataTypeTests,
)
)
Expand Down Expand Up @@ -1116,6 +1097,32 @@ func Test_encodeState_encodeArray(t *testing.T) {
}

func Test_encodeState_encodeMap(t *testing.T) {
mapTests := []struct {
name string
in interface{}
wantErr bool
wantOneOf [][]byte
}{
{
name: "testMap1",
in: map[int8][]byte{2: []byte("some string")},
wantOneOf: [][]byte{{4, 2, 44, 115, 111, 109, 101, 32, 115, 116, 114, 105, 110, 103}},
},
{
name: "testMap2",
in: map[int8][]byte{
2: []byte("some string"),
16: []byte("lorem ipsum"),
},
wantOneOf: [][]byte{
{8, 2, 44, 115, 111, 109, 101, 32, 115, 116, 114, 105, 110, 103, 16, 44, 108, 111, 114, 101, 109, 32,
105, 112, 115, 117, 109},
{8, 16, 44, 108, 111, 114, 101, 109, 32, 105, 112, 115, 117, 109, 2, 44, 115, 111, 109, 101, 32, 115,
116, 114, 105, 110, 103},
},
},
}

for _, tt := range mapTests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -1127,9 +1134,7 @@ func Test_encodeState_encodeMap(t *testing.T) {
if err := es.marshal(tt.in); (err != nil) != tt.wantErr {
t.Errorf("encodeState.encodeMap() error = %v, wantErr %v", err, tt.wantErr)
}
if !reflect.DeepEqual(buffer.Bytes(), tt.want) {
t.Errorf("encodeState.encodeMap() = %v, want %v", buffer.Bytes(), tt.want)
}
assert.Contains(t, tt.wantOneOf, buffer.Bytes())
})
}
}
Expand Down

0 comments on commit c331361

Please sign in to comment.