diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index d367d5e6b..9c2c9715f 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -71,6 +71,15 @@ var ( // ErrLocalAddFailed signals that the ADD htlc for a local payment // failed to be processed. ErrLocalAddFailed = errors.New("local add HTLC failed") + + // errDustThresholdExceeded is only surfaced to callers of SendHTLC and + // signals that sending the HTLC would exceed the outgoing link's dust + // threshold. + errDustThresholdExceeded = errors.New("dust threshold exceeded") + + // defaultDustThreshold is the default threshold after which we'll fail + // payments if they are dust. This is currently set to 500k sats. + defaultDustThreshold = lnwire.MilliSatoshi(500_000_000) ) // plexPacket encapsulates switch packet and adds error channel to receive @@ -455,6 +464,51 @@ func (s *Switch) SendHTLC(firstHop lnwire.ShortChannelID, attemptID uint64, htlc: htlc, } + // Attempt to fetch the target link before creating a circuit so that + // we don't leave dangling circuits. The getLocalLink method does not + // require the circuit variable to be set on the *htlcPacket. + link, linkErr := s.getLocalLink(packet, htlc) + if linkErr != nil { + // Notify the htlc notifier of a link failure on our outgoing + // link. Incoming timelock/amount values are not set because + // they are not present for local sends. + s.cfg.HtlcNotifier.NotifyLinkFailEvent( + newHtlcKey(packet), + HtlcInfo{ + OutgoingTimeLock: htlc.Expiry, + OutgoingAmt: htlc.Amount, + }, + HtlcEventTypeSend, + linkErr, + false, + ) + + return linkErr + } + + // Evaluate whether this HTLC would increase our exposure to dust. If + // it does, don't send it out and instead return an error. + if s.evaluateDustThreshold(link, htlc.Amount, false) { + // Notify the htlc notifier of a link failure on our outgoing + // link. We use the FailTemporaryChannelFailure in place of a + // more descriptive error message. + linkErr := NewLinkError( + &lnwire.FailTemporaryChannelFailure{}, + ) + s.cfg.HtlcNotifier.NotifyLinkFailEvent( + newHtlcKey(packet), + HtlcInfo{ + OutgoingTimeLock: htlc.Expiry, + OutgoingAmt: htlc.Amount, + }, + HtlcEventTypeSend, + linkErr, + false, + ) + + return errDustThresholdExceeded + } + circuit := newPaymentCircuit(&htlc.PaymentHash, packet) actions, err := s.circuits.CommitCircuits(circuit) if err != nil { @@ -474,27 +528,6 @@ func (s *Switch) SendHTLC(firstHop lnwire.ShortChannelID, attemptID uint64, // Send packet to link. packet.circuit = circuit - // User has created the htlc update therefore we should find the - // appropriate channel link and send the payment over this link. - link, linkErr := s.getLocalLink(packet, htlc) - if linkErr != nil { - // Notify the htlc notifier of a link failure on our - // outgoing link. Incoming timelock/amount values are - // not set because they are not present for local sends. - s.cfg.HtlcNotifier.NotifyLinkFailEvent( - newHtlcKey(packet), - HtlcInfo{ - OutgoingTimeLock: htlc.Expiry, - OutgoingAmt: htlc.Amount, - }, - HtlcEventTypeSend, - linkErr, - false, - ) - - return linkErr - } - return link.handleLocalAddPacket(packet) } @@ -1084,6 +1117,50 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { // what the best channel is. destination := destinations[rand.Intn(len(destinations))] + // Retrieve the incoming link by its ShortChannelID. Note that + // the incomingChanID is never set to hop.Source here. + s.indexMtx.RLock() + incomingLink, err := s.getLinkByShortID(packet.incomingChanID) + s.indexMtx.RUnlock() + if err != nil { + // If we couldn't find the incoming link, we can't + // evaluate the incoming's exposure to dust, so we just + // fail the HTLC back. + linkErr := NewLinkError( + &lnwire.FailTemporaryChannelFailure{}, + ) + + return s.failAddPacket(packet, linkErr) + } + + // Evaluate whether this HTLC would increase our exposure to + // dust on the incoming link. If it does, fail it backwards. + if s.evaluateDustThreshold( + incomingLink, packet.incomingAmount, true, + ) { + // The incoming dust exceeds the threshold, so we fail + // the add back. + linkErr := NewLinkError( + &lnwire.FailTemporaryChannelFailure{}, + ) + + return s.failAddPacket(packet, linkErr) + } + + // Also evaluate whether this HTLC would increase our exposure + // to dust on the destination link. If it does, fail it back. + if s.evaluateDustThreshold( + destination, packet.amount, false, + ) { + // The outgoing dust exceeds the threshold, so we fail + // the add back. + linkErr := NewLinkError( + &lnwire.FailTemporaryChannelFailure{}, + ) + + return s.failAddPacket(packet, linkErr) + } + // Send the packet to the destination channel link which // manages the channel. packet.outgoingChanID = destination.ShortChanID() @@ -2254,3 +2331,73 @@ func (s *Switch) FlushForwardingEvents() error { func (s *Switch) BestHeight() uint32 { return atomic.LoadUint32(&s.bestHeight) } + +// evaluateDustThreshold takes in a ChannelLink, HTLC amount, and a boolean to +// determine whether the default dust threshold has been exceeded. This +// heuristic takes into account the trimmed-to-dust mechanism. The sum of the +// commitment's dust with the mailbox's dust with the amount is checked against +// the default threshold. If incoming is true, then the amount is not included +// in the sum as it was already included in the commitment's dust. A boolean is +// returned telling the caller whether the HTLC should be failed back. +func (s *Switch) evaluateDustThreshold(link ChannelLink, + amount lnwire.MilliSatoshi, incoming bool) bool { + + // Retrieve the link's current commitment feerate and dustClosure. + feeRate := link.getFeeRate() + isDust := link.getDustClosure() + + // Evaluate if the HTLC is dust on either sides' commitment. + isLocalDust := isDust(feeRate, incoming, true, amount.ToSatoshis()) + isRemoteDust := isDust(feeRate, incoming, false, amount.ToSatoshis()) + + if !(isLocalDust || isRemoteDust) { + // If the HTLC is not dust on either commitment, it's fine to + // forward. + return false + } + + // Fetch the dust sums currently in the mailbox for this link. + cid := link.ChanID() + sid := link.ShortChanID() + mailbox := s.mailOrchestrator.GetOrCreateMailBox(cid, sid) + localMailDust, remoteMailDust := mailbox.DustPackets() + + // If the htlc is dust on the local commitment, we'll obtain the dust + // sum for it. + if isLocalDust { + localSum := link.getDustSum(false) + localSum += localMailDust + + // Optionally include the HTLC amount only for outgoing + // HTLCs. + if !incoming { + localSum += amount + } + + // Finally check against the defined dust threshold. + if localSum > defaultDustThreshold { + return true + } + } + + // Also check if the htlc is dust on the remote commitment, if we've + // reached this point. + if isRemoteDust { + remoteSum := link.getDustSum(true) + remoteSum += remoteMailDust + + // Optionally include the HTLC amount only for outgoing + // HTLCs. + if !incoming { + remoteSum += amount + } + + // Finally check against the defined dust threshold. + if remoteSum > defaultDustThreshold { + return true + } + } + + // If we reached this point, this HTLC is fine to forward. + return false +} diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 2c4e0f394..09c38d030 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -13,10 +13,12 @@ import ( "github.com/btcsuite/btcutil" "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/htlcswitch/hodl" "github.com/lightningnetwork/lnd/htlcswitch/hop" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/ticker" + "github.com/stretchr/testify/require" ) var zeroCircuit = channeldb.CircuitKey{} @@ -3318,3 +3320,363 @@ func TestSwitchHoldForward(t *testing.T) { assertOutgoingLinkReceive(t, aliceChannelLink, true) assertNumCircuits(t, s, 0, 0) } + +// TestSwitchDustForwarding tests that the switch properly fails HTLC's which +// have incoming or outgoing links that breach their dust thresholds. +func TestSwitchDustForwarding(t *testing.T) { + t.Parallel() + + // We'll create a three-hop network: + // - Alice has a dust limit of 200sats with Bob + // - Bob has a dust limit of 800sats with Alice + // - Bob has a dust limit of 200sats with Carol + // - Carol has a dust limit of 800sats with Bob + channels, cleanUp, _, err := createClusterChannels( + btcutil.SatoshiPerBitcoin, btcutil.SatoshiPerBitcoin, + ) + require.NoError(t, err) + defer cleanUp() + + n := newThreeHopNetwork( + t, channels.aliceToBob, channels.bobToAlice, + channels.bobToCarol, channels.carolToBob, testStartingHeight, + ) + err = n.start() + require.NoError(t, err) + + // We'll also put Alice and Bob into hodl.ExitSettle mode, such that + // they won't settle incoming exit-hop HTLC's automatically. + n.aliceChannelLink.cfg.HodlMask = hodl.ExitSettle.Mask() + n.firstBobChannelLink.cfg.HodlMask = hodl.ExitSettle.Mask() + + // We'll test that once the default threshold is exceeded on the + // Alice -> Bob channel, either side's calls to SendHTLC will fail. + // This does not rely on the mailbox sum since there's no intermediate + // hop. + // + // Alice will send 357 HTLC's of 700sats. Bob will also send 357 HTLC's + // of 700sats. If either side attempts to send a dust HTLC, it will + // fail so amounts below 800sats will breach the dust threshold. + amt := lnwire.NewMSatFromSatoshis(700) + aliceBobFirstHop := n.aliceChannelLink.ShortChanID() + + sendDustHtlcs(t, n, true, amt, aliceBobFirstHop) + sendDustHtlcs(t, n, false, amt, aliceBobFirstHop) + + // Generate the parameters needed for Bob to send another dust HTLC. + _, timelock, hops := generateHops( + amt, testStartingHeight, n.aliceChannelLink, + ) + + blob, err := generateRoute(hops...) + require.NoError(t, err) + + // Assert that if Bob sends a dust HTLC it will fail. + failingPreimage := lntypes.Preimage{0, 0, 3} + failingHash := failingPreimage.Hash() + failingHtlc := &lnwire.UpdateAddHTLC{ + PaymentHash: failingHash, + Amount: amt, + Expiry: timelock, + OnionBlob: blob, + } + + // Assert that the HTLC is failed due to the dust threshold. + err = n.bobServer.htlcSwitch.SendHTLC( + aliceBobFirstHop, uint64(357), failingHtlc, + ) + require.ErrorIs(t, err, errDustThresholdExceeded) + + // Generate the parameters needed for bob to send a non-dust HTLC. + nondustAmt := lnwire.NewMSatFromSatoshis(10_000) + _, _, hops = generateHops( + nondustAmt, testStartingHeight, n.aliceChannelLink, + ) + + blob, err = generateRoute(hops...) + require.NoError(t, err) + + // Now attempt to send an HTLC above Bob's dust limit. It should + // succeed. + nondustPreimage := lntypes.Preimage{0, 0, 4} + nondustHash := nondustPreimage.Hash() + nondustHtlc := &lnwire.UpdateAddHTLC{ + PaymentHash: nondustHash, + Amount: nondustAmt, + Expiry: timelock, + OnionBlob: blob, + } + + // Assert that SendHTLC succeeds and evaluateDustThreshold returns + // false. + err = n.bobServer.htlcSwitch.SendHTLC( + aliceBobFirstHop, uint64(358), nondustHtlc, + ) + require.NoError(t, err) + + // Introduce Carol into the mix and assert that sending a multi-hop + // dust HTLC to Alice will fail. Bob should fail back the HTLC with a + // temporary channel failure. + carolAmt, carolTimelock, carolHops := generateHops( + amt, testStartingHeight, n.secondBobChannelLink, + n.aliceChannelLink, + ) + + carolBlob, err := generateRoute(carolHops...) + require.NoError(t, err) + + carolPreimage := lntypes.Preimage{0, 0, 5} + carolHash := carolPreimage.Hash() + carolHtlc := &lnwire.UpdateAddHTLC{ + PaymentHash: carolHash, + Amount: carolAmt, + Expiry: carolTimelock, + OnionBlob: carolBlob, + } + + // Initialize Carol's attempt ID. + carolAttemptID := 0 + + err = n.carolServer.htlcSwitch.SendHTLC( + n.carolChannelLink.ShortChanID(), uint64(carolAttemptID), + carolHtlc, + ) + require.NoError(t, err) + carolAttemptID++ + + carolResultChan, err := n.carolServer.htlcSwitch.GetPaymentResult( + uint64(carolAttemptID-1), carolHash, newMockDeobfuscator(), + ) + require.NoError(t, err) + + select { + case result, ok := <-carolResultChan: + require.True(t, ok) + assertFailureCode( + t, result.Error, lnwire.CodeTemporaryChannelFailure, + ) + + case <-time.After(5 * time.Second): + t.Fatal("no result arrived for carol's dust htlc") + } + + // Send an HTLC from Alice to Carol and assert that it is failed at the + // call to SendHTLC. + htlcAmt, totalTimelock, aliceHops := generateHops( + amt, testStartingHeight, n.firstBobChannelLink, + n.carolChannelLink, + ) + + blob, err = generateRoute(aliceHops...) + require.NoError(t, err) + + aliceMultihopPreimage := lntypes.Preimage{0, 0, 6} + aliceMultihopHash := aliceMultihopPreimage.Hash() + aliceMultihopHtlc := &lnwire.UpdateAddHTLC{ + PaymentHash: aliceMultihopHash, + Amount: htlcAmt, + Expiry: totalTimelock, + OnionBlob: blob, + } + + err = n.aliceServer.htlcSwitch.SendHTLC( + n.aliceChannelLink.ShortChanID(), uint64(357), + aliceMultihopHtlc, + ) + require.ErrorIs(t, err, errDustThresholdExceeded) +} + +// sendDustHtlcs is a helper function used to send many dust HTLC's to test the +// Switch's dust-threshold logic. It takes a boolean denoting whether or not +// Alice is the sender. +func sendDustHtlcs(t *testing.T, n *threeHopNetwork, alice bool, + amt lnwire.MilliSatoshi, sid lnwire.ShortChannelID) { + + t.Helper() + + // The number of dust HTLC's we'll send for both Alice and Bob. + numHTLCs := 357 + + // Extract the destination into a variable. If alice is the sender, the + // destination is Bob. + destLink := n.aliceChannelLink + if alice { + destLink = n.firstBobChannelLink + } + + // Create hops that will be used in the onion payload. + htlcAmt, totalTimelock, hops := generateHops( + amt, testStartingHeight, destLink, + ) + + // Convert the hops to a blob that will be put in the Add message. + blob, err := generateRoute(hops...) + require.NoError(t, err) + + // Create a slice to store the preimages. + preimages := make([]lntypes.Preimage, numHTLCs) + + // Initialize the attempt ID used in SendHTLC calls. + attemptID := uint64(0) + + // Deterministically generate preimages. Avoid the all-zeroes preimage + // because that will be rejected by the database. We'll use a different + // third byte for Alice and Bob. + endByte := byte(2) + if alice { + endByte = byte(3) + } + + for i := 0; i < numHTLCs; i++ { + preimages[i] = lntypes.Preimage{byte(i >> 8), byte(i), endByte} + } + + sendingSwitch := n.bobServer.htlcSwitch + if alice { + sendingSwitch = n.aliceServer.htlcSwitch + } + + // Call SendHTLC in a loop for numHTLCs. + for i := 0; i < numHTLCs; i++ { + // Construct the htlc packet. + hash := preimages[i].Hash() + + htlc := &lnwire.UpdateAddHTLC{ + PaymentHash: hash, + Amount: htlcAmt, + Expiry: totalTimelock, + OnionBlob: blob, + } + + err = sendingSwitch.SendHTLC(sid, attemptID, htlc) + require.NoError(t, err) + attemptID++ + } +} + +// TestSwitchMailboxDust tests that the switch takes into account the mailbox +// dust when evaluating the dust threshold. The mockChannelLink does not have +// channel state, so this only tests the switch-mailbox interaction. +func TestSwitchMailboxDust(t *testing.T) { + t.Parallel() + + alicePeer, err := newMockServer( + t, "alice", testStartingHeight, nil, testDefaultDelta, + ) + require.NoError(t, err) + + bobPeer, err := newMockServer( + t, "bob", testStartingHeight, nil, testDefaultDelta, + ) + require.NoError(t, err) + + carolPeer, err := newMockServer( + t, "carol", testStartingHeight, nil, testDefaultDelta, + ) + require.NoError(t, err) + + s, err := initSwitchWithDB(testStartingHeight, nil) + require.NoError(t, err) + err = s.Start() + require.NoError(t, err) + defer func() { + _ = s.Stop() + }() + + chanID1, chanID2, aliceChanID, bobChanID := genIDs() + + chanID3, carolChanID := genID() + + aliceLink := newMockChannelLink( + s, chanID1, aliceChanID, alicePeer, true, + ) + err = s.AddLink(aliceLink) + require.NoError(t, err) + + bobLink := newMockChannelLink( + s, chanID2, bobChanID, bobPeer, true, + ) + err = s.AddLink(bobLink) + require.NoError(t, err) + + carolLink := newMockChannelLink( + s, chanID3, carolChanID, carolPeer, true, + ) + err = s.AddLink(carolLink) + require.NoError(t, err) + + // mockChannelLink sets the local and remote dust limits of the mailbox + // to 400 satoshis and the feerate to 0. We'll fill the mailbox up with + // dust packets and assert that calls to SendHTLC will fail. + preimage, err := genPreimage() + require.NoError(t, err) + rhash := sha256.Sum256(preimage[:]) + amt := lnwire.NewMSatFromSatoshis(350) + addMsg := &lnwire.UpdateAddHTLC{ + PaymentHash: rhash, + Amount: amt, + ChanID: chanID1, + } + + // Initialize the carolHTLCID. + var carolHTLCID uint64 + + // It will take aliceCount HTLC's of 350sats to fill up Alice's mailbox + // to the point where another would put Alice over the dust threshold. + aliceCount := 1428 + + mailbox := s.mailOrchestrator.GetOrCreateMailBox(chanID1, aliceChanID) + + for i := 0; i < aliceCount; i++ { + alicePkt := &htlcPacket{ + incomingChanID: carolChanID, + incomingHTLCID: carolHTLCID, + outgoingChanID: aliceChanID, + obfuscator: NewMockObfuscator(), + incomingAmount: amt, + amount: amt, + htlc: addMsg, + } + + err = mailbox.AddPacket(alicePkt) + require.NoError(t, err) + + carolHTLCID++ + } + + // Sending one more HTLC to Alice should result in the dust threshold + // being breached. + err = s.SendHTLC(aliceChanID, 0, addMsg) + require.ErrorIs(t, err, errDustThresholdExceeded) + + // We'll now call ForwardPackets from Bob to ensure that the mailbox + // sum is also accounted for in the forwarding case. + packet := &htlcPacket{ + incomingChanID: bobChanID, + incomingHTLCID: 0, + outgoingChanID: aliceChanID, + obfuscator: NewMockObfuscator(), + incomingAmount: amt, + amount: amt, + htlc: &lnwire.UpdateAddHTLC{ + PaymentHash: rhash, + Amount: amt, + ChanID: chanID1, + }, + } + + err = s.ForwardPackets(nil, packet) + require.NoError(t, err) + + // Bob should receive a failure from the switch. + select { + case p := <-bobLink.packets: + require.NotEmpty(t, p.linkFailure) + assertFailureCode( + t, p.linkFailure, lnwire.CodeTemporaryChannelFailure, + ) + + case <-time.After(5 * time.Second): + t.Fatal("no timely reply from switch") + } +}