From a217764ee4a34043bea96f23f75d1c2369fad3f5 Mon Sep 17 00:00:00 2001 From: wizeguyy Date: Wed, 12 Jun 2024 12:03:30 -0500 Subject: [PATCH] fix topic encode/decode --- common/types.go | 4 +- p2p/node/api.go | 4 +- p2p/node/pubsubManager/utils.go | 45 +++++++++++++-------- p2p/node/pubsubManager/utils_test.go | 58 ++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 21 deletions(-) create mode 100644 p2p/node/pubsubManager/utils_test.go diff --git a/common/types.go b/common/types.go index 31b10c0425..320b42fbfa 100644 --- a/common/types.go +++ b/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 { diff --git a/p2p/node/api.go b/p2p/node/api.go index 798082dea9..62a2362149 100644 --- a/p2p/node/api.go +++ b/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, diff --git a/p2p/node/pubsubManager/utils.go b/p2p/node/pubsubManager/utils.go index 366c08c948..fd6336648b 100644 --- a/p2p/node/pubsubManager/utils.go +++ b/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 } diff --git a/p2p/node/pubsubManager/utils_test.go b/p2p/node/pubsubManager/utils_test.go new file mode 100644 index 0000000000..f7150537ec --- /dev/null +++ b/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") + } + } +}