Merge pull request #240 from SiaFoundation/nate/wallet-fix-funding

Fix renewal funding

internal/test/wallet.go

@@ -46,9 +46,9 @@ func (w *Wallet) SendSiacoins(outputs []types.SiacoinOutput) (txn types.Transact
 	}
 	defer release()
 	if err := w.SignTransaction(w.ChainManager().TipState(), &txn, toSign, types.CoveredFields{WholeTransaction: true}); err != nil {
-		return types.Transaction{}, fmt.Errorf("failed to sign transaction: %w", err)
+		return txn, fmt.Errorf("failed to sign transaction: %w", err)
 	} else if err := w.tp.AcceptTransactionSet([]types.Transaction{txn}); err != nil {
-		return types.Transaction{}, fmt.Errorf("failed to accept transaction set: %w", err)
+		return txn, fmt.Errorf("failed to accept transaction set: %w", err)
 	}
 	return txn, nil
 }

wallet/wallet.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"sort"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -18,6 +19,18 @@ import (
 	"go.uber.org/zap"
 )
 
+const (
+	// transactionDefragThreshold is the number of utxos at which the wallet
+	// will attempt to defrag itself by including small utxos in transactions.
+	transactionDefragThreshold = 30
+	// maxInputsForDefrag is the maximum number of inputs a transaction can
+	// have before the wallet will stop adding inputs
+	maxInputsForDefrag = 30
+	// maxDefragUTXOs is the maximum number of utxos that will be added to a
+	// transaction when defragging
+	maxDefragUTXOs = 10
+)
+
 // transaction sources indicate the source of a transaction. Transactions can
 // either be created by sending Siacoins between unlock hashes or they can be
 // created by consensus (e.g. a miner payout, a siafund claim, or a contract).
@@ -267,29 +280,68 @@ func (sw *SingleAddressWallet) FundTransaction(txn *types.Transaction, amount ty
 	if err != nil {
 		return nil, nil, err
 	}
-	var inputSum types.Currency
-	var fundingElements []SiacoinElement
+
+	// remove locked and spent outputs
+	usableUTXOs := utxos[:0]
 	for _, sce := range utxos {
 		if sw.locked[sce.ID] || sw.tpoolSpent[sce.ID] || sw.consensusLocked[sce.ID] {
 			continue
 		}
-		fundingElements = append(fundingElements, sce)
-		inputSum = inputSum.Add(sce.Value)
+		usableUTXOs = append(usableUTXOs, sce)
+	}
+
+	// sort by value, descending
+	sort.Slice(usableUTXOs, func(i, j int) bool {
+		return usableUTXOs[i].Value.Cmp(usableUTXOs[j].Value) > 0
+	})
+
+	// fund the transaction using the largest utxos first
+	var selected []SiacoinElement
+	var inputSum types.Currency
+	for i, sce := range usableUTXOs {
 		if inputSum.Cmp(amount) >= 0 {
+			usableUTXOs = usableUTXOs[i:]
 			break
 		}
+		selected = append(selected, sce)
+		inputSum = inputSum.Add(sce.Value)
 	}
+
+	// if the transaction can't be funded, return an error
 	if inputSum.Cmp(amount) < 0 {
 		return nil, nil, ErrNotEnoughFunds
-	} else if inputSum.Cmp(amount) > 0 {
+	}
+
+	// check if remaining utxos should be defragged
+	txnInputs := len(txn.SiacoinInputs) + len(selected)
+	if len(usableUTXOs) > transactionDefragThreshold && txnInputs < maxInputsForDefrag {
+		// add the smallest utxos to the transaction
+		defraggable := usableUTXOs
+		if len(defraggable) > maxDefragUTXOs {
+			defraggable = defraggable[len(defraggable)-maxDefragUTXOs:]
+		}
+		for i := len(defraggable) - 1; i >= 0; i-- {
+			if txnInputs >= maxInputsForDefrag {
+				break
+			}
+
+			sce := defraggable[i]
+			selected = append(selected, sce)
+			inputSum = inputSum.Add(sce.Value)
+			txnInputs++
+		}
+	}
+
+	// add a change output if necessary
+	if inputSum.Cmp(amount) > 0 {
 		txn.SiacoinOutputs = append(txn.SiacoinOutputs, types.SiacoinOutput{
 			Value:   inputSum.Sub(amount),
 			Address: sw.addr,
 		})
 	}
 
-	toSign := make([]types.Hash256, len(fundingElements))
-	for i, sce := range fundingElements {
+	toSign := make([]types.Hash256, len(selected))
+	for i, sce := range selected {
 		txn.SiacoinInputs = append(txn.SiacoinInputs, types.SiacoinInput{
 			ParentID:         types.SiacoinOutputID(sce.ID),
 			UnlockConditions: types.StandardUnlockConditions(sw.priv.PublicKey()),

wallet/wallet_test.go

@@ -1,6 +1,8 @@
 package wallet_test
 
 import (
+	"encoding/json"
+	"sort"
 	"testing"
 	"time"
 
@@ -90,12 +92,14 @@ func TestWallet(t *testing.T) {
 			Address: w.Address(),
 		}
 	}
-	if _, err = w.SendSiacoins(splitOutputs); err != nil {
+	if txn, err := w.SendSiacoins(splitOutputs); err != nil {
+		buf, _ := json.MarshalIndent(txn, "", "  ")
+		t.Log(string(buf))
 		t.Fatal(err)
 	}
 
 	time.Sleep(250 * time.Millisecond) // sleep for tpool sync
-	// check that the wallet's spendable balance and unconfiremed balance are
+	// check that the wallet's spendable balance and unconfirmed balance are
 	// correct
 	spendable, balance, unconfirmed, err := w.Balance()
 	if err != nil {
@@ -325,3 +329,79 @@ func TestWalletReset(t *testing.T) {
 		t.Fatal("expected zero balance")
 	}
 }
+
+func TestWalletUTXOSelection(t *testing.T) {
+	log := zaptest.NewLogger(t)
+	w, err := test.NewWallet(types.GeneratePrivateKey(), t.TempDir(), log.Named("wallet"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer w.Close()
+
+	_, balance, _, err := w.Balance()
+	if err != nil {
+		t.Fatal(err)
+	} else if !balance.Equals(types.ZeroCurrency) {
+		t.Fatalf("expected zero balance, got %v", balance)
+	}
+
+	// mine until the wallet has 100 mature outputs
+	if err := w.MineBlocks(w.Address(), 100+int(stypes.MaturityDelay)); err != nil {
+		t.Fatal(err)
+	}
+
+	time.Sleep(time.Second) // sleep for consensus sync
+
+	// check that the expected utxos were used
+	utxos, err := w.Store().UnspentSiacoinElements()
+	if err != nil {
+		t.Fatal(err)
+	} else if len(utxos) != 100 {
+		t.Fatalf("expected 100 utxos, got %v", len(utxos))
+	}
+	sort.Slice(utxos, func(i, j int) bool {
+		return utxos[i].Value.Cmp(utxos[j].Value) > 0
+	})
+
+	// send a transaction to the burn address
+	sendAmount := types.Siacoins(10)
+	minerFee := types.Siacoins(1)
+	txn := types.Transaction{
+		MinerFees: []types.Currency{minerFee},
+		SiacoinOutputs: []types.SiacoinOutput{
+			{Address: types.VoidAddress, Value: sendAmount},
+		},
+	}
+
+	fundAmount := sendAmount.Add(minerFee)
+	toSign, release, err := w.FundTransaction(&txn, fundAmount)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer release()
+
+	if len(txn.SiacoinInputs) != 11 {
+		t.Fatalf("expected 10 additional defrag inputs, got %v", len(toSign)-1)
+	} else if len(txn.SiacoinOutputs) != 2 {
+		t.Fatalf("expected a change output, got %v", len(txn.SiacoinOutputs))
+	}
+
+	// check that the expected UTXOs were added
+	spent := []wallet.SiacoinElement{utxos[0]}
+	rem := utxos[90:]
+	for i := len(rem) - 1; i >= 0; i-- {
+		spent = append(spent, rem[i])
+	}
+
+	for i := range txn.SiacoinInputs {
+		if txn.SiacoinInputs[i].ParentID != spent[i].ID {
+			t.Fatalf("expected input %v to spend %v, got %v", i, spent[i].ID, txn.SiacoinInputs[i].ParentID)
+		}
+	}
+
+	if err := w.SignTransaction(w.TipState(), &txn, toSign, types.CoveredFields{WholeTransaction: true}); err != nil {
+		t.Fatal(err)
+	} else if err := w.TPool().AcceptTransactionSet([]types.Transaction{txn}); err != nil {
+		t.Fatal(err)
+	}
+}