From 269d3c8f217cbd544ac8096c3ddb74286acf0d98 Mon Sep 17 00:00:00 2001 From: nkohen Date: Thu, 18 Apr 2019 16:05:52 -0500 Subject: [PATCH] Implemented simple greedy coin selection Added tests for current coin selection algorithm options Responded to code review Implemented fee estimation calculation in coin selection Updated tests after rebase Fixed CoinSelectorTest Fixed CoinSelectorTest after rebase --- .../wallet/api/CoinSelectorTest.scala | 94 ++++++++++++++++ .../scala/org/bitcoins/wallet/Wallet.scala | 15 ++- .../bitcoins/wallet/api/CoinSelector.scala | 101 ++++++++++++++++++ 3 files changed, 201 insertions(+), 9 deletions(-) create mode 100644 wallet-test/src/test/scala/org/bitcoins/wallet/api/CoinSelectorTest.scala create mode 100644 wallet/src/main/scala/org/bitcoins/wallet/api/CoinSelector.scala diff --git a/wallet-test/src/test/scala/org/bitcoins/wallet/api/CoinSelectorTest.scala b/wallet-test/src/test/scala/org/bitcoins/wallet/api/CoinSelectorTest.scala new file mode 100644 index 0000000000..c7497af87d --- /dev/null +++ b/wallet-test/src/test/scala/org/bitcoins/wallet/api/CoinSelectorTest.scala @@ -0,0 +1,94 @@ +package org.bitcoins.wallet.api + +import org.bitcoins.core.currency.{CurrencyUnits, Satoshis} +import org.bitcoins.core.number.Int64 +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.testkit.core.gen.{TransactionGenerators, WitnessGenerators} +import org.bitcoins.wallet.models.{ + NativeV0UTXOSpendingInfoDb, + UTXOSpendingInfoDb +} +import org.bitcoins.wallet.util.{BitcoinSWalletTest, WalletTestUtil} +import org.scalatest.FutureOutcome + +class CoinSelectorTest extends BitcoinSWalletTest { + case class CoinSelectionFixture( + output: TransactionOutput, + feeRate: FeeUnit, + utxo1: UTXOSpendingInfoDb, + utxo2: UTXOSpendingInfoDb, + utxo3: UTXOSpendingInfoDb) { + val utxoSet: Vector[UTXOSpendingInfoDb] = Vector(utxo1, utxo2, utxo3) + } + + override type FixtureParam = CoinSelectionFixture + + override def withFixture(test: OneArgAsyncTest): FutureOutcome = { + val output = TransactionOutput(Satoshis(Int64(99L)), ScriptPubKey.empty) + val feeRate = SatoshisPerByte(CurrencyUnits.zero) + + val utxo1 = NativeV0UTXOSpendingInfoDb( + id = Some(1), + outPoint = TransactionGenerators.outPoint.sample.get, + output = TransactionOutput(Satoshis(Int64(10)), ScriptPubKey.empty), + privKeyPath = WalletTestUtil.sampleSegwitPath, + scriptWitness = WitnessGenerators.scriptWitness.sample.get + ) + val utxo2 = NativeV0UTXOSpendingInfoDb( + id = Some(2), + outPoint = TransactionGenerators.outPoint.sample.get, + output = TransactionOutput(Satoshis(Int64(90)), ScriptPubKey.empty), + privKeyPath = WalletTestUtil.sampleSegwitPath, + scriptWitness = WitnessGenerators.scriptWitness.sample.get + ) + val utxo3 = NativeV0UTXOSpendingInfoDb( + id = Some(3), + outPoint = TransactionGenerators.outPoint.sample.get, + output = TransactionOutput(Satoshis(Int64(20)), ScriptPubKey.empty), + privKeyPath = WalletTestUtil.sampleSegwitPath, + scriptWitness = WitnessGenerators.scriptWitness.sample.get + ) + + test(CoinSelectionFixture(output, feeRate, utxo1, utxo2, utxo3)) + } + + behavior of "CoinSelector" + + it must "accumulate largest outputs" in { fixture => + val selection = + CoinSelector.accumulateLargest(walletUtxos = fixture.utxoSet, + outputs = Vector(fixture.output), + feeRate = fixture.feeRate) + + assert(selection == Vector(fixture.utxo2, fixture.utxo3)) + } + + it must "accumulate smallest outputs" in { fixture => + val selection = + CoinSelector.accumulateSmallestViable(walletUtxos = fixture.utxoSet, + outputs = Vector(fixture.output), + feeRate = fixture.feeRate) + + assert(selection == Vector(fixture.utxo1, fixture.utxo3, fixture.utxo2)) + } + + it must "accumulate outputs in order" in { fixture => + val selection = CoinSelector.accumulate(walletUtxos = fixture.utxoSet, + outputs = Vector(fixture.output), + feeRate = fixture.feeRate) + + assert(selection == Vector(fixture.utxo1, fixture.utxo2)) + } + + it must "correctly approximate transaction input size" in { fixture => + val expected1 = 32 + 4 + 1 + 4 + fixture.utxo1.scriptWitnessOpt.get.bytes.length + val expected2 = 32 + 4 + 1 + 4 + fixture.utxo2.scriptWitnessOpt.get.bytes.length + val expected3 = 32 + 4 + 1 + 4 + fixture.utxo3.scriptWitnessOpt.get.bytes.length + + assert(CoinSelector.approximateUtxoSize(fixture.utxo1) == expected1) + assert(CoinSelector.approximateUtxoSize(fixture.utxo2) == expected2) + assert(CoinSelector.approximateUtxoSize(fixture.utxo3) == expected3) + } +} diff --git a/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala b/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala index d56c1924df..bf380302ce 100644 --- a/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala +++ b/wallet/src/main/scala/org/bitcoins/wallet/Wallet.scala @@ -50,17 +50,14 @@ sealed abstract class Wallet change <- getNewChangeAddress(fromAccount) walletUtxos <- listUtxos() txBuilder <- { - val destinations: Seq[TransactionOutput] = List( + val destinations = Vector( TransactionOutput(amount, address.scriptPubKey)) - // currencly just grabs one utxos, throws if can't find big enough - // todo: implement coin selection - val utxos: List[BitcoinUTXOSpendingInfo] = - List( - walletUtxos - .find(_.value >= amount) - .get - .toUTXOSpendingInfo(fromAccount, seed)) + // currencly just grabs the biggest utxos until it finds enough + val utxos: Vector[BitcoinUTXOSpendingInfo] = + CoinSelector + .accumulateLargest(walletUtxos, destinations, feeRate) + .map(_.toUTXOSpendingInfo(fromAccount, seed)) logger.info({ val utxosStr = utxos diff --git a/wallet/src/main/scala/org/bitcoins/wallet/api/CoinSelector.scala b/wallet/src/main/scala/org/bitcoins/wallet/api/CoinSelector.scala new file mode 100644 index 0000000000..99bc5fbf89 --- /dev/null +++ b/wallet/src/main/scala/org/bitcoins/wallet/api/CoinSelector.scala @@ -0,0 +1,101 @@ +package org.bitcoins.wallet.api + +import org.bitcoins.core.currency.{CurrencyUnit, CurrencyUnits} +import org.bitcoins.core.protocol.transaction.TransactionOutput +import org.bitcoins.core.wallet.fee.FeeUnit +import org.bitcoins.wallet.models.UTXOSpendingInfoDb + +import scala.annotation.tailrec + +/** Implements algorithms for selecting from a UTXO set to spend to an output set at a given fee rate. */ +trait CoinSelector { + + /** + * Greedily selects from walletUtxos starting with the largest outputs, skipping outputs with values + * below their fees. Better for high fee environments than accumulateSmallestViable. + */ + def accumulateLargest( + walletUtxos: Vector[UTXOSpendingInfoDb], + outputs: Vector[TransactionOutput], + feeRate: FeeUnit): Vector[UTXOSpendingInfoDb] = { + val sortedUtxos = + walletUtxos.sortBy(_.value.satoshis.toLong).reverse + + accumulate(sortedUtxos, outputs, feeRate) + } + + /** + * Greedily selects from walletUtxos starting with the smallest outputs, skipping outputs with values + * below their fees. Good for low fee environments to consolidate UTXOs. + * + * Has the potential privacy breach of connecting a ton of UTXOs to one address. + */ + def accumulateSmallestViable( + walletUtxos: Vector[UTXOSpendingInfoDb], + outputs: Vector[TransactionOutput], + feeRate: FeeUnit): Vector[UTXOSpendingInfoDb] = { + val sortedUtxos = walletUtxos.sortBy(_.value.satoshis.toLong) + + accumulate(sortedUtxos, outputs, feeRate) + } + + /** Greedily selects from walletUtxos in order, skipping outputs with values below their fees */ + def accumulate( + walletUtxos: Vector[UTXOSpendingInfoDb], + outputs: Vector[TransactionOutput], + feeRate: FeeUnit): Vector[UTXOSpendingInfoDb] = { + val totalValue = outputs.foldLeft(CurrencyUnits.zero) { + case (totVal, output) => totVal + output.value + } + + @tailrec + def addUtxos( + alreadyAdded: Vector[UTXOSpendingInfoDb], + valueSoFar: CurrencyUnit, + bytesSoFar: Long, + utxosLeft: Vector[UTXOSpendingInfoDb]): Vector[UTXOSpendingInfoDb] = { + val fee = feeRate.currencyUnit * bytesSoFar + if (valueSoFar > totalValue + fee) { + alreadyAdded + } else if (utxosLeft.isEmpty) { + throw new RuntimeException( + s"Not enough value in given outputs ($valueSoFar) to make transaction spending $totalValue") + } else { + val nextUtxo = utxosLeft.head + val approxUtxoSize = CoinSelector.approximateUtxoSize(nextUtxo) + val nextUtxoFee = feeRate.currencyUnit * approxUtxoSize + if (nextUtxo.value < nextUtxoFee) { + addUtxos(alreadyAdded, valueSoFar, bytesSoFar, utxosLeft.tail) + } else { + val newAdded = alreadyAdded.:+(nextUtxo) + val newValue = valueSoFar + nextUtxo.value + + addUtxos(newAdded, + newValue, + bytesSoFar + approxUtxoSize, + utxosLeft.tail) + } + } + } + + addUtxos(Vector.empty, CurrencyUnits.zero, bytesSoFar = 0L, walletUtxos) + } +} + +object CoinSelector extends CoinSelector { + + /** Cribbed from [[https://github.com/bitcoinjs/coinselect/blob/master/utils.js]] */ + def approximateUtxoSize(utxo: UTXOSpendingInfoDb): 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 => 25 // PUBKEYHASH + } + } + + inputBase + scriptSize + } +}