From f83f47541d58aef7530f52221ee5f6f93f61d75e Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 16 Jan 2018 21:20:00 +0100 Subject: [PATCH] channel test: add TestDesyncHTLCs This commit adds a test that trigger a case where the balance could end up being negative when we used the logIndex when calculating the channel's available balance. This could happen when the logs got out of sync, and we would use the balance from a settled HTLC even though we wouldn't include it when signing the next state. --- lnwallet/channel_test.go | 70 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 03487741..6d903477 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -3914,6 +3914,13 @@ func TestChanAvailableBandwidth(t *testing.T) { t.Fatalf("unable to recv htlc cancel: %v", err) } + // We must do a state transition before the balance is available + // for Alice. + if err := forceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("unable to complete alice's state "+ + "transition: %v", err) + } + // With the HTLC's settled in the log, we'll now assert that if we // initiate a state transition, then our guess was correct. assertBandwidthEstimateCorrect(false) @@ -4293,4 +4300,67 @@ func TestChannelUnilateralCloseHtlcResolution(t *testing.T) { } } +// TestDesyncHTLCs checks that we cannot add HTLCs that would make the +// balance negative, when the remote and local update logs are desynced. +func TestDesyncHTLCs(t *testing.T) { + t.Parallel() + + // We'll kick off the test by creating our channels which both are + // loaded with 5 BTC each. + aliceChannel, bobChannel, cleanUp, err := createTestChannels(1) + if err != nil { + t.Fatalf("unable to create test channels: %v", err) + } + defer cleanUp() + + // First add one HTLC of value 4.1 BTC. + htlcAmt := lnwire.NewMSatFromSatoshis(4.1 * btcutil.SatoshiPerBitcoin) + htlc, _ := createHTLC(0, htlcAmt) + aliceIndex, err := aliceChannel.AddHTLC(htlc) + if err != nil { + t.Fatalf("unable to add htlc: %v", err) + } + bobIndex, err := bobChannel.ReceiveHTLC(htlc) + if err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + + // Lock this HTLC in. + if err := forceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("unable to complete state update: %v", err) + } + + // Now let let Bob fail this HTLC. + if err := bobChannel.FailHTLC(bobIndex, []byte("failreason")); err != nil { + t.Fatalf("unable to cancel HTLC: %v", err) + } + if err := aliceChannel.ReceiveFailHTLC(aliceIndex, []byte("bad")); err != nil { + t.Fatalf("unable to recv htlc cancel: %v", err) + } + + // Alice now has gotten all here original balance (5 BTC) back, + // however, adding a new HTLC at this point SHOULD fail, since + // if she add the HTLC and sign the next state, Bob cannot assume + // she received the FailHTLC, and must assume she doesn't have + // the necessary balance available. + // + // We try adding an HTLC of value 1 BTC, which should fail + // because the balance is unavailable. + htlcAmt = lnwire.NewMSatFromSatoshis(1 * btcutil.SatoshiPerBitcoin) + htlc, _ = createHTLC(1, htlcAmt) + if _, err = aliceChannel.AddHTLC(htlc); err != ErrInsufficientBalance { + t.Fatalf("expected ErrInsufficientBalance, instead received: %v", + err) + } + + // Now do a state transition, which will ACK the FailHTLC, making + // Alice able to add the new HTLC. + if err := forceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("unable to complete state update: %v", err) + } + if _, err = aliceChannel.AddHTLC(htlc); err != nil { + t.Fatalf("unable to add htlc: %v", err) + } +} + // TODO(roasbeef): testing.Quick test case for retrans!!!