From 517ad4e4f563c3dbf280b1ec4e3c1e076f14b2ab Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Tue, 24 Sep 2019 14:33:59 +0200
Subject: [PATCH 1/8] lnwallet: log empty commit sig event

To facilitate the logging, this commit adds a new OweCommitment method.
For the logging, we only need to consider the remote perspective. In a
later commit, we'll also start using the local perspective to support
the decision to send another signature.
---
 lnwallet/channel.go | 85 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index ddd8cd41..5a87aca8 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -3233,6 +3233,14 @@ func (lc *LightningChannel) SignNextCommitment() (lnwire.Sig, []lnwire.Sig, []ch
 	lc.Lock()
 	defer lc.Unlock()
 
+	// Check for empty commit sig. This should never happen, but we don't
+	// dare to fail hard here. We assume peers can deal with the empty sig
+	// and continue channel operation. We log an error so that the bug
+	// causing this can be tracked down.
+	if !lc.oweCommitment(true) {
+		lc.log.Errorf("sending empty commit sig")
+	}
+
 	var (
 		sig      lnwire.Sig
 		htlcSigs []lnwire.Sig
@@ -3987,6 +3995,18 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig,
 	lc.Lock()
 	defer lc.Unlock()
 
+	// Check for empty commit sig. Because of a previously existing bug, it
+	// is possible that we receive an empty commit sig from nodes running an
+	// older version. This is a relaxation of the spec, but it is still
+	// possible to handle it. To not break any channels with those older
+	// nodes, we just log the event. This check is also not totally
+	// reliable, because it could be that we've sent out a new sig, but the
+	// remote hasn't received it yet. We could then falsely assume that they
+	// should add our updates to their remote commitment tx.
+	if !lc.oweCommitment(false) {
+		lc.log.Warnf("empty commit sig message received")
+	}
+
 	// Determine the last update on the local log that has been locked in.
 	localACKedIndex := lc.remoteCommitChain.tail().ourMessageIndex
 	localHtlcIndex := lc.remoteCommitChain.tail().ourHtlcIndex
@@ -4140,6 +4160,71 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig,
 	return nil
 }
 
+// OweCommitment returns a boolean value reflecting whether we need to send
+// out a commitment signature because there are outstanding local updates and/or
+// updates in the local commit tx that aren't reflected in the remote commit tx
+// yet.
+func (lc *LightningChannel) OweCommitment(local bool) bool {
+	lc.RLock()
+	defer lc.RUnlock()
+
+	return lc.oweCommitment(local)
+}
+
+// oweCommitment is the internal version of OweCommitment. This function expects
+// to be executed with a lock held.
+func (lc *LightningChannel) oweCommitment(local bool) bool {
+	var (
+		remoteUpdatesPending, localUpdatesPending bool
+
+		lastLocalCommit  = lc.localCommitChain.tip()
+		lastRemoteCommit = lc.remoteCommitChain.tip()
+
+		perspective string
+	)
+
+	if local {
+		perspective = "local"
+
+		// There are local updates pending if our local update log is
+		// not in sync with our remote commitment tx.
+		localUpdatesPending = lc.localUpdateLog.logIndex !=
+			lastRemoteCommit.ourMessageIndex
+
+		// There are remote updates pending if their remote commitment
+		// tx (our local commitment tx) contains updates that we don't
+		// have added to our remote commitment tx yet.
+		remoteUpdatesPending = lastLocalCommit.theirMessageIndex !=
+			lastRemoteCommit.theirMessageIndex
+
+	} else {
+		perspective = "remote"
+
+		// There are local updates pending (local updates from the
+		// perspective of the remote party) if the remote party has
+		// updates to their remote tx pending for which they haven't
+		// signed yet.
+		localUpdatesPending = lc.remoteUpdateLog.logIndex !=
+			lastLocalCommit.theirMessageIndex
+
+		// There are remote updates pending (remote updates from the
+		// perspective of the remote party) if we have updates on our
+		// remote commitment tx that they haven't added to theirs yet.
+		remoteUpdatesPending = lastRemoteCommit.ourMessageIndex !=
+			lastLocalCommit.ourMessageIndex
+	}
+
+	// If any of the conditions above is true, we owe a commitment
+	// signature.
+	oweCommitment := localUpdatesPending || remoteUpdatesPending
+
+	lc.log.Tracef("%v owes commit: %v (local updates: %v, "+
+		"remote updates %v)", perspective, oweCommitment,
+		localUpdatesPending, remoteUpdatesPending)
+
+	return oweCommitment
+}
+
 // FullySynced returns a boolean value reflecting if both commitment chains
 // (remote+local) are fully in sync. Both commitment chains are fully in sync
 // if the tip of each chain includes the latest committed changes from both

From 64f4421d6c2eb388474b651918c9c625e98ada3b Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Wed, 6 Nov 2019 09:16:10 +0100
Subject: [PATCH 2/8] htlcswitch/test: add test cases that triggers empty
 commit sig

Co-authored-by: Johan T. Halseth <johanth@gmail.com>
---
 htlcswitch/link_isolated_test.go |  34 ++++++-
 htlcswitch/link_test.go          | 158 +++++++++++++++++++++++++++++++
 2 files changed, 187 insertions(+), 5 deletions(-)

diff --git a/htlcswitch/link_isolated_test.go b/htlcswitch/link_isolated_test.go
index 8f059d74..f108853f 100644
--- a/htlcswitch/link_isolated_test.go
+++ b/htlcswitch/link_isolated_test.go
@@ -139,6 +139,21 @@ func (l *linkTestContext) receiveRevAndAckAliceToBob() {
 func (l *linkTestContext) receiveCommitSigAliceToBob(expHtlcs int) {
 	l.t.Helper()
 
+	comSig := l.receiveCommitSigAlice(expHtlcs)
+
+	err := l.bobChannel.ReceiveNewCommitment(
+		comSig.CommitSig, comSig.HtlcSigs,
+	)
+	if err != nil {
+		l.t.Fatalf("bob failed receiving commitment: %v", err)
+	}
+}
+
+// receiveCommitSigAlice waits for Alice to send a CommitSig, signing expHtlcs
+// numbers of HTLCs.
+func (l *linkTestContext) receiveCommitSigAlice(expHtlcs int) *lnwire.CommitSig {
+	l.t.Helper()
+
 	var msg lnwire.Message
 	select {
 	case msg = <-l.aliceMsgs:
@@ -155,11 +170,8 @@ func (l *linkTestContext) receiveCommitSigAliceToBob(expHtlcs int) {
 		l.t.Fatalf("expected %d htlc sigs, got %d", expHtlcs,
 			len(comSig.HtlcSigs))
 	}
-	err := l.bobChannel.ReceiveNewCommitment(comSig.CommitSig,
-		comSig.HtlcSigs)
-	if err != nil {
-		l.t.Fatalf("bob failed receiving commitment: %v", err)
-	}
+
+	return comSig
 }
 
 // sendRevAndAckBobToAlice make Bob revoke his current commitment, then hand
@@ -242,3 +254,15 @@ func (l *linkTestContext) receiveFailAliceToBob() {
 		l.t.Fatalf("unable to apply received fail htlc: %v", err)
 	}
 }
+
+// assertNoMsgFromAlice asserts that Alice hasn't sent a message. Before
+// calling, make sure that Alice has had the opportunity to send the message.
+func (l *linkTestContext) assertNoMsgFromAlice(timeout time.Duration) {
+	l.t.Helper()
+
+	select {
+	case msg := <-l.aliceMsgs:
+		l.t.Fatalf("unexpected message from Alice: %v", msg)
+	case <-time.After(timeout):
+	}
+}
diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go
index 93eb3763..68d425b5 100644
--- a/htlcswitch/link_test.go
+++ b/htlcswitch/link_test.go
@@ -4620,6 +4620,92 @@ func TestChannelLinkWaitForRevocation(t *testing.T) {
 	assertNoMsgFromAlice()
 }
 
+// TestChannelLinkNoEmptySig asserts that no empty commit sig message is sent
+// when the commitment txes are out of sync.
+func TestChannelLinkNoEmptySig(t *testing.T) {
+	t.Parallel()
+
+	const chanAmt = btcutil.SatoshiPerBitcoin * 5
+	const chanReserve = btcutil.SatoshiPerBitcoin * 1
+	aliceLink, bobChannel, batchTicker, start, cleanUp, _, err :=
+		newSingleLinkTestHarness(chanAmt, chanReserve)
+	if err != nil {
+		t.Fatalf("unable to create link: %v", err)
+	}
+	defer cleanUp()
+
+	if err := start(); err != nil {
+		t.Fatalf("unable to start test harness: %v", err)
+	}
+	defer aliceLink.Stop()
+
+	var (
+		coreLink  = aliceLink.(*channelLink)
+		aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs
+	)
+
+	ctx := linkTestContext{
+		t:          t,
+		aliceLink:  aliceLink,
+		aliceMsgs:  aliceMsgs,
+		bobChannel: bobChannel,
+	}
+
+	// Send htlc 1 from Alice to Bob.
+	htlc1, _ := generateHtlcAndInvoice(t, 0)
+	ctx.sendHtlcAliceToBob(0, htlc1)
+	ctx.receiveHtlcAliceToBob()
+
+	// Tick the batch ticker to trigger a commitsig from Alice->Bob.
+	select {
+	case batchTicker <- time.Now():
+	case <-time.After(5 * time.Second):
+		t.Fatalf("could not force commit sig")
+	}
+
+	// Receive a CommitSig from Alice covering the Add from above.
+	ctx.receiveCommitSigAliceToBob(1)
+
+	// Bob revokes previous commitment tx.
+	ctx.sendRevAndAckBobToAlice()
+
+	// Alice sends htlc 2 to Bob.
+	htlc2, _ := generateHtlcAndInvoice(t, 0)
+	ctx.sendHtlcAliceToBob(1, htlc2)
+	ctx.receiveHtlcAliceToBob()
+
+	// Tick the batch ticker to trigger a commitsig from Alice->Bob.
+	select {
+	case batchTicker <- time.Now():
+	case <-time.After(5 * time.Second):
+		t.Fatalf("could not force commit sig")
+	}
+
+	// Get the commit sig from Alice, but don't send it to Bob yet.
+	commitSigAlice := ctx.receiveCommitSigAlice(2)
+
+	// Bob adds htlc 1 to its remote commit tx.
+	ctx.sendCommitSigBobToAlice(1)
+
+	// Now send Bob the signature from Alice covering both htlcs.
+	err = bobChannel.ReceiveNewCommitment(
+		commitSigAlice.CommitSig, commitSigAlice.HtlcSigs,
+	)
+	if err != nil {
+		t.Fatalf("bob failed receiving commitment: %v", err)
+	}
+
+	// Both Alice and Bob revoke their previous commitment txes.
+	ctx.receiveRevAndAckAliceToBob()
+	ctx.sendRevAndAckBobToAlice()
+
+	// The situation now is that Alice still doesn't have her two htlcs on
+	// the local commit tx. Bob needs to send a new signature and Alice can
+	// only wait for that. However, Alice's log commit timer fires and Alice
+	// sends a commitment tx containing no updates. THIS SHOULD NOT HAPPEN!
+	ctx.receiveCommitSigAliceToBob(2)
+}
+
 // TestChannelLinkBatchPreimageWrite asserts that a link will batch preimage
 // writes when just as it receives a CommitSig to lock in any Settles, and also
 // if the link is aware of any uncommitted preimages if the link is stopped,
@@ -5962,6 +6048,78 @@ func TestChannelLinkRevocationWindowHodl(t *testing.T) {
 	}
 }
 
+// TestChannelLinkReceiveEmptySig tests the response of the link to receiving an
+// empty commit sig. This should be tolerated, but we shouldn't send out an
+// empty sig ourselves.
+func TestChannelLinkReceiveEmptySig(t *testing.T) {
+	t.Parallel()
+
+	const chanAmt = btcutil.SatoshiPerBitcoin * 5
+	const chanReserve = btcutil.SatoshiPerBitcoin * 1
+	aliceLink, bobChannel, batchTicker, start, cleanUp, _, err :=
+		newSingleLinkTestHarness(chanAmt, chanReserve)
+	if err != nil {
+		t.Fatalf("unable to create link: %v", err)
+	}
+	defer cleanUp()
+
+	if err := start(); err != nil {
+		t.Fatalf("unable to start test harness: %v", err)
+	}
+
+	var (
+		coreLink  = aliceLink.(*channelLink)
+		aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs
+	)
+
+	ctx := linkTestContext{
+		t:          t,
+		aliceLink:  aliceLink,
+		aliceMsgs:  aliceMsgs,
+		bobChannel: bobChannel,
+	}
+
+	htlc, _ := generateHtlcAndInvoice(t, 0)
+
+	// First, send an Add from Alice to Bob.
+	ctx.sendHtlcAliceToBob(0, htlc)
+	ctx.receiveHtlcAliceToBob()
+
+	// Tick the batch ticker to trigger a commitsig from Alice->Bob.
+	select {
+	case batchTicker <- time.Now():
+	case <-time.After(5 * time.Second):
+		t.Fatalf("could not force commit sig")
+	}
+
+	// Make Bob send a CommitSig. Since Bob hasn't received Alice's sig, he
+	// cannot add the htlc to his remote tx yet. The commit sig that we
+	// force Bob to send will be empty. Note that this normally does not
+	// happen, because the link (which is not present for Bob in this test)
+	// check whether Bob actually owes a sig first.
+	ctx.sendCommitSigBobToAlice(0)
+
+	// Receive a CommitSig from Alice covering the htlc from above.
+	ctx.receiveCommitSigAliceToBob(1)
+
+	// Wait for RevokeAndAck Alice->Bob. Even though Bob sent an empty
+	// commit sig, Alice still needs to revoke the previous commitment tx.
+	ctx.receiveRevAndAckAliceToBob()
+
+	// Send RevokeAndAck Bob->Alice to ack the added htlc.
+	ctx.sendRevAndAckBobToAlice()
+
+	// No new updates to sign, Alice still sends out an empty sig. THIS
+	// SHOULD NOT HAPPEN!
+	ctx.receiveCommitSigAliceToBob(1)
+
+	// No other messages are expected.
+	ctx.assertNoMsgFromAlice(time.Second)
+
+	// Stop the link
+	aliceLink.Stop()
+}
+
 // assertFailureCode asserts that an error is of type ForwardingError and that
 // the failure code is as expected.
 func assertFailureCode(t *testing.T, err error, code lnwire.FailCode) {

From f59b4d62bfe04387e81e3783684c4ba4b79ac3df Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Mon, 23 Sep 2019 14:27:06 +0200
Subject: [PATCH 3/8] htlcswitch: check for signature owed in link

Previously the channel method FullySynced was used to decide whether to
send a new commit sig message. However, it could happen that FullySynced
was false, but that we didn't owe a commitment signature. Instead we
were waiting on the other party to send us a signature. If that
happened, we'd send out an empty commit sig. This commit modifies the
condition that triggers a new commit sig and fixes this deviation from
the spec.
---
 htlcswitch/link.go      |  4 ++--
 htlcswitch/link_test.go | 14 ++++++--------
 lnwallet/channel.go     | 22 +---------------------
 3 files changed, 9 insertions(+), 31 deletions(-)

diff --git a/htlcswitch/link.go b/htlcswitch/link.go
index 4da9c0db..20b58f23 100644
--- a/htlcswitch/link.go
+++ b/htlcswitch/link.go
@@ -1103,7 +1103,7 @@ out:
 			// update in some time, check to see if we have any
 			// pending updates we need to commit due to our
 			// commitment chains being desynchronized.
-			if l.channel.FullySynced() {
+			if !l.channel.OweCommitment(true) {
 				continue
 			}
 
@@ -1808,7 +1808,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 		// If both commitment chains are fully synced from our PoV,
 		// then we don't need to reply with a signature as both sides
 		// already have a commitment with the latest accepted.
-		if l.channel.FullySynced() {
+		if !l.channel.OweCommitment(true) {
 			return
 		}
 
diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go
index 68d425b5..b09d0219 100644
--- a/htlcswitch/link_test.go
+++ b/htlcswitch/link_test.go
@@ -4699,11 +4699,10 @@ func TestChannelLinkNoEmptySig(t *testing.T) {
 	ctx.receiveRevAndAckAliceToBob()
 	ctx.sendRevAndAckBobToAlice()
 
-	// The situation now is that Alice still doesn't have her two htlcs on
-	// the local commit tx. Bob needs to send a new signature and Alice can
-	// only wait for that. However, Alice's log commit timer fires and Alice
-	// sends a commitment tx containing no updates. THIS SHOULD NOT HAPPEN!
-	ctx.receiveCommitSigAliceToBob(2)
+	// The commit txes are not in sync, but it is Bob's turn to send a new
+	// signature. We don't expect Alice to send out any message. This check
+	// allows some time for the log commit ticker to trigger for Alice.
+	ctx.assertNoMsgFromAlice(time.Second)
 }
 
 // TestChannelLinkBatchPreimageWrite asserts that a link will batch preimage
@@ -6109,9 +6108,8 @@ func TestChannelLinkReceiveEmptySig(t *testing.T) {
 	// Send RevokeAndAck Bob->Alice to ack the added htlc.
 	ctx.sendRevAndAckBobToAlice()
 
-	// No new updates to sign, Alice still sends out an empty sig. THIS
-	// SHOULD NOT HAPPEN!
-	ctx.receiveCommitSigAliceToBob(1)
+	// We received an empty commit sig, we accepted it, but there is nothing
+	// new to sign for us.
 
 	// No other messages are expected.
 	ctx.assertNoMsgFromAlice(time.Second)
diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index 5a87aca8..c1ac5967 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -3535,7 +3535,7 @@ func (lc *LightningChannel) ProcessChanSyncMsg(
 		// but died before the signature was sent. We re-transmit our
 		// revocation, but also initiate a state transition to re-sync
 		// them.
-		if !lc.FullySynced() {
+		if lc.OweCommitment(true) {
 			commitSig, htlcSigs, _, err := lc.SignNextCommitment()
 			switch {
 
@@ -4225,26 +4225,6 @@ func (lc *LightningChannel) oweCommitment(local bool) bool {
 	return oweCommitment
 }
 
-// FullySynced returns a boolean value reflecting if both commitment chains
-// (remote+local) are fully in sync. Both commitment chains are fully in sync
-// if the tip of each chain includes the latest committed changes from both
-// sides.
-func (lc *LightningChannel) FullySynced() bool {
-	lc.RLock()
-	defer lc.RUnlock()
-
-	lastLocalCommit := lc.localCommitChain.tip()
-	lastRemoteCommit := lc.remoteCommitChain.tip()
-
-	localUpdatesSynced := (lastLocalCommit.ourMessageIndex ==
-		lastRemoteCommit.ourMessageIndex)
-
-	remoteUpdatesSynced := (lastLocalCommit.theirMessageIndex ==
-		lastRemoteCommit.theirMessageIndex)
-
-	return localUpdatesSynced && remoteUpdatesSynced
-}
-
 // RevokeCurrentCommitment revokes the next lowest unrevoked commitment
 // transaction in the local commitment chain. As a result the edge of our
 // revocation window is extended by one, and the tail of our local commitment

From 03b32d046a25f3d77ec2cf9bea3537d1afef6f15 Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Wed, 10 Apr 2019 13:10:25 +0200
Subject: [PATCH 4/8] htlcswitch+lnwallet: replace updateNeeded by check on
 channel itself

Instead of tracking local updates in a separate link variable, query
this state from the channel itself.

This commit also fixes the issue where the commit tx was not updated
anymore after a failed first attempt because the revocation window was
closed. Also those pending updates will be taken into account when the
remote party revokes.
---
 htlcswitch/link.go  | 77 ++++++++++++++++++---------------------------
 lnwallet/channel.go | 11 +++++++
 2 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/htlcswitch/link.go b/htlcswitch/link.go
index 20b58f23..fd798fc5 100644
--- a/htlcswitch/link.go
+++ b/htlcswitch/link.go
@@ -742,19 +742,15 @@ func (l *channelLink) resolveFwdPkgs() error {
 
 	l.log.Debugf("loaded %d fwd pks", len(fwdPkgs))
 
-	var needUpdate bool
 	for _, fwdPkg := range fwdPkgs {
-		hasUpdate, err := l.resolveFwdPkg(fwdPkg)
-		if err != nil {
+		if err := l.resolveFwdPkg(fwdPkg); err != nil {
 			return err
 		}
-
-		needUpdate = needUpdate || hasUpdate
 	}
 
 	// If any of our reprocessing steps require an update to the commitment
 	// txn, we initiate a state transition to capture all relevant changes.
-	if needUpdate {
+	if l.channel.PendingLocalUpdateCount() > 0 {
 		return l.updateCommitTx()
 	}
 
@@ -764,7 +760,7 @@ func (l *channelLink) resolveFwdPkgs() error {
 // resolveFwdPkg interprets the FwdState of the provided package, either
 // reprocesses any outstanding htlcs in the package, or performs garbage
 // collection on the package.
-func (l *channelLink) resolveFwdPkg(fwdPkg *channeldb.FwdPkg) (bool, error) {
+func (l *channelLink) resolveFwdPkg(fwdPkg *channeldb.FwdPkg) error {
 	// Remove any completed packages to clear up space.
 	if fwdPkg.State == channeldb.FwdStateCompleted {
 		l.log.Debugf("removing completed fwd pkg for height=%d",
@@ -774,7 +770,7 @@ func (l *channelLink) resolveFwdPkg(fwdPkg *channeldb.FwdPkg) (bool, error) {
 		if err != nil {
 			l.log.Errorf("unable to remove fwd pkg for height=%d: "+
 				"%v", fwdPkg.Height, err)
-			return false, err
+			return err
 		}
 	}
 
@@ -793,7 +789,7 @@ func (l *channelLink) resolveFwdPkg(fwdPkg *channeldb.FwdPkg) (bool, error) {
 		if err != nil {
 			l.log.Errorf("unable to process remote log updates: %v",
 				err)
-			return false, err
+			return err
 		}
 		l.processRemoteSettleFails(fwdPkg, settleFails)
 	}
@@ -802,7 +798,6 @@ func (l *channelLink) resolveFwdPkg(fwdPkg *channeldb.FwdPkg) (bool, error) {
 	// downstream logic is able to filter out any duplicates, but we must
 	// shove the entire, original set of adds down the pipeline so that the
 	// batch of adds presented to the sphinx router does not ever change.
-	var needUpdate bool
 	if !fwdPkg.AckFilter.IsFull() {
 		adds, err := lnwallet.PayDescsFromRemoteLogUpdates(
 			fwdPkg.Source, fwdPkg.Height, fwdPkg.Adds,
@@ -810,20 +805,20 @@ func (l *channelLink) resolveFwdPkg(fwdPkg *channeldb.FwdPkg) (bool, error) {
 		if err != nil {
 			l.log.Errorf("unable to process remote log updates: %v",
 				err)
-			return false, err
+			return err
 		}
-		needUpdate = l.processRemoteAdds(fwdPkg, adds)
+		l.processRemoteAdds(fwdPkg, adds)
 
 		// If the link failed during processing the adds, we must
 		// return to ensure we won't attempted to update the state
 		// further.
 		if l.failed {
-			return false, fmt.Errorf("link failed while " +
+			return fmt.Errorf("link failed while " +
 				"processing remote adds")
 		}
 	}
 
-	return needUpdate, nil
+	return nil
 }
 
 // fwdPkgGarbager periodically reads all forwarding packages from disk and
@@ -1873,7 +1868,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 		}
 
 		l.processRemoteSettleFails(fwdPkg, settleFails)
-		needUpdate := l.processRemoteAdds(fwdPkg, adds)
+		l.processRemoteAdds(fwdPkg, adds)
 
 		// If the link failed during processing the adds, we must
 		// return to ensure we won't attempted to update the state
@@ -1882,7 +1877,11 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 			return
 		}
 
-		if needUpdate {
+		// If there are pending local updates, try to update the commit
+		// tx. Pending updates could already have been present because
+		// of a previously failed update to the commit tx or freshly
+		// added by processRemoteAdds.
+		if l.channel.PendingLocalUpdateCount() > 0 {
 			if err := l.updateCommitTx(); err != nil {
 				l.fail(LinkFailureError{code: ErrInternalError},
 					"unable to update commitment: %v", err)
@@ -2532,7 +2531,7 @@ func (l *channelLink) processRemoteSettleFails(fwdPkg *channeldb.FwdPkg,
 // whether we are reprocessing as a result of a failure or restart. Adds that
 // have already been acknowledged in the forwarding package will be ignored.
 func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
-	lockedInHtlcs []*lnwallet.PaymentDescriptor) bool {
+	lockedInHtlcs []*lnwallet.PaymentDescriptor) {
 
 	l.log.Tracef("processing %d remote adds for height %d",
 		len(lockedInHtlcs), fwdPkg.Height)
@@ -2571,13 +2570,10 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 	if sphinxErr != nil {
 		l.fail(LinkFailureError{code: ErrInternalError},
 			"unable to decode hop iterators: %v", sphinxErr)
-		return false
+		return
 	}
 
-	var (
-		needUpdate    bool
-		switchPackets []*htlcPacket
-	)
+	var switchPackets []*htlcPacket
 
 	for i, pd := range lockedInHtlcs {
 		idx := uint16(i)
@@ -2614,7 +2610,6 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 			// sender.
 			l.sendMalformedHTLCError(pd.HtlcIndex, failureCode,
 				onionBlob[:], pd.SourceRef)
-			needUpdate = true
 
 			l.log.Errorf("unable to decode onion hop "+
 				"iterator: %v", failureCode)
@@ -2633,7 +2628,6 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 			l.sendMalformedHTLCError(
 				pd.HtlcIndex, failureCode, onionBlob[:], pd.SourceRef,
 			)
-			needUpdate = true
 
 			l.log.Errorf("unable to decode onion "+
 				"obfuscator: %v", failureCode)
@@ -2664,7 +2658,6 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 				lnwire.NewInvalidOnionPayload(failedType, 0),
 				obfuscator, pd.SourceRef,
 			)
-			needUpdate = true
 
 			l.log.Errorf("unable to decode forwarding "+
 				"instructions: %v", err)
@@ -2675,7 +2668,7 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 
 		switch fwdInfo.NextHop {
 		case hop.Exit:
-			updated, err := l.processExitHop(
+			err := l.processExitHop(
 				pd, obfuscator, fwdInfo, heightNow, pld,
 			)
 			if err != nil {
@@ -2683,10 +2676,7 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 					err.Error(),
 				)
 
-				return false
-			}
-			if updated {
-				needUpdate = true
+				return
 			}
 
 		// There are additional channels left within this route. So
@@ -2781,7 +2771,6 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 				l.sendHTLCError(
 					pd.HtlcIndex, failure, obfuscator, pd.SourceRef,
 				)
-				needUpdate = true
 				continue
 			}
 
@@ -2821,12 +2810,12 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 		if err != nil {
 			l.fail(LinkFailureError{code: ErrInternalError},
 				"unable to set fwd filter: %v", err)
-			return false
+			return
 		}
 	}
 
 	if len(switchPackets) == 0 {
-		return needUpdate
+		return
 	}
 
 	l.log.Debugf("forwarding %d packets to switch", len(switchPackets))
@@ -2837,15 +2826,13 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 	// opened circuits, which violates assumptions made by the circuit
 	// trimming.
 	l.forwardBatch(switchPackets...)
-
-	return needUpdate
 }
 
 // processExitHop handles an htlc for which this link is the exit hop. It
 // returns a boolean indicating whether the commitment tx needs an update.
 func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 	obfuscator hop.ErrorEncrypter, fwdInfo hop.ForwardingInfo,
-	heightNow uint32, payload invoices.Payload) (bool, error) {
+	heightNow uint32, payload invoices.Payload) error {
 
 	// If hodl.ExitSettle is requested, we will not validate the final hop's
 	// ADD, nor will we settle the corresponding invoice or respond with the
@@ -2853,7 +2840,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 	if l.cfg.HodlMask.Active(hodl.ExitSettle) {
 		l.log.Warnf(hodl.ExitSettle.Warning())
 
-		return false, nil
+		return nil
 	}
 
 	// As we're the exit hop, we'll double check the hop-payload included in
@@ -2868,7 +2855,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 		failure := lnwire.NewFinalIncorrectHtlcAmount(pd.Amount)
 		l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef)
 
-		return true, nil
+		return nil
 	}
 
 	// We'll also ensure that our time-lock value has been computed
@@ -2881,7 +2868,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 		failure := lnwire.NewFinalIncorrectCltvExpiry(pd.Timeout)
 		l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef)
 
-		return true, nil
+		return nil
 	}
 
 	// Notify the invoiceRegistry of the exit hop htlc. If we crash right
@@ -2906,14 +2893,14 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 		failure := lnwire.NewFailIncorrectDetails(pd.Amount, heightNow)
 		l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef)
 
-		return true, nil
+		return nil
 
 	// No error.
 	case nil:
 
 	// Pass error to caller.
 	default:
-		return false, err
+		return err
 	}
 
 	// Create a hodlHtlc struct and decide either resolved now or later.
@@ -2926,15 +2913,11 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 		// Save payment descriptor for future reference.
 		l.hodlMap[circuitKey] = htlc
 
-		return false, nil
+		return nil
 	}
 
 	// Process the received resolution.
-	err = l.processHodlEvent(*event, htlc)
-	if err != nil {
-		return false, err
-	}
-	return true, nil
+	return l.processHodlEvent(*event, htlc)
 }
 
 // settleHTLC settles the HTLC on the channel.
diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index c1ac5967..57f6b370 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -4225,6 +4225,17 @@ func (lc *LightningChannel) oweCommitment(local bool) bool {
 	return oweCommitment
 }
 
+// PendingLocalUpdateCount returns the number of local updates that still need
+// to be applied to the remote commitment tx.
+func (lc *LightningChannel) PendingLocalUpdateCount() uint64 {
+	lc.RLock()
+	defer lc.RUnlock()
+
+	lastRemoteCommit := lc.remoteCommitChain.tip()
+
+	return lc.localUpdateLog.logIndex - lastRemoteCommit.ourMessageIndex
+}
+
 // RevokeCurrentCommitment revokes the next lowest unrevoked commitment
 // transaction in the local commitment chain. As a result the edge of our
 // revocation window is extended by one, and the tail of our local commitment

From 2482d84d7da595385129b24cc69c4c4dae7737b1 Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Tue, 24 Sep 2019 11:49:32 +0200
Subject: [PATCH 5/8] htlcswitch: stop batch timer if there are no updates

---
 htlcswitch/link.go | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/htlcswitch/link.go b/htlcswitch/link.go
index fd798fc5..d4888b88 100644
--- a/htlcswitch/link.go
+++ b/htlcswitch/link.go
@@ -1023,11 +1023,14 @@ out:
 			break out
 		}
 
-		// If the previous event resulted in a non-empty
-		// batch, reinstate the batch ticker so that it can be
-		// cleared.
+		// If the previous event resulted in a non-empty batch, resume
+		// the batch ticker so that it can be cleared. Otherwise pause
+		// the ticker to prevent waking up the htlcManager while the
+		// batch is empty.
 		if l.batchCounter > 0 {
 			l.cfg.BatchTicker.Resume()
+		} else {
+			l.cfg.BatchTicker.Pause()
 		}
 
 		select {
@@ -1109,19 +1112,10 @@ out:
 			}
 
 		case <-l.cfg.BatchTicker.Ticks():
-			// If the current batch is empty, then we have no work
-			// here. We also disable the batch ticker from waking up
-			// the htlcManager while the batch is empty.
-			if l.batchCounter == 0 {
-				l.cfg.BatchTicker.Pause()
-				continue
-			}
-
-			// Otherwise, attempt to extend the remote commitment
-			// chain including all the currently pending entries.
-			// If the send was unsuccessful, then abandon the
-			// update, waiting for the revocation window to open
-			// up.
+			// Attempt to extend the remote commitment chain
+			// including all the currently pending entries. If the
+			// send was unsuccessful, then abandon the update,
+			// waiting for the revocation window to open up.
 			if err := l.updateCommitTx(); err != nil {
 				l.fail(LinkFailureError{code: ErrInternalError},
 					"unable to update commitment: %v", err)

From 5078d662efdffc79b2e476ebd321b345c47a4202 Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Wed, 10 Apr 2019 15:28:55 +0200
Subject: [PATCH 6/8] htlcswitch: remove batch counter

Now that channel exposes the number of pending local updates, it is no
longer necessary to track the batch size separately in the link.
---
 htlcswitch/link.go | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/htlcswitch/link.go b/htlcswitch/link.go
index d4888b88..a2f89bb3 100644
--- a/htlcswitch/link.go
+++ b/htlcswitch/link.go
@@ -291,15 +291,6 @@ type channelLink struct {
 	// sure we don't process any more updates.
 	failed bool
 
-	// batchCounter is the number of updates which we received from remote
-	// side, but not include in commitment transaction yet and plus the
-	// current number of settles that have been sent, but not yet committed
-	// to the commitment.
-	//
-	// TODO(andrew.shvv) remove after we add additional BatchNumber()
-	// method in state machine.
-	batchCounter uint32
-
 	// keystoneBatch represents a volatile list of keystones that must be
 	// written before attempting to sign the next commitment txn. These
 	// represent all the HTLC's forwarded to the link from the switch. Once
@@ -1027,7 +1018,7 @@ out:
 		// the batch ticker so that it can be cleared. Otherwise pause
 		// the ticker to prevent waking up the htlcManager while the
 		// batch is empty.
-		if l.batchCounter > 0 {
+		if l.channel.PendingLocalUpdateCount() > 0 {
 			l.cfg.BatchTicker.Resume()
 		} else {
 			l.cfg.BatchTicker.Pause()
@@ -1145,9 +1136,9 @@ out:
 			if ok && l.overflowQueue.Length() != 0 {
 				l.log.Infof("downstream htlc add update with "+
 					"payment hash(%x) have been added to "+
-					"reprocessing queue, batch_size=%v",
+					"reprocessing queue, pend_updates=%v",
 					htlc.PaymentHash[:],
-					l.batchCounter)
+					l.channel.PendingLocalUpdateCount())
 
 				l.overflowQueue.AddPkt(pkt)
 				continue
@@ -1225,8 +1216,6 @@ loop:
 func (l *channelLink) processHodlEvent(hodlEvent invoices.HodlEvent,
 	htlc hodlHtlc) error {
 
-	l.batchCounter++
-
 	circuitKey := hodlEvent.CircuitKey
 
 	// Determine required action for the resolution.
@@ -1296,9 +1285,9 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 			case lnwallet.ErrMaxHTLCNumber:
 				l.log.Infof("downstream htlc add update with "+
 					"payment hash(%x) have been added to "+
-					"reprocessing queue, batch: %v",
+					"reprocessing queue, pend_updates: %v",
 					htlc.PaymentHash[:],
-					l.batchCounter)
+					l.channel.PendingLocalUpdateCount())
 
 				l.overflowQueue.AddPkt(pkt)
 				return
@@ -1375,8 +1364,9 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 		}
 
 		l.log.Tracef("received downstream htlc: payment_hash=%x, "+
-			"local_log_index=%v, batch_size=%v",
-			htlc.PaymentHash[:], index, l.batchCounter+1)
+			"local_log_index=%v, pend_updates=%v",
+			htlc.PaymentHash[:], index,
+			l.channel.PendingLocalUpdateCount())
 
 		pkt.outgoingChanID = l.ShortChanID()
 		pkt.outgoingHTLCID = index
@@ -1507,11 +1497,11 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 		isSettle = true
 	}
 
-	l.batchCounter++
-
 	// If this newly added update exceeds the min batch size for adds, or
 	// this is a settle request, then initiate an update.
-	if l.batchCounter >= l.cfg.BatchSize || isSettle {
+	if l.channel.PendingLocalUpdateCount() >= uint64(l.cfg.BatchSize) ||
+		isSettle {
+
 		if err := l.updateCommitTx(); err != nil {
 			l.fail(LinkFailureError{code: ErrInternalError},
 				"unable to update commitment: %v", err)
@@ -1989,8 +1979,9 @@ func (l *channelLink) updateCommitTx() error {
 	theirCommitSig, htlcSigs, pendingHTLCs, err := l.channel.SignNextCommitment()
 	if err == lnwallet.ErrNoWindow {
 		l.log.Tracef("revocation window exhausted, unable to send: "+
-			"%v, dangling_opens=%v, dangling_closes%v",
-			l.batchCounter, newLogClosure(func() string {
+			"%v, pend_updates=%v, dangling_closes%v",
+			l.channel.PendingLocalUpdateCount(),
+			newLogClosure(func() string {
 				return spew.Sdump(l.openedCircuits)
 			}),
 			newLogClosure(func() string {
@@ -2036,10 +2027,6 @@ func (l *channelLink) updateCommitTx() error {
 	}
 	l.logCommitTick = nil
 
-	// Finally, clear our the current batch, so we can accurately make
-	// further batch flushing decisions.
-	l.batchCounter = 0
-
 	return nil
 }
 

From ae67b1a4a40b95e45bd9ed29d21adb23b24a84cd Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Wed, 10 Apr 2019 13:10:01 +0200
Subject: [PATCH 7/8] htlcswitch/test: test revocation window exhaustion

---
 htlcswitch/link_test.go | 97 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go
index b09d0219..acbc4506 100644
--- a/htlcswitch/link_test.go
+++ b/htlcswitch/link_test.go
@@ -5902,6 +5902,103 @@ func TestChannelLinkHoldInvoiceRestart(t *testing.T) {
 	}
 }
 
+// TestChannelLinkRevocationWindowRegular asserts that htlcs paying to a regular
+// invoice are settled even if the revocation window gets exhausted.
+func TestChannelLinkRevocationWindowRegular(t *testing.T) {
+	t.Parallel()
+
+	const (
+		chanAmt = btcutil.SatoshiPerBitcoin * 5
+	)
+
+	// We'll start by creating a new link with our chanAmt (5 BTC). We will
+	// only be testing Alice's behavior, so the reference to Bob's channel
+	// state is unnecessary.
+	aliceLink, bobChannel, _, start, cleanUp, _, err :=
+		newSingleLinkTestHarness(chanAmt, 0)
+	if err != nil {
+		t.Fatalf("unable to create link: %v", err)
+	}
+	defer cleanUp()
+
+	if err := start(); err != nil {
+		t.Fatalf("unable to start test harness: %v", err)
+	}
+	defer aliceLink.Stop()
+
+	var (
+		coreLink  = aliceLink.(*channelLink)
+		registry  = coreLink.cfg.Registry.(*mockInvoiceRegistry)
+		aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs
+	)
+
+	ctx := linkTestContext{
+		t:          t,
+		aliceLink:  aliceLink,
+		aliceMsgs:  aliceMsgs,
+		bobChannel: bobChannel,
+	}
+
+	registry.settleChan = make(chan lntypes.Hash)
+
+	htlc1, invoice1 := generateHtlcAndInvoice(t, 0)
+	htlc2, invoice2 := generateHtlcAndInvoice(t, 1)
+
+	// We must add the invoice to the registry, such that Alice
+	// expects this payment.
+	err = registry.AddInvoice(*invoice1, htlc1.PaymentHash)
+	if err != nil {
+		t.Fatalf("unable to add invoice to registry: %v", err)
+	}
+	err = registry.AddInvoice(*invoice2, htlc2.PaymentHash)
+	if err != nil {
+		t.Fatalf("unable to add invoice to registry: %v", err)
+	}
+
+	// Lock in htlc 1 on both sides.
+	ctx.sendHtlcBobToAlice(htlc1)
+	ctx.sendCommitSigBobToAlice(1)
+	ctx.receiveRevAndAckAliceToBob()
+	ctx.receiveCommitSigAliceToBob(1)
+	ctx.sendRevAndAckBobToAlice()
+
+	// We expect a call to the invoice registry to notify the arrival of the
+	// htlc.
+	select {
+	case <-registry.settleChan:
+	case <-time.After(5 * time.Second):
+		t.Fatal("expected invoice to be settled")
+	}
+
+	// Expect alice to send a settle and commitsig message to bob. Bob does
+	// not yet send the revocation.
+	ctx.receiveSettleAliceToBob()
+	ctx.receiveCommitSigAliceToBob(0)
+
+	// Pay invoice 2.
+	ctx.sendHtlcBobToAlice(htlc2)
+	ctx.sendCommitSigBobToAlice(2)
+	ctx.receiveRevAndAckAliceToBob()
+
+	// At this point, Alice cannot send a new commit sig to bob because the
+	// revocation window is exhausted.
+
+	// Sleep to let the commit ticker expire. The revocation window is still
+	// exhausted.
+	time.Sleep(time.Second)
+
+	// Bob sends revocation and signs commit with htlc1 settled.
+	ctx.sendRevAndAckBobToAlice()
+
+	// Allow some time for the log commit ticker to trigger for Alice.
+	time.Sleep(time.Second)
+
+	// Now that Bob revoked, Alice should send the sig she owes.
+	//
+	// THIS SHOULD NOT HAPPEN.
+	ctx.assertNoMsgFromAlice(time.Second)
+}
+
 // TestChannelLinkRevocationWindowHodl asserts that htlcs paying to a hodl
 // invoice are settled even if the revocation window gets exhausted.
 func TestChannelLinkRevocationWindowHodl(t *testing.T) {

From 0b5afa64f37e5cbdfe5067ab058f3d9842413d21 Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Wed, 10 Apr 2019 16:05:09 +0200
Subject: [PATCH 8/8] htlcswitch: remove logCommitTick

Replace logCommitTick as a way to deal with revocation window exhaustion
by retrying to update the commit tx when the remote revocation is
received.

The rationale is that the revocation window always opens up because of a
revoke message that is received from the other party. It is therefore
not necessary to set a timer for this. The reception of the revoke
message is the trigger to send a new commit sig if necessary.
---
 htlcswitch/link.go      | 75 ++++++++---------------------------------
 htlcswitch/link_test.go | 14 ++------
 2 files changed, 17 insertions(+), 72 deletions(-)

diff --git a/htlcswitch/link.go b/htlcswitch/link.go
index a2f89bb3..83a9c70e 100644
--- a/htlcswitch/link.go
+++ b/htlcswitch/link.go
@@ -344,14 +344,6 @@ type channelLink struct {
 	// sub-systems with the latest set of active HTLC's on our channel.
 	htlcUpdates chan *contractcourt.ContractUpdate
 
-	// logCommitTimer is a timer which is sent upon if we go an interval
-	// without receiving/sending a commitment update. It's role is to
-	// ensure both chains converge to identical state in a timely manner.
-	//
-	// TODO(roasbeef): timer should be >> then RTT
-	logCommitTimer *time.Timer
-	logCommitTick  <-chan time.Time
-
 	// updateFeeTimer is the timer responsible for updating the link's
 	// commitment fee every time it fires.
 	updateFeeTimer *time.Timer
@@ -397,13 +389,12 @@ func NewChannelLink(cfg ChannelLinkConfig,
 		channel:     channel,
 		shortChanID: channel.ShortChanID(),
 		// TODO(roasbeef): just do reserve here?
-		logCommitTimer: time.NewTimer(300 * time.Millisecond),
-		overflowQueue:  newPacketQueue(input.MaxHTLCNumber / 2),
-		htlcUpdates:    make(chan *contractcourt.ContractUpdate),
-		hodlMap:        make(map[channeldb.CircuitKey]hodlHtlc),
-		hodlQueue:      queue.NewConcurrentQueue(10),
-		log:            build.NewPrefixLog(logPrefix, log),
-		quit:           make(chan struct{}),
+		overflowQueue: newPacketQueue(input.MaxHTLCNumber / 2),
+		htlcUpdates:   make(chan *contractcourt.ContractUpdate),
+		hodlMap:       make(map[channeldb.CircuitKey]hodlHtlc),
+		hodlQueue:     queue.NewConcurrentQueue(10),
+		log:           build.NewPrefixLog(logPrefix, log),
+		quit:          make(chan struct{}),
 	}
 }
 
@@ -1087,21 +1078,6 @@ out:
 
 			break out
 
-		case <-l.logCommitTick:
-			// If we haven't sent or received a new commitment
-			// update in some time, check to see if we have any
-			// pending updates we need to commit due to our
-			// commitment chains being desynchronized.
-			if !l.channel.OweCommitment(true) {
-				continue
-			}
-
-			if err := l.updateCommitTx(); err != nil {
-				l.fail(LinkFailureError{code: ErrInternalError},
-					"unable to update commitment: %v", err)
-				break out
-			}
-
 		case <-l.cfg.BatchTicker.Ticks():
 			// Attempt to extend the remote commitment chain
 			// including all the currently pending entries. If the
@@ -1769,21 +1745,6 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 			return
 		}
 
-		// As we've just received a commitment signature, we'll
-		// re-start the log commit timer to wake up the main processing
-		// loop to check if we need to send a commitment signature as
-		// we owe one.
-		//
-		// TODO(roasbeef): instead after revocation?
-		if !l.logCommitTimer.Stop() {
-			select {
-			case <-l.logCommitTimer.C:
-			default:
-			}
-		}
-		l.logCommitTimer.Reset(300 * time.Millisecond)
-		l.logCommitTick = l.logCommitTimer.C
-
 		// If both commitment chains are fully synced from our PoV,
 		// then we don't need to reply with a signature as both sides
 		// already have a commitment with the latest accepted.
@@ -1861,11 +1822,14 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 			return
 		}
 
-		// If there are pending local updates, try to update the commit
-		// tx. Pending updates could already have been present because
-		// of a previously failed update to the commit tx or freshly
-		// added by processRemoteAdds.
-		if l.channel.PendingLocalUpdateCount() > 0 {
+		// The revocation window opened up. If there are pending local
+		// updates, try to update the commit tx. Pending updates could
+		// already have been present because of a previously failed
+		// update to the commit tx or freshly added in by
+		// processRemoteAdds. Also in case there are no local updates,
+		// but there are still remote updates that are not in the remote
+		// commit tx yet, send out an update.
+		if l.channel.OweCommitment(true) {
 			if err := l.updateCommitTx(); err != nil {
 				l.fail(LinkFailureError{code: ErrInternalError},
 					"unable to update commitment: %v", err)
@@ -2016,17 +1980,6 @@ func (l *channelLink) updateCommitTx() error {
 	}
 	l.cfg.Peer.SendMessage(false, commitSig)
 
-	// We've just initiated a state transition, attempt to stop the
-	// logCommitTimer. If the timer already ticked, then we'll consume the
-	// value, dropping
-	if l.logCommitTimer != nil && !l.logCommitTimer.Stop() {
-		select {
-		case <-l.logCommitTimer.C:
-		default:
-		}
-	}
-	l.logCommitTick = nil
-
 	return nil
 }
 
diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go
index acbc4506..7d870fc0 100644
--- a/htlcswitch/link_test.go
+++ b/htlcswitch/link_test.go
@@ -5983,20 +5983,12 @@ func TestChannelLinkRevocationWindowRegular(t *testing.T) {
 	// At this point, Alice cannot send a new commit sig to bob because the
 	// revocation window is exhausted.
 
-	// Sleep to let the commit ticker expire. The revocation window is still
-	// exhausted.
-	time.Sleep(time.Second)
-
 	// Bob sends revocation and signs commit with htlc1 settled.
 	ctx.sendRevAndAckBobToAlice()
 
-	// Allow some time for the log commit ticker to trigger for Alice.
-	time.Sleep(time.Second)
-
-	// Now that Bob revoked, Alice should send the sig she owes.
-	//
-	// THIS SHOULD NOT HAPPEN.
-	ctx.assertNoMsgFromAlice(time.Second)
+	// After the revocation, it is again possible for Alice to send a commit
+	// sig with htlc2.
+	ctx.receiveCommitSigAliceToBob(1)
 }
 
 // TestChannelLinkRevocationWindowHodl asserts that htlcs paying to a hodl