Fix warning messages

docs/release-notes/release-notes-0.16.0.md

@@ -106,6 +106,8 @@ https://github.com/lightningnetwork/lnd/pull/6963/)
 * [Create a helper function to wait for peer to come
   online](https://github.com/lightningnetwork/lnd/pull/6931).
 
+* [Stop handling peer warning messages as errors](https://github.com/lightningnetwork/lnd/pull/6840)
+
 ## `lncli`
 * [Add an `insecure` flag to skip tls auth as well as a `metadata` string slice
   flag](https://github.com/lightningnetwork/lnd/pull/6818) that allows the
@@ -200,6 +202,7 @@ details.
 * Jordi Montes
 * Matt Morehouse
 * Michael Street
+* Jordi Montes
 * Olaoluwa Osuntokun
 * Oliver Gugger
 * Priyansh Rastogi

funding/manager.go

@@ -859,15 +859,23 @@ func (f *Manager) reservationCoordinator() {
 			switch msg := fmsg.msg.(type) {
 			case *lnwire.OpenChannel:
 				f.handleFundingOpen(fmsg.peer, msg)
+
 			case *lnwire.AcceptChannel:
 				f.handleFundingAccept(fmsg.peer, msg)
+
 			case *lnwire.FundingCreated:
 				f.handleFundingCreated(fmsg.peer, msg)
+
 			case *lnwire.FundingSigned:
 				f.handleFundingSigned(fmsg.peer, msg)
+
 			case *lnwire.FundingLocked:
 				f.wg.Add(1)
 				go f.handleFundingLocked(fmsg.peer, msg)
+
+			case *lnwire.Warning:
+				f.handleWarningMsg(fmsg.peer, msg)
+
 			case *lnwire.Error:
 				f.handleErrorMsg(fmsg.peer, msg)
 			}
@@ -4193,12 +4201,16 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) {
 	}
 }
 
+// handleWarningMsg processes the warning which was received from remote peer.
+func (f *Manager) handleWarningMsg(peer lnpeer.Peer, msg *lnwire.Warning) {
+	log.Warnf("received warning message from peer %x: %v",
+		peer.IdentityKey().SerializeCompressed(), msg.Warning())
+}
+
 // handleErrorMsg processes the error which was received from remote peer,
 // depending on the type of error we should do different clean up steps and
 // inform the user about it.
-func (f *Manager) handleErrorMsg(peer lnpeer.Peer,
-	msg *lnwire.Error) {
-
+func (f *Manager) handleErrorMsg(peer lnpeer.Peer, msg *lnwire.Error) {
 	chanID := msg.ChanID
 	peerKey := peer.IdentityKey()
 

funding/manager_test.go

@@ -774,6 +774,14 @@ func fundChannel(t *testing.T, alice, bob *testNode, localFundingAmt,
 	// Forward the response to Alice.
 	alice.fundingMgr.ProcessFundingMsg(acceptChannelResponse, bob)
 
+	// Check that sending warning messages does not abort the funding
+	// process.
+	warningMsg := &lnwire.Warning{
+		Data: []byte("random warning"),
+	}
+	alice.fundingMgr.ProcessFundingMsg(warningMsg, bob)
+	bob.fundingMgr.ProcessFundingMsg(warningMsg, alice)
+
 	// Alice responds with a FundingCreated message.
 	fundingCreated := assertFundingMsgSent(
 		t, alice.msgChan, "FundingCreated",

htlcswitch/link.go

@@ -2069,7 +2069,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 	// although we "MAY" do so according to the specification.
 	case *lnwire.Warning:
 		l.log.Warnf("received warning message from peer: %v",
-			msg.Error.Error())
+			msg.Warning())
 
 	case *lnwire.Error:
 		// Error received from remote, MUST fail channel, but should

htlcswitch/link_test.go

@@ -5196,13 +5196,21 @@ func TestChannelLinkFail(t *testing.T) {
 	t.Parallel()
 
 	testCases := []struct {
+		// name is the description for this test case.
+		name string
+
 		// options is used to set up mocks and configure the link
 		// before it is started.
 		options func(*channelLink)
 
 		// link test is used to execute the given test on the channel
 		// link after it is started.
-		linkTest func(*testing.T, *channelLink, *lnwallet.LightningChannel)
+		linkTest func(*testing.T, *channelLink,
+			*lnwallet.LightningChannel)
+
+		// shouldFail indicates whether or not the link should fail
+		// during this test case.
+		shouldFail bool
 
 		// shouldForceClose indicates whether we expect the link to
 		// force close the channel in response to the actions performed
@@ -5215,24 +5223,43 @@ func TestChannelLinkFail(t *testing.T) {
 		permanentFailure bool
 	}{
 		{
-			// Test that we don't force close if syncing states
-			// fails at startup.
+			"don't fail the channel if we receive a warning " +
+				"message",
+			func(c *channelLink) {
+			},
+			func(_ *testing.T, c *channelLink,
+				_ *lnwallet.LightningChannel) {
+
+				warningMsg := &lnwire.Warning{
+					Data: []byte("random warning"),
+				}
+				c.HandleChannelUpdate(warningMsg)
+			},
+			false,
+			false,
+			false,
+		},
+		{
+			"don't force close if syncing states fails at startup",
 			func(c *channelLink) {
 				c.cfg.SyncStates = true
 
 				// Make the syncChanStateCall fail by making
 				// the SendMessage call fail.
 				c.cfg.Peer.(*mockPeer).disconnected = true
 			},
-			func(t *testing.T, c *channelLink, _ *lnwallet.LightningChannel) {
+			func(*testing.T, *channelLink,
+				*lnwallet.LightningChannel) { // nolint:whitespace,lll
+
 				// Should fail at startup.
 			},
+			true,
 			false,
 			false,
 		},
 		{
-			// Test that we don't force closes the channel if
-			// resolving forward packages fails at startup.
+			"we don't force closes the channel if resolving " +
+				"forward packages fails at startup",
 			func(c *channelLink) {
 				// We make the call to resolveFwdPkgs fail by
 				// making the underlying forwarder fail.
@@ -5241,18 +5268,23 @@ func TestChannelLinkFail(t *testing.T) {
 				}
 				c.channel.State().Packager = pkg
 			},
-			func(t *testing.T, c *channelLink, _ *lnwallet.LightningChannel) {
+			func(*testing.T, *channelLink,
+				*lnwallet.LightningChannel) { // nolint:whitespace,lll
+
 				// Should fail at startup.
 			},
+			true,
 			false,
 			false,
 		},
 		{
-			// Test that we don't force close the channel if we
-			// receive an invalid Settle message.
+			"don't force close the channel if we receive an " +
+				"invalid Settle message",
 			func(c *channelLink) {
 			},
-			func(t *testing.T, c *channelLink, _ *lnwallet.LightningChannel) {
+			func(_ *testing.T, c *channelLink,
+				_ *lnwallet.LightningChannel) {
+
 				// Recevive an htlc settle for an htlc that was
 				// never added.
 				htlcSettle := &lnwire.UpdateFulfillHTLC{
@@ -5261,16 +5293,17 @@ func TestChannelLinkFail(t *testing.T) {
 				}
 				c.HandleChannelUpdate(htlcSettle)
 			},
+			true,
 			false,
 			false,
 		},
 		{
-			// Test that we force close the channel if we receive
-			// an invalid CommitSig, not containing enough HTLC
-			// sigs.
+			"force close the channel if we receive an invalid " +
+				"CommitSig, not containing enough HTLC sigs",
 			func(c *channelLink) {
 			},
-			func(t *testing.T, c *channelLink, remoteChannel *lnwallet.LightningChannel) {
+			func(_ *testing.T, c *channelLink,
+				remoteChannel *lnwallet.LightningChannel) {
 
 				// Generate an HTLC and send to the link.
 				htlc1 := generateHtlc(t, c, 0)
@@ -5300,15 +5333,18 @@ func TestChannelLinkFail(t *testing.T) {
 				c.HandleChannelUpdate(commitSig)
 			},
 			true,
+			true,
 			false,
 		},
 		{
-			// Test that we force close the channel if we receive
-			// an invalid CommitSig, where the sig itself is
-			// corrupted.
+			"force close the channel if we receive an invalid " +
+				"CommitSig, where the sig itself is corrupted",
 			func(c *channelLink) {
 			},
-			func(t *testing.T, c *channelLink, remoteChannel *lnwallet.LightningChannel) {
+			func(t *testing.T, c *channelLink,
+				remoteChannel *lnwallet.LightningChannel) {
+
+				t.Helper()
 
 				// Generate an HTLC and send to the link.
 				htlc1 := generateHtlc(t, c, 0)
@@ -5340,17 +5376,21 @@ func TestChannelLinkFail(t *testing.T) {
 				c.HandleChannelUpdate(commitSig)
 			},
 			true,
+			true,
 			false,
 		},
 		{
-			// Test that we consider the failure permanent if we
-			// receive a link error from the remote.
+			"consider the failure permanent if we receive a link " +
+				"error from the remote",
 			func(c *channelLink) {
 			},
-			func(t *testing.T, c *channelLink, remoteChannel *lnwallet.LightningChannel) {
+			func(_ *testing.T, c *channelLink,
+				remoteChannel *lnwallet.LightningChannel) {
+
 				err := &lnwire.Error{}
 				c.HandleChannelUpdate(err)
 			},
+			true,
 			false,
 			// TODO(halseth) For compatibility with CL we currently
 			// don't treat Errors as permanent errors.
@@ -5361,12 +5401,10 @@ func TestChannelLinkFail(t *testing.T) {
 	const chanAmt = btcutil.SatoshiPerBitcoin * 5
 
 	// Execute each test case.
-	for i, test := range testCases {
+	for _, test := range testCases {
 		link, remoteChannel, _, start, _, err :=
 			newSingleLinkTestHarness(t, chanAmt, 0)
-		if err != nil {
-			t.Fatalf("unable to create link: %v", err)
-		}
+		require.NoError(t, err, test.name)
 
 		coreLink := link.(*channelLink)
 
@@ -5375,41 +5413,50 @@ func TestChannelLinkFail(t *testing.T) {
 		linkErrors := make(chan LinkFailureError, 1)
 		coreLink.cfg.OnChannelFailure = func(_ lnwire.ChannelID,
 			_ lnwire.ShortChannelID, linkErr LinkFailureError) {
+
 			linkErrors <- linkErr
 		}
 
 		// Set up the link before starting it.
 		test.options(coreLink)
-		if err := start(); err != nil {
-			t.Fatalf("unable to start test harness: %v", err)
-		}
+		err = start()
+		require.NoError(t, err, test.name)
 
 		// Execute the test case.
 		test.linkTest(t, coreLink, remoteChannel)
 
 		// Currently we expect all test cases to lead to link error.
 		var linkErr LinkFailureError
+		errReceived := false
 		select {
 		case linkErr = <-linkErrors:
+			errReceived = true
+
 		case <-time.After(10 * time.Second):
-			t.Fatalf("%d) Alice did not fail"+
-				"channel", i)
+			// If we do not receive a link error in 10s we assume
+			// that we won't receive any.
 		}
 
+		require.Equal(t, test.shouldFail, errReceived, test.name)
+
+		// Check that the link is up and return.
+		if !test.shouldFail {
+			require.False(t, coreLink.failed)
+			return
+		}
+
+		require.True(t, coreLink.failed)
+
 		// If we expect the link to force close the channel in this
 		// case, check that it happens. If not, make sure it does not
 		// happen.
-		if test.shouldForceClose != linkErr.ForceClose {
-			t.Fatalf("%d) Expected Alice to force close(%v), "+
-				"instead got(%v)", i, test.shouldForceClose,
-				linkErr.ForceClose)
-		}
-
-		if test.permanentFailure != linkErr.PermanentFailure {
-			t.Fatalf("%d) Expected Alice set permanent failure(%v), "+
-				"instead got(%v)", i, test.permanentFailure,
-				linkErr.PermanentFailure)
-		}
+		require.Equal(
+			t, test.shouldForceClose, linkErr.ForceClose, test.name,
+		)
+		require.Equal(
+			t, test.permanentFailure, linkErr.PermanentFailure,
+			test.name,
+		)
 	}
 }