From fbe82c3531344a0422bc5454f82ed44c83e93124 Mon Sep 17 00:00:00 2001
From: Wilmer Paulino <wilmer.paulino@gmail.com>
Date: Wed, 29 Aug 2018 15:28:40 -0700
Subject: [PATCH] wtxmgr: check existing unspent outputs before adding the
 credit

In this commit, we resolve a lingering bug within the wallet where it's
possible that an output is added as unconfirmed credit after the fact
that it has already confirmed. This would lead to duplicate entries for
the same output within the wallet causing double spend transactions to
be crafted.
---
 wtxmgr/tx.go          |  6 ++++++
 wtxmgr/unconfirmed.go | 19 ++++++++++++++++---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/wtxmgr/tx.go b/wtxmgr/tx.go
index a9b9651..0b232ea 100644
--- a/wtxmgr/tx.go
+++ b/wtxmgr/tx.go
@@ -405,10 +405,16 @@ func (s *Store) AddCredit(ns walletdb.ReadWriteBucket, rec *TxRecord, block *Blo
 // duplicate (false).
 func (s *Store) addCredit(ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta, index uint32, change bool) (bool, error) {
 	if block == nil {
+		// If the outpoint that we should mark as credit already exists
+		// within the store, either as unconfirmed or confirmed, then we
+		// have nothing left to do and can exit.
 		k := canonicalOutPoint(&rec.Hash, index)
 		if existsRawUnminedCredit(ns, k) != nil {
 			return false, nil
 		}
+		if existsRawUnspent(ns, k) != nil {
+			return false, nil
+		}
 		v := valueUnminedCredit(btcutil.Amount(rec.MsgTx.TxOut[index].Value), change)
 		return true, putRawUnminedCredit(ns, k, v)
 	}
diff --git a/wtxmgr/unconfirmed.go b/wtxmgr/unconfirmed.go
index 812d2d2..37bf7e1 100644
--- a/wtxmgr/unconfirmed.go
+++ b/wtxmgr/unconfirmed.go
@@ -14,12 +14,25 @@ import (
 // insertMemPoolTx inserts the unmined transaction record.  It also marks
 // previous outputs referenced by the inputs as spent.
 func (s *Store) insertMemPoolTx(ns walletdb.ReadWriteBucket, rec *TxRecord) error {
-	v := existsRawUnmined(ns, rec.Hash[:])
-	if v != nil {
-		// TODO: compare serialized txs to ensure this isn't a hash collision?
+	// Check whether the transaction has already been added to the
+	// unconfirmed bucket.
+	if existsRawUnmined(ns, rec.Hash[:]) != nil {
+		// TODO: compare serialized txs to ensure this isn't a hash
+		// collision?
 		return nil
 	}
 
+	// Since transaction records within the store are keyed by their
+	// transaction _and_ block confirmation, we'll iterate through the
+	// transaction's outputs to determine if we've already seen them to
+	// prevent from adding this transaction to the unconfirmed bucket.
+	for i := range rec.MsgTx.TxOut {
+		k := canonicalOutPoint(&rec.Hash, uint32(i))
+		if existsRawUnspent(ns, k) != nil {
+			return nil
+		}
+	}
+
 	log.Infof("Inserting unconfirmed transaction %v", rec.Hash)
 	v, err := valueTxRecord(rec)
 	if err != nil {