From 0f6bc3538d45d5ad39f9d44c2c455957133c26da Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Thu, 5 Jul 2018 20:11:06 -0700 Subject: [PATCH 1/2] routing/chainview/btcd: improve filterMtx locking In this commit, we modify the granularity of the locking around the filterMtx in the btcd 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/btcd.go | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/routing/chainview/btcd.go b/routing/chainview/btcd.go index e5dad71dc..6fefcf1c9 100644 --- a/routing/chainview/btcd.go +++ b/routing/chainview/btcd.go @@ -153,6 +153,7 @@ func (b *BtcdFilteredChainView) onFilteredBlockConnected(height int32, header *wire.BlockHeader, txns []*btcutil.Tx) { mtxs := make([]*wire.MsgTx, len(txns)) + b.filterMtx.Lock() for i, tx := range txns { mtx := tx.MsgTx() mtxs[i] = mtx @@ -164,12 +165,11 @@ func (b *BtcdFilteredChainView) 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 @@ -254,19 +254,30 @@ func (b *BtcdFilteredChainView) 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 + } } @@ -312,11 +323,12 @@ func (b *BtcdFilteredChainView) 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 btcd, which will cause // all following notifications from and calls to it From daa28e0d0ff62c2b47e1ab90b10be0a423f38b44 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Thu, 5 Jul 2018 20:14:05 -0700 Subject: [PATCH 2/2] 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 db7ec3012..5b52384a0 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