coreapi: name/key review suggestions

License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>

Author: magik6k

core/coreapi/interface/interface.go

@@ -28,16 +28,21 @@ type Path interface {
 type Node ipld.Node
 type Link ipld.Link
 
-type IpnsEntry struct {
-	Name  string
-	Value Path
-}
-
 type Reader interface {
 	io.ReadSeeker
 	io.Closer
 }
 
+type IpnsEntry interface {
+	Name() string
+	Value() Path
+}
+
+type Key interface {
+	Name() string
+	Path() Path
+}
+
 // CoreAPI defines an unified interface to IPFS for Go programs.
 type CoreAPI interface {
 	// Unixfs returns an implementation of Unixfs API
@@ -108,16 +113,17 @@ type DagAPI interface {
 // You can use .Key API to list and generate more names and their respective keys.
 type NameAPI interface {
 	// Publish announces new IPNS name
-	Publish(ctx context.Context, path Path, opts ...options.NamePublishOption) (*IpnsEntry, error)
+	Publish(ctx context.Context, path Path, opts ...options.NamePublishOption) (IpnsEntry, error)
 
 	// WithValidTime is an option for Publish which specifies for how long the
 	// entry will remain valid. Default value is 24h
 	WithValidTime(validTime time.Duration) options.NamePublishOption
 
 	// WithKey is an option for Publish which specifies the key to use for
 	// publishing. Default value is "self" which is the node's own PeerID.
+	// The key parameter must be either PeerID or keystore key alias.
 	//
-	// You can use .Key API to list and generate more names and their respective keys.
+	// You can use KeyAPI to list and generate more names and their respective keys.
 	WithKey(key string) options.NamePublishOption
 
 	// Resolve attempts to resolve the newest version of the specified name
@@ -131,41 +137,42 @@ type NameAPI interface {
 	// offline. Default value is false
 	WithLocal(local bool) options.NameResolveOption
 
-	// WithNoCache is an option for Resolve which specifies when set to true
-	// disables the use of local name cache. Default value is false
-	WithNoCache(nocache bool) options.NameResolveOption
+	// WithCache is an option for Resolve which specifies if cache should be used.
+	// Default value is true
+	WithCache(cache bool) options.NameResolveOption
 }
 
 // KeyAPI specifies the interface to Keystore
 type KeyAPI interface {
 	// Generate generates new key, stores it in the keystore under the specified
 	// name and returns a base58 encoded multihash of it's public key
-	Generate(ctx context.Context, name string, opts ...options.KeyGenerateOption) (string, error)
+	Generate(ctx context.Context, name string, opts ...options.KeyGenerateOption) (Key, error)
 
 	// WithAlgorithm is an option for Generate which specifies which algorithm
-	// should be used for the key. Default is "rsa"
+	// should be used for the key. Default is options.RSAKey
 	//
 	// Supported algorithms:
-	// * rsa
-	// * ed25519
+	// * options.RSAKey
+	// * options.Ed25519Key
 	WithAlgorithm(algorithm string) options.KeyGenerateOption
 
 	// WithSize is an option for Generate which specifies the size of the key to
 	// generated. Default is 0
 	WithSize(size int) options.KeyGenerateOption
 
-	// Rename renames oldName key to newName.
-	Rename(ctx context.Context, oldName string, newName string, opts ...options.KeyRenameOption) (string, bool, error)
+	// Rename renames oldName key to newName. Returns the key and whether another
+	// key was overwritten, or an error
+	Rename(ctx context.Context, oldName string, newName string, opts ...options.KeyRenameOption) (Key, bool, error)
 
 	// WithForce is an option for Rename which specifies whether to allow to
 	// replace existing keys.
 	WithForce(force bool) options.KeyRenameOption
 
 	// List lists keys stored in keystore
-	List(ctx context.Context) (map[string]string, error) //TODO: better key type?
+	List(ctx context.Context) ([]Key, error)
 
-	// Remove removes keys from keystore
-	Remove(ctx context.Context, name string) (string, error)
+	// Remove removes keys from keystore. Returns ipns path of the removed key
+	Remove(ctx context.Context, name string) (Path, error)
 }
 
 // type ObjectAPI interface {

core/coreapi/interface/options/key.go

@@ -1,5 +1,10 @@
 package options
 
+const (
+	RSAKey     = "rsa"
+	Ed25519Key = "ed25519"
+)
+
 type KeyGenerateSettings struct {
 	Algorithm string
 	Size      int
@@ -14,7 +19,7 @@ type KeyRenameOption func(*KeyRenameSettings) error
 
 func KeyGenerateOptions(opts ...KeyGenerateOption) (*KeyGenerateSettings, error) {
 	options := &KeyGenerateSettings{
-		Algorithm: "rsa",
+		Algorithm: RSAKey,
 		Size:      0,
 	}
 

core/coreapi/interface/options/name.go

@@ -4,6 +4,10 @@ import (
 	"time"
 )
 
+const (
+	DefaultNameValidTime = 24 * time.Hour
+)
+
 type NamePublishSettings struct {
 	ValidTime time.Duration
 	Key       string
@@ -12,15 +16,15 @@ type NamePublishSettings struct {
 type NameResolveSettings struct {
 	Recursive bool
 	Local     bool
-	Nocache   bool
+	Cache     bool
 }
 
 type NamePublishOption func(*NamePublishSettings) error
 type NameResolveOption func(*NameResolveSettings) error
 
 func NamePublishOptions(opts ...NamePublishOption) (*NamePublishSettings, error) {
 	options := &NamePublishSettings{
-		ValidTime: 24 * time.Hour,
+		ValidTime: DefaultNameValidTime,
 		Key:       "self",
 	}
 
@@ -38,7 +42,7 @@ func NameResolveOptions(opts ...NameResolveOption) (*NameResolveSettings, error)
 	options := &NameResolveSettings{
 		Recursive: false,
 		Local:     false,
-		Nocache:   false,
+		Cache:     true,
 	}
 
 	for _, opt := range opts {
@@ -81,9 +85,9 @@ func (api *NameOptions) WithLocal(local bool) NameResolveOption {
 	}
 }
 
-func (api *NameOptions) WithNoCache(nocache bool) NameResolveOption {
+func (api *NameOptions) WithCache(cache bool) NameResolveOption {
 	return func(settings *NameResolveSettings) error {
-		settings.Nocache = nocache
+		settings.Cache = cache
 		return nil
 	}
 }

core/coreapi/key.go

@@ -8,8 +8,9 @@ import (
 
 	coreiface "github.com/ipfs/go-ipfs/core/coreapi/interface"
 	caopts "github.com/ipfs/go-ipfs/core/coreapi/interface/options"
+	ipfspath "github.com/ipfs/go-ipfs/path"
 
-	peer "gx/ipfs/QmXYjuNuxVzXKJCfWasQk1RqkhVLDM9jtUKhqc2WPQmFSB/go-libp2p-peer"
+	peer "gx/ipfs/QmWNY7dV54ZDYmTA1ykVdwNCqC11mpU4zSUp6XDpLTH9eG/go-libp2p-peer"
 	crypto "gx/ipfs/QmaPbCnUMBohSGo3KnxEa2bHqyJVVeEEcwtqJAYxerieBo/go-libp2p-crypto"
 )
 
@@ -18,10 +19,23 @@ type KeyAPI struct {
 	*caopts.KeyOptions
 }
 
-func (api *KeyAPI) Generate(ctx context.Context, name string, opts ...caopts.KeyGenerateOption) (string, error) {
+type key struct {
+	name   string
+	peerId string
+}
+
+func (k *key) Name() string {
+	return k.name
+}
+
+func (k *key) Path() coreiface.Path {
+	return &path{path: ipfspath.FromString(ipfspath.Join([]string{"/ipns/", k.peerId}))}
+}
+
+func (api *KeyAPI) Generate(ctx context.Context, name string, opts ...caopts.KeyGenerateOption) (coreiface.Key, error) {
 	options, err := caopts.KeyGenerateOptions(opts...)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	var sk crypto.PrivKey
@@ -30,54 +44,54 @@ func (api *KeyAPI) Generate(ctx context.Context, name string, opts ...caopts.Key
 	switch options.Algorithm {
 	case "rsa":
 		if options.Size == 0 {
-			return "", fmt.Errorf("please specify a key size with WithSize option")
+			return nil, fmt.Errorf("please specify a key size with WithSize option")
 		}
 
 		priv, pub, err := crypto.GenerateKeyPairWithReader(crypto.RSA, options.Size, rand.Reader)
 		if err != nil {
-			return "", err
+			return nil, err
 		}
 
 		sk = priv
 		pk = pub
 	case "ed25519":
 		priv, pub, err := crypto.GenerateEd25519Key(rand.Reader)
 		if err != nil {
-			return "", err
+			return nil, err
 		}
 
 		sk = priv
 		pk = pub
 	default:
-		return "", fmt.Errorf("unrecognized key type: %s", options.Algorithm)
+		return nil, fmt.Errorf("unrecognized key type: %s", options.Algorithm)
 	}
 
 	err = api.node.Repo.Keystore().Put(name, sk)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	pid, err := peer.IDFromPublicKey(pk)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
-	return pid.String(), nil
+	return &key{name, pid.String()}, nil
 }
 
-func (api *KeyAPI) List(ctx context.Context) (map[string]string, error) {
+func (api *KeyAPI) List(ctx context.Context) ([]coreiface.Key, error) {
 	keys, err := api.node.Repo.Keystore().List()
 	if err != nil {
 		return nil, err
 	}
 
 	sort.Strings(keys)
 
-	out := make(map[string]string, len(keys)+1)
-	out["self"] = api.node.Identity.Pretty()
+	out := make([]coreiface.Key, len(keys)+1)
+	out[0] = &key{"self", api.node.Identity.Pretty()}
 
-	for _, key := range keys {
-		privKey, err := api.node.Repo.Keystore().Get(key)
+	for n, k := range keys {
+		privKey, err := api.node.Repo.Keystore().Get(k)
 		if err != nil {
 			return nil, err
 		}
@@ -89,88 +103,88 @@ func (api *KeyAPI) List(ctx context.Context) (map[string]string, error) {
 			return nil, err
 		}
 
-		out[key] = pid.Pretty()
+		out[n+1] = &key{k, pid.Pretty()}
 	}
 	return out, nil
 }
 
-func (api *KeyAPI) Rename(ctx context.Context, oldName string, newName string, opts ...caopts.KeyRenameOption) (string, bool, error) {
+func (api *KeyAPI) Rename(ctx context.Context, oldName string, newName string, opts ...caopts.KeyRenameOption) (coreiface.Key, bool, error) {
 	options, err := caopts.KeyRenameOptions(opts...)
-	if newName == "self" {
-		return "", false, err
+	if err != nil {
+		return nil, false, err
 	}
 
 	ks := api.node.Repo.Keystore()
 
 	if oldName == "self" {
-		return "", false, fmt.Errorf("cannot rename key with name 'self'")
+		return nil, false, fmt.Errorf("cannot rename key with name 'self'")
 	}
 
 	if newName == "self" {
-		return "", false, fmt.Errorf("cannot overwrite key with name 'self'")
+		return nil, false, fmt.Errorf("cannot overwrite key with name 'self'")
 	}
 
 	oldKey, err := ks.Get(oldName)
 	if err != nil {
-		return "", false, fmt.Errorf("no key named %s was found", oldName)
+		return nil, false, fmt.Errorf("no key named %s was found", oldName)
 	}
 
 	pubKey := oldKey.GetPublic()
 
 	pid, err := peer.IDFromPublicKey(pubKey)
 	if err != nil {
-		return "", false, err
+		return nil, false, err
 	}
 
 	overwrite := false
 	if options.Force {
 		exist, err := ks.Has(newName)
 		if err != nil {
-			return "", false, err
+			return nil, false, err
 		}
 
 		if exist {
 			overwrite = true
 			err := ks.Delete(newName)
 			if err != nil {
-				return "", false, err
+				return nil, false, err
 			}
 		}
 	}
 
 	err = ks.Put(newName, oldKey)
 	if err != nil {
-		return "", false, err
+		return nil, false, err
 	}
 
-	return pid.Pretty(), overwrite, ks.Delete(oldName)
+	return &key{newName, pid.Pretty()}, overwrite, ks.Delete(oldName)
 }
 
-func (api *KeyAPI) Remove(ctx context.Context, name string) (string, error) {
+func (api *KeyAPI) Remove(ctx context.Context, name string) (coreiface.Path, error) {
 	ks := api.node.Repo.Keystore()
 
 	if name == "self" {
-		return "", fmt.Errorf("cannot remove key with name 'self'")
+		return nil, fmt.Errorf("cannot remove key with name 'self'")
 	}
 
 	removed, err := ks.Get(name)
 	if err != nil {
-		return "", fmt.Errorf("no key named %s was found", name)
+		return nil, fmt.Errorf("no key named %s was found", name)
 	}
 
 	pubKey := removed.GetPublic()
 
 	pid, err := peer.IDFromPublicKey(pubKey)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	err = ks.Delete(name)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
-	return pid.Pretty(), nil
+	return (&key{"", pid.Pretty()}).Path(), nil
 }
 
 func (api *KeyAPI) core() coreiface.CoreAPI {