Skip to content
This repository was archived by the owner on Feb 12, 2025. It is now read-only.

Commit e35f2ba

Browse files
jcoatelen-ledgerjcoatelen-ledger
authored
VG-13274 - Confirmed-first ordering of utxos when crafting a transaction + add configuration (#933)
* add CONFIRMED_UTXO_FIRST configuration to select the order for querying utxos * use confirmed-first approach in BitcoinLikeStrategyUtxoPicker * test OptimizeSize with confirmed first utxo * add tests for confirmed-first utxo picking * fix side effect in test
1 parent b60d548 commit e35f2ba

File tree

8 files changed

+164
-34
lines changed

8 files changed

+164
-34
lines changed

‎core/idl/wallet/configuration.djinni

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ Configuration = interface +c {
127127

128128
# Allow the generation of the P2TR (Taproot) outputs
129129
const ALLOW_P2TR: string = "ALLOW_P2TR";
130+
131+
# Use confirmed UTXOs first in utxo picking strategies
132+
const CONFIRMED_UTXO_FIRST: string = "CONFIRMED_UTXO_FIRST";
130133
}
131134

132135
# Configuration of wallet pools.

‎core/src/api/Configuration.cpp

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎core/src/api/Configuration.hpp

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎core/src/wallet/bitcoin/BitcoinLikeAccount.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,10 @@ namespace ledger {
185185
_synchronizer = synchronizer;
186186
_keychain = keychain;
187187
_keychain->getAllObservableAddresses(0, 40);
188-
_picker = std::make_shared<BitcoinLikeStrategyUtxoPicker>(getWallet()->getPool()->getThreadPoolExecutionContext(), getWallet()->getCurrency());
188+
_picker = std::make_shared<BitcoinLikeStrategyUtxoPicker>(
189+
getWallet()->getPool()->getThreadPoolExecutionContext(),
190+
getWallet()->getCurrency(),
191+
getWallet()->getConfig()->getBoolean(api::Configuration::CONFIRMED_UTXO_FIRST).value_or(true));
189192
_currentBlockHeight = 0;
190193
}
191194

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

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,24 @@
4343
namespace ledger {
4444
namespace core {
4545

46-
BitcoinLikeStrategyUtxoPicker::BitcoinLikeStrategyUtxoPicker(const std::shared_ptr<api::ExecutionContext> &context,
47-
const api::Currency &currency) : BitcoinLikeUtxoPicker(context, currency) {
46+
std::optional<bool> compareConfirmation(const BitcoinLikeUtxo &u1, const BitcoinLikeUtxo &u2) {
47+
bool isU1Confirmed = u1.blockHeight.hasValue();
48+
bool isU2Confirmed = u2.blockHeight.hasValue();
49+
50+
if ((isU1Confirmed && isU2Confirmed) || (!isU1Confirmed && !isU2Confirmed)) {
51+
// ignore the case where confirmation status is similar
52+
return {};
53+
}
54+
55+
// otherwise confirmed utxo should be first
56+
return isU1Confirmed;
4857
}
4958

59+
BitcoinLikeStrategyUtxoPicker::BitcoinLikeStrategyUtxoPicker(const std::shared_ptr<api::ExecutionContext> &context,
60+
const api::Currency &currency,
61+
bool useConfirmedFirst)
62+
: BitcoinLikeUtxoPicker(context, currency), _useConfirmedFirst{useConfirmedFirst} {}
63+
5064
Future<std::vector<BitcoinLikeUtxo>>
5165
BitcoinLikeStrategyUtxoPicker::filterInputs(const std::shared_ptr<BitcoinLikeUtxoPicker::Buddy> &buddy) {
5266
return computeAggregatedAmount(buddy).flatMap<std::vector<BitcoinLikeUtxo>>(getContext(), [=](BigInt const &amount) {
@@ -71,9 +85,9 @@ namespace ledger {
7185
case api::BitcoinLikePickingStrategy::DEEP_OUTPUTS_FIRST:
7286
return filterWithDeepFirst(buddy, utxos, amount, getCurrency());
7387
case api::BitcoinLikePickingStrategy::OPTIMIZE_SIZE:
74-
return filterWithOptimizeSize(buddy, utxos, amount, getCurrency());
88+
return filterWithOptimizeSize(buddy, utxos, amount, getCurrency(), _useConfirmedFirst);
7589
case api::BitcoinLikePickingStrategy::MERGE_OUTPUTS:
76-
return filterWithMergeOutputs(buddy, utxos, amount, getCurrency());
90+
return filterWithMergeOutputs(buddy, utxos, amount, getCurrency(), _useConfirmedFirst);
7791
}
7892

7993
throw make_exception(api::ErrorCode::ILLEGAL_ARGUMENT, "Unknown UTXO picking strategy.");
@@ -173,7 +187,8 @@ namespace ledger {
173187
BitcoinLikeStrategyUtxoPicker::filterWithOptimizeSize(const std::shared_ptr<BitcoinLikeUtxoPicker::Buddy> &buddy,
174188
const std::vector<BitcoinLikeUtxo> &utxos,
175189
const BigInt &aggregatedAmount,
176-
const api::Currency &currency) {
190+
const api::Currency &currency,
191+
bool useConfirmedFirst) {
177192
// NOTE: why are we using buddy->outputAmount here instead of aggregatedAmount ?
178193
// Don't use this strategy for wipe mode (we have more performent strategies for this use case)
179194
if (buddy->request.wipe) {
@@ -272,7 +287,13 @@ namespace ledger {
272287
throw make_exception(api::ErrorCode::NOT_ENOUGH_FUNDS, "Cannot gather enough funds.");
273288
}
274289

275-
auto descendingEffectiveValue = [](const EffectiveUtxo &lhs, const EffectiveUtxo &rhs) -> bool {
290+
auto descendingEffectiveValue = [useConfirmedFirst](const EffectiveUtxo &lhs, const EffectiveUtxo &rhs) -> bool {
291+
if (useConfirmedFirst) {
292+
std::optional<bool> comp = compareConfirmation(*lhs.utxo, *rhs.utxo);
293+
if (comp.has_value()) {
294+
return comp.value();
295+
}
296+
}
276297
return lhs.effectiveValue > rhs.effectiveValue;
277298
};
278299

@@ -347,7 +368,7 @@ namespace ledger {
347368
// If no selection found fallback on filterWithDeepFirst
348369
if (bestSelection.empty()) {
349370
buddy->logger->debug("No best selection found, fallback on filterWithKnapsackSolver coin selection");
350-
return filterWithKnapsackSolver(buddy, utxos, aggregatedAmount, currency);
371+
return filterWithKnapsackSolver(buddy, utxos, aggregatedAmount, currency, useConfirmedFirst);
351372
}
352373

353374
// Prepare result
@@ -414,7 +435,8 @@ namespace ledger {
414435
const std::shared_ptr<BitcoinLikeUtxoPicker::Buddy> &buddy,
415436
const std::vector<BitcoinLikeUtxo> &utxos,
416437
const BigInt &aggregatedAmount,
417-
const api::Currency &currency) {
438+
const api::Currency &currency,
439+
bool useConfirmedFirst) {
418440
// Tx fixed size
419441
auto const fixedSize = BitcoinLikeTransactionApi::estimateSize(0,
420442
0,
@@ -473,12 +495,18 @@ namespace ledger {
473495
std::vector<BitcoinLikeUtxo> out;
474496

475497
// Random shuffle utxos
476-
std::vector<size_t> indexes(utxos.size());
477-
std::iota(indexes.begin(), indexes.end(), 0);
478-
498+
const auto &sz = utxos.size();
499+
std::vector<size_t> indexes(sz, 0);
479500
auto const seed = std::chrono::system_clock::now().time_since_epoch().count();
501+
std::iota(indexes.begin(), indexes.end(), 0);
480502
std::shuffle(indexes.begin(), indexes.end(), std::default_random_engine(seed));
481503

504+
if (useConfirmedFirst) {
505+
std::stable_sort(indexes.begin(), indexes.end(), [&utxos](auto id1, auto id2) {
506+
return compareConfirmation(utxos[id1], utxos[id2]).value_or(true);
507+
});
508+
}
509+
482510
// Add fees for a signed input to amount
483511
for (auto index : indexes) {
484512
auto &utxo = utxos[index];
@@ -516,7 +544,13 @@ namespace ledger {
516544
}
517545

518546
// Sort vUTXOs descending
519-
std::sort(vUTXOs.begin(), vUTXOs.end(), [](auto const &lhs, auto const &rhs) {
547+
std::sort(vUTXOs.begin(), vUTXOs.end(), [useConfirmedFirst](auto const &lhs, auto const &rhs) {
548+
if (useConfirmedFirst) {
549+
std::optional<bool> comp = compareConfirmation(lhs, rhs);
550+
if (comp.has_value()) {
551+
return comp.value();
552+
}
553+
}
520554
return lhs.value.toLong() > rhs.value.toLong();
521555
});
522556

@@ -586,10 +620,18 @@ namespace ledger {
586620
const std::shared_ptr<BitcoinLikeUtxoPicker::Buddy> &buddy,
587621
const std::vector<BitcoinLikeUtxo> &utxos,
588622
const BigInt &aggregatedAmount,
589-
const api::Currency &currency) {
623+
const api::Currency &currency,
624+
bool useConfirmedFirst) {
590625
buddy->logger->debug("Start filterWithMergeOutputs");
591626

592-
return filterWithSort(buddy, utxos, aggregatedAmount, currency, [](auto &lhs, auto &rhs) {
627+
return filterWithSort(buddy, utxos, aggregatedAmount, currency, [useConfirmedFirst](auto &lhs, auto &rhs) {
628+
if (useConfirmedFirst) {
629+
std::optional<bool> comp = compareConfirmation(lhs, rhs);
630+
if (comp.has_value()) {
631+
return comp.value();
632+
}
633+
}
634+
593635
return lhs.value.toLong() < rhs.value.toLong();
594636
});
595637
}
@@ -631,5 +673,6 @@ namespace ledger {
631673

632674
return pickedUtxos;
633675
}
676+
634677
} // namespace core
635678
} // namespace ledger

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,27 +46,33 @@ namespace ledger {
4646
class BitcoinLikeStrategyUtxoPicker : public BitcoinLikeUtxoPicker {
4747
public:
4848
BitcoinLikeStrategyUtxoPicker(const std::shared_ptr<api::ExecutionContext> &context,
49-
const api::Currency &currency);
49+
const api::Currency &currency,
50+
bool useConfirmedFirst);
5051

5152
public:
5253
static std::vector<BitcoinLikeUtxo> filterWithKnapsackSolver(const std::shared_ptr<Buddy> &buddy,
5354
const std::vector<BitcoinLikeUtxo> &utxos,
5455
const BigInt &aggregatedAmount,
55-
const api::Currency &currrency);
56+
const api::Currency &currrency,
57+
bool useConfirmedFirst);
5658

5759
static std::vector<BitcoinLikeUtxo> filterWithOptimizeSize(const std::shared_ptr<Buddy> &buddy,
5860
const std::vector<BitcoinLikeUtxo> &utxos,
5961
const BigInt &aggregatedAmount,
60-
const api::Currency &currrency);
62+
const api::Currency &currrency,
63+
bool useConfirmedFirst);
6164

6265
static std::vector<BitcoinLikeUtxo> filterWithMergeOutputs(const std::shared_ptr<Buddy> &buddy,
6366
const std::vector<BitcoinLikeUtxo> &utxos,
6467
const BigInt &aggregatedAmount,
65-
const api::Currency &currrency);
68+
const api::Currency &currrency,
69+
bool useConfirmedFirst);
70+
6671
static std::vector<BitcoinLikeUtxo> filterWithDeepFirst(const std::shared_ptr<Buddy> &buddy,
6772
const std::vector<BitcoinLikeUtxo> &utxo,
6873
const BigInt &aggregatedAmount,
6974
const api::Currency &currrency);
75+
7076
static bool hasEnough(const std::shared_ptr<Buddy> &buddy,
7177
const BigInt &aggregatedAmount,
7278
int inputCount,
@@ -93,6 +99,8 @@ namespace ledger {
9399
BigInt amount,
94100
const api::Currency &currency,
95101
std::function<bool(BitcoinLikeUtxo &, BitcoinLikeUtxo &)> const &functor);
102+
103+
bool _useConfirmedFirst{true};
96104
};
97105
} // namespace core
98106
} // namespace ledger

0 commit comments

Comments
 (0)