Added handling for mixed RLP key data types

This the initial implementation, a number of known test fails have been introduced with this commit and will be resolved as a matter of priority.

waku/handshake.go

@@ -6,12 +6,26 @@ import (
 	"io"
 	"math"
 	"reflect"
+	"strconv"
 
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
 type statusOptionKey uint
 
+type statusOptionKeyType uint
+
+type statusOptionKeyToType struct {
+	Idx  int
+	Key  statusOptionKey
+	Type statusOptionKeyType
+}
+
+const (
+	sOKTS statusOptionKeyType = iota // Status Option Key Type String
+	sOKTU                            // Status Option Key Type Uint
+)
+
 var (
 	defaultMinPoW = math.Float64bits(0.001)
 	idxFieldKey   = map[int]statusOptionKey{
@@ -32,6 +46,11 @@ var (
 	}
 )
 
+type keyTypeMapping struct {
+	idxFieldKey map[int]*statusOptionKeyToType
+	keyFieldIdx map[statusOptionKey]*statusOptionKeyToType
+}
+
 // statusOptions defines additional information shared between peers
 // during the handshake.
 // There might be more options provided then fields in statusOptions
@@ -45,6 +64,7 @@ type statusOptions struct {
 	ConfirmationsEnabled *bool       `rlp:"key=3"`
 	RateLimits           *RateLimits `rlp:"key=4"`
 	TopicInterest        []TopicType `rlp:"key=5"`
+	keyTypeMapping       keyTypeMapping
 }
 
 // WithDefaults adds the default values for a given peer.
@@ -94,15 +114,41 @@ func (o statusOptions) EncodeRLP(w io.Writer) error {
 	var optionsList []interface{}
 	for i := 0; i < v.NumField(); i++ {
 		field := v.Field(i)
-		if !field.IsNil() {
+
+		// skip unexported fields
+		if !field.CanInterface() {
+			continue
+		}
+
+		if field.IsNil() {
+			continue
+		}
+
 		value := field.Interface()
 		key, ok := idxFieldKey[i]
 		if !ok {
 			continue
 		}
-			if value != nil {
-				optionsList = append(optionsList, []interface{}{key, value})
+
+		var val []interface{}
+
+		if o.keyTypeMapping.idxFieldKey == nil {
+			val = append(val, key, value)
 		}
+
+		k, ok := o.keyTypeMapping.idxFieldKey[i]
+		if !ok {
+			val = append(val, key, value)
+		} else {
+			ke, err := k.encode()
+			if err != nil {
+				return fmt.Errorf("key encoding fail: %v", err)
+			}
+			val = append(val, ke, value)
+		}
+
+		if value != nil {
+			optionsList = append(optionsList, val)
 		}
 	}
 	return rlp.Encode(w, optionsList)
@@ -127,34 +173,30 @@ loop:
 			return fmt.Errorf("expected an inner list: %v", err)
 		}
 
-		var key statusOptionKey
-		if err := s.Decode(&key); err != nil {
+		ktt := statusOptionKeyToType{}
+		if err = ktt.decodeStream(s); 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 := keyFieldIdx[key]
+		idx, ok := keyFieldIdx[ktt.Key]
 		if !ok {
 			// Read the rest of the list items and dump them.
 			_, err := s.Raw()
 			if err != nil {
-				return fmt.Errorf("failed to read the value of key %d: %v", key, err)
+				return fmt.Errorf("failed to read the value of key %d: %v", ktt.Key, err)
 			}
 			continue
 		}
+
+		ktt.Idx = idx
+		o.addKeyToType(&ktt)
+
 		if err := s.Decode(v.Elem().Field(idx).Addr().Interface()); err != nil {
-			return fmt.Errorf("failed to decode an option %d: %v", key, err)
+			return fmt.Errorf("failed to decode an option %d: %v", ktt.Key, err)
 		}
 		if err := s.ListEnd(); err != nil {
 			return err
@@ -164,6 +206,58 @@ loop:
 	return s.ListEnd()
 }
 
+func (o *statusOptions) addKeyToType(ktt *statusOptionKeyToType) {
+
+	if o.keyTypeMapping.idxFieldKey == nil {
+		o.keyTypeMapping.idxFieldKey = make(map[int]*statusOptionKeyToType)
+	}
+
+	if o.keyTypeMapping.keyFieldIdx == nil {
+		o.keyTypeMapping.keyFieldIdx = make(map[statusOptionKey]*statusOptionKeyToType)
+	}
+
+	o.keyTypeMapping.idxFieldKey[ktt.Idx] = ktt
+	o.keyTypeMapping.keyFieldIdx[ktt.Key] = ktt
+}
+
+func (k statusOptionKeyToType) decodeStream(s *rlp.Stream) error {
+	var key statusOptionKey
+
+	// If uint can be decoded return it
+	if err := s.Decode(&key); err == nil {
+		k.Key = key
+		k.Type = sOKTU
+		return nil
+	}
+
+	// Attempt decoding into a string
+	var sKey string
+	if err := s.Decode(&sKey); err != nil {
+		return err
+	}
+
+	// Parse string into uint
+	uKey, err := strconv.ParseUint(sKey, 10, 64)
+	if err != nil {
+		return err
+	}
+
+	k.Key = statusOptionKey(uKey)
+	k.Type = sOKTS
+	return nil
+}
+
+func (k statusOptionKeyToType) encode() (interface{}, error) {
+	switch k.Type {
+	case sOKTU:
+		return k.Key, nil
+	case sOKTS:
+		return fmt.Sprint(k.Key), nil
+	default:
+		return nil, fmt.Errorf("failed to encode key '%d', unknown key type '%d'", k.Key, k.Type)
+	}
+}
+
 func (o statusOptions) Validate() error {
 	if len(o.TopicInterest) > 1000 {
 		return errors.New("topic interest is limited by 1000 items")

waku/handshake_test.go

@@ -2,10 +2,10 @@ package waku
 
 import (
 	"math"
-	"testing"
 	"reflect"
 	"strconv"
 	"strings"
+	"testing"
 
 	"github.com/stretchr/testify/require"
 
@@ -103,12 +103,12 @@ func TestStatusOptionKeys(t *testing.T) {
 
 	// 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)
+		require.Exactly(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)
+		require.Exactly(t, kfi[k], v)
 	}
 
 	// Test that the statusOptions' derived ifk length matches the global idxFieldKey length
@@ -116,11 +116,11 @@ func TestStatusOptionKeys(t *testing.T) {
 
 	// 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)
+		require.Exactly(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)
+		require.Exactly(t, ifk[k], v)
 	}
 }