From 9ff79ae59517c34444d66c4a13e802b0cdac52d5 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Feb 2020 12:27:42 +0100 Subject: [PATCH] lnwallet/channel: account for HTLC fee when reporting available balance --- htlcswitch/link_test.go | 23 ++++--- lnwallet/channel.go | 26 +++++--- lnwallet/channel_test.go | 135 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 166 insertions(+), 18 deletions(-) diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 427322ee..ca31d68d 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1982,8 +1982,11 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { ) // The starting bandwidth of the channel should be exactly the amount - // that we created the channel between her and Bob. - expectedBandwidth := lnwire.NewMSatFromSatoshis(chanAmt - defaultCommitFee) + // that we created the channel between her and Bob, minus the + // commitment fee and fee for adding an additional HTLC. + expectedBandwidth := lnwire.NewMSatFromSatoshis( + chanAmt-defaultCommitFee, + ) - htlcFee assertLinkBandwidth(t, aliceLink, expectedBandwidth) // Next, we'll create an HTLC worth 1 BTC, and send it into the link as @@ -2656,8 +2659,10 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { // The starting bandwidth of the channel should be exactly the amount // that we created the channel between her and Bob, minus the commitment - // fee. - expectedBandwidth := lnwire.NewMSatFromSatoshis(chanAmt - defaultCommitFee) + // fee and fee of adding an HTLC. + expectedBandwidth := lnwire.NewMSatFromSatoshis( + chanAmt-defaultCommitFee, + ) - htlcFee assertLinkBandwidth(t, alice.link, expectedBandwidth) // Capture Alice's starting bandwidth to perform later, relative @@ -2935,8 +2940,10 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { // The starting bandwidth of the channel should be exactly the amount // that we created the channel between her and Bob, minus the commitment - // fee. - expectedBandwidth := lnwire.NewMSatFromSatoshis(chanAmt - defaultCommitFee) + // fee and fee for adding an additional HTLC. + expectedBandwidth := lnwire.NewMSatFromSatoshis( + chanAmt-defaultCommitFee, + ) - htlcFee assertLinkBandwidth(t, alice.link, expectedBandwidth) // Capture Alice's starting bandwidth to perform later, relative @@ -3191,9 +3198,9 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { // The starting bandwidth of the channel should be exactly the amount // that we created the channel between her and Bob, minus the channel - // reserve. + // reserve, commitment fee and fee for adding an additional HTLC. expectedBandwidth := lnwire.NewMSatFromSatoshis( - chanAmt - defaultCommitFee - chanReserve) + chanAmt-defaultCommitFee-chanReserve) - htlcFee assertLinkBandwidth(t, aliceLink, expectedBandwidth) // Next, we'll create an HTLC worth 3 BTC, and send it into the link as diff --git a/lnwallet/channel.go b/lnwallet/channel.go index f8512d72..f3368218 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -6068,20 +6068,30 @@ func (lc *LightningChannel) availableCommitmentBalance(view *htlcView) ( ourBalance = 0 } - // Given the commitment weight, find the commitment fee in case of no - // added HTLC output. + // Calculate the commitment fee in the case where we would add another + // HTLC to the commitment, as only the balance remaining after this fee + // has been paid is actually available for sending. feePerKw := filteredView.feePerKw - baseCommitFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(commitWeight), + htlcCommitFee := lnwire.NewMSatFromSatoshis( + feePerKw.FeeForWeight(commitWeight + input.HTLCWeight), ) - // If we are the channel initiator, we must to subtract the commitment - // fee from our available balance. + // If we are the channel initiator, we must to subtract this commitment + // fee from our available balance in order to ensure we can afford both + // the value of the HTLC and the additional commitment fee from adding + // the HTLC. if lc.channelState.IsInitiator { - if ourBalance < baseCommitFee { + // There is an edge case where our non-zero balance is lower + // than the htlcCommitFee, where we could still be sending dust + // HTLCs, but we return 0 in this case. This is to avoid + // lowering our balance even further, as this takes us into a + // bad state wehere neither we nor our channel counterparty can + // add HTLCs. + if ourBalance < htlcCommitFee { return 0, commitWeight } - ourBalance -= baseCommitFee + + ourBalance -= htlcCommitFee } return ourBalance, commitWeight diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 77af8d3f..09697629 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -4602,6 +4602,12 @@ func TestChanAvailableBandwidth(t *testing.T) { aliceReserve := lnwire.NewMSatFromSatoshis( aliceChannel.channelState.LocalChanCfg.ChanReserve, ) + feeRate := chainfee.SatPerKWeight( + aliceChannel.channelState.LocalCommitment.FeePerKw, + ) + htlcFee := lnwire.NewMSatFromSatoshis( + feeRate.FeeForWeight(input.HTLCWeight), + ) assertBandwidthEstimateCorrect := func(aliceInitiate bool) { // With the HTLC's added, we'll now query the AvailableBalance @@ -4631,8 +4637,9 @@ func TestChanAvailableBandwidth(t *testing.T) { aliceBalance := aliceChannel.channelState.LocalCommitment.LocalBalance // The balance we have available for new HTLCs should be the - // current local commitment balance, minus the channel reserve. - expBalance := aliceBalance - aliceReserve + // current local commitment balance, minus the channel reserve + // and the fee for adding an HTLC. + expBalance := aliceBalance - aliceReserve - htlcFee if expBalance != aliceAvailableBalance { _, _, line, _ := runtime.Caller(1) t.Fatalf("line: %v, incorrect balance: expected %v, "+ @@ -4714,6 +4721,130 @@ func TestChanAvailableBandwidth(t *testing.T) { // TODO(roasbeef): additional tests from diff starting conditions } +// TestChanAvailableBalanceNearHtlcFee checks that we get the expected reported +// balance when it is close to the htlc fee. +func TestChanAvailableBalanceNearHtlcFee(t *testing.T) { + t.Parallel() + + // Create a test channel which will be used for the duration of this + // unittest. The channel will be funded evenly with Alice having 5 BTC, + // and Bob having 5 BTC. + aliceChannel, bobChannel, cleanUp, err := CreateTestChannels(true) + if err != nil { + t.Fatalf("unable to create test channels: %v", err) + } + defer cleanUp() + + // Alice starts with half the channel capacity. + aliceBalance := lnwire.NewMSatFromSatoshis(5 * btcutil.SatoshiPerBitcoin) + + aliceReserve := lnwire.NewMSatFromSatoshis( + aliceChannel.channelState.LocalChanCfg.ChanReserve, + ) + + aliceDustlimit := lnwire.NewMSatFromSatoshis( + aliceChannel.channelState.LocalChanCfg.DustLimit, + ) + feeRate := chainfee.SatPerKWeight( + aliceChannel.channelState.LocalCommitment.FeePerKw, + ) + htlcFee := lnwire.NewMSatFromSatoshis( + feeRate.FeeForWeight(input.HTLCWeight), + ) + commitFee := lnwire.NewMSatFromSatoshis( + aliceChannel.channelState.LocalCommitment.CommitFee, + ) + htlcTimeoutFee := lnwire.NewMSatFromSatoshis( + htlcTimeoutFee(feeRate), + ) + + // Helper method to check the current reported balance. + checkBalance := func(t *testing.T, expBalanceAlice lnwire.MilliSatoshi) { + t.Helper() + balance := aliceChannel.AvailableBalance() + if balance != expBalanceAlice { + t.Fatalf("Expected balance %v, got %v", expBalanceAlice, + balance) + } + } + + // Helper method to send an HTLC from Alice to Bob, decreasing Alice's + // balance. + htlcIndex := uint64(0) + sendHtlc := func(htlcAmt lnwire.MilliSatoshi) { + t.Helper() + + htlc, preImage := createHTLC(int(htlcIndex), htlcAmt) + if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { + t.Fatalf("unable to add htlc: %v", err) + } + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + + if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("unable to complete alice's state "+ + "transition: %v", err) + } + + err = bobChannel.SettleHTLC(preImage, htlcIndex, nil, nil, nil) + if err != nil { + t.Fatalf("unable to settle htlc: %v", err) + } + err = aliceChannel.ReceiveHTLCSettle(preImage, htlcIndex) + if err != nil { + t.Fatalf("unable to settle htlc: %v", err) + } + + if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("unable to complete alice's state "+ + "transition: %v", err) + } + + htlcIndex++ + aliceBalance -= htlcAmt + } + + // Balance should start out equal to half the channel capacity minus + // the commitment fee Alice must pay and the channel reserve. In + // addition the HTLC fee will be subtracted fromt the balance to + // reflect that this value must be reserved for any payment above the + // dust limit. + expAliceBalance := aliceBalance - commitFee - aliceReserve - htlcFee + checkBalance(t, expAliceBalance) + + // Find the minumim size of a non-dust HTLC. + aliceNonDustHtlc := aliceDustlimit + htlcTimeoutFee + + // Send a HTLC leaving the remaining balance just enough to have + // nonDustHtlc left after paying the commit fee and htlc fee. + htlcAmt := aliceBalance - (commitFee + aliceReserve + htlcFee + aliceNonDustHtlc) + sendHtlc(htlcAmt) + + // Now the real balance left will be + // nonDustHtlc+commitfee+aliceReserve+htlcfee. The available balance + // reported will just be nonDustHtlc, since the rest of the balance is + // reserved. + expAliceBalance = aliceNonDustHtlc + checkBalance(t, expAliceBalance) + + // Send an HTLC using all but one msat of the reported balance. + htlcAmt = aliceNonDustHtlc - 1 + sendHtlc(htlcAmt) + + // 1 msat should be left. + expAliceBalance = 1 + checkBalance(t, expAliceBalance) + + // Sendng the last msat. + htlcAmt = 1 + sendHtlc(htlcAmt) + + // No balance left. + expAliceBalance = 0 + checkBalance(t, expAliceBalance) +} + // TestSignCommitmentFailNotLockedIn tests that a channel will not attempt to // create a new state if it doesn't yet know of the next revocation point for // the remote party.