chanfunding: export change amount calculation

We want to re-use the logic that determines what change amount is left
over depending on whether we add or don't add a change output to a
transaction, respecting the change output's dust limit.

lnwallet/chanfunding/coin_select.go

@@ -185,55 +185,95 @@ func CoinSelect(feeRate chainfee.SatPerKWeight, amt, dustLimit btcutil.Amount,
 			return nil, 0, err
 		}
 
-		// The difference between the selected amount and the amount
+		changeAmount, newAmtNeeded, err := CalculateChangeAmount(
-		// requested will be used to pay fees, and generate a change
+			totalSat, amt, requiredFeeNoChange,
-		// output with the remaining.
+			requiredFeeWithChange, dustLimit, changeType,
-		overShootAmt := totalSat - amt
+		)
-
-		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
-		}
-
-		// In case we would end up with a dust output if we created a
-		// change output, we instead just let the dust amount go to
-		// fees. Unless we want the change to go to an existing output,
-		// in that case we can increase that output value by any amount.
-		if changeAmt < dustLimit && changeType != ExistingChangeAddress {
-			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
+		// Need another round, the selected coins aren't enough to pay
+		// for the fees.
+		if newAmtNeeded != 0 {
+			amtNeeded = newAmtNeeded
+
+			continue
+		}
+
+		// Coin selection was successful.
+		return selectedUtxos, changeAmount, nil
 	}
 }
 
+// CalculateChangeAmount calculates the change amount being left over when the
+// given total amount of sats is provided as inputs for the required output
+// amount. The calculation takes into account that we might not want to add a
+// change output if the change amount is below the dust limit. The first amount
+// returned is the change amount. If that is non-zero, change is left over and
+// should be dealt with. The second amount, if non-zero, indicates that the
+// total input amount was just not enough to pay for the required amount and
+// fees and that more coins need to be selected.
+func CalculateChangeAmount(totalInputAmt, requiredAmt, requiredFeeNoChange,
+	requiredFeeWithChange, dustLimit btcutil.Amount,
+	changeType ChangeAddressType) (btcutil.Amount, btcutil.Amount, error) {
+
+	// This is just a sanity check to make sure the function is used
+	// correctly.
+	if changeType == ExistingChangeAddress &&
+		requiredFeeNoChange != requiredFeeWithChange {
+
+		return 0, 0, fmt.Errorf("when using existing change address, " +
+			"the fees for with or without change must be the same")
+	}
+
+	// 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 := totalInputAmt - requiredAmt
+
+	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:
+		return 0, requiredAmt + requiredFeeNoChange, nil
+
+	// 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
+	}
+
+	// In case we would end up with a dust output if we created a
+	// change output, we instead just let the dust amount go to
+	// fees. Unless we want the change to go to an existing output,
+	// in that case we can increase that output value by any amount.
+	if changeAmt < dustLimit && changeType != ExistingChangeAddress {
+		changeAmt = 0
+	}
+
+	// Sanity check the resulting output values to make sure we
+	// don't burn a great part to fees.
+	totalOut := requiredAmt + changeAmt
+	err := sanityCheckFee(totalOut, totalInputAmt-totalOut)
+	if err != nil {
+		return 0, 0, err
+	}
+
+	return changeAmt, 0, nil
+}
+
 // CoinSelectSubtractFees attempts to select coins such that we'll spend up to
 // amt in total after fees, adhering to the specified fee rate. The selected
 // coins, the final output and change values are returned.

lnwallet/chanfunding/coin_select_test.go

@@ -342,6 +342,106 @@ func TestCoinSelect(t *testing.T) {
 	}
 }
 
+// TestCalculateChangeAmount tests that the change amount calculation performs
+// correctly, taking into account the type of change output and whether we want
+// to create a change output in the first place.
+func TestCalculateChangeAmount(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name          string
+		totalInputAmt btcutil.Amount
+		requiredAmt   btcutil.Amount
+		feeNoChange   btcutil.Amount
+		feeWithChange btcutil.Amount
+		dustLimit     btcutil.Amount
+		changeType    ChangeAddressType
+
+		expectErr       string
+		expectChangeAmt btcutil.Amount
+		expectNeedMore  btcutil.Amount
+	}{{
+		// Coin selection returned a coin larger than the required
+		// amount, but still not enough to pay for the fees. This should
+		// trigger another round of coin selection with a larger
+		// required amount.
+		name:          "need to select more",
+		totalInputAmt: 500,
+		requiredAmt:   490,
+		feeNoChange:   12,
+
+		expectNeedMore: 502,
+	}, {
+		// We are using an existing change output, so we'll only want
+		// to make sure to select enough for a TX _without_ a change
+		// output added. Because we're using an existing output, the
+		// dust limit calculation should also be skipped.
+		name:          "sufficiently large for existing change output",
+		totalInputAmt: 500,
+		requiredAmt:   400,
+		feeNoChange:   10,
+		feeWithChange: 10,
+		dustLimit:     100,
+		changeType:    ExistingChangeAddress,
+
+		expectChangeAmt: 90,
+	}, {
+		name:          "sufficiently large for adding a change output",
+		totalInputAmt: 500,
+		requiredAmt:   300,
+		feeNoChange:   40,
+		feeWithChange: 50,
+		dustLimit:     100,
+
+		expectChangeAmt: 150,
+	}, {
+		name: "sufficiently large for tx without change " +
+			"amount",
+		totalInputAmt: 500,
+		requiredAmt:   460,
+		feeNoChange:   40,
+		feeWithChange: 50,
+
+		expectChangeAmt: 0,
+	}, {
+		name:          "fee percent too large",
+		totalInputAmt: 100,
+		requiredAmt:   50,
+		feeNoChange:   10,
+		feeWithChange: 45,
+		dustLimit:     5,
+
+		expectErr: "fee 0.00000045 BTC on total output value " +
+			"0.00000055",
+	}, {
+		name:          "invalid usage of function",
+		feeNoChange:   5,
+		feeWithChange: 10,
+		changeType:    ExistingChangeAddress,
+
+		expectErr: "fees for with or without change must be the same",
+	}}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(tt *testing.T) {
+			changeAmt, needMore, err := CalculateChangeAmount(
+				tc.totalInputAmt, tc.requiredAmt,
+				tc.feeNoChange, tc.feeWithChange, tc.dustLimit,
+				tc.changeType,
+			)
+
+			if tc.expectErr != "" {
+				require.ErrorContains(tt, err, tc.expectErr)
+				return
+			}
+
+			require.EqualValues(tt, tc.expectChangeAmt, changeAmt)
+			require.EqualValues(tt, tc.expectNeedMore, needMore)
+		})
+	}
+}
+
 // TestCoinSelectSubtractFees tests that we pick coins adding up to the
 // expected amount when creating a funding transaction, and that a change
 // output is created only when necessary.