From 99f79ea50715273f23d3b307e4dc676713f1abbb Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 9 Dec 2021 16:16:38 -0800 Subject: [PATCH 1/2] funding: always send a channel type in explicit mode In this commit, we switch to always sending a channel type when we're in explicit mode. This is compatible with prior versions of lnd as they won't send a channel type, and we'll just arrive at the same type via the existing implicit funding. Fixes https://github.com/lightningnetwork/lnd/issues/6067 --- funding/commitment_type_negotiation.go | 29 +++++--- funding/commitment_type_negotiation_test.go | 73 +++++++++++++++------ funding/manager.go | 10 +-- 3 files changed, 78 insertions(+), 34 deletions(-) diff --git a/funding/commitment_type_negotiation.go b/funding/commitment_type_negotiation.go index b559d6b66..1c8fe1a40 100644 --- a/funding/commitment_type_negotiation.go +++ b/funding/commitment_type_negotiation.go @@ -26,8 +26,8 @@ var ( // will be attempted if the set of both local and remote features support it. // Otherwise, implicit negotiation will be attempted. func negotiateCommitmentType(channelType *lnwire.ChannelType, - local, remote *lnwire.FeatureVector, - mustBeExplicit bool) (bool, lnwallet.CommitmentType, error) { + local, remote *lnwire.FeatureVector, mustBeExplicit bool, +) (bool, *lnwire.ChannelType, lnwallet.CommitmentType, error) { if channelType != nil { // If the peer does know explicit negotiation, let's attempt @@ -36,7 +36,7 @@ func negotiateCommitmentType(channelType *lnwire.ChannelType, chanType, err := explicitNegotiateCommitmentType( *channelType, local, remote, ) - return true, chanType, err + return true, channelType, chanType, err } // If we're the funder, and we are attempting to use an @@ -45,11 +45,12 @@ func negotiateCommitmentType(channelType *lnwire.ChannelType, // user doesn't end up with an unexpected channel type via // implicit negotiation. if mustBeExplicit { - return false, 0, errUnsupportedExplicitNegotiation + return false, nil, 0, errUnsupportedExplicitNegotiation } } - return false, implicitNegotiateCommitmentType(local, remote), nil + chanType, commitType := implicitNegotiateCommitmentType(local, remote) + return false, chanType, commitType, nil } // explicitNegotiateCommitmentType attempts to explicitly negotiate for a @@ -113,12 +114,17 @@ func explicitNegotiateCommitmentType(channelType lnwire.ChannelType, // implicitly by choosing the latest type supported by the local and remote // fetures. func implicitNegotiateCommitmentType(local, - remote *lnwire.FeatureVector) lnwallet.CommitmentType { + remote *lnwire.FeatureVector) (*lnwire.ChannelType, lnwallet.CommitmentType) { // If both peers are signalling support for anchor commitments with // zero-fee HTLC transactions, we'll use this type. if hasFeatures(local, remote, lnwire.AnchorsZeroFeeHtlcTxOptional) { - return lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx + chanType := lnwire.ChannelType(*lnwire.NewRawFeatureVector( + lnwire.AnchorsZeroFeeHtlcTxRequired, + lnwire.StaticRemoteKeyRequired, + )) + + return &chanType, lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx } // Since we don't want to support the "legacy" anchor type, we will fall @@ -128,11 +134,16 @@ func implicitNegotiateCommitmentType(local, // If both nodes are signaling the proper feature bit for tweakless // commitments, we'll use that. if hasFeatures(local, remote, lnwire.StaticRemoteKeyOptional) { - return lnwallet.CommitmentTypeTweakless + chanType := lnwire.ChannelType(*lnwire.NewRawFeatureVector( + lnwire.StaticRemoteKeyRequired, + )) + + return &chanType, lnwallet.CommitmentTypeTweakless } // Otherwise we'll fall back to the legacy type. - return lnwallet.CommitmentTypeLegacy + chanType := lnwire.ChannelType(*lnwire.NewRawFeatureVector()) + return &chanType, lnwallet.CommitmentTypeLegacy } // hasFeatures determines whether a set of features is supported by both the set diff --git a/funding/commitment_type_negotiation_test.go b/funding/commitment_type_negotiation_test.go index 6973026ee..689b37cb1 100644 --- a/funding/commitment_type_negotiation_test.go +++ b/funding/commitment_type_negotiation_test.go @@ -14,13 +14,14 @@ func TestCommitmentTypeNegotiation(t *testing.T) { t.Parallel() testCases := []struct { - name string - mustBeExplicit bool - channelFeatures *lnwire.RawFeatureVector - localFeatures *lnwire.RawFeatureVector - remoteFeatures *lnwire.RawFeatureVector - expectsRes lnwallet.CommitmentType - expectsErr error + name string + mustBeExplicit bool + channelFeatures *lnwire.RawFeatureVector + localFeatures *lnwire.RawFeatureVector + remoteFeatures *lnwire.RawFeatureVector + expectsCommitType lnwallet.CommitmentType + expectsChanType lnwire.ChannelType + expectsErr error }{ { name: "explicit missing remote negotiation feature", @@ -37,7 +38,11 @@ func TestCommitmentTypeNegotiation(t *testing.T) { lnwire.StaticRemoteKeyOptional, lnwire.AnchorsZeroFeeHtlcTxOptional, ), - expectsRes: lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx, + expectsCommitType: lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx, + expectsChanType: lnwire.ChannelType(*lnwire.NewRawFeatureVector( + lnwire.StaticRemoteKeyRequired, + lnwire.AnchorsZeroFeeHtlcTxRequired, + )), expectsErr: nil, }, { @@ -92,7 +97,11 @@ func TestCommitmentTypeNegotiation(t *testing.T) { lnwire.AnchorsZeroFeeHtlcTxOptional, lnwire.ExplicitChannelTypeOptional, ), - expectsRes: lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx, + expectsCommitType: lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx, + expectsChanType: lnwire.ChannelType(*lnwire.NewRawFeatureVector( + lnwire.StaticRemoteKeyRequired, + lnwire.AnchorsZeroFeeHtlcTxRequired, + )), expectsErr: nil, }, { @@ -110,7 +119,10 @@ func TestCommitmentTypeNegotiation(t *testing.T) { lnwire.AnchorsZeroFeeHtlcTxOptional, lnwire.ExplicitChannelTypeOptional, ), - expectsRes: lnwallet.CommitmentTypeTweakless, + expectsCommitType: lnwallet.CommitmentTypeTweakless, + expectsChanType: lnwire.ChannelType(*lnwire.NewRawFeatureVector( + lnwire.StaticRemoteKeyRequired, + )), expectsErr: nil, }, { @@ -126,9 +138,13 @@ func TestCommitmentTypeNegotiation(t *testing.T) { lnwire.AnchorsZeroFeeHtlcTxOptional, lnwire.ExplicitChannelTypeOptional, ), - expectsRes: lnwallet.CommitmentTypeLegacy, - expectsErr: nil, + expectsCommitType: lnwallet.CommitmentTypeLegacy, + expectsChanType: lnwire.ChannelType(*lnwire.NewRawFeatureVector()), + expectsErr: nil, }, + // Both sides signal the explicit chan type bit, so we expect + // that we return the corresponding chan type feature bits, + // even though we didn't set an explicit channel type. { name: "implicit anchors", channelFeatures: nil, @@ -142,7 +158,11 @@ func TestCommitmentTypeNegotiation(t *testing.T) { lnwire.AnchorsZeroFeeHtlcTxOptional, lnwire.ExplicitChannelTypeOptional, ), - expectsRes: lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx, + expectsCommitType: lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx, + expectsChanType: lnwire.ChannelType(*lnwire.NewRawFeatureVector( + lnwire.StaticRemoteKeyRequired, + lnwire.AnchorsZeroFeeHtlcTxRequired, + )), expectsErr: nil, }, { @@ -155,7 +175,10 @@ func TestCommitmentTypeNegotiation(t *testing.T) { remoteFeatures: lnwire.NewRawFeatureVector( lnwire.StaticRemoteKeyOptional, ), - expectsRes: lnwallet.CommitmentTypeTweakless, + expectsCommitType: lnwallet.CommitmentTypeTweakless, + expectsChanType: lnwire.ChannelType(*lnwire.NewRawFeatureVector( + lnwire.StaticRemoteKeyRequired, + )), expectsErr: nil, }, { @@ -166,8 +189,9 @@ func TestCommitmentTypeNegotiation(t *testing.T) { lnwire.StaticRemoteKeyOptional, lnwire.AnchorsZeroFeeHtlcTxOptional, ), - expectsRes: lnwallet.CommitmentTypeLegacy, - expectsErr: nil, + expectsCommitType: lnwallet.CommitmentTypeLegacy, + expectsChanType: lnwire.ChannelType(*lnwire.NewRawFeatureVector()), + expectsErr: nil, }, } @@ -188,13 +212,13 @@ func TestCommitmentTypeNegotiation(t *testing.T) { *testCase.channelFeatures, ) } - _, localType, err := negotiateCommitmentType( + _, localChanType, localCommitType, err := negotiateCommitmentType( channelType, localFeatures, remoteFeatures, testCase.mustBeExplicit, ) require.Equal(t, testCase.expectsErr, err) - _, remoteType, err := negotiateCommitmentType( + _, remoteChanType, remoteCommitType, err := negotiateCommitmentType( channelType, remoteFeatures, localFeatures, testCase.mustBeExplicit, ) @@ -205,11 +229,20 @@ func TestCommitmentTypeNegotiation(t *testing.T) { } require.Equal( - t, testCase.expectsRes, localType, + t, testCase.expectsCommitType, localCommitType, testCase.name, ) require.Equal( - t, testCase.expectsRes, remoteType, + t, testCase.expectsCommitType, remoteCommitType, + testCase.name, + ) + + require.Equal( + t, testCase.expectsChanType, *localChanType, + testCase.name, + ) + require.Equal( + t, testCase.expectsChanType, *remoteChanType, testCase.name, ) }) diff --git a/funding/manager.go b/funding/manager.go index 405291a05..031b9e770 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -1274,7 +1274,7 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer, // the remote peer are signaling the proper feature bit if we're using // implicit negotiation, and simply the channel type sent over if we're // using explicit negotiation. - wasExplicit, commitType, err := negotiateCommitmentType( + wasExplicit, _, commitType, err := negotiateCommitmentType( msg.ChannelType, peer.LocalFeatures(), peer.RemoteFeatures(), false, ) @@ -1599,14 +1599,14 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer, // 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( + _, implicitChannelType := implicitNegotiateCommitmentType( peer.LocalFeatures(), peer.RemoteFeatures(), ) // We pass in false here as the funder since at this point, we // didn't set a chan type ourselves, so falling back to // implicit funding is acceptable. - _, negotiatedChannelType, err := negotiateCommitmentType( + _, _, negotiatedChannelType, err := negotiateCommitmentType( msg.ChannelType, peer.LocalFeatures(), peer.RemoteFeatures(), false, ) @@ -3258,7 +3258,7 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) { // Before we init the channel, we'll also check to see what commitment // format we can use with this peer. This is dependent on *both* us and // the remote peer are signaling the proper feature bit. - _, commitType, err := negotiateCommitmentType( + _, chanType, commitType, err := negotiateCommitmentType( msg.ChannelType, msg.Peer.LocalFeatures(), msg.Peer.RemoteFeatures(), true, ) @@ -3415,7 +3415,7 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) { FirstCommitmentPoint: ourContribution.FirstCommitmentPoint, ChannelFlags: channelFlags, UpfrontShutdownScript: shutdown, - ChannelType: msg.ChannelType, + ChannelType: chanType, LeaseExpiry: leaseExpiry, } if err := msg.Peer.SendMessage(true, &fundingOpen); err != nil { From e78560c4728fedc32670c91b2aa22918c5dc5238 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 15 Dec 2021 16:37:36 -0800 Subject: [PATCH 2/2] docs/release-notes: add entry for chan type fix --- docs/release-notes/release-notes-0.14.2.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.14.2.md b/docs/release-notes/release-notes-0.14.2.md index d0adc2757..9b3a24555 100644 --- a/docs/release-notes/release-notes-0.14.2.md +++ b/docs/release-notes/release-notes-0.14.2.md @@ -5,6 +5,9 @@ * [Return the nearest known fee rate when a given conf target cannot be found from Web API fee estimator.](https://github.com/lightningnetwork/lnd/pull/6062) +* [We now _always_ set a channel type if the other party signals the feature + bit](https://github.com/lightningnetwork/lnd/pull/6075). + ## Wallet * A bug that prevented opening anchor-based channels from external wallets when