VG-13274 - Confirmed-first ordering of utxos when crafting a transaction + add configuration

core/idl/wallet/configuration.djinni

@@ -127,6 +127,9 @@ Configuration = interface +c {
 
  # Allow the generation of the P2TR (Taproot) outputs
  const ALLOW_P2TR: string = "ALLOW_P2TR";
+
+ # Use confirmed UTXOs first in utxo picking strategies
+ const CONFIRMED_UTXO_FIRST: string = "CONFIRMED_UTXO_FIRST";
 }
 
 # Configuration of wallet pools.

core/src/wallet/bitcoin/BitcoinLikeAccount.cpp

@@ -185,7 +185,10 @@ namespace ledger {
  _synchronizer = synchronizer;
  _keychain = keychain;
  _keychain->getAllObservableAddresses(0, 40);
- _picker = std::make_shared<BitcoinLikeStrategyUtxoPicker>(getWallet()->getPool()->getThreadPoolExecutionContext(), getWallet()->getCurrency());
+ _picker = std::make_shared<BitcoinLikeStrategyUtxoPicker>(
+ getWallet()->getPool()->getThreadPoolExecutionContext(),
+ getWallet()->getCurrency(),
+ getWallet()->getConfig()->getBoolean(api::Configuration::CONFIRMED_UTXO_FIRST).value_or(true));
  _currentBlockHeight = 0;
  }
 

core/src/wallet/bitcoin/transaction_builders/BitcoinLikeStrategyUtxoPicker.cpp

@@ -43,10 +43,24 @@
 namespace ledger {
  namespace core {
 
- BitcoinLikeStrategyUtxoPicker::BitcoinLikeStrategyUtxoPicker(const std::shared_ptr<api::ExecutionContext> &context,
- const api::Currency &currency) : BitcoinLikeUtxoPicker(context, currency) {
+ std::optional<bool> compareConfirmation(const BitcoinLikeUtxo &u1, const BitcoinLikeUtxo &u2) {
+ bool isU1Confirmed = u1.blockHeight.hasValue();
+ bool isU2Confirmed = u2.blockHeight.hasValue();
+
+ if ((isU1Confirmed && isU2Confirmed) || (!isU1Confirmed && !isU2Confirmed)) {
+ // ignore the case where confirmation status is similar
+ return {};
+ }
+
+ // otherwise confirmed utxo should be first
+ return isU1Confirmed;
  }
 
+ BitcoinLikeStrategyUtxoPicker::BitcoinLikeStrategyUtxoPicker(const std::shared_ptr<api::ExecutionContext> &context,
+ const api::Currency &currency,
+ bool useConfirmedFirst)
+ : BitcoinLikeUtxoPicker(context, currency), _useConfirmedFirst{useConfirmedFirst} {}
+
  Future<std::vector<BitcoinLikeUtxo>>
  BitcoinLikeStrategyUtxoPicker::filterInputs(const std::shared_ptr<BitcoinLikeUtxoPicker::Buddy> &buddy) {
  return computeAggregatedAmount(buddy).flatMap<std::vector<BitcoinLikeUtxo>>(getContext(), [=](BigInt const &amount) {
@@ -71,9 +85,9 @@ namespace ledger {
  case api::BitcoinLikePickingStrategy::DEEP_OUTPUTS_FIRST:
  return filterWithDeepFirst(buddy, utxos, amount, getCurrency());
  case api::BitcoinLikePickingStrategy::OPTIMIZE_SIZE:
- return filterWithOptimizeSize(buddy, utxos, amount, getCurrency());
+ return filterWithOptimizeSize(buddy, utxos, amount, getCurrency(), _useConfirmedFirst);
  case api::BitcoinLikePickingStrategy::MERGE_OUTPUTS:
- return filterWithMergeOutputs(buddy, utxos, amount, getCurrency());
+ return filterWithMergeOutputs(buddy, utxos, amount, getCurrency(), _useConfirmedFirst);
  }
 
  throw make_exception(api::ErrorCode::ILLEGAL_ARGUMENT, "Unknown UTXO picking strategy.");
@@ -173,7 +187,8 @@ namespace ledger {
  BitcoinLikeStrategyUtxoPicker::filterWithOptimizeSize(const std::shared_ptr<BitcoinLikeUtxoPicker::Buddy> &buddy,
  const std::vector<BitcoinLikeUtxo> &utxos,
  const BigInt &aggregatedAmount,
- const api::Currency &currency) {
+ const api::Currency &currency,
+ bool useConfirmedFirst) {
  // NOTE: why are we using buddy->outputAmount here instead of aggregatedAmount ?
  // Don't use this strategy for wipe mode (we have more performent strategies for this use case)
  if (buddy->request.wipe) {
@@ -272,7 +287,13 @@ namespace ledger {
  throw make_exception(api::ErrorCode::NOT_ENOUGH_FUNDS, "Cannot gather enough funds.");
  }
 
- auto descendingEffectiveValue = [](const EffectiveUtxo &lhs, const EffectiveUtxo &rhs) -> bool {
+ auto descendingEffectiveValue = [useConfirmedFirst](const EffectiveUtxo &lhs, const EffectiveUtxo &rhs) -> bool {
+ if (useConfirmedFirst) {
+ std::optional<bool> comp = compareConfirmation(*lhs.utxo, *rhs.utxo);
+ if (comp.has_value()) {
+ return comp.value();
+ }
+ }
  return lhs.effectiveValue > rhs.effectiveValue;
  };
 
@@ -347,7 +368,7 @@ namespace ledger {
  // If no selection found fallback on filterWithDeepFirst
  if (bestSelection.empty()) {
  buddy->logger->debug("No best selection found, fallback on filterWithKnapsackSolver coin selection");
- return filterWithKnapsackSolver(buddy, utxos, aggregatedAmount, currency);
+ return filterWithKnapsackSolver(buddy, utxos, aggregatedAmount, currency, useConfirmedFirst);
  }
 
  // Prepare result
@@ -414,7 +435,8 @@ namespace ledger {
  const std::shared_ptr<BitcoinLikeUtxoPicker::Buddy> &buddy,
  const std::vector<BitcoinLikeUtxo> &utxos,
  const BigInt &aggregatedAmount,
- const api::Currency &currency) {
+ const api::Currency &currency,
+ bool useConfirmedFirst) {
  // Tx fixed size
  auto const fixedSize = BitcoinLikeTransactionApi::estimateSize(0,
  0,
@@ -473,11 +495,39 @@ namespace ledger {
  std::vector<BitcoinLikeUtxo> out;
 
  // Random shuffle utxos
- std::vector<size_t> indexes(utxos.size());
- std::iota(indexes.begin(), indexes.end(), 0);
+ const auto &sz = utxos.size();
+ std::vector<size_t> indexes;
+ indexes.reserve(sz);
+ {
+ auto const seed = std::chrono::system_clock::now().time_since_epoch().count();
+
+ if (useConfirmedFirst) {
+ std::vector<size_t> indexesTmp(sz, 0);
+ std::iota(indexesTmp.begin(), indexesTmp.end(), 0);
+ std::shuffle(indexesTmp.begin(), indexesTmp.end(), std::default_random_engine(seed));
+
+ std::list<size_t> allindexes;
+ std::list<size_t> confirmed;
+ std::list<size_t> unconfirmed;
+
+ for (const auto idx : indexesTmp) {
+ if (utxos[idx].blockHeight.hasValue()) {
+ confirmed.emplace_back(idx);
+ } else {
+ unconfirmed.emplace_back(idx);
+ }
+ }
 
- auto const seed = std::chrono::system_clock::now().time_since_epoch().count();
- std::shuffle(indexes.begin(), indexes.end(), std::default_random_engine(seed));
+ allindexes.splice(allindexes.end(), confirmed);
+ allindexes.splice(allindexes.end(), unconfirmed);
+ std::move(std::begin(allindexes), std::end(allindexes), std::back_inserter(indexes));
+
+ } else {
+ indexes.resize(sz);
+ std::iota(indexes.begin(), indexes.end(), 0);
+ std::shuffle(indexes.begin(), indexes.end(), std::default_random_engine(seed));
+ }
+ }
 
  // Add fees for a signed input to amount
  for (auto index : indexes) {
@@ -516,7 +566,13 @@ namespace ledger {
  }
 
  // Sort vUTXOs descending
- std::sort(vUTXOs.begin(), vUTXOs.end(), [](auto const &lhs, auto const &rhs) {
+ std::sort(vUTXOs.begin(), vUTXOs.end(), [useConfirmedFirst](auto const &lhs, auto const &rhs) {
+ if (useConfirmedFirst) {
+ std::optional<bool> comp = compareConfirmation(lhs, rhs);
+ if (comp.has_value()) {
+ return comp.value();
+ }
+ }
  return lhs.value.toLong() > rhs.value.toLong();
  });
 
@@ -586,10 +642,18 @@ namespace ledger {
  const std::shared_ptr<BitcoinLikeUtxoPicker::Buddy> &buddy,
  const std::vector<BitcoinLikeUtxo> &utxos,
  const BigInt &aggregatedAmount,
- const api::Currency &currency) {
+ const api::Currency &currency,
+ bool useConfirmedFirst) {
  buddy->logger->debug("Start filterWithMergeOutputs");
 
- return filterWithSort(buddy, utxos, aggregatedAmount, currency, [](auto &lhs, auto &rhs) {
+ return filterWithSort(buddy, utxos, aggregatedAmount, currency, [useConfirmedFirst](auto &lhs, auto &rhs) {
+ if (useConfirmedFirst) {
+ std::optional<bool> comp = compareConfirmation(lhs, rhs);
+ if (comp.has_value()) {
+ return comp.value();
+ }
+ }
+
  return lhs.value.toLong() < rhs.value.toLong();
  });
  }
@@ -631,5 +695,6 @@ namespace ledger {
 
  return pickedUtxos;
  }
+
  } // namespace core
 } // namespace ledger

core/src/wallet/bitcoin/transaction_builders/BitcoinLikeStrategyUtxoPicker.h

@@ -46,27 +46,33 @@ namespace ledger {
  class BitcoinLikeStrategyUtxoPicker : public BitcoinLikeUtxoPicker {
  public:
  BitcoinLikeStrategyUtxoPicker(const std::shared_ptr<api::ExecutionContext> &context,
- const api::Currency &currency);
+ const api::Currency &currency,
+ bool useConfirmedFirst);
 
  public:
  static std::vector<BitcoinLikeUtxo> filterWithKnapsackSolver(const std::shared_ptr<Buddy> &buddy,
  const std::vector<BitcoinLikeUtxo> &utxos,
  const BigInt &aggregatedAmount,
- const api::Currency &currrency);
+ const api::Currency &currrency,
+ bool useConfirmedFirst);
 
  static std::vector<BitcoinLikeUtxo> filterWithOptimizeSize(const std::shared_ptr<Buddy> &buddy,
  const std::vector<BitcoinLikeUtxo> &utxos,
  const BigInt &aggregatedAmount,
- const api::Currency &currrency);
+ const api::Currency &currrency,
+ bool useConfirmedFirst);
 
  static std::vector<BitcoinLikeUtxo> filterWithMergeOutputs(const std::shared_ptr<Buddy> &buddy,
  const std::vector<BitcoinLikeUtxo> &utxos,
  const BigInt &aggregatedAmount,
- const api::Currency &currrency);
+ const api::Currency &currrency,
+ bool useConfirmedFirst);
+
  static std::vector<BitcoinLikeUtxo> filterWithDeepFirst(const std::shared_ptr<Buddy> &buddy,
  const std::vector<BitcoinLikeUtxo> &utxo,
  const BigInt &aggregatedAmount,
  const api::Currency &currrency);
+
  static bool hasEnough(const std::shared_ptr<Buddy> &buddy,
  const BigInt &aggregatedAmount,
  int inputCount,
@@ -93,6 +99,8 @@ namespace ledger {
  BigInt amount,
  const api::Currency &currency,
  std::function<bool(BitcoinLikeUtxo &, BitcoinLikeUtxo &)> const &functor);
+
+ bool _useConfirmedFirst{true};
  };
  } // namespace core
 } // namespace ledger