Fix multiple REST issues, allow only one macaroon to be used for lnd connection #210
Conversation
server.go
Outdated
| s.cfg.Network, | ||
| s.cfg.Lnd.Host, s.cfg.Lnd.TLSPath, | ||
| path.Dir(s.cfg.Lnd.MacaroonPath), s.cfg.Network, | ||
| lndclient.MacFilename(defaultLndMacaroon), |
There was a problem hiding this comment.
does this work if admin macaroon is not used?
There was a problem hiding this comment.
Good catch! It would indeed not have worked. Fixed it by using path.Base() instead.
|
Great PR and additions :) |
| ] | ||
| } | ||
| }, | ||
| "/v1/pool/batch/snapshots/{start_batch_id}": { |
There was a problem hiding this comment.
Does this work with /v1/pool/batch/snapshots/ as well, without start_batch_id? It states the parameter is required (line 473), but with or without trailing slash should work the same IMO.
There was a problem hiding this comment.
Yes, that's what line 409 is for. There are three URL patterns now:
/v1/pool/batch/snapshots
/v1/pool/batch/snapshots/{start_batch_id}
/v1/pool/batch/snapshots/{start_batch_id}/{num_batches_back}
And you can use the GET parameters too:
/v1/pool/batch/snapshots?num_batches_back=10
/v1/pool/batch/snapshots/{start_batch_id}?num_batches_back=10
There was a problem hiding this comment.
My comment was on /v1/pool/batch/snapshots/, with a trailing slash and without start_batch_id query param. It looks like that's not included in the pattern. But it's a minor thing.
|
Nice additions! Two more things for getinfo, but these might be out of scope for this:
|
949d472 to
add330c
Compare
|
Thanks for the review, @mutatrum!
Great ideas, added both of them. |
| c.streamMutex.Lock() | ||
| defer c.streamMutex.Unlock() | ||
| needToConnect := c.serverStream == nil | ||
| c.streamMutex.Unlock() |
There was a problem hiding this comment.
Don't we also need the lock for subscribedAccts below?
There was a problem hiding this comment.
Yes, added an extra mutex.
Fixes lightninglabs#208 by always rendering the default values in a JSON response, even if they are "falsey" in the RPC message.
The order manager already stores the local lnd node's identity public key. We return it in an exported method so the RPC server can access it as well without needing to query it again. To make sure we only allow querying it after the manager is started, we add an atomic flag for the startup state.
The IsSubscribed() method informs about the subscription connection status to the auctioneer server.
This commit fixes a missing mutex lock in the SendAuctionMessage. To make it possible to hold it there, we need to remove the deferred unlock in connectAndAuthenticate into connectServerStream. Otherwise we'd run into deadlocks.
Because the subscribedAccts map is potentially also accessed by multiple goroutines at the same time, we add a mutex for it as well.
To give developers and users more information about the current state of the trader daemon, we add a new GetInfo RPC that shows a summary of all accounts and orders known to the system. And because there currently is no other way than to <ctrl>+c or kill the daemon, we add a StopDaemon RPC as well for proper shutdown.
This commit adds the required macaroon permissions for the two new RPC methods added in the previous commit. We don't want to introduce a new permission that didn't exist before because that would mean all users would need to re-create their macaroon to use the commands. Therefore we use the account:write permission for the shutdown command as that is probably the most sensitive permission anyway as it gives access to on-chain funds.
Fixes lightninglabs#204 by allowing multiple REST URI patterns for the batch snapshots call.
Fixes lightninglabs#196 by allowing only one macaroon to be specified in the --lnd.macaroonpath config option.
venue: ignore all messages from traders not part of the batch
Fixes #208.
Fixes #209.
Fixes #196.
Fixes #204.
Contains a number of smaller fixes that should help developers and users alike.