lnwallet: make selectCoinsAndChange return selected coins

This makes the method independent of the ChannelContribution struct.

We also add a function closure to the return of selectCoinsAndChange,
that let is unlock the selected output in case of error.

lnwallet/wallet.go

@@ -477,15 +477,16 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg
 	if req.LocalFundingAmt != 0 {
 		// Coin selection is done on the basis of sat/kw, so we'll use
 		// the fee rate passed in to perform coin selection.
-		err := l.selectCoinsAndChange(
+		selected, err := l.selectCoinsAndChange(
 			req.FundingFeePerKw, req.LocalFundingAmt, req.MinConfs,
-			reservation.ourContribution,
 		)
 		if err != nil {
 			req.err <- err
 			req.resp <- nil
 			return
 		}
+		reservation.ourContribution.Inputs = selected.coins
+		reservation.ourContribution.ChangeOutputs = selected.change
 	}
 
 	// Next, we'll grab a series of keys from the wallet which will be used
@@ -1273,14 +1274,21 @@ func (l *LightningWallet) WithCoinSelectLock(f func() error) error {
 	return f()
 }
 
+// coinSelection holds the result from selectCoinsAndChange.
+type coinSelection struct {
+	coins       []*wire.TxIn
+	change      []*wire.TxOut
+	unlockCoins func()
+}
+
 // selectCoinsAndChange performs coin selection in order to obtain witness
-// outputs which sum to at least 'numCoins' amount of satoshis. If coin
+// outputs which sum to at least 'amt' amount of satoshis. If necessary,
-// selection is successful/possible, then the selected coins are available
+// a change address will also be generated. If coin selection is
-// within the passed contribution's inputs. If necessary, a change address will
+// successful/possible, then the selected coins and change outputs are
-// also be generated.
+// returned. This method locks the selected outputs, and a function closure to
+// unlock them in case of an error is returned.
 func (l *LightningWallet) selectCoinsAndChange(feeRate SatPerKWeight,
-	amt btcutil.Amount, minConfs int32,
+	amt btcutil.Amount, minConfs int32) (*coinSelection, error) {
-	contribution *ChannelContribution) error {
 
 	// We hold the coin select mutex while querying for outputs, and
 	// performing coin selection in order to avoid inadvertent double
@@ -1295,7 +1303,7 @@ func (l *LightningWallet) selectCoinsAndChange(feeRate SatPerKWeight,
 	// number of confirmations required.
 	coins, err := l.ListUnspentWitness(minConfs, math.MaxInt32)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// Perform coin selection over our available, unlocked unspent outputs
@@ -1303,13 +1311,34 @@ func (l *LightningWallet) selectCoinsAndChange(feeRate SatPerKWeight,
 	// requirements.
 	selectedCoins, changeAmt, err := coinSelect(feeRate, amt, coins)
 	if err != nil {
-		return err
+		return nil, err
+	}
+
+	// Record any change output(s) generated as a result of the coin
+	// selection, but only if the addition of the output won't lead to the
+	// creation of dust.
+	var changeOutputs []*wire.TxOut
+	if changeAmt != 0 && changeAmt > DefaultDustLimit() {
+		changeAddr, err := l.NewAddress(WitnessPubKey, true)
+		if err != nil {
+			return nil, err
+		}
+		changeScript, err := txscript.PayToAddrScript(changeAddr)
+		if err != nil {
+			return nil, err
+		}
+
+		changeOutputs = make([]*wire.TxOut, 1)
+		changeOutputs[0] = &wire.TxOut{
+			Value:    int64(changeAmt),
+			PkScript: changeScript,
+		}
 	}
 
 	// Lock the selected coins. These coins are now "reserved", this
 	// prevents concurrent funding requests from referring to and this
 	// double-spending the same set of coins.
-	contribution.Inputs = make([]*wire.TxIn, len(selectedCoins))
+	inputs := make([]*wire.TxIn, len(selectedCoins))
 	for i, coin := range selectedCoins {
 		outpoint := &coin.OutPoint
 		l.lockedOutPoints[*outpoint] = struct{}{}
@@ -1317,30 +1346,25 @@ func (l *LightningWallet) selectCoinsAndChange(feeRate SatPerKWeight,
 
 		// Empty sig script, we'll actually sign if this reservation is
 		// queued up to be completed (the other side accepts).
-		contribution.Inputs[i] = wire.NewTxIn(outpoint, nil, nil)
+		inputs[i] = wire.NewTxIn(outpoint, nil, nil)
 	}
 
-	// Record any change output(s) generated as a result of the coin
+	unlock := func() {
-	// selection, but only if the addition of the output won't lead to the
+		l.coinSelectMtx.Lock()
-	// creation of dust.
+		defer l.coinSelectMtx.Unlock()
-	if changeAmt != 0 && changeAmt > DefaultDustLimit() {
-		changeAddr, err := l.NewAddress(WitnessPubKey, true)
-		if err != nil {
-			return err
-		}
-		changeScript, err := txscript.PayToAddrScript(changeAddr)
-		if err != nil {
-			return err
-		}
 
-		contribution.ChangeOutputs = make([]*wire.TxOut, 1)
+		for _, coin := range selectedCoins {
-		contribution.ChangeOutputs[0] = &wire.TxOut{
+			outpoint := &coin.OutPoint
-			Value:    int64(changeAmt),
+			delete(l.lockedOutPoints, *outpoint)
-			PkScript: changeScript,
+			l.UnlockOutpoint(*outpoint)
 		}
 	}
 
-	return nil
+	return &coinSelection{
+		coins:       inputs,
+		change:      changeOutputs,
+		unlockCoins: unlock,
+	}, nil
 }
 
 // DeriveStateHintObfuscator derives the bytes to be used for obfuscating the