diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index e85e74b34..ff29e6966 100644 --- a/contractcourt/channel_arbitrator.go +++ b/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() - if err != nil { + var chanState *channeldb.OpenChannel + 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() - if err != nil { + var chanState *channeldb.OpenChannel + 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) } diff --git a/docs/release-notes/release-notes-0.14.1.md b/docs/release-notes/release-notes-0.14.1.md index 11828dd53..655629e26 100644 --- a/docs/release-notes/release-notes-0.14.1.md +++ b/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 diff --git a/feature/manager.go b/feature/manager.go index 50a54d625..9d5d1cad4 100644 --- a/feature/manager.go +++ b/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) diff --git a/feature/manager_internal_test.go b/feature/manager_internal_test.go index 9c97bd052..2778a86d9 100644 --- a/feature/manager_internal_test.go +++ b/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) } diff --git a/funding/commitment_type_negotiation.go b/funding/commitment_type_negotiation.go index 091c6fcbb..392b7c824 100644 --- a/funding/commitment_type_negotiation.go +++ b/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) { - return 0, errUnsupportedExplicitNegotiation + // If the peer does know explicit negotiation, let's attempt + // that now. + if hasFeatures(local, remote, lnwire.ExplicitChannelTypeOptional) { + return explicitNegotiateCommitmentType( + *channelType, local, remote, + ) } - return explicitNegotiateCommitmentType( - *channelType, local, remote, - ) } return implicitNegotiateCommitmentType(local, remote), nil diff --git a/funding/commitment_type_negotiation_test.go b/funding/commitment_type_negotiation_test.go index c9ac25a7a..c3032685c 100644 --- a/funding/commitment_type_negotiation_test.go +++ b/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(t, testCase.expectsRes, remoteType) + require.Equal( + t, testCase.expectsRes, localType, + testCase.name, + ) + require.Equal( + t, testCase.expectsRes, remoteType, + testCase.name, + ) }) if !ok { return diff --git a/funding/manager.go b/funding/manager.go index b4fad5570..9a50685c8 100644 --- a/funding/manager.go +++ b/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") - f.failFundingFlow(peer, msg.PendingChannelID, err) - return + // The spec isn't too clear about whether it's okay to set the + // channel type in the accept_channel response if we didn't + // 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