lnwallet+link: move marking channel states to link

Instead of marking the database state when processing the channel
reestablishment message, we wait for the result of this processing to
arrive in the link, and mark it accordingly in the database here.

We do this move the logic determining whether we should force close the
channel or not, and what state to mark it in the DB, to the same place,
as these need to be consistent.

htlcswitch/link.go

@@ -897,7 +897,7 @@ func (l *channelLink) htlcManager() {
 		if err != nil {
 			log.Warnf("Error when syncing channel states: %v", err)
 
-			_, localDataLoss :=
+			errDataLoss, localDataLoss :=
 				err.(*lnwallet.ErrCommitSyncLocalDataLoss)
 
 			switch {
@@ -922,6 +922,12 @@ func (l *channelLink) htlcManager() {
 			// what they sent us before.
 			// TODO(halseth): ban peer?
 			case err == lnwallet.ErrInvalidLocalUnrevokedCommitPoint:
+				err = l.channel.MarkBorked()
+				if err != nil {
+					log.Errorf("Unable to mark channel "+
+						"borked: %v", err)
+				}
+
 				l.fail(
 					LinkFailureError{
 						code:       ErrSyncError,
@@ -935,13 +941,18 @@ func (l *channelLink) htlcManager() {
 			// We have lost state and cannot safely force close the
 			// channel. Fail the channel and wait for the remote to
 			// hopefully force close it. The remote has sent us its
-			// latest unrevoked commitment point, that we stored in
+			// latest unrevoked commitment point, and we'll store
-			// the database, that we can use to retrieve the funds
+			// it in the database, such that we can attempt to
-			// when the remote closes the channel.
+			// recover the funds if the remote force closes the
-			// TODO(halseth): mark this, such that we prevent
+			// channel.
-			// channel from being force closed by the user or
-			// contractcourt etc.
 			case localDataLoss:
+				err := l.channel.MarkDataLoss(
+					errDataLoss.CommitPoint,
+				)
+				if err != nil {
+					log.Errorf("Unable to mark channel "+
+						"data loss: %v", err)
+				}
 
 			// We determined the commit chains were not possible to
 			// sync. We cautiously fail the channel, but don't
@@ -949,6 +960,10 @@ func (l *channelLink) htlcManager() {
 			// TODO(halseth): can we safely force close in any
 			// cases where this error is returned?
 			case err == lnwallet.ErrCannotSyncCommitChains:
+				if err := l.channel.MarkBorked(); err != nil {
+					log.Errorf("Unable to mark channel "+
+						"borked: %v", err)
+				}
 
 			// Other, unspecified error.
 			default:

lnwallet/channel.go

@@ -3307,10 +3307,6 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 			// doesn't support data loss protection. In either case
 			// it is not safe for us to keep using the channel, so
 			// we mark it borked and fail the channel.
-			if err := lc.channelState.MarkBorked(); err != nil {
-				return nil, nil, nil, err
-			}
-
 			walletLog.Errorf("ChannelPoint(%v), sync failed: "+
 				"local data loss, but no recovery option.",
 				lc.channelState.FundingOutpoint)
@@ -3318,16 +3314,7 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 		}
 
 		// In this case, we've likely lost data and shouldn't proceed
-		// with channel updates. So we'll store the commit point we
+		// with channel updates.
-		// were given in the database, such that we can attempt to
-		// recover the funds if the remote force closes the channel.
-		err := lc.channelState.MarkDataLoss(
-			msg.LocalUnrevokedCommitPoint,
-		)
-		if err != nil {
-			return nil, nil, nil, err
-		}
-
 		return nil, nil, nil, &ErrCommitSyncLocalDataLoss{
 			ChannelPoint: lc.channelState.FundingOutpoint,
 			CommitPoint:  msg.LocalUnrevokedCommitPoint,
@@ -3341,10 +3328,6 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 			"believes our tail height is %v, while we have %v!",
 			lc.channelState.FundingOutpoint,
 			msg.RemoteCommitTailHeight, localTailHeight)
-
-		if err := lc.channelState.MarkBorked(); err != nil {
-			return nil, nil, nil, err
-		}
 		return nil, nil, nil, ErrCommitSyncRemoteDataLoss
 
 	// Their view of our commit chain is consistent with our view.
@@ -3408,10 +3391,6 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 			"believes our tail height is %v, while we have %v!",
 			lc.channelState.FundingOutpoint,
 			msg.RemoteCommitTailHeight, localTailHeight)
-
-		if err := lc.channelState.MarkBorked(); err != nil {
-			return nil, nil, nil, err
-		}
 		return nil, nil, nil, ErrCannotSyncCommitChains
 	}
 
@@ -3430,9 +3409,6 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 			lc.channelState.FundingOutpoint,
 			msg.NextLocalCommitHeight, remoteTipHeight)
 
-		if err := lc.channelState.MarkBorked(); err != nil {
-			return nil, nil, nil, err
-		}
 		return nil, nil, nil, ErrCannotSyncCommitChains
 
 	// They are waiting for a state they have already ACKed.
@@ -3444,9 +3420,6 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 
 		// They previously ACKed our current tail, and now they are
 		// waiting for it. They probably lost state.
-		if err := lc.channelState.MarkBorked(); err != nil {
-			return nil, nil, nil, err
-		}
 		return nil, nil, nil, ErrCommitSyncRemoteDataLoss
 
 	// They have received our latest commitment, life is good.
@@ -3492,10 +3465,6 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 			"next commit height is %v, while we believe it is %v!",
 			lc.channelState.FundingOutpoint,
 			msg.NextLocalCommitHeight, remoteTipHeight)
-
-		if err := lc.channelState.MarkBorked(); err != nil {
-			return nil, nil, nil, err
-		}
 		return nil, nil, nil, ErrCannotSyncCommitChains
 	}
 
@@ -3533,12 +3502,6 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 			"sent invalid commit point for height %v!",
 			lc.channelState.FundingOutpoint,
 			msg.NextLocalCommitHeight)
-
-		if err := lc.channelState.MarkBorked(); err != nil {
-			return nil, nil, nil, err
-		}
-
-		// TODO(halseth): force close?
 		return nil, nil, nil, ErrInvalidLocalUnrevokedCommitPoint
 	}
 
@@ -6274,6 +6237,16 @@ func (lc *LightningChannel) State() *channeldb.OpenChannel {
 	return lc.channelState
 }
 
+// MarkBorked marks the event when the channel as reached an irreconcilable
+// state, such as a channel breach or state desynchronization. Borked channels
+// should never be added to the switch.
+func (lc *LightningChannel) MarkBorked() error {
+	lc.Lock()
+	defer lc.Unlock()
+
+	return lc.channelState.MarkBorked()
+}
+
 // MarkCommitmentBroadcasted marks the channel as a commitment transaction has
 // been broadcast, either our own or the remote, and we should watch the chain
 // for it to confirm before taking any further action.
@@ -6284,6 +6257,16 @@ func (lc *LightningChannel) MarkCommitmentBroadcasted(tx *wire.MsgTx) error {
 	return lc.channelState.MarkCommitmentBroadcasted(tx)
 }
 
+// MarkDataLoss marks sets the channel status to LocalDataLoss and stores the
+// passed commitPoint for use to retrieve funds in case the remote force closes
+// the channel.
+func (lc *LightningChannel) MarkDataLoss(commitPoint *btcec.PublicKey) error {
+	lc.Lock()
+	defer lc.Unlock()
+
+	return lc.channelState.MarkDataLoss(commitPoint)
+}
+
 // ActiveHtlcs returns a slice of HTLC's which are currently active on *both*
 // commitment transactions.
 func (lc *LightningChannel) ActiveHtlcs() []channeldb.HTLC {