diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 9519e7fe..8b259365 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -4642,8 +4642,11 @@ func (lc *LightningChannel) AddHTLC(htlc *lnwire.UpdateAddHTLC, } // Make sure adding this HTLC won't violate any of the constraints we - // must keep on our commitment transaction. + // must keep on the commitment transactions. remoteACKedIndex := lc.localCommitChain.tail().theirMessageIndex + + // First we'll check whether this HTLC can be added to the remote + // commitment transaction without violation any of the constraints. err := lc.validateCommitmentSanity( remoteACKedIndex, lc.localUpdateLog.logIndex, true, pd, nil, ) @@ -4651,6 +4654,19 @@ func (lc *LightningChannel) AddHTLC(htlc *lnwire.UpdateAddHTLC, return 0, err } + // We must also check whether it can be added to our own commitment + // transaction, or the remote node will refuse to sign. This is not + // totally bullet proof, as the remote might be adding updates + // concurrently, but if we fail this check there is for sure not + // possible for us to add the HTLC. + err = lc.validateCommitmentSanity( + lc.remoteUpdateLog.logIndex, lc.localUpdateLog.logIndex, + false, pd, nil, + ) + if err != nil { + return 0, err + } + lc.localUpdateLog.appendHtlc(pd) return pd.HtlcIndex, nil diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 3f97f9f9..e9455b75 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -6061,6 +6061,51 @@ func TestChanReserveRemoteInitiator(t *testing.T) { } } +// TestChanReserveLocalInitiatorDustHtlc tests that fee the initiator must pay +// when adding HTLCs is accounted for, even though the HTLC is considered dust +// by the remote bode. +func TestChanReserveLocalInitiatorDustHtlc(t *testing.T) { + t.Parallel() + + aliceChannel, bobChannel, cleanUp, err := CreateTestChannels( + true, + ) + if err != nil { + t.Fatal(err) + } + defer cleanUp() + + // The amount of the HTLC should not be considered dust according to + // Alice's dust limit (200 sat), but be dust according to Bob's dust + // limit (1300 sat). It is considered dust if the amount remaining + // after paying the HTLC fee is below the dustlimit, so we choose a + // size of 500+htlcFee. + htlcSat := btcutil.Amount(500) + htlcTimeoutFee( + chainfee.SatPerKWeight( + aliceChannel.channelState.LocalCommitment.FeePerKw, + ), + ) + + // Set Alice's channel reserve to be low enough to carry the value of + // the HTLC, but not low enough to allow the extra fee from adding the + // HTLC to the commitment. + commitFee := aliceChannel.channelState.LocalCommitment.CommitFee + aliceMinReserve := 5*btcutil.SatoshiPerBitcoin - commitFee - htlcSat + + aliceChannel.channelState.LocalChanCfg.ChanReserve = aliceMinReserve + bobChannel.channelState.RemoteChanCfg.ChanReserve = aliceMinReserve + + htlcDustAmt := lnwire.NewMSatFromSatoshis(htlcSat) + htlc, _ := createHTLC(0, htlcDustAmt) + + // Alice should realize that the fee she must pay to add this HTLC to + // the local commitment would take her below the channel reserve. + _, err = aliceChannel.AddHTLC(htlc, nil) + if err != ErrBelowChanReserve { + t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err) + } +} + // TestMinHTLC tests that the ErrBelowMinHTLC error is thrown if an HTLC is added // that is below the minimm allowed value for HTLCs. func TestMinHTLC(t *testing.T) {