Fix Two KeyManagers in scope for fundRawTransactionInternal (#1986)

* Fix Two KeyManagers in scope for fundRawTransactionInternal

* Only fetch txs once
This commit is contained in:
Ben Carman 2020-09-09 09:40:38 -05:00 committed by GitHub
parent 4c0437e352
commit 5f7356c526
4 changed files with 13 additions and 48 deletions

View file

@ -83,7 +83,6 @@ class AddressTagIntegrationTest extends BitcoinSWalletTest {
.fundRawTransactionInternal(destinations = Vector(output), .fundRawTransactionInternal(destinations = Vector(output),
feeRate = feeRate, feeRate = feeRate,
fromAccount = account, fromAccount = account,
keyManagerOpt = Some(wallet.keyManager),
fromTagOpt = Some(exampleTag)) fromTagOpt = Some(exampleTag))
} }
utx <- txBuilder.buildTx() utx <- txBuilder.buildTx()

View file

@ -235,7 +235,6 @@ class FundTransactionHandlingTest extends BitcoinSWalletTest {
destinations = Vector(destination), destinations = Vector(destination),
feeRate = feeRate, feeRate = feeRate,
fromAccount = account, fromAccount = account,
keyManagerOpt = Some(wallet.keyManager),
fromTagOpt = Some(tag), fromTagOpt = Some(tag),
markAsReserved = true markAsReserved = true
) )

View file

@ -452,7 +452,6 @@ abstract class Wallet
destinations = Vector(destination), destinations = Vector(destination),
feeRate = feeRate, feeRate = feeRate,
fromAccount = fromAccount, fromAccount = fromAccount,
keyManagerOpt = Some(keyManager),
coinSelectionAlgo = algo, coinSelectionAlgo = algo,
fromTagOpt = None) fromTagOpt = None)
@ -544,7 +543,6 @@ abstract class Wallet
destinations = outputs, destinations = outputs,
feeRate = feeRate, feeRate = feeRate,
fromAccount = fromAccount, fromAccount = fromAccount,
keyManagerOpt = Some(keyManager),
fromTagOpt = None) fromTagOpt = None)
sentAmount = outputs.foldLeft(CurrencyUnits.zero)(_ + _.value) sentAmount = outputs.foldLeft(CurrencyUnits.zero)(_ + _.value)
tx <- finishSend(txBuilder, utxoInfos, sentAmount, feeRate, newTags) tx <- finishSend(txBuilder, utxoInfos, sentAmount, feeRate, newTags)

View file

@ -1,11 +1,7 @@
package org.bitcoins.wallet.internal package org.bitcoins.wallet.internal
import org.bitcoins.core.api.wallet.db.{AccountDb, SpendingInfoDb} import org.bitcoins.core.api.wallet.db.{AccountDb, SpendingInfoDb}
import org.bitcoins.core.api.wallet.{ import org.bitcoins.core.api.wallet.{CoinSelectionAlgo, CoinSelector}
AddressInfo,
CoinSelectionAlgo,
CoinSelector
}
import org.bitcoins.core.consensus.Consensus import org.bitcoins.core.consensus.Consensus
import org.bitcoins.core.protocol.transaction._ import org.bitcoins.core.protocol.transaction._
import org.bitcoins.core.util.FutureUtil 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.fee.FeeUnit
import org.bitcoins.core.wallet.utxo._ import org.bitcoins.core.wallet.utxo._
import org.bitcoins.crypto.Sign
import org.bitcoins.keymanager.bip39.BIP39KeyManager
import org.bitcoins.wallet.{Wallet, WalletLogger} import org.bitcoins.wallet.{Wallet, WalletLogger}
import scala.concurrent.Future import scala.concurrent.Future
@ -49,7 +43,6 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet =>
fundRawTransactionInternal(destinations = destinations, fundRawTransactionInternal(destinations = destinations,
feeRate = feeRate, feeRate = feeRate,
fromAccount = fromAccount, fromAccount = fromAccount,
keyManagerOpt = None,
fromTagOpt = fromTagOpt, fromTagOpt = fromTagOpt,
markAsReserved = markAsReserved) markAsReserved = markAsReserved)
.flatMap(_._1.buildTx()) .flatMap(_._1.buildTx())
@ -69,14 +62,13 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet =>
destinations: Vector[TransactionOutput], destinations: Vector[TransactionOutput],
feeRate: FeeUnit, feeRate: FeeUnit,
fromAccount: AccountDb, fromAccount: AccountDb,
keyManagerOpt: Option[BIP39KeyManager],
coinSelectionAlgo: CoinSelectionAlgo = coinSelectionAlgo: CoinSelectionAlgo =
CoinSelectionAlgo.AccumulateLargest, CoinSelectionAlgo.AccumulateLargest,
fromTagOpt: Option[AddressTag], fromTagOpt: Option[AddressTag],
markAsReserved: Boolean = false): Future[( markAsReserved: Boolean = false): Future[(
RawTxBuilderWithFinalizer[ShufflingNonInteractiveFinalizer], RawTxBuilderWithFinalizer[ShufflingNonInteractiveFinalizer],
Vector[ScriptSignatureParams[InputInfo]])] = { Vector[ScriptSignatureParams[InputInfo]])] = {
def utxosF: Future[Vector[SpendingInfoDb]] = def utxosF: Future[Vector[(SpendingInfoDb, Transaction)]] =
for { for {
utxos <- fromTagOpt match { utxos <- fromTagOpt match {
case None => case None =>
@ -105,53 +97,30 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet =>
confsOpt.isDefined && confsOpt.get < Consensus.coinbaseMaturity confsOpt.isDefined && confsOpt.get < Consensus.coinbaseMaturity
} }
.map(_._1) .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 { for {
walletUtxos <- utxosF walletUtxos <- utxosF
utxos = CoinSelector.selectByAlgo(coinSelectionAlgo = coinSelectionAlgo, utxos = CoinSelector.selectByAlgo(coinSelectionAlgo = coinSelectionAlgo,
walletUtxos = walletUtxos, walletUtxos = walletUtxos.map(_._1),
outputs = destinations, outputs = destinations,
feeRate = feeRate) feeRate = feeRate)
} yield utxos
val addrInfosWithUtxoF: Future[ } yield walletUtxos.filter(utxo => utxos.contains(utxo._1))
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
val resultF = for { val resultF = for {
addrInfosWithUtxo <- addrInfosWithUtxoF selectedUtxos <- selectedUtxosF
change <- getNewChangeAddress(fromAccount) change <- getNewChangeAddress(fromAccount)
utxoSpendingInfos = { utxoSpendingInfos = {
addrInfosWithUtxo.map { selectedUtxos.map {
case (utxo, prevTx, addrInfo) => case (utxo, prevTx) =>
keyManagerOpt match { utxo.toUTXOInfo(keyManager = self.keyManager, prevTx)
case Some(km) =>
utxo.toUTXOInfo(keyManager = km, prevTx)
case None =>
utxo.toUTXOInfo(sign = Sign.dummySign(addrInfo.pubkey), prevTx)
}
} }
} }
_ <- _ <-
if (markAsReserved) markUTXOsAsReserved(addrInfosWithUtxo.map(_._1)) if (markAsReserved) markUTXOsAsReserved(selectedUtxos.map(_._1))
else FutureUtil.unit else FutureUtil.unit
} yield { } yield {
logger.info { logger.info {
@ -191,7 +160,7 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet =>
if (markAsReserved) { if (markAsReserved) {
for { for {
utxos <- selectedUtxosF utxos <- selectedUtxosF
_ <- unmarkUTXOsAsReserved(utxos) _ <- unmarkUTXOsAsReserved(utxos.map(_._1))
} yield error } yield error
} else Future.failed(error) } else Future.failed(error)
} }