From c331361ed4a6bcbb39ba0302d94cf78c3a381ccd Mon Sep 17 00:00:00 2001
From: Axay Sagathiya <axaysagathiya@gmail.com>
Date: Wed, 2 Nov 2022 13:22:04 +0530
Subject: [PATCH] enhancement(pkg/scale): remove sorting logic from encodeMap
 method (#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.
---
 pkg/scale/encode.go      | 15 ++++--------
 pkg/scale/encode_test.go | 51 ++++++++++++++++++++++------------------
 2 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/pkg/scale/encode.go b/pkg/scale/encode.go
index e32696bb60..20b071ef87 100644
--- a/pkg/scale/encode.go
+++ b/pkg/scale/encode.go
@@ -10,7 +10,6 @@ import (
 	"io"
 	"math/big"
 	"reflect"
-	"sort"
 )
 
 // Encoder scale encodes to a given io.Writer.
@@ -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
 		}
diff --git a/pkg/scale/encode_test.go b/pkg/scale/encode_test.go
index 6864c256de..519beae934 100644
--- a/pkg/scale/encode_test.go
+++ b/pkg/scale/encode_test.go
@@ -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,
 	)
 )
@@ -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) {
@@ -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())
 		})
 	}
 }