code restructuring

modules/wallet/update.go

@@ -554,41 +554,27 @@ func (w *Wallet) ReceiveUpdatedUnconfirmedTransactions(diff *modules.Transaction
 			continue
 		}
 
-		// Mark all of the transactions that appeared in this set.
-		//
-		// Check if commitTransactionSet already added the transaction to the
-		// unconfirmedSets
-		if _, exists := w.unconfirmedSets[unconfirmedTxnSet.ID]; !exists {
-			// Maybe acceptTransactionSet added a subset with a different
-			// id. If that's the case, replace it with the one from the
-			// tpool.
-			isSubset := false
-			for tSetID, ids := range w.unconfirmedSets {
-				if isSuperset(unconfirmedTxnSet.IDs, ids) {
-					// Add the new id
-					w.unconfirmedSets[unconfirmedTxnSet.ID] = w.unconfirmedSets[tSetID]
-					if err := dbPutUnconfirmedSet(w.dbTx, unconfirmedTxnSet.ID, unconfirmedTxnSet.IDs); err != nil {
-						build.Critical(err)
-					}
-					// Remove the old id
-					delete(w.unconfirmedSets, tSetID)
-					if err := dbDeleteUnconfirmedSet(w.dbTx, tSetID); err != nil {
-						build.Critical(err)
-					}
-					isSubset = true
-					break
-				}
-			}
-			// If we couldn't find a matching superset, the transaction
-			// probably wasn't created by this wallet. In that case we can add
-			// the transaction.
-			if !isSubset {
-				w.unconfirmedSets[unconfirmedTxnSet.ID] = unconfirmedTxnSet.IDs
-				if err := dbPutUnconfirmedSet(w.dbTx, unconfirmedTxnSet.ID, unconfirmedTxnSet.IDs); err != nil {
+		// Check if unconfirmedSets already contains a subset of the
+		// unconfirmedTxnSet's ids. This might happen if the wallet manually
+		// added the set to unconfirmedSets after AcceptTransactionSet failed
+		// in commitTransactionSet.  If it contains a subset it should be
+		// deleted and be replaced by the superset.
+		for tSetID, ids := range w.unconfirmedSets {
+			if isSuperset(unconfirmedTxnSet.IDs, ids) {
+				// Remove the old id
+				delete(w.unconfirmedSets, tSetID)
+				if err := dbDeleteUnconfirmedSet(w.dbTx, tSetID); err != nil {
 					build.Critical(err)
 				}
+				break
 			}
 		}
+		// Add the set to the unconfirmedSets
+		w.unconfirmedSets[unconfirmedTxnSet.ID] = unconfirmedTxnSet.IDs
+		err := dbPutUnconfirmedSet(w.dbTx, unconfirmedTxnSet.ID, unconfirmedTxnSet.IDs)
+		if err != nil {
+			build.Critical(err)
+		}
 
 		// Get the values for the spent outputs.
 		spentSiacoinOutputs := make(map[types.SiacoinOutputID]types.SiacoinOutput)
@@ -600,6 +586,17 @@ func (w *Wallet) ReceiveUpdatedUnconfirmedTransactions(diff *modules.Transaction
 			}
 		}
 
+		// Build an index that maps a transaction id to it's index in
+		// unconfirmedProcessedTransactions. This allows us to find duplicates
+		// in unconfirmedProcessedTransactions. There is a chance that after a
+		// failed AcceptTransactionSet in commitTransactionSet the wallet
+		// already added a particular transaction. In that case we want to
+		// replace it instead of appending.
+		ptIndices := make(map[types.TransactionID]int)
+		for i, pt := range w.unconfirmedProcessedTransactions {
+			ptIndices[pt.TransactionID] = i
+		}
+
 		// Add each transaction to our set of unconfirmed transactions.
 		for i, txn := range unconfirmedTxnSet.Transactions {
 			// only create a ProcessedTransaction if txn is relevant
@@ -638,7 +635,13 @@ func (w *Wallet) ReceiveUpdatedUnconfirmedTransactions(diff *modules.Transaction
 					Value:    fee,
 				})
 			}
-			w.unconfirmedProcessedTransactions = append(w.unconfirmedProcessedTransactions, pt)
+			// Check if a transaction with that id already consists. If it does
+			// we replace it. Otherwise we append
+			if _, exists := ptIndices[pt.TransactionID]; exists {
+				w.unconfirmedProcessedTransactions[i] = pt
+			} else {
+				w.unconfirmedProcessedTransactions = append(w.unconfirmedProcessedTransactions, pt)
+			}
 		}
 	}
 }

modules/wallet/wallet.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"math"
 	"sort"
 	"sync"
 
@@ -119,27 +120,44 @@ func (w *Wallet) Height() types.BlockHeight {
 // AcceptTransactionSet method. It only returns an error if the transaction was
 // rejected and won't be rebroadcasted over time
 func (w *Wallet) commitTransactionSet(txns []types.Transaction) error {
-	// Get the transaction ids and add them to the unconfirmedSets
+	w.mu.Unlock()
+	err := w.tpool.AcceptTransactionSet(txns)
+	w.mu.Lock()
+	if err == nil {
+		// If we were able to add the transactions to the pool we are done. The
+		// wallet already updated the unconfirmedSets and
+		// unconfirmedProcessedTransactions fields in
+		// ReceiveUpdatedUnconfirmedTransactions
+		return nil
+	}
+	if err != nil {
+		// TODO: There might be some errors that make us abort here
+	}
+
+	// If we couldn't add the transaction but still want the wallet to track it
+	// we need to add it manually to the unconfirmedSets and
+	// unconfirmedProcessedTransactions
 	tSetID := modules.TransactionSetID(crypto.HashObject(txns))
 	ids := make([]types.TransactionID, 0, len(txns))
+	pts := make([]modules.ProcessedTransaction, 0, len(txns))
 	for _, txn := range txns {
 		ids = append(ids, txn.ID())
+		pt := modules.ProcessedTransaction{
+			Transaction:           txn,
+			TransactionID:         txn.ID(),
+			ConfirmationHeight:    types.BlockHeight(math.MaxUint64),
+			ConfirmationTimestamp: types.Timestamp(math.MaxUint64),
+		}
+		// TODO Also add processed inputs and outputs
+		pts = append(pts, pt)
 	}
+	// Add the unconfirmed set
 	w.unconfirmedSets[tSetID] = ids
 	if err := dbPutUnconfirmedSet(w.dbTx, tSetID, ids); err != nil {
 		return err
 	}
-
-	// Accept the set. If the error returned prevents the set from ever being
-	// accepted we should remove it from the wallet again.
-	w.mu.Unlock()
-	err := w.tpool.AcceptTransactionSet(txns)
-	w.mu.Lock()
-	if err != nil { // TODO: There are exceptions to this
-		delete(w.unconfirmedSets, tSetID)
-		err2 := dbDeleteUnconfirmedSet(w.dbTx, tSetID)
-		return build.ComposeErrors(err, err2)
-	}
+	// Add the unconfirmed processed transactions
+	w.unconfirmedProcessedTransactions = append(w.unconfirmedProcessedTransactions, pts)
 	return nil
 }