DashCoreAutoGuix · PastaPastaPasta · Aug 5, 2022
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
@@ -189,7 +189,7 @@ class Wallet
     virtual WalletTx getWalletTx(const uint256& txid) = 0;
 
     //! Get list of all wallet transactions.
-    virtual std::vector<WalletTx> getWalletTxs() = 0;
+    virtual std::set<WalletTx> getWalletTxs() = 0;
 
     //! Try to get updated status for a particular transaction, if possible without blocking.
     virtual bool tryGetTxStatus(const uint256& txid,
@@ -423,6 +423,8 @@ struct WalletTx
     bool is_coinbase;
     bool is_platform_transfer{false};
     bool is_denominate;
+
+    bool operator<(const WalletTx& a) const { return tx->GetHash() < a.tx->GetHash(); }
 };
 
 //! Updated transaction status.

diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
@@ -0,0 +1,278 @@
+// Copyright (c) 2017-2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <interfaces/chain.h>
+#include <policy/fees.h>
+#include <policy/policy.h>
+#include <util/moneystr.h>
+#include <util/rbf.h>
+#include <util/system.h>
+#include <util/translation.h>
+#include <wallet/coincontrol.h>
+#include <wallet/feebumper.h>
+#include <wallet/fees.h>
+#include <wallet/receive.h>
+#include <wallet/spend.h>
+#include <wallet/wallet.h>
+
+namespace wallet {
+//! Check whether transaction has descendant in wallet or mempool, or has been
+//! mined, or conflicts with a mined transaction. Return a feebumper::Result.
+static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
+{
+    if (wallet.HasWalletSpend(wtx.tx)) {
+        errors.push_back(Untranslated("Transaction has descendants in the wallet"));
+        return feebumper::Result::INVALID_PARAMETER;
+    }
+
+    {
+        if (wallet.chain().hasDescendantsInMempool(wtx.GetHash())) {
+            errors.push_back(Untranslated("Transaction has descendants in the mempool"));
+            return feebumper::Result::INVALID_PARAMETER;
+        }
+    }
+
+    if (wallet.GetTxDepthInMainChain(wtx) != 0) {
+        errors.push_back(Untranslated("Transaction has been mined, or is conflicted with a mined transaction"));
+        return feebumper::Result::WALLET_ERROR;
+    }
+
+    if (!SignalsOptInRBF(*wtx.tx)) {
+        errors.push_back(Untranslated("Transaction is not BIP 125 replaceable"));
+        return feebumper::Result::WALLET_ERROR;
+    }
+
+    if (wtx.mapValue.count("replaced_by_txid")) {
+        errors.push_back(strprintf(Untranslated("Cannot bump transaction %s which was already bumped by transaction %s"), wtx.GetHash().ToString(), wtx.mapValue.at("replaced_by_txid")));
+        return feebumper::Result::WALLET_ERROR;
+    }
+
+    // check that original tx consists entirely of our inputs
+    // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
+    isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
+    if (!AllInputsMine(wallet, *wtx.tx, filter)) {
+        errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet"));
+        return feebumper::Result::WALLET_ERROR;
+    }
+
+
+    return feebumper::Result::OK;
+}
+
+//! Check if the user provided a valid feeRate
+static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector<bilingual_str>& errors)
+{
+    // check that fee rate is higher than mempool's minimum fee
+    // (no point in bumping fee if we know that the new tx won't be accepted to the mempool)
+    // This may occur if the user set fee_rate or paytxfee too low, if fallbackfee is too low, or, perhaps,
+    // in a rare situation where the mempool minimum fee increased significantly since the fee estimation just a
+    // moment earlier. In this case, we report an error to the user, who may adjust the fee.
+    CFeeRate minMempoolFeeRate = wallet.chain().mempoolMinFee();
+
+    if (newFeerate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) {
+        errors.push_back(strprintf(
+            Untranslated("New fee rate (%s) is lower than the minimum fee rate (%s) to get into the mempool -- "),
+            FormatMoney(newFeerate.GetFeePerK()),
+            FormatMoney(minMempoolFeeRate.GetFeePerK())));
+        return feebumper::Result::WALLET_ERROR;
+    }
+
+    CAmount new_total_fee = newFeerate.GetFee(maxTxSize);
+
+    CFeeRate incrementalRelayFee = std::max(wallet.chain().relayIncrementalFee(), CFeeRate(WALLET_INCREMENTAL_RELAY_FEE));
+
+    // Given old total fee and transaction size, calculate the old feeRate
+    isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
+    CAmount old_fee = CachedTxGetDebit(wallet, wtx, filter) - wtx.tx->GetValueOut();
+    const int64_t txSize = GetVirtualTransactionSize(*(wtx.tx));
+    CFeeRate nOldFeeRate(old_fee, txSize);
+    // Min total fee is old fee + relay fee
+    CAmount minTotalFee = nOldFeeRate.GetFee(maxTxSize) + incrementalRelayFee.GetFee(maxTxSize);
+
+    if (new_total_fee < minTotalFee) {
+        errors.push_back(strprintf(Untranslated("Insufficient total fee %s, must be at least %s (oldFee %s + incrementalFee %s)"),
+            FormatMoney(new_total_fee), FormatMoney(minTotalFee), FormatMoney(nOldFeeRate.GetFee(maxTxSize)), FormatMoney(incrementalRelayFee.GetFee(maxTxSize))));
+        return feebumper::Result::INVALID_PARAMETER;
+    }
+
+    CAmount requiredFee = GetRequiredFee(wallet, maxTxSize);
+    if (new_total_fee < requiredFee) {
+        errors.push_back(strprintf(Untranslated("Insufficient total fee (cannot be less than required fee %s)"),
+            FormatMoney(requiredFee)));
+        return feebumper::Result::INVALID_PARAMETER;
+    }
+
+    // Check that in all cases the new fee doesn't violate maxTxFee
+    const CAmount max_tx_fee = wallet.m_default_max_tx_fee;
+    if (new_total_fee > max_tx_fee) {
+        errors.push_back(strprintf(Untranslated("Specified or calculated fee %s is too high (cannot be higher than -maxtxfee %s)"),
+            FormatMoney(new_total_fee), FormatMoney(max_tx_fee)));
+        return feebumper::Result::WALLET_ERROR;
+    }
+
+    return feebumper::Result::OK;
+}
+
+static CFeeRate EstimateFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CAmount old_fee, const CCoinControl& coin_control)
+{
+    // Get the fee rate of the original transaction. This is calculated from
+    // the tx fee/vsize, so it may have been rounded down. Add 1 satoshi to the
+    // result.
+    int64_t txSize = GetVirtualTransactionSize(*(wtx.tx));
+    CFeeRate feerate(old_fee, txSize);
+    feerate += CFeeRate(1);
+
+    // The node has a configurable incremental relay fee. Increment the fee by
+    // the minimum of that and the wallet's conservative
+    // WALLET_INCREMENTAL_RELAY_FEE value to future proof against changes to
+    // network wide policy for incremental relay fee that our node may not be
+    // aware of. This ensures we're over the required relay fee rate
+    // (BIP 125 rule 4).  The replacement tx will be at least as large as the
+    // original tx, so the total fee will be greater (BIP 125 rule 3)
+    CFeeRate node_incremental_relay_fee = wallet.chain().relayIncrementalFee();
+    CFeeRate wallet_incremental_relay_fee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE);
+    feerate += std::max(node_incremental_relay_fee, wallet_incremental_relay_fee);
+
+    // Fee rate must also be at least the wallet's GetMinimumFeeRate
+    CFeeRate min_feerate(GetMinimumFeeRate(wallet, coin_control, /*feeCalc=*/nullptr));
+
+    // Set the required fee rate for the replacement transaction in coin control.
+    return std::max(feerate, min_feerate);
+}
+
+namespace feebumper {
+
+bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
+{
+    LOCK(wallet.cs_wallet);
+    const CWalletTx* wtx = wallet.GetWalletTx(txid);
+    if (wtx == nullptr) return false;
+
+    std::vector<bilingual_str> errors_dummy;
+    feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy);
+    return res == feebumper::Result::OK;
+}
+
+Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors,
+                                 CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx)
+{
+    // We are going to modify coin control later, copy to re-use
+    CCoinControl new_coin_control(coin_control);
+
+    LOCK(wallet.cs_wallet);
+    errors.clear();
+    auto it = wallet.mapWallet.find(txid);
+    if (it == wallet.mapWallet.end()) {
+        errors.push_back(Untranslated("Invalid or non-wallet transaction id"));
+        return Result::INVALID_ADDRESS_OR_KEY;
+    }
+    const CWalletTx& wtx = it->second;
+
+    Result result = PreconditionChecks(wallet, wtx, errors);
+    if (result != Result::OK) {
+        return result;
+    }
+
+    // Fill in recipients(and preserve a single change key if there is one)
+    std::vector<CRecipient> recipients;
+    for (const auto& output : wtx.tx->vout) {
+        if (!OutputIsChange(wallet, output)) {
+            CRecipient recipient = {output.scriptPubKey, output.nValue, false};
+            recipients.push_back(recipient);
+        } else {
+            CTxDestination change_dest;
+            ExtractDestination(output.scriptPubKey, change_dest);
+            new_coin_control.destChange = change_dest;
+        }
+    }
+
+    isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
+    old_fee = CachedTxGetDebit(wallet, wtx, filter) - wtx.tx->GetValueOut();
+
+    if (coin_control.m_feerate) {
+        // The user provided a feeRate argument.
+        // We calculate this here to avoid compiler warning on the cs_wallet lock
+        const int64_t maxTxSize{CalculateMaximumSignedTxSize(*wtx.tx, &wallet).vsize};
+        Result res = CheckFeeRate(wallet, wtx, *new_coin_control.m_feerate, maxTxSize, errors);
+        if (res != Result::OK) {
+            return res;
+        }
+    } else {
+        // The user did not provide a feeRate argument
+        new_coin_control.m_feerate = EstimateFeeRate(wallet, wtx, old_fee, new_coin_control);
+    }
+
+    // Fill in required inputs we are double-spending(all of them)
+    // N.B.: bip125 doesn't require all the inputs in the replaced transaction to be
+    // used in the replacement transaction, but it's very important for wallets to make
+    // sure that happens. If not, it would be possible to bump a transaction A twice to
+    // A2 and A3 where A2 and A3 don't conflict (or alternatively bump A to A2 and A2
+    // to A3 where A and A3 don't conflict). If both later get confirmed then the sender
+    // has accidentally double paid.
+    for (const auto& inputs : wtx.tx->vin) {
+        new_coin_control.Select(COutPoint(inputs.prevout));
+    }
+    new_coin_control.m_allow_other_inputs = true;
+
+    // We cannot source new unconfirmed inputs(bip125 rule 2)
+    new_coin_control.m_min_depth = 1;
+
+    constexpr int RANDOM_CHANGE_POSITION = -1;
+    auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, new_coin_control, false);
+    if (!res) {
+        errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + util::ErrorString(res));
+        return Result::WALLET_ERROR;
+    }
+
+    const auto& txr = *res;
+    // Write back new fee if successful
+    new_fee = txr.fee;
+
+    // Write back transaction
+    mtx = CMutableTransaction(*txr.tx);
+
+    return Result::OK;
+}
+
+bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
+    LOCK(wallet.cs_wallet);
+    return wallet.SignTransaction(mtx);
+}
+
+Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, uint256& bumped_txid)
+{
+    LOCK(wallet.cs_wallet);
+    if (!errors.empty()) {
+        return Result::MISC_ERROR;
+    }
+    auto it = txid.IsNull() ? wallet.mapWallet.end() : wallet.mapWallet.find(txid);
+    if (it == wallet.mapWallet.end()) {
+        errors.push_back(Untranslated("Invalid or non-wallet transaction id"));
+        return Result::MISC_ERROR;
+    }
+    const CWalletTx& oldWtx = it->second;
+
+    // make sure the transaction still has no descendants and hasn't been mined in the meantime
+    Result result = PreconditionChecks(wallet, oldWtx, errors);
+    if (result != Result::OK) {
+        return result;
+    }
+
+    // commit/broadcast the tx
+    CTransactionRef tx = MakeTransactionRef(std::move(mtx));
+    mapValue_t mapValue = oldWtx.mapValue;
+    mapValue["replaces_txid"] = oldWtx.GetHash().ToString();
+
+    wallet.CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm);
+
+    // mark the original tx as bumped
+    bumped_txid = tx->GetHash();
+    if (!wallet.MarkReplaced(oldWtx.GetHash(), bumped_txid)) {
+        errors.push_back(Untranslated("Created new bumpfee transaction but could not mark the original transaction as replaced"));
+    }
+    return Result::OK;
+}
+
+} // namespace feebumper
+} // namespace wallet
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
@@ -338,13 +338,12 @@ class WalletImpl : public Wallet
         }
         return {};
     }
-    std::vector<WalletTx> getWalletTxs() override
+    std::set<WalletTx> getWalletTxs() override
     {
         LOCK(m_wallet->cs_wallet);
-        std::vector<WalletTx> result;
-        result.reserve(m_wallet->mapWallet.size());
+        std::set<WalletTx> result;
         for (const auto& entry : m_wallet->mapWallet) {
-            result.emplace_back(MakeWalletTx(*m_wallet, entry.second));
+            result.emplace(MakeWalletTx(*m_wallet, entry.second));
         }
         return result;
     }

diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
@@ -2161,10 +2161,21 @@ std::unique_ptr<FlatSigningProvider> DescriptorScriptPubKeyMan::GetSigningProvid
 std::unique_ptr<FlatSigningProvider> DescriptorScriptPubKeyMan::GetSigningProvider(int32_t index, bool include_private) const
 {
     AssertLockHeld(cs_desc_man);
-    // Get the scripts, keys, and key origins for this script
+
     std::unique_ptr<FlatSigningProvider> out_keys = std::make_unique<FlatSigningProvider>();
-    std::vector<CScript> scripts_temp;
-    if (!m_wallet_descriptor.descriptor->ExpandFromCache(index, m_wallet_descriptor.cache, scripts_temp, *out_keys)) return nullptr;
+
+    // Fetch SigningProvider from cache to avoid re-deriving
+    auto it = m_map_signing_providers.find(index);
+    if (it != m_map_signing_providers.end()) {
+        *out_keys = Merge(*out_keys, it->second);
+    } else {
+        // Get the scripts, keys, and key origins for this script
+        std::vector<CScript> scripts_temp;
+        if (!m_wallet_descriptor.descriptor->ExpandFromCache(index, m_wallet_descriptor.cache, scripts_temp, *out_keys)) return nullptr;
+
+        // Cache SigningProvider so we don't need to re-derive if we need this SigningProvider again
+        m_map_signing_providers[index] = *out_keys;
+    }
 
     if (HavePrivateKeys() && include_private) {
         FlatSigningProvider master_provider;

diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
@@ -524,6 +524,8 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
 
     KeyMap GetKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man);
 
+    // Cached FlatSigningProviders to avoid regenerating them each time they are needed.
+    mutable std::map<int32_t, FlatSigningProvider> m_map_signing_providers;
     // Fetch the SigningProvider for the given script and optionally include private keys
     std::unique_ptr<FlatSigningProvider> GetSigningProvider(const CScript& script, bool include_private = false) const;
     // Fetch the SigningProvider for the given pubkey and always include private keys. This should only be called by signing code.

diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
@@ -171,12 +171,18 @@ void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const C
 
             std::unique_ptr<SigningProvider> provider = wallet.GetSolvingProvider(wtx.tx->vout[i].scriptPubKey);
 
-            bool solvable = provider ? IsSolvable(*provider, wtx.tx->vout[i].scriptPubKey) : false;
+            int input_bytes = CalculateMaximumSignedInputSize(wtx.tx->vout[i], &wallet);
+            // Because CalculateMaximumSignedInputSize just uses ProduceSignature and makes a dummy signature,
+            // it is safe to assume that this input is solvable if input_bytes is greater -1.
+            bool solvable = input_bytes > -1;
             bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
-            int input_bytes = GetTxSpendSize(wallet, wtx, i, (coinControl && coinControl->fAllowWatchOnly));
 
-            vCoins.emplace_back(COutPoint(wtx.GetHash(), i), wtx.tx->vout.at(i), nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me);
+            // If we couldn't calculate input_bytes, fall back to the Dash-specific method
+            if (input_bytes == -1) {
+                input_bytes = GetTxSpendSize(wallet, wtx, i, (coinControl && coinControl->fAllowWatchOnly));
+            }
 
+            vCoins.emplace_back(COutPoint(wtx.GetHash(), i), wtx.tx->vout.at(i), nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me);
             // Checks the sum amount of all UTXO's.
             if (nMinimumSumAmount != MAX_MONEY) {
                 nTotal += wtx.tx->vout[i].nValue;

diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
@@ -850,16 +850,16 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup)
 
     {
         auto block_hash = block_tx.GetHash();
-        auto prev_hash = m_coinbase_txns[0]->GetHash();
+        auto prev_tx = m_coinbase_txns[0];
 
         LOCK(wallet->cs_wallet);
-        BOOST_CHECK(wallet->HasWalletSpend(prev_hash));
+        BOOST_CHECK(wallet->HasWalletSpend(prev_tx));
         BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_hash), 1u);
 
         std::vector<uint256> vHashIn{ block_hash }, vHashOut;
         BOOST_CHECK_EQUAL(wallet->ZapSelectTx(vHashIn, vHashOut), DBErrors::LOAD_OK);
 
-        BOOST_CHECK(!wallet->HasWalletSpend(prev_hash));
+        BOOST_CHECK(!wallet->HasWalletSpend(prev_tx));
         BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_hash), 0u);
     }