From daa28e0d0ff62c2b47e1ab90b10be0a423f38b44 Mon Sep 17 00:00:00 2001
From: Conner Fromknecht <conner@mit.edu>
Date: Thu, 5 Jul 2018 20:14:05 -0700
Subject: [PATCH] routing/chainview/bitcoind: improve filterMtx locking

In this commit, we modify the granularity of the locking
around the filterMtx in the bitcoind chainview, such that
we only lock once per block connected or filter update.
Currently, we acquire and release the lock for every
update to the map.

We also fix a bug that would cause us to not fully remove
all previous outpoints spent by a txn when doing manual
filter, as we previously would only remove the first output
detected.
---
 routing/chainview/bitcoind.go | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/routing/chainview/bitcoind.go b/routing/chainview/bitcoind.go
index db7ec301..5b52384a 100644
--- a/routing/chainview/bitcoind.go
+++ b/routing/chainview/bitcoind.go
@@ -154,6 +154,7 @@ func (b *BitcoindFilteredChainView) onFilteredBlockConnected(height int32,
 	hash chainhash.Hash, txns []*wtxmgr.TxRecord) {
 
 	mtxs := make([]*wire.MsgTx, len(txns))
+	b.filterMtx.Lock()
 	for i, tx := range txns {
 		mtxs[i] = &tx.MsgTx
 
@@ -164,12 +165,11 @@ func (b *BitcoindFilteredChainView) onFilteredBlockConnected(height int32,
 			// that's okay since it would never be wise to consider
 			// the channel open again (since a spending transaction
 			// exists on the network).
-			b.filterMtx.Lock()
 			delete(b.chainFilter, txIn.PreviousOutPoint)
-			b.filterMtx.Unlock()
 		}
 
 	}
+	b.filterMtx.Unlock()
 
 	// We record the height of the last connected block added to the
 	// blockQueue such that we can scan up to this height in case of
@@ -246,19 +246,29 @@ func (b *BitcoindFilteredChainView) chainFilterer() {
 	// watched. Additionally, the chain filter will also be updated by
 	// removing any spent outputs.
 	filterBlock := func(blk *wire.MsgBlock) []*wire.MsgTx {
+		b.filterMtx.Lock()
+		defer b.filterMtx.Unlock()
+
 		var filteredTxns []*wire.MsgTx
 		for _, tx := range blk.Transactions {
+			var txAlreadyFiltered bool
 			for _, txIn := range tx.TxIn {
 				prevOp := txIn.PreviousOutPoint
-				if _, ok := b.chainFilter[prevOp]; ok {
-					filteredTxns = append(filteredTxns, tx)
-
-					b.filterMtx.Lock()
-					delete(b.chainFilter, prevOp)
-					b.filterMtx.Unlock()
-
-					break
+				if _, ok := b.chainFilter[prevOp]; !ok {
+					continue
 				}
+
+				delete(b.chainFilter, prevOp)
+
+				// Only add this txn to our list of filtered
+				// txns if it is the first previous outpoint to
+				// cause a match.
+				if txAlreadyFiltered {
+					continue
+				}
+
+				filteredTxns = append(filteredTxns, tx)
+				txAlreadyFiltered = true
 			}
 		}
 
@@ -304,11 +314,12 @@ func (b *BitcoindFilteredChainView) chainFilterer() {
 			// process.
 			log.Debugf("Updating chain filter with new UTXO's: %v",
 				update.newUtxos)
+
+			b.filterMtx.Lock()
 			for _, newOp := range update.newUtxos {
-				b.filterMtx.Lock()
 				b.chainFilter[newOp] = struct{}{}
-				b.filterMtx.Unlock()
 			}
+			b.filterMtx.Unlock()
 
 			// Apply the new TX filter to the chain client, which
 			// will cause all following notifications from and