[Bitcoin]: UTXO selection and dust improvements (#3675)

* [Bitcoin]: Add Fixed Dust threshold to SigningInput * Filter out Dust UTXOs before actual UTXO selecting * [Bitcoin]: Check the Change output to be at least Dust threshold * [Bitcoin]: Add Fixed Dust threshold to SigningInput * Filter out Dust UTXOs before actual UTXO selecting * [Bitcoin]: Fix iOS test * [Bitcoin]: Fix comments * [Bitcoin]: Update KMP WC * [Bitcoin]: Return Error_not_enough_utxos if max amount requested, but total amount is dust
trustwallet · Mar 1, 2024 · 7c71119 · 7c71119
1 parent f8ccbc9
commit 7c71119
Show file tree

Hide file tree

Showing 15 changed files with 552 additions and 70 deletions.
diff --git 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
@@ -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<FixedDustCalculator>(0);
+    }
+
+    if (input.has_fixed_dust_threshold()) {
+        return std::make_shared<FixedDustCalculator>(input.fixed_dust_threshold());
+    }
+
+    const auto coinType = static_cast<TWCoinType>(input.coin_type());
+    return std::make_shared<LegacyDustCalculator>(coinType);
+}
+
+} // namespace TW::Bitcoin
diff --git a/src/Bitcoin/DustCalculator.h 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 <memory>
+#include <TrustWalletCore/TWCoinType.h>
+
+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<DustCalculator>;
+
+DustCalculatorShared getDustCalculator(const Proto::SigningInput& input);
+
+} // namespace TW::Bitcoin
diff --git a/src/Bitcoin/InputSelector.cpp b/src/Bitcoin/InputSelector.cpp
@@ -25,8 +25,8 @@ template <typename TypeWithAmount>
 std::vector<TypeWithAmount>
 InputSelector<TypeWithAmount>::filterOutDust(const std::vector<TypeWithAmount>& inputs,
                                              int64_t byteFee) noexcept {
-    auto inputFeeLimit = static_cast<uint64_t>(feeCalculator.calculateSingleInput(byteFee));
-    return filterThreshold(inputs, inputFeeLimit);
+    auto dustThreshold = static_cast<uint64_t>(dustCalculator->dustAmount(byteFee));
+    return filterThreshold(inputs, dustThreshold);
 }
 
 // Filters utxos that are dust
@@ -36,7 +36,7 @@ InputSelector<TypeWithAmount>::filterThreshold(const std::vector<TypeWithAmount>
                                                uint64_t minimumAmount) noexcept {
     std::vector<TypeWithAmount> filtered;
     for (auto& i : inputs) {
-        if (static_cast<uint64_t>(i.amount) > minimumAmount) {
+        if (static_cast<uint64_t>(i.amount) >= minimumAmount) {
             filtered.push_back(i);
         }
     }
@@ -70,22 +70,24 @@ InputSelector<TypeWithAmount>::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<TypeWithAmount> 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<TypeWithAmount> 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<uint64_t> maxWithXInputs = std::vector<uint64_t>();
@@ -104,7 +106,7 @@ InputSelector<TypeWithAmount>::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<TypeWithAmount>::select(uint64_t targetValue, uint64_t byteFee, ui
                                    const std::vector<TypeWithAmount>& rhs) {
                           return distFrom2x(sum(lhs)) < distFrom2x(sum(rhs));
                       });
-            return filterOutDust(slices.front(), byteFee);
+            return slices.front();
         }
     }
 
@@ -150,11 +152,14 @@ InputSelector<TypeWithAmount>::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 <typename TypeWithAmount>
@@ -170,13 +175,13 @@ std::vector<TypeWithAmount> InputSelector<TypeWithAmount>::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<TypeWithAmount> InputSelector<TypeWithAmount>::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 <typename TypeWithAmount>

diff --git a/src/Bitcoin/InputSelector.h b/src/Bitcoin/InputSelector.h
@@ -5,6 +5,7 @@
 #pragma once
 
 #include "FeeCalculator.h"
+#include "DustCalculator.h"
 #include <TrustWalletCore/TWCoinType.h>
 
 #include <numeric>
@@ -17,29 +18,39 @@ 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<TypeWithAmount> 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<TypeWithAmount> selectSimple(int64_t targetValue, int64_t byteFee,
                                              int64_t numOutputs = 2);
 
     /// Selects UTXOs for max amount; select all except those which would reduce output (dust).
     /// Return indices. One output and no change is assumed.
     std::vector<TypeWithAmount> 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<TypeWithAmount>& 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<TypeWithAmount>& inputs) noexcept
-        : InputSelector(inputs, getFeeCalculator(TWCoinTypeBitcoin)) {}
+        : _inputs(inputs),
+          feeCalculator(getFeeCalculator(TWCoinTypeBitcoin)),
+          dustCalculator(std::make_shared<LegacyDustCalculator>(TWCoinTypeBitcoin)) {
+    }
 
     /// Sum of input amounts
     static uint64_t sum(const std::vector<TypeWithAmount>& amounts) noexcept;
@@ -53,6 +64,7 @@ class InputSelector {
 private:
     const std::vector<TypeWithAmount> _inputs;
     const FeeCalculator& feeCalculator;
+    const DustCalculatorShared dustCalculator;
 };
 
 } // namespace TW::Bitcoin
diff --git a/src/Bitcoin/SigningInput.cpp b/src/Bitcoin/SigningInput.cpp
@@ -6,6 +6,10 @@
 
 namespace TW::Bitcoin {
 
+SigningInput::SigningInput()
+    : dustCalculator(std::make_shared<LegacyDustCalculator>(TWCoinTypeBitcoin)) {
+}
+
 SigningInput::SigningInput(const Proto::SigningInput& input) {
     hashType = static_cast<TWBitcoinSigHashType>(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
@@ -5,6 +5,7 @@
 #pragma once
 
 #include "Amount.h"
+#include "DustCalculator.h"
 #include "Transaction.h"
 #include "UTXO.h"
 #include <TrustWalletCore/TWBitcoinSigHashType.h>
@@ -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);
 };