fix topic encode/decode

common/types.go

@@ -557,7 +557,7 @@ func NewLocation(region, zone int) (Location, error) {
 }
 
 func (l *Location) SetRegion(region int) error {
-	if region < 0 || region >= 0xf {
+	if region < 0 || region > 15 {
 		return ErrInvalidLocation
 	}
 	if len(*l) < 1 {
@@ -570,7 +570,7 @@ func (l *Location) SetRegion(region int) error {
 }
 
 func (l *Location) SetZone(zone int) error {
-	if zone < 0 || zone > 0xf {
+	if zone < 0 || zone > 15 {
 		return ErrInvalidLocation
 	}
 	if len(*l) < 2 {

p2p/node/api.go

@@ -256,7 +256,7 @@ func (p *P2PNode) MarkLivelyPeer(peer p2p.PeerID, topic string) {
 		"topic": topic,
 	}).Debug("Recording well-behaving peer")
 
-	t, err := pubsubManager.TopicFromString(p.pubsub.GetGenesis(), topic)
+	t, err := pubsubManager.TopicFromString(topic)
 	if err != nil {
 		log.Global.WithFields(log.Fields{
 			"topic": topic,
@@ -274,7 +274,7 @@ func (p *P2PNode) MarkLatentPeer(peer p2p.PeerID, topic string) {
 		"topic": topic,
 	}).Debug("Recording misbehaving peer")
 
-	t, err := pubsubManager.TopicFromString(p.pubsub.GetGenesis(), topic)
+	t, err := pubsubManager.TopicFromString(topic)
 	if err != nil {
 		log.Global.WithFields(log.Fields{
 			"topic": topic,

p2p/node/pubsubManager/gossipsub.go

@@ -23,7 +23,6 @@ const msgChanSize = 500 // 500 requests per subscription
 
 var (
 	ErrUnsupportedType = errors.New("data type not supported")
-	ErrMalformedTopic  = errors.New("malformed/invalid topic")
 )
 
 type PubsubManager struct {

p2p/node/pubsubManager/utils.go

@@ -94,41 +94,52 @@ func NewTopic(genesis common.Hash, location common.Location, data interface{}) (
 	return t, nil
 }
 
-func TopicFromString(genesis common.Hash, topic string) (*Topic, error) {
+func TopicFromString(topic string) (*Topic, error) {
 	topicParts := strings.Split(topic, "/")
 	if len(topicParts) < 3 {
-		return nil, ErrMalformedTopic
+		return nil, errors.New("topic string should have 3 parts")
+	}
+	if len(topicParts[0]) != 66 {
+		return nil, errors.New("invalid genesis hash")
+	}
+	genHash := common.HexToHash(topicParts[0])
+	if genHash.String() == "0x0000000000000000000000000000000000000000000000000000000000000000" {
+		return nil, errors.New("invalid genesis hash")
 	}
 	var location common.Location
-	locationStr := strings.Split(topicParts[1], ",")
-	if len(locationStr) > 0 {
-		if len(locationStr) >= 1 && locationStr[0] != "" {
-			// Region specified
-			region, err := strconv.Atoi(locationStr[0])
+	if loc := topicParts[1]; loc != "" {
+		locationParts := strings.Split(loc, ",")
+		if len(locationParts) > 2 {
+			return nil, errors.New("invalid location encoding")
+		}
+		if len(locationParts) > 1 {
+			zone, err := strconv.Atoi(locationParts[1])
 			if err != nil {
 				return nil, err
 			}
-			location.SetRegion(region)
+			if err := location.SetZone(zone); err != nil {
+				return nil, err
+			}
 		}
-		if len(locationStr) == 2 && locationStr[1] != "" {
-			// Zone specified
-			zone, err := strconv.Atoi(locationStr[1])
+		if len(locationParts) > 0 {
+			region, err := strconv.Atoi(locationParts[0])
 			if err != nil {
 				return nil, err
 			}
-			location.SetZone(zone)
+			if err := location.SetRegion(region); err != nil {
+				return nil, err
+			}
 		}
 	}
-
 	switch topicParts[2] {
 	case C_headerType:
-		return NewTopic(genesis, location, &types.WorkObjectHeaderView{})
+		return NewTopic(genHash, location, &types.WorkObjectHeaderView{})
 	case C_workObjectType:
-		return NewTopic(genesis, location, &types.WorkObjectBlockView{})
+		return NewTopic(genHash, location, &types.WorkObjectBlockView{})
 	case C_transactionType:
-		return NewTopic(genesis, location, &types.Transactions{})
+		return NewTopic(genHash, location, &types.Transactions{})
 	case C_workObjectHeaderType:
-		return NewTopic(genesis, location, &types.WorkObjectHeader{})
+		return NewTopic(genHash, location, &types.WorkObjectHeader{})
 	default:
 		return nil, ErrUnsupportedType
 	}

p2p/node/pubsubManager/utils_test.go

@@ -0,0 +1,58 @@
+package pubsubManager_test
+
+import (
+	"testing"
+
+	"github.com/dominant-strategies/go-quai/p2p/node/pubsubManager"
+)
+
+func TestTopicFromString(t *testing.T) {
+	type testcase struct {
+		input      string
+		shouldPass bool
+	}
+	cases := []testcase{
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/0,0/blocks", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/0,0/headers", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/1,0/transactions", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/7,0/blocks", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/15,0/woHeaders", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/15,15/woHeaders", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/0,15/woHeaders", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/0,0,0/blocks", false},           // bad location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/0,cinnamon buns/blocks", false}, // bad zone location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/0,a/blocks", false},             // bad zone location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/16,0/blocks", false},            // bad zone location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/16,16/blocks", false},           // bad zone location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/15,16/blocks", false},           // bad zone location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/128,0/blocks", false},           // bad zone location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/-1,0/blocks", false},            // bad zone location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/0,-1/blocks", false},            // bad zone location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/-1000000,0/blocks", false},      // bad zone location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/-1/blocks", false},              // bad region location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/0/blocks", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/1/blocks", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/15/blocks", true},
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/16/blocks", false},                          // bad region location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/PoW vs PoS? Which is right?/blocks", false}, // bad region location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011/10000000/blocks", false},                    // bad region location
+		{"0x0011223344556677889900112233445566778899001122334455667788990011//blocks", true},
+		{"0x001122334455667788990011223344556677889900112233445566778899001/0,0/blocks", false},   // hash too short
+		{"0x00112233445566778899001122334455667788990011223344556677889900111/0,0/blocks", false}, // hash too long
+		{"0xG011223344556677889900112233445566778899001122334455667788990011/0,0/blocks", false},  // invalid hex char
+	}
+	for _, c := range cases {
+		topic, err := pubsubManager.TopicFromString(c.input)
+		if err != nil {
+			if c.shouldPass {
+				t.Errorf("Error building topic for \"%s\": %s", c.input, err.Error())
+			}
+			continue
+		}
+		if toString := topic.String(); toString != c.input {
+			t.Logf("expected %s", c.input)
+			t.Logf("actual   %s", toString)
+			t.Errorf("Encoded string does not match decoded")
+		}
+	}
+}