diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 797c3c724f..cd2d1fca4c 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -2163,26 +2163,21 @@ func (l *channelLink) ChanID() lnwire.ChannelID { // // NOTE: Part of the ChannelLink interface. func (l *channelLink) Bandwidth() lnwire.MilliSatoshi { + // Get the balance available on the channel for new HTLCs. This takes + // the channel reserve into account so HTLCs up to this value won't + // violate it. channelBandwidth := l.channel.AvailableBalance() - overflowBandwidth := l.overflowQueue.TotalHtlcAmount() // To compute the total bandwidth, we'll take the current available // bandwidth, then subtract the overflow bandwidth as we'll eventually // also need to evaluate those HTLC's once space on the commitment // transaction is free. - linkBandwidth := channelBandwidth - overflowBandwidth - - // If the channel reserve is greater than the total available balance - // of the link, just return 0. - reserve := lnwire.NewMSatFromSatoshis(l.channel.LocalChanReserve()) - if linkBandwidth < reserve { + overflowBandwidth := l.overflowQueue.TotalHtlcAmount() + if channelBandwidth < overflowBandwidth { return 0 } - // Else the amount that is available to flow through the link at this - // point is the available balance minus the reserve amount we are - // required to keep as collateral. - return linkBandwidth - reserve + return channelBandwidth - overflowBandwidth } // AttachMailBox updates the current mailbox used by this link, and hooks up diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 3b48c157a4..0b162cd032 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1983,8 +1983,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 @@ -2657,8 +2660,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 @@ -2936,8 +2941,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 @@ -3192,9 +3199,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 fb774c1a5a..08ec5ebcaf 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -3103,12 +3103,13 @@ func (lc *LightningChannel) getUnsignedAckedUpdates() []channeldb.LogUpdate { // validateCommitmentSanity is used to validate the current state of the // commitment transaction in terms of the ChannelConstraints that we and our -// remote peer agreed upon during the funding workflow. The predictAdded -// parameter should be set to a valid PaymentDescriptor if we are validating -// in the state when adding a new HTLC, or nil otherwise. +// remote peer agreed upon during the funding workflow. The +// predict[Our|Their]Add should parameters should be set to a valid +// PaymentDescriptor if we are validating in the state when adding a new HTLC, +// or nil otherwise. func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, ourLogCounter uint64, remoteChain bool, - predictAdded *PaymentDescriptor) error { + predictOurAdd, predictTheirAdd *PaymentDescriptor) error { // Fetch all updates not committed. view := lc.fetchHTLCView(theirLogCounter, ourLogCounter) @@ -3116,14 +3117,11 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, // If we are checking if we can add a new HTLC, we add this to the // appropriate update log, in order to validate the sanity of the // commitment resulting from _actually adding_ this HTLC to the state. - if predictAdded != nil { - // If the remoteChain bool is true, add to ourUpdates. - if remoteChain { - view.ourUpdates = append(view.ourUpdates, predictAdded) - } else { - // Else add to theirUpdates. - view.theirUpdates = append(view.theirUpdates, predictAdded) - } + if predictOurAdd != nil { + view.ourUpdates = append(view.ourUpdates, predictOurAdd) + } + if predictTheirAdd != nil { + view.theirUpdates = append(view.theirUpdates, predictTheirAdd) } commitChain := lc.localCommitChain @@ -3296,7 +3294,7 @@ func (lc *LightningChannel) SignNextCommitment() (lnwire.Sig, []lnwire.Sig, []ch // party set up when we initially set up the channel. If we are, then // we'll abort this state transition. err := lc.validateCommitmentSanity( - remoteACKedIndex, lc.localUpdateLog.logIndex, true, nil, + remoteACKedIndex, lc.localUpdateLog.logIndex, true, nil, nil, ) if err != nil { return sig, htlcSigs, nil, err @@ -4050,7 +4048,7 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, // the constraints we specified during initial channel setup. If not, // then we'll abort the channel as they've violated our constraints. err := lc.validateCommitmentSanity( - lc.remoteUpdateLog.logIndex, localACKedIndex, false, nil, + lc.remoteUpdateLog.logIndex, localACKedIndex, false, nil, nil, ) if err != nil { return err @@ -4625,6 +4623,12 @@ func (lc *LightningChannel) InitNextRevocation(revKey *btcec.PublicKey) error { // The additional openKey argument corresponds to the incoming CircuitKey of the // committed circuit for this HTLC. This value should never be nil. // +// Note that AddHTLC doesn't reserve the HTLC fee for future payment (like +// AvailableBalance does), so one could get into the "stuck channel" state by +// sending dust HTLCs. +// TODO(halseth): fix this either by using additional reserve, or better commit +// format. See https://github.com/lightningnetwork/lightning-rfc/issues/728 +// // NOTE: It is okay for sourceRef to be nil when unit testing the wallet. func (lc *LightningChannel) AddHTLC(htlc *lnwire.UpdateAddHTLC, openKey *channeldb.CircuitKey) (uint64, error) { @@ -4644,10 +4648,26 @@ func (lc *LightningChannel) AddHTLC(htlc *lnwire.UpdateAddHTLC, } // Make sure adding this HTLC won't violate any of the constraints we - // must keep on our commitment transaction. + // must keep on the commitment transactions. remoteACKedIndex := lc.localCommitChain.tail().theirMessageIndex + + // First we'll check whether this HTLC can be added to the remote + // commitment transaction without violation any of the constraints. err := lc.validateCommitmentSanity( - remoteACKedIndex, lc.localUpdateLog.logIndex, true, pd, + remoteACKedIndex, lc.localUpdateLog.logIndex, true, pd, nil, + ) + if err != nil { + return 0, err + } + + // We must also check whether it can be added to our own commitment + // transaction, or the remote node will refuse to sign. This is not + // totally bullet proof, as the remote might be adding updates + // concurrently, but if we fail this check there is for sure not + // possible for us to add the HTLC. + err = lc.validateCommitmentSanity( + lc.remoteUpdateLog.logIndex, lc.localUpdateLog.logIndex, + false, pd, nil, ) if err != nil { return 0, err @@ -4685,7 +4705,7 @@ func (lc *LightningChannel) ReceiveHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, err // Clamp down on the number of HTLC's we can receive by checking the // commitment sanity. err := lc.validateCommitmentSanity( - lc.remoteUpdateLog.logIndex, localACKedIndex, false, pd, + lc.remoteUpdateLog.logIndex, localACKedIndex, false, nil, pd, ) if err != nil { return 0, err @@ -5983,11 +6003,14 @@ func (lc *LightningChannel) CompleteCooperativeClose(localSig, remoteSig []byte, return closeTx, ourBalance, nil } -// AvailableBalance returns the current available balance within the channel. -// By available balance, we mean that if at this very instance s new commitment -// were to be created which evals all the log entries, what would our available -// balance me. This method is useful when deciding if a given channel can -// accept an HTLC in the multi-hop forwarding scenario. +// AvailableBalance returns the current balance available for sending within +// the channel. By available balance, we mean that if at this very instance a +// new commitment were to be created which evals all the log entries, what +// would our available balance for adding an additional HTLC be. It takes into +// account the fee that must be paid for adding this HTLC (if we're the +// initiator), and that we cannot spend from the channel reserve. This method +// is useful when deciding if a given channel can accept an HTLC in the +// multi-hop forwarding scenario. func (lc *LightningChannel) AvailableBalance() lnwire.MilliSatoshi { lc.RLock() defer lc.RUnlock() @@ -6007,19 +6030,132 @@ func (lc *LightningChannel) availableBalance() (lnwire.MilliSatoshi, int64) { htlcView := lc.fetchHTLCView(remoteACKedIndex, lc.localUpdateLog.logIndex) - // Then compute our current balance for that view. - ourBalance, _, commitWeight, filteredView, err := - lc.computeView(htlcView, false, false) + // Calculate our available balance from our local commitment. + // TODO(halseth): could reuse parts validateCommitmentSanity to do this + // balance calculation, as most of the logic is the same. + // + // NOTE: This is not always accurate, since the remote node can always + // add updates concurrently, causing our balance to go down if we're + // the initiator, but this is a problem on the protocol level. + ourLocalCommitBalance, commitWeight := lc.availableCommitmentBalance( + htlcView, false, + ) + + // Do the same calculation from the remote commitment point of view. + ourRemoteCommitBalance, _ := lc.availableCommitmentBalance( + htlcView, true, + ) + + // Return which ever balance is lowest. + if ourRemoteCommitBalance < ourLocalCommitBalance { + return ourRemoteCommitBalance, commitWeight + } + + return ourLocalCommitBalance, commitWeight +} + +// availableCommitmentBalance attempts to calculate the balance we have +// available for HTLCs on the local/remote commitment given the htlcView. To +// account for sending HTLCs of different sizes, it will report the balance +// available for sending non-dust HTLCs, which will be manifested on the +// commitment, increasing the commitment fee we must pay as an initiator, +// eating into our balance. It will make sure we won't violate the channel +// reserve constraints for this amount. +func (lc *LightningChannel) availableCommitmentBalance(view *htlcView, + remoteChain bool) (lnwire.MilliSatoshi, int64) { + + // Compute the current balances for this commitment. This will take + // into account HTLCs to determine the commit weight, which the + // initiator must pay the fee for. + ourBalance, theirBalance, commitWeight, filteredView, err := lc.computeView( + view, remoteChain, false, + ) if err != nil { lc.log.Errorf("Unable to fetch available balance: %v", err) return 0, 0 } - // If we are the channel initiator, we must remember to subtract the - // commitment fee from our available balance. - commitFee := filteredView.feePerKw.FeeForWeight(commitWeight) + // We can never spend from the channel reserve, so we'll subtract it + // from our available balance. + ourReserve := lnwire.NewMSatFromSatoshis( + lc.channelState.LocalChanCfg.ChanReserve, + ) + if ourReserve <= ourBalance { + ourBalance -= ourReserve + } else { + ourBalance = 0 + } + + // 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 + htlcCommitFee := lnwire.NewMSatFromSatoshis( + feePerKw.FeeForWeight(commitWeight + input.HTLCWeight), + ) + + // 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 { - ourBalance -= lnwire.NewMSatFromSatoshis(commitFee) + // 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 + } + + return ourBalance - htlcCommitFee, commitWeight + } + + // If we're not the initiator, we must check whether the remote has + // enough balance to pay for the fee of our HTLC. We'll start by also + // subtracting our counterparty's reserve from their balance. + theirReserve := lnwire.NewMSatFromSatoshis( + lc.channelState.RemoteChanCfg.ChanReserve, + ) + if theirReserve <= theirBalance { + theirBalance -= theirReserve + } else { + theirBalance = 0 + } + + // We'll use the dustlimit and htlcFee to find the largest HTLC value + // that will be considered dust on the commitment. + dustlimit := lnwire.NewMSatFromSatoshis( + lc.channelState.LocalChanCfg.DustLimit, + ) + + // For an extra HTLC fee to be paid on our commitment, the HTLC must be + // large enough to make a non-dust HTLC timeout transaction. + htlcFee := lnwire.NewMSatFromSatoshis( + htlcTimeoutFee(feePerKw), + ) + + // If we are looking at the remote commitment, we must use the remote + // dust limit and the fee for adding an HTLC success transaction. + if remoteChain { + dustlimit = lnwire.NewMSatFromSatoshis( + lc.channelState.RemoteChanCfg.DustLimit, + ) + htlcFee = lnwire.NewMSatFromSatoshis( + htlcSuccessFee(feePerKw), + ) + } + + // The HTLC output will be manifested on the commitment if it + // is non-dust after paying the HTLC fee. + nonDustHtlcAmt := dustlimit + htlcFee + + // If they cannot pay the fee if we add another non-dust HTLC, we'll + // report our available balance just below the non-dust amount, to + // avoid attempting HTLCs larger than this size. + if theirBalance < htlcCommitFee && ourBalance >= nonDustHtlcAmt { + ourBalance = nonDustHtlcAmt - 1 } return ourBalance, commitWeight @@ -6230,13 +6366,14 @@ func (lc *LightningChannel) MaxFeeRate(maxAllocation float64) chainfee.SatPerKWe // The maximum fee depends of the available balance that can be // committed towards fees. - balance, weight := lc.availableBalance() + commit := lc.channelState.LocalCommitment feeBalance := float64( - balance.ToSatoshis() + lc.channelState.LocalCommitment.CommitFee, + commit.LocalBalance.ToSatoshis() + commit.CommitFee, ) maxFee := feeBalance * maxAllocation // Ensure the fee rate doesn't dip below the fee floor. + _, weight := lc.availableBalance() maxFeeRate := maxFee / (float64(weight) / 1000) return chainfee.SatPerKWeight( math.Max(maxFeeRate, float64(chainfee.FeePerKwFloor)), diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 7cd5aa9f12..c4c886709d 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -4599,6 +4599,16 @@ func TestChanAvailableBandwidth(t *testing.T) { } defer cleanUp() + 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 // method for the current available channel bandwidth from @@ -4625,11 +4635,15 @@ func TestChanAvailableBandwidth(t *testing.T) { // Now, we'll obtain the current available bandwidth in Alice's // latest commitment and compare that to the prior estimate. aliceBalance := aliceChannel.channelState.LocalCommitment.LocalBalance - if aliceBalance != aliceAvailableBalance { + + // The balance we have available for new HTLCs should be the + // 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, "+ - "got %v", line, aliceBalance, - aliceAvailableBalance) + "got %v", line, expBalance, aliceAvailableBalance) } } @@ -4707,6 +4721,185 @@ 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 and Bob start with half the channel capacity. + aliceBalance := lnwire.NewMSatFromSatoshis(5 * btcutil.SatoshiPerBitcoin) + bobBalance := lnwire.NewMSatFromSatoshis(5 * btcutil.SatoshiPerBitcoin) + + aliceReserve := lnwire.NewMSatFromSatoshis( + aliceChannel.channelState.LocalChanCfg.ChanReserve, + ) + bobReserve := lnwire.NewMSatFromSatoshis( + bobChannel.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), + ) + htlcSuccessFee := lnwire.NewMSatFromSatoshis( + htlcSuccessFee(feeRate), + ) + + // Helper method to check the current reported balance. + checkBalance := func(t *testing.T, expBalanceAlice, + expBalanceBob lnwire.MilliSatoshi) { + + t.Helper() + aliceBalance := aliceChannel.AvailableBalance() + if aliceBalance != expBalanceAlice { + t.Fatalf("Expected alice balance %v, got %v", + expBalanceAlice, aliceBalance) + } + + bobBalance := bobChannel.AvailableBalance() + if bobBalance != expBalanceBob { + t.Fatalf("Expected bob balance %v, got %v", + expBalanceBob, bobBalance) + } + } + + // 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 + bobBalance += 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 + + // Bob is not the initiator, so he will have all his balance available, + // since Alice pays for fees. Bob only need to keep his balance above + // the reserve. + expBobBalance := bobBalance - bobReserve + checkBalance(t, expAliceBalance, expBobBalance) + + // Find the minumim size of a non-dust HTLC. + aliceNonDustHtlc := aliceDustlimit + htlcTimeoutFee + + // Send a HTLC leaving Alice's 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 + expBobBalance = bobBalance - bobReserve + checkBalance(t, expAliceBalance, expBobBalance) + + // Send an HTLC using all but one msat of the reported balance. + htlcAmt = aliceNonDustHtlc - 1 + sendHtlc(htlcAmt) + + // 1 msat should be left. + expAliceBalance = 1 + + // Bob should still have all his balance available, since even though + // Alice cannot afford to add a non-dust HTLC, she can afford to add a + // non-dust HTLC from Bob. + expBobBalance = bobBalance - bobReserve + checkBalance(t, expAliceBalance, expBobBalance) + + // Sendng the last msat. + htlcAmt = 1 + sendHtlc(htlcAmt) + + // No balance left. + expAliceBalance = 0 + + // We try to always reserve enough for the non-iniitator to be able to + // add an HTLC, hence Bob should still have all his non-reserved + // balance available. + expBobBalance = bobBalance - bobReserve + checkBalance(t, expAliceBalance, expBobBalance) + + // Even though Alice has a reported balance of 0, this is because we + // try to avoid getting into the position where she cannot pay the fee + // for Bob adding another HTLC. This means she actually _has_ some + // balance left, and we now force the channel into this situation by + // sending yet another HTLC. In practice this can also happen if a fee + // update eats into Alice's balance. + htlcAmt = 1 + sendHtlc(htlcAmt) + + // Now Alice balance is so low that she cannot even afford to add a new + // HTLC from Bob to the commitment transaction. Bob's balance should + // reflect this, by only reporting dust amount being available. Alice + // should still report a zero balance. + + // Since the dustlimit is different for the two commitments, the + // largest HTLC Bob can send that Alice can afford on both commitments + // (remember she cannot afford to pay the HTLC fee) is the largest dust + // HTLC on Alice's commitemnt, since her dust limit is lower. + bobNonDustHtlc := aliceDustlimit + htlcSuccessFee + expBobBalance = bobNonDustHtlc - 1 + expAliceBalance = 0 + checkBalance(t, expAliceBalance, expBobBalance) +} + // 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. @@ -6007,6 +6200,105 @@ func TestChanReserve(t *testing.T) { ) } +// TestChanReserveRemoteInitiator tests that the channel reserve of the +// initiator is accounted for when adding HTLCs, whether the initiator is the +// local or remote node. +func TestChanReserveRemoteInitiator(t *testing.T) { + t.Parallel() + + // We start out with a channel where both parties have 5 BTC. + aliceChannel, bobChannel, cleanUp, err := CreateTestChannels( + true, + ) + if err != nil { + t.Fatal(err) + } + defer cleanUp() + + // Set Alice's channel reserve to be 5 BTC-commitfee. This means she + // has just enough balance to cover the comitment fee, but not enough + // to add any more HTLCs to the commitment. Although a reserve this + // high is unrealistic, a channel can easiliy get into a situation + // where the initiator cannot pay for the fee of any more HTLCs. + commitFee := aliceChannel.channelState.LocalCommitment.CommitFee + aliceMinReserve := 5*btcutil.SatoshiPerBitcoin - commitFee + + aliceChannel.channelState.LocalChanCfg.ChanReserve = aliceMinReserve + bobChannel.channelState.RemoteChanCfg.ChanReserve = aliceMinReserve + + // Now let Bob attempt to add an HTLC of 0.1 BTC. He has plenty of + // money available to spend, but Alice, which is the initiator, cannot + // afford any more HTLCs on the commitment transaction because that + // would take here below her channel reserve.. + htlcAmt := lnwire.NewMSatFromSatoshis(0.1 * btcutil.SatoshiPerBitcoin) + htlc, _ := createHTLC(0, htlcAmt) + + // Bob should refuse to add this HTLC, since he realizes it will create + // an invalid commitment. + _, err = bobChannel.AddHTLC(htlc, nil) + if err != ErrBelowChanReserve { + t.Fatalf("expected ErrBelowChanReserve, instead received: %v", + err) + } + + // Of course Alice will also not have enough balance to add it herself. + _, err = aliceChannel.AddHTLC(htlc, nil) + if err != ErrBelowChanReserve { + t.Fatalf("expected ErrBelowChanReserve, instead received: %v", + err) + } + + // Same for Alice, she should refuse to accept this second HTLC. + if _, err := aliceChannel.ReceiveHTLC(htlc); err != ErrBelowChanReserve { + t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err) + } +} + +// TestChanReserveLocalInitiatorDustHtlc tests that fee the initiator must pay +// when adding HTLCs is accounted for, even though the HTLC is considered dust +// by the remote bode. +func TestChanReserveLocalInitiatorDustHtlc(t *testing.T) { + t.Parallel() + + aliceChannel, bobChannel, cleanUp, err := CreateTestChannels( + true, + ) + if err != nil { + t.Fatal(err) + } + defer cleanUp() + + // The amount of the HTLC should not be considered dust according to + // Alice's dust limit (200 sat), but be dust according to Bob's dust + // limit (1300 sat). It is considered dust if the amount remaining + // after paying the HTLC fee is below the dustlimit, so we choose a + // size of 500+htlcFee. + htlcSat := btcutil.Amount(500) + htlcTimeoutFee( + chainfee.SatPerKWeight( + aliceChannel.channelState.LocalCommitment.FeePerKw, + ), + ) + + // Set Alice's channel reserve to be low enough to carry the value of + // the HTLC, but not low enough to allow the extra fee from adding the + // HTLC to the commitment. + commitFee := aliceChannel.channelState.LocalCommitment.CommitFee + aliceMinReserve := 5*btcutil.SatoshiPerBitcoin - commitFee - htlcSat + + aliceChannel.channelState.LocalChanCfg.ChanReserve = aliceMinReserve + bobChannel.channelState.RemoteChanCfg.ChanReserve = aliceMinReserve + + htlcDustAmt := lnwire.NewMSatFromSatoshis(htlcSat) + htlc, _ := createHTLC(0, htlcDustAmt) + + // Alice should realize that the fee she must pay to add this HTLC to + // the local commitment would take her below the channel reserve. + _, err = aliceChannel.AddHTLC(htlc, nil) + if err != ErrBelowChanReserve { + t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err) + } +} + // TestMinHTLC tests that the ErrBelowMinHTLC error is thrown if an HTLC is added // that is below the minimm allowed value for HTLCs. func TestMinHTLC(t *testing.T) {