GetPublicKey: Remove unneccesary public key verification. Simplify select.

records.go

@@ -40,12 +40,20 @@ func (dht *IpfsDHT) GetPublicKey(ctx context.Context, p peer.ID) (ci.PubKey, err
 
 	// Try getting the public key both directly from the node it identifies
 	// and from the DHT, in parallel
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
 	resp := make(chan pubkrs, 2)
 	go func() {
 		pubk, err := dht.getPublicKeyFromNode(ctx, p)
 		resp <- pubkrs{pubk, err}
 	}()
 
+	// Note that the number of open connections is capped by the dial
+	// limiter, so there is a chance that getPublicKeyFromDHT(), which
+	// potentially opens a lot of connections, will block
+	// getPublicKeyFromNode() from getting a connection.
+	// Currently this doesn't seem to cause an issue so leaving as is
+	// for now.
 	go func() {
 		pubk, err := dht.getPublicKeyFromDHT(ctx, p)
 		resp <- pubkrs{pubk, err}
@@ -55,20 +63,16 @@ func (dht *IpfsDHT) GetPublicKey(ctx context.Context, p peer.ID) (ci.PubKey, err
 	// a public key (or for both to error out)
 	var err error
 	for i := 0; i < 2; i++ {
-		select {
-		case <-ctx.Done():
-			return nil, ctx.Err()
-		case r := <-resp:
-			if r.err == nil {
-				// Found the public key
-				err := dht.peerstore.AddPubKey(p, r.pubk)
-				if err != nil {
-					log.Warningf("Failed to add public key to peerstore for %v", p)
-				}
-				return r.pubk, nil
+		r := <-resp
+		if r.err == nil {
+			// Found the public key
+			err := dht.peerstore.AddPubKey(p, r.pubk)
+			if err != nil {
+				log.Warningf("Failed to add public key to peerstore for %v", p)
 			}
-			err = r.err
+			return r.pubk, nil
 		}
+		err = r.err
 	}
 
 	// Both go routines failed to find a public key
@@ -89,14 +93,16 @@ func (dht *IpfsDHT) getPublicKeyFromDHT(ctx context.Context, p peer.ID) (ci.PubK
 		return nil, routing.ErrNotFound
 	}
 
-	_, err = dht.Selector.BestRecord(pkkey, [][]byte{vals[0].Val})
+	pubk, err := ci.UnmarshalPublicKey(vals[0].Val)
 	if err != nil {
-		log.Errorf("Failed to verify public key for %v retrieved from DHT: %s", p, err)
+		log.Errorf("Could not unmarshall public key retrieved from DHT for %v", p)
 		return nil, err
 	}
 
-	log.Debugf("DHT got public key for %s from DHT", p)
-	return dht.pubkFromVal(p, vals[0].Val)
+	// Note: No need to check that public key hash matches peer ID
+	// because this is done by GetValues()
+	log.Debugf("Got public key for %s from DHT", p)
+	return pubk, nil
 }
 
 func (dht *IpfsDHT) getPublicKeyFromNode(ctx context.Context, p peer.ID) (ci.PubKey, error) {
@@ -116,30 +122,25 @@ func (dht *IpfsDHT) getPublicKeyFromNode(ctx context.Context, p peer.ID) (ci.Pub
 	// node doesn't have key :(
 	record := pmes.GetRecord()
 	if record == nil {
-		return nil, fmt.Errorf("Node %v not responding with its public key", p)
+		return nil, fmt.Errorf("node %v not responding with its public key", p)
 	}
 
-	val := record.GetValue()
-	log.Debugf("DHT got public key from node %v itself", p)
-	return dht.pubkFromVal(p, val)
-}
-
-func (dht *IpfsDHT) pubkFromVal(p peer.ID, val []byte) (ci.PubKey, error) {
-	pubk, err := ci.UnmarshalPublicKey(val)
+	pubk, err := ci.UnmarshalPublicKey(record.GetValue())
 	if err != nil {
-		log.Errorf("DHT could not unmarshall public key for %v", p)
+		log.Errorf("Could not unmarshall public key for %v", p)
 		return nil, err
 	}
 
 	// Make sure the public key matches the peer ID
 	id, err := peer.IDFromPublicKey(pubk)
 	if err != nil {
-		log.Errorf("DHT could not extract peer id from public key for %v", p)
+		log.Errorf("Could not extract peer id from public key for %v", p)
 		return nil, err
 	}
 	if id != p {
 		return nil, fmt.Errorf("public key %v does not match peer %v", id, p)
 	}
 
+	log.Debugf("Got public key from node %v itself", p)
 	return pubk, nil
 }