From c210052640ab9e84c18f318208ac6d6798b8053d Mon Sep 17 00:00:00 2001 From: benthecarman Date: Wed, 20 Jul 2022 08:40:11 -0500 Subject: [PATCH] Refactor coin selection to be not be bitcoin-s specific (#4496) * Refactor coin selection to be not be bitcoin-s specific * Add to CoinSelectorUtxo --- .../core/wallet/CoinSelectorTest.scala | 7 +- .../core/api/wallet/CoinSelector.scala | 64 +++++++++++-------- .../core/api/wallet/CoinSelectorUtxo.scala | 21 ++++++ .../core/api/wallet/db/SpendingInfoDb.scala | 5 ++ .../bitcoins/wallet/CoinSelectorTest.scala | 49 ++++++-------- .../bitcoins/wallet/WalletSendingTest.scala | 7 +- .../internal/FundTransactionHandling.scala | 6 +- 7 files changed, 95 insertions(+), 64 deletions(-) create mode 100644 core/src/main/scala/org/bitcoins/core/api/wallet/CoinSelectorUtxo.scala diff --git a/core-test/src/test/scala/org/bitcoins/core/wallet/CoinSelectorTest.scala b/core-test/src/test/scala/org/bitcoins/core/wallet/CoinSelectorTest.scala index 5be5284d3f..6b62dc4245 100644 --- a/core-test/src/test/scala/org/bitcoins/core/wallet/CoinSelectorTest.scala +++ b/core-test/src/test/scala/org/bitcoins/core/wallet/CoinSelectorTest.scala @@ -1,6 +1,6 @@ package org.bitcoins.core.wallet -import org.bitcoins.core.api.wallet.CoinSelector +import org.bitcoins.core.api.wallet.{CoinSelector, CoinSelectorUtxo} import org.bitcoins.core.api.wallet.db._ import org.bitcoins.core.currency._ import org.bitcoins.core.hd._ @@ -16,9 +16,10 @@ class CoinSelectorTest extends BitcoinSUnitTest { behavior of "CoinSelector" - val utxos: Vector[SpendingInfoDb] = + val utxos: Vector[CoinSelectorUtxo] = createSpendingInfoDbs(Vector(Bitcoins(1), Bitcoins(2))) - val inAmt: CurrencyUnit = utxos.map(_.output.value).sum + .map(CoinSelectorUtxo.fromSpendingInfoDb) + val inAmt: CurrencyUnit = utxos.map(_.prevOut.value).sum val target: Bitcoins = Bitcoins(2) val changeCost: Satoshis = Satoshis.one diff --git a/core/src/main/scala/org/bitcoins/core/api/wallet/CoinSelector.scala b/core/src/main/scala/org/bitcoins/core/api/wallet/CoinSelector.scala index 7a2d3660f5..bbfd831c83 100644 --- a/core/src/main/scala/org/bitcoins/core/api/wallet/CoinSelector.scala +++ b/core/src/main/scala/org/bitcoins/core/api/wallet/CoinSelector.scala @@ -1,9 +1,9 @@ package org.bitcoins.core.api.wallet import org.bitcoins.core.api.wallet.CoinSelectionAlgo._ -import org.bitcoins.core.api.wallet.db.SpendingInfoDb import org.bitcoins.core.currency._ -import org.bitcoins.core.protocol.transaction.TransactionOutput +import org.bitcoins.core.protocol.script._ +import org.bitcoins.core.protocol.transaction._ import org.bitcoins.core.wallet.fee.FeeUnit import scala.annotation.tailrec @@ -16,9 +16,9 @@ trait CoinSelector { * should only be used for research purposes */ def randomSelection( - walletUtxos: Vector[SpendingInfoDb], + walletUtxos: Vector[CoinSelectorUtxo], outputs: Vector[TransactionOutput], - feeRate: FeeUnit): Vector[SpendingInfoDb] = { + feeRate: FeeUnit): Vector[CoinSelectorUtxo] = { val randomUtxos = Random.shuffle(walletUtxos) accumulate(randomUtxos, outputs, feeRate) @@ -28,11 +28,11 @@ trait CoinSelector { * below their fees. Better for high fee environments than accumulateSmallestViable. */ def accumulateLargest( - walletUtxos: Vector[SpendingInfoDb], + walletUtxos: Vector[CoinSelectorUtxo], outputs: Vector[TransactionOutput], - feeRate: FeeUnit): Vector[SpendingInfoDb] = { + feeRate: FeeUnit): Vector[CoinSelectorUtxo] = { val sortedUtxos = - walletUtxos.sortBy(_.output.value).reverse + walletUtxos.sortBy(_.prevOut.value).reverse accumulate(sortedUtxos, outputs, feeRate) } @@ -43,29 +43,29 @@ trait CoinSelector { * Has the potential privacy breach of connecting a ton of UTXOs to one address. */ def accumulateSmallestViable( - walletUtxos: Vector[SpendingInfoDb], + walletUtxos: Vector[CoinSelectorUtxo], outputs: Vector[TransactionOutput], - feeRate: FeeUnit): Vector[SpendingInfoDb] = { - val sortedUtxos = walletUtxos.sortBy(_.output.value) + feeRate: FeeUnit): Vector[CoinSelectorUtxo] = { + val sortedUtxos = walletUtxos.sortBy(_.prevOut.value) accumulate(sortedUtxos, outputs, feeRate) } /** Greedily selects from walletUtxos in order, skipping outputs with values below their fees */ def accumulate( - walletUtxos: Vector[SpendingInfoDb], + walletUtxos: Vector[CoinSelectorUtxo], outputs: Vector[TransactionOutput], - feeRate: FeeUnit): Vector[SpendingInfoDb] = { + feeRate: FeeUnit): Vector[CoinSelectorUtxo] = { val totalValue = outputs.foldLeft(CurrencyUnits.zero) { case (totVal, output) => totVal + output.value } @tailrec def addUtxos( - alreadyAdded: Vector[SpendingInfoDb], + alreadyAdded: Vector[CoinSelectorUtxo], valueSoFar: CurrencyUnit, bytesSoFar: Long, - utxosLeft: Vector[SpendingInfoDb]): Vector[SpendingInfoDb] = { + utxosLeft: Vector[CoinSelectorUtxo]): Vector[CoinSelectorUtxo] = { val fee = feeRate * bytesSoFar if (valueSoFar > totalValue + fee) { alreadyAdded @@ -79,7 +79,7 @@ trait CoinSelector { addUtxos(alreadyAdded, valueSoFar, bytesSoFar, utxosLeft.tail) } else { val newAdded = alreadyAdded.:+(nextUtxo) - val newValue = valueSoFar + nextUtxo.output.value + val newValue = valueSoFar + nextUtxo.prevOut.value val approxUtxoSize = CoinSelector.approximateUtxoSize(nextUtxo) addUtxos(newAdded, @@ -93,30 +93,38 @@ trait CoinSelector { addUtxos(Vector.empty, CurrencyUnits.zero, bytesSoFar = 0L, walletUtxos) } - def calculateUtxoFee(utxo: SpendingInfoDb, feeRate: FeeUnit): CurrencyUnit = { + def calculateUtxoFee( + utxo: CoinSelectorUtxo, + feeRate: FeeUnit): CurrencyUnit = { val approxUtxoSize = CoinSelector.approximateUtxoSize(utxo) feeRate * approxUtxoSize } def calcEffectiveValue( - utxo: SpendingInfoDb, + utxo: CoinSelectorUtxo, feeRate: FeeUnit): CurrencyUnit = { val utxoFee = calculateUtxoFee(utxo, feeRate) - utxo.output.value - utxoFee + utxo.prevOut.value - utxoFee } } object CoinSelector extends CoinSelector { /** Cribbed from [[https://github.com/bitcoinjs/coinselect/blob/master/utils.js]] */ - def approximateUtxoSize(utxo: SpendingInfoDb): Long = { + def approximateUtxoSize(utxo: CoinSelectorUtxo): Long = { val inputBase = 32 + 4 + 1 + 4 val scriptSize = utxo.redeemScriptOpt match { case Some(script) => script.bytes.length case None => utxo.scriptWitnessOpt match { case Some(script) => script.bytes.length - case None => 107 // PUBKEYHASH + case None => + utxo.prevOut.scriptPubKey match { + case _: NonWitnessScriptPubKey => 107 // P2PKH + case _: WitnessScriptPubKeyV0 => 107 // P2WPKH + case _: TaprootScriptPubKey => 64 // Single Schnorr signature + case _: UnassignedWitnessScriptPubKey => 0 // unknown + } } } @@ -125,10 +133,10 @@ object CoinSelector extends CoinSelector { def selectByAlgo( coinSelectionAlgo: CoinSelectionAlgo, - walletUtxos: Vector[SpendingInfoDb], + walletUtxos: Vector[CoinSelectorUtxo], outputs: Vector[TransactionOutput], feeRate: FeeUnit, - longTermFeeRateOpt: Option[FeeUnit] = None): Vector[SpendingInfoDb] = + longTermFeeRateOpt: Option[FeeUnit] = None): Vector[CoinSelectorUtxo] = coinSelectionAlgo match { case RandomSelection => randomSelection(walletUtxos, outputs, feeRate) @@ -147,7 +155,7 @@ object CoinSelector extends CoinSelector { "longTermFeeRateOpt must be defined for LeastWaste") } case SelectedUtxos(outPoints) => - val result = walletUtxos.foldLeft(Vector.empty[SpendingInfoDb]) { + val result = walletUtxos.foldLeft(Vector.empty[CoinSelectorUtxo]) { (acc, utxo) => val outPoint = (utxo.outPoint.txId, utxo.outPoint.vout.toInt) if (outPoints(outPoint)) acc :+ utxo else acc @@ -170,7 +178,7 @@ object CoinSelector extends CoinSelector { private case class CoinSelectionResults( waste: CurrencyUnit, totalSpent: CurrencyUnit, - selection: Vector[SpendingInfoDb]) + selection: Vector[CoinSelectorUtxo]) implicit private val coinSelectionResultsOrder: Ordering[CoinSelectionResults] = { @@ -181,11 +189,11 @@ object CoinSelector extends CoinSelector { } def selectByLeastWaste( - walletUtxos: Vector[SpendingInfoDb], + walletUtxos: Vector[CoinSelectorUtxo], outputs: Vector[TransactionOutput], feeRate: FeeUnit, longTermFeeRate: FeeUnit - ): Vector[SpendingInfoDb] = { + ): Vector[CoinSelectorUtxo] = { val target = outputs.map(_.value).sum val results = CoinSelectionAlgo.independentAlgos.flatMap { algo => // Skip failed selection attempts @@ -207,7 +215,7 @@ object CoinSelector extends CoinSelector { feeRate, longTermFeeRate) - val totalSpent = selection.map(_.output.value).sum + val totalSpent = selection.map(_.prevOut.value).sum CoinSelectionResults(waste, totalSpent, selection) }.toOption } @@ -236,7 +244,7 @@ object CoinSelector extends CoinSelector { * @return The waste */ def calculateSelectionWaste( - utxos: Vector[SpendingInfoDb], + utxos: Vector[CoinSelectorUtxo], changeCostOpt: Option[CurrencyUnit], target: CurrencyUnit, feeRate: FeeUnit, diff --git a/core/src/main/scala/org/bitcoins/core/api/wallet/CoinSelectorUtxo.scala b/core/src/main/scala/org/bitcoins/core/api/wallet/CoinSelectorUtxo.scala new file mode 100644 index 0000000000..261df289b9 --- /dev/null +++ b/core/src/main/scala/org/bitcoins/core/api/wallet/CoinSelectorUtxo.scala @@ -0,0 +1,21 @@ +package org.bitcoins.core.api.wallet + +import org.bitcoins.core.api.wallet.db.SpendingInfoDb +import org.bitcoins.core.protocol.script._ +import org.bitcoins.core.protocol.transaction._ + +case class CoinSelectorUtxo( + prevOut: TransactionOutput, + outPoint: TransactionOutPoint, + redeemScriptOpt: Option[ScriptPubKey], + scriptWitnessOpt: Option[ScriptWitness]) + +object CoinSelectorUtxo { + + def fromSpendingInfoDb(db: SpendingInfoDb): CoinSelectorUtxo = { + CoinSelectorUtxo(db.output, + db.outPoint, + db.redeemScriptOpt, + db.scriptWitnessOpt) + } +} diff --git a/core/src/main/scala/org/bitcoins/core/api/wallet/db/SpendingInfoDb.scala b/core/src/main/scala/org/bitcoins/core/api/wallet/db/SpendingInfoDb.scala index 3d69a14189..824b71ac0e 100644 --- a/core/src/main/scala/org/bitcoins/core/api/wallet/db/SpendingInfoDb.scala +++ b/core/src/main/scala/org/bitcoins/core/api/wallet/db/SpendingInfoDb.scala @@ -2,6 +2,7 @@ package org.bitcoins.core.api.wallet.db import org.bitcoins.core.api.db.DbRowAutoInc import org.bitcoins.core.api.keymanager.BIP39KeyManagerApi +import org.bitcoins.core.api.wallet.CoinSelectorUtxo import org.bitcoins.core.hd._ import org.bitcoins.core.protocol.script.{ P2SHScriptPubKey, @@ -195,6 +196,10 @@ sealed trait SpendingInfoDb extends DbRowAutoInc[SpendingInfoDb] { hashType ) } + + def toCoinSelectorUtxo: CoinSelectorUtxo = { + CoinSelectorUtxo.fromSpendingInfoDb(this) + } } object SpendingInfoDb { diff --git a/wallet-test/src/test/scala/org/bitcoins/wallet/CoinSelectorTest.scala b/wallet-test/src/test/scala/org/bitcoins/wallet/CoinSelectorTest.scala index 02f651208d..86f68a3ea0 100644 --- a/wallet-test/src/test/scala/org/bitcoins/wallet/CoinSelectorTest.scala +++ b/wallet-test/src/test/scala/org/bitcoins/wallet/CoinSelectorTest.scala @@ -1,13 +1,11 @@ package org.bitcoins.wallet -import org.bitcoins.core.api.wallet.CoinSelector -import org.bitcoins.core.api.wallet.db.{SegwitV0SpendingInfo, SpendingInfoDb} +import org.bitcoins.core.api.wallet.{CoinSelector, CoinSelectorUtxo} import org.bitcoins.core.currency._ import org.bitcoins.core.protocol.script.ScriptPubKey import org.bitcoins.core.protocol.transaction.TransactionOutput import org.bitcoins.core.wallet.fee.{FeeUnit, SatoshisPerByte} -import org.bitcoins.core.wallet.utxo.TxoState -import org.bitcoins.testkit.wallet.{BitcoinSWalletTest, WalletTestUtil} +import org.bitcoins.testkit.wallet.BitcoinSWalletTest import org.bitcoins.testkitcore.Implicits._ import org.bitcoins.testkitcore.gen.{TransactionGenerators, WitnessGenerators} import org.scalatest.FutureOutcome @@ -17,10 +15,12 @@ class CoinSelectorTest extends BitcoinSWalletTest { case class CoinSelectionFixture( output: TransactionOutput, feeRate: FeeUnit, - utxo1: SpendingInfoDb, - utxo2: SpendingInfoDb, - utxo3: SpendingInfoDb) { - val utxoSet: Vector[SpendingInfoDb] = Vector(utxo1, utxo2, utxo3) + utxo1: CoinSelectorUtxo, + utxo2: CoinSelectorUtxo, + utxo3: CoinSelectorUtxo) { + + val utxoSet: Vector[CoinSelectorUtxo] = Vector(utxo1, utxo2, utxo3) + } override type FixtureParam = CoinSelectionFixture @@ -30,35 +30,26 @@ class CoinSelectorTest extends BitcoinSWalletTest { val feeRate = SatoshisPerByte(CurrencyUnits.zero) val outpoint1 = TransactionGenerators.outPoint.sampleSome - val utxo1 = SegwitV0SpendingInfo( - state = TxoState.PendingConfirmationsReceived, - id = Some(1), + val utxo1 = CoinSelectorUtxo( outPoint = outpoint1, - output = TransactionOutput(10.sats, ScriptPubKey.empty), - privKeyPath = WalletTestUtil.sampleSegwitPath, - scriptWitness = WitnessGenerators.scriptWitness.sampleSome, - spendingTxIdOpt = None + prevOut = TransactionOutput(10.sats, ScriptPubKey.empty), + scriptWitnessOpt = Some(WitnessGenerators.scriptWitness.sampleSome), + redeemScriptOpt = None ) val outPoint2 = TransactionGenerators.outPoint.sampleSome - val utxo2 = SegwitV0SpendingInfo( - state = TxoState.ConfirmedReceived, - id = Some(2), + val utxo2 = CoinSelectorUtxo( outPoint = outPoint2, - output = TransactionOutput(90.sats, ScriptPubKey.empty), - privKeyPath = WalletTestUtil.sampleSegwitPath, - scriptWitness = WitnessGenerators.scriptWitness.sampleSome, - spendingTxIdOpt = None + prevOut = TransactionOutput(90.sats, ScriptPubKey.empty), + scriptWitnessOpt = Some(WitnessGenerators.scriptWitness.sampleSome), + redeemScriptOpt = None ) val outPoint3 = TransactionGenerators.outPoint.sampleSome - val utxo3 = SegwitV0SpendingInfo( - state = TxoState.ConfirmedReceived, - id = Some(3), + val utxo3 = CoinSelectorUtxo( outPoint = outPoint3, - output = TransactionOutput(20.sats, ScriptPubKey.empty), - privKeyPath = WalletTestUtil.sampleSegwitPath, - scriptWitness = WitnessGenerators.scriptWitness.sampleSome, - spendingTxIdOpt = None + prevOut = TransactionOutput(20.sats, ScriptPubKey.empty), + scriptWitnessOpt = Some(WitnessGenerators.scriptWitness.sampleSome), + redeemScriptOpt = None ) test(CoinSelectionFixture(output, feeRate, utxo1, utxo2, utxo3)) diff --git a/wallet-test/src/test/scala/org/bitcoins/wallet/WalletSendingTest.scala b/wallet-test/src/test/scala/org/bitcoins/wallet/WalletSendingTest.scala index 0bd577bec4..b3f6912436 100644 --- a/wallet-test/src/test/scala/org/bitcoins/wallet/WalletSendingTest.scala +++ b/wallet-test/src/test/scala/org/bitcoins/wallet/WalletSendingTest.scala @@ -1,6 +1,6 @@ package org.bitcoins.wallet -import org.bitcoins.core.api.wallet.{CoinSelectionAlgo, CoinSelector} +import org.bitcoins.core.api.wallet._ import org.bitcoins.core.currency._ import org.bitcoins.core.number.{Int32, UInt32} import org.bitcoins.core.protocol.BitcoinAddress @@ -464,7 +464,10 @@ class WalletSendingTest extends BitcoinSWalletTest { for { account <- wallet.getDefaultAccount() feeRate <- wallet.getFeeRate() - allUtxos <- wallet.listUtxos(account.hdAccount) + allUtxos <- wallet + .listUtxos(account.hdAccount) + .map(_.map(CoinSelectorUtxo.fromSpendingInfoDb)) + output = TransactionOutput(amountToSend, testAddress.scriptPubKey) expectedUtxos = CoinSelector.selectByAlgo(algo, allUtxos, Vector(output), feeRate) 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 866c8b0194..16bde40126 100644 --- a/wallet/src/main/scala/org/bitcoins/wallet/internal/FundTransactionHandling.scala +++ b/wallet/src/main/scala/org/bitcoins/wallet/internal/FundTransactionHandling.scala @@ -1,7 +1,7 @@ package org.bitcoins.wallet.internal import org.bitcoins.core.api.wallet.db.{AccountDb, SpendingInfoDb} -import org.bitcoins.core.api.wallet.{CoinSelectionAlgo, CoinSelector} +import org.bitcoins.core.api.wallet._ import org.bitcoins.core.policy.Policy import org.bitcoins.core.protocol.transaction._ import org.bitcoins.core.wallet.builder._ @@ -93,6 +93,7 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet => selectableUtxos = walletUtxos .map(_._1) .filter(_.output.value > Policy.dustThreshold) + .map(CoinSelectorUtxo.fromSpendingInfoDb) utxos = CoinSelector.selectByAlgo( coinSelectionAlgo = coinSelectionAlgo, @@ -101,7 +102,8 @@ trait FundTransactionHandling extends WalletLogger { self: Wallet => feeRate = feeRate, longTermFeeRateOpt = Some(self.walletConfig.longTermFeeRate) ) - filtered = walletUtxos.filter(utxo => utxos.contains(utxo._1)) + filtered = walletUtxos.filter(utxo => + utxos.exists(_.outPoint == utxo._1.outPoint)) _ <- if (markAsReserved) markUTXOsAsReserved(filtered.map(_._1)) else Future.unit