backport: v0.20 pr5

ci/test/00_setup_env.sh

@@ -6,6 +6,12 @@
 
 export LC_ALL=C.UTF-8
 
+# The root dir.
+# The ci system copies this folder.
+# This is where the build is done (depends and dist).
+BASE_ROOT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )"/../../ >/dev/null 2>&1 && pwd )
+export BASE_ROOT_DIR
+
 echo "Setting specific values in env"
 if [ -n "${FILE_ENV}" ]; then
   set -o errexit;
@@ -17,9 +23,6 @@ export BUILD_TARGET=${BUILD_TARGET:-linux64}
 export PULL_REQUEST=${PULL_REQUEST:-false}
 export JOB_NUMBER=${JOB_NUMBER:-1}
 
-BASE_ROOT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )"/../../ >/dev/null 2>&1 && pwd )
-export BASE_ROOT_DIR
-
 echo "Fallback to default values in env (if not yet set)"
 # The number of parallel jobs to pass down to make and test_runner.py
 MAKEJOBS="-j$(nproc)"

ci/test/00_setup_env_native_tsan.sh

@@ -14,3 +14,7 @@ export GOAL="install"
 export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --with-sanitizers=thread"
 export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG"
 export PYZMQ=true
+
+# xenial comes with old clang versions that can not parse the sanitizer suppressions files
+# Remove unparseable lines as a hacky workaround
+sed -i '/^implicit-/d' "${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan"

ci/test/04_install.sh

@@ -43,7 +43,7 @@ mkdir -p "${CCACHE_DIR}"
 export ASAN_OPTIONS="detect_stack_use_after_return=1"
 export LSAN_OPTIONS="suppressions=${BASE_BUILD_DIR}/test/sanitizer_suppressions/lsan"
 export TSAN_OPTIONS="suppressions=${BASE_BUILD_DIR}/test/sanitizer_suppressions/tsan"
-export UBSAN_OPTIONS="suppressions=${BASE_BUILD_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1"
+export UBSAN_OPTIONS="suppressions=${BASE_BUILD_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1"
 env | grep -E '^(CCACHE_|WINEDEBUG|LC_ALL|BOOST_TEST_RANDOM|CONFIG_SHELL|(ASAN|LSAN|TSAN|UBSAN)_OPTIONS)' | tee /tmp/env
 if [[ $HOST = *-mingw32 ]]; then
   DOCKER_ADMIN="--cap-add SYS_ADMIN"

contrib/seeds/makeseeds.py

@@ -19,8 +19,9 @@
 
 # These are hosts that have been observed to be behaving strangely (e.g.
 # aggressively connecting to every node).
-SUSPICIOUS_HOSTS = {
-}
+with open("suspicious_hosts.txt", mode="r", encoding="utf-8") as f:
+    SUSPICIOUS_HOSTS = {s.strip() for s in f if s.strip()}
+
 
 PATTERN_IPV4 = re.compile(r"^((\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})):(\d+)$")
 PATTERN_IPV6 = re.compile(r"^\[([0-9a-z:]+)\]:(\d+)$")

doc/release-notes-13756.md

@@ -7,8 +7,8 @@ A new wallet flag `avoid_reuse` has been added (default off). When enabled,
 a wallet will distinguish between used and unused addresses, and default to not
 use the former in coin selection.
 
-(Note: rescanning the blockchain is required, to correctly mark previously
-used destinations.)
+Rescanning the blockchain is required, to correctly mark previously
+used destinations.
 
 Together with "avoid partial spends" (present as of Bitcoin v0.17), this
 addresses a serious privacy issue where a malicious user can track spends by
@@ -30,10 +30,12 @@ These include:
 
 - createwallet
 - getbalance
+- getbalances
 - sendtoaddress
 
-In addition, `sendtoaddress` has been changed to enable `-avoidpartialspends` when
-`avoid_reuse` is enabled.
+In addition, `sendtoaddress` has been changed to avoid partial spends when `avoid_reuse`
+is enabled (if not already enabled via the  `-avoidpartialspends` command line flag),
+as it would otherwise risk using up the "wrong" UTXO for an address reuse case.
 
 The listunspent RPC has also been updated to now include a "reused" bool, for nodes
 with "avoid_reuse" enabled.

src/Makefile.am

@@ -750,12 +750,9 @@ if TARGET_WINDOWS
 dashd_SOURCES += dashd-res.rc
 endif
 
-# Libraries below may be listed more than once to resolve circular dependencies (see
-# https://eli.thegreenplace.net/2013/07/09/library-order-in-static-linking#circular-dependency)
 dashd_LDADD = \
   $(LIBBITCOIN_SERVER) \
   $(LIBBITCOIN_WALLET) \
-  $(LIBBITCOIN_SERVER) \
   $(LIBBITCOIN_COMMON) \
   $(LIBUNIVALUE) \
   $(LIBBITCOIN_UTIL) \

src/net_processing.cpp

@@ -80,6 +80,8 @@ static const unsigned int MAX_GETDATA_SZ = 1000;
 static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60;
 /** Minimum time between orphan transactions expire time checks in seconds */
 static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60;
+/** How long to cache transactions in mapRelay for normal relay */
+static constexpr std::chrono::seconds RELAY_TX_CACHE_TIME{15 * 60};
 /** Headers download timeout expressed in microseconds
  *  Timeout = base + per_header * (expected number of headers) */
 static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_BASE = 15 * 60 * 1000000; // 15 minutes
@@ -1179,7 +1181,7 @@ static bool TxRelayMayResultInDisconnect(const CValidationState& state)
 /**
  * Potentially mark a node discouraged based on the contents of a CValidationState object
  *
- * @param[in] via_compact_block: this bool is passed in because net_processing should
+ * @param[in] via_compact_block this bool is passed in because net_processing should
  * punish peers differently depending on whether the data was provided in a compact
  * block message or not. If the compact block had a valid header, but contained invalid
  * txs, the peer should not be punished. See BIP 152.
@@ -1814,6 +1816,10 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
     const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
 
     {
+        // mempool entries added before this time have likely expired from mapRelay
+        const std::chrono::seconds longlived_mempool_time = GetTime<std::chrono::seconds>() - RELAY_TX_CACHE_TIME;
+        const std::chrono::seconds mempool_req = pfrom->m_tx_relay->m_last_mempool_req.load();
+
         LOCK(cs_main);
 
         while (it != pfrom->vRecvGetData.end() && it->IsKnownType()) {
@@ -1851,11 +1857,15 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
                         connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *mi->second));
                     }
                     push = true;
-                } else if (pfrom->m_tx_relay->m_last_mempool_req.load().count()) {
+                } else {
                     auto txinfo = mempool.info(inv.hash);
                     // To protect privacy, do not answer getdata using the mempool when
-                    // that TX couldn't have been INVed in reply to a MEMPOOL request.
-                    if (txinfo.tx && txinfo.m_time <= pfrom->m_tx_relay->m_last_mempool_req.load()) {
+                    // that TX couldn't have been INVed in reply to a MEMPOOL request,
+                    // or when it's too recent to have expired from mapRelay.
+                    if (txinfo.tx && (
+                         (mempool_req.count() && txinfo.m_time <= mempool_req)
+                          || (txinfo.m_time <= longlived_mempool_time)))
+                    {
                         if (dstx) {
                             connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::DSTX, dstx));
                         } else {
@@ -4897,7 +4907,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
 
                             auto ret = mapRelay.insert(std::make_pair(hash, std::move(txinfo.tx)));
                             if (ret.second) {
-                                vRelayExpiration.push_back(std::make_pair(nNow + 15 * 60 * 1000000, ret.first));
+                                vRelayExpiration.push_back(std::make_pair(nNow + std::chrono::microseconds{RELAY_TX_CACHE_TIME}.count(), ret.first));
                             }
                         }
                         int nInvType = CCoinJoin::GetDSTX(hash) ? MSG_DSTX : MSG_TX;

src/netbase.cpp

@@ -782,7 +782,7 @@ bool IsProxy(const CNetAddr &addr) {
  * @param hSocket The socket on which to connect to the SOCKS5 proxy.
  * @param nTimeout Wait this many milliseconds for the connection to the SOCKS5
  *                 proxy to be established.
- * @param outProxyConnectionFailed[out] Whether or not the connection to the
+ * @param[out] outProxyConnectionFailed Whether or not the connection to the
  *                                      SOCKS5 proxy failed.
  *
  * @returns Whether or not the operation succeeded.

src/node/transaction.h

@@ -22,10 +22,10 @@ struct NodeContext;
  *
  * @param[in]  node reference to node context
  * @param[in]  tx the transaction to broadcast
- * @param[out] &err_string reference to std::string to fill with error string if available
+ * @param[out] err_string reference to std::string to fill with error string if available
  * @param[in]  max_tx_fee reject txs with fees higher than this (if 0, accept any fee)
  * @param[in]  relay flag if both mempool insertion and p2p relay are requested
- * @param[in]  wait_callback, wait until callbacks have been processed to avoid stale result due to a sequentially RPC.
+ * @param[in]  wait_callback wait until callbacks have been processed to avoid stale result due to a sequentially RPC.
  * return error
  */
 [[nodiscard]] TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, std::string& err_string, const CAmount& highfee, bool relay, bool wait_callback, bool bypass_limits = false);

src/psbt.h

@@ -533,15 +533,15 @@ void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransactio
 /**
  * Finalizes a PSBT if possible, combining partial signatures.
  *
- * @param[in,out] &psbtx reference to PartiallySignedTransaction to finalize
+ * @param[in,out] psbtx PartiallySignedTransaction to finalize
  * return True if the PSBT is now complete, false otherwise
  */
 bool FinalizePSBT(PartiallySignedTransaction& psbtx);
 
 /**
  * Finalizes a PSBT if possible, and extracts it to a CMutableTransaction if it could be finalized.
  *
- * @param[in]  &psbtx reference to PartiallySignedTransaction
+ * @param[in]  psbtx PartiallySignedTransaction
  * @param[out] result CMutableTransaction representing the complete transaction, if successful
  * @return True if we successfully extracted the transaction, false otherwise
  */
@@ -550,7 +550,7 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti
 /**
  * Combines PSBTs with the same underlying transaction, resulting in a single PSBT with all partial signatures from each input.
  *
- * @param[out] &out   the combined PSBT, if successful
+ * @param[out] out   the combined PSBT, if successful
  * @param[in]  psbtxs the PSBTs to combine
  * @return error (OK if we successfully combined the transactions, other error if they were not compatible)
  */

src/rpc/rawtransaction_util.h

@@ -19,6 +19,7 @@ class SigningProvider;
  * Sign a transaction with the given keystore and previous transactions
  *
  * @param  mtx           The transaction to-be-signed
+ * @param  prevTxsUnival Array of previous txns outputs that tx depends on but may not yet be in the block chain
  * @param  keystore      Temporary keystore containing signing keys
  * @param  coins         Map of unspent outputs
  * @param  hashType      The signature hash type

src/script/descriptor.h

@@ -90,20 +90,20 @@ struct Descriptor {
 
     /** Expand a descriptor at a specified position.
      *
-     * @param[in] pos: The position at which to expand the descriptor. If IsRange() is false, this is ignored.
-     * @param[in] provider: The provider to query for private keys in case of hardened derivation.
-     * @param[out] output_scripts: The expanded scriptPubKeys.
-     * @param[out] out: Scripts and public keys necessary for solving the expanded scriptPubKeys (may be equal to `provider`).
-     * @param[out] write_cache: Cache data necessary to evaluate the descriptor at this point without access to private keys.
+     * @param[in] pos The position at which to expand the descriptor. If IsRange() is false, this is ignored.
+     * @param[in] provider The provider to query for private keys in case of hardened derivation.
+     * @param[out] output_scripts The expanded scriptPubKeys.
+     * @param[out] out Scripts and public keys necessary for solving the expanded scriptPubKeys (may be equal to `provider`).
+     * @param[out] write_cache Cache data necessary to evaluate the descriptor at this point without access to private keys.
      */
     virtual bool Expand(int pos, const SigningProvider& provider, std::vector<CScript>& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache = nullptr) const = 0;
 
     /** Expand a descriptor at a specified position using cached expansion data.
      *
-     * @param[in] pos: The position at which to expand the descriptor. If IsRange() is false, this is ignored.
-     * @param[in] read_cache: Cached expansion data.
-     * @param[out] output_scripts: The expanded scriptPubKeys.
-     * @param[out] out: Scripts and public keys necessary for solving the expanded scriptPubKeys (may be equal to `provider`).
+     * @param[in] pos The position at which to expand the descriptor. If IsRange() is false, this is ignored.
+     * @param[in] read_cache Cached expansion data.
+     * @param[out] output_scripts The expanded scriptPubKeys.
+     * @param[out] out Scripts and public keys necessary for solving the expanded scriptPubKeys (may be equal to `provider`).
      */
     virtual bool ExpandFromCache(int pos, const DescriptorCache& read_cache, std::vector<CScript>& output_scripts, FlatSigningProvider& out) const = 0;
 

src/test/crypto_tests.cpp

@@ -190,14 +190,15 @@ static void TestHKDF_SHA256_32(const std::string &ikm_hex, const std::string &sa
     BOOST_CHECK(HexStr(out) == okm_check_hex);
 }
 
-static std::string LongTestString() {
+static std::string LongTestString()
+{
     std::string ret;
-    for (int i=0; i<200000; i++) {
-        ret += (unsigned char)(i);
-        ret += (unsigned char)(i >> 4);
-        ret += (unsigned char)(i >> 8);
-        ret += (unsigned char)(i >> 12);
-        ret += (unsigned char)(i >> 16);
+    for (int i = 0; i < 200000; i++) {
+        ret += (char)(i);
+        ret += (char)(i >> 4);
+        ret += (char)(i >> 8);
+        ret += (char)(i >> 12);
+        ret += (char)(i >> 16);
     }
     return ret;
 }

src/test/transaction_tests.cpp

@@ -451,6 +451,30 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
     reason.clear();
     BOOST_CHECK(!IsStandardTx(CTransaction(t), reason));
     BOOST_CHECK_EQUAL(reason, "multi-op-return");
+
+    // Check large scriptSig (non-standard if size is >1650 bytes)
+    t.vout.resize(1);
+    t.vout[0].nValue = MAX_MONEY;
+    t.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID());
+    // OP_PUSHDATA2 with len (3 bytes) + data (1647 bytes) = 1650 bytes
+    t.vin[0].scriptSig = CScript() << std::vector<unsigned char>(1647, 0); // 1650
+    BOOST_CHECK(IsStandardTx(CTransaction(t), reason));
+
+    t.vin[0].scriptSig = CScript() << std::vector<unsigned char>(1648, 0); // 1651
+    reason.clear();
+    BOOST_CHECK(!IsStandardTx(CTransaction(t), reason));
+    BOOST_CHECK_EQUAL(reason, "scriptsig-size");
+
+    // Check bare multisig (standard if policy flag fIsBareMultisigStd is set)
+    fIsBareMultisigStd = true;
+    t.vout[0].scriptPubKey = GetScriptForMultisig(1, {key.GetPubKey()}); // simple 1-of-1
+    t.vin[0].scriptSig = CScript() << std::vector<unsigned char>(65, 0);
+    BOOST_CHECK(IsStandardTx(CTransaction(t), reason));
+
+    fIsBareMultisigStd = false;
+    reason.clear();
+    BOOST_CHECK(!IsStandardTx(CTransaction(t), reason));
+    BOOST_CHECK_EQUAL(reason, "bare-multisig");
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/wallet/psbtwallet.h

@@ -15,8 +15,8 @@
  * finalize.) Sets `error` and returns false if something goes wrong.
  *
  * @param[in]  pwallet pointer to a wallet
- * @param[in]  &psbtx reference to PartiallySignedTransaction to fill in
- * @param[out] &complete indicates whether the PSBT is now complete
+ * @param[in]  psbtx PartiallySignedTransaction to fill in
+ * @param[out] complete indicates whether the PSBT is now complete
  * @param[in]  sighash_type the sighash type to use when signing (if PSBT does not specify)
  * @param[in]  sign whether to sign or not
  * @param[in]  bip32derivs whether to fill in bip32 derivation information if available