feat(requestid): use uuids for requestids

Ref: #278
Closes: #279
Closes: #281

Author: rvagg

go.mod

@@ -3,6 +3,7 @@ module github.com/ipfs/go-graphsync
 go 1.16
 
 require (
+	github.com/google/uuid v1.3.0
 	github.com/hannahhoward/cbor-gen-for v0.0.0-20200817222906-ea96cece81f1
 	github.com/hannahhoward/go-pubsub v0.0.0-20200423002714-8d62886cc36e
 	github.com/ipfs/go-block-format v0.0.3

graphsync.go

@@ -5,18 +5,29 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/google/uuid"
 	"github.com/ipfs/go-cid"
 	"github.com/ipld/go-ipld-prime"
 	"github.com/ipld/go-ipld-prime/traversal"
 	"github.com/libp2p/go-libp2p-core/peer"
 )
 
 // RequestID is a unique identifier for a GraphSync request.
-type RequestID int32
+type RequestID uuid.UUID
 
 // Tag returns an easy way to identify this request id as a graphsync request (for libp2p connections)
 func (r RequestID) Tag() string {
-	return fmt.Sprintf("graphsync-request-%d", r)
+	return r.String()
+}
+
+// String form of a RequestID (should be a well-formed UUIDv4 string)
+func (r RequestID) String() string {
+	return uuid.UUID(r).String()
+}
+
+// Create a new, random RequestID (should be a UUIDv4)
+func NewRequestID() RequestID {
+	return RequestID(uuid.New())
 }
 
 // Priority a priority for a GraphSync request.

impl/graphsync_test.go

@@ -8,7 +8,6 @@ import (
 	"io"
 	"io/ioutil"
 	"math"
-	"math/rand"
 	"os"
 	"path/filepath"
 	"testing"
@@ -133,7 +132,7 @@ func TestSendResponseToIncomingRequest(t *testing.T) {
 	blockChainLength := 100
 	blockChain := testutil.SetupBlockChain(ctx, t, td.persistence2, 100, blockChainLength)
 
-	requestID := graphsync.RequestID(rand.Int31())
+	requestID := graphsync.NewRequestID()
 
 	builder := gsmsg.NewBuilder()
 	builder.AddRequest(gsmsg.NewRequest(requestID, blockChain.TipLink.(cidlink.Link).Cid, blockChain.Selector(), graphsync.Priority(math.MaxInt32), td.extension))

linktracker/linktracker_test.go

@@ -1,7 +1,6 @@
 package linktracker
 
 import (
-	"math/rand"
 	"testing"
 
 	"github.com/ipld/go-ipld-prime"
@@ -74,7 +73,7 @@ func TestBlockRefCount(t *testing.T) {
 			linkTracker := New()
 			link := testutil.NewTestLink()
 			for _, rq := range data.requests {
-				requestID := graphsync.RequestID(rand.Int31())
+				requestID := graphsync.NewRequestID()
 				for _, present := range rq.traversals {
 					linkTracker.RecordLinkTraversal(requestID, link, present)
 				}
@@ -116,7 +115,7 @@ func TestFinishRequest(t *testing.T) {
 	for testCase, data := range testCases {
 		t.Run(testCase, func(t *testing.T) {
 			linkTracker := New()
-			requestID := graphsync.RequestID(rand.Int31())
+			requestID := graphsync.NewRequestID()
 			for _, lt := range data.linksTraversed {
 				linkTracker.RecordLinkTraversal(requestID, lt.link, lt.blockPresent)
 			}
@@ -151,7 +150,7 @@ func TestIsKnownMissingLink(t *testing.T) {
 		t.Run(testCase, func(t *testing.T) {
 			linkTracker := New()
 			link := testutil.NewTestLink()
-			requestID := graphsync.RequestID(rand.Int31())
+			requestID := graphsync.NewRequestID()
 			for _, present := range data.traversals {
 				linkTracker.RecordLinkTraversal(requestID, link, present)
 			}

message/builder_test.go

@@ -2,7 +2,6 @@ package message
 
 import (
 	"io"
-	"math/rand"
 	"testing"
 
 	"github.com/ipld/go-ipld-prime"
@@ -20,6 +19,7 @@ func TestMessageBuilding(t *testing.T) {
 	for _, block := range blocks {
 		links = append(links, cidlink.Link{Cid: block.Cid()})
 	}
+
 	extensionData1 := testutil.RandomBytes(100)
 	extensionName1 := graphsync.ExtensionName("AppleSauce/McGee")
 	extension1 := graphsync.ExtensionData{
@@ -32,10 +32,10 @@ func TestMessageBuilding(t *testing.T) {
 		Name: extensionName2,
 		Data: extensionData2,
 	}
-	requestID1 := graphsync.RequestID(rand.Int31())
-	requestID2 := graphsync.RequestID(rand.Int31())
-	requestID3 := graphsync.RequestID(rand.Int31())
-	requestID4 := graphsync.RequestID(rand.Int31())
+	requestID1 := graphsync.NewRequestID()
+	requestID2 := graphsync.NewRequestID()
+	requestID3 := graphsync.NewRequestID()
+	requestID4 := graphsync.NewRequestID()
 	closer := io.NopCloser(nil)
 	testCases := map[string]struct {
 		build           func(*Builder)

message/message.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/google/uuid"
 	blocks "github.com/ipfs/go-block-format"
 	cid "github.com/ipfs/go-cid"
 	"github.com/ipld/go-ipld-prime"
@@ -162,7 +163,12 @@ func newMessageFromProto(pbm *pb.Message) (GraphSyncMessage, error) {
 		if exts == nil {
 			exts = make(map[string][]byte)
 		}
-		requests[graphsync.RequestID(req.Id)] = newRequest(graphsync.RequestID(req.Id), root, selector, graphsync.Priority(req.Priority), req.Cancel, req.Update, exts)
+		uid, err := uuid.FromBytes(req.Id)
+		if err != nil {
+			return GraphSyncMessage{}, err
+		}
+		id := graphsync.RequestID(uid)
+		requests[id] = newRequest(id, root, selector, graphsync.Priority(req.Priority), req.Cancel, req.Update, exts)
 	}
 
 	responses := make(map[graphsync.RequestID]GraphSyncResponse, len(pbm.GetResponses()))
@@ -174,7 +180,12 @@ func newMessageFromProto(pbm *pb.Message) (GraphSyncMessage, error) {
 		if exts == nil {
 			exts = make(map[string][]byte)
 		}
-		responses[graphsync.RequestID(res.Id)] = newResponse(graphsync.RequestID(res.Id), graphsync.ResponseStatusCode(res.Status), exts)
+		uid, err := uuid.FromBytes(res.Id)
+		if err != nil {
+			return GraphSyncMessage{}, err
+		}
+		id := graphsync.RequestID(uid)
+		responses[id] = newResponse(id, graphsync.ResponseStatusCode(res.Status), exts)
 	}
 
 	blks := make(map[cid.Cid]blocks.Block, len(pbm.GetData()))
@@ -278,7 +289,7 @@ func (gsm GraphSyncMessage) ToProto() (*pb.Message, error) {
 			}
 		}
 		pbm.Requests = append(pbm.Requests, &pb.Message_Request{
-			Id:         int32(request.id),
+			Id:         request.id[:],
 			Root:       request.root.Bytes(),
 			Selector:   selector,
 			Priority:   int32(request.priority),
@@ -291,7 +302,7 @@ func (gsm GraphSyncMessage) ToProto() (*pb.Message, error) {
 	pbm.Responses = make([]*pb.Message_Response, 0, len(gsm.responses))
 	for _, response := range gsm.responses {
 		pbm.Responses = append(pbm.Responses, &pb.Message_Response{
-			Id:         int32(response.requestID),
+			Id:         response.requestID[:],
 			Status:     int32(response.status),
 			Extensions: response.extensions,
 		})

message/message_test.go

@@ -26,7 +26,7 @@ func TestAppendingRequests(t *testing.T) {
 	root := testutil.GenerateCids(1)[0]
 	ssb := builder.NewSelectorSpecBuilder(basicnode.Prototype.Any)
 	selector := ssb.Matcher().Node()
-	id := graphsync.RequestID(rand.Int31())
+	id := graphsync.NewRequestID()
 	priority := graphsync.Priority(rand.Int31())
 
 	builder := NewBuilder()
@@ -51,7 +51,7 @@ func TestAppendingRequests(t *testing.T) {
 	require.NoError(t, err)
 
 	pbRequest := pbMessage.Requests[0]
-	require.Equal(t, int32(id), pbRequest.Id)
+	require.Equal(t, id[:], pbRequest.Id)
 	require.Equal(t, int32(priority), pbRequest.Priority)
 	require.False(t, pbRequest.Cancel)
 	require.False(t, pbRequest.Update)
@@ -82,7 +82,7 @@ func TestAppendingResponses(t *testing.T) {
 		Name: extensionName,
 		Data: testutil.RandomBytes(100),
 	}
-	requestID := graphsync.RequestID(rand.Int31())
+	requestID := graphsync.NewRequestID()
 	status := graphsync.RequestAcknowledged
 
 	builder := NewBuilder()
@@ -102,7 +102,7 @@ func TestAppendingResponses(t *testing.T) {
 	pbMessage, err := gsm.ToProto()
 	require.NoError(t, err, "serialize to protobuf errored")
 	pbResponse := pbMessage.Responses[0]
-	require.Equal(t, int32(requestID), pbResponse.Id)
+	require.Equal(t, requestID[:], pbResponse.Id)
 	require.Equal(t, int32(status), pbResponse.Status)
 	require.Equal(t, extension.Data, pbResponse.Extensions["graphsync/awesome"])
 
@@ -154,7 +154,7 @@ func contains(strs []string, x string) bool {
 func TestRequestCancel(t *testing.T) {
 	ssb := builder.NewSelectorSpecBuilder(basicnode.Prototype.Any)
 	selector := ssb.Matcher().Node()
-	id := graphsync.RequestID(rand.Int31())
+	id := graphsync.NewRequestID()
 	priority := graphsync.Priority(rand.Int31())
 	root := testutil.GenerateCids(1)[0]
 
@@ -184,7 +184,7 @@ func TestRequestCancel(t *testing.T) {
 
 func TestRequestUpdate(t *testing.T) {
 
-	id := graphsync.RequestID(rand.Int31())
+	id := graphsync.NewRequestID()
 	extensionName := graphsync.ExtensionName("graphsync/awesome")
 	extension := graphsync.ExtensionData{
 		Name: extensionName,
@@ -235,7 +235,7 @@ func TestToNetFromNetEquivalency(t *testing.T) {
 		Name: extensionName,
 		Data: testutil.RandomBytes(100),
 	}
-	id := graphsync.RequestID(rand.Int31())
+	id := graphsync.NewRequestID()
 	priority := graphsync.Priority(rand.Int31())
 	status := graphsync.RequestAcknowledged
 
@@ -325,7 +325,7 @@ func TestMergeExtensions(t *testing.T) {
 	root := testutil.GenerateCids(1)[0]
 	ssb := builder.NewSelectorSpecBuilder(basicnode.Prototype.Any)
 	selector := ssb.Matcher().Node()
-	id := graphsync.RequestID(rand.Int31())
+	id := graphsync.NewRequestID()
 	priority := graphsync.Priority(rand.Int31())
 	defaultRequest := NewRequest(id, root, selector, priority, initialExtensions...)
 	t.Run("when merging into empty", func(t *testing.T) {

message/pb/message.pb.go

@@ -7,7 +7,7 @@ option go_package = ".;graphsync_message_pb";
 message Message {
 
   message Request {
-    int32 id = 1;       // unique id set on the requester side
+    bytes id = 1;       // unique id set on the requester side
     bytes root = 2;     // a CID for the root node in the query
     bytes selector = 3; // ipld selector to retrieve
     map<string, bytes> extensions = 4;    // aux information. useful for other protocols
@@ -17,7 +17,7 @@ message Message {
   }
 
   message Response {
-    int32 id = 1;     // the request id
+    bytes id = 1;     // the request id
     int32 status = 2; // a status code.
     map<string, bytes> extensions = 3; // additional data
   }