Merge pull request #5681 from vijaydasmp/bp22_17

backport: Merge bitcoin#20466,14604,15710,17458,18722,(partial) 17934

depends/README.md

@@ -12,15 +12,18 @@ For example:
 
     make HOST=x86_64-w64-mingw32 -j4
 
-**Dash Core's configure script by default will ignore the depends output.** In
+**Dash Core's `configure` script by default will ignore the depends output.** In
 order for it to pick up libraries, tools, and settings from the depends build,
-you must point it at the appropriate `--prefix` directory generated by the
-build. In the above example, a prefix dir named x86_64-w64-mingw32 will be
-created. To use it for Dash:
+you must set the `CONFIG_SITE` environment variable to point to a `config.site` settings file.
+In the above example, a file named `depends/x86_64-w64-mingw32/share/config.site` will be
+created. To use it during compilation:
 
-    ./configure --prefix=$PWD/depends/x86_64-w64-mingw32
+    CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site ./configure
 
-Common `host-platform-triplets` for cross compilation are:
+The default install prefix when using `config.site` is `--prefix=depends/<host-platform-triplet>`,
+so depends build outputs will be installed in that location.
+
+Common `host-platform-triplet`s for cross compilation are:
 
 - `i686-pc-linux-gnu` for Linux 32 bit
 - `x86_64-pc-linux-gnu` for x86 Linux
@@ -138,4 +141,3 @@ This is an example command for a default build with no disabled dependencies:
 
 - [description.md](description.md): General description of the depends system
 - [packages.md](packages.md): Steps for adding packages
-

doc/build-unix.md

@@ -331,7 +331,8 @@ To build executables for ARM:
     cd depends
     make HOST=arm-linux-gnueabihf NO_QT=1
     cd ..
-    ./configure --prefix=$PWD/depends/arm-linux-gnueabihf --enable-reduce-exports LDFLAGS=-static-libstdc++
+    ./autogen.sh
+    CONFIG_SITE=$PWD/depends/arm-linux-gnueabihf/share/config.site ./configure --enable-reduce-exports LDFLAGS=-static-libstdc++
     make
 
 

src/Makefile.test.include

@@ -182,6 +182,7 @@ BITCOIN_TESTS += \
   wallet/test/coinjoin_tests.cpp \
   wallet/test/psbt_wallet_tests.cpp \
   wallet/test/wallet_tests.cpp \
+  wallet/test/walletdb_tests.cpp \
   wallet/test/wallet_crypto_tests.cpp \
   wallet/test/coinselector_tests.cpp \
   wallet/test/init_tests.cpp \

src/addrman.cpp

@@ -13,6 +13,8 @@
 
 #include <cmath>
 #include <optional>
+#include <unordered_map>
+#include <unordered_set>
 
 int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asmap) const
 {
@@ -115,12 +117,12 @@ CAddrInfo* CAddrMan::Find(const CService& addr, int* pnId)
         addr2.SetPort(0);
     }
 
-    std::map<CService, int>::iterator it = mapAddr.find(addr2);
+    const auto it = mapAddr.find(addr2);
     if (it == mapAddr.end())
         return nullptr;
     if (pnId)
         *pnId = (*it).second;
-    std::map<int, CAddrInfo>::iterator it2 = mapInfo.find((*it).second);
+    const auto it2 = mapInfo.find((*it).second);
     if (it2 != mapInfo.end())
         return &(*it2).second;
     return nullptr;
@@ -482,8 +484,8 @@ CAddrInfo CAddrMan::Select_(bool newOnly) const
 int CAddrMan::Check_()
 {
     AssertLockHeld(cs);
-    std::set<int> setTried;
-    std::map<int, int> mapNew;
+    std::unordered_set<int> setTried;
+    std::unordered_map<int, int> mapNew;
 
     if (vRandom.size() != (size_t)(nTried + nNew))
         return -7;

src/addrman.h

@@ -8,22 +8,22 @@
 
 #include <clientversion.h>
 #include <config/bitcoin-config.h>
+#include <fs.h>
+#include <hash.h>
 #include <netaddress.h>
 #include <protocol.h>
 #include <random.h>
+#include <streams.h>
 #include <sync.h>
 #include <timedata.h>
 #include <tinyformat.h>
 #include <util/system.h>
 
-#include <fs.h>
-#include <hash.h>
 #include <ios>
-#include <map>
 #include <optional>
 #include <set>
 #include <stdint.h>
-#include <streams.h>
+#include <unordered_map>
 #include <vector>
 
 /**
@@ -257,7 +257,7 @@ class CAddrMan
 
         int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30);
         s << nUBuckets;
-        std::map<int, int> mapUnkIds;
+        std::unordered_map<int, int> mapUnkIds;
         int nIds = 0;
         for (const auto& entry : mapInfo) {
             mapUnkIds[entry.first] = nIds;
@@ -448,13 +448,13 @@ class CAddrMan
 
         // Prune new entries with refcount 0 (as a result of collisions).
         int nLostUnk = 0;
-        for (std::map<int, CAddrInfo>::const_iterator it = mapInfo.begin(); it != mapInfo.end(); ) {
+        for (auto it = mapInfo.cbegin(); it != mapInfo.cend(); ) {
             if (it->second.fInTried == false && it->second.nRefCount == 0) {
-                std::map<int, CAddrInfo>::const_iterator itCopy = it++;
+                const auto itCopy = it++;
                 Delete(itCopy->first);
-                nLostUnk++;
+                ++nLostUnk;
             } else {
-                it++;
+                ++it;
             }
         }
         if (nLost + nLostUnk > 0) {
@@ -682,10 +682,10 @@ class CAddrMan
     int nIdCount GUARDED_BY(cs);
 
     //! table with information about all nIds
-    std::map<int, CAddrInfo> mapInfo GUARDED_BY(cs);
+    std::unordered_map<int, CAddrInfo> mapInfo GUARDED_BY(cs);
 
     //! find an nId based on its network address
-    std::map<CService, int> mapAddr GUARDED_BY(cs);
+    std::unordered_map<CNetAddr, int, CNetAddrHash> mapAddr GUARDED_BY(cs);
 
     //! randomly-ordered vector of all nIds
     //! This is mutable because it is unobservable outside the class, so any

src/netaddress.h

@@ -11,7 +11,9 @@
 
 #include <attributes.h>
 #include <compat.h>
+#include <crypto/siphash.h>
 #include <prevector.h>
+#include <random.h>
 #include <serialize.h>
 #include <tinyformat.h>
 #include <util/strencodings.h>
@@ -251,6 +253,7 @@ class CNetAddr
         }
     }
 
+    friend class CNetAddrHash;
     friend class CSubNet;
 
 private:
@@ -465,6 +468,22 @@ class CNetAddr
     }
 };
 
+class CNetAddrHash
+{
+public:
+    size_t operator()(const CNetAddr& a) const noexcept
+    {
+        CSipHasher hasher(m_salt_k0, m_salt_k1);
+        hasher.Write(a.m_net);
+        hasher.Write(a.m_addr.data(), a.m_addr.size());
+        return static_cast<size_t>(hasher.Finalize());
+    }
+
+private:
+    const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max());
+    const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max());
+};
+
 class CSubNet
 {
 protected:

src/wallet/coinselection.cpp

@@ -4,6 +4,7 @@
 
 #include <wallet/coinselection.h>
 
+#include <policy/feerate.h>
 #include <util/system.h>
 #include <util/moneystr.h>
 
@@ -365,7 +366,7 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group
 void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants) {
     m_outputs.push_back(output);
     m_from_me &= from_me;
-    m_value += output.effective_value;
+    m_value += output.txout.nValue;
     m_depth = std::min(m_depth, depth);
     // ancestors here express the number of ancestors the new coin will end up having, which is
     // the sum, rather than the max; this will overestimate in the cases where multiple inputs
@@ -374,15 +375,19 @@ void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size
     // descendants is the count as seen from the top ancestor, not the descendants as seen from the
     // coin itself; thus, this value is counted as the max, not the sum
     m_descendants = std::max(m_descendants, descendants);
-    effective_value = m_value;
+    effective_value += output.effective_value;
+    fee += output.m_fee;
+    long_term_fee += output.m_long_term_fee;
 }
 
 std::vector<CInputCoin>::iterator OutputGroup::Discard(const CInputCoin& output) {
     auto it = m_outputs.begin();
     while (it != m_outputs.end() && it->outpoint != output.outpoint) ++it;
     if (it == m_outputs.end()) return it;
-    m_value -= output.effective_value;
+    m_value -= output.txout.nValue;
     effective_value -= output.effective_value;
+    fee -= output.m_fee;
+    long_term_fee -= output.m_long_term_fee;
     return m_outputs.erase(it);
 }
 
@@ -401,3 +406,35 @@ bool OutputGroup::EligibleForSpending(const CoinEligibilityFilter& eligibility_f
         && m_ancestors <= eligibility_filter.max_ancestors
         && m_descendants <= eligibility_filter.max_descendants;
 }
+
+void OutputGroup::SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate)
+{
+    fee = 0;
+    long_term_fee = 0;
+    effective_value = 0;
+    for (CInputCoin& coin : m_outputs) {
+        coin.m_fee = coin.m_input_bytes < 0 ? 0 : effective_feerate.GetFee(coin.m_input_bytes);
+        fee += coin.m_fee;
+
+        coin.m_long_term_fee = coin.m_input_bytes < 0 ? 0 : long_term_feerate.GetFee(coin.m_input_bytes);
+        long_term_fee += coin.m_long_term_fee;
+
+        coin.effective_value = coin.txout.nValue - coin.m_fee;
+        effective_value += coin.effective_value;
+    }
+}
+
+OutputGroup OutputGroup::GetPositiveOnlyGroup()
+{
+    OutputGroup group(*this);
+    for (auto it = group.m_outputs.begin(); it != group.m_outputs.end(); ) {
+        const CInputCoin& coin = *it;
+        // Only include outputs that are positive effective value (i.e. not dust)
+        if (coin.effective_value <= 0) {
+            it = group.Discard(coin);
+        } else {
+            ++it;
+        }
+    }
+    return group;
+}

src/wallet/coinselection.h

@@ -9,6 +9,8 @@
 #include <primitives/transaction.h>
 #include <random.h>
 
+class CFeeRate;
+
 //! target minimum change amount
 static constexpr CAmount MIN_CHANGE{COIN / 100};
 //! final minimum change amount after paying for fees
@@ -36,6 +38,8 @@ class CInputCoin {
     COutPoint outpoint;
     CTxOut txout;
     CAmount effective_value;
+    CAmount m_fee{0};
+    CAmount m_long_term_fee{0};
 
     /** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */
     int m_input_bytes{-1};
@@ -92,6 +96,10 @@ struct OutputGroup
     std::vector<CInputCoin>::iterator Discard(const CInputCoin& output);
     bool IsLockedByInstantSend() const;
     bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const;
+
+    //! Update the OutputGroup's fee, long_term_fee, and effective_value based on the given feerates
+    void SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate);
+    OutputGroup GetPositiveOnlyGroup();
 };
 
 bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret, CAmount not_input_fees);

src/wallet/test/walletdb_tests.cpp

@@ -0,0 +1,29 @@
+// Copyright (c) 2012-2019 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 <test/util/setup_common.h>
+#include <clientversion.h>
+#include <streams.h>
+#include <uint256.h>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(walletdb_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(walletdb_readkeyvalue)
+{
+    /**
+     * When ReadKeyValue() reads from either a "key" or "wkey" it first reads the CDataStream steam into a
+     * CPrivKey or CWalletKey respectively and then reads a hash of the pubkey and privkey into a uint256.
+     * Wallets from 0.8 or before do not store the pubkey/privkey hash, trying to read the hash from old
+     * wallets throws an exception, for backwards compatibility this read is wrapped in a try block to
+     * silently fail. The test here makes sure the type of exception thrown from CDataStream::read()
+     * matches the type we expect, otherwise we need to update the "key"/"wkey" exception type caught.
+     */
+    CDataStream ssValue(SER_DISK, CLIENT_VERSION);
+    uint256 dummy;
+    BOOST_CHECK_THROW(ssValue >> dummy, std::ios_base::failure);
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/wallet/wallet.cpp

@@ -2745,27 +2745,15 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil
         for (OutputGroup& group : groups) {
             if (!group.EligibleForSpending(eligibility_filter)) continue;
 
-            group.fee = 0;
-            group.long_term_fee = 0;
-            group.effective_value = 0;
-            for (auto it = group.m_outputs.begin(); it != group.m_outputs.end(); ) {
-                const CInputCoin& coin = *it;
-                CAmount effective_value = coin.txout.nValue - (coin.m_input_bytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(coin.m_input_bytes));
-                // Only include outputs that are positive effective value (i.e. not dust)
-                if (effective_value > 0) {
-                    group.fee += coin.m_input_bytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(coin.m_input_bytes);
-                    group.long_term_fee += coin.m_input_bytes < 0 ? 0 : long_term_feerate.GetFee(coin.m_input_bytes);
-                    if (coin_selection_params.m_subtract_fee_outputs) {
-                        group.effective_value += coin.txout.nValue;
-                    } else {
-                        group.effective_value += effective_value;
-                    }
-                    ++it;
-                } else {
-                    it = group.Discard(coin);
-                }
+            if (coin_selection_params.m_subtract_fee_outputs) {
+                // Set the effective feerate to 0 as we don't want to use the effective value since the fees will be deducted from the output
+                group.SetFees(CFeeRate(0) /* effective_feerate */, long_term_feerate);
+            } else {
+                group.SetFees(coin_selection_params.effective_fee, long_term_feerate);
             }
-            if (group.effective_value > 0) utxo_pool.push_back(group);
+
+            OutputGroup pos_group = group.GetPositiveOnlyGroup();
+            if (pos_group.effective_value > 0) utxo_pool.push_back(pos_group);
         }
         // Calculate the fees for things that aren't inputs
         CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size);

src/wallet/walletdb.cpp

@@ -327,7 +327,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
             {
                 ssValue >> hash;
             }
-            catch (...) {}
+            catch (const std::ios_base::failure&) {}
 
             bool fSkipCheck = false;