diff --git a/channeldb/channel.go b/channeldb/channel.go index 9d8e0199..cc339d5e 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -2396,8 +2396,9 @@ func (c *OpenChannel) AdvanceCommitChainTail(fwdPkg *FwdPkg, // in their new commitment. updateBytes := chanBucket.Get(unsignedAckedUpdatesKey) if updateBytes == nil { - // If there are no updates to sign, we don't need to - // filter out any updates. + // This shouldn't normally happen as we always store + // the number of updates, but could still be + // encountered by nodes that are upgrading. newRemoteCommit = &newCommit.Commitment return nil } diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 792e5c59..9e8c5410 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2120,6 +2120,8 @@ func (lc *LightningChannel) restorePendingLocalUpdates( // If we did have a dangling commit, then we'll examine which updates // we included in that state and re-insert them into our update log. for _, logUpdate := range pendingRemoteCommitDiff.LogUpdates { + logUpdate := logUpdate + payDesc, err := lc.logUpdateToPayDesc( &logUpdate, lc.remoteUpdateLog, pendingHeight, chainfee.SatPerKWeight(pendingCommit.FeePerKw), @@ -3258,7 +3260,7 @@ func (lc *LightningChannel) getUnsignedAckedUpdates() []channeldb.LogUpdate { chanID := lnwire.NewChanIDFromOutPoint(&lc.channelState.FundingOutpoint) // Fetch the last remote update that we have signed for. - lastRemoteCommitted := lc.remoteCommitChain.tip().theirMessageIndex + lastRemoteCommitted := lc.remoteCommitChain.tail().theirMessageIndex // Fetch the last remote update that we have acked. lastLocalCommitted := lc.localCommitChain.tail().theirMessageIndex diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 27ea6ac9..85ccc305 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -9521,3 +9521,125 @@ func TestChannelLocalUnsignedUpdatesFailure(t *testing.T) { err = newAliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs) require.NoError(t, err) } + +// TestChannelSignedAckRegression tests a previously-regressing state +// transition no longer causes channel desynchronization. +// +// The full state transition of this test is: +// +// Alice Bob +// <----add------- +// <----sig------- +// -----rev------> +// -----sig------> +// <----rev------- +// ----settle----> +// -----sig------> +// <----rev------- +// <----sig------- +// -----add------> +// -----sig------> +// <----rev------- +// *restarts* +// -----rev------> +// <----sig------- +// +func TestChannelSignedAckRegression(t *testing.T) { + t.Parallel() + + // Create test channels to test out this state transition. + aliceChannel, bobChannel, cleanUp, err := CreateTestChannels( + channeldb.SingleFunderTweaklessBit, + ) + require.NoError(t, err) + defer cleanUp() + + // Create an HTLC that Bob will send to Alice. + htlc, preimage := createHTLC(0, lnwire.MilliSatoshi(5000000)) + + // <----add------ + _, err = bobChannel.AddHTLC(htlc, nil) + require.NoError(t, err) + _, err = aliceChannel.ReceiveHTLC(htlc) + require.NoError(t, err) + + // Force a state transition to lock in the HTLC. + // <----sig------ + // -----rev-----> + // -----sig-----> + // <----rev------ + err = ForceStateTransition(bobChannel, aliceChannel) + require.NoError(t, err) + + // Alice settles the HTLC back to Bob. + // ----settle---> + err = aliceChannel.SettleHTLC(preimage, uint64(0), nil, nil, nil) + require.NoError(t, err) + err = bobChannel.ReceiveHTLCSettle(preimage, uint64(0)) + require.NoError(t, err) + + // -----sig----> + aliceSig, aliceHtlcSigs, _, err := aliceChannel.SignNextCommitment() + require.NoError(t, err) + err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs) + require.NoError(t, err) + + // <----rev----- + bobRevocation, _, err := bobChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) + require.NoError(t, err) + + // <----sig----- + bobSig, bobHtlcSigs, _, err := bobChannel.SignNextCommitment() + require.NoError(t, err) + err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs) + require.NoError(t, err) + + // Create an HTLC that Alice will send to Bob. + htlc2, _ := createHTLC(0, lnwire.MilliSatoshi(5000000)) + + // -----add----> + _, err = aliceChannel.AddHTLC(htlc2, nil) + require.NoError(t, err) + _, err = bobChannel.ReceiveHTLC(htlc2) + require.NoError(t, err) + + // -----sig----> + aliceSig, aliceHtlcSigs, _, err = aliceChannel.SignNextCommitment() + require.NoError(t, err) + err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs) + require.NoError(t, err) + + // <----rev----- + bobRevocation, _, err = bobChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) + require.NoError(t, err) + + // Restart Bob's channel state here. + newBobChannel, err := NewLightningChannel( + bobChannel.Signer, bobChannel.channelState, + bobChannel.sigPool, + ) + require.NoError(t, err) + + // -----rev----> + aliceRevocation, _, err := aliceChannel.RevokeCurrentCommitment() + require.NoError(t, err) + fwdPkg, _, _, _, err := newBobChannel.ReceiveRevocation(aliceRevocation) + require.NoError(t, err) + + // Assert that the fwdpkg is not empty. + require.Equal(t, len(fwdPkg.SettleFails), 1) + + // Bob should no longer fail to sign this commitment due to faulty + // update logs. + // <----sig----- + bobSig, bobHtlcSigs, _, err = newBobChannel.SignNextCommitment() + require.NoError(t, err) + + // Alice should receive the new commitment without hiccups. + err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs) + require.NoError(t, err) +}