chanfunding: fee estimation based on change output in CoinSelect

Add a dust-limit to `CoinSelect` and let the fee estimation consider if a change output is needed or not, assuring that if the change is below the dust-limit it will go towards the fee instead.

lnwallet/chanfunding/coin_select.go

@@ -122,7 +122,7 @@ func sanityCheckFee(totalOut, fee btcutil.Amount) error {
 // change output to fund amt satoshis, adhering to the specified fee rate. The
 // specified fee rate should be expressed in sat/kw for coin selection to
 // function properly.
-func CoinSelect(feeRate chainfee.SatPerKWeight, amt btcutil.Amount,
+func CoinSelect(feeRate chainfee.SatPerKWeight, amt, dustLimit btcutil.Amount,
 	coins []Coin) ([]Coin, btcutil.Amount, error) {
 
 	amtNeeded := amt
@@ -136,29 +136,55 @@ func CoinSelect(feeRate chainfee.SatPerKWeight, amt btcutil.Amount,
 
 		// Obtain fee estimates both with and without using a change
 		// output.
-		_, requiredFeeWithChange, err := calculateFees(
+		requiredFeeNoChange, requiredFeeWithChange, err := calculateFees(
 			selectedUtxos, feeRate,
 		)
 		if err != nil {
 			return nil, 0, err
 		}
+
 		// The difference between the selected amount and the amount
 		// requested will be used to pay fees, and generate a change
 		// output with the remaining.
 		overShootAmt := totalSat - amt
 
-		// Based on the estimated size and fee rate, if the excess
-		// amount isn't enough to pay fees, then increase the requested
-		// coin amount by the estimate required fee, performing another
-		// round of coin selection.
-		if overShootAmt < requiredFeeWithChange {
-			amtNeeded = amt + requiredFeeWithChange
+		var changeAmt btcutil.Amount
+
+		switch {
+
+		// If the excess amount isn't enough to pay for fees based on
+		// fee rate and estimated size without using a change output,
+		// then increase the requested coin amount by the estimate
+		// required fee without using change, performing another round
+		// of coin selection.
+		case overShootAmt < requiredFeeNoChange:
+			amtNeeded = amt + requiredFeeNoChange
 			continue
+
+		// If sufficient funds were selected to cover the fee required
+		// to include a change output, the remainder will be our change
+		// amount.
+		case overShootAmt > requiredFeeWithChange:
+			changeAmt = overShootAmt - requiredFeeWithChange
+
+		// Otherwise we have selected enough to pay for a tx without a
+		// change output.
+		default:
+			changeAmt = 0
+
 		}
 
-		// If the fee is sufficient, then calculate the size of the
-		// change output.
-		changeAmt := overShootAmt - requiredFeeWithChange
+		if changeAmt < dustLimit {
+			changeAmt = 0
+		}
+
+		// Sanity check the resulting output values to make sure we
+		// don't burn a great part to fees.
+		totalOut := amt + changeAmt
+		err = sanityCheckFee(totalOut, totalSat-totalOut)
+		if err != nil {
+			return nil, 0, err
+		}
 
 		return selectedUtxos, changeAmt, nil
 	}

lnwallet/chanfunding/coin_select_test.go

@@ -143,7 +143,7 @@ func TestCoinSelect(t *testing.T) {
 	t.Parallel()
 
 	const feeRate = chainfee.SatPerKWeight(100)
-	const dust = btcutil.Amount(100)
+	const dustLimit = btcutil.Amount(1000)
 
 	type testCase struct {
 		name        string
@@ -197,7 +197,7 @@ func TestCoinSelect(t *testing.T) {
 		{
 			// We have a 1 BTC input, and want to create an output
 			// as big as possible, such that the remaining change
-			// will be dust.
+			// would be dust but instead goes to fees.
 			name: "dust change",
 			coins: []Coin{
 				{
@@ -208,41 +208,53 @@ func TestCoinSelect(t *testing.T) {
 				},
 			},
 			// We tune the output value by subtracting the expected
-			// fee and a small dust amount.
-			outputValue: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, true) - dust,
+			// fee and the dustlimit.
+			outputValue: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, false) - dustLimit,
 
 			expectedInput: []btcutil.Amount{
 				1 * btcutil.SatoshiPerBitcoin,
 			},
 
-			// Change will the dust.
-			expectedChange: dust,
+			// Change must be zero.
+			expectedChange: 0,
 		},
 		{
-			// We have a 1 BTC input, and want to create an output
-			// as big as possible, such that there is nothing left
-			// for change.
-			name: "no change",
+			// We got just enough funds to create a change output above the
+			// dust limit.
+			name: "change right above dustlimit",
 			coins: []Coin{
 				{
 					TxOut: wire.TxOut{
 						PkScript: p2wkhScript,
-						Value:    1 * btcutil.SatoshiPerBitcoin,
+						Value:    int64(fundingFee(feeRate, 1, true) + 2*(dustLimit+1)),
 					},
 				},
 			},
-			// We tune the output value to be the maximum amount
-			// possible, leaving just enough for fees.
-			outputValue: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, true),
+			// We tune the output value to be just above the dust limit.
+			outputValue: dustLimit + 1,
 
 			expectedInput: []btcutil.Amount{
-				1 * btcutil.SatoshiPerBitcoin,
+				fundingFee(feeRate, 1, true) + 2*(dustLimit+1),
 			},
-			// We have just enough left to pay the fee, so there is
-			// nothing left for change.
-			// TODO(halseth): currently coinselect estimates fees
-			// assuming a change output.
-			expectedChange: 0,
+
+			// After paying for the fee the change output should be just above
+			// the dust limit.
+			expectedChange: dustLimit + 1,
+		},
+		{
+			// If more than 20% of funds goes to fees, it should fail.
+			name: "high fee",
+			coins: []Coin{
+				{
+					TxOut: wire.TxOut{
+						PkScript: p2wkhScript,
+						Value:    int64(5 * fundingFee(feeRate, 1, false)),
+					},
+				},
+			},
+			outputValue: 4 * fundingFee(feeRate, 1, false),
+
+			expectErr: true,
 		},
 	}
 
@@ -252,7 +264,7 @@ func TestCoinSelect(t *testing.T) {
 			t.Parallel()
 
 			selected, changeAmt, err := CoinSelect(
-				feeRate, test.outputValue, test.coins,
+				feeRate, test.outputValue, dustLimit, test.coins,
 			)
 			if !test.expectErr && err != nil {
 				t.Fatalf(err.Error())

lnwallet/chanfunding/wallet_assembler.go

@@ -289,9 +289,10 @@ func (w *WalletAssembler) ProvisionChannel(r *Request) (Intent, error) {
 		// Otherwise do a normal coin selection where we target a given
 		// funding amount.
 		default:
+			dustLimit := w.cfg.DustLimit
 			localContributionAmt = r.LocalAmt
 			selectedCoins, changeAmt, err = CoinSelect(
-				r.FeeRate, r.LocalAmt, coins,
+				r.FeeRate, r.LocalAmt, dustLimit, coins,
 			)
 			if err != nil {
 				return err