From 5f7356c5267746876bfbfab7ec6f8ae61725d66a Mon Sep 17 00:00:00 2001 From: Ben Carman Date: Wed, 9 Sep 2020 09:40:38 -0500 Subject: [PATCH] Fix Two KeyManagers in scope for fundRawTransactionInternal (#1986) * Fix Two KeyManagers in scope for fundRawTransactionInternal * Only fetch txs once --- .../wallet/AddressTagIntegrationTest.scala | 1 - .../wallet/FundTransactionHandlingTest.scala | 1 - .../scala/org/bitcoins/wallet/Wallet.scala | 2 - .../internal/FundTransactionHandling.scala | 57 +++++-------------- 4 files changed, 13 insertions(+), 48 deletions(-) diff --git a/wallet-test/src/test/scala/org/bitcoins/wallet/AddressTagIntegrationTest.scala b/wallet-test/src/test/scala/org/bitcoins/wallet/AddressTagIntegrationTest.scala index 9deca48db5..63d2d3b052 100644 --- a/wallet-test/src/test/scala/org/bitcoins/wallet/AddressTagIntegrationTest.scala +++ b/wallet-test/src/test/scala/org/bitcoins/wallet/AddressTagIntegrationTest.scala @@ -83,7 +83,6 @@ class AddressTagIntegrationTest extends BitcoinSWalletTest { .fundRawTransactionInternal(destinations = Vector(output), feeRate = feeRate, fromAccount = account, - keyManagerOpt = Some(wallet.keyManager), fromTagOpt = Some(exampleTag)) } utx <- txBuilder.buildTx() diff --git a/wallet-test/src/test/scala/org/bitcoins/wallet/FundTransactionHandlingTest.scala b/wallet-test/src/test/scala/org/bitcoins/wallet/FundTransactionHandlingTest.scala index 5119986d56..79a4b84761 100644 --- a/wallet-test/src/test/scala/org/bitcoins/wallet/FundTransactionHandlingTest.scala +++ b/wallet-test/src/test/scala/org/bitcoins/wallet/FundTransactionHandlingTest.scala @@ -235,7 +235,6 @@ class FundTransactionHandlingTest extends BitcoinSWalletTest { destinations = Vector(destination), feeRate = feeRate, fromAccount = account, - keyManagerOpt = Some(wallet.keyManager), fromTagOpt = Some(tag), markAsReserved = true ) diff --git a/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala b/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala index 8f89452a10..487c3c0fb8 100644 --- a/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala +++ b/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala @@ -452,7 +452,6 @@ abstract class Wallet destinations = Vector(destination), feeRate = feeRate, fromAccount = fromAccount, - keyManagerOpt = Some(keyManager), coinSelectionAlgo = algo, fromTagOpt = None) @@ -544,7 +543,6 @@ abstract class Wallet destinations = outputs, feeRate = feeRate, fromAccount = fromAccount, - keyManagerOpt = Some(keyManager), fromTagOpt = None) sentAmount = outputs.foldLeft(CurrencyUnits.zero)(_ + _.value) tx <- finishSend(txBuilder, utxoInfos, sentAmount, feeRate, newTags) diff --git a/wallet/src/main/scala/org/bitcoins/wallet/internal/FundTransactionHandling.scala b/wallet/src/main/scala/org/bitcoins/wallet/internal/FundTransactionHandling.scala index 449a178394..12484f02ed 100644 --- a/wallet/src/main/scala/org/bitcoins/wallet/internal/FundTransactionHandling.scala +++ b/wallet/src/main/scala/org/bitcoins/wallet/internal/FundTransactionHandling.scala @@ -1,11 +1,7 @@ package org.bitcoins.wallet.internal import org.bitcoins.core.api.wallet.db.{AccountDb, SpendingInfoDb} -import org.bitcoins.core.api.wallet.{ - AddressInfo, - CoinSelectionAlgo, - CoinSelector -} +import org.bitcoins.core.api.wallet.{CoinSelectionAlgo, CoinSelector} import org.bitcoins.core.consensus.Consensus import org.bitcoins.core.protocol.transaction._ import org.bitcoins.core.util.FutureUtil @@ -16,8 +12,6 @@ import org.bitcoins.core.wallet.builder.{ } import org.bitcoins.core.wallet.fee.FeeUnit import org.bitcoins.core.wallet.utxo._ -import org.bitcoins.crypto.Sign -import org.bitcoins.keymanager.bip39.BIP39KeyManager import org.bitcoins.wallet.{Wallet, WalletLogger} import scala.concurrent.Future @@ -49,7 +43,6 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet => fundRawTransactionInternal(destinations = destinations, feeRate = feeRate, fromAccount = fromAccount, - keyManagerOpt = None, fromTagOpt = fromTagOpt, markAsReserved = markAsReserved) .flatMap(_._1.buildTx()) @@ -69,14 +62,13 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet => destinations: Vector[TransactionOutput], feeRate: FeeUnit, fromAccount: AccountDb, - keyManagerOpt: Option[BIP39KeyManager], coinSelectionAlgo: CoinSelectionAlgo = CoinSelectionAlgo.AccumulateLargest, fromTagOpt: Option[AddressTag], markAsReserved: Boolean = false): Future[( RawTxBuilderWithFinalizer[ShufflingNonInteractiveFinalizer], Vector[ScriptSignatureParams[InputInfo]])] = { - def utxosF: Future[Vector[SpendingInfoDb]] = + def utxosF: Future[Vector[(SpendingInfoDb, Transaction)]] = for { utxos <- fromTagOpt match { case None => @@ -105,53 +97,30 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet => confsOpt.isDefined && confsOpt.get < Consensus.coinbaseMaturity } .map(_._1) - } yield utxos.filter(utxo => !immatureCoinbases.exists(_._1 == utxo)) + } yield utxoWithTxs.filter(utxo => + !immatureCoinbases.exists(_._1 == utxo._1)) - val selectedUtxosF: Future[Vector[SpendingInfoDb]] = + val selectedUtxosF: Future[Vector[(SpendingInfoDb, Transaction)]] = for { walletUtxos <- utxosF utxos = CoinSelector.selectByAlgo(coinSelectionAlgo = coinSelectionAlgo, - walletUtxos = walletUtxos, + walletUtxos = walletUtxos.map(_._1), outputs = destinations, feeRate = feeRate) - } yield utxos - val addrInfosWithUtxoF: Future[ - Vector[(SpendingInfoDb, Transaction, AddressInfo)]] = - for { - selectedUtxos <- selectedUtxosF - _ = selectedUtxosF.failed.foreach(err => - logger.error("Error selecting utxos to fund transaction ", err)) - addrInfoOptF = selectedUtxos.map { utxo => - // .gets should be safe here because of foreign key at the database level - for { - addrInfo <- getAddressInfo(utxo, networkParameters).map(_.get) - prevTx <- - transactionDAO - .findByOutPoint(utxo.outPoint) - .map(_.get.transaction) - } yield (utxo, prevTx, addrInfo) - } - vec <- FutureUtil.collect(addrInfoOptF).map(_.toVector) - } yield vec + } yield walletUtxos.filter(utxo => utxos.contains(utxo._1)) val resultF = for { - addrInfosWithUtxo <- addrInfosWithUtxoF + selectedUtxos <- selectedUtxosF change <- getNewChangeAddress(fromAccount) utxoSpendingInfos = { - addrInfosWithUtxo.map { - case (utxo, prevTx, addrInfo) => - keyManagerOpt match { - case Some(km) => - utxo.toUTXOInfo(keyManager = km, prevTx) - case None => - utxo.toUTXOInfo(sign = Sign.dummySign(addrInfo.pubkey), prevTx) - } - + selectedUtxos.map { + case (utxo, prevTx) => + utxo.toUTXOInfo(keyManager = self.keyManager, prevTx) } } _ <- - if (markAsReserved) markUTXOsAsReserved(addrInfosWithUtxo.map(_._1)) + if (markAsReserved) markUTXOsAsReserved(selectedUtxos.map(_._1)) else FutureUtil.unit } yield { logger.info { @@ -191,7 +160,7 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet => if (markAsReserved) { for { utxos <- selectedUtxosF - _ <- unmarkUTXOsAsReserved(utxos) + _ <- unmarkUTXOsAsReserved(utxos.map(_._1)) } yield error } else Future.failed(error) }