refactor: cleanup some names and types

namesys/ipns_publisher.go

@@ -2,6 +2,7 @@ package namesys
 
 import (
 	"context"
+	"errors"
 	"strings"
 	"sync"
 	"time"
@@ -50,7 +51,7 @@ func (p *IPNSPublisher) Publish(ctx context.Context, priv crypto.PrivKey, value
 		return err
 	}
 
-	return PublishRecord(ctx, p.routing, priv.GetPublic(), record)
+	return PublishIPNSRecord(ctx, p.routing, priv.GetPublic(), record)
 }
 
 // IpnsDsKey returns a datastore key given an IPNS identifier (peer
@@ -155,9 +156,9 @@ func (p *IPNSPublisher) updateRecord(ctx context.Context, k crypto.PrivKey, valu
 		return nil, err
 	}
 
-	seqno := uint64(0)
+	seq := uint64(0)
 	if rec != nil {
-		seqno, err = rec.Sequence()
+		seq, err = rec.Sequence()
 		if err != nil {
 			return nil, err
 		}
@@ -169,14 +170,14 @@ func (p *IPNSPublisher) updateRecord(ctx context.Context, k crypto.PrivKey, valu
 		if value != path.Path(p.String()) {
 			// Don't bother incrementing the sequence number unless the
 			// value changes.
-			seqno++
+			seq++
 		}
 	}
 
 	opts := ProcessPublishOptions(options)
 
 	// Create record
-	r, err := ipns.NewRecord(k, value, seqno, opts.EOL, opts.TTL, ipns.WithV1Compatibility(opts.CompatibleWithV1))
+	r, err := ipns.NewRecord(k, value, seq, opts.EOL, opts.TTL, ipns.WithV1Compatibility(opts.CompatibleWithV1))
 	if err != nil {
 		return nil, err
 	}
@@ -194,13 +195,14 @@ func (p *IPNSPublisher) updateRecord(ctx context.Context, k crypto.PrivKey, valu
 	if err := p.ds.Sync(ctx, key); err != nil {
 		return nil, err
 	}
+
 	return r, nil
 }
 
-// PublishRecord publishes the given entry using the provided ValueStore,
-// keyed on the ID associated with the provided public key. The public key is
-// also made available to the routing system so that entries can be verified.
-func PublishRecord(ctx context.Context, r routing.ValueStore, k crypto.PubKey, rec *ipns.Record) error {
+// PublishIPNSRecord publishes the given [ipns.Record] for the provided [crypto.PubKey] in
+// the provided [routing.ValueStore]. The public key is also made available to the routing
+// system if it cannot be derived from the corresponding [peer.ID].
+func PublishIPNSRecord(ctx context.Context, r routing.ValueStore, pubKey crypto.PubKey, rec *ipns.Record) error {
 	ctx, span := startSpan(ctx, "PutRecordToRouting")
 	defer span.End()
 
@@ -209,25 +211,22 @@ func PublishRecord(ctx context.Context, r routing.ValueStore, k crypto.PubKey, r
 
 	errs := make(chan error, 2) // At most two errors (IPNS, and public key)
 
-	id, err := peer.IDFromPublicKey(k)
+	pid, err := peer.IDFromPublicKey(pubKey)
 	if err != nil {
 		return err
 	}
 
 	go func() {
-		errs <- PublishIPNSRecord(ctx, r, ipns.NameFromPeer(id), rec)
+		errs <- PutIPNSRecord(ctx, r, ipns.NameFromPeer(pid), rec)
 	}()
 
-	// Publish the public key if a public key cannot be extracted from the ID
-	// TODO: once v0.4.16 is widespread enough, we can stop doing this
-	// and at that point we can even deprecate the /pk/ namespace in the dht
-	//
-	// NOTE: This check actually checks if the public key has been embedded
-	// in the IPNS entry. This check is sufficient because we embed the
-	// public key in the IPNS entry if it can't be extracted from the ID.
-	if _, err := rec.PubKey(); err == nil {
+	// Publish the public key if the public key cannot be extracted from the peer ID.
+	// This is most likely not necessary since IPNS Records include, by default, the public
+	// key in those cases. However, this ensures it's still possible to easily retrieve
+	// the public key if, for some reason, it is not embedded.
+	if _, err := pid.ExtractPublicKey(); errors.Is(err, peer.ErrNoPublicKey) {
 		go func() {
-			errs <- PublishPublicKey(ctx, r, PkKeyForID(id), k)
+			errs <- PutPublicKey(ctx, r, pid, pubKey)
 		}()
 
 		if err := waitOnErrChan(ctx, errs); err != nil {
@@ -247,39 +246,37 @@ func waitOnErrChan(ctx context.Context, errs chan error) error {
 	}
 }
 
-// PublishPublicKey stores the given [crypto.PubKey] for the given key in the [routing.ValueStore].
-func PublishPublicKey(ctx context.Context, r routing.ValueStore, key string, pubKey crypto.PubKey) error {
-	ctx, span := startSpan(ctx, "PublishPublicKey", trace.WithAttributes(attribute.String("Key", key)))
+// PutPublicKey puts the given [crypto.PubKey] for the given [peer.ID] in the [routing.ValueStore].
+func PutPublicKey(ctx context.Context, r routing.ValueStore, pid peer.ID, pubKey crypto.PubKey) error {
+	routingKey := PkRoutingKey(pid)
+	ctx, span := startSpan(ctx, "PublishPublicKey", trace.WithAttributes(attribute.String("Key", routingKey)))
 	defer span.End()
 
-	log.Debugf("Storing pubkey at: %q", key)
 	bytes, err := crypto.MarshalPublicKey(pubKey)
 	if err != nil {
 		return err
 	}
 
-	// Store associated public key
-	return r.PutValue(ctx, key, bytes)
+	log.Debugf("Storing public key at: %x", routingKey)
+	return r.PutValue(ctx, routingKey, bytes)
 }
 
-// PublishIPNSRecord stores the given [ipns.Record] for the given [ipns.Name] in the given [routing.ValueStore].
-func PublishIPNSRecord(ctx context.Context, r routing.ValueStore, name ipns.Name, rec *ipns.Record) error {
-	routingKey := string(name.RoutingKey())
+// PkRoutingKey returns the public key routing key for the given [peer.ID].
+func PkRoutingKey(id peer.ID) string {
+	return "/pk/" + string(id)
+}
 
+// PutIPNSRecord puts the given [ipns.Record] for the given [ipns.Name] in the [routing.ValueStore].
+func PutIPNSRecord(ctx context.Context, r routing.ValueStore, name ipns.Name, rec *ipns.Record) error {
+	routingKey := string(name.RoutingKey())
 	ctx, span := startSpan(ctx, "PublishEntry", trace.WithAttributes(attribute.String("IPNSKey", routingKey)))
 	defer span.End()
 
-	data, err := ipns.MarshalRecord(rec)
+	bytes, err := ipns.MarshalRecord(rec)
 	if err != nil {
 		return err
 	}
 
-	log.Debugf("Storing ipns entry at: %x", routingKey)
-	// Store ipns entry at "/ipns/"+h(pubkey)
-	return r.PutValue(ctx, routingKey, data)
-}
-
-// PkKeyForID returns the public key routing key for the given [peer.ID].
-func PkKeyForID(id peer.ID) string {
-	return "/pk/" + string(id)
+	log.Debugf("Storing ipns record at: %x", routingKey)
+	return r.PutValue(ctx, routingKey, bytes)
 }

namesys/ipns_publisher_test.go

@@ -68,11 +68,11 @@ func testNamekeyPublisher(t *testing.T, keyType int, expectedErr error, expected
 	rec, err := ipns.NewRecord(privKey, value, seq, eol, 0)
 	require.NoError(t, err)
 
-	err = PublishRecord(ctx, r, pubKey, rec)
+	err = PublishIPNSRecord(ctx, r, pubKey, rec)
 	require.NoError(t, err)
 
 	// Check for namekey existence in value store
-	namekey := PkKeyForID(id)
+	namekey := PkRoutingKey(id)
 	_, err = r.GetValue(ctx, namekey)
 	require.ErrorIs(t, err, expectedErr)
 

namesys/ipns_resolver.go

@@ -2,14 +2,12 @@ package namesys
 
 import (
 	"context"
-	"strings"
 	"time"
 
 	"github.com/ipfs/boxo/ipns"
 	"github.com/ipfs/boxo/path"
 
 	dht "github.com/libp2p/go-libp2p-kad-dht"
-	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/libp2p/go-libp2p/core/routing"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/trace"
@@ -47,38 +45,31 @@ func (r *IPNSResolver) ResolveAsync(ctx context.Context, name string, options ..
 	return resolveAsync(ctx, r, name, ProcessResolveOptions(options))
 }
 
-func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan ResolveResult {
-	ctx, span := startSpan(ctx, "IpnsResolver.ResolveOnceAsync", trace.WithAttributes(attribute.String("Name", name)))
+func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, nameStr string, options ResolveOptions) <-chan ResolveResult {
+	ctx, span := startSpan(ctx, "IpnsResolver.ResolveOnceAsync", trace.WithAttributes(attribute.String("Name", nameStr)))
 	defer span.End()
 
 	out := make(chan ResolveResult, 1)
-	log.Debugf("RoutingResolver resolving %s", name)
+	log.Debugf("RoutingResolver resolving %s", nameStr)
 	cancel := func() {}
 
 	if options.DhtTimeout != 0 {
 		// Resolution must complete within the timeout
 		ctx, cancel = context.WithTimeout(ctx, options.DhtTimeout)
 	}
 
-	name = strings.TrimPrefix(name, "/ipns/")
-
-	pid, err := peer.Decode(name)
+	name, err := ipns.NameFromString(nameStr)
 	if err != nil {
-		log.Debugf("RoutingResolver: could not convert public key hash %s to peer ID: %s\n", name, err)
+		log.Debugf("RoutingResolver: could not convert key %q to IPNS name: %s\n", nameStr, err)
 		out <- ResolveResult{Err: err}
 		close(out)
 		cancel()
 		return out
 	}
 
-	// Use the routing system to get the name.
-	// Note that the DHT will call the ipns validator when retrieving
-	// the value, which in turn verifies the ipns record signature
-	ipnsKey := string(ipns.NameFromPeer(pid).RoutingKey())
-
-	vals, err := r.routing.SearchValue(ctx, ipnsKey, dht.Quorum(int(options.DhtRecordCount)))
+	vals, err := r.routing.SearchValue(ctx, string(name.RoutingKey()), dht.Quorum(int(options.DhtRecordCount)))
 	if err != nil {
-		log.Debugf("RoutingResolver: dht get for name %s failed: %s", name, err)
+		log.Debugf("RoutingResolver: dht get for name %s failed: %s", nameStr, err)
 		out <- ResolveResult{Err: err}
 		close(out)
 		cancel()
@@ -100,7 +91,7 @@ func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, name string, option
 
 				rec, err := ipns.UnmarshalRecord(val)
 				if err != nil {
-					log.Debugf("RoutingResolver: could not unmarshal value for name %s: %s", name, err)
+					log.Debugf("RoutingResolver: could not unmarshal value for name %s: %s", nameStr, err)
 					emitOnceResult(ctx, out, ResolveResult{Err: err})
 					return
 				}

namesys/ipns_resolver_test.go

@@ -51,7 +51,7 @@ func TestPrexistingExpiredRecord(t *testing.T) {
 	entry, err := ipns.NewRecord(identity.PrivateKey(), h, 0, eol, 0)
 	require.NoError(t, err)
 
-	err = PublishRecord(context.Background(), d, identity.PublicKey(), entry)
+	err = PublishIPNSRecord(context.Background(), d, identity.PublicKey(), entry)
 	require.NoError(t, err)
 
 	// Now, with an old record in the system already, try and publish a new one
@@ -77,7 +77,7 @@ func TestPrexistingRecord(t *testing.T) {
 	entry, err := ipns.NewRecord(identity.PrivateKey(), h, 0, eol, 0)
 	require.NoError(t, err)
 
-	err = PublishRecord(context.Background(), d, identity.PublicKey(), entry)
+	err = PublishIPNSRecord(context.Background(), d, identity.PublicKey(), entry)
 	require.NoError(t, err)
 
 	// Now, with an old record in the system already, try and publish a new one