From 64f4421d6c2eb388474b651918c9c625e98ada3b Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 6 Nov 2019 09:16:10 +0100 Subject: [PATCH] htlcswitch/test: add test cases that triggers empty commit sig Co-authored-by: Johan T. Halseth --- 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) {