diff --git a/rust/tw_coin_entry/src/error.rs b/rust/tw_coin_entry/src/error.rs index e8ceb589066..bd28fddd6c3 100644 --- a/rust/tw_coin_entry/src/error.rs +++ b/rust/tw_coin_entry/src/error.rs @@ -126,6 +126,7 @@ impl fmt::Display for SigningError { SigningErrorType::Error_invalid_params => "Incorrect input parameter", SigningErrorType::Error_invalid_requested_token_amount => "Invalid input token amount", SigningErrorType::Error_not_supported => "Operation not supported for the chain", + SigningErrorType::Error_dust_amount_requested => "Requested amount is too low (less dust)", }; write!(f, "{str}") } diff --git a/src/Bitcoin/DustCalculator.cpp b/src/Bitcoin/DustCalculator.cpp new file mode 100644 index 00000000000..3aeea03cecb --- /dev/null +++ b/src/Bitcoin/DustCalculator.cpp @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright © 2017 Trust Wallet. + +#include "DustCalculator.h" + +namespace TW::Bitcoin { + +FixedDustCalculator::FixedDustCalculator(Amount fixed) noexcept + : fixedDustAmount(fixed) { +} + +Amount FixedDustCalculator::dustAmount([[maybe_unused]] Amount byteFee) noexcept { + return fixedDustAmount; +} + +LegacyDustCalculator::LegacyDustCalculator(TWCoinType coinType) noexcept + : feeCalculator(getFeeCalculator(coinType, false)) { +} + +Amount LegacyDustCalculator::dustAmount([[maybe_unused]] Amount byteFee) noexcept { + return feeCalculator.calculateSingleInput(byteFee); +} + +DustCalculatorShared getDustCalculator(const Proto::SigningInput& input) { + if (input.disable_dust_filter()) { + return std::make_shared(0); + } + + if (input.has_fixed_dust_threshold()) { + return std::make_shared(input.fixed_dust_threshold()); + } + + const auto coinType = static_cast(input.coin_type()); + return std::make_shared(coinType); +} + +} // namespace TW::Bitcoin diff --git a/src/Bitcoin/DustCalculator.h b/src/Bitcoin/DustCalculator.h new file mode 100644 index 00000000000..825ea9b2ba6 --- /dev/null +++ b/src/Bitcoin/DustCalculator.h @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright © 2017 Trust Wallet. + +#pragma once + +#include "Amount.h" +#include "FeeCalculator.h" +#include "proto/Bitcoin.pb.h" + +#include +#include + +namespace TW::Bitcoin { + +/// Interface for transaction dust amount calculator. +struct DustCalculator { + virtual ~DustCalculator() noexcept = default; + + /// Returns a Dust threshold of a transaction UTXO or output. + virtual Amount dustAmount(Amount byteFee) noexcept = 0; +}; + +/// Always returns a fixed Dust amount specified in the signing request. +class FixedDustCalculator final: public DustCalculator { +public: + explicit FixedDustCalculator(Amount fixed) noexcept; + + Amount dustAmount([[maybe_unused]] Amount byteFee) noexcept override; + +private: + Amount fixedDustAmount {0}; +}; + +/// Legacy Dust filter implementation using [`FeeCalculator::calculateSingleInput`]. +/// Depends on a coin type, sats/Byte fee. +class LegacyDustCalculator final: public DustCalculator { +public: + explicit LegacyDustCalculator(TWCoinType coinType) noexcept; + + Amount dustAmount(Amount byteFee) noexcept override; + +private: + const FeeCalculator& feeCalculator; +}; + +using DustCalculatorShared = std::shared_ptr; + +DustCalculatorShared getDustCalculator(const Proto::SigningInput& input); + +} // namespace TW::Bitcoin diff --git a/src/Bitcoin/InputSelector.cpp b/src/Bitcoin/InputSelector.cpp index 9b2bc7bb175..98a5f4e3358 100644 --- a/src/Bitcoin/InputSelector.cpp +++ b/src/Bitcoin/InputSelector.cpp @@ -25,8 +25,8 @@ template std::vector InputSelector::filterOutDust(const std::vector& inputs, int64_t byteFee) noexcept { - auto inputFeeLimit = static_cast(feeCalculator.calculateSingleInput(byteFee)); - return filterThreshold(inputs, inputFeeLimit); + auto dustThreshold = static_cast(dustCalculator->dustAmount(byteFee)); + return filterThreshold(inputs, dustThreshold); } // Filters utxos that are dust @@ -36,7 +36,7 @@ InputSelector::filterThreshold(const std::vector uint64_t minimumAmount) noexcept { std::vector filtered; for (auto& i : inputs) { - if (static_cast(i.amount) > minimumAmount) { + if (static_cast(i.amount) >= minimumAmount) { filtered.push_back(i); } } @@ -70,22 +70,24 @@ InputSelector::select(uint64_t targetValue, uint64_t byteFee, ui return {}; } + // Get all possible utxo selections up to a maximum size, sort by total amount, increasing + std::vector sorted = filterOutDust(_inputs, byteFee); + std::sort( + sorted.begin(), + sorted.end(), + [](const TypeWithAmount& lhs, const TypeWithAmount& rhs) { + return lhs.amount < rhs.amount; + }); + // total values of utxos should be greater than targetValue - if (_inputs.empty() || sum(_inputs) < targetValue) { + if (sorted.empty() || sum(sorted) < targetValue) { return {}; } - assert(_inputs.size() >= 1); + assert(sorted.size() >= 1); // definitions for the following calculation const auto doubleTargetValue = targetValue * 2; - // Get all possible utxo selections up to a maximum size, sort by total amount, increasing - std::vector sorted = _inputs; - std::sort(sorted.begin(), sorted.end(), - [](const TypeWithAmount& lhs, const TypeWithAmount& rhs) { - return lhs.amount < rhs.amount; - }); - // Precompute maximum amount possible to obtain with given number of inputs const auto n = sorted.size(); std::vector maxWithXInputs = std::vector(); @@ -104,7 +106,7 @@ InputSelector::select(uint64_t targetValue, uint64_t byteFee, ui return doubleTargetValue - val; }; - const int64_t dustThreshold = feeCalculator.calculateSingleInput(byteFee); + const int64_t dustThreshold = dustCalculator->dustAmount(byteFee); // 1. Find a combination of the fewest inputs that is // (1) bigger than what we need @@ -131,7 +133,7 @@ InputSelector::select(uint64_t targetValue, uint64_t byteFee, ui const std::vector& rhs) { return distFrom2x(sum(lhs)) < distFrom2x(sum(rhs)); }); - return filterOutDust(slices.front(), byteFee); + return slices.front(); } } @@ -150,11 +152,14 @@ InputSelector::select(uint64_t targetValue, uint64_t byteFee, ui }), slices.end()); if (!slices.empty()) { - return filterOutDust(slices.front(), byteFee); + return slices.front(); } } - return {}; + // If we couldn't find a combination of inputs to cover estimated transaction fee and the target amount, + // return the whole set of UTXOs. Later, the transaction fee will be calculated more accurately, + // and these UTXOs can be enough. + return sorted; } template @@ -170,13 +175,13 @@ std::vector InputSelector::selectSimple(int64_t } assert(_inputs.size() >= 1); - // target value is larger that original, but not by a factor of 2 (optimized for large UTXO + // target value is larger than original, but not by a factor of 2 (optimized for large UTXO // cases) const auto increasedTargetValue = (uint64_t)((double)targetValue * 1.1 + feeCalculator.calculate(_inputs.size(), numOutputs, byteFee) + 1000); - const int64_t dustThreshold = feeCalculator.calculateSingleInput(byteFee); + const int64_t dustThreshold = dustCalculator->dustAmount(byteFee); // Go through inputs in a single pass, in the order they appear, no optimization uint64_t sum = 0; @@ -193,8 +198,10 @@ std::vector InputSelector::selectSimple(int64_t } } - // not enough - return {}; + // If we couldn't find a combination of inputs to cover estimated transaction fee and the target amount, + // return the whole set of UTXOs. Later, the transaction fee will be calculated more accurately, + // and these UTXOs can be enough. + return selected; } template diff --git a/src/Bitcoin/InputSelector.h b/src/Bitcoin/InputSelector.h index e2941e812d1..848dae390d4 100644 --- a/src/Bitcoin/InputSelector.h +++ b/src/Bitcoin/InputSelector.h @@ -5,6 +5,7 @@ #pragma once #include "FeeCalculator.h" +#include "DustCalculator.h" #include #include @@ -17,16 +18,18 @@ class InputSelector { public: /// Selects unspent transactions to use given a target transaction value, using complete logic. /// - /// \returns the list of indices of selected inputs, or an empty list if there are insufficient - /// funds. + /// \returns the list of indices of selected inputs. May return the entire list of UTXOs + /// even if they aren't enough to cover `targetValue + fee`. + /// That's because `InputSelector` has a rough segwit fee estimation algorithm, and the UTXOs can actually be enough. std::vector select(uint64_t targetValue, uint64_t byteFee, uint64_t numOutputs = 2); /// Selects unspent transactions to use given a target transaction value; /// Simplified version suitable for large number of inputs /// - /// \returns the list of indices of selected inputs, or an empty list if there are insufficient - /// funds. + /// \returns the list of indices of selected inputs. May return the entire list of UTXOs + /// even if they aren't enough to cover `targetValue + fee`. + /// That's because `InputSelector` has a rough segwit fee estimation algorithm, and the UTXOs can actually be enough. std::vector selectSimple(int64_t targetValue, int64_t byteFee, int64_t numOutputs = 2); @@ -34,12 +37,20 @@ class InputSelector { /// Return indices. One output and no change is assumed. std::vector selectMaxAmount(int64_t byteFee) noexcept; - /// Construct, using provided feeCalculator (see getFeeCalculator()). + /// Construct, using provided feeCalculator (see getFeeCalculator()) and dustCalculator (see getDustCalculator()). explicit InputSelector(const std::vector& inputs, - const FeeCalculator& feeCalculator) noexcept - : _inputs(inputs), feeCalculator(feeCalculator) {} + const FeeCalculator& feeCalculator, + DustCalculatorShared dustCalculator) noexcept + : _inputs(inputs), + feeCalculator(feeCalculator), + dustCalculator(std::move(dustCalculator)) { + } + explicit InputSelector(const std::vector& inputs) noexcept - : InputSelector(inputs, getFeeCalculator(TWCoinTypeBitcoin)) {} + : _inputs(inputs), + feeCalculator(getFeeCalculator(TWCoinTypeBitcoin)), + dustCalculator(std::make_shared(TWCoinTypeBitcoin)) { + } /// Sum of input amounts static uint64_t sum(const std::vector& amounts) noexcept; @@ -53,6 +64,7 @@ class InputSelector { private: const std::vector _inputs; const FeeCalculator& feeCalculator; + const DustCalculatorShared dustCalculator; }; } // namespace TW::Bitcoin diff --git a/src/Bitcoin/SigningInput.cpp b/src/Bitcoin/SigningInput.cpp index bcdf4e50f8f..3367652dd4a 100644 --- a/src/Bitcoin/SigningInput.cpp +++ b/src/Bitcoin/SigningInput.cpp @@ -6,6 +6,10 @@ namespace TW::Bitcoin { +SigningInput::SigningInput() + : dustCalculator(std::make_shared(TWCoinTypeBitcoin)) { +} + SigningInput::SigningInput(const Proto::SigningInput& input) { hashType = static_cast(input.hash_type()); amount = input.amount(); @@ -37,6 +41,8 @@ SigningInput::SigningInput(const Proto::SigningInput& input) { extraOutputsAmount += output.amount(); extraOutputs.push_back(std::make_pair(output.to_address(), output.amount())); } + + dustCalculator = getDustCalculator(input); } } // namespace TW::Bitcoin diff --git a/src/Bitcoin/SigningInput.h b/src/Bitcoin/SigningInput.h index 7686df1d1bc..a2b40419323 100644 --- a/src/Bitcoin/SigningInput.h +++ b/src/Bitcoin/SigningInput.h @@ -5,6 +5,7 @@ #pragma once #include "Amount.h" +#include "DustCalculator.h" #include "Transaction.h" #include "UTXO.h" #include @@ -73,8 +74,10 @@ class SigningInput { // Total amount of the `extraOutputs`. Amount extraOutputsAmount = 0; + DustCalculatorShared dustCalculator; + public: - SigningInput() = default; + SigningInput(); SigningInput(const Proto::SigningInput& input); }; diff --git a/src/Bitcoin/TransactionBuilder.cpp b/src/Bitcoin/TransactionBuilder.cpp index b4f167d92d6..b343b2bd924 100644 --- a/src/Bitcoin/TransactionBuilder.cpp +++ b/src/Bitcoin/TransactionBuilder.cpp @@ -88,6 +88,7 @@ TransactionPlan TransactionBuilder::plan(const SigningInput& input) { bool maxAmount = input.useMaxAmount; Amount totalAmount = input.amount + input.extraOutputsAmount; + Amount dustThreshold = input.dustCalculator->dustAmount(input.byteFee); if (totalAmount == 0 && !maxAmount) { plan.error = Common::Proto::Error_zero_amount_requested; @@ -95,7 +96,7 @@ TransactionPlan TransactionBuilder::plan(const SigningInput& input) { plan.error = Common::Proto::Error_missing_input_utxos; } else { const auto& feeCalculator = getFeeCalculator(static_cast(input.coinType), input.disableDustFilter); - auto inputSelector = InputSelector(input.utxos, feeCalculator); + auto inputSelector = InputSelector(input.utxos, feeCalculator, input.dustCalculator); auto inputSum = InputSelector::sum(input.utxos); // select UTXOs @@ -111,6 +112,8 @@ TransactionPlan TransactionBuilder::plan(const SigningInput& input) { auto output_size = 2; UTXOs selectedInputs; if (!maxAmount) { + // Please note that there may not be a "change" output if the "change.amount" is less than "dust", + // but we use a max amount of transaction outputs to simplify the algorithm, so the fee can be slightly bigger in rare cases. output_size = 2 + extraOutputs; // output + change if (input.useMaxUtxo) { selectedInputs = inputSelector.selectMaxAmount(input.byteFee); @@ -145,10 +148,16 @@ TransactionPlan TransactionBuilder::plan(const SigningInput& input) { } else { plan.availableAmount = InputSelector::sum(plan.utxos); + // There can be less UTXOs after Dust filtering. + if (!maxAmount && totalAmount > plan.availableAmount) { + TransactionPlan errorPlan; + errorPlan.error = Common::Proto::Error_not_enough_utxos; + return errorPlan; + } + // Compute fee. // must preliminary set change so that there is a second output if (!maxAmount) { - assert(totalAmount <= plan.availableAmount); plan.amount = input.amount; plan.fee = 0; plan.change = plan.availableAmount - totalAmount; @@ -158,6 +167,16 @@ TransactionPlan TransactionBuilder::plan(const SigningInput& input) { plan.change = 0; } plan.fee = estimateSegwitFee(feeCalculator, plan, output_size, input); + + // `InputSelector` has a rough segwit fee estimation algorithm, + // so the fee could be increased or decreased (see `InputSelector::select`). + // We need to make sure if we have enough UTXOs to cover "requested amount + final fee". + if (!maxAmount && plan.availableAmount < plan.fee + plan.amount) { + TransactionPlan errorPlan; + errorPlan.error = Common::Proto::Error_not_enough_utxos; + return errorPlan; + } + // If fee is larger than availableAmount (can happen in special maxAmount case), we reduce it (and hope it will go through) plan.fee = std::min(plan.availableAmount, plan.fee); assert(plan.fee >= 0 && plan.fee <= plan.availableAmount); @@ -175,13 +194,28 @@ TransactionPlan TransactionBuilder::plan(const SigningInput& input) { // The total amount that will be spent. Amount totalSpendAmount = plan.amount + input.extraOutputsAmount + plan.fee; + // Make sure that the output amount is greater or at least equal to the dust threshold. + if (plan.amount < dustThreshold) { + TransactionPlan errorPlan; + errorPlan.error = maxAmount ? Common::Proto::Error_not_enough_utxos : Common::Proto::Error_dust_amount_requested; + return errorPlan; + } + // Make sure that we have enough available UTXOs to spend `fee`, `amount` and `extraOutputsAmount`. if (plan.availableAmount < totalSpendAmount) { - plan.amount = 0; - plan.error = Common::Proto::Error_not_enough_utxos; + TransactionPlan errorPlan; + errorPlan.error = Common::Proto::Error_not_enough_utxos; + return errorPlan; + } + + auto changeAmount = plan.availableAmount - totalSpendAmount; + // Compute change if it's not dust. + if (changeAmount >= dustThreshold) { + plan.change = changeAmount; } else { - // compute change - plan.change = plan.availableAmount - totalSpendAmount; + // Spend the change as tx fee if it's dust, otherwise the transaction won't be mined. + plan.change = 0; + plan.fee += changeAmount; } } } diff --git a/src/proto/Bitcoin.proto b/src/proto/Bitcoin.proto index 4345c7093cc..0add08247a7 100644 --- a/src/proto/Bitcoin.proto +++ b/src/proto/Bitcoin.proto @@ -164,6 +164,13 @@ message SigningInput { // If set, uses Bitcoin 2.0 Signing protocol. // As a result, `Bitcoin.Proto.SigningOutput.signing_result_v2` is set. BitcoinV2.Proto.SigningInput signing_v2 = 21; + + // One of the "Dust" amount policies. + // Later, we plan to add support for `DynamicDust` policy with a `min_relay_fee` amount. + oneof dust_policy { + // Use a constant "Dust" threshold. + int64 fixed_dust_threshold = 24; + } } // Describes a preliminary transaction plan. diff --git a/src/proto/Common.proto b/src/proto/Common.proto index 0a9944bff49..af13469d47f 100644 --- a/src/proto/Common.proto +++ b/src/proto/Common.proto @@ -69,4 +69,6 @@ enum SigningError { Error_invalid_requested_token_amount = 23; // Operation not supported for the chain. Error_not_supported = 24; + // Requested amount is too low (less dust). + Error_dust_amount_requested = 25; } diff --git a/swift/Tests/Blockchains/BitcoinTests.swift b/swift/Tests/Blockchains/BitcoinTests.swift index 40c9e894a7f..ac1b6dd1206 100644 --- a/swift/Tests/Blockchains/BitcoinTests.swift +++ b/swift/Tests/Blockchains/BitcoinTests.swift @@ -305,6 +305,9 @@ class BitcoinTransactionSignerTests: XCTestCase { $0.byteFee = 1 $0.toAddress = "1Bp9U1ogV3A14FMvKbRJms7ctyso4Z4Tcx" $0.changeAddress = "1FQc5LdgGHMHEN9nwkjmz6tWkxhPpxBvBU" + // Set the very low fixed Dust threshold just to fix the tests. + // Actually, the transaction in this test has change=79 that will lead to Dust error when broadcasting it. + $0.fixedDustThreshold = 79; } input.scripts["593128f9f90e38b706c18623151e37d2da05c229"] = Data(hexString: "2103596d3451025c19dbbdeb932d6bf8bfb4ad499b95b6f88db8899efac102e5fc71ac")! diff --git a/tests/chains/Bitcoin/InputSelectorTests.cpp b/tests/chains/Bitcoin/InputSelectorTests.cpp index 999b9d950a6..eef037af45a 100644 --- a/tests/chains/Bitcoin/InputSelectorTests.cpp +++ b/tests/chains/Bitcoin/InputSelectorTests.cpp @@ -125,7 +125,9 @@ TEST(BitcoinInputSelector, SelectOneInsufficientEqual) { auto selector = InputSelector(utxos); auto selected = selector.select(100'000, 1); - EXPECT_TRUE(verifySelectedUTXOs(selected, {})); + // `InputSelector` returns the entire list of UTXOs even if they are not enough. + // That's because `InputSelector` has a rough segwit fee estimation algorithm, and the UTXOs can actually be enough. + EXPECT_TRUE(verifySelectedUTXOs(selected, {100'000})); } TEST(BitcoinInputSelector, SelectOneInsufficientHigher) { @@ -134,14 +136,28 @@ TEST(BitcoinInputSelector, SelectOneInsufficientHigher) { auto selector = InputSelector(utxos); auto selected = selector.select(99'900, 1); - EXPECT_TRUE(verifySelectedUTXOs(selected, {})); + // `InputSelector` returns the entire list of UTXOs even if they are not enough. + // That's because `InputSelector` has a rough segwit fee estimation algorithm, and the UTXOs can actually be enough. + EXPECT_TRUE(verifySelectedUTXOs(selected, {100'000})); +} + +TEST(BitcoinInputSelector, SelectOneInsufficientHigherFilterDust) { + auto utxos = buildTestUTXOs({22, 100'000, 40}); + + auto selector = InputSelector(utxos); + auto selected = selector.select(99'900, 1); + + // `InputSelector` returns the entire list of UTXOs even if they are not enough. + // That's because `InputSelector` has a rough segwit fee estimation algorithm, and the UTXOs can actually be enough. + // However, the list of result UTXOs does not include dust inputs. + EXPECT_TRUE(verifySelectedUTXOs(selected, {100'000})); } TEST(BitcoinInputSelector, SelectOneFitsExactly) { auto utxos = buildTestUTXOs({100'000}); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto expectedFee = 174; auto selected = selector.select(100'000 - expectedFee, 1); @@ -150,10 +166,11 @@ TEST(BitcoinInputSelector, SelectOneFitsExactly) { EXPECT_EQ(feeCalculator.calculate(1, 2, 1), expectedFee); EXPECT_EQ(feeCalculator.calculate(1, 1, 1), 143); - // 1 sat more and does not fit any more + // 1 sat more and does not fit any more. + // However, `InputSelector` returns the entire list of UTXOs even if they are not enough. + // That's because `InputSelector` has a rough segwit fee estimation algorithm, and the UTXOs can actually be enough. selected = selector.select(100'000 - expectedFee + 1, 1); - - EXPECT_TRUE(verifySelectedUTXOs(selected, {})); + EXPECT_TRUE(verifySelectedUTXOs(selected, {100'000})); } TEST(BitcoinInputSelector, SelectOneFitsExactlyHighfee) { @@ -161,7 +178,7 @@ TEST(BitcoinInputSelector, SelectOneFitsExactlyHighfee) { const auto byteFee = 10; auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto expectedFee = 1740; auto selected = selector.select(100'000 - expectedFee, byteFee); @@ -170,17 +187,18 @@ TEST(BitcoinInputSelector, SelectOneFitsExactlyHighfee) { EXPECT_EQ(feeCalculator.calculate(1, 2, byteFee), expectedFee); EXPECT_EQ(feeCalculator.calculate(1, 1, byteFee), 1430); - // 1 sat more and does not fit any more + // 1 sat more and does not fit any more. + // However, `InputSelector` returns the entire list of UTXOs even if they are not enough. + // That's because `InputSelector` has a rough segwit fee estimation algorithm, and the UTXOs can actually be enough. selected = selector.select(100'000 - expectedFee + 1, byteFee); - - EXPECT_TRUE(verifySelectedUTXOs(selected, {})); + EXPECT_TRUE(verifySelectedUTXOs(selected, {100'000})); } TEST(BitcoinInputSelector, SelectThreeNoDust) { auto utxos = buildTestUTXOs({100'000, 70'000, 75'000}); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto selected = selector.select(100'000 - 174 - 10, 1); // 100'000 would fit with dust; instead two UTXOs are selected not to leave dust @@ -274,7 +292,7 @@ TEST(BitcoinInputSelector, SelectTenThreeExact) { auto utxos = buildTestUTXOs({1'000, 2'000, 100'000, 3'000, 4'000, 5'000, 125'000, 6'000, 150'000, 7'000}); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); const auto dustLimit = 102; auto selected = selector.select(375'000 - 376 - dustLimit, 1); @@ -292,7 +310,7 @@ TEST(BitcoinInputSelector, SelectMaxAmountOne) { auto utxos = buildTestUTXOs({10189534}); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto selected = selector.selectMaxAmount(1); EXPECT_TRUE(verifySelectedUTXOs(selected, {10189534})); @@ -304,7 +322,7 @@ TEST(BitcoinInputSelector, SelectAllAvail) { auto utxos = buildTestUTXOs({10189534}); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto selected = selector.select(10189534 - 226, 1); EXPECT_TRUE(verifySelectedUTXOs(selected, {10189534})); @@ -316,7 +334,7 @@ TEST(BitcoinInputSelector, SelectMaxAmount5of5) { auto utxos = buildTestUTXOs({400, 500, 600, 800, 1000}); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto byteFee = 1; auto selected = selector.selectMaxAmount(byteFee); @@ -330,7 +348,7 @@ TEST(BitcoinInputSelector, SelectMaxAmount4of5) { auto utxos = buildTestUTXOs({400, 500, 600, 800, 1000}); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto byteFee = 4; auto selected = selector.selectMaxAmount(byteFee); @@ -344,7 +362,7 @@ TEST(BitcoinInputSelector, SelectMaxAmount1of5) { auto utxos = buildTestUTXOs({400, 500, 600, 800, 1000}); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto byteFee = 8; auto selected = selector.selectMaxAmount(byteFee); @@ -358,7 +376,7 @@ TEST(BitcoinInputSelector, SelectMaxAmountNone) { auto utxos = buildTestUTXOs({400, 500, 600, 800, 1000}); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto byteFee = 10; auto selected = selector.selectMaxAmount(byteFee); @@ -370,8 +388,7 @@ TEST(BitcoinInputSelector, SelectMaxAmountNone) { TEST(BitcoinInputSelector, SelectMaxAmountNoUTXOs) { auto utxos = buildTestUTXOs({}); - auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); - auto selector = InputSelector(utxos, feeCalculator); + auto selector = InputSelector(utxos); auto selected = selector.selectMaxAmount(1); EXPECT_TRUE(verifySelectedUTXOs(selected, {})); @@ -380,7 +397,7 @@ TEST(BitcoinInputSelector, SelectMaxAmountNoUTXOs) { TEST(BitcoinInputSelector, SelectZcashUnspents) { auto utxos = buildTestUTXOs({100000, 2592, 73774}); - auto selector = InputSelector(utxos, getFeeCalculator(TWCoinTypeZcash)); + auto selector = InputSelector(utxos, getFeeCalculator(TWCoinTypeZcash), std::make_shared(TWCoinTypeZcash)); auto selected = selector.select(10000, 1); EXPECT_TRUE(verifySelectedUTXOs(selected, {73774})); @@ -389,7 +406,7 @@ TEST(BitcoinInputSelector, SelectZcashUnspents) { TEST(BitcoinInputSelector, SelectGroestlUnspents) { auto utxos = buildTestUTXOs({499971976}); - auto selector = InputSelector(utxos, getFeeCalculator(TWCoinTypeZcash)); + auto selector = InputSelector(utxos, getFeeCalculator(TWCoinTypeZcash), std::make_shared(TWCoinTypeZcash)); auto selected = selector.select(499951976, 1, 1); EXPECT_TRUE(verifySelectedUTXOs(selected, {499971976})); @@ -398,7 +415,7 @@ TEST(BitcoinInputSelector, SelectGroestlUnspents) { TEST(BitcoinInputSelector, SelectZcashMaxAmount) { auto utxos = buildTestUTXOs({100000, 2592, 73774}); - auto selector = InputSelector(utxos, getFeeCalculator(TWCoinTypeZcash)); + auto selector = InputSelector(utxos, getFeeCalculator(TWCoinTypeZcash), std::make_shared(TWCoinTypeZcash)); auto selected = selector.selectMaxAmount(1); EXPECT_TRUE(verifySelectedUTXOs(selected, {100000, 2592, 73774})); @@ -407,10 +424,12 @@ TEST(BitcoinInputSelector, SelectZcashMaxAmount) { TEST(BitcoinInputSelector, SelectZcashMaxUnspents2) { auto utxos = buildTestUTXOs({100000, 2592, 73774}); - auto selector = InputSelector(utxos, getFeeCalculator(TWCoinTypeZcash)); + auto selector = InputSelector(utxos, getFeeCalculator(TWCoinTypeZcash), std::make_shared(TWCoinTypeZcash)); auto selected = selector.select(176366 - 6, 1); - EXPECT_TRUE(verifySelectedUTXOs(selected, {})); + // `InputSelector` returns the entire list of UTXOs even if they are not enough. + // That's because `InputSelector` has a rough segwit fee estimation algorithm, and the UTXOs can actually be enough. + EXPECT_TRUE(verifySelectedUTXOs(selected, {2592, 73774, 100000})); } TEST(BitcoinInputSelector, ManyUtxos_900) { diff --git a/tests/chains/Bitcoin/TWBitcoinSigningTests.cpp b/tests/chains/Bitcoin/TWBitcoinSigningTests.cpp index 2306de4eb31..d3e64144553 100644 --- a/tests/chains/Bitcoin/TWBitcoinSigningTests.cpp +++ b/tests/chains/Bitcoin/TWBitcoinSigningTests.cpp @@ -32,8 +32,6 @@ namespace TW::Bitcoin { -constexpr uint64_t ONE_BTC = 100'000'000; - // clang-format off SigningInput buildInputP2PKH(bool omitKey = false) { auto hash0 = parse_hex("fff7f7881a8099afa6940d42d1e7f6362bec38171ea3edf433541db4e4ad969f"); @@ -85,6 +83,68 @@ SigningInput buildInputP2PKH(bool omitKey = false) { return input; } +TEST(BitcoinSigning, SpendMinimumAmountP2WPKH) { + auto myPrivateKey = PrivateKey(parse_hex("9ea2172511ed73ae0096be8e593c3b75631700edaf729f1abbae607314a20e35")); + + auto myPublicKey = myPrivateKey.getPublicKey(TWPublicKeyTypeSECP256k1); + auto utxoPubkeyHash = Hash::ripemd(Hash::sha256(myPublicKey.bytes)); + auto redeemScript = Script::buildPayToWitnessPublicKeyHash(utxoPubkeyHash); + + // Both two UTXOs came from the same transaction. + auto utxoHash = parse_hex("e8b3c2d0d5851cef139d87dfb5794db8897ce90ce1b6961526f61923baf5b5a3"); + std::reverse(utxoHash.begin(), utxoHash.end()); + + auto segwitDustAmount = 294; + + // Setup input + SigningInput input; + input.hashType = hashTypeForCoin(TWCoinTypeBitcoin); + input.amount = 546; + input.useMaxAmount = false; + input.useMaxUtxo = true; + input.byteFee = 27; + input.toAddress = "bc1qvrt7ukvhvmdny0a3j9k8l8jasx92lrqm30t2u2"; + input.changeAddress = "bc1qvrt7ukvhvmdny0a3j9k8l8jasx92lrqm30t2u2"; + input.coinType = TWCoinTypeBitcoin; + input.dustCalculator = std::make_shared(segwitDustAmount); + + input.privateKeys.push_back(myPrivateKey); + input.scripts[hex(utxoPubkeyHash)] = redeemScript; + + UTXO utxo0; + utxo0.script = redeemScript; + utxo0.amount = segwitDustAmount; + utxo0.outPoint = OutPoint(utxoHash, 0, UINT32_MAX); + input.utxos.push_back(utxo0); + + UTXO utxo1; + utxo1.script = redeemScript; + utxo1.amount = 16776; + utxo1.outPoint = OutPoint(utxoHash, 1, UINT32_MAX); + input.utxos.push_back(utxo1); + + { + // test plan (but do not reuse plan result) + auto plan = TransactionBuilder::plan(input); + EXPECT_TRUE(verifyPlan(plan, {294, 16776}, 546, 5643)); + EXPECT_EQ(plan.change, 10881); + } + + // Signs + auto result = TransactionSigner::sign(input); + + ASSERT_TRUE(result) << std::to_string(result.error()); + const auto signedTx = result.payload(); + + Data serialized; + signedTx.encode(serialized); + + EXPECT_EQ( + hex(serialized), + "01000000000102a3b5f5ba2319f6261596b6e10ce97c89b84d79b5df879d13ef1c85d5d0c2b3e80000000000ffffffffa3b5f5ba2319f6261596b6e10ce97c89b84d79b5df879d13ef1c85d5d0c2b3e80100000000ffffffff02220200000000000016001460d7ee599766db323fb1916c7f9e5d818aaf8c1b812a00000000000016001460d7ee599766db323fb1916c7f9e5d818aaf8c1b02483045022100d7e4d267e94547bd365736229219a85b21f79cf896a65baa444e339215b4b36f022078c0dee3d1d603f77855fee8f23291fe180b50afaa2c9ae9f724b7418d76da75012103a11506993946e20ea82686b157bf08f944759f43d91af8d84650ee73a482431c02483045022100c10cdbe21cedab3b4e7db9422f69c7074764711d552a63545104d71c905b138802204999f3ecb5fdadfd8669a8c14f04643c59bb3e98aaf52c52f829a0f6ef5d6abb012103a11506993946e20ea82686b157bf08f944759f43d91af8d84650ee73a482431c00000000" + ); +} + TEST(BitcoinSigning, ExtraOutputs) { auto privateKey = parse_hex("e253373989199da27c48680e3a3fc0f648d50f9a727ef17a7fe6a4dc3b159129"); auto ownAddress = "1MhdctqCwYMn2DT4mshpwvYtfF98wBojXS"; @@ -202,6 +262,9 @@ TEST(BitcoinSigning, ExtraOutputsRequireExtraInputs) { signingInput.set_to_address(toAddress); signingInput.set_change_address(ownAddress); signingInput.add_private_key(privateKey.data(), privateKey.size()); + // Dust threshold will be 612 (102 * 6) if otherwise is not set. + // So to fix the test, we should set the 313 dust threshold for the change output to be included. + signingInput.set_fixed_dust_threshold(313); auto utxoScript = Script::lockScriptForAddress(ownAddress, TWCoinTypeBitcoin); auto& utxo0 = *signingInput.add_utxo(); @@ -626,6 +689,202 @@ TEST(BitcoinSigning, SignNftInscriptionReveal) { ASSERT_EQ(result.substr(292, result.size() - 292), expectedHex.substr(292, result.size() - 292)); } +TEST(BitcoinSigning, SignPlanTransactionWithDustAmount) { + const auto privateKey = parse_hex("4646464646464646464646464646464646464646464646464646464646464646"); + const auto ownAddress = "bc1qhkfq3zahaqkkzx5mjnamwjsfpq2jk7z00ppggv"; + + const auto revUtxoHash0 = + parse_hex("07c42b969286be06fae38528c85f0a1ce508d4df837eb5ac4cf5f2a7a9d65fa8"); + const auto utxoScript0 = parse_hex("0014bd92088bb7e82d611a9b94fbb74a0908152b784f"); + + const auto dustAmount = 546; + // Use an amount less than dust. + const auto sendAmount = 545; + const auto availableAmount = 10'189'534; + + Proto::SigningInput signingInput; + signingInput.set_coin_type(TWCoinTypeBitcoin); + signingInput.set_hash_type(TWBitcoinSigHashTypeAll); + signingInput.set_amount(sendAmount); + signingInput.set_byte_fee(1); + signingInput.set_to_address("bc1q2dsdlq3343vk29runkgv4yc292hmq53jedfjmp"); + signingInput.set_change_address(ownAddress); + signingInput.set_fixed_dust_threshold(dustAmount); + + *signingInput.add_private_key() = std::string(privateKey.begin(), privateKey.end()); + + // Add UTXO + auto utxo = signingInput.add_utxo(); + utxo->set_script(utxoScript0.data(), utxoScript0.size()); + utxo->set_amount(availableAmount); + utxo->mutable_out_point()->set_hash( + std::string(revUtxoHash0.begin(), revUtxoHash0.end())); + utxo->mutable_out_point()->set_index(0); + utxo->mutable_out_point()->set_sequence(UINT32_MAX); + + Proto::TransactionPlan plan; + ANY_PLAN(signingInput, plan, TWCoinTypeBitcoin); + EXPECT_EQ(plan.error(), Common::Proto::Error_dust_amount_requested); + + // `AnySigner.sign` should return the same error. + Proto::SigningOutput output; + ANY_SIGN(signingInput, TWCoinTypeBitcoin); + EXPECT_EQ(output.error(), Common::Proto::Error_dust_amount_requested); +} + +// If the change amount is less than "dust", there should not be a change output. +TEST(BitcoinSigning, SignPlanTransactionNoChange) { + const auto myPrivateKey = PrivateKey(parse_hex("9ea2172511ed73ae0096be8e593c3b75631700edaf729f1abbae607314a20e35")); + auto myPublicKey = myPrivateKey.getPublicKey(TWPublicKeyTypeSECP256k1); + auto utxoPubkeyHash = Hash::ripemd(Hash::sha256(myPublicKey.bytes)); + auto redeemScript = Script::buildPayToWitnessPublicKeyHash(utxoPubkeyHash); + + const auto ownAddress = "bc1qvrt7ukvhvmdny0a3j9k8l8jasx92lrqm30t2u2"; + + auto utxoHash0 = + parse_hex("b33082a5fad105c1d9712e8d503971fe4d84713065bd323fd1019636ed940e8d"); + std::reverse(utxoHash0.begin(), utxoHash0.end()); + auto utxoAmount0 = 30269; + auto utxoOutputIndex0 = 1; + + auto utxoHash1 = + parse_hex("1f62c18bfc5f8293a2b7b061587c427bf830fb224289f9a806e6ad48de6a4c7d"); + std::reverse(utxoHash1.begin(), utxoHash1.end()); + auto utxoAmount1 = 4863; + auto utxoOutputIndex1 = 1; + + auto utxoHash2 = + parse_hex("71c3343dfca5f1914e1bfc04153517d73650cb9c931e8511d24d1f5290120f6f"); + std::reverse(utxoHash2.begin(), utxoHash2.end()); + // This UTXO will be filtered out as less than dust threshold. + auto utxoAmount2 = 300; + auto utxoOutputIndex2 = 0; + + const auto dustAmount = 546; + // Change amount is too low (less than dust), so we just waste it as the transaction fee. + const auto dustChange = 200; + const auto sendAmount = 28235 - dustChange; + + Proto::SigningInput signingInput; + signingInput.set_coin_type(TWCoinTypeBitcoin); + signingInput.set_hash_type(TWBitcoinSigHashTypeAll); + signingInput.set_byte_fee(33); + signingInput.set_amount(sendAmount); + signingInput.set_to_address("bc1q2dsdlq3343vk29runkgv4yc292hmq53jedfjmp"); + signingInput.set_change_address(ownAddress); + signingInput.set_fixed_dust_threshold(dustAmount); + + signingInput.add_private_key(myPrivateKey.bytes.data(), myPrivateKey.bytes.size()); + + // Add UTXO 0 + auto utxo0 = signingInput.add_utxo(); + utxo0->set_script(redeemScript.bytes.data(), redeemScript.bytes.size()); + utxo0->set_amount(utxoAmount0); + utxo0->mutable_out_point()->set_hash( + std::string(utxoHash0.begin(), utxoHash0.end())); + utxo0->mutable_out_point()->set_index(utxoOutputIndex0); + utxo0->mutable_out_point()->set_sequence(UINT32_MAX); + + // Add UTXO 1 + auto utxo1 = signingInput.add_utxo(); + utxo1->set_script(redeemScript.bytes.data(), redeemScript.bytes.size()); + utxo1->set_amount(utxoAmount1); + utxo1->mutable_out_point()->set_hash( + std::string(utxoHash1.begin(), utxoHash1.end())); + utxo1->mutable_out_point()->set_index(utxoOutputIndex1); + utxo1->mutable_out_point()->set_sequence(UINT32_MAX); + + // Add UTXO 2 + auto utxo2 = signingInput.add_utxo(); + utxo2->set_script(redeemScript.bytes.data(), redeemScript.bytes.size()); + utxo2->set_amount(utxoAmount2); + utxo2->mutable_out_point()->set_hash( + std::string(utxoHash2.begin(), utxoHash2.end())); + utxo2->mutable_out_point()->set_index(utxoOutputIndex2); + utxo2->mutable_out_point()->set_sequence(UINT32_MAX); + + Proto::TransactionPlan plan; + ANY_PLAN(signingInput, plan, TWCoinTypeBitcoin); + EXPECT_EQ(plan.error(), Common::Proto::OK); + + auto fee = 6897 + dustChange; + // UTXO-2 with 300 satoshis should be filtered out as less than dust. + EXPECT_TRUE(verifyPlan(plan, {4863, 30269}, sendAmount, fee)); + + Proto::SigningOutput output; + ANY_SIGN(signingInput, TWCoinTypeBitcoin); + EXPECT_EQ(output.error(), Common::Proto::OK); + // Successfully broadcasted: https://mempool.space/tx/5d6bf53576a54be4d92cd8abf58d28ecc9ea7956eaf970d24d6bfcb9fcfe9855 + EXPECT_EQ(hex(output.encoded()), "010000000001027d4c6ade48ade606a8f9894222fb30f87b427c5861b0b7a293825ffc8bc1621f0100000000ffffffff8d0e94ed369601d13f32bd653071844dfe7139508d2e71d9c105d1faa58230b30100000000ffffffff01836d0000000000001600145360df8231ac5965147c9d90ca930a2aafb0523202483045022100f95f9ac5d39f4b47dcd8c86daaaeac86374258d9960f922333ba0d5fdaa15b7e0220761794672dc9fbd71398d608f72f5d21a0f6c1306c6b700ad0d82f747c221062012103a11506993946e20ea82686b157bf08f944759f43d91af8d84650ee73a482431c02483045022100eb6ba0dcc64af61b2186b7efdab1ff03784d585ee03437f9a53875e93429db080220015a268d308436d3564b83ceaed90bc7272ca164016298ea855d1936568002a7012103a11506993946e20ea82686b157bf08f944759f43d91af8d84650ee73a482431c00000000"); +} + +// Not enough funds to send requested amount after UTXO dust filtering. +TEST(BitcoinSigning, SignPlanTransactionNotSufficientAfterDustFiltering) { + const auto myPrivateKey = PrivateKey(parse_hex("9ea2172511ed73ae0096be8e593c3b75631700edaf729f1abbae607314a20e35")); + auto myPublicKey = myPrivateKey.getPublicKey(TWPublicKeyTypeSECP256k1); + auto utxoPubkeyHash = Hash::ripemd(Hash::sha256(myPublicKey.bytes)); + auto redeemScript = Script::buildPayToWitnessPublicKeyHash(utxoPubkeyHash); + + const auto ownAddress = "bc1qvrt7ukvhvmdny0a3j9k8l8jasx92lrqm30t2u2"; + + auto utxoHash0 = + parse_hex("b33082a5fad105c1d9712e8d503971fe4d84713065bd323fd1019636ed940e8d"); + std::reverse(utxoHash0.begin(), utxoHash0.end()); + auto utxoAmount0 = 30269; + auto utxoOutputIndex0 = 1; + + auto utxoHash1 = + parse_hex("1f62c18bfc5f8293a2b7b061587c427bf830fb224289f9a806e6ad48de6a4c7d"); + std::reverse(utxoHash1.begin(), utxoHash1.end()); + auto utxoAmount1 = 545; + auto utxoOutputIndex1 = 1; + + const auto utxoScript0 = parse_hex("0014bd92088bb7e82d611a9b94fbb74a0908152b784f"); + + const auto dustAmount = 546; + const auto sendAmount = 25620; + + Proto::SigningInput signingInput; + signingInput.set_coin_type(TWCoinTypeBitcoin); + signingInput.set_hash_type(TWBitcoinSigHashTypeAll); + signingInput.set_byte_fee(33); + signingInput.set_amount(sendAmount); + signingInput.set_to_address("bc1q2dsdlq3343vk29runkgv4yc292hmq53jedfjmp"); + signingInput.set_change_address(ownAddress); + signingInput.set_fixed_dust_threshold(dustAmount); + + signingInput.add_private_key(myPrivateKey.bytes.data(), myPrivateKey.bytes.size()); + + // Add UTXO 0 + auto utxo0 = signingInput.add_utxo(); + utxo0->set_script(redeemScript.bytes.data(), redeemScript.bytes.size()); + utxo0->set_amount(utxoAmount0); + utxo0->mutable_out_point()->set_hash( + std::string(utxoHash0.begin(), utxoHash0.end())); + utxo0->mutable_out_point()->set_index(utxoOutputIndex0); + utxo0->mutable_out_point()->set_sequence(UINT32_MAX); + + // Add UTXO 1 + auto utxo1 = signingInput.add_utxo(); + utxo1->set_script(redeemScript.bytes.data(), redeemScript.bytes.size()); + utxo1->set_amount(utxoAmount1); + utxo1->mutable_out_point()->set_hash( + std::string(utxoHash1.begin(), utxoHash1.end())); + utxo1->mutable_out_point()->set_index(utxoOutputIndex1); + utxo1->mutable_out_point()->set_sequence(UINT32_MAX); + + // sum() + + Proto::TransactionPlan plan; + ANY_PLAN(signingInput, plan, TWCoinTypeBitcoin); + EXPECT_EQ(plan.error(), Common::Proto::Error_not_enough_utxos); + + // `AnySigner.sign` should return the same error. + Proto::SigningOutput output; + ANY_SIGN(signingInput, TWCoinTypeBitcoin); + EXPECT_EQ(output.error(), Common::Proto::Error_not_enough_utxos); +} + TEST(BitcoinSigning, SignP2PKH) { auto input = buildInputP2PKH(); @@ -1025,6 +1284,9 @@ SigningInput buildInputP2WSH(enum TWBitcoinSigHashType hashType, bool omitScript input.byteFee = 1; input.toAddress = "1Bp9U1ogV3A14FMvKbRJms7ctyso4Z4Tcx"; input.changeAddress = "1FQc5LdgGHMHEN9nwkjmz6tWkxhPpxBvBU"; + // Set the very low fixed Dust threshold just to fix the tests. + // Actually, transactions in these tests have change=79 and change=52 that will lead to Dust error when broadcasting it. + input.dustCalculator = std::make_shared(50); if (!omitKeys) { auto utxoKey0 = PrivateKey(parse_hex("ed00a0841cd53aedf89b0c616742d1d2a930f8ae2b0fb514765a17bb62c7521a")); diff --git a/tests/chains/Bitcoin/TransactionPlanTests.cpp b/tests/chains/Bitcoin/TransactionPlanTests.cpp index 7953fdd8ef4..5a4d6597f74 100644 --- a/tests/chains/Bitcoin/TransactionPlanTests.cpp +++ b/tests/chains/Bitcoin/TransactionPlanTests.cpp @@ -59,17 +59,29 @@ TEST(TransactionPlan, OneInsufficientLower100) { EXPECT_TRUE(verifyPlan(txPlan, {}, 0, 0, Common::Proto::Error_not_enough_utxos)); } -TEST(TransactionPlan, OneInsufficientLower170) { +TEST(TransactionPlan, OneInsufficient146) { // requested is only slightly lower than avail, not enough for fee, cannot be satisfied auto utxos = buildTestUTXOs({100'000}); - auto sigingInput = buildSigningInput(100'000 - 170, 1, utxos); + auto sigingInput = buildSigningInput(100'000 - 146, 1, utxos); auto txPlan = TransactionBuilder::plan(sigingInput); EXPECT_TRUE(verifyPlan(txPlan, {}, 0, 0, Common::Proto::Error_not_enough_utxos)); } -TEST(TransactionPlan, OneInsufficientLower300) { +TEST(TransactionPlan, OneSufficientLower170) { + // requested is only slightly lower than avail, not enough for fee, cannot be satisfied + auto utxos = buildTestUTXOs({100'000}); + auto sigingInput = buildSigningInput(100'000 - 170, 1, utxos); + + auto txPlan = TransactionBuilder::plan(sigingInput); + + auto dustChange = 23; + auto actualFee = 147 + dustChange; + EXPECT_TRUE(verifyPlan(txPlan, {100'000}, 100'000 - 170, actualFee)); +} + +TEST(TransactionPlan, OneSufficientLower300) { auto utxos = buildTestUTXOs({100'000}); auto sigingInput = buildSigningInput(100'000 - 300, 1, utxos); @@ -92,7 +104,9 @@ TEST(TransactionPlan, OneMoreRequested) { TEST(TransactionPlan, OneFitsExactly) { auto utxos = buildTestUTXOs({100'000}); auto byteFee = 1; - auto expectedFee = 147; + auto dustChange = 27; + // Change amount is too low (less than dust), so we just waste it as the transaction fee. + auto expectedFee = 147 + dustChange; auto sigingInput = buildSigningInput(100'000 - 174, byteFee, utxos); auto txPlan = TransactionBuilder::plan(sigingInput); @@ -106,7 +120,9 @@ TEST(TransactionPlan, OneFitsExactly) { TEST(TransactionPlan, OneFitsExactlyHighFee) { auto utxos = buildTestUTXOs({100'000}); auto byteFee = 10; - auto expectedFee = 1470; + auto dustChange = 270; + // Change amount is too low (less than dust), so we just waste it as the transaction fee. + auto expectedFee = 1470 + dustChange; auto sigingInput = buildSigningInput(100'000 - 1740, byteFee, utxos); auto txPlan = TransactionBuilder::plan(sigingInput); @@ -239,6 +255,26 @@ TEST(TransactionPlan, SelectionSuboptimal_ExtraSmallUtxo) { EXPECT_EQ(firstUtxo, 500); } +TEST(TransactionPlan, SelectionSuboptimal_ExtraSmallUtxoFixedDust) { + // Solution found 4-in-2-out {500, 600, 800, 1000} avail 2900 txamount 1390 fee 702 change 628 + // Better solution: 3-in-2-out {600, 800, 1000} avail 2400 txamount 1390 fee 566 change 444 + // Previously, with with higher fee estimation used in UTXO selection, solution found was 5-in-2-out {400, 500, 600, 800, 1000} avail 3300 txamount 1390 fee 838 change 1072 + auto utxos = buildTestUTXOs({400, 500, 600, 800, 1'000}); + auto byteFee = 2; + auto signingInput = buildSigningInput(1'390, byteFee, utxos); + signingInput.dustCalculator = std::make_shared(450); + + // UTXOs smaller than singleInputFee are not included + auto txPlan = TransactionBuilder::plan(signingInput); + + auto expectedFee = 702; + EXPECT_TRUE(verifyPlan(txPlan, {500, 600, 800, 1'000}, 1'390, expectedFee)); + auto change = 2'900 - 1'390 - expectedFee; + auto firstUtxo = txPlan.utxos[0].amount; + EXPECT_EQ(change, 808); + EXPECT_EQ(firstUtxo, 500); +} + TEST(TransactionPlan, Selection_Satisfied5) { // 5-input case, with a 5-input solution. // Previously, with with higher fee estimation used in UTXO selection, no solution would be found. @@ -484,15 +520,15 @@ TEST(TransactionPlan, MaxAmountDustAllFee10) { } TEST(TransactionPlan, One_MaxAmount_FeeMoreThanAvailable) { - auto utxos = buildTestUTXOs({170}); + auto utxos = buildTestUTXOs({340}); auto byteFee = 1; auto expectedFee = 113; - auto sigingInput = buildSigningInput(300, byteFee, utxos, true); + auto sigingInput = buildSigningInput(340, byteFee, utxos, true); auto txPlan = TransactionBuilder::plan(sigingInput); // Fee is reduced to availableAmount - EXPECT_TRUE(verifyPlan(txPlan, {170}, 170 - expectedFee, expectedFee)); + EXPECT_TRUE(verifyPlan(txPlan, {340}, 340 - expectedFee, expectedFee)); auto& feeCalculator = getFeeCalculator(TWCoinTypeBitcoin); EXPECT_EQ(feeCalculator.calculate(1, 1, byteFee), 143); diff --git a/tests/chains/Bitcoin/TxComparisonHelper.cpp b/tests/chains/Bitcoin/TxComparisonHelper.cpp index c003744bd13..f674bcaaff7 100644 --- a/tests/chains/Bitcoin/TxComparisonHelper.cpp +++ b/tests/chains/Bitcoin/TxComparisonHelper.cpp @@ -45,6 +45,7 @@ SigningInput buildSigningInput(Amount amount, int byteFee, const UTXOs& utxos, b input.byteFee = byteFee; input.useMaxAmount = useMaxAmount; input.coinType = coin; + input.dustCalculator = std::make_shared(coin); if (!omitPrivateKey) { auto utxoKey = PrivateKey(parse_hex("619c335025c7f4012e556c2a58b2506e30b8511b53ade95ea316fd8c3286feb9"));