Added new test and used static var declarations for keyFieldIdx

waku/handshake.go

@@ -6,15 +6,31 @@ import (
 	"io"
 	"math"
 	"reflect"
-	"strconv"
-	"strings"
 
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
-var defaultMinPoW = math.Float64bits(0.001)
+type statusOptionKey uint
 
-type statusOptionKey int64
+var (
+	defaultMinPoW = math.Float64bits(0.001)
+	idxFieldKey = map[int]statusOptionKey{
+		0: 0,
+		1: 1,
+		2: 2,
+		3: 3,
+		4: 4,
+		5: 5,
+	}
+	keyFieldIdx = map[statusOptionKey]int{
+		0: 0,
+		1: 1,
+		2: 2,
+		3: 3,
+		4: 4,
+		5: 5,
+	}
+)
 
 // statusOptions defines additional information shared between peers
 // during the handshake.
@@ -29,9 +45,6 @@ type statusOptions struct {
 	ConfirmationsEnabled *bool       `rlp:"key=3"`
 	RateLimits           *RateLimits `rlp:"key=4"`
 	TopicInterest        []TopicType `rlp:"key=5"`
-
-	idxFieldKey map[int]statusOptionKey
-	keyFieldIdx map[statusOptionKey]int
 }
 
 // WithDefaults adds the default values for a given peer.
@@ -63,51 +76,6 @@ func (o statusOptions) WithDefaults() statusOptions {
 	return o
 }
 
-//var idxFieldKey = make(map[int]statusOptionKey)
-//var keyFieldIdx = map[statusOptionKey]int{}
-
-func (o *statusOptions) parseStatusOptionKeys() error {
-	kfi := make(map[statusOptionKey]int)
-	ifk := make(map[int]statusOptionKey)
-
-	v := reflect.ValueOf(o)
-
-	for i := 0; i < v.NumField(); i++ {
-		// skip unexported fields
-		if !v.Field(i).CanInterface() {
-			continue
-		}
-		rlpTag := v.Type().Field(i).Tag.Get("rlp")
-		// skip fields without rlp field tag
-		if rlpTag == "" {
-			continue
-		}
-
-		keys := strings.Split(rlpTag, "=")
-		// skip rlp tags that cannot be split by "="
-		// TODO Do we want to throw an error here if the length is not exactly 2?
-		if len(keys) < 2 {
-			continue
-		}
-
-		// parse keys[1] as an int
-		rkey, err := strconv.ParseInt(keys[1], 10, 64)
-		if err != nil {
-			return err
-		}
-
-		// typecast rkey to be of statusOptionKey type
-		key := statusOptionKey(rkey)
-		kfi[key] = i
-		ifk[i] = key
-	}
-
-	o.keyFieldIdx = kfi
-	o.idxFieldKey = ifk
-
-	return nil
-}
-
 func (o statusOptions) PoWRequirementF() *float64 {
 	if o.PoWRequirement == nil {
 		return nil
@@ -128,7 +96,7 @@ func (o statusOptions) EncodeRLP(w io.Writer) error {
 		field := v.Field(i)
 		if !field.IsNil() {
 			value := field.Interface()
-			key, ok := o.idxFieldKey[i]
+			key, ok := idxFieldKey[i]
 			if !ok {
 				continue
 			}
@@ -145,12 +113,6 @@ func (o *statusOptions) DecodeRLP(s *rlp.Stream) error {
 	if err != nil {
 		return fmt.Errorf("expected an outer list: %v", err)
 	}
-
-	err = o.parseStatusOptionKeys()
-	if err != nil {
-		return err
-	}
-
 	v := reflect.ValueOf(o)
 
 loop:
@@ -164,15 +126,25 @@ loop:
 		default:
 			return fmt.Errorf("expected an inner list: %v", err)
 		}
+
 		var key statusOptionKey
 		if err := s.Decode(&key); err != nil {
 			return fmt.Errorf("invalid key: %v", err)
 		}
+
+		// TODO encode as a string and parse in case incoming is encoded as string
+
+		// TODO once key data type is determined record the data type against the key value
+
+		// TODO this will mean that using a static keyFieldIdx mapping won't work as there is no way to know for any
+		//  incoming stream what data types the field keys will be defined as. Therefore the keyFieldIdx would need to
+		//  be associated with the instantiated struct.
+
 		// Skip processing if a key does not exist.
 		// It might happen when there is a new peer
 		// which supports a new option with
 		// a higher index.
-		idx, ok := o.keyFieldIdx[key]
+		idx, ok := keyFieldIdx[key]
 		if !ok {
 			// Read the rest of the list items and dump them.
 			_, err := s.Raw()

waku/handshake_test.go

@@ -3,6 +3,9 @@ package waku
 import (
 	"math"
 	"testing"
+	"reflect"
+	"strconv"
+	"strings"
 
 	"github.com/stretchr/testify/require"
 
@@ -63,3 +66,61 @@ func TestForwardCompatibility(t *testing.T) {
 	require.NoError(t, err)
 	require.EqualValues(t, statusOptions{PoWRequirement: &pow}, optsDecoded)
 }
+
+func TestStatusOptionKeys(t *testing.T) {
+	o := statusOptions{}
+
+	kfi := make(map[statusOptionKey]int)
+	ifk := make(map[int]statusOptionKey)
+
+	v := reflect.ValueOf(o)
+
+	for i := 0; i < v.NumField(); i++ {
+		// skip unexported fields
+		if !v.Field(i).CanInterface() {
+			continue
+		}
+		rlpTag := v.Type().Field(i).Tag.Get("rlp")
+		// skip fields without rlp field tag
+		if rlpTag == "" {
+			continue
+		}
+
+		keys := strings.Split(rlpTag, "=")
+		require.Equal(t, 2, len(keys))
+
+		// parse keys[1] as an int
+		key, err := strconv.ParseUint(keys[1], 10, 64)
+		require.NoError(t, err)
+
+		// typecast key to be of statusOptionKey type
+		kfi[statusOptionKey(key)] = i
+		ifk[i] = statusOptionKey(key)
+	}
+
+	// Test that the statusOptions' derived kfi length matches the global keyFieldIdx length
+	require.Equal(t, len(keyFieldIdx), len(kfi))
+
+	// Test that each index of the statusOptions' derived kfi values matches the global keyFieldIdx of the same index
+	for k, v := range kfi {
+		require.Equal(t, keyFieldIdx[k], v)
+	}
+
+	// Test that each index of the global keyFieldIdx values matches statusOptions' derived kfi values of the same index
+	for k, v := range keyFieldIdx {
+		require.Equal(t, kfi[k], v)
+	}
+
+	// Test that the statusOptions' derived ifk length matches the global idxFieldKey length
+	require.Equal(t, len(idxFieldKey), len(ifk))
+
+	// Test that each index of the statusOptions' derived ifk values matches the global idxFieldKey of the same index
+	for k, v := range ifk {
+		require.Equal(t, idxFieldKey[k], v)
+	}
+
+	// Test that each index of the global idxFieldKey values matches statusOptions' derived ifk values of the same index
+	for k, v := range idxFieldKey {
+		require.Equal(t, ifk[k], v)
+	}
+}