Merge pull request #594 from torkelrogstad/2019-07-10-bloom

Add all wallet outpoints to bloom filter

db-commons/src/main/scala/org/bitcoins/db/CRUD.scala

@@ -153,12 +153,17 @@ case class SafeDatabase(config: AppConfig) extends BitcoinSLogger {
       throw err
   }
 
+  /** Runs the given DB action */
   def run[R](action: DBIOAction[R, NoStream, _])(
       implicit ec: ExecutionContext): Future[R] = {
     val result = database.run[R](foreignKeysPragma >> action)
     result.recoverWith { logAndThrowError(action) }
   }
 
+  /**
+    * Runs the given DB sequence-returning DB action
+    * and converts the result to a vector
+    */
   def runVec[R](action: DBIOAction[Seq[R], NoStream, _])(
       implicit ec: ExecutionContext): Future[Vector[R]] = {
     val result = database.run[Seq[R]](foreignKeysPragma >> action)

wallet-test/src/test/scala/org/bitcoins/wallet/WalletBloomTest.scala

@@ -0,0 +1,57 @@
+package org.bitcoins.wallet
+
+import org.bitcoins.testkit.wallet.BitcoinSWalletTest
+import org.bitcoins.wallet.api.UnlockedWalletApi
+import org.scalatest.FutureOutcome
+import org.bitcoins.wallet.api.UnlockWalletError._
+import org.bitcoins.wallet.api.UnlockWalletSuccess
+import org.bitcoins.core.util.FutureUtil
+import org.bitcoins.core.currency._
+import org.bitcoins.testkit.Implicits._
+
+class WalletBloomTest extends BitcoinSWalletTest {
+  behavior of "Wallet bloom filter"
+
+  override type FixtureParam = WalletWithBitcoind
+
+  override def withFixture(test: OneArgAsyncTest): FutureOutcome =
+    withNewWalletAndBitcoind(test)
+
+  it should "generate a bloom filter that matches the pubkeys in our wallet" in {
+    param =>
+      val WalletWithBitcoind(walletApi, _) = param
+      val wallet = walletApi.asInstanceOf[Wallet]
+      for {
+        _ <- FutureUtil.sequentially(0 until 10)(_ => wallet.getNewAddress())
+        bloom <- wallet.getBloomFilter()
+        pubkeys <- wallet.listPubkeys()
+      } yield {
+        pubkeys.map { (pub) =>
+          assert(bloom.contains(pub))
+        }.toAssertion
+      }
+  }
+
+  // TODO: change fixture to withFundedWalletAndBitcoind once #577 goes in
+  // https://github.com/bitcoin-s/bitcoin-s/pull/577/files#diff-0fb6ac004fe1e550b7c13258d7d0706cR154
+  it should "generate a bloom filter that matches the outpoints in our wallet" in {
+    param =>
+      val WalletWithBitcoind(walletApi, bitcoind) = param
+      val wallet = walletApi.asInstanceOf[Wallet]
+
+      for {
+        address <- wallet.getNewAddress()
+        tx <- bitcoind
+          .sendToAddress(address, 5.bitcoins)
+          .flatMap(bitcoind.getRawTransaction(_))
+        _ <- wallet.processTransaction(tx.hex, confirmations = 0)
+        outpoints <- wallet.listOutpoints()
+
+        bloom <- wallet.getBloomFilter()
+      } yield {
+        outpoints.map { (out) =>
+          assert(bloom.contains(out))
+        }.toAssertion
+      }
+  }
+}

wallet-test/src/test/scala/org/bitcoins/wallet/WalletUnitTest.scala

@@ -132,33 +132,6 @@ class WalletUnitTest extends BitcoinSWalletTest {
     } yield res
   }
 
-  it should "generate a bloom filter" in { walletApi: UnlockedWalletApi =>
-    val wallet = walletApi.asInstanceOf[Wallet]
-    for {
-      _ <- FutureUtil.sequentially(0 until 10)(_ => wallet.getNewAddress())
-      bloom <- wallet.getBloomFilter()
-      pubkeys <- wallet.listPubkeys()
-    } yield {
-      pubkeys.foldLeft(succeed) { (_, pub) =>
-        assert(bloom.contains(pub))
-      }
-    }
-
-  }
-
-  it should "lock and unlock the wallet" in { wallet: UnlockedWalletApi =>
-    val passphrase = wallet.passphrase
-    val locked = wallet.lock()
-    val unlocked = wallet.unlock(passphrase) match {
-      case MnemonicNotFound                       => fail(MnemonicNotFound)
-      case BadPassword                            => fail(BadPassword)
-      case JsonParsingError(message)              => fail(message)
-      case UnlockWalletSuccess(unlockedWalletApi) => unlockedWalletApi
-    }
-
-    assert(wallet.mnemonicCode == unlocked.mnemonicCode)
-  }
-
   it should "fail to unlock the wallet with a bad password" in {
     wallet: UnlockedWalletApi =>
       val badpassphrase = AesPassword.fromNonEmptyString("bad")

wallet/src/main/scala/org/bitcoins/wallet/LockedWallet.scala

@@ -14,6 +14,7 @@ import org.bitcoins.core.bloom.BloomFilter
 import org.bitcoins.core.bloom.BloomUpdateAll
 import org.bitcoins.core.util.BitcoinSLogger
 import org.bitcoins.wallet.internal._
+import org.bitcoins.core.protocol.transaction.TransactionOutPoint
 
 abstract class LockedWallet
     extends LockedWalletApi
@@ -81,26 +82,35 @@ abstract class LockedWallet
     }
   }
 
-  /** Gets the size of the bloom filter for this wallet  */
-  private def getBloomFilterSize(): Future[Int] = {
-    for {
-      pubkeys <- listPubkeys()
-    } yield {
-      // when a public key is inserted into a filter
-      // both the pubkey and the hash of the pubkey
-      // gets inserted
-      pubkeys.length * 2
-    }
+  /** Enumerates all the TX outpoints in the wallet  */
+  protected[wallet] def listOutpoints(): Future[Vector[TransactionOutPoint]] =
+    spendingInfoDAO.findAllOutpoints()
 
-  }
+  /** Gets the size of the bloom filter for this wallet  */
+  private def getBloomFilterSize(
+      pubkeys: Seq[ECPublicKey],
+      outpoints: Seq[TransactionOutPoint]): Int = {
+    // when a public key is inserted into a filter
+    // both the pubkey and the hash of the pubkey
+    // gets inserted
+    pubkeys.length * 2
+  } + outpoints.length
 
   // todo: insert TXIDs? need to track which txids we should
   // ask for, somehow
+  // We add all outpoints to the bloom filter as a way
+  // of working around the fact that bloom filters
+  // was never updated to incorporate SegWit changes.
+  // see this mailing list thread for context:
+  //   https://www.mail-archive.com/bitcoin-dev@lists.linuxfoundation.org/msg06950.html
+  // especially this email from Jim Posen:
+  //   https://www.mail-archive.com/bitcoin-dev@lists.linuxfoundation.org/msg06952.html
   override def getBloomFilter(): Future[BloomFilter] = {
     for {
       pubkeys <- listPubkeys()
-      filterSize <- getBloomFilterSize()
+      outpoints <- listOutpoints()
     } yield {
+      val filterSize = getBloomFilterSize(pubkeys, outpoints)
 
       // todo: Is this the best flag to use?
       val bloomFlag = BloomUpdateAll
@@ -110,7 +120,8 @@ abstract class LockedWallet
                     falsePositiveRate = walletConfig.bloomFalsePositiveRate,
                     flags = bloomFlag)
 
-      pubkeys.foldLeft(baseBloom) { _.insert(_) }
+      val withPubs = pubkeys.foldLeft(baseBloom) { _.insert(_) }
+      outpoints.foldLeft(withPubs) { _.insert(_) }
     }
   }
 }

wallet/src/main/scala/org/bitcoins/wallet/models/SpendingInfoDAO.scala

@@ -9,6 +9,7 @@ import scala.concurrent.Future
 import org.bitcoins.core.protocol.transaction.Transaction
 import org.bitcoins.core.crypto.DoubleSha256DigestBE
 import org.bitcoins.core.protocol.transaction.TransactionOutput
+import org.bitcoins.core.protocol.transaction.TransactionOutPoint
 
 case class SpendingInfoDAO()(
     implicit val ec: ExecutionContext,
@@ -96,4 +97,10 @@ case class SpendingInfoDAO()(
 
     database.run(query.result).map(_.toVector)
   }
+
+  /** Enumerates all TX outpoints in the wallet */
+  def findAllOutpoints(): Future[Vector[TransactionOutPoint]] = {
+    val query = table.map(_.outPoint)
+    database.runVec(query.result).map(_.toVector)
+  }
 }