From 32beb68234cb68a60d5764298da2eb4949774858 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Mon, 4 Sep 2023 21:03:51 +0200 Subject: [PATCH] refactor: cleanup some names and types --- namesys/ipns_publisher.go | 73 ++++++++++++++++------------------ namesys/ipns_publisher_test.go | 4 +- namesys/ipns_resolver.go | 25 ++++-------- namesys/ipns_resolver_test.go | 4 +- 4 files changed, 47 insertions(+), 59 deletions(-) diff --git a/namesys/ipns_publisher.go b/namesys/ipns_publisher.go index ef058138ec..6510563229 100644 --- a/namesys/ipns_publisher.go +++ b/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) } diff --git a/namesys/ipns_publisher_test.go b/namesys/ipns_publisher_test.go index ceb959f122..ae1c92d039 100644 --- a/namesys/ipns_publisher_test.go +++ b/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) diff --git a/namesys/ipns_resolver.go b/namesys/ipns_resolver.go index be4b0412ba..9a00f3be79 100644 --- a/namesys/ipns_resolver.go +++ b/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,12 +45,12 @@ 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 { @@ -60,25 +58,18 @@ func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, name string, option 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 } diff --git a/namesys/ipns_resolver_test.go b/namesys/ipns_resolver_test.go index 20d9909f19..e2edb1b2e3 100644 --- a/namesys/ipns_resolver_test.go +++ b/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