From a6d22635692a2bbcbc49e19b79f0e9e58e61ddbb Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 8 Oct 2018 15:46:13 +0200 Subject: [PATCH 1/4] htlcswitch: add PermanentFailure to LinkFailureError --- htlcswitch/linkfailure.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/htlcswitch/linkfailure.go b/htlcswitch/linkfailure.go index 840a4d8da..32aa83ade 100644 --- a/htlcswitch/linkfailure.go +++ b/htlcswitch/linkfailure.go @@ -57,6 +57,10 @@ type LinkFailureError struct { // because of this error. ForceClose bool + // PermanentFailure indicates whether this failure is permanent, and + // the channel should not be attempted loaded again. + PermanentFailure bool + // SendData is a byte slice that will be sent to the peer. If nil a // generic error will be sent. SendData []byte From a97182b1e9944e91b5ea5fa07157f5ca9d7dcd4d Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 8 Oct 2018 15:46:14 +0200 Subject: [PATCH 2/4] peer: mark channel borked in case of PermanentFailure --- peer/brontide.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/peer/brontide.go b/peer/brontide.go index c50f4ebae..12780ee49 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -2496,6 +2496,13 @@ type linkFailureReport struct { // force closing the channel depending on severity, and sending the error // message back to the remote party. func (p *Brontide) handleLinkFailure(failure linkFailureReport) { + // Retrieve the channel from the map of active channels. We do this to + // have access to it even after WipeChannel remove it from the map. + chanID := lnwire.NewChanIDFromOutPoint(&failure.chanPoint) + p.activeChanMtx.Lock() + lnChan := p.activeChannels[chanID] + p.activeChanMtx.Unlock() + // We begin by wiping the link, which will remove it from the switch, // such that it won't be attempted used for any more updates. // @@ -2524,6 +2531,17 @@ func (p *Brontide) handleLinkFailure(failure linkFailureReport) { } } + // If this is a permanent failure, we will mark the channel borked. + if failure.linkErr.PermanentFailure && lnChan != nil { + peerLog.Warnf("Marking link(%v) borked due to permanent "+ + "failure", failure.shortChanID) + + if err := lnChan.State().MarkBorked(); err != nil { + peerLog.Errorf("Unable to mark channel %v borked: %v", + failure.shortChanID, err) + } + } + // Send an error to the peer, why we failed the channel. if failure.linkErr.ShouldSendToPeer() { // If SendData is set, send it to the peer. If not, we'll use From dde6763cef4d09130146770083370eca0a770546 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 8 Oct 2018 15:46:14 +0200 Subject: [PATCH 3/4] htlcswitch/link: set PermanentFailure in case of remote error --- htlcswitch/link.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 974e3cc19..f43bc3a42 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1900,9 +1900,14 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) { // Error received from remote, MUST fail channel, but should // only print the contents of the error message if all // characters are printable ASCII. - l.fail(LinkFailureError{code: ErrRemoteError}, + l.fail( + LinkFailureError{ + code: ErrRemoteError, + PermanentFailure: true, + }, "ChannelPoint(%v): received error from peer: %v", - l.channel.ChannelPoint(), msg.Error()) + l.channel.ChannelPoint(), msg.Error(), + ) default: l.log.Warnf("received unknown message of type %T", msg) } From c3397d7e77dc7fd32622e4ddfb5b0801e50c6b41 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 8 Oct 2018 15:56:25 +0200 Subject: [PATCH 4/4] htlcswitch/link test: add test for permanent failure --- htlcswitch/link_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 5bf076e9f..970054ca9 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -5129,6 +5129,11 @@ func TestChannelLinkFail(t *testing.T) { // force close the channel in response to the actions performed // during the linkTest. shouldForceClose bool + + // permanentFailure indicates whether we expect the link to + // consider the failure permanent in response to the actions + // performed during the linkTest. + permanentFailure bool }{ { // Test that we don't force close if syncing states @@ -5144,6 +5149,7 @@ func TestChannelLinkFail(t *testing.T) { // Should fail at startup. }, false, + false, }, { // Test that we don't force closes the channel if @@ -5160,6 +5166,7 @@ func TestChannelLinkFail(t *testing.T) { // Should fail at startup. }, false, + false, }, { // Test that we force close the channel if we receive @@ -5176,6 +5183,7 @@ func TestChannelLinkFail(t *testing.T) { c.HandleChannelUpdate(htlcSettle) }, true, + false, }, { // Test that we force close the channel if we receive @@ -5213,6 +5221,7 @@ func TestChannelLinkFail(t *testing.T) { c.HandleChannelUpdate(commitSig) }, true, + false, }, { // Test that we force close the channel if we receive @@ -5252,6 +5261,19 @@ func TestChannelLinkFail(t *testing.T) { c.HandleChannelUpdate(commitSig) }, true, + false, + }, + { + // Test that we 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) { + err := &lnwire.Error{} + c.HandleChannelUpdate(err) + }, + false, + true, }, } @@ -5302,6 +5324,12 @@ func TestChannelLinkFail(t *testing.T) { linkErr.ForceClose) } + if test.permanentFailure != linkErr.PermanentFailure { + t.Fatalf("%d) Expected Alice set permanent failure(%v), "+ + "instead got(%v)", i, test.permanentFailure, + linkErr.PermanentFailure) + } + // Clean up before starting next test case. cleanUp() }