diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 2760c42bc..f82327b2b 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -12,6 +12,7 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btclog" + "github.com/btcsuite/btcutil" "github.com/davecgh/go-spew/spew" "github.com/go-errors/errors" "github.com/lightningnetwork/lnd/build" @@ -2192,6 +2193,36 @@ func (l *channelLink) MayAddOutgoingHtlc() error { return l.channel.MayAddOutgoingHtlc() } +// dustClosure is a function that evaluates whether an HTLC is dust. It returns +// true if the HTLC is dust. It takes in a feerate, a boolean denoting whether +// the HTLC is incoming (i.e. one that the remote sent), a boolean denoting +// whether to evaluate on the local or remote commit, and finally an HTLC +// amount to test. +type dustClosure func(chainfee.SatPerKWeight, bool, bool, btcutil.Amount) bool + +// dustHelper is used to construct the dustClosure. +func dustHelper(chantype channeldb.ChannelType, localDustLimit, + remoteDustLimit btcutil.Amount) dustClosure { + + isDust := func(feerate chainfee.SatPerKWeight, incoming, + localCommit bool, amt btcutil.Amount) bool { + + if localCommit { + return lnwallet.HtlcIsDust( + chantype, incoming, true, feerate, amt, + localDustLimit, + ) + } + + return lnwallet.HtlcIsDust( + chantype, incoming, false, feerate, amt, + remoteDustLimit, + ) + } + + return isDust +} + // AttachMailBox updates the current mailbox used by this link, and hooks up // the mailbox's message and packet outboxes to the link's upstream and // downstream chans, respectively. diff --git a/htlcswitch/mailbox.go b/htlcswitch/mailbox.go index b082d9ba1..6bec19f39 100644 --- a/htlcswitch/mailbox.go +++ b/htlcswitch/mailbox.go @@ -9,6 +9,7 @@ import ( "time" "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" ) @@ -66,6 +67,17 @@ type MailBox interface { // Reset the packet head to point at the first element in the list. ResetPackets() error + // SetDustClosure takes in a closure that is used to evaluate whether + // mailbox HTLC's are dust. + SetDustClosure(isDust dustClosure) + + // SetFeeRate sets the feerate to be used when evaluating dust. + SetFeeRate(feerate chainfee.SatPerKWeight) + + // DustPackets returns the dust sum for Adds in the mailbox for the + // local and remote commitments. + DustPackets() (lnwire.MilliSatoshi, lnwire.MilliSatoshi) + // Start starts the mailbox and any goroutines it needs to operate // properly. Start() @@ -131,6 +143,17 @@ type memoryMailBox struct { wireShutdown chan struct{} pktShutdown chan struct{} quit chan struct{} + + // feeRate is set when the link receives or sends out fee updates. It + // is refreshed when AttachMailBox is called in case a fee update did + // not get committed. In some cases it may be out of sync with the + // channel's feerate, but it should eventually get back in sync. + feeRate chainfee.SatPerKWeight + + // isDust is set when AttachMailBox is called and serves to evaluate + // the outstanding dust in the memoryMailBox given the current set + // feeRate. + isDust dustClosure } // newMemoryMailBox creates a new instance of the memoryMailBox. @@ -610,6 +633,61 @@ func (m *memoryMailBox) AddPacket(pkt *htlcPacket) error { return nil } +// SetFeeRate sets the memoryMailBox's feerate for use in DustPackets. +func (m *memoryMailBox) SetFeeRate(feeRate chainfee.SatPerKWeight) { + m.pktCond.L.Lock() + defer m.pktCond.L.Unlock() + + m.feeRate = feeRate +} + +// SetDustClosure sets the memoryMailBox's dustClosure for use in DustPackets. +func (m *memoryMailBox) SetDustClosure(isDust dustClosure) { + m.pktCond.L.Lock() + defer m.pktCond.L.Unlock() + + m.isDust = isDust +} + +// DustPackets returns the dust sum for add packets in the mailbox. The first +// return value is the local dust sum and the second is the remote dust sum. +// This will keep track of a given dust HTLC from the time it is added via +// AddPacket until it is removed via AckPacket. +func (m *memoryMailBox) DustPackets() (lnwire.MilliSatoshi, + lnwire.MilliSatoshi) { + + m.pktCond.L.Lock() + defer m.pktCond.L.Unlock() + + var ( + localDustSum lnwire.MilliSatoshi + remoteDustSum lnwire.MilliSatoshi + ) + + // Run through the map of HTLC's and determine the dust sum with calls + // to the memoryMailBox's isDust closure. Note that all mailbox packets + // are outgoing so the second argument to isDust will be false. + for _, e := range m.addIndex { + addPkt := e.Value.(*pktWithExpiry).pkt + + // Evaluate whether this HTLC is dust on the local commitment. + if m.isDust( + m.feeRate, false, true, addPkt.amount.ToSatoshis(), + ) { + localDustSum += addPkt.amount + } + + // Evaluate whether this HTLC is dust on the remote commitment. + if m.isDust( + m.feeRate, false, false, addPkt.amount.ToSatoshis(), + ) { + remoteDustSum += addPkt.amount + } + } + + return localDustSum, remoteDustSum +} + // FailAdd fails an UpdateAddHTLC that exists within the mailbox, removing it // from the in-memory replay buffer. This will prevent the packet from being // delivered after the link restarts if the switch has remained online. The diff --git a/htlcswitch/mailbox_test.go b/htlcswitch/mailbox_test.go index ef199467a..86c8faa6d 100644 --- a/htlcswitch/mailbox_test.go +++ b/htlcswitch/mailbox_test.go @@ -6,9 +6,13 @@ import ( "testing" "time" + "github.com/btcsuite/btcutil" "github.com/davecgh/go-spew/spew" + "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" + "github.com/stretchr/testify/require" ) const testExpiry = time.Minute @@ -536,6 +540,126 @@ func TestMailBoxDuplicateAddPacket(t *testing.T) { }) } +// TestMailBoxDustHandling tests that DustPackets returns the expected values +// for the local and remote dust sum after calling SetFeeRate and +// SetDustClosure. +func TestMailBoxDustHandling(t *testing.T) { + t.Run("tweakless mailbox dust", func(t *testing.T) { + testMailBoxDust(t, channeldb.SingleFunderTweaklessBit) + }) + t.Run("zero htlc fee anchors mailbox dust", func(t *testing.T) { + testMailBoxDust(t, channeldb.SingleFunderTweaklessBit| + channeldb.AnchorOutputsBit| + channeldb.ZeroHtlcTxFeeBit, + ) + }) +} + +func testMailBoxDust(t *testing.T, chantype channeldb.ChannelType) { + t.Parallel() + + ctx := newMailboxContext(t, time.Now(), testExpiry) + defer ctx.mailbox.Stop() + + _, _, aliceID, bobID := genIDs() + + // It should not be the case that the MailBox has packets before the + // feeRate or dustClosure is set. This is because the mailbox is always + // created *with* its associated link and attached via AttachMailbox, + // where these parameters will be set. Even though the lifetime is + // longer than the link, the setting will persist across multiple link + // creations. + ctx.mailbox.SetFeeRate(chainfee.SatPerKWeight(253)) + + localDustLimit := btcutil.Amount(400) + remoteDustLimit := btcutil.Amount(500) + isDust := dustHelper(chantype, localDustLimit, remoteDustLimit) + ctx.mailbox.SetDustClosure(isDust) + + // The first packet will be dust according to the remote dust limit, + // but not the local. We set a different amount if this is a zero fee + // htlc channel type. + firstAmt := lnwire.MilliSatoshi(600_000) + + if chantype.ZeroHtlcTxFee() { + firstAmt = lnwire.MilliSatoshi(450_000) + } + + firstPkt := &htlcPacket{ + outgoingChanID: aliceID, + outgoingHTLCID: 0, + incomingChanID: bobID, + incomingHTLCID: 0, + amount: firstAmt, + htlc: &lnwire.UpdateAddHTLC{ + ID: uint64(0), + }, + } + + err := ctx.mailbox.AddPacket(firstPkt) + require.NoError(t, err) + + // Assert that the local sum is 0, and the remote sum accounts for this + // added packet. + localSum, remoteSum := ctx.mailbox.DustPackets() + require.Equal(t, lnwire.MilliSatoshi(0), localSum) + require.Equal(t, firstAmt, remoteSum) + + // The next packet will be dust according to both limits. + secondAmt := lnwire.MilliSatoshi(300_000) + secondPkt := &htlcPacket{ + outgoingChanID: aliceID, + outgoingHTLCID: 1, + incomingChanID: bobID, + incomingHTLCID: 1, + amount: secondAmt, + htlc: &lnwire.UpdateAddHTLC{ + ID: uint64(1), + }, + } + + err = ctx.mailbox.AddPacket(secondPkt) + require.NoError(t, err) + + // Assert that both the local and remote sums have increased by the + // second amount. + localSum, remoteSum = ctx.mailbox.DustPackets() + require.Equal(t, secondAmt, localSum) + require.Equal(t, firstAmt+secondAmt, remoteSum) + + // Now we pull both packets off of the queue. + for i := 0; i < 2; i++ { + select { + case <-ctx.mailbox.PacketOutBox(): + case <-time.After(50 * time.Millisecond): + ctx.t.Fatalf("did not receive packet in time") + } + } + + // Assert that the sums haven't changed. + localSum, remoteSum = ctx.mailbox.DustPackets() + require.Equal(t, secondAmt, localSum) + require.Equal(t, firstAmt+secondAmt, remoteSum) + + // Remove the first packet from the mailbox. + removed := ctx.mailbox.AckPacket(firstPkt.inKey()) + require.True(t, removed) + + // Assert that the remote sum does not include the firstAmt. + localSum, remoteSum = ctx.mailbox.DustPackets() + require.Equal(t, secondAmt, localSum) + require.Equal(t, secondAmt, remoteSum) + + // Remove the second packet from the mailbox. + removed = ctx.mailbox.AckPacket(secondPkt.inKey()) + require.True(t, removed) + + // Assert that both sums are equal to 0. + localSum, remoteSum = ctx.mailbox.DustPackets() + require.Equal(t, lnwire.MilliSatoshi(0), localSum) + require.Equal(t, lnwire.MilliSatoshi(0), remoteSum) +} + // TestMailOrchestrator asserts that the orchestrator properly buffers packets // for channels that haven't been made live, such that they are delivered // immediately after BindLiveShortChanID. It also tests that packets are delivered