Merge pull request #1554 from halseth/bitcoind-mempool-spends

Filter out bitcoind mempool spends

chainntnfs/bitcoindnotify/bitcoind.go

@@ -352,16 +352,40 @@ func (b *BitcoindNotifier) handleRelevantTx(tx chain.RelevantTx, bestHeight int3
 			// TODO(roasbeef): after change to
 			// loadfilter, only notify on block
 			// inclusion?
+
+			confirmedSpend := false
 			if tx.Block != nil {
+				confirmedSpend = true
 				spendDetails.SpendingHeight = tx.Block.Height
 			} else {
 				spendDetails.SpendingHeight = bestHeight + 1
 			}
 
-			for _, ntfn := range clients {
+			// Keep spendNotifications that are
-				chainntnfs.Log.Infof("Dispatching "+
+			// waiting for a confirmation around.
+			// They will be notified when we find
+			// the spend within a block.
+			rem := make(map[uint64]*spendNotification)
+			for c, ntfn := range clients {
+				// If this is a mempool spend,
+				// and this client didn't want
+				// to be notified on mempool
+				// spends, store it for later.
+				if !confirmedSpend && !ntfn.mempool {
+					rem[c] = ntfn
+					continue
+				}
+
+				confStr := "unconfirmed"
+				if confirmedSpend {
+					confStr = "confirmed"
+				}
+
+				chainntnfs.Log.Infof("Dispatching %s "+
 					"spend notification for "+
-					"outpoint=%v", ntfn.targetOutpoint)
+					"outpoint=%v at height %v",
+					confStr, ntfn.targetOutpoint,
+					spendDetails.SpendingHeight)
 				ntfn.spendChan <- spendDetails
 
 				// Close spendChan to ensure that any calls to Cancel will not
@@ -370,6 +394,12 @@ func (b *BitcoindNotifier) handleRelevantTx(tx chain.RelevantTx, bestHeight int3
 				close(ntfn.spendChan)
 			}
 			delete(b.spendNotifications, prevOut)
+
+			// If we had any clients left, add them
+			// back to the map.
+			if len(rem) > 0 {
+				b.spendNotifications[prevOut] = rem
+			}
 		}
 	}
 }
@@ -530,6 +560,8 @@ type spendNotification struct {
 	spendID uint64
 
 	heightHint uint32
+
+	mempool bool
 }
 
 // spendCancel is a message sent to the BitcoindNotifier when a client wishes
@@ -548,12 +580,13 @@ type spendCancel struct {
 // across the 'Spend' channel. The heightHint should represent the earliest
 // height in the chain where the transaction could have been spent in.
 func (b *BitcoindNotifier) RegisterSpendNtfn(outpoint *wire.OutPoint,
-	heightHint uint32, _ bool) (*chainntnfs.SpendEvent, error) {
+	heightHint uint32, mempool bool) (*chainntnfs.SpendEvent, error) {
 
 	ntfn := &spendNotification{
 		targetOutpoint: outpoint,
 		spendChan:      make(chan *chainntnfs.SpendDetail, 1),
 		spendID:        atomic.AddUint64(&b.spendClientCounter, 1),
+		mempool:        mempool,
 	}
 
 	select {

chainntnfs/interface_test.go

@@ -425,14 +425,30 @@ func testSpendNotification(miner *rpctest.Harness,
 		t.Fatalf("tx not relayed to miner: %v", err)
 	}
 
-	// Make sure notifications are not yet sent.
+	// Make sure notifications are not yet sent. We launch a go routine for
+	// all the spend clients, such that we can wait for them all in
+	// parallel.
+	//
+	// Since bitcoind is at times very slow at notifying about txs in the
+	// mempool, we use a quite large timeout of 10 seconds.
+	// TODO(halseth): change this when mempool spends are removed.
+	mempoolSpendTimeout := 10 * time.Second
+	mempoolSpends := make(chan *chainntnfs.SpendDetail, numClients)
 	for _, c := range spendClients {
-		select {
+		go func(client *chainntnfs.SpendEvent) {
-		case <-c.Spend:
+			select {
-			t.Fatalf("did not expect to get notification before " +
+			case s := <-client.Spend:
-				"block was mined")
+				mempoolSpends <- s
-		case <-time.After(50 * time.Millisecond):
+			case <-time.After(mempoolSpendTimeout):
-		}
+			}
+		}(c)
+	}
+
+	select {
+	case <-mempoolSpends:
+		t.Fatalf("did not expect to get notification before " +
+			"block was mined")
+	case <-time.After(mempoolSpendTimeout):
 	}
 
 	// Now we mine a single block, which should include our spend. The