From f40f4620f7da416c18a54bbbc36b51d24ab6b149 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 6 Sep 2019 13:14:39 +0200 Subject: [PATCH] lnwallet/channel: make ErrCommitSyncLocalDataLoss type This commit converts the ErrCommitSyncLocalDataLoss error into a struct, that also holds the received last unrevoked commit point from the remote party. --- htlcswitch/link.go | 7 ++++++- lnwallet/channel.go | 40 +++++++++++++++++++++++++++++----------- lnwallet/channel_test.go | 4 ++-- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index c7ef3800..5c283038 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -895,6 +895,11 @@ func (l *channelLink) htlcManager() { if l.cfg.SyncStates { err := l.syncChanStates() if err != nil { + log.Warnf("Error when syncing channel states: %v", err) + + _, localDataLoss := + err.(*lnwallet.ErrCommitSyncLocalDataLoss) + switch { case err == ErrLinkShuttingDown: log.Debugf("unable to sync channel states, " + @@ -936,7 +941,7 @@ func (l *channelLink) htlcManager() { // TODO(halseth): mark this, such that we prevent // channel from being force closed by the user or // contractcourt etc. - case err == lnwallet.ErrCommitSyncLocalDataLoss: + case localDataLoss: // We determined the commit chains were not possible to // sync. We cautiously fail the channel, but don't diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 00a7f0db..1b61a637 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -81,16 +81,6 @@ var ( ErrInvalidLocalUnrevokedCommitPoint = fmt.Errorf("unrevoked commit " + "point is invalid") - // ErrCommitSyncLocalDataLoss is returned in the case that we receive a - // valid commit secret within the ChannelReestablish message from the - // remote node AND they advertise a RemoteCommitTailHeight higher than - // our current known height. This means we have lost some critical - // data, and must fail the channel and MUST NOT force close it. Instead - // we should wait for the remote to force close it, such that we can - // attempt to sweep our funds. - ErrCommitSyncLocalDataLoss = fmt.Errorf("possible local commitment " + - "state data loss") - // ErrCommitSyncRemoteDataLoss is returned in the case that we receive // a ChannelReestablish message from the remote that advertises a // NextLocalCommitHeight that is lower than what they have already @@ -101,6 +91,30 @@ var ( "state data loss") ) +// ErrCommitSyncLocalDataLoss is returned in the case that we receive a valid +// commit secret within the ChannelReestablish message from the remote node AND +// they advertise a RemoteCommitTailHeight higher than our current known +// height. This means we have lost some critical data, and must fail the +// channel and MUST NOT force close it. Instead we should wait for the remote +// to force close it, such that we can attempt to sweep our funds. The +// commitment point needed to sweep the remote's force close is encapsuled. +type ErrCommitSyncLocalDataLoss struct { + // ChannelPoint is the identifier for the channel that experienced data + // loss. + ChannelPoint wire.OutPoint + + // CommitPoint is the last unrevoked commit point, sent to us by the + // remote when we determined we had lost state. + CommitPoint *btcec.PublicKey +} + +// Error returns a string representation of the local data loss error. +func (e *ErrCommitSyncLocalDataLoss) Error() string { + return fmt.Sprintf("ChannelPoint(%v) with CommitPoint(%x) had "+ + "possible local commitment state data loss", e.ChannelPoint, + e.CommitPoint.SerializeCompressed()) +} + // channelState is an enum like type which represents the current state of a // particular channel. // TODO(roasbeef): actually update state @@ -3313,7 +3327,11 @@ func (lc *LightningChannel) ProcessChanSyncMsg( if err != nil { return nil, nil, nil, err } - return nil, nil, nil, ErrCommitSyncLocalDataLoss + + return nil, nil, nil, &ErrCommitSyncLocalDataLoss{ + ChannelPoint: lc.channelState.FundingOutpoint, + CommitPoint: msg.LocalUnrevokedCommitPoint, + } // If the height of our commitment chain reported by the remote party // is behind our view of the chain, then they probably lost some state, diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 81077862..3a0281c5 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -3688,7 +3688,7 @@ func TestChanSyncFailure(t *testing.T) { // Alice should detect from Bob's message that she lost state. _, _, _, err = aliceOld.ProcessChanSyncMsg(bobSyncMsg) - if err != ErrCommitSyncLocalDataLoss { + if _, ok := err.(*ErrCommitSyncLocalDataLoss); !ok { t.Fatalf("wrong error, expected "+ "ErrCommitSyncLocalDataLoss instead got: %v", err) @@ -4377,7 +4377,7 @@ func TestChanSyncInvalidLastSecret(t *testing.T) { // Alice's former self should conclude that she possibly lost data as // Bob is sending a valid commit secret for the latest state. _, _, _, err = aliceOld.ProcessChanSyncMsg(bobChanSync) - if err != ErrCommitSyncLocalDataLoss { + if _, ok := err.(*ErrCommitSyncLocalDataLoss); !ok { t.Fatalf("wrong error, expected ErrCommitSyncLocalDataLoss "+ "instead got: %v", err) }