From 7d16e58b5cd4049bb25f01feef8d4b935e2d69cd Mon Sep 17 00:00:00 2001 From: eugene Date: Tue, 28 Sep 2021 11:34:10 -0400 Subject: [PATCH] lnwallet: introduce GetDustSum method to calculate worst-case dust sum It over-estimates the local or remote commitment's dust sum by counting all updates in both updateLogs that are dust using the trimmed-to-dust mechanism if applicable. The over-estimation is done because ensuring an accurate counting is a trade-off between code simplicity and accuracy. --- lnwallet/channel.go | 85 ++++++++++++++++--- lnwallet/channel_test.go | 175 +++++++++++++++++++++++++++++++++++++++ lnwallet/commitment.go | 8 +- lnwallet/test_utils.go | 5 +- 4 files changed, 255 insertions(+), 18 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 9143238b0..b914ac789 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -615,7 +615,7 @@ func (c *commitment) populateHtlcIndexes(chanType channeldb.ChannelType, // populateIndex is a helper function that populates the necessary // indexes within the commitment view for a particular HTLC. populateIndex := func(htlc *PaymentDescriptor, incoming bool) error { - isDust := htlcIsDust( + isDust := HtlcIsDust( chanType, incoming, c.isOurs, c.feePerKw, htlc.Amount.ToSatoshis(), c.dustLimit, ) @@ -792,7 +792,7 @@ func (lc *LightningChannel) diskHtlcToPayDesc(feeRate chainfee.SatPerKWeight, // generate them in order to locate the outputs within the commitment // transaction. As we'll mark dust with a special output index in the // on-disk state snapshot. - isDustLocal := htlcIsDust( + isDustLocal := HtlcIsDust( chanType, htlc.Incoming, true, feeRate, htlc.Amt.ToSatoshis(), lc.channelState.LocalChanCfg.DustLimit, ) @@ -805,7 +805,7 @@ func (lc *LightningChannel) diskHtlcToPayDesc(feeRate chainfee.SatPerKWeight, return pd, err } } - isDustRemote := htlcIsDust( + isDustRemote := HtlcIsDust( chanType, htlc.Incoming, false, feeRate, htlc.Amt.ToSatoshis(), lc.channelState.RemoteChanCfg.DustLimit, ) @@ -1436,7 +1436,7 @@ func (lc *LightningChannel) logUpdateToPayDesc(logUpdate *channeldb.LogUpdate, pd.OnionBlob = make([]byte, len(wireMsg.OnionBlob)) copy(pd.OnionBlob[:], wireMsg.OnionBlob[:]) - isDustRemote := htlcIsDust( + isDustRemote := HtlcIsDust( lc.channelState.ChanType, false, false, feeRate, wireMsg.Amount.ToSatoshis(), remoteDustLimit, ) @@ -2401,7 +2401,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, for _, htlc := range revokedSnapshot.Htlcs { // If the HTLC is dust, then we'll skip it as it doesn't have // an output on the commitment transaction. - if htlcIsDust( + if HtlcIsDust( chanState.ChanType, htlc.Incoming, false, chainfee.SatPerKWeight(revokedSnapshot.FeePerKw), htlc.Amt.ToSatoshis(), chanState.RemoteChanCfg.DustLimit, @@ -2473,13 +2473,13 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, }, nil } -// htlcIsDust determines if an HTLC output is dust or not depending on two +// HtlcIsDust determines if an HTLC output is dust or not depending on two // bits: if the HTLC is incoming and if the HTLC will be placed on our // commitment transaction, or theirs. These two pieces of information are // require as we currently used second-level HTLC transactions as off-chain // covenants. Depending on the two bits, we'll either be using a timeout or // success transaction which have different weights. -func htlcIsDust(chanType channeldb.ChannelType, +func HtlcIsDust(chanType channeldb.ChannelType, incoming, ourCommit bool, feePerKw chainfee.SatPerKWeight, htlcAmt, dustLimit btcutil.Amount) bool { @@ -2995,7 +2995,7 @@ func genRemoteHtlcSigJobs(keyRing *CommitmentKeyRing, // dust output after taking into account second-level HTLC fees, then a // sigJob will be generated and appended to the current batch. for _, htlc := range remoteCommitView.incomingHTLCs { - if htlcIsDust( + if HtlcIsDust( chanType, true, false, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, ) { @@ -3049,7 +3049,7 @@ func genRemoteHtlcSigJobs(keyRing *CommitmentKeyRing, sigBatch = append(sigBatch, sigJob) } for _, htlc := range remoteCommitView.outgoingHTLCs { - if htlcIsDust( + if HtlcIsDust( chanType, false, false, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, ) { @@ -4040,7 +4040,7 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, // weight, needed to calculate the transaction fee. var totalHtlcWeight int64 for _, htlc := range filteredHTLCView.ourUpdates { - if htlcIsDust( + if HtlcIsDust( lc.channelState.ChanType, false, !remoteChain, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, ) { @@ -4050,7 +4050,7 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, totalHtlcWeight += input.HTLCWeight } for _, htlc := range filteredHTLCView.theirUpdates { - if htlcIsDust( + if HtlcIsDust( lc.channelState.ChanType, true, !remoteChain, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, ) { @@ -4976,6 +4976,67 @@ func (lc *LightningChannel) AddHTLC(htlc *lnwire.UpdateAddHTLC, return pd.HtlcIndex, nil } +// GetDustSum takes in a boolean that determines which commitment to evaluate +// the dust sum on. The return value is the sum of dust on the desired +// commitment tx. +// +// NOTE: This over-estimates the dust exposure. +func (lc *LightningChannel) GetDustSum(remote bool) lnwire.MilliSatoshi { + lc.RLock() + defer lc.RUnlock() + + var dustSum lnwire.MilliSatoshi + + dustLimit := lc.channelState.LocalChanCfg.DustLimit + commit := lc.channelState.LocalCommitment + if remote { + // Calculate dust sum on the remote's commitment. + dustLimit = lc.channelState.RemoteChanCfg.DustLimit + commit = lc.channelState.RemoteCommitment + } + + chanType := lc.channelState.ChanType + feeRate := chainfee.SatPerKWeight(commit.FeePerKw) + + // Grab all of our HTLCs and evaluate against the dust limit. + for e := lc.localUpdateLog.Front(); e != nil; e = e.Next() { + pd := e.Value.(*PaymentDescriptor) + if pd.EntryType != Add { + continue + } + + amt := pd.Amount.ToSatoshis() + + // If the satoshi amount is under the dust limit, add the msat + // amount to the dust sum. + if HtlcIsDust( + chanType, false, !remote, feeRate, amt, dustLimit, + ) { + dustSum += pd.Amount + } + } + + // Grab all of their HTLCs and evaluate against the dust limit. + for e := lc.remoteUpdateLog.Front(); e != nil; e = e.Next() { + pd := e.Value.(*PaymentDescriptor) + if pd.EntryType != Add { + continue + } + + amt := pd.Amount.ToSatoshis() + + // If the satoshi amount is under the dust limit, add the msat + // amount to the dust sum. + if HtlcIsDust( + chanType, true, !remote, feeRate, amt, dustLimit, + ) { + dustSum += pd.Amount + } + } + + return dustSum +} + // MayAddOutgoingHtlc validates whether we can add an outgoing htlc to this // channel. We don't have a value or circuit for this htlc, because we just // want to test that we have slots for a potential htlc so we use a "mock" @@ -6056,7 +6117,7 @@ func extractHtlcResolutions(feePerKw chainfee.SatPerKWeight, ourCommit bool, // We'll skip any HTLC's which were dust on the commitment // transaction, as these don't have a corresponding output // within the commitment transaction. - if htlcIsDust( + if HtlcIsDust( chanType, htlc.Incoming, ourCommit, feePerKw, htlc.Amt.ToSatoshis(), dustLimit, ) { diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index c201cc787..0ef5c8514 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -9816,3 +9816,178 @@ func assertCleanOrDirty(clean bool, alice, bob *LightningChannel, require.False(t, alice.IsChannelClean()) require.False(t, bob.IsChannelClean()) } + +// TestChannelGetDustSum tests that we correctly calculate the channel's dust +// sum for the local and remote commitments. +func TestChannelGetDustSum(t *testing.T) { + t.Run("dust sum tweakless", func(t *testing.T) { + testGetDustSum(t, channeldb.SingleFunderTweaklessBit) + }) + t.Run("dust sum anchors zero htlc fee", func(t *testing.T) { + testGetDustSum(t, channeldb.SingleFunderTweaklessBit| + channeldb.AnchorOutputsBit| + channeldb.ZeroHtlcTxFeeBit, + ) + }) +} + +func testGetDustSum(t *testing.T, chantype channeldb.ChannelType) { + t.Parallel() + + // This makes a channel with Alice's dust limit set to 200sats and + // Bob's dust limit set to 1300sats. + aliceChannel, bobChannel, cleanUp, err := CreateTestChannels(chantype) + require.NoError(t, err) + defer cleanUp() + + // Use a function closure to assert the dust sum for a passed channel's + // local and remote commitments match the expected values. + checkDust := func(c *LightningChannel, expLocal, + expRemote lnwire.MilliSatoshi) { + + localDustSum := c.GetDustSum(false) + require.Equal(t, expLocal, localDustSum) + remoteDustSum := c.GetDustSum(true) + require.Equal(t, expRemote, remoteDustSum) + } + + // We'll lower the fee from 6000sats/kWU to 253sats/kWU for our test. + fee := chainfee.SatPerKWeight(253) + err = aliceChannel.UpdateFee(fee) + require.NoError(t, err) + err = bobChannel.ReceiveUpdateFee(fee) + require.NoError(t, err) + err = ForceStateTransition(aliceChannel, bobChannel) + require.NoError(t, err) + + // Create an HTLC that Bob will send to Alice which is above Alice's + // dust limit and below Bob's dust limit. This takes into account dust + // trimming for non-zero-fee channels. + htlc1Amt := lnwire.MilliSatoshi(700_000) + htlc1, preimage1 := createHTLC(0, htlc1Amt) + + _, err = bobChannel.AddHTLC(htlc1, nil) + require.NoError(t, err) + _, err = aliceChannel.ReceiveHTLC(htlc1) + require.NoError(t, err) + + // Assert that GetDustSum from Alice's perspective does not consider + // the HTLC dust on her commitment, but does on Bob's commitment. + checkDust(aliceChannel, lnwire.MilliSatoshi(0), htlc1Amt) + + // Assert that GetDustSum from Bob's perspective results in the same + // conditions above holding. + checkDust(bobChannel, htlc1Amt, lnwire.MilliSatoshi(0)) + + // Forcing a state transition to occur should not change the dust sum. + err = ForceStateTransition(bobChannel, aliceChannel) + require.NoError(t, err) + checkDust(aliceChannel, lnwire.MilliSatoshi(0), htlc1Amt) + checkDust(bobChannel, htlc1Amt, lnwire.MilliSatoshi(0)) + + // Settling the HTLC back from Alice to Bob should not change the dust + // sum because the HTLC is counted until it's removed from the update + // logs via compactLogs. + err = aliceChannel.SettleHTLC(preimage1, uint64(0), nil, nil, nil) + require.NoError(t, err) + err = bobChannel.ReceiveHTLCSettle(preimage1, uint64(0)) + require.NoError(t, err) + checkDust(aliceChannel, lnwire.MilliSatoshi(0), htlc1Amt) + checkDust(bobChannel, htlc1Amt, lnwire.MilliSatoshi(0)) + + // Forcing a state transition will remove the HTLC in-memory for Bob + // since ReceiveRevocation is called which calls compactLogs. Bob + // should have a zero dust sum at this point. Alice will see Bob as + // having the original dust sum since compactLogs hasn't been called. + err = ForceStateTransition(aliceChannel, bobChannel) + require.NoError(t, err) + checkDust(aliceChannel, lnwire.MilliSatoshi(0), htlc1Amt) + checkDust(bobChannel, lnwire.MilliSatoshi(0), lnwire.MilliSatoshi(0)) + + // Alice now sends an HTLC of 100sats, which is below both sides' dust + // limits. + htlc2Amt := lnwire.MilliSatoshi(100_000) + htlc2, _ := createHTLC(0, htlc2Amt) + + _, err = aliceChannel.AddHTLC(htlc2, nil) + require.NoError(t, err) + _, err = bobChannel.ReceiveHTLC(htlc2) + require.NoError(t, err) + + // Assert that GetDustSum from Alice's perspective includes the new + // HTLC as dust on both commitments. + checkDust(aliceChannel, htlc2Amt, htlc1Amt+htlc2Amt) + + // Assert that GetDustSum from Bob's perspective also includes the HTLC + // on both commitments. + checkDust(bobChannel, htlc2Amt, htlc2Amt) + + // Alice signs for this HTLC and neither perspective should change. + aliceSig, aliceHtlcSigs, _, err := aliceChannel.SignNextCommitment() + require.NoError(t, err) + err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs) + require.NoError(t, err) + checkDust(aliceChannel, htlc2Amt, htlc1Amt+htlc2Amt) + checkDust(bobChannel, htlc2Amt, htlc2Amt) + + // Bob now sends a revocation for his prior commitment, and this should + // change Alice's perspective to no longer include the first HTLC as + // dust. + bobRevocation, _, err := bobChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) + require.NoError(t, err) + checkDust(aliceChannel, htlc2Amt, htlc2Amt) + checkDust(bobChannel, htlc2Amt, htlc2Amt) + + // The rest of the dance is completed and neither perspective should + // change. + bobSig, bobHtlcSigs, _, err := bobChannel.SignNextCommitment() + require.NoError(t, err) + err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs) + require.NoError(t, err) + aliceRevocation, _, err := aliceChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = bobChannel.ReceiveRevocation(aliceRevocation) + require.NoError(t, err) + checkDust(aliceChannel, htlc2Amt, htlc2Amt) + checkDust(bobChannel, htlc2Amt, htlc2Amt) + + // We'll now assert that if Alice sends an HTLC above her dust limit + // and then updates the fee of the channel to trigger the trimmed to + // dust mechanism, Alice will count this HTLC in the dust sum for her + // commitment in the non-zero-fee case. + htlc3Amt := lnwire.MilliSatoshi(400_000) + htlc3, _ := createHTLC(1, htlc3Amt) + + _, err = aliceChannel.AddHTLC(htlc3, nil) + require.NoError(t, err) + _, err = bobChannel.ReceiveHTLC(htlc3) + require.NoError(t, err) + + // Assert that this new HTLC is not counted on Alice's local commitment + // in the dust sum. Bob's commitment should count it. + checkDust(aliceChannel, htlc2Amt, htlc2Amt+htlc3Amt) + checkDust(bobChannel, htlc2Amt+htlc3Amt, htlc2Amt) + + // Alice will now send UpdateFee with a large feerate and neither + // perspective should change. + fee = chainfee.SatPerKWeight(50_000) + err = aliceChannel.UpdateFee(fee) + require.NoError(t, err) + err = bobChannel.ReceiveUpdateFee(fee) + require.NoError(t, err) + checkDust(aliceChannel, htlc2Amt, htlc2Amt+htlc3Amt) + checkDust(bobChannel, htlc2Amt+htlc3Amt, htlc2Amt) + + // Forcing a state transition should change in the non-zero-fee case. + err = ForceStateTransition(aliceChannel, bobChannel) + require.NoError(t, err) + if chantype.ZeroHtlcTxFee() { + checkDust(aliceChannel, htlc2Amt, htlc2Amt+htlc3Amt) + checkDust(bobChannel, htlc2Amt+htlc3Amt, htlc2Amt) + } else { + checkDust(aliceChannel, htlc2Amt+htlc3Amt, htlc2Amt+htlc3Amt) + checkDust(bobChannel, htlc2Amt+htlc3Amt, htlc2Amt+htlc3Amt) + } +} diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index 96960f9dd..a93efa90e 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -440,7 +440,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, numHTLCs := int64(0) for _, htlc := range filteredHTLCView.ourUpdates { - if htlcIsDust( + if HtlcIsDust( cb.chanState.ChanType, false, isOurs, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, ) { @@ -450,7 +450,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, numHTLCs++ } for _, htlc := range filteredHTLCView.theirUpdates { - if htlcIsDust( + if HtlcIsDust( cb.chanState.ChanType, true, isOurs, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, ) { @@ -529,7 +529,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, // purposes of sorting. cltvs := make([]uint32, len(commitTx.TxOut)) for _, htlc := range filteredHTLCView.ourUpdates { - if htlcIsDust( + if HtlcIsDust( cb.chanState.ChanType, false, isOurs, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, ) { @@ -546,7 +546,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, cltvs = append(cltvs, htlc.Timeout) } for _, htlc := range filteredHTLCView.theirUpdates { - if htlcIsDust( + if HtlcIsDust( cb.chanState.ChanType, true, isOurs, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, ) { diff --git a/lnwallet/test_utils.go b/lnwallet/test_utils.go index bd048b2c0..dcd9e202d 100644 --- a/lnwallet/test_utils.go +++ b/lnwallet/test_utils.go @@ -93,6 +93,9 @@ var ( 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22, 0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09, } + + aliceDustLimit = btcutil.Amount(200) + bobDustLimit = btcutil.Amount(1300) ) // CreateTestChannels creates to fully populated channels to be used within @@ -112,8 +115,6 @@ func CreateTestChannels(chanType channeldb.ChannelType) ( } channelBal := channelCapacity / 2 - aliceDustLimit := btcutil.Amount(200) - bobDustLimit := btcutil.Amount(1300) csvTimeoutAlice := uint32(5) csvTimeoutBob := uint32(4)