Re-enable NeutrinoNodeWithWalletTest for Linux (#1366)

* Re-enable NeutrinoNodeWithWalletTest with it disabled for mac

* Change to only Linux

* Only disable for CI runs

* Move IsCI to BitcoinSUtil

* Fix compile issue for earlier versions

* Fix syncing compact filters to not skip last filter

* Fix test to be compatible with new start state

* Use correct isCI

* Fix compile issue

* set CI env

core/src/main/scala/org/bitcoins/core/util/BitcoinSUtil.scala

@@ -4,6 +4,7 @@ import org.bitcoins.core.protocol.NetworkElement
 import scodec.bits.{BitVector, ByteVector}
 
 import scala.math.BigInt
+import scala.util.Properties
 
 /**
   * Created by chris on 2/26/16.
@@ -94,6 +95,15 @@ trait BitcoinSUtil {
     h.foldLeft(ByteVector.empty)(_ ++ _.bytes)
   }
 
+  private val osName = System.getProperty("os.name")
+
+  lazy val isLinux: Boolean = osName.startsWith("Linux")
+
+  lazy val isMac: Boolean = osName.startsWith("Mac")
+
+  lazy val isWindows: Boolean = osName.startsWith("Windows")
+
+  lazy val isCI: Boolean = Properties.envOrNone("CI").contains("1")
 }
 
 object BitcoinSUtil extends BitcoinSUtil

node-test/src/test/scala/org/bitcoins/node/NeutrinoNodeWithWalletTest.scala

@@ -1,6 +1,7 @@
 package org.bitcoins.node
 
 import org.bitcoins.core.currency._
+import org.bitcoins.core.util.BitcoinSUtil
 import org.bitcoins.core.wallet.fee.SatoshisPerByte
 import org.bitcoins.node.networking.peer.DataMessageHandler
 import org.bitcoins.node.networking.peer.DataMessageHandler.OnCompactFiltersReceived
@@ -13,12 +14,10 @@ import org.bitcoins.testkit.node.NodeUnitTest.NeutrinoNodeFundedWalletBitcoind
 import org.bitcoins.testkit.node.{NodeTestUtil, NodeUnitTest}
 import org.bitcoins.testkit.wallet.BitcoinSWalletTest
 import org.bitcoins.wallet.api.UnlockedWalletApi
-import org.bitcoins.wallet.config.WalletAppConfig
-import org.scalatest.{DoNotDiscover, FutureOutcome}
+import org.scalatest.FutureOutcome
 
 import scala.concurrent.{Future, Promise}
 
-@DoNotDiscover
 class NeutrinoNodeWithWalletTest extends NodeUnitTest {
 
   /** Wallet config with data directory set to user temp directory */
@@ -28,9 +27,16 @@ class NeutrinoNodeWithWalletTest extends NodeUnitTest {
   override type FixtureParam = NeutrinoNodeFundedWalletBitcoind
 
   def withFixture(test: OneArgAsyncTest): FutureOutcome = {
-    withNeutrinoNodeFundedWalletBitcoind(test,
-                                         callbacks,
-                                         Some(BitcoindVersion.Experimental))
+    // We need to disable the test on non-linux CI runs
+    // because we do not have a mac binary of the BIP 157
+    // compatible version of bitcoin core
+    if (BitcoinSUtil.isCI && !BitcoinSUtil.isLinux) {
+      FutureOutcome.succeeded
+    } else {
+      withNeutrinoNodeFundedWalletBitcoind(test,
+                                           callbacks,
+                                           Some(BitcoindVersion.Experimental))
+    }
   }
 
   private var walletP: Promise[UnlockedWalletApi] = Promise()
@@ -55,7 +61,7 @@ class NeutrinoNodeWithWalletTest extends NodeUnitTest {
         _ <- wallet.processBlock(block)
       } yield ()
     }
-    val onCompactFilter: OnCompactFiltersReceived = { blockFilters =>
+    val onCompactFilters: OnCompactFiltersReceived = { blockFilters =>
       for {
         wallet <- walletF
         _ <- wallet.processCompactFilters(blockFilters)
@@ -64,7 +70,7 @@ class NeutrinoNodeWithWalletTest extends NodeUnitTest {
 
     NodeCallbacks(
       onBlockReceived = Seq(onBlock),
-      onCompactFiltersReceived = Seq(onCompactFilter)
+      onCompactFiltersReceived = Seq(onCompactFilters)
     )
   }
 
@@ -95,17 +101,17 @@ class NeutrinoNodeWithWalletTest extends NodeUnitTest {
       val condition1 = () => {
         condition(
           expectedConfirmedAmount = 0.sats,
-          expectedUnconfirmedAmount = BitcoinSWalletTest.initialFunds - TestAmount - TestFees,
-          expectedUtxos = 1,
-          expectedAddresses = 2
+          expectedUnconfirmedAmount = BitcoinSWalletTest.expectedDefaultAmt - TestAmount - TestFees,
+          expectedUtxos = 3,
+          expectedAddresses = 7
         )
       }
       val condition2 = { () =>
         condition(
-          expectedConfirmedAmount = TestAmount,
-          expectedUnconfirmedAmount = BitcoinSWalletTest.initialFunds - TestAmount - TestFees,
-          expectedUtxos = 2,
-          expectedAddresses = 3
+          expectedConfirmedAmount = 0.sats,
+          expectedUnconfirmedAmount = BitcoinSWalletTest.expectedDefaultAmt - TestFees,
+          expectedUtxos = 4,
+          expectedAddresses = 8
         )
       }
 
@@ -147,12 +153,12 @@ class NeutrinoNodeWithWalletTest extends NodeUnitTest {
 
       def condition(): Future[Boolean] = {
         for {
-          balance <- wallet.getConfirmedBalance()
+          balance <- wallet.getBalance()
           addresses <- wallet.listAddresses()
           utxos <- wallet.listUtxos()
         } yield {
-          balance == BitcoinSWalletTest.initialFunds + TestAmount &&
-          utxos.size == 2 &&
+          balance == BitcoinSWalletTest.expectedDefaultAmt + TestAmount &&
+          utxos.size == 4 &&
           addresses.map(_.scriptPubKey.hex).sorted == utxos
             .map(_.output.scriptPubKey.hex)
             .sorted
@@ -162,8 +168,8 @@ class NeutrinoNodeWithWalletTest extends NodeUnitTest {
       for {
         addresses <- wallet.listAddresses()
         utxos <- wallet.listUtxos()
-        _ = assert(addresses.size == 1)
-        _ = assert(utxos.size == 1)
+        _ = assert(addresses.size == 6)
+        _ = assert(utxos.size == 3)
 
         _ <- node.sync()
         _ <- NodeTestUtil.awaitSync(node, bitcoind)
@@ -175,24 +181,22 @@ class NeutrinoNodeWithWalletTest extends NodeUnitTest {
 
         addresses <- wallet.listAddresses()
         utxos <- wallet.listUtxos()
-        _ = assert(addresses.size == 2)
-        _ = assert(utxos.size == 1)
+        _ = assert(addresses.size == 7)
+        _ = assert(utxos.size == 3)
 
-        _ <- wallet.clearUtxosAndAddresses()
+        _ <- wallet.clearAllUtxosAndAddresses()
 
         addresses <- wallet.listAddresses()
         utxos <- wallet.listUtxos()
-        _ = assert(addresses.size == 2)
-        _ = assert(utxos.size == 0)
+        _ = assert(addresses.isEmpty)
+        _ = assert(utxos.isEmpty)
 
         _ <- bitcoind.getNewAddress
           .flatMap(bitcoind.generateToAddress(1, _))
         _ <- NodeTestUtil.awaitSync(node, bitcoind)
         _ <- NodeTestUtil.awaitCompactFiltersSync(node, bitcoind)
 
-        _ <- wallet.rescanNeutrinoWallet(startOpt = None,
-                                         endOpt = None,
-                                         addressBatchSize = 2)
+        _ <- wallet.fullRescanNeutrinoWallet(addressBatchSize = 7)
 
         _ <- AsyncUtil.awaitConditionF(condition)
       } yield succeed

node/src/main/scala/org/bitcoins/node/networking/peer/DataMessageHandler.scala

@@ -100,8 +100,8 @@ case class DataMessageHandler(
           }
           newChainApi <- chainApi.processFilter(filter)
           // If we are not syncing or our filter batch is full, process the filters
-          _ <- if (!syncing || batchSizeFull) {
-            val blockFilters = currentFilterBatch.map { filter =>
+          _ <- if (!newSyncing || batchSizeFull) {
+            val blockFilters = newBatch.map { filter =>
               (filter.blockHash,
                BlockFilter.fromBytes(filter.filterBytes, filter.blockHash))
             }

testkit/src/main/scala/org/bitcoins/testkit/rpc/BitcoindRpcTestUtil.scala

@@ -29,7 +29,7 @@ import org.bitcoins.core.protocol.transaction.{
   TransactionInput,
   TransactionOutPoint
 }
-import org.bitcoins.core.util.BitcoinSLogger
+import org.bitcoins.core.util.{BitcoinSLogger, BitcoinSUtil}
 import org.bitcoins.rpc.BitcoindException
 import org.bitcoins.rpc.client.common.BitcoindVersion.{
   Unknown,
@@ -982,8 +982,7 @@ object BitcoindRpcTestUtil extends BitcoindRpcTestUtil {
     * Used for long running async tasks
     */
   private val DEFAULT_LONG_DURATION = {
-    val isCI = Properties.envOrNone("CI").contains("1")
-    if (Properties.isMac && isCI) 10.seconds
+    if (BitcoinSUtil.isMac && BitcoinSUtil.isCI) 10.seconds
     else 3.seconds
   }
 }

testkit/src/main/scala/org/bitcoins/testkit/wallet/BitcoinSWalletTest.scala

@@ -245,7 +245,17 @@ trait BitcoinSWalletTest extends BitcoinSFixture with WalletLogger {
 
 object BitcoinSWalletTest extends WalletLogger {
 
-  lazy val initialFunds = 25.bitcoins
+  val defaultAcctAmts = Vector(1.bitcoin, 2.bitcoin, 3.bitcoin)
+
+  val expectedDefaultAmt: CurrencyUnit =
+    defaultAcctAmts.fold(CurrencyUnits.zero)(_ + _)
+
+  val account1Amt = Vector(Bitcoins(0.2), Bitcoins(0.3), Bitcoins(0.5))
+
+  val expectedAccount1Amt: CurrencyUnit =
+    account1Amt.fold(CurrencyUnits.zero)(_ + _)
+
+  lazy val initialFunds: CurrencyUnit = expectedDefaultAmt + expectedAccount1Amt
 
   object MockNodeApi extends NodeApi {
 
@@ -497,11 +507,6 @@ object BitcoinSWalletTest extends WalletLogger {
       implicit ec: ExecutionContext): Future[T] = {
     val (wallet, bitcoind) = (pair.wallet, pair.bitcoind)
 
-    val defaultAcctAmts = Vector(1.bitcoin, 2.bitcoin, 3.bitcoin)
-    val expectedDefaultAmt = defaultAcctAmts.fold(CurrencyUnits.zero)(_ + _)
-    val account1Amt = Vector(Bitcoins(0.2), Bitcoins(0.3), Bitcoins(0.5))
-    val expectedAccount1Amt = account1Amt.fold(CurrencyUnits.zero)(_ + _)
-
     val defaultAccount = wallet.walletConfig.defaultAccount
     val fundedDefaultAccountWalletF =
       FundWalletUtil.fundAccountForWalletWithBitcoind(

testkit/src/main/scala/org/bitcoins/testkit/wallet/FundWalletUtil.scala

@@ -3,7 +3,7 @@ package org.bitcoins.testkit.wallet
 import akka.actor.ActorSystem
 import com.typesafe.config.Config
 import org.bitcoins.core.api.{ChainQueryApi, NodeApi}
-import org.bitcoins.core.currency.{Bitcoins, CurrencyUnit, CurrencyUnits, _}
+import org.bitcoins.core.currency.CurrencyUnit
 import org.bitcoins.core.hd.HDAccount
 import org.bitcoins.core.protocol.BitcoinAddress
 import org.bitcoins.core.protocol.transaction.TransactionOutput
@@ -76,14 +76,9 @@ trait FundWalletUtil {
   def fundWallet(wallet: Wallet)(
       implicit ec: ExecutionContext): Future[FundedWallet] = {
 
-    val defaultAcctAmts = Vector(1.bitcoin, 2.bitcoin, 3.bitcoin)
-    val expectedDefaultAmt = defaultAcctAmts.fold(CurrencyUnits.zero)(_ + _)
-    val account1Amt = Vector(Bitcoins(0.2), Bitcoins(0.3), Bitcoins(0.5))
-    val expectedAccount1Amt = account1Amt.fold(CurrencyUnits.zero)(_ + _)
-
     val defaultAccount = wallet.walletConfig.defaultAccount
     val fundedDefaultAccountWalletF = FundWalletUtil.fundAccountForWallet(
-      amts = defaultAcctAmts,
+      amts = BitcoinSWalletTest.defaultAcctAmts,
       account = defaultAccount,
       wallet = wallet
     )
@@ -92,7 +87,7 @@ trait FundWalletUtil {
     val fundedAccount1WalletF = for {
       fundedDefaultAcct <- fundedDefaultAccountWalletF
       fundedAcct1 <- FundWalletUtil.fundAccountForWallet(
-        amts = account1Amt,
+        amts = BitcoinSWalletTest.account1Amt,
         account = hdAccount1,
         fundedDefaultAcct
       )
@@ -103,14 +98,15 @@ trait FundWalletUtil {
       fundedWallet <- fundedAccount1WalletF
       balance <- fundedWallet.getBalance(defaultAccount)
       _ = require(
-        balance == expectedDefaultAmt,
-        s"Funding wallet fixture failed to fund the wallet, got balance=${balance} expected=${expectedDefaultAmt}")
+        balance == BitcoinSWalletTest.expectedDefaultAmt,
+        s"Funding wallet fixture failed to fund the wallet, got balance=${balance} expected=${BitcoinSWalletTest.expectedDefaultAmt}"
+      )
 
       account1Balance <- fundedWallet.getBalance(hdAccount1)
       _ = require(
-        account1Balance == expectedAccount1Amt,
+        account1Balance == BitcoinSWalletTest.expectedAccount1Amt,
         s"Funding wallet fixture failed to fund account 1, " +
-          s"got balance=${hdAccount1} expected=${expectedAccount1Amt}"
+          s"got balance=${hdAccount1} expected=${BitcoinSWalletTest.expectedAccount1Amt}"
       )
 
     } yield FundedWallet(fundedWallet)