[release-v1.6] Use extra split output rather than selecting random UTXOs

After we create an extra split output to deal with UTXO contention in
a VSP ticket purchase, there is no need to perform UTXO selection
again to reserve an output for a single VSP fee payment. Add an
unexported field to the purchase ticket parameters so that upon
reentry, this output can be used directly.

wallet/createtx.go

@@ -1434,21 +1434,30 @@ func (w *Wallet) purchaseTickets(ctx context.Context, op errors.Op,
 				}
 			}
 		}()
+		if req.extraSplitOutput != nil {
+			vspFeeCredits = make([][]Input, 1)
+			vspFeeCredits[0] = []Input{*req.extraSplitOutput}
+			op := &req.extraSplitOutput.OutPoint
+			w.LockOutpoint(&op.Hash, op.Index)
+		}
 		var lowBalance bool
 		for i := 0; i < req.Count; i++ {
-			credits, err := w.ReserveOutputsForAmount(ctx, req.SourceAccount, fee,
-				req.MinConf)
-			if errors.Is(err, errors.InsufficientBalance) {
-				lowBalance = true
-				break
-			}
-			if err != nil {
-				log.Errorf("ReserveOutputsForAmount failed: %v", err)
-				return nil, err
+			if req.extraSplitOutput == nil {
+				credits, err := w.ReserveOutputsForAmount(ctx,
+					req.SourceAccount, fee, req.MinConf)
+
+				if errors.Is(err, errors.InsufficientBalance) {
+					lowBalance = true
+					break
+				}
+				if err != nil {
+					log.Errorf("ReserveOutputsForAmount failed: %v", err)
+					return nil, err
+				}
+				vspFeeCredits = append(vspFeeCredits, credits)
 			}
-			vspFeeCredits = append(vspFeeCredits, credits)
 
-			credits, err = w.ReserveOutputsForAmount(ctx, req.SourceAccount,
+			credits, err := w.ReserveOutputsForAmount(ctx, req.SourceAccount,
 				ticketPrice, req.MinConf)
 			if errors.Is(err, errors.InsufficientBalance) {
 				lowBalance = true
@@ -1472,32 +1481,52 @@ func (w *Wallet) purchaseTickets(ctx context.Context, op errors.Op,
 			// UTXOs and the tickets that can be purchased, UTXOs
 			// which were selected for paying VSP fees are instead
 			// allocated towards purchasing tickets.  We sort the
-			// UTXOs picked for fees by decreasing amounts and
-			// incrementally reserve them for ticket purchases while
-			// reducing the total number of fees (and therefore
-			// tickets) that will be purchased.
-			sort.Slice(vspFeeCredits, func(i, j int) bool {
-				return total(vspFeeCredits[i]) > total(vspFeeCredits[j])
+			// UTXOs picked for fees and tickets by decreasing
+			// amounts and incrementally reserve them for ticket
+			// purchases while reducing the total number of fees
+			// (and therefore tickets) that will be purchased.  The
+			// final UTXOs chosen for ticket purchases must be
+			// unlocked for UTXO selection to work, while all inputs
+			// for fee payments must be locked.
+			credits := vspFeeCredits[:len(vspFeeCredits):len(vspFeeCredits)]
+			credits = append(credits, ticketCredits...)
+			sort.Slice(credits, func(i, j int) bool {
+				return total(credits[i]) > total(credits[j])
 			})
+			if len(credits) == 0 {
+				return nil, errors.E(errors.InsufficientBalance)
+			}
+			if req.Count > len(credits)-1 {
+				req.Count = len(credits) - 1
+			}
 			var freedBalance int64
-			req.Count = len(vspFeeCredits)
 			extraSplit := true
 			for req.Count > 1 {
-				for _, in := range vspFeeCredits[0] {
-					freedBalance += in.PrevOut.Value
-					w.UnlockOutpoint(&in.OutPoint.Hash, in.OutPoint.Index)
+				for _, c := range credits[0] {
+					freedBalance += c.PrevOut.Value
+					w.UnlockOutpoint(&c.OutPoint.Hash, c.OutPoint.Index)
 				}
-				req.Count--
-				vspFeeCredits = vspFeeCredits[1:]
+				credits = credits[1:]
 				// XXX this is a bad estimate because it doesn't
 				// consider the transaction fees
-				if int64(ticketPrice)*int64(req.Count) < freedBalance {
+				if freedBalance > int64(ticketPrice)*int64(req.Count) {
 					extraSplit = false
 					break
 				}
+				req.Count--
+			}
+			vspFeeCredits = credits
+			for _, c := range vspFeeCredits {
+				for i := range c {
+					w.LockOutpoint(&c[i].OutPoint.Hash, c[i].OutPoint.Index)
+				}
 			}
-			if req.Count == 1 && extraSplit {
-				remaining := total(vspFeeCredits[0])
+
+			if req.Count < 2 && extraSplit {
+				var remaining int64
+				if len(vspFeeCredits) > 0 {
+					remaining = total(vspFeeCredits[0])
+				}
 				if int64(ticketPrice) > remaining { // XXX still a bad estimate
 					return nil, errors.E(errors.InsufficientBalance)
 				}

wallet/wallet.go

@@ -1554,6 +1554,15 @@ type PurchaseTicketsRequest struct {
 	// VSPFeePaymentProcess processes the payment of the vsp fee and returns
 	// the paid fee tx.
 	VSPFeePaymentProcess func(context.Context, *chainhash.Hash, *wire.MsgTx) error
+
+	// extraSplitOutput is an additional transaction output created during
+	// UTXO contention, to be used as the input to pay a VSP fee
+	// transaction, in order that both VSP fees and a single ticket purchase
+	// may be created by spending distinct outputs.  After purchaseTickets
+	// reentry, this output is locked and UTXO selection is only used to
+	// fund the split transaction for a ticket purchase, without reserving
+	// any additional outputs to pay the VSP fee.
+	extraSplitOutput *Input
 }
 
 // PurchaseTicketsResponse describes the response for purchasing tickets request.
@@ -1640,6 +1649,19 @@ func (w *Wallet) PurchaseTickets(ctx context.Context, n NetworkBackend,
 	}
 
 	req.MinConf = 0
+	req.Count = 1
+	var index uint32 = 0
+	if a.atx.ChangeIndex == 0 {
+		index = 1
+	}
+	req.extraSplitOutput = &Input{
+		OutPoint: wire.OutPoint{
+			Hash:  a.atx.Tx.TxHash(),
+			Index: index,
+			Tree:  0,
+		},
+		PrevOut: *a.atx.Tx.TxOut[index],
+	}
 	return w.purchaseTickets(ctx, op, n, req)
 }