Merge pull request #210 from guggero/rest-fixes

Fix multiple REST issues, allow only one macaroon to be used for lnd connection

auctioneer/client.go

@@ -114,12 +114,14 @@ type Client struct {
 	serverConn *grpc.ClientConn
 	client     auctioneerrpc.ChannelAuctioneerClient
 
-	quit            chan struct{}
-	wg              sync.WaitGroup
-	serverStream    auctioneerrpc.ChannelAuctioneer_SubscribeBatchAuctionClient
-	streamMutex     sync.Mutex
-	streamCancel    func()
-	subscribedAccts map[[33]byte]*acctSubscription
+	quit         chan struct{}
+	wg           sync.WaitGroup
+	serverStream auctioneerrpc.ChannelAuctioneer_SubscribeBatchAuctionClient
+	streamMutex  sync.Mutex
+	streamCancel func()
+
+	subscribedAccts    map[[33]byte]*acctSubscription
+	subscribedAcctsMtx sync.Mutex
 }
 
 // NewClient returns a new instance to initiate auctions with.
@@ -228,10 +230,12 @@ func (c *Client) closeStream() error {
 	c.serverStream = nil
 
 	// Close all pending subscriptions.
+	c.subscribedAcctsMtx.Lock()
 	for _, subscription := range c.subscribedAccts {
 		close(subscription.quit)
 		close(subscription.msgChan)
 	}
+	c.subscribedAcctsMtx.Unlock()
 
 	return err
 }
@@ -483,7 +487,9 @@ func (c *Client) connectAndAuthenticate(ctx context.Context,
 	copy(acctPubKey[:], acctKey.PubKey.SerializeCompressed())
 
 	// Don't subscribe more than once.
+	c.subscribedAcctsMtx.Lock()
 	sub, ok := c.subscribedAccts[acctPubKey]
+	c.subscribedAcctsMtx.Unlock()
 	if ok {
 		if recovery {
 			return sub, true, fmt.Errorf("account %x is already "+
@@ -492,10 +498,14 @@ func (c *Client) connectAndAuthenticate(ctx context.Context,
 		return sub, true, nil
 	}
 
+	// Guard the read access only. We can't use defer for the unlock because
+	// both the connectServerStream and SendAuctionMessage need to hold the
+	// mutex as well.
 	c.streamMutex.Lock()
-	defer c.streamMutex.Unlock()
+	needToConnect := c.serverStream == nil
+	c.streamMutex.Unlock()
 
-	if c.serverStream == nil {
+	if needToConnect {
 		err := c.connectServerStream(0, initialConnectRetries)
 		if err != nil {
 			return sub, false, fmt.Errorf("connecting server "+
@@ -530,7 +540,9 @@ func (c *Client) connectAndAuthenticate(ctx context.Context,
 		errChan: tempErrChan,
 		quit:    make(chan struct{}),
 	}
+	c.subscribedAcctsMtx.Lock()
 	c.subscribedAccts[acctPubKey] = sub
+	c.subscribedAcctsMtx.Unlock()
 	err := sub.authenticate(ctx)
 	if err != nil {
 		return sub, false, err
@@ -769,6 +781,9 @@ func incompleteAcctFromErr(traderKey *keychain.KeyDescriptor,
 // the auction server. A message can only be sent as a response to a server
 // message, therefore the stream must already be open.
 func (c *Client) SendAuctionMessage(msg *auctioneerrpc.ClientAuctionMessage) error {
+	c.streamMutex.Lock()
+	defer c.streamMutex.Unlock()
+
 	if c.serverStream == nil {
 		return fmt.Errorf("cannot send message, stream not open")
 	}
@@ -788,11 +803,23 @@ func (c *Client) wait(backoff time.Duration) error {
 	}
 }
 
+// IsSubscribed returns true if at least one account is in an active state and
+// the subscription stream to the server was established successfully.
+func (c *Client) IsSubscribed() bool {
+	c.streamMutex.Lock()
+	defer c.streamMutex.Unlock()
+
+	return c.serverStream != nil
+}
+
 // connectServerStream opens the initial connection to the server for the stream
 // of account updates and handles reconnect trials with incremental backoff.
 func (c *Client) connectServerStream(initialBackoff time.Duration,
 	numRetries int) error {
 
+	c.streamMutex.Lock()
+	defer c.streamMutex.Unlock()
+
 	var (
 		backoff = initialBackoff
 		ctxb    = context.Background()
@@ -926,11 +953,13 @@ func (c *Client) readIncomingStream() { // nolint:gocyclo
 			var commitHash [32]byte
 			copy(commitHash[:], t.Challenge.CommitHash)
 			var acctSub *acctSubscription
+			c.subscribedAcctsMtx.Lock()
 			for traderKey, sub := range c.subscribedAccts {
 				if sub.commitHash == commitHash {
 					acctSub = c.subscribedAccts[traderKey]
 				}
 			}
+			c.subscribedAcctsMtx.Unlock()
 			if acctSub == nil {
 				c.errChanSwitch.ErrChan() <- fmt.Errorf("no "+
 					"subscription found for commit hash %x",
@@ -1029,7 +1058,9 @@ func (c *Client) sendToSubscription(traderAccountKey []byte,
 	// added.
 	var traderKey [33]byte
 	copy(traderKey[:], traderAccountKey)
+	c.subscribedAcctsMtx.Lock()
 	acctSub, ok := c.subscribedAccts[traderKey]
+	c.subscribedAcctsMtx.Unlock()
 	if !ok {
 		return fmt.Errorf("no subscription found for account key %x",
 			traderAccountKey)
@@ -1060,12 +1091,8 @@ func (c *Client) HandleServerShutdown(err error) error {
 		log.Errorf("Error closing stream connection: %v", err)
 	}
 
-	// Guard the server stream from concurrent access. We can't use defer
-	// to unlock here because SubscribeAccountUpdates is called later on
-	// which requires access to the lock as well.
-	c.streamMutex.Lock()
+	// Try to get a new connection, retry if not successful immediately.
 	err = c.connectServerStream(c.cfg.MinBackoff, reconnectRetries)
-	c.streamMutex.Unlock()
 	if err != nil {
 		return err
 	}
@@ -1079,11 +1106,13 @@ func (c *Client) HandleServerShutdown(err error) error {
 
 	// Subscribe to all accounts again. Remove the old subscriptions in the
 	// same move as new ones will be created.
+	c.subscribedAcctsMtx.Lock()
 	acctKeys := make([]*keychain.KeyDescriptor, 0, len(c.subscribedAccts))
 	for key, subscription := range c.subscribedAccts {
 		acctKeys = append(acctKeys, subscription.acctKey)
 		delete(c.subscribedAccts, key)
 	}
+	c.subscribedAcctsMtx.Unlock()
 	for _, acctKey := range acctKeys {
 		err := c.SubscribeAccountUpdates(context.Background(), acctKey)
 		if err != nil {

cmd/pool/info.go

@@ -0,0 +1,35 @@
+package main
+
+import (
+	"context"
+
+	"github.com/lightninglabs/pool/poolrpc"
+	"github.com/urfave/cli"
+)
+
+var getInfoCommand = cli.Command{
+	Name:  "getinfo",
+	Usage: "show info about the daemon's current state",
+	Description: "Displays basic info about the current state of the Pool " +
+		"trader daemon",
+	Action: getInfo,
+}
+
+func getInfo(ctx *cli.Context) error {
+	client, cleanup, err := getClient(ctx)
+	if err != nil {
+		return err
+	}
+	defer cleanup()
+
+	resp, err := client.GetInfo(
+		context.Background(), &poolrpc.GetInfoRequest{},
+	)
+	if err != nil {
+		return err
+	}
+
+	printRespJSON(resp)
+
+	return nil
+}

cmd/pool/main.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"context"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
@@ -134,14 +135,37 @@ func main() {
 	app.Commands = append(app.Commands, ordersCommands...)
 	app.Commands = append(app.Commands, auctionCommands...)
 	app.Commands = append(app.Commands, listAuthCommand)
+	app.Commands = append(app.Commands, getInfoCommand)
 	app.Commands = append(app.Commands, debugCommands...)
+	app.Commands = append(app.Commands, stopDaemonCommand)
 
 	err := app.Run(os.Args)
 	if err != nil {
 		fatal(err)
 	}
 }
 
+var stopDaemonCommand = cli.Command{
+	Name:  "stop",
+	Usage: "gracefully shut down the daemon",
+	Description: "Sends the stop command to the Pool trader daemon to " +
+		"initiate a graceful shutdown",
+	Action: stop,
+}
+
+func stop(ctx *cli.Context) error {
+	client, cleanup, err := getClient(ctx)
+	if err != nil {
+		return err
+	}
+	defer cleanup()
+
+	_, err = client.StopDaemon(
+		context.Background(), &poolrpc.StopDaemonRequest{},
+	)
+	return err
+}
+
 func getClient(ctx *cli.Context) (poolrpc.TraderClient, func(),
 	error) {
 

config.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"path"
 	"path/filepath"
 	"time"
 
@@ -51,6 +52,10 @@ var (
 
 	defaultSelfSignedOrganization = "pool autogenerated cert"
 
+	// defaultLndMacaroon is the default macaroon file we use if the old,
+	// deprecated --lnd.macaroondir config option is used.
+	defaultLndMacaroon = "admin.macaroon"
+
 	// DefaultTLSCertPath is the default full path of the autogenerated TLS
 	// certificate.
 	DefaultTLSCertPath = filepath.Join(
@@ -75,9 +80,20 @@ var (
 )
 
 type LndConfig struct {
-	Host        string `long:"host" description:"lnd instance rpc address"`
-	MacaroonDir string `long:"macaroondir" description:"Path to the directory containing all the required lnd macaroons"`
-	TLSPath     string `long:"tlspath" description:"Path to lnd tls certificate"`
+	Host string `long:"host" description:"lnd instance rpc address"`
+
+	// MacaroonDir is the directory that contains all the macaroon files
+	// required for the remote connection.
+	MacaroonDir string `long:"macaroondir" description:"DEPRECATED: Use macaroonpath."`
+
+	// MacaroonPath is the path to the single macaroon that should be used
+	// instead of needing to specify the macaroon directory that contains
+	// all of lnd's macaroons. The specified macaroon MUST have all
+	// permissions that all the subservers use, otherwise permission errors
+	// will occur.
+	MacaroonPath string `long:"macaroonpath" description:"The full path to the single macaroon to use, either the admin.macaroon or a custom baked one. Cannot be specified at the same time as macaroondir. A custom macaroon must contain ALL permissions required for all subservers to work, otherwise permission errors will occur."`
+
+	TLSPath string `long:"tlspath" description:"Path to lnd tls certificate"`
 }
 
 type Config struct {
@@ -239,6 +255,30 @@ func Validate(cfg *Config) error {
 		return err
 	}
 
+	// Make sure only one of the macaroon options is used.
+	switch {
+	case cfg.Lnd.MacaroonPath != "" && cfg.Lnd.MacaroonDir != "":
+		return fmt.Errorf("use --lnd.macaroonpath only")
+
+	case cfg.Lnd.MacaroonDir != "":
+		// With the new version of lndclient we can only specify a
+		// single macaroon instead of all of them. If the old
+		// macaroondir is used, we use the admin macaroon located in
+		// that directory.
+		cfg.Lnd.MacaroonPath = path.Join(
+			lncfg.CleanAndExpandPath(cfg.Lnd.MacaroonDir),
+			defaultLndMacaroon,
+		)
+
+	case cfg.Lnd.MacaroonPath != "":
+		cfg.Lnd.MacaroonPath = lncfg.CleanAndExpandPath(
+			cfg.Lnd.MacaroonPath,
+		)
+
+	default:
+		return fmt.Errorf("must specify --lnd.macaroonpath")
+	}
+
 	return nil
 }
 

docs/install.md

@@ -54,13 +54,13 @@ $ poold &
 $ poold --network=testnet
 ```
 
-In the case that `lnd` is running on a remote node, the `tls.cert` and all `*.macaroon` files from the `lnd` data directory need to be copied to the machine where `poold` is running.
+In the case that `lnd` is running on a remote node, the `tls.cert` and the `admin.macaroon` files from the `lnd` data directory need to be copied to the machine where `poold` is running.
 
 The daemon can then be configured to connect to the remote `lnd` node by using the following command line flags:
 
 ```text
 $ poold --lnd.host=<the_remote_host_IP_address>:10009 \
-        --lnd.macaroondir=/some/directory/with/lnd/data/macaroons \
+        --lnd.macaroonpath=/some/directory/with/lnd/data/macaroons/admin.macaroon \
         --lnd.tlspath=/some/directory/with/lnd/data/tls.cert
 ```
 
@@ -70,7 +70,7 @@ To persist this configuration, these values can also be written to a configurati
 >
 > ```text
 > lnd.host=<the_remote_host_IP_address>:10009
-> lnd.macaroondir=/some/directory/with/lnd/data/macaroons
+> lnd.macaroonpath=/some/directory/with/lnd/data/macaroons/admin.macaroon
 > lnd.tlspath=/some/directory/with/lnd/data/tls.cert
 > ```
 

macaroons.go

@@ -27,6 +27,23 @@ var (
 	// RequiredPermissions is a map of all pool RPC methods and their
 	// required macaroon permissions to access poold.
 	RequiredPermissions = map[string][]bakery.Op{
+		"/poolrpc.Trader/GetInfo": {{
+			Entity: "account",
+			Action: "read",
+		}, {
+			Entity: "order",
+			Action: "read",
+		}, {
+			Entity: "auction",
+			Action: "read",
+		}, {
+			Entity: "auth",
+			Action: "read",
+		}},
+		"/poolrpc.Trader/StopDaemon": {{
+			Entity: "account",
+			Action: "write",
+		}},
 		"/poolrpc.Trader/QuoteAccount": {{
 			Entity: "account",
 			Action: "read",