Merge pull request #8275 from ProofOfKeags/chore/enforce-feature-bits

multi: make legacy feature bits compulsory

channeldb/invoice_test.go

@@ -24,7 +24,7 @@ var (
 	emptyFeatures = lnwire.NewFeatureVector(nil, lnwire.Features)
 	ampFeatures   = lnwire.NewFeatureVector(
 		lnwire.NewRawFeatureVector(
-			lnwire.TLVOnionPayloadOptional,
+			lnwire.TLVOnionPayloadRequired,
 			lnwire.PaymentAddrOptional,
 			lnwire.AMPRequired,
 		),
@@ -3158,7 +3158,7 @@ func TestAddInvoiceInvalidFeatureDeps(t *testing.T) {
 
 	invoice.Terms.Features = lnwire.NewFeatureVector(
 		lnwire.NewRawFeatureVector(
-			lnwire.TLVOnionPayloadOptional,
+			lnwire.TLVOnionPayloadRequired,
 			lnwire.MPPOptional,
 		),
 		lnwire.Features,

docs/release-notes/release-notes-0.18.0.md

@@ -221,6 +221,10 @@
   with a lower min_final_cltv_expiry_delta value than 18 blocks since
   LND 0.11.0.
 
+* [Make Legacy Features Compulsory](https://github.com/lightningnetwork/lnd/pull/8275).
+  This change implements changes codified in [bolts#1092](https://github.com/lightning/bolts/pull/1092)
+  and makes TLV Onions, Static Remote Keys, Gossip Queries, compulsory features for
+  LND's peers. Data Loss Protection has been compulsory for years.
 
 ## Testing
 

feature/default_sets.go

@@ -14,11 +14,11 @@ var defaultSetDesc = setDesc{
 		SetInit:    {}, // I
 		SetNodeAnn: {}, // N
 	},
-	lnwire.GossipQueriesOptional: {
+	lnwire.GossipQueriesRequired: {
 		SetInit:    {}, // I
 		SetNodeAnn: {}, // N
 	},
-	lnwire.TLVOnionPayloadOptional: {
+	lnwire.TLVOnionPayloadRequired: {
 		SetInit:         {}, // I
 		SetNodeAnn:      {}, // N
 		SetInvoice:      {}, // 9

feature/deps_test.go

@@ -33,7 +33,7 @@ var depTests = []depTest{
 	{
 		name: "one dep optional",
 		raw: lnwire.NewRawFeatureVector(
-			lnwire.TLVOnionPayloadOptional,
+			lnwire.TLVOnionPayloadRequired,
 			lnwire.PaymentAddrOptional,
 		),
 	},
@@ -61,7 +61,7 @@ var depTests = []depTest{
 	{
 		name: "two dep optional",
 		raw: lnwire.NewRawFeatureVector(
-			lnwire.TLVOnionPayloadOptional,
+			lnwire.TLVOnionPayloadRequired,
 			lnwire.PaymentAddrOptional,
 			lnwire.MPPOptional,
 		),
@@ -93,7 +93,7 @@ var depTests = []depTest{
 	{
 		name: "two dep first missing optional",
 		raw: lnwire.NewRawFeatureVector(
-			lnwire.TLVOnionPayloadOptional,
+			lnwire.TLVOnionPayloadRequired,
 			lnwire.MPPOptional,
 		),
 		expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional},
@@ -110,7 +110,7 @@ var depTests = []depTest{
 		name: "forest optional",
 		raw: lnwire.NewRawFeatureVector(
 			lnwire.GossipQueriesOptional,
-			lnwire.TLVOnionPayloadOptional,
+			lnwire.TLVOnionPayloadRequired,
 			lnwire.PaymentAddrOptional,
 			lnwire.MPPOptional,
 		),
@@ -128,7 +128,7 @@ var depTests = []depTest{
 		name: "broken forest optional",
 		raw: lnwire.NewRawFeatureVector(
 			lnwire.GossipQueriesOptional,
-			lnwire.TLVOnionPayloadOptional,
+			lnwire.TLVOnionPayloadRequired,
 			lnwire.MPPOptional,
 		),
 		expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional},

feature/manager_internal_test.go

@@ -19,11 +19,11 @@ var testSetDesc = setDesc{
 	lnwire.DataLossProtectRequired: {
 		SetNodeAnn: {}, // I
 	},
-	lnwire.TLVOnionPayloadOptional: {
+	lnwire.TLVOnionPayloadRequired: {
 		SetInit:    {}, // I
 		SetNodeAnn: {}, // N
 	},
-	lnwire.StaticRemoteKeyOptional: {
+	lnwire.StaticRemoteKeyRequired: {
 		SetInit:    {}, // I
 		SetNodeAnn: {}, // N
 	},
@@ -104,9 +104,11 @@ func testManager(t *testing.T, test managerTest) {
 		// Assert that the manager properly unset the configured feature
 		// bits from all sets.
 		if test.cfg.NoTLVOnion {
+			assertUnset(lnwire.TLVOnionPayloadRequired)
 			assertUnset(lnwire.TLVOnionPayloadOptional)
 		}
 		if test.cfg.NoStaticRemoteKey {
+			assertUnset(lnwire.StaticRemoteKeyRequired)
 			assertUnset(lnwire.StaticRemoteKeyOptional)
 		}
 		if test.cfg.NoAnchors {
@@ -127,12 +129,12 @@ func testManager(t *testing.T, test managerTest) {
 		}
 	}
 
-	assertSet(lnwire.DataLossProtectOptional)
+	assertSet(lnwire.DataLossProtectRequired)
 	if !test.cfg.NoTLVOnion {
 		assertSet(lnwire.TLVOnionPayloadRequired)
 	}
 	if !test.cfg.NoStaticRemoteKey {
-		assertSet(lnwire.StaticRemoteKeyOptional)
+		assertSet(lnwire.StaticRemoteKeyRequired)
 	}
 }
 
@@ -149,7 +151,7 @@ func TestUpdateFeatureSets(t *testing.T) {
 			SetInit:    {}, // I
 			SetNodeAnn: {}, // N
 		},
-		lnwire.GossipQueriesOptional: {
+		lnwire.GossipQueriesRequired: {
 			SetNodeAnn: {}, // N
 		},
 	}
@@ -199,7 +201,7 @@ func TestUpdateFeatureSets(t *testing.T) {
 				),
 				SetNodeAnn: lnwire.NewRawFeatureVector(
 					lnwire.DataLossProtectRequired,
-					lnwire.GossipQueriesOptional,
+					lnwire.GossipQueriesRequired,
 					lnwire.FeatureBit(1000),
 				),
 			},
@@ -220,7 +222,7 @@ func TestUpdateFeatureSets(t *testing.T) {
 				),
 				SetNodeAnn: lnwire.NewRawFeatureVector(
 					lnwire.DataLossProtectRequired,
-					lnwire.GossipQueriesOptional,
+					lnwire.GossipQueriesRequired,
 				),
 			},
 			config: Config{
@@ -240,7 +242,7 @@ func TestUpdateFeatureSets(t *testing.T) {
 				),
 				SetNodeAnn: lnwire.NewRawFeatureVector(
 					lnwire.DataLossProtectRequired,
-					lnwire.GossipQueriesOptional,
+					lnwire.GossipQueriesRequired,
 					lnwire.FeatureBit(500),
 				),
 				SetInvoice: lnwire.NewRawFeatureVector(

funding/commitment_type_negotiation_test.go

@@ -31,7 +31,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 				lnwire.AnchorsZeroFeeHtlcTxRequired,
 			),
 			localFeatures: lnwire.NewRawFeatureVector(
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ExplicitChannelTypeOptional,
 			),
@@ -39,6 +39,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 				lnwire.StaticRemoteKeyOptional,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 			),
+			//nolint:lll
 			expectsCommitType: lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx,
 			expectsChanType:   nil,
 			expectsErr:        nil,
@@ -50,7 +51,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 				lnwire.AnchorsZeroFeeHtlcTxRequired,
 			),
 			localFeatures: lnwire.NewRawFeatureVector(
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ExplicitChannelTypeOptional,
 			),
@@ -70,7 +71,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 			),
 			localFeatures: lnwire.NewRawFeatureVector(
 				lnwire.ZeroConfOptional,
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ScriptEnforcedLeaseOptional,
 				lnwire.ExplicitChannelTypeOptional,
@@ -103,7 +104,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 			),
 			localFeatures: lnwire.NewRawFeatureVector(
 				lnwire.ZeroConfOptional,
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ExplicitChannelTypeOptional,
 			),
@@ -134,7 +135,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 			),
 			localFeatures: lnwire.NewRawFeatureVector(
 				lnwire.ScidAliasOptional,
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ScriptEnforcedLeaseOptional,
 				lnwire.ExplicitChannelTypeOptional,
@@ -167,7 +168,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 			),
 			localFeatures: lnwire.NewRawFeatureVector(
 				lnwire.ScidAliasOptional,
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ExplicitChannelTypeOptional,
 			),
@@ -195,7 +196,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 				lnwire.AnchorsZeroFeeHtlcTxRequired,
 			),
 			localFeatures: lnwire.NewRawFeatureVector(
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ExplicitChannelTypeOptional,
 			),
@@ -219,7 +220,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 				lnwire.StaticRemoteKeyRequired,
 			),
 			localFeatures: lnwire.NewRawFeatureVector(
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ExplicitChannelTypeOptional,
 			),
@@ -240,7 +241,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 			name:            "explicit legacy",
 			channelFeatures: lnwire.NewRawFeatureVector(),
 			localFeatures: lnwire.NewRawFeatureVector(
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ExplicitChannelTypeOptional,
 			),
@@ -262,7 +263,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 			name:            "default explicit anchors",
 			channelFeatures: nil,
 			localFeatures: lnwire.NewRawFeatureVector(
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 				lnwire.ExplicitChannelTypeOptional,
 			),
@@ -284,7 +285,7 @@ func TestCommitmentTypeNegotiation(t *testing.T) {
 			name:            "implicit tweakless",
 			channelFeatures: nil,
 			localFeatures: lnwire.NewRawFeatureVector(
-				lnwire.StaticRemoteKeyOptional,
+				lnwire.StaticRemoteKeyRequired,
 				lnwire.AnchorsZeroFeeHtlcTxOptional,
 			),
 			remoteFeatures: lnwire.NewRawFeatureVector(

invoices/invoiceregistry.go

@@ -756,10 +756,10 @@ func (i *InvoiceRegistry) processKeySend(ctx invoiceUpdateCtx) error {
 	// Create an invoice for the htlc amount.
 	amt := ctx.amtPaid
 
-	// Set tlv optional feature vector on the invoice. Otherwise we wouldn't
+	// Set tlv required feature vector on the invoice. Otherwise we wouldn't
 	// be able to pay to it with keysend.
 	rawFeatures := lnwire.NewRawFeatureVector(
-		lnwire.TLVOnionPayloadOptional,
+		lnwire.TLVOnionPayloadRequired,
 	)
 	features := lnwire.NewFeatureVector(rawFeatures, lnwire.Features)
 
@@ -820,11 +820,11 @@ func (i *InvoiceRegistry) processAMP(ctx invoiceUpdateCtx) error {
 	// record.
 	amt := ctx.mpp.TotalMsat()
 
-	// Set the TLV and MPP optional features on the invoice. We'll also make
-	// the AMP features required so that it can't be paid by legacy or MPP
-	// htlcs.
+	// Set the TLV required and MPP optional features on the invoice. We'll
+	// also make the AMP features required so that it can't be paid by
+	// legacy or MPP htlcs.
 	rawFeatures := lnwire.NewRawFeatureVector(
-		lnwire.TLVOnionPayloadOptional,
+		lnwire.TLVOnionPayloadRequired,
 		lnwire.PaymentAddrOptional,
 		lnwire.AMPRequired,
 	)

invoices/invoiceregistry_test.go

@@ -1460,7 +1460,7 @@ func TestSettleInvoicePaymentAddrRequired(t *testing.T) {
 			Expiry:          time.Hour,
 			Features: lnwire.NewFeatureVector(
 				lnwire.NewRawFeatureVector(
-					lnwire.TLVOnionPayloadOptional,
+					lnwire.TLVOnionPayloadRequired,
 					lnwire.PaymentAddrRequired,
 				),
 				lnwire.Features,
@@ -1549,7 +1549,7 @@ func TestSettleInvoicePaymentAddrRequiredOptionalGrace(t *testing.T) {
 			Expiry:          time.Hour,
 			Features: lnwire.NewFeatureVector(
 				lnwire.NewRawFeatureVector(
-					lnwire.TLVOnionPayloadOptional,
+					lnwire.TLVOnionPayloadRequired,
 					lnwire.PaymentAddrOptional,
 				),
 				lnwire.Features,

itest/lnd_channel_force_close_test.go

@@ -245,7 +245,6 @@ func testChannelForceClosure(ht *lntest.HarnessTest) {
 	// We'll test the scenario for some of the commitment types, to ensure
 	// outputs can be swept.
 	commitTypes := []lnrpc.CommitmentType{
-		lnrpc.CommitmentType_LEGACY,
 		lnrpc.CommitmentType_ANCHORS,
 		lnrpc.CommitmentType_SIMPLE_TAPROOT,
 	}

itest/lnd_funding_test.go

@@ -32,7 +32,6 @@ func testBasicChannelFunding(ht *lntest.HarnessTest) {
 	// Run through the test with combinations of all the different
 	// commitment types.
 	allTypes := []lnrpc.CommitmentType{
-		lnrpc.CommitmentType_LEGACY,
 		lnrpc.CommitmentType_STATIC_REMOTE_KEY,
 		lnrpc.CommitmentType_ANCHORS,
 		lnrpc.CommitmentType_SIMPLE_TAPROOT,

itest/lnd_multi-hop_test.go

@@ -30,10 +30,6 @@ var commitWithZeroConf = []struct {
 	commitType lnrpc.CommitmentType
 	zeroConf   bool
 }{
-	{
-		commitType: lnrpc.CommitmentType_LEGACY,
-		zeroConf:   false,
-	},
 	{
 		commitType: lnrpc.CommitmentType_ANCHORS,
 		zeroConf:   false,

itest/lnd_payment_test.go

@@ -26,7 +26,6 @@ func testSendDirectPayment(ht *lntest.HarnessTest) {
 
 	// Create a list of commitment types we want to test.
 	commitTyes := []lnrpc.CommitmentType{
-		lnrpc.CommitmentType_LEGACY,
 		lnrpc.CommitmentType_ANCHORS,
 		lnrpc.CommitmentType_SIMPLE_TAPROOT,
 	}

itest/lnd_revocation_test.go

@@ -195,7 +195,6 @@ func breachRetributionTestCase(ht *lntest.HarnessTest,
 // the mempool.
 func testRevokedCloseRetribution(ht *lntest.HarnessTest) {
 	for _, commitType := range []lnrpc.CommitmentType{
-		lnrpc.CommitmentType_LEGACY,
 		lnrpc.CommitmentType_SIMPLE_TAPROOT,
 	} {
 		testName := fmt.Sprintf("%v", commitType.String())
@@ -378,7 +377,6 @@ func revokedCloseRetributionZeroValueRemoteOutputCase(ht *lntest.HarnessTest,
 // commitment output has zero-value.
 func testRevokedCloseRetributionZeroValueRemoteOutput(ht *lntest.HarnessTest) {
 	for _, commitType := range []lnrpc.CommitmentType{
-		lnrpc.CommitmentType_LEGACY,
 		lnrpc.CommitmentType_SIMPLE_TAPROOT,
 	} {
 		testName := fmt.Sprintf("%v", commitType.String())
@@ -700,7 +698,6 @@ func revokedCloseRetributionRemoteHodlCase(ht *lntest.HarnessTest,
 // remote party breaches before settling extended HTLCs.
 func testRevokedCloseRetributionRemoteHodl(ht *lntest.HarnessTest) {
 	for _, commitType := range []lnrpc.CommitmentType{
-		lnrpc.CommitmentType_LEGACY,
 		lnrpc.CommitmentType_SIMPLE_TAPROOT,
 	} {
 		testName := fmt.Sprintf("%v", commitType.String())

peer/brontide.go

@@ -754,6 +754,13 @@ func (p *Brontide) initGossipSync() {
 	if p.remoteFeatures.HasFeature(lnwire.GossipQueriesOptional) {
 		p.log.Info("Negotiated chan series queries")
 
+		if p.cfg.AuthGossiper == nil {
+			// This should only ever be hit in the unit tests.
+			p.log.Warn("No AuthGossiper configured. Abandoning " +
+				"gossip sync.")
+			return
+		}
+
 		// Register the peer's gossip syncer with the gossiper.
 		// This blocks synchronously to ensure the gossip syncer is
 		// registered with the gossiper before attempting to read

peer/brontide_test.go

@@ -1080,6 +1080,7 @@ func TestPeerCustomMessage(t *testing.T) {
 		initReplyMsg := lnwire.NewInitMessage(
 			lnwire.NewRawFeatureVector(
 				lnwire.DataLossProtectRequired,
+				lnwire.GossipQueriesOptional,
 			),
 			lnwire.NewRawFeatureVector(),
 		)

routing/integrated_routing_context_test.go

@@ -181,11 +181,13 @@ func (c *integratedRoutingContext) testPayment(maxParts uint32,
 		FeeLimit:       lnwire.MaxMilliSatoshi,
 		Target:         c.target.pubkey,
 		PaymentAddr:    &paymentAddr,
-		DestFeatures:   lnwire.NewFeatureVector(baseFeatureBits, nil),
-		Amount:         c.amt,
-		CltvLimit:      math.MaxUint32,
-		MaxParts:       maxParts,
-		RouteHints:     c.routeHints,
+		DestFeatures: lnwire.NewFeatureVector(
+			baseFeatureBits, lnwire.Features,
+		),
+		Amount:     c.amt,
+		CltvLimit:  math.MaxUint32,
+		MaxParts:   maxParts,
+		RouteHints: c.routeHints,
 	}
 
 	var paymentHash [32]byte

routing/integrated_routing_test.go

@@ -379,7 +379,7 @@ func TestPaymentAddrOnlyNoSplit(t *testing.T) {
 	twoPathGraph(ctx.graph, chanSize, chanSize)
 
 	payAddrOnlyFeatures := []lnwire.FeatureBit{
-		lnwire.TLVOnionPayloadOptional,
+		lnwire.TLVOnionPayloadRequired,
 		lnwire.PaymentAddrOptional,
 	}
 

routing/pathfind_test.go

@@ -60,7 +60,7 @@ var (
 
 	tlvFeatures = lnwire.NewFeatureVector(
 		lnwire.NewRawFeatureVector(
-			lnwire.TLVOnionPayloadOptional,
+			lnwire.TLVOnionPayloadRequired,
 		), lnwire.Features,
 	)
 
@@ -72,13 +72,13 @@ var (
 
 	tlvPayAddrFeatures = lnwire.NewFeatureVector(
 		lnwire.NewRawFeatureVector(
-			lnwire.TLVOnionPayloadOptional,
+			lnwire.TLVOnionPayloadRequired,
 			lnwire.PaymentAddrOptional,
 		), lnwire.Features,
 	)
 
 	mppFeatures = lnwire.NewRawFeatureVector(
-		lnwire.TLVOnionPayloadOptional,
+		lnwire.TLVOnionPayloadRequired,
 		lnwire.PaymentAddrOptional,
 		lnwire.MPPOptional,
 	)