From 3df216f6c31f160efa41a8b3e57de208a465cddb Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Sat, 6 Nov 2021 21:22:45 +0100 Subject: [PATCH 1/7] channeldb: avoid locking the graph cache while iterating channels It may happen that we do pathfinding while also attempt to change the graph. In this case changing the database, channel state, graph while reading from the same sources may create deadlocks. To resolve this we change locking policy in the graph cache when pathfinding. --- channeldb/graph_cache.go | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/channeldb/graph_cache.go b/channeldb/graph_cache.go index 3618b6d43..0af8fb591 100644 --- a/channeldb/graph_cache.go +++ b/channeldb/graph_cache.go @@ -401,10 +401,9 @@ func (c *GraphCache) UpdateChannel(info *ChannelEdgeInfo) { } } -// ForEachChannel invokes the given callback for each channel of the given node. -func (c *GraphCache) ForEachChannel(node route.Vertex, - cb func(channel *DirectedChannel) error) error { - +// getChannels returns a copy of the passed node's channels or nil if there +// isn't any. +func (c *GraphCache) getChannels(node route.Vertex) []*DirectedChannel { c.mtx.RLock() defer c.mtx.RUnlock() @@ -428,6 +427,8 @@ func (c *GraphCache) ForEachChannel(node route.Vertex, return node } + i := 0 + channelsCopy := make([]*DirectedChannel, len(channels)) for _, channel := range channels { // We need to copy the channel and policy to avoid it being // updated in the cache if the path finding algorithm sets @@ -439,9 +440,30 @@ func (c *GraphCache) ForEachChannel(node route.Vertex, channelCopy.InPolicy.ToNodeFeatures = features } - if err := cb(channelCopy); err != nil { + channelsCopy[i] = channelCopy + i++ + } + + return channelsCopy +} + +// ForEachChannel invokes the given callback for each channel of the given node. +func (c *GraphCache) ForEachChannel(node route.Vertex, + cb func(channel *DirectedChannel) error) error { + + // Obtain a copy of the node's channels. We need do this in order to + // avoid deadlocks caused by interaction with the graph cache, channel + // state and the graph database from multiple goroutines. This snapshot + // is only used for path finding where being stale is acceptable since + // the real world graph and our representation may always become + // slightly out of sync for a short time and the actual channel state + // is stored separately. + channels := c.getChannels(node) + for _, channel := range channels { + if err := cb(channel); err != nil { return err } + } return nil From 0beaee0336a3e7a3f1b25961c191869ae51976fd Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Sat, 6 Nov 2021 21:32:39 +0100 Subject: [PATCH 2/7] docs: update release notes --- docs/release-notes/release-notes-0.14.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md index c1fa4b253..c40cbd2c5 100644 --- a/docs/release-notes/release-notes-0.14.0.md +++ b/docs/release-notes/release-notes-0.14.0.md @@ -618,6 +618,9 @@ messages directly. There is no routing/path finding involved. * [Fixed an issue with external listeners and the `--noseedbackup` development flag](https://github.com/lightningnetwork/lnd/pull/5930). +* [Fix deadlock when using the graph cache]( + https://github.com/lightningnetwork/lnd/pull/5941) + ## Documentation The [code contribution guidelines have been updated to mention the new From 338afef8628b3f99bc2cd109d9384539ebc4b43a Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 8 Nov 2021 11:31:02 +0100 Subject: [PATCH 3/7] lnd: standardize printing to stdout With this commit we standardize the error messages in the config parsing section of the main package. We only print to stdout/stderr in a single place and also make sure the same error is printed to the log (which might or might not yet be initialized at that point). --- chainreg/chainregistry.go | 64 +++--- config.go | 407 +++++++++++++++++++------------------- config_builder.go | 7 +- lnd.go | 16 +- 4 files changed, 257 insertions(+), 237 deletions(-) diff --git a/chainreg/chainregistry.go b/chainreg/chainregistry.go index 0980b729e..1eff0ca5b 100644 --- a/chainreg/chainregistry.go +++ b/chainreg/chainregistry.go @@ -53,7 +53,8 @@ type Config struct { // queries if true. HeightHintCacheQueryDisable bool - // NeutrinoMode defines settings for connecting to a neutrino light-client. + // NeutrinoMode defines settings for connecting to a neutrino + // light-client. NeutrinoMode *lncfg.Neutrino // BitcoindMode defines settings for connecting to a bitcoind node. @@ -83,8 +84,8 @@ type Config struct { // the main wallet. WalletUnlockParams *walletunlocker.WalletUnlockParams - // NeutrinoCS is a pointer to a neutrino ChainService. Must be non-nil if - // using neutrino. + // NeutrinoCS is a pointer to a neutrino ChainService. Must be non-nil + // if using neutrino. NeutrinoCS *neutrino.ChainService // ActiveNetParams details the current chain we are on. @@ -152,8 +153,8 @@ const ( BtcToLtcConversionRate = 60 ) -// DefaultLtcChannelConstraints is the default set of channel constraints that are -// meant to be used when initially funding a Litecoin channel. +// DefaultLtcChannelConstraints is the default set of channel constraints that +// are meant to be used when initially funding a Litecoin channel. var DefaultLtcChannelConstraints = channeldb.ChannelConstraints{ DustLimit: DefaultLitecoinDustLimit, MaxAcceptedHtlcs: input.MaxHTLCNumber / 2, @@ -285,8 +286,8 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { DefaultLitecoinStaticFeePerKW, 0, ) default: - return nil, nil, fmt.Errorf("default routing policy for chain %v is "+ - "unknown", cfg.PrimaryChain()) + return nil, nil, fmt.Errorf("default routing policy for chain "+ + "%v is unknown", cfg.PrimaryChain()) } var err error @@ -329,7 +330,8 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { if cfg.NeutrinoMode.FeeURL != "" { if cfg.FeeURL != "" { return nil, nil, errors.New("feeurl and " + - "neutrino.feeurl are mutually exclusive") + "neutrino.feeurl are mutually " + + "exclusive") } cfg.FeeURL = cfg.NeutrinoMode.FeeURL @@ -414,8 +416,8 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { } if err := bitcoindConn.Start(); err != nil { - return nil, nil, fmt.Errorf("unable to connect to bitcoind: "+ - "%v", err) + return nil, nil, fmt.Errorf("unable to connect to "+ + "bitcoind: %v", err) } cc.ChainNotifier = bitcoindnotify.New( @@ -439,8 +441,8 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { HTTPPostMode: true, } if cfg.Bitcoin.Active && !cfg.Bitcoin.RegTest { - log.Infof("Initializing bitcoind backed fee estimator in "+ - "%s mode", bitcoindMode.EstimateMode) + log.Infof("Initializing bitcoind backed fee estimator "+ + "in %s mode", bitcoindMode.EstimateMode) // Finally, we'll re-initialize the fee estimator, as // if we're using bitcoind as a backend, then we can @@ -455,8 +457,9 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { return nil, nil, err } } else if cfg.Litecoin.Active && !cfg.Litecoin.RegTest { - log.Infof("Initializing litecoind backed fee estimator in "+ - "%s mode", bitcoindMode.EstimateMode) + log.Infof("Initializing litecoind backed fee "+ + "estimator in %s mode", + bitcoindMode.EstimateMode) // Finally, we'll re-initialize the fee estimator, as // if we're using litecoind as a backend, then we can @@ -558,8 +561,8 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { return nil, nil, err } - // Finally, we'll create an instance of the default chain view to be - // used within the routing layer. + // Finally, we'll create an instance of the default chain view + // to be used within the routing layer. cc.ChainView, err = chainview.NewBtcdFilteredChainView( *rpcConfig, cfg.BlockCache, ) @@ -568,10 +571,12 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { return nil, nil, err } - // Create a special websockets rpc client for btcd which will be used - // by the wallet for notifications, calls, etc. - chainRPC, err := chain.NewRPCClient(cfg.ActiveNetParams.Params, btcdHost, - btcdUser, btcdPass, rpcCert, false, 20) + // Create a special websockets rpc client for btcd which will be + // used by the wallet for notifications, calls, etc. + chainRPC, err := chain.NewRPCClient( + cfg.ActiveNetParams.Params, btcdHost, btcdUser, + btcdPass, rpcCert, false, 20, + ) if err != nil { return nil, nil, err } @@ -615,8 +620,8 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { case cfg.FeeURL == "" && cfg.Bitcoin.MainNet && homeChainConfig.Node == "neutrino": - return nil, nil, fmt.Errorf("--feeurl parameter required when " + - "running neutrino on mainnet") + return nil, nil, fmt.Errorf("--feeurl parameter required " + + "when running neutrino on mainnet") // Override default fee estimator if an external service is specified. case cfg.FeeURL != "": @@ -638,7 +643,8 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { ccCleanup := func() { if cc.FeeEstimator != nil { if err := cc.FeeEstimator.Stop(); err != nil { - log.Errorf("Failed to stop feeEstimator: %v", err) + log.Errorf("Failed to stop feeEstimator: %v", + err) } } } @@ -686,12 +692,12 @@ func NewChainControl(walletConfig lnwallet.Config, lnWallet, err := lnwallet.NewLightningWallet(walletConfig) if err != nil { - fmt.Printf("unable to create wallet: %v\n", err) - return nil, ccCleanup, err + return nil, ccCleanup, fmt.Errorf("unable to create wallet: %v", + err) } if err := lnWallet.Startup(); err != nil { - fmt.Printf("unable to start wallet: %v\n", err) - return nil, ccCleanup, err + return nil, ccCleanup, fmt.Errorf("unable to create wallet: %v", + err) } log.Info("LightningWallet opened") @@ -872,7 +878,9 @@ func (c *ChainRegistry) LookupChain(targetChain ChainCode) ( // LookupChainByHash attempts to look up an active ChainControl which // corresponds to the passed genesis hash. -func (c *ChainRegistry) LookupChainByHash(chainHash chainhash.Hash) (*ChainControl, bool) { +func (c *ChainRegistry) LookupChainByHash( + chainHash chainhash.Hash) (*ChainControl, bool) { + c.RLock() defer c.RUnlock() diff --git a/config.go b/config.go index 158e152be..c4c70540d 100644 --- a/config.go +++ b/config.go @@ -647,15 +647,34 @@ func LoadConfig(interceptor signal.Interceptor) (*Config, error) { } // Make sure everything we just loaded makes sense. - cleanCfg, err := ValidateConfig(cfg, usageMessage, interceptor, - fileParser, flagParser) + cleanCfg, err := ValidateConfig( + cfg, interceptor, fileParser, flagParser, + ) + if usageErr, ok := err.(*usageError); ok { + // The logging system might not yet be initialized, so we also + // write to stderr to make sure the error appears somewhere. + _, _ = fmt.Fprintln(os.Stderr, usageMessage) + ltndLog.Warnf("Incorrect usage: %v", usageMessage) + + // The log subsystem might not yet be initialized. But we still + // try to log the error there since some packaging solutions + // might only look at the log and not stdout/stderr. + ltndLog.Warnf("Error validating config: %v", usageErr.err) + + return nil, usageErr.err + } if err != nil { + // The log subsystem might not yet be initialized. But we still + // try to log the error there since some packaging solutions + // might only look at the log and not stdout/stderr. + ltndLog.Warnf("Error validating config: %v", err) + return nil, err } // Warn about missing config file only after all other configuration is - // done. This prevents the warning on help messages and invalid - // options. Note this should go directly before the return. + // done. This prevents the warning on help messages and invalid options. + // Note this should go directly before the return. if configFileError != nil { ltndLog.Warnf("%v", configFileError) } @@ -663,12 +682,24 @@ func LoadConfig(interceptor signal.Interceptor) (*Config, error) { return cleanCfg, nil } +// usageError is an error type that signals a problem with the supplied flags. +type usageError struct { + err error +} + +// Error returns the error string. +// +// NOTE: This is part of the error interface. +func (u *usageError) Error() string { + return u.err.Error() +} + // ValidateConfig check the given configuration to be sane. This makes sure no // illegal values or combination of values are set. All file system paths are // normalized. The cleaned up config is returned on success. -func ValidateConfig(cfg Config, usageMessage string, - interceptor signal.Interceptor, fileParser, +func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser, flagParser *flags.Parser) (*Config, error) { + // If the provided lnd directory is not the default, we'll modify the // path to all of the files and directories that will live within it. lndDir := CleanAndExpandPath(cfg.LndDir) @@ -685,12 +716,16 @@ func ValidateConfig(cfg Config, usageMessage string, // user has not requested a different location, we'll move the // location to be relative to the specified lnd directory. if cfg.Watchtower.TowerDir == defaultTowerDir { - cfg.Watchtower.TowerDir = - filepath.Join(cfg.DataDir, defaultTowerSubDirname) + cfg.Watchtower.TowerDir = filepath.Join( + cfg.DataDir, defaultTowerSubDirname, + ) } } - funcName := "loadConfig" + funcName := "ValidateConfig" + mkErr := func(format string, args ...interface{}) error { + return fmt.Errorf(funcName+": "+format, args...) + } makeDirectory := func(dir string) error { err := os.MkdirAll(dir, 0700) if err != nil { @@ -705,10 +740,8 @@ func ValidateConfig(cfg Config, usageMessage string, } } - str := "%s: Failed to create lnd directory: %v" - err := fmt.Errorf(str, funcName, err) - _, _ = fmt.Fprintln(os.Stderr, err) - return err + str := "Failed to create lnd directory: %v" + return mkErr(str, err) } return nil @@ -716,21 +749,17 @@ func ValidateConfig(cfg Config, usageMessage string, // IsSet returns true if an option has been set in either the config // file or by a flag. - isSet := func(field string) bool { + isSet := func(field string) (bool, error) { fieldName, ok := reflect.TypeOf(Config{}).FieldByName(field) if !ok { - str := "%s: could not find field %s" - err := fmt.Errorf(str, funcName, field) - _, _ = fmt.Fprintln(os.Stderr, err) - return false + str := "could not find field %s" + return false, mkErr(str, field) } long, ok := fieldName.Tag.Lookup("long") if !ok { - str := "%s: field %s does not have a long tag" - err := fmt.Errorf(str, funcName, field) - _, _ = fmt.Fprintln(os.Stderr, err) - return false + str := "field %s does not have a long tag" + return false, mkErr(str, field) } // The user has the option to set the flag in either the config @@ -751,9 +780,10 @@ func ValidateConfig(cfg Config, usageMessage string, ) return (fileOption != nil && fileOption.IsSet()) || - (fileOptionNested != nil && fileOptionNested.IsSet()) || - (flagOption != nil && flagOption.IsSet()) || - (flagOptionNested != nil && flagOptionNested.IsSet()) + (fileOptionNested != nil && fileOptionNested.IsSet()) || + (flagOption != nil && flagOption.IsSet()) || + (flagOptionNested != nil && flagOptionNested.IsSet()), + nil } // As soon as we're done parsing configuration options, ensure all paths @@ -800,40 +830,28 @@ func ValidateConfig(cfg Config, usageMessage string, // Ensure that the user didn't attempt to specify negative values for // any of the autopilot params. if cfg.Autopilot.MaxChannels < 0 { - str := "%s: autopilot.maxchannels must be non-negative" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.maxchannels must be non-negative" + return nil, mkErr(str) } if cfg.Autopilot.Allocation < 0 { - str := "%s: autopilot.allocation must be non-negative" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.allocation must be non-negative" + return nil, mkErr(str) } if cfg.Autopilot.MinChannelSize < 0 { - str := "%s: autopilot.minchansize must be non-negative" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.minchansize must be non-negative" + return nil, mkErr(str) } if cfg.Autopilot.MaxChannelSize < 0 { - str := "%s: autopilot.maxchansize must be non-negative" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.maxchansize must be non-negative" + return nil, mkErr(str) } if cfg.Autopilot.MinConfs < 0 { - str := "%s: autopilot.minconfs must be non-negative" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.minconfs must be non-negative" + return nil, mkErr(str) } if cfg.Autopilot.ConfTarget < 1 { - str := "%s: autopilot.conftarget must be positive" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.conftarget must be positive" + return nil, mkErr(str) } // Ensure that the specified values for the min and max channel size @@ -846,7 +864,7 @@ func ValidateConfig(cfg Config, usageMessage string, } if _, err := validateAtplCfg(cfg.Autopilot); err != nil { - return nil, err + return nil, mkErr("error validating autopilot: %v", err) } // Ensure that --maxchansize is properly handled when set by user. @@ -865,32 +883,33 @@ func ValidateConfig(cfg Config, usageMessage string, // Ensure that the user specified values for the min and max channel // size make sense. if cfg.MaxChanSize < cfg.MinChanSize { - return nil, fmt.Errorf("invalid channel size parameters: "+ - "max channel size %v, must be no less than min chan size %v", - cfg.MaxChanSize, cfg.MinChanSize, + return nil, mkErr("invalid channel size parameters: "+ + "max channel size %v, must be no less than min chan "+ + "size %v", cfg.MaxChanSize, cfg.MinChanSize, ) } // Don't allow superflous --maxchansize greater than // BOLT 02 soft-limit for non-wumbo channel - if !cfg.ProtocolOptions.Wumbo() && cfg.MaxChanSize > int64(MaxFundingAmount) { - return nil, fmt.Errorf("invalid channel size parameters: "+ - "maximum channel size %v is greater than maximum non-wumbo"+ - " channel size %v", - cfg.MaxChanSize, MaxFundingAmount, + if !cfg.ProtocolOptions.Wumbo() && + cfg.MaxChanSize > int64(MaxFundingAmount) { + + return nil, mkErr("invalid channel size parameters: "+ + "maximum channel size %v is greater than maximum "+ + "non-wumbo channel size %v", cfg.MaxChanSize, + MaxFundingAmount, ) } // Ensure a valid max channel fee allocation was set. if cfg.MaxChannelFeeAllocation <= 0 || cfg.MaxChannelFeeAllocation > 1 { - return nil, fmt.Errorf("invalid max channel fee allocation: "+ - "%v, must be within (0, 1]", - cfg.MaxChannelFeeAllocation) + return nil, mkErr("invalid max channel fee allocation: %v, "+ + "must be within (0, 1]", cfg.MaxChannelFeeAllocation) } if cfg.MaxCommitFeeRateAnchors < 1 { - return nil, fmt.Errorf("invalid max commit fee rate anchors: "+ - "%v, must be at least 1 sat/vbyte", + return nil, mkErr("invalid max commit fee rate anchors: %v, "+ + "must be at least 1 sat/vByte", cfg.MaxCommitFeeRateAnchors) } @@ -912,7 +931,7 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.net.ResolveTCPAddr, ) if err != nil { - return nil, err + return nil, mkErr("error parsing tor dns: %v", err) } cfg.Tor.DNS = dns.String() } @@ -922,25 +941,24 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.net.ResolveTCPAddr, ) if err != nil { - return nil, err + return nil, mkErr("error parsing tor control address: %v", err) } cfg.Tor.Control = control.String() // Ensure that tor socks host:port is not equal to tor control // host:port. This would lead to lnd not starting up properly. if cfg.Tor.SOCKS == cfg.Tor.Control { - str := "%s: tor.socks and tor.control can not use " + - "the same host:port" - return nil, fmt.Errorf(str, funcName) + str := "tor.socks and tor.control can not us the same host:port" + return nil, mkErr(str) } switch { case cfg.Tor.V2 && cfg.Tor.V3: - return nil, errors.New("either tor.v2 or tor.v3 can be set, " + + return nil, mkErr("either tor.v2 or tor.v3 can be set, " + "but not both") case cfg.DisableListen && (cfg.Tor.V2 || cfg.Tor.V3): - return nil, errors.New("listening must be enabled when " + - "enabling inbound connections over Tor") + return nil, mkErr("listening must be enabled when enabling " + + "inbound connections over Tor") } if cfg.Tor.PrivateKeyPath == "" { @@ -960,11 +978,13 @@ func ValidateConfig(cfg Config, usageMessage string, switch { case cfg.Tor.V2: cfg.Tor.WatchtowerKeyPath = filepath.Join( - cfg.Watchtower.TowerDir, defaultTorV2PrivateKeyFilename, + cfg.Watchtower.TowerDir, + defaultTorV2PrivateKeyFilename, ) case cfg.Tor.V3: cfg.Tor.WatchtowerKeyPath = filepath.Join( - cfg.Watchtower.TowerDir, defaultTorV3PrivateKeyFilename, + cfg.Watchtower.TowerDir, + defaultTorV3PrivateKeyFilename, ) } } @@ -984,11 +1004,11 @@ func ValidateConfig(cfg Config, usageMessage string, } if cfg.DisableListen && cfg.NAT { - return nil, errors.New("NAT traversal cannot be used when " + + return nil, mkErr("NAT traversal cannot be used when " + "listening is disabled") } if cfg.NAT && len(cfg.ExternalHosts) != 0 { - return nil, errors.New("NAT support and externalhosts are " + + return nil, mkErr("NAT support and externalhosts are " + "mutually exclusive, only one should be selected") } @@ -996,20 +1016,22 @@ func ValidateConfig(cfg Config, usageMessage string, switch { // At this moment, multiple active chains are not supported. case cfg.Litecoin.Active && cfg.Bitcoin.Active: - str := "%s: Currently both Bitcoin and Litecoin cannot be " + + str := "Currently both Bitcoin and Litecoin cannot be " + "active together" - return nil, fmt.Errorf(str, funcName) + return nil, mkErr(str) // Either Bitcoin must be active, or Litecoin must be active. // Otherwise, we don't know which chain we're on. case !cfg.Bitcoin.Active && !cfg.Litecoin.Active: - return nil, fmt.Errorf("%s: either bitcoin.active or "+ - "litecoin.active must be set to 1 (true)", funcName) + return nil, mkErr("either bitcoin.active or " + + "litecoin.active must be set to 1 (true)") case cfg.Litecoin.Active: - err := cfg.Litecoin.Validate(minTimeLockDelta, funding.MinLtcRemoteDelay) + err := cfg.Litecoin.Validate( + minTimeLockDelta, funding.MinLtcRemoteDelay, + ) if err != nil { - return nil, err + return nil, mkErr("error validating litecoin: %v", err) } // Multiple networks can't be selected simultaneously. Count @@ -1034,25 +1056,22 @@ func ValidateConfig(cfg Config, usageMessage string, ltcParams = chainreg.LitecoinSimNetParams } if cfg.Litecoin.SigNet { - return nil, fmt.Errorf("%s: litecoin.signet is not "+ - "supported", funcName) + return nil, mkErr("litecoin.signet is not supported") } if numNets > 1 { - str := "%s: The mainnet, testnet, and simnet params " + + str := "The mainnet, testnet, and simnet params " + "can't be used together -- choose one of the " + "three" - err := fmt.Errorf(str, funcName) - return nil, err + return nil, mkErr(str) } // The target network must be provided, otherwise, we won't // know how to initialize the daemon. if numNets == 0 { - str := "%s: either --litecoin.mainnet, or " + + str := "either --litecoin.mainnet, or " + "litecoin.testnet must be specified" - err := fmt.Errorf(str, funcName) - return nil, err + return nil, mkErr(str) } // The litecoin chain is the current active chain. However @@ -1063,36 +1082,39 @@ func ValidateConfig(cfg Config, usageMessage string, switch cfg.Litecoin.Node { case "ltcd": - err := parseRPCParams(cfg.Litecoin, cfg.LtcdMode, - chainreg.LitecoinChain, funcName, cfg.ActiveNetParams) + err := parseRPCParams( + cfg.Litecoin, cfg.LtcdMode, + chainreg.LitecoinChain, cfg.ActiveNetParams, + ) if err != nil { - err := fmt.Errorf("unable to load RPC "+ + return nil, mkErr("unable to load RPC "+ "credentials for ltcd: %v", err) - return nil, err } case "litecoind": if cfg.Litecoin.SimNet { - return nil, fmt.Errorf("%s: litecoind does not "+ - "support simnet", funcName) + return nil, mkErr("litecoind does not " + + "support simnet") } - err := parseRPCParams(cfg.Litecoin, cfg.LitecoindMode, - chainreg.LitecoinChain, funcName, cfg.ActiveNetParams) + err := parseRPCParams( + cfg.Litecoin, cfg.LitecoindMode, + chainreg.LitecoinChain, cfg.ActiveNetParams, + ) if err != nil { - err := fmt.Errorf("unable to load RPC "+ + return nil, mkErr("unable to load RPC "+ "credentials for litecoind: %v", err) - return nil, err } default: - str := "%s: only ltcd and litecoind mode supported for " + + str := "only ltcd and litecoind mode supported for " + "litecoin at this time" - return nil, fmt.Errorf(str, funcName) + return nil, mkErr(str) } - cfg.Litecoin.ChainDir = filepath.Join(cfg.DataDir, - defaultChainSubDirname, - chainreg.LitecoinChain.String()) + cfg.Litecoin.ChainDir = filepath.Join( + cfg.DataDir, defaultChainSubDirname, + chainreg.LitecoinChain.String(), + ) - // Finally we'll register the litecoin chain as our current + // Finally, we'll register the litecoin chain as our current // primary chain. cfg.registeredChains.RegisterPrimaryChain(chainreg.LitecoinChain) MaxFundingAmount = funding.MaxLtcFundingAmount @@ -1133,9 +1155,9 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.Bitcoin.SigNetChallenge, ) if err != nil { - return nil, fmt.Errorf("%s: Invalid "+ + return nil, mkErr("Invalid "+ "signet challenge, hex decode "+ - "failed: %v", funcName, err) + "failed: %v", err) } sigNetChallenge = challenge } @@ -1158,66 +1180,66 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.ActiveNetParams.Params = &chainParams } if numNets > 1 { - str := "%s: The mainnet, testnet, regtest, and " + - "simnet params can't be used together -- " + - "choose one of the four" - err := fmt.Errorf(str, funcName) - return nil, err + str := "The mainnet, testnet, regtest, and simnet " + + "params can't be used together -- choose one " + + "of the four" + return nil, mkErr(str) } // The target network must be provided, otherwise, we won't // know how to initialize the daemon. if numNets == 0 { - str := "%s: either --bitcoin.mainnet, or " + - "bitcoin.testnet, bitcoin.simnet, or bitcoin.regtest " + + str := "either --bitcoin.mainnet, or bitcoin.testnet," + + "bitcoin.simnet, or bitcoin.regtest " + "must be specified" - err := fmt.Errorf(str, funcName) - return nil, err + return nil, mkErr(str) } - err := cfg.Bitcoin.Validate(minTimeLockDelta, funding.MinBtcRemoteDelay) + err := cfg.Bitcoin.Validate( + minTimeLockDelta, funding.MinBtcRemoteDelay, + ) if err != nil { - return nil, err + return nil, mkErr("error validating bitcoin params: %v", + err) } switch cfg.Bitcoin.Node { case "btcd": err := parseRPCParams( - cfg.Bitcoin, cfg.BtcdMode, chainreg.BitcoinChain, funcName, - cfg.ActiveNetParams, + cfg.Bitcoin, cfg.BtcdMode, + chainreg.BitcoinChain, cfg.ActiveNetParams, ) if err != nil { - err := fmt.Errorf("unable to load RPC "+ + return nil, mkErr("unable to load RPC "+ "credentials for btcd: %v", err) - return nil, err } case "bitcoind": if cfg.Bitcoin.SimNet { - return nil, fmt.Errorf("%s: bitcoind does not "+ - "support simnet", funcName) + return nil, mkErr("bitcoind does not " + + "support simnet") } err := parseRPCParams( - cfg.Bitcoin, cfg.BitcoindMode, chainreg.BitcoinChain, funcName, - cfg.ActiveNetParams, + cfg.Bitcoin, cfg.BitcoindMode, + chainreg.BitcoinChain, cfg.ActiveNetParams, ) if err != nil { - err := fmt.Errorf("unable to load RPC "+ + return nil, mkErr("unable to load RPC "+ "credentials for bitcoind: %v", err) - return nil, err } case "neutrino": // No need to get RPC parameters. default: - str := "%s: only btcd, bitcoind, and neutrino mode " + + str := "only btcd, bitcoind, and neutrino mode " + "supported for bitcoin at this time" - return nil, fmt.Errorf(str, funcName) + return nil, mkErr(str) } - cfg.Bitcoin.ChainDir = filepath.Join(cfg.DataDir, - defaultChainSubDirname, - chainreg.BitcoinChain.String()) + cfg.Bitcoin.ChainDir = filepath.Join( + cfg.DataDir, defaultChainSubDirname, + chainreg.BitcoinChain.String(), + ) // Finally we'll register the bitcoin chain as our current // primary chain. @@ -1227,28 +1249,20 @@ func ValidateConfig(cfg Config, usageMessage string, // Ensure that the user didn't attempt to specify negative values for // any of the autopilot params. if cfg.Autopilot.MaxChannels < 0 { - str := "%s: autopilot.maxchannels must be non-negative" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.maxchannels must be non-negative" + return nil, mkErr(str) } if cfg.Autopilot.Allocation < 0 { - str := "%s: autopilot.allocation must be non-negative" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.allocation must be non-negative" + return nil, mkErr(str) } if cfg.Autopilot.MinChannelSize < 0 { - str := "%s: autopilot.minchansize must be non-negative" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.minchansize must be non-negative" + return nil, mkErr(str) } if cfg.Autopilot.MaxChannelSize < 0 { - str := "%s: autopilot.maxchansize must be non-negative" - err := fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "autopilot.maxchansize must be non-negative" + return nil, mkErr(str) } // Ensure that the specified values for the min and max channel size @@ -1270,19 +1284,13 @@ func ValidateConfig(cfg Config, usageMessage string, // Determine if the port is valid. profilePort, err := strconv.Atoi(hostPort) if err != nil || profilePort < 1024 || profilePort > 65535 { - err = fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - _, _ = fmt.Fprintln(os.Stderr, usageMessage) - return nil, err + return nil, &usageError{mkErr(str)} } } else { // Try to parse Profile as a port. profilePort, err := strconv.Atoi(cfg.Profile) if err != nil || profilePort < 1024 || profilePort > 65535 { - err = fmt.Errorf(str, funcName) - _, _ = fmt.Fprintln(os.Stderr, err) - _, _ = fmt.Fprintln(os.Stderr, usageMessage) - return nil, err + return nil, &usageError{mkErr(str)} } // Since the user just set a port, we will serve debugging @@ -1334,14 +1342,15 @@ func ValidateConfig(cfg Config, usageMessage string, // Append the network type to the log directory so it is "namespaced" // per network in the same fashion as the data directory. - cfg.LogDir = filepath.Join(cfg.LogDir, - cfg.registeredChains.PrimaryChain().String(), - lncfg.NormalizeNetwork(cfg.ActiveNetParams.Name)) + cfg.LogDir = filepath.Join( + cfg.LogDir, cfg.registeredChains.PrimaryChain().String(), + lncfg.NormalizeNetwork(cfg.ActiveNetParams.Name), + ) // A log writer must be passed in, otherwise we can't function and would // run into a panic later on. if cfg.LogWriter == nil { - return nil, fmt.Errorf("log writer missing in config") + return nil, mkErr("log writer missing in config") } // Special show command to list supported subsystems and exit. @@ -1358,19 +1367,15 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.MaxLogFileSize, cfg.MaxLogFiles, ) if err != nil { - str := "%s: log rotation setup failed: %v" - err = fmt.Errorf(str, funcName, err.Error()) - _, _ = fmt.Fprintln(os.Stderr, err) - return nil, err + str := "log rotation setup failed: %v" + return nil, mkErr(str, err) } // Parse, validate, and set debug log level(s). err = build.ParseAndSetDebugLevels(cfg.DebugLevel, cfg.LogWriter) if err != nil { - err = fmt.Errorf("%s: %v", funcName, err.Error()) - _, _ = fmt.Fprintln(os.Stderr, err) - _, _ = fmt.Fprintln(os.Stderr, usageMessage) - return nil, err + str := "error parsing debug level: %v" + return nil, &usageError{mkErr(str, err)} } // At least one RPCListener is required. So listen on localhost per @@ -1406,7 +1411,7 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.net.ResolveTCPAddr, ) if err != nil { - return nil, err + return nil, mkErr("error normalizing RPC listen addrs: %v", err) } // Add default port to all REST listener addresses if needed and remove @@ -1416,25 +1421,25 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.net.ResolveTCPAddr, ) if err != nil { - return nil, err + return nil, mkErr("error normalizing REST listen addrs: %v", err) } switch { // The no seed backup and auto unlock are mutually exclusive. case cfg.NoSeedBackup && cfg.WalletUnlockPasswordFile != "": - return nil, fmt.Errorf("cannot set noseedbackup and " + + return nil, mkErr("cannot set noseedbackup and " + "wallet-unlock-password-file at the same time") // The "allow-create" flag cannot be set without the auto unlock file. case cfg.WalletUnlockAllowCreate && cfg.WalletUnlockPasswordFile == "": - return nil, fmt.Errorf("cannot set wallet-unlock-allow-create " + + return nil, mkErr("cannot set wallet-unlock-allow-create " + "without wallet-unlock-password-file") // If a password file was specified, we need it to exist. case cfg.WalletUnlockPasswordFile != "" && !lnrpc.FileExists(cfg.WalletUnlockPasswordFile): - return nil, fmt.Errorf("wallet unlock password file %s does "+ + return nil, mkErr("wallet unlock password file %s does "+ "not exist", cfg.WalletUnlockPasswordFile) } @@ -1446,7 +1451,8 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.RPCListeners, !cfg.NoMacaroons, true, ) if err != nil { - return nil, err + return nil, mkErr("error enforcing safe authentication on "+ + "RPC ports: %v", err) } if cfg.DisableRest { @@ -1457,7 +1463,8 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.RESTListeners, !cfg.NoMacaroons, !cfg.DisableRestTLS, ) if err != nil { - return nil, err + return nil, mkErr("error enforcing safe "+ + "authentication on REST ports: %v", err) } } @@ -1475,7 +1482,8 @@ func ValidateConfig(cfg Config, usageMessage string, cfg.net.ResolveTCPAddr, ) if err != nil { - return nil, err + return nil, mkErr("error normalizing p2p listen "+ + "addrs: %v", err) } // Add default port to all external IP addresses if needed and remove @@ -1493,10 +1501,9 @@ func ValidateConfig(cfg Config, usageMessage string, // that. for _, p2pListener := range cfg.Listeners { if lncfg.IsUnix(p2pListener) { - err := fmt.Errorf("unix socket addresses cannot be "+ - "used for the p2p connection listener: %s", - p2pListener) - return nil, err + return nil, mkErr("unix socket addresses "+ + "cannot be used for the p2p "+ + "connection listener: %s", p2pListener) } } } @@ -1504,15 +1511,18 @@ func ValidateConfig(cfg Config, usageMessage string, // Ensure that the specified minimum backoff is below or equal to the // maximum backoff. if cfg.MinBackoff > cfg.MaxBackoff { - return nil, fmt.Errorf("maxbackoff must be greater than " + - "minbackoff") + return nil, mkErr("maxbackoff must be greater than minbackoff") } // Newer versions of lnd added a new sub-config for bolt-specific - // parameters. However we want to also allow existing users to use the + // parameters. However, we want to also allow existing users to use the // value on the top-level config. If the outer config value is set, // then we'll use that directly. - if isSet("SyncFreelist") { + flagSet, err := isSet("SyncFreelist") + if err != nil { + return nil, mkErr("error parsing freelist sync flag: %v", err) + } + if flagSet { cfg.DB.Bolt.NoFreelistSync = !cfg.SyncFreelist } @@ -1520,13 +1530,13 @@ func ValidateConfig(cfg Config, usageMessage string, // than the protocol maximum. maxRemoteHtlcs := uint16(input.MaxHTLCNumber / 2) if cfg.DefaultRemoteMaxHtlcs > maxRemoteHtlcs { - return nil, fmt.Errorf("default-remote-max-htlcs (%v) must be "+ + return nil, mkErr("default-remote-max-htlcs (%v) must be "+ "less than %v", cfg.DefaultRemoteMaxHtlcs, maxRemoteHtlcs) } if err := cfg.Gossip.Parse(); err != nil { - return nil, err + return nil, mkErr("error parsing gossip syncer: %v", err) } // Log a warning if our expiry delta is not greater than our incoming @@ -1560,11 +1570,11 @@ func ValidateConfig(cfg Config, usageMessage string, // the wallet. _, err = parseHexColor(cfg.Color) if err != nil { - return nil, fmt.Errorf("unable to parse node color: %v", err) + return nil, mkErr("unable to parse node color: %v", err) } // All good, return the sanitized result. - return &cfg, err + return &cfg, nil } // graphDatabaseDir returns the default directory where the local bolt graph db @@ -1636,8 +1646,7 @@ func CleanAndExpandPath(path string) string { } func parseRPCParams(cConfig *lncfg.Chain, nodeConfig interface{}, - net chainreg.ChainCode, funcName string, - netParams chainreg.BitcoinNetParams) error { // nolint:unparam + net chainreg.ChainCode, netParams chainreg.BitcoinNetParams) error { // First, we'll check our node config to make sure the RPC parameters // were set correctly. We'll also determine the path to the conf file @@ -1726,9 +1735,8 @@ func parseRPCParams(cConfig *lncfg.Chain, nodeConfig interface{}, // the RPC credentials from the configuration. So if lnd wasn't // specified the parameters, then we won't be able to start. if cConfig.SimNet { - str := "%v: rpcuser and rpcpass must be set to your btcd " + - "node's RPC parameters for simnet mode" - return fmt.Errorf(str, funcName) + return fmt.Errorf("rpcuser and rpcpass must be set to your " + + "btcd node's RPC parameters for simnet mode") } fmt.Println("Attempting automatic RPC configuration to " + daemonName) @@ -1739,19 +1747,18 @@ func parseRPCParams(cConfig *lncfg.Chain, nodeConfig interface{}, nConf := nodeConfig.(*lncfg.Btcd) rpcUser, rpcPass, err := extractBtcdRPCParams(confFile) if err != nil { - return fmt.Errorf("unable to extract RPC credentials:"+ - " %v, cannot start w/o RPC connection", - err) + return fmt.Errorf("unable to extract RPC credentials: "+ + "%v, cannot start w/o RPC connection", err) } nConf.RPCUser, nConf.RPCPass = rpcUser, rpcPass + case "bitcoind", "litecoind": nConf := nodeConfig.(*lncfg.Bitcoind) rpcUser, rpcPass, zmqBlockHost, zmqTxHost, err := extractBitcoindRPCParams(netParams.Params.Name, confFile) if err != nil { - return fmt.Errorf("unable to extract RPC credentials:"+ - " %v, cannot start w/o RPC connection", - err) + return fmt.Errorf("unable to extract RPC credentials: "+ + "%v, cannot start w/o RPC connection", err) } nConf.RPCUser, nConf.RPCPass = rpcUser, rpcPass nConf.ZMQPubRawBlock, nConf.ZMQPubRawTx = zmqBlockHost, zmqTxHost diff --git a/config_builder.go b/config_builder.go index b30a74f6d..f1a3f1c40 100644 --- a/config_builder.go +++ b/config_builder.go @@ -601,7 +601,7 @@ func (d *DefaultWalletImpl) BuildChainControl( *walletConfig, partialChainControl.Cfg.BlockCache, ) if err != nil { - fmt.Printf("unable to create wallet controller: %v\n", err) + err := fmt.Errorf("unable to create wallet controller: %v", err) d.logger.Error(err) return nil, nil, err } @@ -676,7 +676,7 @@ func (d *RPCSignerWalletImpl) BuildChainControl( *walletConfig, partialChainControl.Cfg.BlockCache, ) if err != nil { - fmt.Printf("unable to create wallet controller: %v\n", err) + err := fmt.Errorf("unable to create wallet controller: %v", err) d.logger.Error(err) return nil, nil, err } @@ -691,7 +691,8 @@ func (d *RPCSignerWalletImpl) BuildChainControl( rpcwallet.DefaultRPCTimeout, ) if err != nil { - fmt.Printf("unable to create RPC remote signing wallet %v", err) + err := fmt.Errorf("unable to create RPC remote signing wallet "+ + "%v", err) d.logger.Error(err) return nil, nil, err } diff --git a/lnd.go b/lnd.go index 37df3ef4b..4ddf0a56f 100644 --- a/lnd.go +++ b/lnd.go @@ -403,12 +403,14 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, if cfg.Tor.Active { if cfg.Tor.SkipProxyForClearNetTargets { - srvrLog.Info("Onion services are accessible via Tor! NOTE: " + - "Traffic to clearnet services is not routed via Tor.") + srvrLog.Info("Onion services are accessible via Tor! " + + "NOTE: Traffic to clearnet services is not " + + "routed via Tor.") } else { srvrLog.Infof("Proxying all network traffic via Tor "+ - "(stream_isolation=%v)! NOTE: Ensure the backend node "+ - "is proxying over Tor as well", cfg.Tor.StreamIsolation) + "(stream_isolation=%v)! NOTE: Ensure the "+ + "backend node is proxying over Tor as well", + cfg.Tor.StreamIsolation) } } @@ -423,13 +425,15 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, // Start the tor controller before giving it to any other subsystems. if err := torController.Start(); err != nil { - err := fmt.Errorf("unable to initialize tor controller: %v", err) + err := fmt.Errorf("unable to initialize tor "+ + "controller: %v", err) ltndLog.Error(err) return err } defer func() { if err := torController.Stop(); err != nil { - ltndLog.Errorf("error stopping tor controller: %v", err) + ltndLog.Errorf("error stopping tor "+ + "controller: %v", err) } }() } From 602d2065ebf8527a6a1007dd96dc7e37a3c8c6e0 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 8 Nov 2021 11:58:52 +0100 Subject: [PATCH 4/7] lnd: make sure error is logged to file Fixes #5936. This commit makes sure every error that causes the Main() function to return is logged to the log file in addition to being printed to stderr. --- lnd.go | 151 ++++++++++++++++++++++----------------------------------- 1 file changed, 59 insertions(+), 92 deletions(-) diff --git a/lnd.go b/lnd.go index 4ddf0a56f..dda5b939a 100644 --- a/lnd.go +++ b/lnd.go @@ -70,7 +70,9 @@ const ( // // NOTE: This should only be called after the RPCListener has signaled it is // ready. -func AdminAuthOptions(cfg *Config, skipMacaroons bool) ([]grpc.DialOption, error) { +func AdminAuthOptions(cfg *Config, skipMacaroons bool) ([]grpc.DialOption, + error) { + creds, err := credentials.NewClientTLSFromFile(cfg.TLSCertPath, "") if err != nil { return nil, fmt.Errorf("unable to read TLS cert: %v", err) @@ -107,8 +109,8 @@ func AdminAuthOptions(cfg *Config, skipMacaroons bool) ([]grpc.DialOption, error return opts, nil } -// ListenerWithSignal is a net.Listener that has an additional Ready channel that -// will be closed when a server starts listening. +// ListenerWithSignal is a net.Listener that has an additional Ready channel +// that will be closed when a server starts listening. type ListenerWithSignal struct { net.Listener @@ -148,10 +150,16 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, } }() + mkErr := func(format string, args ...interface{}) error { + ltndLog.Errorf("Shutting down because error in main "+ + "method: "+format, args...) + return fmt.Errorf(format, args...) + } + // Show version at startup. - ltndLog.Infof("Version: %s commit=%s, build=%s, logging=%s, debuglevel=%s", - build.Version(), build.Commit, build.Deployment, - build.LoggingType, cfg.DebugLevel) + ltndLog.Infof("Version: %s commit=%s, build=%s, logging=%s, "+ + "debuglevel=%s", build.Version(), build.Commit, + build.Deployment, build.LoggingType, cfg.DebugLevel) var network string switch { @@ -191,10 +199,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, if cfg.CPUProfile != "" { f, err := os.Create(cfg.CPUProfile) if err != nil { - err := fmt.Errorf("unable to create CPU profile: %v", - err) - ltndLog.Error(err) - return err + return mkErr("unable to create CPU profile: %v", err) } pprof.StartCPUProfile(f) defer f.Close() @@ -209,15 +214,13 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, // needs to be done early and once during the startup process, before // any DB access. if err := cfg.DB.Init(ctx, cfg.graphDatabaseDir()); err != nil { - return err + return mkErr("error initializing DBs: %v", err) } // Only process macaroons if --no-macaroons isn't set. serverOpts, restDialOpts, restListen, cleanUp, err := getTLSConfig(cfg) if err != nil { - err := fmt.Errorf("unable to load TLS credentials: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to load TLS credentials: %v", err) } defer cleanUp() @@ -233,9 +236,8 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, // connections. lis, err := lncfg.ListenOnAddress(grpcEndpoint) if err != nil { - ltndLog.Errorf("unable to listen on %s", - grpcEndpoint) - return err + return mkErr("unable to listen on %s: %v", + grpcEndpoint, err) } defer lis.Close() @@ -254,7 +256,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, rpcsLog, cfg.NoMacaroons, cfg.RPCMiddleware.Mandatory, ) if err := interceptorChain.Start(); err != nil { - return err + return mkErr("error starting interceptor chain: %v", err) } defer func() { err := interceptorChain.Stop() @@ -279,14 +281,14 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, rpcServer := newRPCServer(cfg, interceptorChain, implCfg, interceptor) err = rpcServer.RegisterWithGrpcServer(grpcServer) if err != nil { - return err + return mkErr("error registering gRPC server: %v", err) } // Now that both the WalletUnlocker and LightningService have been // registered with the GRPC server, we can start listening. err = startGrpcListen(cfg, grpcServer, grpcListeners) if err != nil { - return err + return mkErr("error starting gRPC listener: %v", err) } // Now start the REST proxy for our gRPC server above. We'll ensure @@ -297,7 +299,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, cfg, rpcServer, restDialOpts, restListen, ) if err != nil { - return err + return mkErr("error starting REST proxy: %v", err) } defer stopProxy() @@ -341,8 +343,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, cfg.Cluster.ID) if err := leaderElector.Campaign(electionCtx); err != nil { - ltndLog.Errorf("Leadership campaign failed: %v", err) - return err + return mkErr("leadership campaign failed: %v", err) } elected = true @@ -355,7 +356,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, ltndLog.Infof("%v, exiting", err) return nil case err != nil: - return fmt.Errorf("unable to open databases: %v", err) + return mkErr("unable to open databases: %v", err) } defer cleanUp() @@ -364,7 +365,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, ctx, dbs, interceptorChain, grpcListeners, ) if err != nil { - return fmt.Errorf("error creating wallet config: %v", err) + return mkErr("error creating wallet config: %v", err) } defer cleanUp() @@ -373,7 +374,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, partialChainControl, walletConfig, ) if err != nil { - return fmt.Errorf("error loading chain control: %v", err) + return mkErr("error loading chain control: %v", err) } defer cleanUp() @@ -392,9 +393,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, }, ) if err != nil { - err := fmt.Errorf("error deriving node key: %v", err) - ltndLog.Error(err) - return err + return mkErr("error deriving node key: %v", err) } if cfg.Tor.StreamIsolation && cfg.Tor.SkipProxyForClearNetTargets { @@ -414,21 +413,21 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, } } - // If tor is active and either v2 or v3 onion services have been specified, - // make a tor controller and pass it into both the watchtower server and - // the regular lnd server. + // If tor is active and either v2 or v3 onion services have been + // specified, make a tor controller and pass it into both the watchtower + // server and the regular lnd server. var torController *tor.Controller if cfg.Tor.Active && (cfg.Tor.V2 || cfg.Tor.V3) { torController = tor.NewController( - cfg.Tor.Control, cfg.Tor.TargetIPAddress, cfg.Tor.Password, + cfg.Tor.Control, cfg.Tor.TargetIPAddress, + cfg.Tor.Password, ) - // Start the tor controller before giving it to any other subsystems. + // Start the tor controller before giving it to any other + // subsystems. if err := torController.Start(); err != nil { - err := fmt.Errorf("unable to initialize tor "+ - "controller: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to initialize tor controller: %v", + err) } defer func() { if err := torController.Stop(); err != nil { @@ -447,9 +446,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, }, ) if err != nil { - err := fmt.Errorf("error deriving tower key: %v", err) - ltndLog.Error(err) - return err + return mkErr("error deriving tower key: %v", err) } wtCfg := &watchtower.Config{ @@ -470,8 +467,8 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, ChainHash: *cfg.ActiveNetParams.GenesisHash, } - // If there is a tor controller (user wants auto hidden services), then - // store a pointer in the watchtower config. + // If there is a tor controller (user wants auto hidden + // services), then store a pointer in the watchtower config. if torController != nil { wtCfg.TorController = torController wtCfg.WatchtowerKeyPath = cfg.Tor.WatchtowerKeyPath @@ -484,19 +481,16 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, } } - wtConfig, err := cfg.Watchtower.Apply(wtCfg, lncfg.NormalizeAddresses) + wtConfig, err := cfg.Watchtower.Apply( + wtCfg, lncfg.NormalizeAddresses, + ) if err != nil { - err := fmt.Errorf("unable to configure watchtower: %v", - err) - ltndLog.Error(err) - return err + return mkErr("unable to configure watchtower: %v", err) } tower, err = watchtower.New(wtConfig) if err != nil { - err := fmt.Errorf("unable to create watchtower: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to create watchtower: %v", err) } } @@ -511,9 +505,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, chainedAcceptor, torController, ) if err != nil { - err := fmt.Errorf("unable to create server: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to create server: %v", err) } // Set up an autopilot manager from the current config. This will be @@ -524,21 +516,15 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, cfg.ActiveNetParams, ) if err != nil { - err := fmt.Errorf("unable to initialize autopilot: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to initialize autopilot: %v", err) } atplManager, err := autopilot.NewManager(atplCfg) if err != nil { - err := fmt.Errorf("unable to create autopilot manager: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to create autopilot manager: %v", err) } if err := atplManager.Start(); err != nil { - err := fmt.Errorf("unable to start autopilot manager: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to start autopilot manager: %v", err) } defer atplManager.Stop() @@ -549,14 +535,10 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, atplManager, server.invoices, tower, chainedAcceptor, ) if err != nil { - err := fmt.Errorf("unable to add deps to RPC server: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to add deps to RPC server: %v", err) } if err := rpcServer.Start(); err != nil { - err := fmt.Errorf("unable to start RPC server: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to start RPC server: %v", err) } defer rpcServer.Stop() @@ -564,7 +546,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, interceptorChain.SetRPCActive() if err := interceptor.Notifier.NotifyReady(true); err != nil { - return err + return mkErr("error notifying ready: %v", err) } // We'll wait until we're fully synced to continue the start up of the @@ -573,10 +555,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, // funds. _, bestHeight, err := activeChainControl.ChainIO.GetBestBlock() if err != nil { - err := fmt.Errorf("unable to determine chain tip: %v", - err) - ltndLog.Error(err) - return err + return mkErr("unable to determine chain tip: %v", err) } ltndLog.Infof("Waiting for chain backend to finish sync, "+ @@ -589,10 +568,8 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, synced, _, err := activeChainControl.Wallet.IsSynced() if err != nil { - err := fmt.Errorf("unable to determine if "+ - "wallet is synced: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to determine if wallet is "+ + "synced: %v", err) } if synced { @@ -604,10 +581,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, _, bestHeight, err = activeChainControl.ChainIO.GetBestBlock() if err != nil { - err := fmt.Errorf("unable to determine chain tip: %v", - err) - ltndLog.Error(err) - return err + return mkErr("unable to determine chain tip: %v", err) } ltndLog.Infof("Chain backend is fully synced (end_height=%v)!", @@ -616,9 +590,7 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, // With all the relevant chains initialized, we can finally start the // server itself. if err := server.Start(); err != nil { - err := fmt.Errorf("unable to start server: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to start server: %v", err) } defer server.Stop() @@ -630,18 +602,13 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, // stopped together with the autopilot service. if cfg.Autopilot.Active { if err := atplManager.StartAgent(); err != nil { - err := fmt.Errorf("unable to start autopilot agent: %v", - err) - ltndLog.Error(err) - return err + return mkErr("unable to start autopilot agent: %v", err) } } if cfg.Watchtower.Active { if err := tower.Start(); err != nil { - err := fmt.Errorf("unable to start watchtower: %v", err) - ltndLog.Error(err) - return err + return mkErr("unable to start watchtower: %v", err) } defer tower.Stop() } From 41c579fd09fb2ee4173b21807f49b0d48856341b Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 8 Nov 2021 12:00:58 +0100 Subject: [PATCH 5/7] multi: run go fmt with go 1.17 --- kvdb/kvdb_no_postgres.go | 1 + kvdb/kvdb_postgres.go | 1 + kvdb/postgres/db.go | 1 + kvdb/postgres/db_test.go | 1 + kvdb/postgres/driver.go | 1 + kvdb/postgres/fixture.go | 1 + kvdb/postgres/readwrite_bucket.go | 1 + kvdb/postgres/readwrite_cursor.go | 1 + kvdb/postgres/readwrite_tx.go | 1 + kvdb/postgres_test.go | 83 ++++++++++++++++--------------- macaroons/service.go | 1 + 11 files changed, 52 insertions(+), 41 deletions(-) diff --git a/kvdb/kvdb_no_postgres.go b/kvdb/kvdb_no_postgres.go index d581c2b55..740f6a276 100644 --- a/kvdb/kvdb_no_postgres.go +++ b/kvdb/kvdb_no_postgres.go @@ -1,3 +1,4 @@ +//go:build !kvdb_postgres // +build !kvdb_postgres package kvdb diff --git a/kvdb/kvdb_postgres.go b/kvdb/kvdb_postgres.go index 023a51840..b0d2edee8 100644 --- a/kvdb/kvdb_postgres.go +++ b/kvdb/kvdb_postgres.go @@ -1,3 +1,4 @@ +//go:build kvdb_postgres // +build kvdb_postgres package kvdb diff --git a/kvdb/postgres/db.go b/kvdb/postgres/db.go index 3276fb557..d2e59bf0c 100644 --- a/kvdb/postgres/db.go +++ b/kvdb/postgres/db.go @@ -1,3 +1,4 @@ +//go:build kvdb_postgres // +build kvdb_postgres package postgres diff --git a/kvdb/postgres/db_test.go b/kvdb/postgres/db_test.go index dab6bf447..84c2b6503 100644 --- a/kvdb/postgres/db_test.go +++ b/kvdb/postgres/db_test.go @@ -1,3 +1,4 @@ +//go:build kvdb_postgres // +build kvdb_postgres package postgres diff --git a/kvdb/postgres/driver.go b/kvdb/postgres/driver.go index 601c0195c..1792eb652 100644 --- a/kvdb/postgres/driver.go +++ b/kvdb/postgres/driver.go @@ -1,3 +1,4 @@ +//go:build kvdb_postgres // +build kvdb_postgres package postgres diff --git a/kvdb/postgres/fixture.go b/kvdb/postgres/fixture.go index 9257f49aa..fe6c323d5 100644 --- a/kvdb/postgres/fixture.go +++ b/kvdb/postgres/fixture.go @@ -1,3 +1,4 @@ +//go:build kvdb_postgres // +build kvdb_postgres package postgres diff --git a/kvdb/postgres/readwrite_bucket.go b/kvdb/postgres/readwrite_bucket.go index b0fb57833..131151d86 100644 --- a/kvdb/postgres/readwrite_bucket.go +++ b/kvdb/postgres/readwrite_bucket.go @@ -1,3 +1,4 @@ +//go:build kvdb_postgres // +build kvdb_postgres package postgres diff --git a/kvdb/postgres/readwrite_cursor.go b/kvdb/postgres/readwrite_cursor.go index 8b95f6470..d56363bbf 100644 --- a/kvdb/postgres/readwrite_cursor.go +++ b/kvdb/postgres/readwrite_cursor.go @@ -1,3 +1,4 @@ +//go:build kvdb_postgres // +build kvdb_postgres package postgres diff --git a/kvdb/postgres/readwrite_tx.go b/kvdb/postgres/readwrite_tx.go index 0b20e19d3..99701a7a8 100644 --- a/kvdb/postgres/readwrite_tx.go +++ b/kvdb/postgres/readwrite_tx.go @@ -1,3 +1,4 @@ +//go:build kvdb_postgres // +build kvdb_postgres package postgres diff --git a/kvdb/postgres_test.go b/kvdb/postgres_test.go index 445f71b73..b26bba537 100644 --- a/kvdb/postgres_test.go +++ b/kvdb/postgres_test.go @@ -1,3 +1,4 @@ +//go:build kvdb_postgres // +build kvdb_postgres package kvdb @@ -35,13 +36,13 @@ func TestPostgres(t *testing.T) { test: testReadWriteCursor, expectedDb: m{ "test_kv": []m{ - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, - m{"id": int64(4), "key": "da", "parent_id": int64(1), "sequence": nil, "value": "3"}, - m{"id": int64(6), "key": "a", "parent_id": int64(1), "sequence": nil, "value": "0"}, - m{"id": int64(7), "key": "f", "parent_id": int64(1), "sequence": nil, "value": "5"}, - m{"id": int64(3), "key": "c", "parent_id": int64(1), "sequence": nil, "value": "3"}, - m{"id": int64(9), "key": "cx", "parent_id": int64(1), "sequence": nil, "value": "x"}, - m{"id": int64(10), "key": "cy", "parent_id": int64(1), "sequence": nil, "value": "y"}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, + {"id": int64(4), "key": "da", "parent_id": int64(1), "sequence": nil, "value": "3"}, + {"id": int64(6), "key": "a", "parent_id": int64(1), "sequence": nil, "value": "0"}, + {"id": int64(7), "key": "f", "parent_id": int64(1), "sequence": nil, "value": "5"}, + {"id": int64(3), "key": "c", "parent_id": int64(1), "sequence": nil, "value": "3"}, + {"id": int64(9), "key": "cx", "parent_id": int64(1), "sequence": nil, "value": "x"}, + {"id": int64(10), "key": "cy", "parent_id": int64(1), "sequence": nil, "value": "y"}, }, }, }, @@ -50,10 +51,10 @@ func TestPostgres(t *testing.T) { test: testReadWriteCursorWithBucketAndValue, expectedDb: m{ "test_kv": []m{ - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, - m{"id": int64(2), "key": "key", "parent_id": int64(1), "sequence": nil, "value": "val"}, - m{"id": int64(3), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, - m{"id": int64(4), "key": "pear", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, + {"id": int64(2), "key": "key", "parent_id": int64(1), "sequence": nil, "value": "val"}, + {"id": int64(3), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(4), "key": "pear", "parent_id": int64(1), "sequence": nil, "value": nil}, }, }, }, @@ -62,10 +63,10 @@ func TestPostgres(t *testing.T) { test: testBucketCreation, expectedDb: m{ "test_kv": []m{ - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, - m{"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, - m{"id": int64(3), "key": "mango", "parent_id": int64(1), "sequence": nil, "value": nil}, - m{"id": int64(4), "key": "pear", "parent_id": int64(2), "sequence": nil, "value": nil}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, + {"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(3), "key": "mango", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(4), "key": "pear", "parent_id": int64(2), "sequence": nil, "value": nil}, }, }, }, @@ -74,10 +75,10 @@ func TestPostgres(t *testing.T) { test: testBucketDeletion, expectedDb: m{ "test_kv": []m{ - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, - m{"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, - m{"id": int64(3), "key": "key1", "parent_id": int64(2), "sequence": nil, "value": "val1"}, - m{"id": int64(5), "key": "key3", "parent_id": int64(2), "sequence": nil, "value": "val3"}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, + {"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(3), "key": "key1", "parent_id": int64(2), "sequence": nil, "value": "val1"}, + {"id": int64(5), "key": "key3", "parent_id": int64(2), "sequence": nil, "value": "val3"}, }, }, }, @@ -86,14 +87,14 @@ func TestPostgres(t *testing.T) { test: testBucketForEach, expectedDb: m{ "test_kv": []m{ - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, - m{"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, - m{"id": int64(3), "key": "key1", "parent_id": int64(1), "sequence": nil, "value": "val1"}, - m{"id": int64(4), "key": "key1", "parent_id": int64(2), "sequence": nil, "value": "val1"}, - m{"id": int64(5), "key": "key2", "parent_id": int64(1), "sequence": nil, "value": "val2"}, - m{"id": int64(6), "key": "key2", "parent_id": int64(2), "sequence": nil, "value": "val2"}, - m{"id": int64(7), "key": "key3", "parent_id": int64(1), "sequence": nil, "value": "val3"}, - m{"id": int64(8), "key": "key3", "parent_id": int64(2), "sequence": nil, "value": "val3"}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, + {"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(3), "key": "key1", "parent_id": int64(1), "sequence": nil, "value": "val1"}, + {"id": int64(4), "key": "key1", "parent_id": int64(2), "sequence": nil, "value": "val1"}, + {"id": int64(5), "key": "key2", "parent_id": int64(1), "sequence": nil, "value": "val2"}, + {"id": int64(6), "key": "key2", "parent_id": int64(2), "sequence": nil, "value": "val2"}, + {"id": int64(7), "key": "key3", "parent_id": int64(1), "sequence": nil, "value": "val3"}, + {"id": int64(8), "key": "key3", "parent_id": int64(2), "sequence": nil, "value": "val3"}, }, }, }, @@ -102,11 +103,11 @@ func TestPostgres(t *testing.T) { test: testBucketForEachWithError, expectedDb: m{ "test_kv": []m{ - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, - m{"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, - m{"id": int64(3), "key": "pear", "parent_id": int64(1), "sequence": nil, "value": nil}, - m{"id": int64(4), "key": "key1", "parent_id": int64(1), "sequence": nil, "value": "val1"}, - m{"id": int64(5), "key": "key2", "parent_id": int64(1), "sequence": nil, "value": "val2"}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, + {"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(3), "key": "pear", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(4), "key": "key1", "parent_id": int64(1), "sequence": nil, "value": "val1"}, + {"id": int64(5), "key": "key2", "parent_id": int64(1), "sequence": nil, "value": "val2"}, }, }, }, @@ -115,8 +116,8 @@ func TestPostgres(t *testing.T) { test: testBucketSequence, expectedDb: m{ "test_kv": []m{ - m{"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": int64(4), "value": nil}, + {"id": int64(2), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": int64(4), "value": nil}, }, }, }, @@ -125,9 +126,9 @@ func TestPostgres(t *testing.T) { test: testKeyClash, expectedDb: m{ "test_kv": []m{ - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, - m{"id": int64(2), "key": "key", "parent_id": int64(1), "sequence": nil, "value": "val"}, - m{"id": int64(3), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, + {"id": int64(2), "key": "key", "parent_id": int64(1), "sequence": nil, "value": "val"}, + {"id": int64(3), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": nil}, }, }, }, @@ -136,8 +137,8 @@ func TestPostgres(t *testing.T) { test: testBucketCreateDelete, expectedDb: m{ "test_kv": []m{ - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, - m{"id": int64(3), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": "value"}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, + {"id": int64(3), "key": "banana", "parent_id": int64(1), "sequence": nil, "value": "value"}, }, }, }, @@ -146,8 +147,8 @@ func TestPostgres(t *testing.T) { test: testTxManualCommit, expectedDb: m{ "test_kv": []m{ - m{"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, - m{"id": int64(2), "key": "testKey", "parent_id": int64(1), "sequence": nil, "value": "testVal"}, + {"id": int64(1), "key": "apple", "parent_id": nil, "sequence": nil, "value": nil}, + {"id": int64(2), "key": "testKey", "parent_id": int64(1), "sequence": nil, "value": "testVal"}, }, }, }, diff --git a/macaroons/service.go b/macaroons/service.go index 3daccf0e2..c0c0c2cbe 100644 --- a/macaroons/service.go +++ b/macaroons/service.go @@ -4,6 +4,7 @@ import ( "context" "encoding/hex" "fmt" + "github.com/lightningnetwork/lnd/kvdb" "google.golang.org/grpc/metadata" "gopkg.in/macaroon-bakery.v2/bakery" From 35b4382f7af697bc6167b4b8c34b9ede618313f2 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 8 Nov 2021 12:02:25 +0100 Subject: [PATCH 6/7] docs: update release notes --- docs/release-notes/release-notes-0.14.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md index c1fa4b253..437c3b87a 100644 --- a/docs/release-notes/release-notes-0.14.0.md +++ b/docs/release-notes/release-notes-0.14.0.md @@ -472,6 +472,9 @@ messages directly. There is no routing/path finding involved. * [Fixed two flakes in itests that were caused by things progressing too fast](https://github.com/lightningnetwork/lnd/pull/5905). +* [Fixes two issues around configuration parsing and error + logging](https://github.com/lightningnetwork/lnd/pull/5948). + ## Database * [Ensure single writer for legacy From 842221aab21df7631af8bac87446efb63bc1f67c Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 8 Nov 2021 15:28:54 +0100 Subject: [PATCH 7/7] config: move directory creation to after path fixes Fixes #5927. This commit moves the code that attempts to create parent directories to the correct place _after_ we've adjusted all path values to point to the correct places. Before this commit the macaroon and tor paths would point to their default locations, even if the --lnddir flag was specified. --- config.go | 46 ++++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/config.go b/config.go index c4c70540d..6666942ce 100644 --- a/config.go +++ b/config.go @@ -740,8 +740,8 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser, } } - str := "Failed to create lnd directory: %v" - return mkErr(str, err) + str := "Failed to create lnd directory '%s': %v" + return mkErr(str, dir, err) } return nil @@ -809,24 +809,6 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser, cfg.WalletUnlockPasswordFile, ) - // Create the lnd directory and all other sub directories if they don't - // already exist. This makes sure that directory trees are also created - // for files that point to outside of the lnddir. - dirs := []string{ - lndDir, cfg.DataDir, - cfg.LetsEncryptDir, cfg.Watchtower.TowerDir, - filepath.Dir(cfg.TLSCertPath), filepath.Dir(cfg.TLSKeyPath), - filepath.Dir(cfg.AdminMacPath), filepath.Dir(cfg.ReadMacPath), - filepath.Dir(cfg.InvoiceMacPath), - filepath.Dir(cfg.Tor.PrivateKeyPath), - filepath.Dir(cfg.Tor.WatchtowerKeyPath), - } - for _, dir := range dirs { - if err := makeDirectory(dir); err != nil { - return nil, err - } - } - // Ensure that the user didn't attempt to specify negative values for // any of the autopilot params. if cfg.Autopilot.MaxChannels < 0 { @@ -1307,12 +1289,6 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser, lncfg.NormalizeNetwork(cfg.ActiveNetParams.Name), ) - // We need to make sure the default network directory exists for when we - // try to create our default macaroons there. - if err := makeDirectory(cfg.networkDir); err != nil { - return nil, err - } - // If a custom macaroon directory wasn't specified and the data // directory has changed from the default path, then we'll also update // the path for the macaroons to be generated. @@ -1332,6 +1308,24 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser, ) } + // Create the lnd directory and all other sub-directories if they don't + // already exist. This makes sure that directory trees are also created + // for files that point to outside the lnddir. + dirs := []string{ + lndDir, cfg.DataDir, cfg.networkDir, + cfg.LetsEncryptDir, cfg.Watchtower.TowerDir, + filepath.Dir(cfg.TLSCertPath), filepath.Dir(cfg.TLSKeyPath), + filepath.Dir(cfg.AdminMacPath), filepath.Dir(cfg.ReadMacPath), + filepath.Dir(cfg.InvoiceMacPath), + filepath.Dir(cfg.Tor.PrivateKeyPath), + filepath.Dir(cfg.Tor.WatchtowerKeyPath), + } + for _, dir := range dirs { + if err := makeDirectory(dir); err != nil { + return nil, err + } + } + // Similarly, if a custom back up file path wasn't specified, then // we'll update the file location to match our set network directory. if cfg.BackupFilePath == "" {