Merge pull request #6026 from guggero/issue-rebase

Rebase bugfix PRs, prepare for minor release

contractcourt/channel_arbitrator.go

@@ -612,8 +612,17 @@ func (c *ChannelArbitrator) relaunchResolvers(commitSet *CommitSet,
 	// We'll also fetch the historical state of this channel, as it should
 	// have been marked as closed by now, and supplement it to each resolver
 	// such that we can properly resolve our pending contracts.
-	chanState, err := c.cfg.FetchHistoricalChannel()
+	var chanState *channeldb.OpenChannel
-	if err != nil {
+	chanState, err = c.cfg.FetchHistoricalChannel()
+	switch {
+	// If we don't find this channel, then it may be the case that it
+	// was closed before we started to retain the final state
+	// information for open channels.
+	case err == channeldb.ErrChannelNotFound:
+		log.Warnf("ChannelArbitrator(%v): unable to fetch historical "+
+			"state", c.cfg.ChanPoint)
+
+	case err != nil:
 		return err
 	}
 
@@ -621,7 +630,9 @@ func (c *ChannelArbitrator) relaunchResolvers(commitSet *CommitSet,
 		"resolvers", c.cfg.ChanPoint, len(unresolvedContracts))
 
 	for _, resolver := range unresolvedContracts {
-		resolver.SupplementState(chanState)
+		if chanState != nil {
+			resolver.SupplementState(chanState)
+		}
 
 		htlcResolver, ok := resolver.(htlcContractResolver)
 		if !ok {
@@ -1930,8 +1941,17 @@ func (c *ChannelArbitrator) prepContractResolutions(
 	// We'll also fetch the historical state of this channel, as it should
 	// have been marked as closed by now, and supplement it to each resolver
 	// such that we can properly resolve our pending contracts.
-	chanState, err := c.cfg.FetchHistoricalChannel()
+	var chanState *channeldb.OpenChannel
-	if err != nil {
+	chanState, err = c.cfg.FetchHistoricalChannel()
+	switch {
+	// If we don't find this channel, then it may be the case that it
+	// was closed before we started to retain the final state
+	// information for open channels.
+	case err == channeldb.ErrChannelNotFound:
+		log.Warnf("ChannelArbitrator(%v): unable to fetch historical "+
+			"state", c.cfg.ChanPoint)
+
+	case err != nil:
 		return nil, nil, err
 	}
 
@@ -2040,7 +2060,9 @@ func (c *ChannelArbitrator) prepContractResolutions(
 				resolver := newTimeoutResolver(
 					resolution, height, htlc, resolverCfg,
 				)
-				resolver.SupplementState(chanState)
+				if chanState != nil {
+					resolver.SupplementState(chanState)
+				}
 				htlcResolvers = append(htlcResolvers, resolver)
 			}
 
@@ -2097,7 +2119,9 @@ func (c *ChannelArbitrator) prepContractResolutions(
 				resolver := newOutgoingContestResolver(
 					resolution, height, htlc, resolverCfg,
 				)
-				resolver.SupplementState(chanState)
+				if chanState != nil {
+					resolver.SupplementState(chanState)
+				}
 				htlcResolvers = append(htlcResolvers, resolver)
 			}
 		}
@@ -2111,7 +2135,9 @@ func (c *ChannelArbitrator) prepContractResolutions(
 			*contractResolutions.CommitResolution, height,
 			c.cfg.ChanPoint, resolverCfg,
 		)
-		resolver.SupplementState(chanState)
+		if chanState != nil {
+			resolver.SupplementState(chanState)
+		}
 		htlcResolvers = append(htlcResolvers, resolver)
 	}
 

docs/release-notes/release-notes-0.14.1.md

@@ -12,8 +12,19 @@
 * [A bug has been fixed in channeldb that uses the return value without checking
   the returned error first](https://github.com/lightningnetwork/lnd/pull/6012).
 
+* [Fixes a bug that would cause lnd to be unable to start if anchors was
+  disabled](https://github.com/lightningnetwork/lnd/pull/6007).
+
+* [Fixed a bug that would cause nodes with older channels to be unable to start
+  up](https://github.com/lightningnetwork/lnd/pull/6003).
+
+* [Addresses an issue with explicit channel type negotiation that caused
+  incompatibilities when opening channels with the latest versions of
+  c-lightning and eclair](https://github.com/lightningnetwork/lnd/pull/6026).
+
 # Contributors (Alphabetical Order)
 
 * Jamie Turley
 * nayuta-ueno
+* Olaoluwa Osuntokun
 * Oliver Gugger

feature/manager.go

@@ -91,6 +91,21 @@ func newManager(cfg Config, desc setDesc) (*Manager, error) {
 		if cfg.NoAnchors {
 			raw.Unset(lnwire.AnchorsZeroFeeHtlcTxOptional)
 			raw.Unset(lnwire.AnchorsZeroFeeHtlcTxRequired)
+
+			// If anchors are disabled, then we also need to
+			// disable all other features that depend on it as
+			// well, as otherwise we may create an invalid feature
+			// bit set.
+			for bit, depFeatures := range deps {
+				for depFeature := range depFeatures {
+					switch {
+					case depFeature == lnwire.AnchorsZeroFeeHtlcTxRequired:
+						fallthrough
+					case depFeature == lnwire.AnchorsZeroFeeHtlcTxOptional:
+						raw.Unset(bit)
+					}
+				}
+			}
 		}
 		if cfg.NoWumbo {
 			raw.Unset(lnwire.WumboChannelsOptional)

feature/manager_internal_test.go

@@ -52,6 +52,12 @@ var managerTests = []managerTest{
 			NoStaticRemoteKey: true,
 		},
 	},
+	{
+		name: "anchors should disable anything dependent on it",
+		cfg: Config{
+			NoAnchors: true,
+		},
+	},
 }
 
 // TestManager asserts basic initialazation and operation of a feature manager,
@@ -104,6 +110,10 @@ func testManager(t *testing.T, test managerTest) {
 		if test.cfg.NoStaticRemoteKey {
 			assertUnset(lnwire.StaticRemoteKeyOptional)
 		}
+		if test.cfg.NoAnchors {
+			assertUnset(lnwire.ScriptEnforcedLeaseRequired)
+			assertUnset(lnwire.ScriptEnforcedLeaseOptional)
+		}
 
 		assertUnset(unknownFeature)
 	}

funding/commitment_type_negotiation.go

@@ -8,12 +8,6 @@ import (
 )
 
 var (
-	// errUnsupportedExplicitNegotiation is an error returned when explicit
-	// channel commitment negotiation is attempted but either peer of the
-	// channel does not support it.
-	errUnsupportedExplicitNegotiation = errors.New("explicit channel " +
-		"type negotiation not supported")
-
 	// errUnsupportedCommitmentType is an error returned when a specific
 	// channel commitment type is being explicitly negotiated but either
 	// peer of the channel does not support it.
@@ -29,12 +23,13 @@ func negotiateCommitmentType(channelType *lnwire.ChannelType,
 	local, remote *lnwire.FeatureVector) (lnwallet.CommitmentType, error) {
 
 	if channelType != nil {
-		if !hasFeatures(local, remote, lnwire.ExplicitChannelTypeOptional) {
+		// If the peer does know explicit negotiation, let's attempt
-			return 0, errUnsupportedExplicitNegotiation
+		// that now.
+		if hasFeatures(local, remote, lnwire.ExplicitChannelTypeOptional) {
+			return explicitNegotiateCommitmentType(
+				*channelType, local, remote,
+			)
 		}
-		return explicitNegotiateCommitmentType(
-			*channelType, local, remote,
-		)
 	}
 
 	return implicitNegotiateCommitmentType(local, remote), nil

funding/commitment_type_negotiation_test.go

@@ -36,7 +36,8 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 				lnwire.StaticRemoteKeyOptional,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 			),
-			expectsErr: errUnsupportedExplicitNegotiation,
+			expectsRes: lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx,
+			expectsErr: nil,
 		},
 		{
 			name: "explicit missing remote commitment feature",
@@ -181,8 +182,14 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 				return
 			}
 
-			require.Equal(t, testCase.expectsRes, localType)
+			require.Equal(
-			require.Equal(t, testCase.expectsRes, remoteType)
+				t, testCase.expectsRes, localType,
+				testCase.name,
+			)
+			require.Equal(
+				t, testCase.expectsRes, remoteType,
+				testCase.name,
+			)
 		})
 		if !ok {
 			return

funding/manager.go

@@ -1587,9 +1587,32 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer,
 			}
 		}
 	} else if msg.ChannelType != nil {
-		err := errors.New("received unexpected channel type")
+		// The spec isn't too clear about whether it's okay to set the
-		f.failFundingFlow(peer, msg.PendingChannelID, err)
+		// channel type in the accept_channel response if we didn't
-		return
+		// explicitly set it in the open_channel message. For now, let's
+		// just log the problem instead of failing the funding flow.
+		implicitChannelType := implicitNegotiateCommitmentType(
+			peer.LocalFeatures(), peer.RemoteFeatures(),
+		)
+		negotiatedChannelType, err := negotiateCommitmentType(
+			msg.ChannelType, peer.LocalFeatures(),
+			peer.RemoteFeatures(),
+		)
+		if err != nil {
+			err := errors.New("received unexpected channel type")
+			f.failFundingFlow(peer, msg.PendingChannelID, err)
+			return
+		}
+
+		// Even though we don't expect a channel type to be set when we
+		// didn't send one in the first place, we check that it's the
+		// same type we'd have arrived through implicit negotiation. If
+		// it's another type, we fail the flow.
+		if implicitChannelType != negotiatedChannelType {
+			err := errors.New("negotiated unexpected channel type")
+			f.failFundingFlow(peer, msg.PendingChannelID, err)
+			return
+		}
 	}
 
 	// The required number of confirmations should not be greater than the