Improved input selection to avoid case of selecting a set of inputs w…

…hich is too small to pay the transaction including fee
pkt-cash · Oct 31, 2021 · 4bd8bb9 · 4bd8bb9
1 parent f9bad7a
commit 4bd8bb9
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 32 deletions.
diff --git a/pktwallet/wallet/createtx.go b/pktwallet/wallet/createtx.go
@@ -18,6 +18,7 @@ import (
 	"github.com/pkt-cash/pktd/btcutil/er"
 	"github.com/pkt-cash/pktd/pktlog/log"
 	"github.com/pkt-cash/pktd/pktwallet/waddrmgr"
+	"github.com/pkt-cash/pktd/pktwallet/wallet/enough"
 	"github.com/pkt-cash/pktd/pktwallet/wallet/txauthor"
 	"github.com/pkt-cash/pktd/pktwallet/wallet/txrules"
 	"github.com/pkt-cash/pktd/pktwallet/walletdb"
@@ -137,19 +138,9 @@ func (w *Wallet) txToOutputs(txr CreateTxReq) (tx *txauthor.AuthoredTx, err er.R
 		return nil, err
 	}
 
-	var sweepOutput *wire.TxOut
-	var needAmount btcutil.Amount
-	for _, out := range txr.Outputs {
-		needAmount += btcutil.Amount(out.Value)
-		if out.Value == 0 {
-			sweepOutput = out
-		}
-	}
-	if sweepOutput != nil {
-		needAmount = 0
-	}
+	isEnough := enough.MkIsEnough(txr.Outputs, txr.FeeSatPerKB)
 	eligibleOuts, err := w.findEligibleOutputs(
-		dbtx, needAmount, txr.InputAddresses, txr.Minconf, bs,
+		dbtx, isEnough, txr.InputAddresses, txr.Minconf, bs,
 		txr.InputMinHeight, txr.InputComparator, txr.MaxInputs)
 	if err != nil {
 		return nil, err
@@ -398,7 +389,7 @@ type eligibleOutputs struct {
 
 func (w *Wallet) findEligibleOutputs(
 	dbtx walletdb.ReadTx,
-	needAmount btcutil.Amount,
+	isEnough enough.IsEnough,
 	fromAddresses *[]btcutil.Address,
 	minconf int32,
 	bs *waddrmgr.BlockStamp,
@@ -498,16 +489,12 @@ func (w *Wallet) findEligibleOutputs(
 		}
 		ha.credits.Put(output, nil)
 		ha.amount += output.Amount
-		if needAmount == 0 {
-			// We're sweeping the wallet
-		} else if ha.amount < needAmount {
-			// We need more coins
-		} else {
+		if isEnough.WellIsIt(ha.credits.Size(), ha.isSegwit, ha.amount) {
 			worst := ha.credits.Right().Key.(*wtxmgr.Credit)
 			if worst == nil {
 				panic("findEligibleOutputs: worst == nil")
 			}
-			if ha.amount-worst.Amount >= needAmount {
+			if isEnough.WellIsIt(ha.credits.Size()-1, ha.isSegwit, ha.amount-worst.Amount) {
 				// Our amount is still fine even if we drop the worst credit
 				// so we'll drop it and continue traversing to find the best outputs
 				ha.credits.Remove(worst)
@@ -526,7 +513,7 @@ func (w *Wallet) findEligibleOutputs(
 
 		if !ha.overLimit(maxInputs) {
 			// We don't have too many inputs
-		} else if needAmount == 0 && inputComparator == nil {
+		} else if isEnough.IsSweeping() && inputComparator == nil {
 			// We're sweeping the wallet with no ordering specified
 			// This means we should just short-circuit with a winner
 			winner = ha
@@ -552,7 +539,7 @@ func (w *Wallet) findEligibleOutputs(
 		// we don't short circuit early so we might have a winner on our hands but not
 		// know it.
 		for _, ac := range haveAmounts {
-			if ac.amount >= needAmount {
+			if isEnough.WellIsIt(ac.credits.Size(), ac.isSegwit, ac.amount) {
 				winner = ac
 			}
 		}
@@ -603,10 +590,10 @@ func (w *Wallet) findEligibleOutputs(
 			out.unusedCount++
 			wasOver = true
 		}
-		if needAmount == 0 && !wasOver {
+		if isEnough.IsSweeping() && !wasOver {
 			// if we were never over the limit and we're sweeping multiple addresses,
 			// lets go around and get another address
-		} else if outAc.amount > needAmount {
+		} else if isEnough.WellIsIt(outAc.credits.Size(), outAc.isSegwit, outAc.amount) {
 			// We have enough money to make the tx
 			// We'll just iterate over the other entries to make unusedAmt and unusedCount correct
 			done = true

diff --git a/pktwallet/wallet/enough/isenough.go b/pktwallet/wallet/enough/isenough.go
@@ -0,0 +1,63 @@
+package enough
+
+import (
+	"github.com/pkt-cash/pktd/btcutil"
+	"github.com/pkt-cash/pktd/pktwallet/wallet/internal/txsizes"
+	"github.com/pkt-cash/pktd/pktwallet/wallet/txrules"
+	"github.com/pkt-cash/pktd/wire"
+)
+
+type IsEnough struct {
+	sweeping      bool
+	baseSize      int
+	sizeOneSegwit int
+	sizeOneLegacy int
+	feePerKb      btcutil.Amount
+	needed        btcutil.Amount
+}
+
+func MkIsEnough(txOutputs []*wire.TxOut, feePerKb btcutil.Amount) IsEnough {
+	sweepOutput := GetSweepOutput(txOutputs)
+	needed := btcutil.Amount(0)
+	for _, o := range txOutputs {
+		needed += btcutil.Amount(o.Value)
+	}
+	// We're going to start with the base tx size for fee estimation
+	// Then we'll take the size with 1 input in order to be able to estimate fee per input
+	baseSize := txsizes.EstimateVirtualSize(0, 0, 0, txOutputs, true)
+	return IsEnough{
+		sweeping:      sweepOutput != nil,
+		sizeOneSegwit: txsizes.EstimateVirtualSize(0, 1, 0, txOutputs, true) - baseSize,
+		sizeOneLegacy: txsizes.EstimateVirtualSize(1, 0, 0, txOutputs, true) - baseSize,
+		baseSize:      baseSize,
+		feePerKb:      feePerKb,
+		needed:        needed,
+	}
+}
+func (ii *IsEnough) IsSweeping() bool {
+	return ii.sweeping
+}
+func (ii *IsEnough) WellIsIt(inputCount int, segwit bool, amt btcutil.Amount) bool {
+	if ii.sweeping {
+	} else if amt < ii.needed {
+	} else {
+		perInput := ii.sizeOneSegwit
+		if !segwit {
+			perInput = ii.sizeOneLegacy
+		}
+		size := ii.baseSize + perInput*inputCount
+		fee := txrules.FeeForSerializeSize(ii.feePerKb, size)
+		return amt > ii.needed+fee
+	}
+	return false
+}
+
+func GetSweepOutput(outs []*wire.TxOut) *wire.TxOut {
+	var sweepOutput *wire.TxOut
+	for _, out := range outs {
+		if out.Value == 0 {
+			sweepOutput = out
+		}
+	}
+	return sweepOutput
+}
diff --git a/pktwallet/wallet/txauthor/author.go b/pktwallet/wallet/txauthor/author.go
@@ -16,6 +16,7 @@ import (
 
 	"github.com/pkt-cash/pktd/btcutil"
 	"github.com/pkt-cash/pktd/chaincfg"
+	"github.com/pkt-cash/pktd/pktwallet/wallet/enough"
 	"github.com/pkt-cash/pktd/pktwallet/wallet/txrules"
 	"github.com/pkt-cash/pktd/txscript"
 	"github.com/pkt-cash/pktd/wire"
@@ -80,15 +81,7 @@ func NewUnsignedTransaction(outputs []*wire.TxOut, relayFeePerKb btcutil.Amount,
 	estimatedSize := txsizes.EstimateVirtualSize(0, 1, 0, outputs, true)
 	targetFee := txrules.FeeForSerializeSize(relayFeePerKb, estimatedSize)
 
-	// If one of the outputs has a value of zero, this means we want to sweep everything
-	// except for fees to that output.
-	var sweepTo *wire.TxOut
-	for _, out := range outputs {
-		if out.Value == 0 {
-			sweepTo = out
-		}
-	}
-
+	sweepTo := enough.GetSweepOutput(outputs)
 	for {
 		synthTargetAmount := targetAmount + targetFee
 		if sweepTo != nil {