ui/qt/qwhisper, whisper, xeth: polish topic filter, fix wildcards

7 files changed, 215 insertions, 45 deletions

diff --git a/ui/qt/qwhisper/whisper.go b/ui/qt/qwhisper/whisper.go
index b7409c57f..4ab6d2e5a 100644
--- a/ui/qt/qwhisper/whisper.go
+++ b/ui/qt/qwhisper/whisper.go
@@ -106,7 +106,7 @@ func filterFromMap(opts map[string]interface{}) (f whisper.Filter) {
 	if topicList, ok := opts["topics"].(*qml.List); ok {
 		var topics []string
 		topicList.Convert(&topics)
-		f.Topics = whisper.NewTopicFilterFromStringsFlat(topics...)
+		f.Topics = whisper.NewFilterTopicsFromStringsFlat(topics...)
 	}
 
 	return
diff --git a/whisper/filter.go b/whisper/filter.go
index 8a398ab76..c946d9380 100644
--- a/whisper/filter.go
+++ b/whisper/filter.go
@@ -16,6 +16,66 @@ type Filter struct {
 	Fn     func(msg *Message) // Handler in case of a match
 }
 
+// NewFilterTopics creates a 2D topic array used by whisper.Filter from binary
+// data elements.
+func NewFilterTopics(data ...[][]byte) [][]Topic {
+	filter := make([][]Topic, len(data))
+	for i, condition := range data {
+		// Handle the special case of condition == [[]byte{}]
+		if len(condition) == 1 && len(condition[0]) == 0 {
+			filter[i] = []Topic{}
+			continue
+		}
+		// Otherwise flatten normally
+		filter[i] = NewTopics(condition...)
+	}
+	return filter
+}
+
+// NewFilterTopicsFlat creates a 2D topic array used by whisper.Filter from flat
+// binary data elements.
+func NewFilterTopicsFlat(data ...[]byte) [][]Topic {
+	filter := make([][]Topic, len(data))
+	for i, element := range data {
+		// Only add non-wildcard topics
+		filter[i] = make([]Topic, 0, 1)
+		if len(element) > 0 {
+			filter[i] = append(filter[i], NewTopic(element))
+		}
+	}
+	return filter
+}
+
+// NewFilterTopicsFromStrings creates a 2D topic array used by whisper.Filter
+// from textual data elements.
+func NewFilterTopicsFromStrings(data ...[]string) [][]Topic {
+	filter := make([][]Topic, len(data))
+	for i, condition := range data {
+		// Handle the special case of condition == [""]
+		if len(condition) == 1 && condition[0] == "" {
+			filter[i] = []Topic{}
+			continue
+		}
+		// Otherwise flatten normally
+		filter[i] = NewTopicsFromStrings(condition...)
+	}
+	return filter
+}
+
+// NewFilterTopicsFromStringsFlat creates a 2D topic array used by whisper.Filter from flat
+// binary data elements.
+func NewFilterTopicsFromStringsFlat(data ...string) [][]Topic {
+	filter := make([][]Topic, len(data))
+	for i, element := range data {
+		// Only add non-wildcard topics
+		filter[i] = make([]Topic, 0, 1)
+		if element != "" {
+			filter[i] = append(filter[i], NewTopicFromString(element))
+		}
+	}
+	return filter
+}
+
 // filterer is the internal, fully initialized filter ready to match inbound
 // messages to a variety of criteria.
 type filterer struct {
diff --git a/whisper/filter_test.go b/whisper/filter_test.go
new file mode 100644
index 000000000..ac0ebaba7
--- /dev/null
+++ b/whisper/filter_test.go
@@ -0,0 +1,149 @@
+package whisper
+
+import (
+	"bytes"
+
+	"testing"
+)
+
+var filterTopicsCreationTests = []struct {
+	topics [][]string
+	filter [][][4]byte
+}{
+	{ // Simple topic filter
+		topics: [][]string{
+			{"abc", "def", "ghi"},
+			{"def"},
+			{"ghi", "abc"},
+		},
+		filter: [][][4]byte{
+			{{0x4e, 0x03, 0x65, 0x7a}, {0x34, 0x60, 0x7c, 0x9b}, {0x21, 0x41, 0x7d, 0xf9}},
+			{{0x34, 0x60, 0x7c, 0x9b}},
+			{{0x21, 0x41, 0x7d, 0xf9}, {0x4e, 0x03, 0x65, 0x7a}},
+		},
+	},
+	{ // Wild-carded topic filter
+		topics: [][]string{
+			{"abc", "def", "ghi"},
+			{},
+			{""},
+			{"def"},
+		},
+		filter: [][][4]byte{
+			{{0x4e, 0x03, 0x65, 0x7a}, {0x34, 0x60, 0x7c, 0x9b}, {0x21, 0x41, 0x7d, 0xf9}},
+			{},
+			{},
+			{{0x34, 0x60, 0x7c, 0x9b}},
+		},
+	},
+}
+
+var filterTopicsCreationFlatTests = []struct {
+	topics []string
+	filter [][][4]byte
+}{
+	{ // Simple topic list
+		topics: []string{"abc", "def", "ghi"},
+		filter: [][][4]byte{
+			{{0x4e, 0x03, 0x65, 0x7a}},
+			{{0x34, 0x60, 0x7c, 0x9b}},
+			{{0x21, 0x41, 0x7d, 0xf9}},
+		},
+	},
+	{ // Wild-carded topic list
+		topics: []string{"abc", "", "ghi"},
+		filter: [][][4]byte{
+			{{0x4e, 0x03, 0x65, 0x7a}},
+			{},
+			{{0x21, 0x41, 0x7d, 0xf9}},
+		},
+	},
+}
+
+func TestFilterTopicsCreation(t *testing.T) {
+	// Check full filter creation
+	for i, tt := range filterTopicsCreationTests {
+		// Check the textual creation
+		filter := NewFilterTopicsFromStrings(tt.topics...)
+		if len(filter) != len(tt.topics) {
+			t.Errorf("test %d: condition count mismatch: have %v, want %v", i, len(filter), len(tt.topics))
+			continue
+		}
+		for j, condition := range filter {
+			if len(condition) != len(tt.filter[j]) {
+				t.Errorf("test %d, condition %d: size mismatch: have %v, want %v", i, j, len(condition), len(tt.filter[j]))
+				continue
+			}
+			for k := 0; k < len(condition); k++ {
+				if bytes.Compare(condition[k][:], tt.filter[j][k][:]) != 0 {
+					t.Errorf("test %d, condition %d, segment %d: filter mismatch: have 0x%x, want 0x%x", i, j, k, condition[k], tt.filter[j][k])
+				}
+			}
+		}
+		// Check the binary creation
+		binary := make([][][]byte, len(tt.topics))
+		for j, condition := range tt.topics {
+			binary[j] = make([][]byte, len(condition))
+			for k, segment := range condition {
+				binary[j][k] = []byte(segment)
+			}
+		}
+		filter = NewFilterTopics(binary...)
+		if len(filter) != len(tt.topics) {
+			t.Errorf("test %d: condition count mismatch: have %v, want %v", i, len(filter), len(tt.topics))
+			continue
+		}
+		for j, condition := range filter {
+			if len(condition) != len(tt.filter[j]) {
+				t.Errorf("test %d, condition %d: size mismatch: have %v, want %v", i, j, len(condition), len(tt.filter[j]))
+				continue
+			}
+			for k := 0; k < len(condition); k++ {
+				if bytes.Compare(condition[k][:], tt.filter[j][k][:]) != 0 {
+					t.Errorf("test %d, condition %d, segment %d: filter mismatch: have 0x%x, want 0x%x", i, j, k, condition[k], tt.filter[j][k])
+				}
+			}
+		}
+	}
+	// Check flat filter creation
+	for i, tt := range filterTopicsCreationFlatTests {
+		// Check the textual creation
+		filter := NewFilterTopicsFromStringsFlat(tt.topics...)
+		if len(filter) != len(tt.topics) {
+			t.Errorf("test %d: condition count mismatch: have %v, want %v", i, len(filter), len(tt.topics))
+			continue
+		}
+		for j, condition := range filter {
+			if len(condition) != len(tt.filter[j]) {
+				t.Errorf("test %d, condition %d: size mismatch: have %v, want %v", i, j, len(condition), len(tt.filter[j]))
+				continue
+			}
+			for k := 0; k < len(condition); k++ {
+				if bytes.Compare(condition[k][:], tt.filter[j][k][:]) != 0 {
+					t.Errorf("test %d, condition %d, segment %d: filter mismatch: have 0x%x, want 0x%x", i, j, k, condition[k], tt.filter[j][k])
+				}
+			}
+		}
+		// Check the binary creation
+		binary := make([][]byte, len(tt.topics))
+		for j, topic := range tt.topics {
+			binary[j] = []byte(topic)
+		}
+		filter = NewFilterTopicsFlat(binary...)
+		if len(filter) != len(tt.topics) {
+			t.Errorf("test %d: condition count mismatch: have %v, want %v", i, len(filter), len(tt.topics))
+			continue
+		}
+		for j, condition := range filter {
+			if len(condition) != len(tt.filter[j]) {
+				t.Errorf("test %d, condition %d: size mismatch: have %v, want %v", i, j, len(condition), len(tt.filter[j]))
+				continue
+			}
+			for k := 0; k < len(condition); k++ {
+				if bytes.Compare(condition[k][:], tt.filter[j][k][:]) != 0 {
+					t.Errorf("test %d, condition %d, segment %d: filter mismatch: have 0x%x, want 0x%x", i, j, k, condition[k], tt.filter[j][k])
+				}
+			}
+		}
+	}
+}
diff --git a/whisper/topic.go b/whisper/topic.go
index b2a264e29..c47c94ae1 100644
--- a/whisper/topic.go
+++ b/whisper/topic.go
@@ -11,6 +11,8 @@ import "github.com/ethereum/go-ethereum/crypto"
 type Topic [4]byte
 
 // NewTopic creates a topic from the 4 byte prefix of the SHA3 hash of the data.
+//
+// Note, empty topics are considered the wildcard, and cannot be used in messages.
 func NewTopic(data []byte) Topic {
 	prefix := [4]byte{}
 	copy(prefix[:], crypto.Sha3(data)[:4])
@@ -27,26 +29,6 @@ func NewTopics(data ...[]byte) []Topic {
 	return topics
 }
 
-// NewTopicFilter creates a 2D topic array used by whisper.Filter from binary
-// data elements.
-func NewTopicFilter(data ...[][]byte) [][]Topic {
-	filter := make([][]Topic, len(data))
-	for i, condition := range data {
-		filter[i] = NewTopics(condition...)
-	}
-	return filter
-}
-
-// NewTopicFilterFlat creates a 2D topic array used by whisper.Filter from flat
-// binary data elements.
-func NewTopicFilterFlat(data ...[]byte) [][]Topic {
-	filter := make([][]Topic, len(data))
-	for i, element := range data {
-		filter[i] = []Topic{NewTopic(element)}
-	}
-	return filter
-}
-
 // NewTopicFromString creates a topic using the binary data contents of the
 // specified string.
 func NewTopicFromString(data string) Topic {
@@ -63,26 +45,6 @@ func NewTopicsFromStrings(data ...string) []Topic {
 	return topics
 }
 
-// NewTopicFilterFromStrings creates a 2D topic array used by whisper.Filter
-// from textual data elements.
-func NewTopicFilterFromStrings(data ...[]string) [][]Topic {
-	filter := make([][]Topic, len(data))
-	for i, condition := range data {
-		filter[i] = NewTopicsFromStrings(condition...)
-	}
-	return filter
-}
-
-// NewTopicFilterFromStringsFlat creates a 2D topic array used by whisper.Filter from flat
-// binary data elements.
-func NewTopicFilterFromStringsFlat(data ...string) [][]Topic {
-	filter := make([][]Topic, len(data))
-	for i, element := range data {
-		filter[i] = []Topic{NewTopicFromString(element)}
-	}
-	return filter
-}
-
 // String converts a topic byte array to a string representation.
 func (self *Topic) String() string {
 	return string(self[:])
diff --git a/whisper/topic_test.go b/whisper/topic_test.go
index 22ee06096..976f3e88d 100644
--- a/whisper/topic_test.go
+++ b/whisper/topic_test.go
@@ -9,9 +9,8 @@ var topicCreationTests = []struct {
 	data []byte
 	hash [4]byte
 }{
-	{hash: [4]byte{0xc5, 0xd2, 0x46, 0x01}, data: nil},
-	{hash: [4]byte{0xc5, 0xd2, 0x46, 0x01}, data: []byte{}},
 	{hash: [4]byte{0x8f, 0x9a, 0x2b, 0x7d}, data: []byte("test name")},
+	{hash: [4]byte{0xf2, 0x6e, 0x77, 0x79}, data: []byte("some other test")},
 }
 
 func TestTopicCreation(t *testing.T) {
diff --git a/whisper/whisper_test.go b/whisper/whisper_test.go
index 8fce0e036..7c5067f51 100644
--- a/whisper/whisper_test.go
+++ b/whisper/whisper_test.go
@@ -129,7 +129,7 @@ func testBroadcast(anonymous bool, t *testing.T) {
 		dones[i] = done
 
 		targets[i].Watch(Filter{
-			Topics: NewTopicFilterFromStrings([]string{"broadcast topic"}),
+			Topics: NewFilterTopicsFromStringsFlat("broadcast topic"),
 			Fn: func(msg *Message) {
 				close(done)
 			},
diff --git a/xeth/whisper.go b/xeth/whisper.go
index 25c4af3b1..36c6ca63f 100644
--- a/xeth/whisper.go
+++ b/xeth/whisper.go
@@ -71,7 +71,7 @@ func (self *Whisper) Watch(to, from string, topics [][]string, fn func(WhisperMe
 	filter := whisper.Filter{
 		To:     crypto.ToECDSAPub(common.FromHex(to)),
 		From:   crypto.ToECDSAPub(common.FromHex(from)),
-		Topics: whisper.NewTopicFilterFromStrings(topics...),
+		Topics: whisper.NewFilterTopicsFromStrings(topics...),
 	}
 	filter.Fn = func(message *whisper.Message) {
 		fn(NewWhisperMessage(message))