Skip to content

Rebase bugfix PRs, prepare for minor release#6026

Merged
guggero merged 7 commits intolightningnetwork:masterfrom
guggero:issue-rebase
Nov 24, 2021
Merged

Rebase bugfix PRs, prepare for minor release#6026
guggero merged 7 commits intolightningnetwork:masterfrom
guggero:issue-rebase

Conversation

@guggero
Copy link
Copy Markdown
Collaborator

@guggero guggero commented Nov 24, 2021

Replaces #6007.
Replaces #6003.

Fixes #6001.
Fixes #6002.
Fixes #5890.

Addresses an issue with explicit channel type negotiation that caused incompatibilities when opening channels with the latest versions of c-lightning and eclair (#5890).

…d be

This fixes an issue where if one tries to unset a feature like anchors,
and other feature depend on it, then `lnd` fails to start as it realizes
that its dependnacy set is inconsistent.

Fixes lightningnetwork#6002
In this commit, we fix a bug that would cause newly updated nodes to be
unable to start up, if they have an older channel that was closed before
we started to store all the historical state for each channel.

The issue is that we started to write the complete state to disk, but
newer channels don't have it, so when we try to supplement the
resolvers, we run into this error.

Ultimately, we only need this new supplemented information for script
enforcement channels. Ideally we would instead check the channel type
there instead, but it doesn't appear to be available in this context as
is, without further changes.

Fixes lightningnetwork#6001.
Copy link
Copy Markdown
Collaborator

@bhandras bhandras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think the PR description needs an update to link to this PR #6003

LGTM 👍

Copy link
Copy Markdown
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latest lgtm 🚀

@Roasbeef
Copy link
Copy Markdown
Member

Patch to resolve the comment I made above:

diff --git a/funding/commitment_type_negotiation.go b/funding/commitment_type_negotiation.go
index e2a9095a7..eb201e5b1 100644
--- a/funding/commitment_type_negotiation.go
+++ b/funding/commitment_type_negotiation.go
@@ -36,25 +36,6 @@ func negotiateCommitmentType(channelType *lnwire.ChannelType,
 				*channelType, local, remote,
 			)
 		}
-
-		// If they don't know explicit negotiation, let's fall back to
-		// implicit negotiation if they just signal one of the known
-		// default types.
-		channelFeatures := lnwire.RawFeatureVector(*channelType)
-		staticRemoteKeyOnly := channelFeatures.OnlyContains(
-			lnwire.StaticRemoteKeyRequired,
-		)
-		anchorOnly := channelFeatures.OnlyContains(
-			lnwire.AnchorsZeroFeeHtlcTxRequired,
-		)
-
-		// It's one of the default types.
-		if staticRemoteKeyOnly || anchorOnly {
-			return implicitNegotiateCommitmentType(local, remote), nil
-		}
-
-		// Something's weird, let's not accept this channel.
-		return 0, errUnsupportedExplicitNegotiation
 	}
 
 	return implicitNegotiateCommitmentType(local, remote), nil
diff --git a/funding/commitment_type_negotiation_test.go b/funding/commitment_type_negotiation_test.go
index c9ac25a7a..4da88f2a9 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,8 @@ 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

@guggero
Copy link
Copy Markdown
Collaborator Author

guggero commented Nov 24, 2021

Thanks! Applied the patch, going to test again vs. c-lightning 0.10.2 locally.

@guggero guggero merged commit 53342b4 into lightningnetwork:master Nov 24, 2021
@guggero guggero deleted the issue-rebase branch November 24, 2021 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

6 participants