diff --git a/docs/release-notes/release-notes-0.18.0.md b/docs/release-notes/release-notes-0.18.0.md index 23ab71b38..3d412a373 100644 --- a/docs/release-notes/release-notes-0.18.0.md +++ b/docs/release-notes/release-notes-0.18.0.md @@ -52,6 +52,15 @@ these unconfirmed transactions are already removed. In addition a new walletrpc endpoint `RemoveTransaction` is introduced which let one easily remove unconfirmed transaction manually. + +* [Fixed](https://github.com/lightningnetwork/lnd/pull/8096) a case where `lnd` + might dip below its channel reserve when htlcs are added concurrently. A + fee buffer (additional balance) is now always kept on the local side ONLY + if the channel was opened locally. This is in accordance with the BOTL 02 + specification and protects against sharp fee changes because there is always + this buffer which can be used to increase the commitment fee and it also + protects against the case where htlcs are added asynchronously resulting in + stuck channels. # New Features ## Functional Enhancements @@ -223,3 +232,4 @@ * Turtle * Ononiwu Maureen Chiamaka * Yong Yu +* Ziggie diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 629c46aeb..49965a97a 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1423,6 +1423,11 @@ func (l *channelLink) handleDownstreamUpdateAdd(pkt *htlcPacket) error { // commitment chains. htlc.ChanID = l.ChanID() openCircuitRef := pkt.inKey() + + // We enforce the fee buffer for the commitment transaction because + // we are in control of adding this htlc. Nothing has locked-in yet so + // we can securely enforce the fee buffer which is only relevant if we + // are the initiator of the channel. index, err := l.channel.AddHTLC(htlc, &openCircuitRef) if err != nil { // The HTLC was unable to be added to the state machine, diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 9c07cd53c..baaa002a2 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -2231,15 +2231,24 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { } var ( - carolChanID = lnwire.NewShortChanIDFromInt(3) - mockBlob [lnwire.OnionPacketSize]byte - coreChan = aliceLink.(*channelLink).channel - coreLink = aliceLink.(*channelLink) - defaultCommitFee = coreChan.StateSnapshot().CommitFee - aliceStartingBandwidth = aliceLink.Bandwidth() - aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs + carolChanID = lnwire.NewShortChanIDFromInt(3) + mockBlob [lnwire.OnionPacketSize]byte + coreLink *channelLink + aliceMsgs chan lnwire.Message + chanAmtMSat = lnwire.NewMSatFromSatoshis(chanAmt) + cType = channeldb.SingleFunderTweaklessBit + commitWeight = lnwallet.CommitWeight(cType) ) + link, ok := aliceLink.(*channelLink) + require.True(t, ok) + coreLink = link + mockedPeer := coreLink.cfg.Peer + + peer, ok := mockedPeer.(*mockPeer) + require.True(t, ok) + aliceMsgs = peer.sentMsgs + // We put Alice into hodl.ExitSettle mode, such that she won't settle // incoming HTLCs automatically. coreLink.cfg.HodlMask = hodl.MaskFromFlags(hodl.ExitSettle) @@ -2247,16 +2256,17 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { estimator := chainfee.NewStaticEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) require.NoError(t, err, "unable to query fee estimator") - htlcFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(input.HTLCWeight), + + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight := int64(1) * input.HTLCWeight + feeBuffer := lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, ) // The starting bandwidth of the channel should be exactly the amount - // 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 + // that we created the channel between her and Bob, minus the feebuffer. + expectedBandwidth := chanAmtMSat - feeBuffer assertLinkBandwidth(t, aliceLink, expectedBandwidth) // Next, we'll create an HTLC worth 1 BTC, and send it into the link as @@ -2284,9 +2294,16 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { } time.Sleep(time.Millisecond * 500) + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight = int64(2) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + // The resulting bandwidth should reflect that Alice is paying the - // htlc amount in addition to the htlc fee. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + // htlc amount in addition to the fee buffer. + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Alice should send the HTLC to Bob. var msg lnwire.Message @@ -2309,7 +2326,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { t.Fatalf("unable to update state: %v", err) } // Locking in the HTLC should not change Alice's bandwidth. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // If we now send in a valid HTLC settle for the prior HTLC we added, // then the bandwidth should remain unchanged as the remote party will @@ -2324,16 +2341,23 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { time.Sleep(time.Millisecond * 500) // Since the settle is not locked in yet, Alice's bandwidth should still - // reflect that she has to pay the fee. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + // reflect that she has to account for the fee buffer. + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Lock in the settle. if err := updateState(tmr, coreLink, bobChannel, false); err != nil { t.Fatalf("unable to update state: %v", err) } - // Now that it is settled, Alice should have gotten the htlc fee back. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt) + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight = int64(1) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + + // Now that it is settled, Alice fee buffer should have been decreased. + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Next, we'll add another HTLC initiated by the switch (of the same // amount as the prior one). @@ -2356,8 +2380,15 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { } time.Sleep(time.Millisecond * 500) - // Again, Alice's bandwidth decreases by htlcAmt+htlcFee. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-2*htlcAmt-htlcFee) + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight = int64(2) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + + // Again, Alice's bandwidth decreases by htlcAmt and fee buffer. + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-2*htlcAmt) // Alice will send the HTLC to Bob. select { @@ -2379,7 +2410,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { t.Fatalf("unable to update state: %v", err) } - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt*2-htlcFee) + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt*2) // With that processed, we'll now generate an HTLC fail (sent by the // remote peer) to cancel the HTLC we just added. This should return us @@ -2398,15 +2429,22 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { time.Sleep(time.Millisecond * 500) // Before the Fail gets locked in, the bandwidth should remain unchanged. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt*2-htlcFee) + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt*2) // Lock in the Fail. if err := updateState(tmr, coreLink, bobChannel, false); err != nil { t.Fatalf("unable to update state: %v", err) } + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight = int64(1) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + // Now the bandwidth should reflect the failed HTLC. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt) + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Moving along, we'll now receive a new HTLC from the remote peer, // with an ID of 0 as this is their first HTLC. The bandwidth should @@ -2435,15 +2473,23 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { aliceLink.HandleChannelUpdate(htlc) // Alice's balance remains unchanged until this HTLC is locked in. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt) + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Lock in the HTLC. if err := updateState(tmr, coreLink, bobChannel, false); err != nil { t.Fatalf("unable to update state: %v", err) } - // Since Bob is adding this HTLC, Alice only needs to pay the fee. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight = int64(2) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + + // Since Bob is adding this HTLC, Alice will only have an increased fee + // buffer. + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) time.Sleep(time.Millisecond * 500) addPkt = htlcPacket{ @@ -2484,8 +2530,15 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { } time.Sleep(time.Millisecond * 500) + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight = int64(1) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + // Settling this HTLC gives Alice all her original bandwidth back. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth) + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) select { case msg = <-aliceMsgs: @@ -2532,13 +2585,20 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { time.Sleep(time.Millisecond * 500) // No changes before the HTLC is locked in. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth) + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) if err := updateState(tmr, coreLink, bobChannel, false); err != nil { t.Fatalf("unable to update state: %v", err) } - // After lock-in, Alice will have to pay the htlc fee. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcFee) + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight = int64(2) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + + // After lock-in, Alice will have to account for a fee buffer. + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) addPkt = htlcPacket{ htlc: htlc, @@ -2575,8 +2635,15 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { } time.Sleep(time.Millisecond * 500) + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight = int64(1) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + // Alice should get all her bandwidth back. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth) + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) // Message should be sent to Bob. select { @@ -2596,7 +2663,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { if err := handleStateUpdate(coreLink, bobChannel); err != nil { t.Fatalf("unable to update state: %v", err) } - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth) + assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) } // genAddsAndCircuits creates `numHtlcs` sequential ADD packets and there @@ -2636,6 +2703,12 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { halfHtlcs = numHtlcs / 2 ) + var ( + chanAmtMSat = lnwire.NewMSatFromSatoshis(chanAmt) + cType = channeldb.SingleFunderTweaklessBit + commitWeight = lnwallet.CommitWeight(cType) + ) + // We'll start by creating a new link with our chanAmt (5 BTC). We will // only be testing Alice's behavior, so the reference to Bob's channel // state is unnecessary. @@ -2657,23 +2730,19 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { feePerKw, err := estimator.EstimateFeePerKW(1) require.NoError(t, err, "unable to query fee estimator") - defaultCommitFee := alice.channel.StateSnapshot().CommitFee - htlcFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(input.HTLCWeight), + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight := int64(1) * input.HTLCWeight + feeBuffer := lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, ) // The starting bandwidth of the channel should be exactly the amount - // that we created the channel between her and Bob, minus the commitment - // fee and fee of adding an HTLC. - expectedBandwidth := lnwire.NewMSatFromSatoshis( - chanAmt-defaultCommitFee, - ) - htlcFee + // that we created the channel between her and Bob, minus the fee + // buffer. + expectedBandwidth := chanAmtMSat - feeBuffer assertLinkBandwidth(t, alice.link, expectedBandwidth) - // Capture Alice's starting bandwidth to perform later, relative - // bandwidth assertions. - aliceStartingBandwidth := alice.link.Bandwidth() - // Next, we'll create an HTLC worth 1 BTC that will be used as a dummy // message for the test. var mockBlob [lnwire.OnionPacketSize]byte @@ -2707,10 +2776,17 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { // Wait until Alice's link has sent both HTLCs via the peer. alice.checkSent(addPkts[:halfHtlcs]) + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight = int64(1+halfHtlcs) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + // The resulting bandwidth should reflect that Alice is paying both - // htlc amounts, in addition to both htlc fees. + // htlc amounts, in addition to the new fee buffer. assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-halfHtlcs*(htlcAmt+htlcFee), + chanAmtMSat-feeBuffer-halfHtlcs*(htlcAmt), ) // Now, initiate a state transition by Alice so that the pending HTLCs @@ -2740,9 +2816,9 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { // The resulting bandwidth should remain unchanged from before, // reflecting that Alice is paying both htlc amounts, in addition to - // both htlc fees. + // the fee buffer. assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-halfHtlcs*(htlcAmt+htlcFee), + chanAmtMSat-feeBuffer-halfHtlcs*(htlcAmt), ) // Now, restart Alice's link *and* the entire switch. This will ensure @@ -2793,8 +2869,12 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { // With two HTLCs on the pending commit, and two added to the in-memory // commitment state, the resulting bandwidth should reflect that Alice // is paying the all htlc amounts in addition to all htlc fees. + htlcWeight = int64(1+numHtlcs) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-numHtlcs*(htlcAmt+htlcFee), + chanAmtMSat-feeBuffer-numHtlcs*(htlcAmt), ) // We will try to initiate a state transition for Alice, which will @@ -2834,7 +2914,7 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { // reflect that Alice is paying the all htlc amounts in addition to all // htlc fees. assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-numHtlcs*(htlcAmt+htlcFee), + chanAmtMSat-feeBuffer-numHtlcs*(htlcAmt), ) // Again, we will try to initiate a state transition for Alice, which @@ -2881,8 +2961,12 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { // Since the latter two HTLCs have been completely dropped from memory, // only the first two HTLCs we added should still be reflected in the // channel bandwidth. + htlcWeight = int64(1+halfHtlcs) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-halfHtlcs*(htlcAmt+htlcFee), + chanAmtMSat-feeBuffer-halfHtlcs*(htlcAmt), ) } @@ -2901,6 +2985,12 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { halfHtlcs = numHtlcs / 2 ) + var ( + chanAmtMSat = lnwire.NewMSatFromSatoshis(chanAmt) + cType = channeldb.SingleFunderTweaklessBit + commitWeight = lnwallet.CommitWeight(cType) + ) + // We'll start by creating a new link with our chanAmt (5 BTC). We will // only be testing Alice's behavior, so the reference to Bob's channel // state is unnecessary. @@ -2927,23 +3017,19 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { feePerKw, err := estimator.EstimateFeePerKW(1) require.NoError(t, err, "unable to query fee estimator") - defaultCommitFee := alice.channel.StateSnapshot().CommitFee - htlcFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(input.HTLCWeight), + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight := int64(1) * input.HTLCWeight + feeBuffer := lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, ) // The starting bandwidth of the channel should be exactly the amount - // 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 + // that we created the channel between her and Bob, minus the fee + // buffer. + expectedBandwidth := chanAmtMSat - feeBuffer assertLinkBandwidth(t, alice.link, expectedBandwidth) - // Capture Alice's starting bandwidth to perform later, relative - // bandwidth assertions. - aliceStartingBandwidth := alice.link.Bandwidth() - // Next, we'll create an HTLC worth 1 BTC that will be used as a dummy // message for the test. var mockBlob [lnwire.OnionPacketSize]byte @@ -2976,10 +3062,17 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { // Wait until Alice's link has sent both HTLCs via the peer. alice.checkSent(addPkts[:halfHtlcs]) + // We account for the 2 htlcs and the additional one which would be + // needed when sending and htlc. + htlcWeight = int64(1+halfHtlcs) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + // The resulting bandwidth should reflect that Alice is paying both // htlc amounts, in addition to both htlc fees. - assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-halfHtlcs*(htlcAmt+htlcFee), + assertLinkBandwidth( + t, alice.link, chanAmtMSat-feeBuffer-halfHtlcs*(htlcAmt), ) alice.assertNumPendingNumOpenCircuits(2, 0) @@ -3012,9 +3105,10 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { alice.checkSent(addPkts[:halfHtlcs]) // The resulting bandwidth should reflect that Alice is paying both htlc - // amounts, in addition to both htlc fees. - assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-halfHtlcs*(htlcAmt+htlcFee), + // amounts, in addition to both htlc fees. The fee buffer remains the + // same as before. + assertLinkBandwidth( + t, alice.link, chanAmtMSat-feeBuffer-halfHtlcs*(htlcAmt), ) // Again, initiate another state transition by Alice to try and commit @@ -3023,7 +3117,7 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { alice.trySignNextCommitment() alice.assertNumPendingNumOpenCircuits(2, 2) - // Now, we we will do a full restart of the link and switch, configuring + // Now, we will do a full restart of the link and switch, configuring // Alice again in hodl.Commit mode. Since none of the HTLCs were // actually committed, the previously opened circuits should be trimmed // by both the link and switch. @@ -3048,7 +3142,12 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { } // Alice's bandwidth should have reverted back to her starting value. - assertLinkBandwidth(t, alice.link, aliceStartingBandwidth) + htlcWeight = int64(1) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + + assertLinkBandwidth(t, alice.link, chanAmtMSat-feeBuffer) // Now, try to commit the last two payment circuits, which are unused // thus far. These should succeed without hesitation. @@ -3068,10 +3167,17 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { // peer. alice.checkSent(addPkts[halfHtlcs:]) + // We account for the 2 htlcs and the additional one which would be + // needed when sending and htlc. + htlcWeight = int64(1+halfHtlcs) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + // The resulting bandwidth should reflect that Alice is paying both htlc // amounts, in addition to both htlc fees. assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-halfHtlcs*(htlcAmt+htlcFee), + chanAmtMSat-feeBuffer-halfHtlcs*(htlcAmt), ) // Now, initiate a state transition for Alice. Since we are hodl.Commit @@ -3109,7 +3215,7 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { // Her bandwidth should now reflect having sent only those two HTLCs. assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-halfHtlcs*(htlcAmt+htlcFee), + chanAmtMSat-feeBuffer-halfHtlcs*(htlcAmt), ) // Now, initiate a state transition for Alice. Since we are hodl.Commit @@ -3141,8 +3247,13 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { t.Fatalf("expected %d packet to be failed", halfHtlcs) } + htlcWeight = int64(1) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + // Alice balance should not have changed since the start. - assertLinkBandwidth(t, alice.link, aliceStartingBandwidth) + assertLinkBandwidth(t, alice.link, chanAmtMSat-feeBuffer) } // TestChannelLinkTrimCircuitsRemoteCommit checks that the switch and link @@ -3156,6 +3267,12 @@ func TestChannelLinkTrimCircuitsRemoteCommit(t *testing.T) { numHtlcs = 2 ) + var ( + chanAmtMSat = lnwire.NewMSatFromSatoshis(chanAmt) + cType = channeldb.SingleFunderTweaklessBit + commitWeight = lnwallet.CommitWeight(cType) + ) + // We'll start by creating a new link with our chanAmt (5 BTC). aliceLink, bobChan, batchTicker, start, restore, err := newSingleLinkTestHarness(t, chanAmt, 0) @@ -3175,22 +3292,19 @@ func TestChannelLinkTrimCircuitsRemoteCommit(t *testing.T) { feePerKw, err := estimator.EstimateFeePerKW(1) require.NoError(t, err, "unable to query fee estimator") - defaultCommitFee := alice.channel.StateSnapshot().CommitFee - htlcFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(input.HTLCWeight), + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight := int64(1) * input.HTLCWeight + feeBuffer := lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, ) - // The starting bandwidth of the channel should be exactly the amount - // that we created the channel between her and Bob, minus the commitment - // fee and fee of adding an HTLC. - expectedBandwidth := lnwire.NewMSatFromSatoshis( - chanAmt-defaultCommitFee, - ) - htlcFee - assertLinkBandwidth(t, alice.link, expectedBandwidth) + expectedBandwidth := chanAmtMSat - feeBuffer - // Capture Alice's starting bandwidth to perform later, relative - // bandwidth assertions. - aliceStartingBandwidth := alice.link.Bandwidth() + // The starting bandwidth of the channel should be exactly the amount + // that we created the channel between her and Bob, minus the fee + // buffer. + assertLinkBandwidth(t, alice.link, expectedBandwidth) // Next, we'll create an HTLC worth 1 BTC that will be used as a dummy // message for the test. @@ -3242,8 +3356,13 @@ func TestChannelLinkTrimCircuitsRemoteCommit(t *testing.T) { // The resulting bandwidth should reflect that Alice is paying both // htlc amounts, in addition to both htlc fees. + htlcWeight = int64(1+numHtlcs) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + assertLinkBandwidth(t, alice.link, - aliceStartingBandwidth-numHtlcs*(htlcAmt+htlcFee), + chanAmtMSat-feeBuffer-numHtlcs*(htlcAmt), ) // Now, initiate a state transition by Alice so that the pending HTLCs @@ -3309,26 +3428,37 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { } var ( - mockBlob [lnwire.OnionPacketSize]byte - coreLink = aliceLink.(*channelLink) - coreChan = coreLink.channel - defaultCommitFee = coreChan.StateSnapshot().CommitFee - aliceStartingBandwidth = aliceLink.Bandwidth() - aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs + mockBlob [lnwire.OnionPacketSize]byte + coreLink *channelLink + aliceMsgs chan lnwire.Message + chanAmtMSat = lnwire.NewMSatFromSatoshis(chanAmt) + chanReserveMSat = lnwire.NewMSatFromSatoshis(chanReserve) + cType = channeldb.SingleFunderTweaklessBit + commitWeight = lnwallet.CommitWeight(cType) ) + link, ok := aliceLink.(*channelLink) + require.True(t, ok) + coreLink = link + mockedPeer := coreLink.cfg.Peer + + peer, ok := mockedPeer.(*mockPeer) + require.True(t, ok) + aliceMsgs = peer.sentMsgs + estimator := chainfee.NewStaticEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) require.NoError(t, err, "unable to query fee estimator") - htlcFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(input.HTLCWeight), - ) + + // Calculate the fee buffer for a channel state. Account for htlcs on + // the potential channel state as well. + htlcWeight := int64(1) * input.HTLCWeight + feeBuffer := lnwallet.CalcFeeBuffer(feePerKw, commitWeight+htlcWeight) // The starting bandwidth of the channel should be exactly the amount // that we created the channel between her and Bob, minus the channel - // reserve, commitment fee and fee for adding an additional HTLC. - expectedBandwidth := lnwire.NewMSatFromSatoshis( - chanAmt-defaultCommitFee-chanReserve) - htlcFee + // reserve and the fee buffer. + expectedBandwidth := chanAmtMSat - feeBuffer - chanReserveMSat assertLinkBandwidth(t, aliceLink, expectedBandwidth) // Next, we'll create an HTLC worth 3 BTC, and send it into the link as @@ -3348,7 +3478,14 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { _ = aliceLink.handleSwitchPacket(addPkt) time.Sleep(time.Millisecond * 100) - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + + htlcWeight = int64(2) * input.HTLCWeight + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+htlcWeight, + ) + assertLinkBandwidth( + t, aliceLink, chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, + ) // Alice should send the HTLC to Bob. var msg lnwire.Message @@ -3371,7 +3508,9 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { t.Fatalf("unable to update state: %v", err) } - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + assertLinkBandwidth( + t, aliceLink, chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, + ) // If we now send in a valid HTLC settle for the prior HTLC we added, // then the bandwidth should remain unchanged as the remote party will @@ -3387,15 +3526,23 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { // Since the settle is not locked in yet, Alice's bandwidth should still // reflect that she has to pay the fee. - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + assertLinkBandwidth( + t, aliceLink, chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, + ) // Lock in the settle. if err := updateState(batchTimer, coreLink, bobChannel, false); err != nil { t.Fatalf("unable to update state: %v", err) } + feeBuffer = lnwallet.CalcFeeBuffer( + feePerKw, commitWeight+input.HTLCWeight, + ) + time.Sleep(time.Millisecond * 100) - assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt) + assertLinkBandwidth( + t, aliceLink, chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, + ) // Now we create a channel that has a channel reserve that is // greater than it's balance. In these case only payments can diff --git a/itest/lnd_amp_test.go b/itest/lnd_amp_test.go index a76aedd99..e3b1898da 100644 --- a/itest/lnd_amp_test.go +++ b/itest/lnd_amp_test.go @@ -60,12 +60,12 @@ func testSendPaymentAMPInvoiceCase(ht *lntest.HarnessTest, // \__ Dave ____/ // mppReq := &mppOpenChannelRequest{ - amtAliceCarol: 235000, - amtAliceDave: 135000, - amtCarolBob: 135000, - amtCarolEve: 135000, - amtDaveBob: 135000, - amtEveBob: 135000, + amtAliceCarol: 285000, + amtAliceDave: 155000, + amtCarolBob: 200000, + amtCarolEve: 155000, + amtDaveBob: 155000, + amtEveBob: 155000, } mts.openChannels(mppReq) chanPointAliceDave := mts.channelPoints[1] @@ -368,12 +368,12 @@ func testSendPaymentAMP(ht *lntest.HarnessTest) { // \__ Dave ____/ // mppReq := &mppOpenChannelRequest{ - amtAliceCarol: 235000, - amtAliceDave: 135000, - amtCarolBob: 135000, - amtCarolEve: 135000, - amtDaveBob: 135000, - amtEveBob: 135000, + amtAliceCarol: 285000, + amtAliceDave: 155000, + amtCarolBob: 200000, + amtCarolEve: 155000, + amtDaveBob: 155000, + amtEveBob: 155000, } mts.openChannels(mppReq) chanPointAliceDave := mts.channelPoints[1] diff --git a/itest/lnd_multi-hop-error-propagation_test.go b/itest/lnd_multi-hop-error-propagation_test.go index 3941accb0..6bd3c1568 100644 --- a/itest/lnd_multi-hop-error-propagation_test.go +++ b/itest/lnd_multi-hop-error-propagation_test.go @@ -1,8 +1,6 @@ package itest import ( - "math" - "github.com/lightningnetwork/lnd/funding" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnrpc/routerrpc" @@ -240,55 +238,42 @@ func testHtlcErrorPropagation(ht *lntest.HarnessTest) { // // To do so, we'll push most of the funds in the channel over to // Alice's side, leaving on 10k satoshis of available balance for bob. - // There's a max payment amount, so we'll have to do this - // incrementally. chanReserve := int64(chanAmt / 100) - amtToSend := int64(chanAmt) - chanReserve - 20000 - amtSent := int64(0) - for amtSent != amtToSend { - // We'll send in chunks of the max payment amount. If we're - // about to send too much, then we'll only send the amount - // remaining. - toSend := int64(math.MaxUint32) - if toSend+amtSent > amtToSend { - toSend = amtToSend - amtSent - } + feeBuffer := lntest.CalcStaticFeeBuffer(cType, 0) + amtToSend := int64(chanAmt) - chanReserve - int64(feeBuffer) - 10000 - invoiceReq = &lnrpc.Invoice{ - Value: toSend, - } - carolInvoice2 := carol.RPC.AddInvoice(invoiceReq) - - req := &routerrpc.SendPaymentRequest{ - PaymentRequest: carolInvoice2.PaymentRequest, - TimeoutSeconds: 60, - FeeLimitMsat: noFeeLimitMsat, - MaxParts: 1, - } - ht.SendPaymentAndAssertStatus(bob, req, lnrpc.Payment_SUCCEEDED) - - // For each send bob makes, we need to check that bob has a - // forward and settle event for his send, and carol has a - // settle event and a final htlc event for her receive. - ht.AssertHtlcEventTypes( - bobEvents, routerrpc.HtlcEvent_SEND, - lntest.HtlcEventForward, - ) - ht.AssertHtlcEventTypes( - bobEvents, routerrpc.HtlcEvent_SEND, - lntest.HtlcEventSettle, - ) - ht.AssertHtlcEventTypes( - carolEvents, routerrpc.HtlcEvent_RECEIVE, - lntest.HtlcEventSettle, - ) - ht.AssertHtlcEventTypes( - carolEvents, routerrpc.HtlcEvent_UNKNOWN, - lntest.HtlcEventFinal, - ) - - amtSent += toSend + invoiceReq = &lnrpc.Invoice{ + Value: amtToSend, } + carolInvoice2 := carol.RPC.AddInvoice(invoiceReq) + + req := &routerrpc.SendPaymentRequest{ + PaymentRequest: carolInvoice2.PaymentRequest, + TimeoutSeconds: 60, + FeeLimitMsat: noFeeLimitMsat, + MaxParts: 1, + } + ht.SendPaymentAndAssertStatus(bob, req, lnrpc.Payment_SUCCEEDED) + + // We need to check that bob has a forward and settle event for his + // send, and carol has a settle event and a final htlc event for her + // receive. + ht.AssertHtlcEventTypes( + bobEvents, routerrpc.HtlcEvent_SEND, + lntest.HtlcEventForward, + ) + ht.AssertHtlcEventTypes( + bobEvents, routerrpc.HtlcEvent_SEND, + lntest.HtlcEventSettle, + ) + ht.AssertHtlcEventTypes( + carolEvents, routerrpc.HtlcEvent_RECEIVE, + lntest.HtlcEventSettle, + ) + ht.AssertHtlcEventTypes( + carolEvents, routerrpc.HtlcEvent_UNKNOWN, + lntest.HtlcEventFinal, + ) // At this point, Alice has 50mil satoshis on her side of the channel, // but Bob only has 10k available on his side of the channel. So a @@ -347,7 +332,7 @@ func testHtlcErrorPropagation(ht *lntest.HarnessTest) { // Reset mission control to forget the temporary channel failure above. alice.RPC.ResetMissionControl() - req := &routerrpc.SendPaymentRequest{ + req = &routerrpc.SendPaymentRequest{ PaymentRequest: carolInvoice.PaymentRequest, TimeoutSeconds: 60, FeeLimitMsat: noFeeLimitMsat, diff --git a/itest/lnd_send_multi_path_payment_test.go b/itest/lnd_send_multi_path_payment_test.go index 143cf496d..10ad3f4a2 100644 --- a/itest/lnd_send_multi_path_payment_test.go +++ b/itest/lnd_send_multi_path_payment_test.go @@ -28,12 +28,12 @@ func testSendMultiPathPayment(ht *lntest.HarnessTest) { // \__ Dave ____/ // req := &mppOpenChannelRequest{ - amtAliceCarol: 235000, - amtAliceDave: 135000, - amtCarolBob: 135000, - amtCarolEve: 135000, - amtDaveBob: 135000, - amtEveBob: 135000, + amtAliceCarol: 285000, + amtAliceDave: 155000, + amtCarolBob: 200000, + amtCarolEve: 155000, + amtDaveBob: 155000, + amtEveBob: 155000, } mts.openChannels(req) chanPointAliceDave := mts.channelPoints[1] diff --git a/lntest/utils.go b/lntest/utils.go index 5ac6c355e..660c42ef9 100644 --- a/lntest/utils.go +++ b/lntest/utils.go @@ -226,3 +226,49 @@ func CalculateMaxHtlc(chanCap btcutil.Amount) uint64 { return uint64(max) } + +// CalcStaticFeeBuffer calculates appropriate fee buffer which must be taken +// into account when sending htlcs. +func CalcStaticFeeBuffer(c lnrpc.CommitmentType, numHTLCs int) btcutil.Amount { + //nolint:lll + const ( + htlcWeight = input.HTLCWeight + defaultSatPerVByte = lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte + scale = 1000 + ) + + var ( + commitWeight = input.CommitWeight + feePerKw = chainfee.SatPerKWeight(DefaultFeeRateSatPerKw) + ) + + switch { + // The taproot commitment type has the extra anchor outputs, but also a + // smaller witness field (will just be a normal key spend), so we need + // to account for that here as well. + case CommitTypeHasTaproot(c): + feePerKw = chainfee.SatPerKVByte( + defaultSatPerVByte * scale, + ).FeePerKWeight() + + commitWeight = input.TaprootCommitWeight + + // The anchor commitment type is slightly heavier, and we must also add + // the value of the two anchors to the resulting fee the initiator + // pays. In addition the fee rate is capped at 10 sat/vbyte for anchor + // channels. + case CommitTypeHasAnchors(c): + feePerKw = chainfee.SatPerKVByte( + defaultSatPerVByte * scale, + ).FeePerKWeight() + commitWeight = input.AnchorCommitWeight + } + + // Account for the HTLC which will be required when sending an htlc. + numHTLCs++ + feeBuffer := lnwallet.CalcFeeBuffer( + feePerKw, int64(commitWeight+numHTLCs*htlcWeight), + ) + + return feeBuffer.ToSatoshis() +} diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 8bbeb8dd0..0aee6c331 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -71,6 +71,11 @@ var ( ErrBelowMinHTLC = fmt.Errorf("proposed HTLC value is below minimum " + "allowed HTLC value") + // ErrFeeBufferNotInitiator is returned when the FeeBuffer is enforced + // although the channel was not initiated (opened) locally. + ErrFeeBufferNotInitiator = fmt.Errorf("unable to enforce FeeBuffer, " + + "not initiator of the channel") + // ErrInvalidHTLCAmt signals that a proposed HTLC has a value that is // not positive. ErrInvalidHTLCAmt = fmt.Errorf("proposed HTLC value must be positive") @@ -3818,6 +3823,140 @@ func (lc *LightningChannel) getUnsignedAckedUpdates() []channeldb.LogUpdate { return logUpdates } +// CalcFeeBuffer calculates a FeeBuffer in accordance with the recommended +// amount specified in BOLT 02. It accounts for two times the current fee rate +// plus an additional htlc at this higher fee rate which allows our peer to add +// an htlc even if our channel is drained locally. +// See: https://github.com/lightning/bolts/blob/master/02-peer-protocol.md +func CalcFeeBuffer(feePerKw chainfee.SatPerKWeight, + commitWeight int64) lnwire.MilliSatoshi { + + // Account for a 100% in fee rate increase. + bufferFeePerKw := 2 * feePerKw + + feeBuffer := lnwire.NewMSatFromSatoshis( + // Account for an additional htlc at the higher fee level. + bufferFeePerKw.FeeForWeight(commitWeight + input.HTLCWeight), + ) + + return feeBuffer +} + +// BufferType is used to determine what kind of additional buffer should be left +// when evaluating the usable balance of a channel. +type BufferType uint8 + +const ( + // NoBuffer means no additional buffer is accounted for. This is + // important when verifying an already locked-in commitment state. + NoBuffer BufferType = iota + + // FeeBuffer accounts for several edge cases. One of them is where + // a locally drained channel might become unusable due to the non-opener + // of the channel not being able to add a non-dust htlc to the channel + // state because we as a channel opener cannot pay the additional fees + // an htlc would require on the commitment tx. + // See: https://github.com/lightningnetwork/lightning-rfc/issues/728 + // + // Moreover it mitigates the situation where htlcs are added + // simultaneously to the commitment transaction. This cannot be avoided + // until the feature __option_simplified_update__ is available in the + // protocol and deployed widely in the network. + // More information about the issue and the simplified commitment flow + // can be found here: + // https://github.com/lightningnetwork/lnd/issues/7657 + // https://github.com/lightning/bolts/pull/867 + // + // The last advantage is that we can react to fee spikes (up or down) + // by accounting for at least twice the size of the current fee rate + // (BOLT02). It also accounts for decreases in the fee rate because + // former dust htlcs might now become normal outputs so the overall + // fee might increase although the fee rate decreases (this is only true + // for non-anchor channels because htlcs have to account for their + // fee of the second-level covenant transactions). + FeeBuffer + + // AdditionalHtlc just accounts for an additional htlc which is helpful + // when deciding about a fee update of the commitment transaction. + // Leaving always room for an additional htlc makes sure that even + // though we are the opener of a channel a new fee update will always + // allow an htlc from our peer to be added to the commitment tx. + AdditionalHtlc +) + +// String returns a human readable name for the buffer type. +func (b BufferType) String() string { + switch b { + case NoBuffer: + return "nobuffer" + case FeeBuffer: + return "feebuffer" + case AdditionalHtlc: + return "additionalhtlc" + default: + return "unknown" + } +} + +// applyCommitFee applies the commitFee including a buffer to the balance amount +// and verifies that it does not become negative. This function returns the new +// balance and the exact buffer amount (excluding the commitment fee). +func (lc *LightningChannel) applyCommitFee( + balance lnwire.MilliSatoshi, commitWeight int64, + feePerKw chainfee.SatPerKWeight, + buffer BufferType) (lnwire.MilliSatoshi, lnwire.MilliSatoshi, error) { + + commitFee := feePerKw.FeeForWeight(commitWeight) + commitFeeMsat := lnwire.NewMSatFromSatoshis(commitFee) + + var bufferAmt lnwire.MilliSatoshi + switch buffer { + // The FeeBuffer is subtracted from the balance. It is of predefined + // size add keeps room for an up to 2x increase in fees of the + // commitment tx and an additional htlc at this fee level reserved for + // the peer. + case FeeBuffer: + // Make sure that we are the initiator of the channel before we + // apply the FeeBuffer. + if !lc.channelState.IsInitiator { + return 0, 0, ErrFeeBufferNotInitiator + } + + // The FeeBuffer already includes the commitFee. + bufferAmt = CalcFeeBuffer(feePerKw, commitWeight) + if bufferAmt < balance { + newBalance := balance - bufferAmt + return newBalance, bufferAmt - commitFeeMsat, nil + } + + // The AdditionalHtlc buffer type does NOT keep a FeeBuffer but solely + // keeps space for an additional htlc on the commitment tx which our + // peer can add. + case AdditionalHtlc: + additionalHtlcFee := lnwire.NewMSatFromSatoshis( + feePerKw.FeeForWeight(input.HTLCWeight), + ) + + bufferAmt = commitFeeMsat + additionalHtlcFee + newBalance := balance - bufferAmt + if bufferAmt < balance { + return newBalance, additionalHtlcFee, nil + } + + // The default case does not account for any buffer on the local balance + // but just subtracts the commit fee. + default: + if commitFeeMsat < balance { + newBalance := balance - commitFeeMsat + return newBalance, 0, nil + } + } + + // We still return the amount and bufferAmt here to log them at a later + // stage. + return balance, bufferAmt, ErrBelowChanReserve +} + // 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 @@ -3825,9 +3964,17 @@ func (lc *LightningChannel) getUnsignedAckedUpdates() []channeldb.LogUpdate { // 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, + ourLogCounter uint64, remoteChain bool, buffer BufferType, predictOurAdd, predictTheirAdd *PaymentDescriptor) error { + // First fetch the initial balance before applying any updates. + commitChain := lc.localCommitChain + if remoteChain { + commitChain = lc.remoteCommitChain + } + ourInitialBalance := commitChain.tip().ourBalance + theirInitialBalance := commitChain.tip().theirBalance + // Fetch all updates not committed. view := lc.fetchHTLCView(theirLogCounter, ourLogCounter) @@ -3841,45 +3988,15 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, view.theirUpdates = append(view.theirUpdates, predictTheirAdd) } - commitChain := lc.localCommitChain - if remoteChain { - commitChain = lc.remoteCommitChain - } - ourInitialBalance := commitChain.tip().ourBalance - theirInitialBalance := commitChain.tip().theirBalance - ourBalance, theirBalance, commitWeight, filteredView, err := lc.computeView( view, remoteChain, false, ) if err != nil { return err } + feePerKw := filteredView.feePerKw - // Calculate the commitment fee, and subtract it from the initiator's - // balance. - commitFee := feePerKw.FeeForWeight(commitWeight) - commitFeeMsat := lnwire.NewMSatFromSatoshis(commitFee) - if lc.channelState.IsInitiator { - ourBalance -= commitFeeMsat - } else { - theirBalance -= commitFeeMsat - } - - // As a quick sanity check, we'll ensure that if we interpret the - // balances as signed integers, they haven't dipped down below zero. If - // they have, then this indicates that a party doesn't have sufficient - // balance to satisfy the final evaluated HTLC's. - switch { - case int64(ourBalance) < 0: - return fmt.Errorf("%w: negative local balance", - ErrBelowChanReserve) - - case int64(theirBalance) < 0: - return fmt.Errorf("%w: negative remote balance", - ErrBelowChanReserve) - } - // Ensure that the fee being applied is enough to be relayed across the // network in a reasonable time frame. if feePerKw < chainfee.FeePerKwFloor { @@ -3887,6 +4004,51 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, feePerKw, chainfee.FeePerKwFloor) } + // The channel opener has to account for the commitment fee. This + // includes also a buffer type. Depending on whether we are the opener + // of the channel we either want to enforce a buffer on the local + // amount. + var bufferAmt lnwire.MilliSatoshi + if lc.channelState.IsInitiator { + ourBalance, bufferAmt, err = lc.applyCommitFee( + ourBalance, commitWeight, feePerKw, buffer) + if err != nil { + commitFee := feePerKw.FeeForWeight(commitWeight) + lc.log.Errorf("Cannot pay for the CommitmentFee of "+ + "the ChannelState: ourBalance is negative "+ + "after applying the fee: ourBalance=%v, "+ + "commitFee=%v, feeBuffer=%v (type=%v) "+ + "local_chan_initiator", int64(ourBalance), + commitFee, bufferAmt, buffer) + + return err + } + } else { + // No FeeBuffer is enforced when we are not the initiator of + // the channel. We cannot do this, because if our peer does not + // enforce the FeeBuffer (older LND software) the peer might + // bring his balance below the FeeBuffer making the channel + // stuck because locally we will never put another outgoing HTLC + // on the channel state. The FeeBuffer should ONLY be enforced + // if we locally pay for the commitment transaction. + theirBalance, bufferAmt, err = lc.applyCommitFee( + theirBalance, commitWeight, feePerKw, NoBuffer) + if err != nil { + commitFee := feePerKw.FeeForWeight(commitWeight) + lc.log.Errorf("Cannot pay for the CommitmentFee "+ + "of the ChannelState: theirBalance is "+ + "negative after applying the fee: "+ + "theiBalance=%v, commitFee=%v, feeBuffer=%v "+ + "(type=%v) remote_chan_initiator", + int64(theirBalance), commitFee, bufferAmt, + buffer) + + return err + } + } + + // The commitment fee was accounted for successfully now make sure we + // still do have enough left to account for the channel reserve. // If the added HTLCs will decrease the balance, make sure they won't // dip the local and remote balances below the channel reserves. ourReserve := lnwire.NewMSatFromSatoshis( @@ -3896,16 +4058,32 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, lc.channelState.RemoteChanCfg.ChanReserve, ) + // Calculate the commitment fee to log the information if needed. + commitFee := feePerKw.FeeForWeight(commitWeight) + commitFeeMsat := lnwire.NewMSatFromSatoshis(commitFee) + switch { + // TODO(ziggie): Allow the peer dip us below the channel reserve when + // our local balance would increase during this commitment dance or + // allow us to dip the peer below its reserve then their balance would + // increase during this commitment dance. This is needed for splicing + // when e.g. a new channel (bigger capacity) has a higher required + // reserve and the peer would need to add an additional htlc to push the + // missing amount to our side and viceversa. + // See: https://github.com/lightningnetwork/lnd/issues/8249 case ourBalance < ourInitialBalance && ourBalance < ourReserve: lc.log.Debugf("Funds below chan reserve: ourBalance=%v, "+ - "ourReserve=%v", ourBalance, ourReserve) + "ourReserve=%v, commitFee=%v, feeBuffer=%v "+ + "chan_initiator=%v", ourBalance, ourReserve, + commitFeeMsat, bufferAmt, lc.channelState.IsInitiator) + return fmt.Errorf("%w: our balance below chan reserve", ErrBelowChanReserve) case theirBalance < theirInitialBalance && theirBalance < theirReserve: lc.log.Debugf("Funds below chan reserve: theirBalance=%v, "+ "theirReserve=%v", theirBalance, theirReserve) + return fmt.Errorf("%w: their balance below chan reserve", ErrBelowChanReserve) } @@ -4056,8 +4234,12 @@ func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) { // ensure that we aren't violating any of the constraints the remote // party set up when we initially set up the channel. If we are, then // we'll abort this state transition. + // We do not enforce the FeeBuffer here because when we reach this + // point all updates will have to get locked-in so we enforce the + // minimum requirement. err := lc.validateCommitmentSanity( - remoteACKedIndex, lc.localUpdateLog.logIndex, true, nil, nil, + remoteACKedIndex, lc.localUpdateLog.logIndex, true, NoBuffer, + nil, nil, ) if err != nil { return nil, err @@ -4619,6 +4801,19 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, } feePerKw := filteredHTLCView.feePerKw + // We need to first check ourBalance and theirBalance to be negative + // because MilliSathoshi is a unsigned type and can underflow in + // `evaluateHTLCView`. This should never happen for views which do not + // include new updates (remote or local). + if int64(ourBalance) < 0 { + err := fmt.Errorf("%w: our balance", ErrBelowChanReserve) + return 0, 0, 0, nil, err + } + if int64(theirBalance) < 0 { + err := fmt.Errorf("%w: their balance", ErrBelowChanReserve) + return 0, 0, 0, nil, err + } + // Now go through all HTLCs at this stage, to calculate the total // weight, needed to calculate the transaction fee. var totalHtlcWeight int64 @@ -4988,8 +5183,14 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSigs *CommitSigs) error { // Ensure that this new local update from the remote node respects all // the constraints we specified during initial channel setup. If not, // then we'll abort the channel as they've violated our constraints. + // + // We do not enforce the FeeBuffer here because when we reach this + // point all updates will have to get locked-in (we already received + // the UpdateAddHTLC msg from our peer prior to receiving the + // commit-sig). err := lc.validateCommitmentSanity( - lc.remoteUpdateLog.logIndex, localACKedIndex, false, nil, nil, + lc.remoteUpdateLog.logIndex, localACKedIndex, false, NoBuffer, + nil, nil, ) if err != nil { return err @@ -5722,27 +5923,37 @@ func (lc *LightningChannel) InitNextRevocation(revKey *btcec.PublicKey) error { return lc.channelState.InsertNextRevocation(revKey) } -// AddHTLC adds an HTLC to the state machine's local update log. This method -// should be called when preparing to send an outgoing HTLC. +// AddHTLC is a wrapper of the `addHTLC` function which always enforces the +// FeeBuffer on the local balance if being the initiator of the channel. This +// method should be called when preparing to send an outgoing HTLC. // // 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 *models.CircuitKey) (uint64, error) { + return lc.addHTLC(htlc, openKey, FeeBuffer) +} + +// addHTLC adds an HTLC to the state machine's local update log. It provides +// the ability to enforce a buffer on the local balance when we are the +// initiator of the channel. This is useful when checking the edge cases of a +// channel state e.g. the BOLT 03 test vectors. +// +// The additional openKey argument corresponds to the incoming CircuitKey of the +// committed circuit for this HTLC. This value should never be nil. +// +// NOTE: It is okay for sourceRef to be nil when unit testing the wallet. +func (lc *LightningChannel) addHTLC(htlc *lnwire.UpdateAddHTLC, + openKey *models.CircuitKey, buffer BufferType) (uint64, error) { + lc.Lock() defer lc.Unlock() pd := lc.htlcAddDescriptor(htlc, openKey) - if err := lc.validateAddHtlc(pd); err != nil { + if err := lc.validateAddHtlc(pd, buffer); err != nil { return 0, err } @@ -5852,7 +6063,9 @@ func (lc *LightningChannel) MayAddOutgoingHtlc(amt lnwire.MilliSatoshi) error { &models.CircuitKey{}, ) - if err := lc.validateAddHtlc(pd); err != nil { + // Enforce the FeeBuffer because we are evaluating whether we can add + // another htlc to the channel state. + if err := lc.validateAddHtlc(pd, FeeBuffer); err != nil { lc.log.Debugf("May add outgoing htlc rejected: %v", err) return err } @@ -5879,7 +6092,8 @@ func (lc *LightningChannel) htlcAddDescriptor(htlc *lnwire.UpdateAddHTLC, // validateAddHtlc validates the addition of an outgoing htlc to our local and // remote commitments. -func (lc *LightningChannel) validateAddHtlc(pd *PaymentDescriptor) error { +func (lc *LightningChannel) validateAddHtlc(pd *PaymentDescriptor, + buffer BufferType) error { // Make sure adding this HTLC won't violate any of the constraints we // must keep on the commitment transactions. remoteACKedIndex := lc.localCommitChain.tail().theirMessageIndex @@ -5887,7 +6101,8 @@ func (lc *LightningChannel) validateAddHtlc(pd *PaymentDescriptor) error { // 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, nil, + remoteACKedIndex, lc.localUpdateLog.logIndex, true, + buffer, pd, nil, ) if err != nil { return err @@ -5900,7 +6115,7 @@ func (lc *LightningChannel) validateAddHtlc(pd *PaymentDescriptor) error { // possible for us to add the HTLC. err = lc.validateCommitmentSanity( lc.remoteUpdateLog.logIndex, lc.localUpdateLog.logIndex, - false, pd, nil, + false, buffer, pd, nil, ) if err != nil { return err @@ -5935,8 +6150,13 @@ 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. + // We do not enforce the FeeBuffer here because one of the reasons it + // was introduced is to protect against asynchronous sending of htlcs so + // we use it here. The current lightning protocol does not allow to + // reject ADDs already sent by the peer. err := lc.validateCommitmentSanity( - lc.remoteUpdateLog.logIndex, localACKedIndex, false, nil, pd, + lc.remoteUpdateLog.logIndex, localACKedIndex, false, NoBuffer, + nil, pd, ) if err != nil { return 0, err @@ -7987,15 +8207,15 @@ func NewAnchorResolution(chanState *channeldb.OpenChannel, // 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. +// account the fee that must be paid for adding this HTLC, that we cannot spend +// from the channel reserve and moreover the FeeBuffer when we are the +// initiator of the channel. 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() - bal, _ := lc.availableBalance() + bal, _ := lc.availableBalance(FeeBuffer) return bal } @@ -8003,7 +8223,9 @@ func (lc *LightningChannel) AvailableBalance() lnwire.MilliSatoshi { // This method is provided so methods that already hold the lock can access // this method. Additionally, the total weight of the next to be created // commitment is returned for accounting purposes. -func (lc *LightningChannel) availableBalance() (lnwire.MilliSatoshi, int64) { +func (lc *LightningChannel) availableBalance( + buffer BufferType) (lnwire.MilliSatoshi, int64) { + // We'll grab the current set of log updates that the remote has // ACKed. remoteACKedIndex := lc.localCommitChain.tip().theirMessageIndex @@ -8018,12 +8240,12 @@ func (lc *LightningChannel) availableBalance() (lnwire.MilliSatoshi, int64) { // 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, + htlcView, false, buffer, ) // Do the same calculation from the remote commitment point of view. ourRemoteCommitBalance, _ := lc.availableCommitmentBalance( - htlcView, true, + htlcView, true, buffer, ) // Return which ever balance is lowest. @@ -8042,7 +8264,7 @@ func (lc *LightningChannel) availableBalance() (lnwire.MilliSatoshi, int64) { // 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) { + remoteChain bool, buffer BufferType) (lnwire.MilliSatoshi, int64) { // Compute the current balances for this commitment. This will take // into account HTLCs to determine the commit weight, which the @@ -8070,26 +8292,42 @@ func (lc *LightningChannel) availableCommitmentBalance(view *htlcView, // 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), + additionalHtlcFee := lnwire.NewMSatFromSatoshis( + feePerKw.FeeForWeight(input.HTLCWeight), ) + commitFee := lnwire.NewMSatFromSatoshis( + feePerKw.FeeForWeight(commitWeight)) - // 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 { - // 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 where neither we nor our channel counterparty can - // add HTLCs. - if ourBalance < htlcCommitFee { + // When the buffer is of type `FeeBuffer` type we know we are + // going to send or forward an htlc over this channel therefore + // we account for an additional htlc output on the commitment + // tx. + futureCommitWeight := commitWeight + if buffer == FeeBuffer { + futureCommitWeight += input.HTLCWeight + } + + // Make sure we do not overwrite `ourBalance` that's why we + // declare bufferAmt beforehand. + var bufferAmt lnwire.MilliSatoshi + ourBalance, bufferAmt, err = lc.applyCommitFee( + ourBalance, futureCommitWeight, feePerKw, buffer, + ) + if err != nil { + lc.log.Warnf("Set available amount to 0 because we "+ + "could not pay for the CommitmentFee of the "+ + "new ChannelState: ourBalance is negative "+ + "after applying the fee: ourBalance=%v, "+ + "current commitFee(w/o additional htlc)=%v, "+ + "feeBuffer=%v (type=%v) local_chan_initiator", + int64(ourBalance), commitFee, + bufferAmt, buffer) + return 0, commitWeight } - return ourBalance - htlcCommitFee, commitWeight + return ourBalance, commitWeight } // If we're not the initiator, we must check whether the remote has @@ -8131,17 +8369,21 @@ func (lc *LightningChannel) availableCommitmentBalance(view *htlcView, // is non-dust after paying the HTLC fee. nonDustHtlcAmt := dustlimit + htlcFee + // commitFeeWithHtlc is the fee our peer has to pay in case we add + // another htlc to the commitment. + commitFeeWithHtlc := commitFee + additionalHtlcFee + // 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 { + if theirBalance < commitFeeWithHtlc && ourBalance >= nonDustHtlcAmt { // see https://github.com/lightning/bolts/issues/728 ourReportedBalance := nonDustHtlcAmt - 1 lc.log.Infof("Reducing local balance (from %v to %v): "+ "remote side does not have enough funds (%v < %v) to "+ "pay for non-dust HTLC in case of unilateral close.", ourBalance, ourReportedBalance, theirBalance, - htlcCommitFee) + commitFeeWithHtlc) ourBalance = ourReportedBalance } @@ -8164,7 +8406,9 @@ func (lc *LightningChannel) validateFeeRate(feePerKw chainfee.SatPerKWeight) err // We'll ensure that we can accommodate this new fee change, yet still // be above our reserve balance. Otherwise, we'll reject the fee // update. - availableBalance, txWeight := lc.availableBalance() + // We do not enforce the FeeBuffer here because it was exactly + // introduced to use this buffer for potential fee rate increases. + availableBalance, txWeight := lc.availableBalance(AdditionalHtlc) oldFee := lnwire.NewMSatFromSatoshis( lc.localCommitChain.tip().feePerKw.FeeForWeight(txWeight), @@ -8441,8 +8685,9 @@ func (lc *LightningChannel) MaxFeeRate(maxAllocation float64) chainfee.SatPerKWe // The maximum fee depends on the available balance that can be // committed towards fees. It takes into account our local reserve - // balance. - availableBalance, weight := lc.availableBalance() + // balance. We do not account for a FeeBuffer here because that is + // exactly why it was introduced to react for sharp fee changes. + availableBalance, weight := lc.availableBalance(AdditionalHtlc) oldFee := lc.localCommitChain.tip().feePerKw.FeeForWeight(weight) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 80a3df31c..db5d0f468 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -1656,7 +1656,10 @@ func TestChannelBalanceDustLimit(t *testing.T) { htlcAmount := lnwire.NewMSatFromSatoshis(htlcSat) htlc, preimage := createHTLC(0, htlcAmount) - aliceHtlcIndex, err := aliceChannel.AddHTLC(htlc, nil) + + // We need to use `addHTLC` instead of `AddHTLC` so that we do not + // enforce the FeeBuffer on alice side. + aliceHtlcIndex, err := aliceChannel.addHTLC(htlc, nil, NoBuffer) require.NoError(t, err, "alice unable to add htlc") bobHtlcIndex, err := bobChannel.ReceiveHTLC(htlc) require.NoError(t, err, "bob unable to receive htlc") @@ -4783,6 +4786,8 @@ func TestChanSyncInvalidLastSecret(t *testing.T) { func TestChanAvailableBandwidth(t *testing.T) { t.Parallel() + cType := channeldb.SingleFunderTweaklessBit + // 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. @@ -4797,11 +4802,10 @@ func TestChanAvailableBandwidth(t *testing.T) { feeRate := chainfee.SatPerKWeight( aliceChannel.channelState.LocalCommitment.FeePerKw, ) - htlcFee := lnwire.NewMSatFromSatoshis( - feeRate.FeeForWeight(input.HTLCWeight), - ) - assertBandwidthEstimateCorrect := func(aliceInitiate bool) { + assertBandwidthEstimateCorrect := func(aliceInitiate bool, + numNonDustHtlcsOnCommit int64) { + // With the HTLC's added, we'll now query the AvailableBalance // method for the current available channel bandwidth from // Alice's PoV. @@ -4826,12 +4830,23 @@ 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 + aliceState := aliceChannel.State().Snapshot() + aliceBalance := aliceState.LocalBalance + commitFee := lnwire.NewMSatFromSatoshis(aliceState.CommitFee) + commitWeight := CommitWeight(cType) + commitWeight += numNonDustHtlcsOnCommit * input.HTLCWeight + + // Add weight for an additional htlc because this is also done + // when evaluating the current balance. + feeBuffer := CalcFeeBuffer( + feeRate, commitWeight+input.HTLCWeight, + ) // 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 + // and the fee buffer we have to account for. + expBalance := aliceBalance + commitFee - aliceReserve - + feeBuffer if expBalance != aliceAvailableBalance { _, _, line, _ := runtime.Caller(1) t.Fatalf("line: %v, incorrect balance: expected %v, "+ @@ -4841,10 +4856,10 @@ func TestChanAvailableBandwidth(t *testing.T) { // First, we'll add 3 outgoing HTLC's from Alice to Bob. const numHtlcs = 3 - var htlcAmt lnwire.MilliSatoshi = 100000 + var dustAmt lnwire.MilliSatoshi = 100000 alicePreimages := make([][32]byte, numHtlcs) for i := 0; i < numHtlcs; i++ { - htlc, preImage := createHTLC(i, htlcAmt) + htlc, preImage := createHTLC(i, dustAmt) if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { t.Fatalf("unable to add htlc: %v", err) } @@ -4855,12 +4870,12 @@ func TestChanAvailableBandwidth(t *testing.T) { alicePreimages[i] = preImage } - assertBandwidthEstimateCorrect(true) + assertBandwidthEstimateCorrect(true, 0) // We'll repeat the same exercise, but with non-dust HTLCs. So we'll // crank up the value of the HTLC's we're adding to the commitment // transaction. - htlcAmt = lnwire.NewMSatFromSatoshis(30000) + htlcAmt := lnwire.NewMSatFromSatoshis(30000) for i := 0; i < numHtlcs; i++ { htlc, preImage := createHTLC(numHtlcs+i, htlcAmt) if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { @@ -4873,10 +4888,10 @@ func TestChanAvailableBandwidth(t *testing.T) { alicePreimages = append(alicePreimages, preImage) } - assertBandwidthEstimateCorrect(true) + assertBandwidthEstimateCorrect(true, 3) - // Next, we'll have Bob 5 of Alice's HTLC's, and cancel one of them (in - // the update log). + // Next, we'll have Bob settle 5 of Alice's HTLC's, and cancel one of + // them (in the update log). for i := 0; i < (numHtlcs*2)-1; i++ { preImage := alicePreimages[i] err := bobChannel.SettleHTLC(preImage, uint64(i), nil, nil, nil) @@ -4904,21 +4919,22 @@ func TestChanAvailableBandwidth(t *testing.T) { // 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) + assertBandwidthEstimateCorrect(true, 0) // 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. +// balance when it is close to the fee buffer. 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. + cType := channeldb.SingleFunderTweaklessBit aliceChannel, bobChannel, err := CreateTestChannels( - t, channeldb.SingleFunderTweaklessBit, + t, cType, ) require.NoError(t, err, "unable to create test channels") @@ -4939,6 +4955,13 @@ func TestChanAvailableBalanceNearHtlcFee(t *testing.T) { feeRate := chainfee.SatPerKWeight( aliceChannel.channelState.LocalCommitment.FeePerKw, ) + + // When calculating the fee buffer sending an htlc we need to account + // for an additional output on the commitment tx which this send will + // generate. + commitWeight := CommitWeight(cType) + feeBuffer := CalcFeeBuffer(feeRate, commitWeight+input.HTLCWeight) + htlcFee := lnwire.NewMSatFromSatoshis( feeRate.FeeForWeight(input.HTLCWeight), ) @@ -4977,7 +5000,12 @@ func TestChanAvailableBalanceNearHtlcFee(t *testing.T) { t.Helper() htlc, preImage := createHTLC(int(htlcIndex), htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { + + // For this testcase we don't want to enforce the FeeBuffer + // so that we can verify the edge cases when our balance reaches + // the channel reserve limit. + _, err := aliceChannel.addHTLC(htlc, nil, NoBuffer) + if err != nil { t.Fatalf("unable to add htlc: %v", err) } if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { @@ -5009,11 +5037,9 @@ func TestChanAvailableBalanceNearHtlcFee(t *testing.T) { } // 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 + // the reserve and the fee buffer because alice is the initiator of + // the channel. + expAliceBalance := aliceBalance - aliceReserve - feeBuffer // 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 @@ -5026,7 +5052,7 @@ func TestChanAvailableBalanceNearHtlcFee(t *testing.T) { // 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) + htlcAmt := aliceBalance - (aliceReserve + feeBuffer + aliceNonDustHtlc) sendHtlc(htlcAmt) // Now the real balance left will be @@ -5037,7 +5063,7 @@ func TestChanAvailableBalanceNearHtlcFee(t *testing.T) { expBobBalance = bobBalance - bobReserve checkBalance(t, expAliceBalance, expBobBalance) - // Send an HTLC using all but one msat of the reported balance. + // Send an dust HTLC using all but one msat of the reported balance. htlcAmt = aliceNonDustHtlc - 1 sendHtlc(htlcAmt) @@ -5063,12 +5089,15 @@ func TestChanAvailableBalanceNearHtlcFee(t *testing.T) { 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. + // The available balance is zero for alice but there is still the + // fee buffer left (which includes the current commitment weight). + // We send the buffer to bob but also keep the funds for the htlc + // available otherwise we would not able to send this amount. + htlcAmt = feeBuffer - commitFee - htlcFee + sendHtlc(htlcAmt) + + // Now we send a dust htlc of 1 msat to bob so that we cannot afford + // to put another non-dust htlc on this commitment. htlcAmt = 1 sendHtlc(htlcAmt) @@ -5159,13 +5188,15 @@ func TestChanCommitWeightDustHtlcs(t *testing.T) { // Helper method that fetches the current remote commitment weight // fromt the given channel's POV. + // When sending htlcs we enforce the feebuffer on the commitment + // transaction. remoteCommitWeight := func(lc *LightningChannel) int64 { remoteACKedIndex := lc.localCommitChain.tip().theirMessageIndex htlcView := lc.fetchHTLCView(remoteACKedIndex, lc.localUpdateLog.logIndex) _, w := lc.availableCommitmentBalance( - htlcView, true, + htlcView, true, FeeBuffer, ) return w @@ -10209,3 +10240,567 @@ func createRandomHTLC(t *testing.T, incoming bool) channeldb.HTLC { ExtraData: extra, } } + +// TestApplyCommitmentFee tests that depending on the buffer type the correct +// commitment fee is calculated which includes the buffer amount which will be +// kept and is not usable hence not considered part of the usable local balance. +func TestApplyCommitmentFee(t *testing.T) { + var ( + balance = lnwire.NewMSatFromSatoshis(5_000_000) + + // balance used to test the case where the commitment fee + // including the buffer is greater than the balance. + balanceBelowReserve = lnwire.NewMSatFromSatoshis(5_000) + + // commitment weight with an additional htlc. + commitWeight int64 = input.BaseAnchorCommitmentTxWeight + + input.HTLCWeight + + // fee rate of 10 sat/vbyte. + feePerKw = chainfee.SatPerKWeight(2500) + commitFee = lnwire.NewMSatFromSatoshis( + feePerKw.FeeForWeight(commitWeight), + ) + additionalHtlc = lnwire.NewMSatFromSatoshis( + feePerKw.FeeForWeight(input.HTLCWeight), + ) + feeBuffer = CalcFeeBuffer(feePerKw, commitWeight) + ) + + // Create test channels so that we can use the `applyCommitFee` + // function. + aliceChannel, bobChannel, err := CreateTestChannels( + t, channeldb.SingleFunderTweaklessBit, + ) + require.NoError(t, err) + + testCases := []struct { + name string + channel *LightningChannel + buffer BufferType + balance lnwire.MilliSatoshi + expectedBalance lnwire.MilliSatoshi + expectedBufferAmt lnwire.MilliSatoshi + bufferAmt lnwire.MilliSatoshi + expectedErr error + }{ + { + name: "apply feebuffer local initiator", + channel: aliceChannel, + buffer: FeeBuffer, + balance: balance, + expectedBalance: balance - feeBuffer, + expectedBufferAmt: feeBuffer - commitFee, + }, + { + name: "apply feebuffer remote initiator", + channel: bobChannel, + buffer: FeeBuffer, + balance: balance, + expectedErr: ErrFeeBufferNotInitiator, + }, + { + name: "apply AdditionalHtlc buffer", + channel: aliceChannel, + buffer: AdditionalHtlc, + balance: balance, + expectedBalance: balance - commitFee - additionalHtlc, + expectedBufferAmt: additionalHtlc, + }, + { + name: "apply NoBuffer", + channel: aliceChannel, + buffer: NoBuffer, + balance: balance, + expectedBalance: balance - commitFee, + expectedBufferAmt: 0, + }, + { + name: "apply FeeBuffer balance negative", + channel: aliceChannel, + buffer: FeeBuffer, + balance: balanceBelowReserve, + expectedBalance: balanceBelowReserve, + expectedErr: ErrBelowChanReserve, + expectedBufferAmt: feeBuffer, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + balance, bufferAmt, err := tc.channel.applyCommitFee( + tc.balance, commitWeight, feePerKw, tc.buffer) + + require.ErrorIs(t, err, tc.expectedErr) + require.Equal(t, tc.expectedBalance, balance) + require.Equal(t, tc.expectedBufferAmt, bufferAmt) + }) + } +} + +// TestAsynchronousSendingContraint tests that when both peers add htlcs to +// their commitment asynchronously and the channel opener does not account for +// an additional buffer locally an unusable channel state can be the worst case +// consequence when the channel is locally drained. +// NOTE: This edge case can only be solved at the protocol level because +// currently both parties can add htlcs to their commitments in simultaneously +// which can lead to a situation where the channel opener cannot pay the fees +// for the additional htlc outputs which were added in parallel. A solution for +// this can either be a fee buffer or a new protocol improvement called +// __option_simplified_update__. +// +// The full state transition of this test is: +// The vertical mark in the middle is the connection which separates alice and +// bob. When a line only goes to this mark it means the signal was only added +// to one side of the channel parties. +// +// +// Alice Bob +// | +// -----add------> | +// |<----add------- +// Add htlcs asynchronously. +// <----add------- | +// |-----add------> +// <----sig------- |--------------- +// -----rev------> | +// -----sig------> | +// (Alice fails with ErrBelowChanReserve) + +func TestAsynchronousSendingContraint(t *testing.T) { + t.Parallel() + + // Create test channels to test out this state transition. The channel + // capactiy is 10 BTC with every side having 5 BTC at start. The fee + // rate is static and 6000 sats/kw. + aliceChannel, bobChannel, err := CreateTestChannels( + t, channeldb.SingleFunderTweaklessBit, + ) + require.NoError(t, err) + + aliceReserve := aliceChannel.channelState.LocalChanCfg.ChanReserve + + capacity := aliceChannel.channelState.Capacity + + // Static fee rate of 6000 sats/kw. + feePerKw := chainfee.SatPerKWeight( + aliceChannel.channelState.LocalCommitment.FeePerKw, + ) + + additionalHtlc := feePerKw.FeeForWeight(input.HTLCWeight) + commitFee := feePerKw.FeeForWeight(input.CommitWeight) + + // We add an htlc to alice commitment with the amount so that everything + // will be used up and the remote party cannot add another htlc because + // alice (the opener of the channel) will not be able to afford the + // additional onchain cost of the htlc output on the commitment tx. + htlcAmount := capacity/2 - aliceReserve - commitFee - additionalHtlc + + // Create an HTLC that alice will send to Bob which let's alice use up + // all its local funds. + // -----add------>| + htlc1, _ := createHTLC(0, lnwire.NewMSatFromSatoshis(htlcAmount)) + _, err = aliceChannel.addHTLC(htlc1, nil, NoBuffer) + require.NoError(t, err) + + // Before making bob aware of this new htlc, let bob add an HTLC on the + // commitment as well. Because bob does not know yet about the htlc + // alice is going to add to the state his adding will succeed as well. + + // |<----add------- + // make sure this htlc is non-dust for alice. + htlcFee := HtlcSuccessFee(channeldb.SingleFunderTweaklessBit, feePerKw) + // We need to take the remote dustlimit amount, because it the greater + // one. + htlcAmt2 := lnwire.NewMSatFromSatoshis( + aliceChannel.channelState.RemoteChanCfg.DustLimit + htlcFee, + ) + htlc2, _ := createHTLC(0, htlcAmt2) + + // We could also use AddHTLC here because bob is not the initiator but + // we stay consistent in this test and use addHTLC instead. + _, err = bobChannel.addHTLC(htlc2, nil, NoBuffer) + require.NoError(t, err) + + // Now lets both channel parties know about these new htlcs. + // <----add-------| + // |-----add------> + _, err = aliceChannel.ReceiveHTLC(htlc2) + require.NoError(t, err) + _, err = bobChannel.ReceiveHTLC(htlc1) + require.NoError(t, err) + + // Bob signs the new state for alice, which ONLY has his htlc on it + // because he only includes acked updates of alice. + // <----sig-------|--------------- + bobNewCommit, err := bobChannel.SignNextCommitment() + require.NoError(t, err) + + err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) + require.NoError(t, err) + + // Alice revokes her local commitment which will lead her to include + // Bobs htlc into the commitment when signing the new state for bob. + // -----rev------>| + _, _, _, err = aliceChannel.RevokeCurrentCommitment() + require.NoError(t, err) + + // Because alice revoked her local commitment she will now include bob's + // incoming htlc in her commitment sig to bob, but this will dip her + // local balance below her reserve because she already used everything + // up when adding her htlc. + _, err = aliceChannel.SignNextCommitment() + require.ErrorIs(t, err, ErrBelowChanReserve) +} + +// TestAsynchronousSendingWithFeeBuffer tests that in case of asynchronous +// adding of htlcs to the channel state a fee buffer will prevent the channel +// from becoming unusable because the channel opener will always keep an +// additional buffer to account either for fee updates or for the asynchronous +// adding of htlcs from both parties. +// The full state transition of this test is: +// The vertical mark in the middle is the connection which separates alice and +// bob. When a line only goes to this mark it means the signal was only added +// to one side of the channel parties. +// +// Alice Bob +// (keeps a feeBuffer) +// +// | +// -----add------> | +// |<----add------- +// | +// <----add------- | +// |-----add------> +// --------------- |-----sig------> +// <----rev------- |--------------- +// <----sig------- |--------------- +// --------------- |-----rev------> +// alice's htlc is locked-in bob +// --------------- |-----sig------> +// <----rev------- |--------------- +// bob's htlc is locked-in for alice +// --------------- |-----fail-----> +// --------------- |-----sig------> +// <----rev------- |--------------- +// <----sig------- |--------------- +// --------------- |-----rev------> +// bob's htlc is failed now. +// use the fee buffer to increase +// the fee of the commitment tx: +// --------------- |----updFee----> +// --------------- |-----sig------> +// <----rev------- |--------------- +// <----sig------- |--------------- +// --------------- |-----rev------> +// let bob add another htlc: +// <----add------- |<-------------- +// <----sig------- |--------------- +// --------------- |-----rev------> +// --------------- |-----sig------> +// <----rev------- |--------------- + +func TestAsynchronousSendingWithFeeBuffer(t *testing.T) { + t.Parallel() + + // Create test channels to test out this state transition. The channel + // capactiy is 10 BTC with every side having 5 BTC at start. The fee + // rate is static and 6000 sats/kw. + aliceChannel, bobChannel, err := CreateTestChannels( + t, channeldb.SingleFunderTweaklessBit, + ) + require.NoError(t, err) + + aliceReserve := aliceChannel.channelState.LocalChanCfg.ChanReserve + + capacity := aliceChannel.channelState.Capacity + + // Static fee rate of 6000 sats/kw. + feePerKw := chainfee.SatPerKWeight( + aliceChannel.channelState.LocalCommitment.FeePerKw, + ) + + // Calculate the fee buffer for the current commitment tx including + // the htlc we are going to add to alice's commitment tx. + feeBuffer := CalcFeeBuffer( + feePerKw, input.CommitWeight+input.HTLCWeight, + ) + + htlcAmount := capacity/2 - aliceReserve - feeBuffer.ToSatoshis() + + // Create an HTLC that alice will send to bob which uses all the local + // balance except the fee buffer (including the commitment fee) and the + // channel reserve. + htlc1, _ := createHTLC(0, lnwire.NewMSatFromSatoshis(htlcAmount)) + + // Add this HTLC only to alice channel for now only including a fee + // buffer. + // -----add------>| + _, err = aliceChannel.AddHTLC(htlc1, nil) + require.NoError(t, err) + + // Before making bob aware of this new htlc, let bob add an HTLC on the + // commitment as well. + // |<----add------- + // make sure this htlc is non-dust for alice. + htlcFee := HtlcSuccessFee(channeldb.SingleFunderTweaklessBit, feePerKw) + htlcAmt2 := lnwire.NewMSatFromSatoshis( + aliceChannel.channelState.LocalChanCfg.DustLimit + htlcFee, + ) + htlc2, _ := createHTLC(0, htlcAmt2) + _, err = bobChannel.AddHTLC(htlc2, nil) + require.NoError(t, err) + + // Now lets both channel parties know about these new htlcs. + // <----add------- | + // |-----add------> + _, err = aliceChannel.ReceiveHTLC(htlc2) + require.NoError(t, err) + _, err = bobChannel.ReceiveHTLC(htlc1) + require.NoError(t, err) + + // Now force the state transisiton. Both sides will succeed although + // we added htlcs asynchronously because we kept a buffer on alice side + // We start the state transition with alice. + // Force a state transition, this will lock-in the htlc of alice. + // -----sig-----> (includes alice htlc) + // <----rev------ + // <----sig------ (includes alice and bobs htlc) + // -----rev-----> (locks in alice's htlc for bob) + // bob's htlc is still not fully locked in. + err = ForceStateTransition(aliceChannel, bobChannel) + require.NoError(t, err) + + // Force a state transition, this will lock-in the htlc of bob. + // ------sig-----> (includes bob's htlc) + // <----rev------ (locks in bob's htlc for alice) + aliceNewCommit, err := aliceChannel.SignNextCommitment() + require.NoError(t, err) + err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) + require.NoError(t, err) + + bobRevocation, _, _, err := bobChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) + require.NoError(t, err) + + // Before testing the behavior of the fee buffer, we are going to fail + // back bob's htlc so that we only have 1 htlc on the commitment tx + // (alice htlc to bob) this makes it possible to exactly increase the + // fee of the commitment by 100%. + // --------------- |-----fail-----> + // --------------- |-----sig------> + // <----rev------- |--------------- + // <----sig------- |--------------- + // --------------- |-----rev------> + err = aliceChannel.FailHTLC(0, []byte{}, nil, nil, nil) + require.NoError(t, err) + + err = bobChannel.ReceiveFailHTLC(0, []byte{}) + require.NoError(t, err) + + err = ForceStateTransition(aliceChannel, bobChannel) + require.NoError(t, err) + + // Use the fee buffer to react to a potential fee rate increase and + // update the fee rate by 100%. + // --------------- |----updFee----> + // --------------- |-----sig------> + // <----rev------- |--------------- + // <----sig------- |--------------- + // --------------- |-----rev------> + err = aliceChannel.UpdateFee(feePerKw * 2) + require.NoError(t, err) + + err = bobChannel.ReceiveUpdateFee(feePerKw * 2) + require.NoError(t, err) + + err = ForceStateTransition(aliceChannel, bobChannel) + require.NoError(t, err) + + // Now let bob add an htlc to the commitment tx and make sure that + // despite the fee update bob can still add an htlc and alice still + // reserved funds for an additional htlc output on the commitment tx. + // <----add------- |<-------------- + // <----sig------- |--------------- + // --------------- |-----rev------> + // --------------- |-----sig------> + // <----rev------- |--------------- + // Update the non-dust amount because we updated the fee by 100%. + htlcFee = HtlcSuccessFee(channeldb.SingleFunderTweaklessBit, feePerKw*2) + htlcAmt3 := lnwire.NewMSatFromSatoshis( + aliceChannel.channelState.LocalChanCfg.DustLimit + htlcFee, + ) + htlc3, _ := createHTLC(1, htlcAmt3) + _, err = bobChannel.AddHTLC(htlc3, nil) + require.NoError(t, err) + + _, err = aliceChannel.ReceiveHTLC(htlc3) + require.NoError(t, err) + + err = ForceStateTransition(bobChannel, aliceChannel) + require.NoError(t, err) + + // Adding an HTLC from Alice side should not be possible because + // all funds are used up, even dust amounts. + htlc4, _ := createHTLC(2, 100) + // First try adding this htlc which should fail because the FeeBuffer + // cannot be kept while sending this HTLC. + _, err = aliceChannel.AddHTLC(htlc4, nil) + require.ErrorIs(t, err, ErrBelowChanReserve) + // Even without enforcing the FeeBuffer we used all of our usable funds + // on this channel and cannot even add a dust-htlc. + _, err = aliceChannel.addHTLC(htlc4, nil, NoBuffer) + require.ErrorIs(t, err, ErrBelowChanReserve) + + // All of alice's balance is used up in fees and htlcs so the local + // balance equals exactly the local reserve. + require.Equal(t, aliceChannel.channelState.LocalCommitment.LocalBalance, + lnwire.NewMSatFromSatoshis(aliceReserve)) +} + +// TestEnforceFeeBuffer tests that in case the channel initiator does NOT have +// enough local balance to pay for the FeeBuffer, adding new HTLCs by the +// initiator will fail. Receiving HTLCs will still work because no FeeBuffer is +// enforced when receiving HTLCs. +// +// The full state transition of this test is: +// The vertical mark in the middle is the connection which separates alice and +// bob. When a line only goes to this mark it means the signal was only added +// to one side of the channel parties. +// +// Alice Bob +// (keeps a feeBuffer) +// +// --------------- |-----add------> +// --------------- |-----sig------> +// <----rev------- |--------------- +// <----sig------- |--------------- +// --------------- |-----rev------> +// alice sends all usable funds to bob +// <----add------- |--------------- +// <----sig------- |--------------- +// --------------- |-----rev------> +// --------------- |-----sig------> +// <----rev------- |--------------- +// bob adds an HTLC to the channel +// -----add------> | +// adding another HTLC fails because +// the FeeBuffer cannot be paid. +// <----add------- |<-------------- +// <----sig------- |--------------- +// --------------- |-----rev------> +// --------------- |-----sig------> +// <----rev------- |--------------- +// bob adds another HTLC to the channel, +// alice can still pay the fee for the +// new commitment tx. +func TestEnforceFeeBuffer(t *testing.T) { + t.Parallel() + + // Create test channels to test out this state transition. The channel + // capactiy is 10 BTC with every side having 5 BTC at start. The fee + // rate is static and 6000 sats/kw. + aliceChannel, bobChannel, err := CreateTestChannels( + t, channeldb.SingleFunderTweaklessBit, + ) + require.NoError(t, err) + + aliceReserve := aliceChannel.channelState.LocalChanCfg.ChanReserve + + capacity := aliceChannel.channelState.Capacity + + // Static fee rate of 6000 sats/kw. + feePerKw := chainfee.SatPerKWeight( + aliceChannel.channelState.LocalCommitment.FeePerKw, + ) + + // Commitment Fee of the channel state (with 1 pending htlc). + commitFee := feePerKw.FeeForWeight( + input.CommitWeight + input.HTLCWeight, + ) + commitFeeMsat := lnwire.NewMSatFromSatoshis(commitFee) + + // Calculate the FeeBuffer for the current commitment tx (non-anchor) + // including the htlc alice is going to add to the commitment tx. + feeBuffer := CalcFeeBuffer( + feePerKw, input.CommitWeight+input.HTLCWeight, + ) + + // The bufferAmt is the FeeBuffer excluding the commitment fee. + bufferAmt := feeBuffer - commitFeeMsat + + htlcAmt1 := capacity/2 - aliceReserve - feeBuffer.ToSatoshis() + + // Create an HTLC that alice will send to bob which uses all the local + // balance except the fee buffer (including the commitment fee) and the + // channel reserve. + // --------------- |-----add------> + // --------------- |-----sig------> + // <----rev------- |--------------- + // <----sig------- |--------------- + // --------------- |-----rev------> + htlc1, _ := createHTLC(0, lnwire.NewMSatFromSatoshis(htlcAmt1)) + + _, err = aliceChannel.AddHTLC(htlc1, nil) + require.NoError(t, err) + _, err = bobChannel.ReceiveHTLC(htlc1) + require.NoError(t, err) + + err = ForceStateTransition(aliceChannel, bobChannel) + require.NoError(t, err) + + // Now bob sends a 1 btc htlc to alice. + // <----add------- |--------------- + // <----sig------- |--------------- + // --------------- |-----rev------> + // --------------- |-----sig------> + // <----rev------- |--------------- + htlcAmt2 := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcent) + htlc2, _ := createHTLC(0, htlcAmt2) + + _, err = bobChannel.AddHTLC(htlc2, nil) + require.NoError(t, err) + _, err = aliceChannel.ReceiveHTLC(htlc2) + require.NoError(t, err) + + err = ForceStateTransition(bobChannel, aliceChannel) + require.NoError(t, err) + + // Alice has the buffer amount left trying to send this amount will + // fail. + htlc3, _ := createHTLC(1, bufferAmt) + + _, err = aliceChannel.AddHTLC(htlc3, nil) + require.ErrorIs(t, err, ErrBelowChanReserve) + + // Now bob sends another 1 btc htlc to alice. + // <----add------- |--------------- + // <----sig------- |--------------- + // --------------- |-----rev------> + // --------------- |-----sig------> + // <----rev------- |--------------- + htlc4, _ := createHTLC(1, htlcAmt2) + + _, err = bobChannel.AddHTLC(htlc4, nil) + require.NoError(t, err) + _, err = aliceChannel.ReceiveHTLC(htlc4) + require.NoError(t, err) + + err = ForceStateTransition(bobChannel, aliceChannel) + require.NoError(t, err) + + // Check that alice has the expected local balance left. + aliceReserveMsat := lnwire.NewMSatFromSatoshis(aliceReserve) + + // The bufferAmt has to pay for the 2 additional incoming htlcs added + // by bob. + feeHTLC := feePerKw.FeeForWeight(input.HTLCWeight) + feeHTLCMsat := lnwire.NewMSatFromSatoshis(feeHTLC) + + aliceBalance := aliceReserveMsat + bufferAmt - 2*feeHTLCMsat + expectedAmt := aliceChannel.channelState.LocalCommitment.LocalBalance + + require.Equal(t, aliceBalance, expectedAmt) +} diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 95a931cf3..550586a4c 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -278,14 +278,14 @@ func addTestHtlcs(t *testing.T, remote, local *LightningChannel, PaymentHash: hash, } if htlc.incoming { - htlcID, err := remote.AddHTLC(msg, nil) + htlcID, err := remote.addHTLC(msg, nil, NoBuffer) require.NoError(t, err, "unable to add htlc") msg.ID = htlcID _, err = local.ReceiveHTLC(msg) require.NoError(t, err, "unable to recv htlc") } else { - htlcID, err := local.AddHTLC(msg, nil) + htlcID, err := local.addHTLC(msg, nil, NoBuffer) require.NoError(t, err, "unable to add htlc") msg.ID = htlcID diff --git a/routing/pathfind.go b/routing/pathfind.go index 091c18d92..642dab90b 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -586,6 +586,10 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // If the total outgoing balance isn't sufficient, it will be // impossible to complete the payment. if total < amt { + log.Warnf("Not enough outbound balance to send "+ + "htlc of amount: %v, only have local "+ + "balance: %v", amt, total) + return nil, 0, errInsufficientBalance }