From 71228a6b06d95020682f21c47be70e6ea3cc16b9 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 4 May 2018 14:19:17 +0200 Subject: [PATCH] lnwallet/channel: don't use commit height to determine FullySynced This commit removes a faulty check we did to determine if the channel commitments were fully synced. We assumed that if out local commitment chain had a height higher than the remote, then we would have state updates present in our chain but not in theirs, and owed a commitment. However, there were cases where this wasn't true, and we would send a new commitment even though we had no new updates to sign. This is a protocol violation. Now we don't longer check the heights to determine if we are fully synced. A consequence of this is that we also need to check if we have any pending fee updates that are nopt yet signed, as those are considered non-empty updates. --- lnwallet/channel.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index f249e555..b048d270 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -3924,15 +3924,27 @@ func (lc *LightningChannel) FullySynced() bool { lastLocalCommit := lc.localCommitChain.tip() lastRemoteCommit := lc.remoteCommitChain.tip() - oweCommitment := lastLocalCommit.height > lastRemoteCommit.height - localUpdatesSynced := (lastLocalCommit.ourMessageIndex == lastRemoteCommit.ourMessageIndex) remoteUpdatesSynced := (lastLocalCommit.theirMessageIndex == lastRemoteCommit.theirMessageIndex) - return !oweCommitment && localUpdatesSynced && remoteUpdatesSynced + pendingFeeAck := false + + // If we have received a fee update which we haven't yet ACKed, then + // we owe a commitment. + if !lc.channelState.IsInitiator { + pendingFeeAck = lc.pendingAckFeeUpdate != nil + } + + // If we have sent a fee update which we haven't yet signed, then + // we owe a commitment. + if lc.channelState.IsInitiator { + pendingFeeAck = lc.pendingFeeUpdate != nil + } + + return localUpdatesSynced && remoteUpdatesSynced && !pendingFeeAck } // RevokeCurrentCommitment revokes the next lowest unrevoked commitment