wtxmgr: store multiple spending tx hashes for outpoints

In this commit, we modify the way we store spending transaction hashes
for unconfirmed spends. Now, rather than only keeping track of one
possible unconfirmed spend, we track multiple in order to ensure we
properly handle transaction replacements, like in the case of RBF,
double spends, etc. With this in, the double spent tests recently added
should now pass.

wtxmgr/db.go

@@ -1214,12 +1214,18 @@ func (it *unminedCreditIterator) reposition(txHash *chainhash.Hash, index uint32
 //
 //   [0:32]   Transaction hash (32 bytes)
 
+// putRawUnminedInput maintains a list of unmined transaction hashes that have
+// spent an outpoint. Each entry in the bucket is keyed by the outpoint being
+// spent.
 func putRawUnminedInput(ns walletdb.ReadWriteBucket, k, v []byte) error {
-	err := ns.NestedReadWriteBucket(bucketUnminedInputs).Put(k, v)
+	spendTxHashes := ns.NestedReadBucket(bucketUnminedInputs).Get(k)
+	spendTxHashes = append(spendTxHashes, v...)
+	err := ns.NestedReadWriteBucket(bucketUnminedInputs).Put(k, spendTxHashes)
 	if err != nil {
 		str := "failed to put unmined input"
 		return storeError(ErrDatabase, str, err)
 	}
+
 	return nil
 }
 
@@ -1227,6 +1233,26 @@ func existsRawUnminedInput(ns walletdb.ReadBucket, k []byte) (v []byte) {
 	return ns.NestedReadBucket(bucketUnminedInputs).Get(k)
 }
 
+// fetchUnminedInputSpendTxHashes fetches the list of unmined transactions that
+// spend the serialized outpoint.
+func fetchUnminedInputSpendTxHashes(ns walletdb.ReadBucket, k []byte) []chainhash.Hash {
+	rawSpendTxHashes := ns.NestedReadBucket(bucketUnminedInputs).Get(k)
+	if rawSpendTxHashes == nil {
+		return nil
+	}
+
+	// Each transaction hash is 32 bytes.
+	spendTxHashes := make([]chainhash.Hash, 0, len(rawSpendTxHashes)/32)
+	for len(rawSpendTxHashes) > 0 {
+		var spendTxHash chainhash.Hash
+		copy(spendTxHash[:], rawSpendTxHashes[:32])
+		spendTxHashes = append(spendTxHashes, spendTxHash)
+		rawSpendTxHashes = rawSpendTxHashes[32:]
+	}
+
+	return spendTxHashes
+}
+
 func deleteRawUnminedInput(ns walletdb.ReadWriteBucket, k []byte) error {
 	err := ns.NestedReadWriteBucket(bucketUnminedInputs).Delete(k)
 	if err != nil {

wtxmgr/tx.go

@@ -664,11 +664,21 @@ func (s *Store) rollback(ns walletdb.ReadWriteBucket, height int32) error {
 
 	for _, op := range coinBaseCredits {
 		opKey := canonicalOutPoint(&op.Hash, op.Index)
-		unminedKey := existsRawUnminedInput(ns, opKey)
-		if unminedKey != nil {
-			unminedVal := existsRawUnmined(ns, unminedKey)
+		unminedSpendTxHashKeys := fetchUnminedInputSpendTxHashes(ns, opKey)
+		for _, unminedSpendTxHashKey := range unminedSpendTxHashKeys {
+			unminedVal := existsRawUnmined(ns, unminedSpendTxHashKey[:])
+
+			// If the spending transaction spends multiple outputs
+			// from the same transaction, we'll find duplicate
+			// entries within the store, so it's possible we're
+			// unable to find it if the conflicts have already been
+			// removed in a previous iteration.
+			if unminedVal == nil {
+				continue
+			}
+
 			var unminedRec TxRecord
-			copy(unminedRec.Hash[:], unminedKey) // Silly but need an array
+			unminedRec.Hash = unminedSpendTxHashKey
 			err = readRawTxRecord(&unminedRec.Hash, unminedVal, &unminedRec)
 			if err != nil {
 				return err

wtxmgr/unconfirmed.go

@@ -53,25 +53,37 @@ func (s *Store) removeDoubleSpends(ns walletdb.ReadWriteBucket, rec *TxRecord) e
 	for _, input := range rec.MsgTx.TxIn {
 		prevOut := &input.PreviousOutPoint
 		prevOutKey := canonicalOutPoint(&prevOut.Hash, prevOut.Index)
-		doubleSpendHash := existsRawUnminedInput(ns, prevOutKey)
-		if doubleSpendHash != nil {
+
+		doubleSpendHashes := fetchUnminedInputSpendTxHashes(ns, prevOutKey)
+		for _, doubleSpendHash := range doubleSpendHashes {
+			doubleSpendVal := existsRawUnmined(ns, doubleSpendHash[:])
+
+			// If the spending transaction spends multiple outputs
+			// from the same transaction, we'll find duplicate
+			// entries within the store, so it's possible we're
+			// unable to find it if the conflicts have already been
+			// removed in a previous iteration.
+			if doubleSpendVal == nil {
+				continue
+			}
+
 			var doubleSpend TxRecord
-			doubleSpendVal := existsRawUnmined(ns, doubleSpendHash)
-			copy(doubleSpend.Hash[:], doubleSpendHash) // Silly but need an array
-			err := readRawTxRecord(&doubleSpend.Hash, doubleSpendVal,
-				&doubleSpend)
+			doubleSpend.Hash = doubleSpendHash
+			err := readRawTxRecord(
+				&doubleSpend.Hash, doubleSpendVal, &doubleSpend,
+			)
 			if err != nil {
 				return err
 			}
 
 			log.Debugf("Removing double spending transaction %v",
 				doubleSpend.Hash)
-			err = s.removeConflict(ns, &doubleSpend)
-			if err != nil {
+			if err := s.removeConflict(ns, &doubleSpend); err != nil {
 				return err
 			}
 		}
 	}
+
 	return nil
 }
 
@@ -83,14 +95,23 @@ func (s *Store) removeConflict(ns walletdb.ReadWriteBucket, rec *TxRecord) error
 	// For each potential credit for this record, each spender (if any) must
 	// be recursively removed as well.  Once the spenders are removed, the
 	// credit is deleted.
-	numOuts := uint32(len(rec.MsgTx.TxOut))
-	for i := uint32(0); i < numOuts; i++ {
-		k := canonicalOutPoint(&rec.Hash, i)
-		spenderHash := existsRawUnminedInput(ns, k)
-		if spenderHash != nil {
+	for i := range rec.MsgTx.TxOut {
+		k := canonicalOutPoint(&rec.Hash, uint32(i))
+		spenderHashes := fetchUnminedInputSpendTxHashes(ns, k)
+		for _, spenderHash := range spenderHashes {
+			spenderVal := existsRawUnmined(ns, spenderHash[:])
+
+			// If the spending transaction spends multiple outputs
+			// from the same transaction, we'll find duplicate
+			// entries within the store, so it's possible we're
+			// unable to find it if the conflicts have already been
+			// removed in a previous iteration.
+			if spenderVal == nil {
+				continue
+			}
+
 			var spender TxRecord
-			spenderVal := existsRawUnmined(ns, spenderHash)
-			copy(spender.Hash[:], spenderHash) // Silly but need an array
+			spender.Hash = spenderHash
 			err := readRawTxRecord(&spender.Hash, spenderVal, &spender)
 			if err != nil {
 				return err
@@ -98,13 +119,11 @@ func (s *Store) removeConflict(ns walletdb.ReadWriteBucket, rec *TxRecord) error
 
 			log.Debugf("Transaction %v is part of a removed conflict "+
 				"chain -- removing as well", spender.Hash)
-			err = s.removeConflict(ns, &spender)
-			if err != nil {
+			if err := s.removeConflict(ns, &spender); err != nil {
 				return err
 			}
 		}
-		err := deleteRawUnminedCredit(ns, k)
-		if err != nil {
+		if err := deleteRawUnminedCredit(ns, k); err != nil {
 			return err
 		}
 	}
@@ -115,8 +134,7 @@ func (s *Store) removeConflict(ns walletdb.ReadWriteBucket, rec *TxRecord) error
 	for _, input := range rec.MsgTx.TxIn {
 		prevOut := &input.PreviousOutPoint
 		k := canonicalOutPoint(&prevOut.Hash, prevOut.Index)
-		err := deleteRawUnminedInput(ns, k)
-		if err != nil {
+		if err := deleteRawUnminedInput(ns, k); err != nil {
 			return err
 		}
 	}