From 7d687f916dafed5e716411ef1d44a97f4256ba37 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 4 Dec 2019 13:22:23 -0500 Subject: [PATCH 01/13] Partial Merge #17517: ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 ci: Remove unparseable lines from supp file for old xenial clang tsan (MarcoFalke) fa1bfc476c9208a4c412c8ca74d05f52bb47766f ci: ubsan report_error_type=1 and add suppressions (MarcoFalke) fa69cef13e5aab8264339eb3d50a9e89d59efd87 test: Print stderr when subprocess fails (MarcoFalke) 2222c305866a77065ab5be24c1c252bae252bb59 test: Use char instead of unsigned char (MarcoFalke) faa8023ce9a47b282e1fac3ca8b3a7bb0042935a ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le (MarcoFalke) Pull request description: Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running the ci system on ppc64le ACKs for top commit: practicalswift: ACK fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 assuming Travis is happy -- diff looks correct :) Tree-SHA512: f4f26232d3a0ef38da245869340f723d279a3db9823befbc735fb5a00024dae041c7306d7ae55d2488e6f86aa96cdea155b007aefb561fba505141e8dbc717dc --- ci/test/00_setup_env.sh | 9 ++-- ci/test/00_setup_env_native_tsan.sh | 4 ++ ci/test/04_install.sh | 2 +- src/test/crypto_tests.cpp | 15 +++---- .../test_framework/test_framework.py | 4 ++ test/sanitizer_suppressions/ubsan | 42 +++++++++++++++++++ 6 files changed, 65 insertions(+), 11 deletions(-) diff --git a/ci/test/00_setup_env.sh b/ci/test/00_setup_env.sh index 179a8fec6232..2fb73c291f5c 100755 --- a/ci/test/00_setup_env.sh +++ b/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)" diff --git a/ci/test/00_setup_env_native_tsan.sh b/ci/test/00_setup_env_native_tsan.sh index d25deea4c7bb..87ac6100ab78 100755 --- a/ci/test/00_setup_env_native_tsan.sh +++ b/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" diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index f81aabfa5152..1ba68025593c 100755 --- a/ci/test/04_install.sh +++ b/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" diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp index 9b0c213a15e4..4806c83571d2 100644 --- a/src/test/crypto_tests.cpp +++ b/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; } diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 6d0cf81e3008..60cd28afa167 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -14,6 +14,7 @@ import pdb import random import shutil +import subprocess import sys import tempfile import time @@ -152,6 +153,9 @@ def main(self): except KeyError: self.log.exception("Key error") self.success = TestStatus.FAILED + except subprocess.CalledProcessError as e: + self.log.exception("Called Process failed with '{}'".format(e.output)) + self.success = TestStatus.FAILED except Exception: self.log.exception("Unexpected exception caught during testing") self.success = TestStatus.FAILED diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index d4886076d0e5..345a86687727 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -41,3 +41,45 @@ unsigned-integer-overflow:txmempool.cpp unsigned-integer-overflow:util/strencodings.cpp unsigned-integer-overflow:validation.cpp vptr:bls/bls.h + +implicit-integer-sign-change:*/include/c++/*/bits/*.h +implicit-integer-sign-change:*/new_allocator.h +implicit-integer-sign-change:/usr/include/boost/date_time/format_date_parser.hpp +implicit-integer-sign-change:arith_uint256.cpp +implicit-integer-sign-change:bech32.cpp +implicit-integer-sign-change:bloom.cpp +implicit-integer-sign-change:chain.* +implicit-integer-sign-change:coins.h +implicit-integer-sign-change:compat/stdin.cpp +implicit-integer-sign-change:compressor.h +implicit-integer-sign-change:crypto/* +implicit-integer-sign-change:key.cpp +implicit-integer-sign-change:noui.cpp +implicit-integer-sign-change:prevector.h +implicit-integer-sign-change:protocol.cpp +implicit-integer-sign-change:script/bitcoinconsensus.cpp +implicit-integer-sign-change:script/interpreter.cpp +implicit-integer-sign-change:serialize.h +implicit-integer-sign-change:test/arith_uint256_tests.cpp +implicit-integer-sign-change:test/coins_tests.cpp +implicit-integer-sign-change:test/pow_tests.cpp +implicit-integer-sign-change:test/prevector_tests.cpp +implicit-integer-sign-change:test/sighash_tests.cpp +implicit-integer-sign-change:test/streams_tests.cpp +implicit-integer-sign-change:test/transaction_tests.cpp +implicit-integer-sign-change:txmempool.cpp +implicit-integer-sign-change:util/strencodings.* +implicit-integer-sign-change:validation.cpp +implicit-integer-sign-change:zmq/zmqpublishnotifier.cpp +implicit-signed-integer-truncation,implicit-integer-sign-change:chain.h +implicit-signed-integer-truncation,implicit-integer-sign-change:test/skiplist_tests.cpp +implicit-signed-integer-truncation:chain.h +implicit-signed-integer-truncation:crypto/* +implicit-signed-integer-truncation:cuckoocache.h +implicit-signed-integer-truncation:leveldb/* +implicit-signed-integer-truncation:streams.h +implicit-signed-integer-truncation:test/arith_uint256_tests.cpp +implicit-signed-integer-truncation:test/skiplist_tests.cpp +implicit-signed-integer-truncation:torcontrol.cpp +implicit-unsigned-integer-truncation:crypto/* +implicit-unsigned-integer-truncation:leveldb/* From f181b0284a4e8c07b4b204941db84f806fb38311 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 24 Oct 2019 17:50:38 -0400 Subject: [PATCH 02/13] Merge #16851: Continue relaying transactions after they expire from mapRelay 168b781fe7f3f13b24c52a151f36de4cdd0a340a Continue relaying transactions after they expire from mapRelay (Anthony Towns) Pull request description: This change allows peers to request transactions even after they've expired from mapRelay and even if they're not doing mempool requests. This is intended to allow for CPFP of old transactions -- if parent tx P wasn't relayed due to low fees, then a higher fee rate child C is relayed, peers will currently request the parent P, but we prior to this patch, we will not relay it due to it not being in mapRelay. ACKs for top commit: MarcoFalke: re-ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a (only change is comment fixup) sdaftuar: re-ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a sipa: ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a Tree-SHA512: b206666dd1450cd0a161ae55fd1a7eda2c3d226842ba27d91fe463b551fd924b65b92551b14d6786692e15cf9a9a989666550dfc980b48ab0f8d4ca305bc7762 --- src/net_processing.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index eb500f9b3846..b3f1ecff4501 100644 --- a/src/net_processing.cpp +++ b/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 @@ -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() - 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; From 6fb0941ca74c25a7f50d9b507f26d2ed0be10cf1 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 13 Jan 2020 11:47:26 +0100 Subject: [PATCH 03/13] Merge #17910: build: remove double LIBBITCOIN_SERVER linking 831e1220bc151b1016412359775406b34cb8f52c build: remove double LIBBITCOIN_SERVER linking (fanquake) Pull request description: Seems that this is no longer required. Have tested building on macOS and Debian. ACKs for top commit: promag: ACK 831e1220bc151b1016412359775406b34cb8f52c. practicalswift: ACK 831e1220bc151b1016412359775406b34cb8f52c laanwj: ACK 831e1220bc151b1016412359775406b34cb8f52c Tree-SHA512: d226d9fa0292189fae7e2af14781a511c3633f1352324f19ae642e941d06c34e2abf8b1df97d2330d76dba6024a93d8d341e02cc4882d7066f97e82585631fe1 --- src/Makefile.am | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index c0b852313d3a..36e66cc3e023 100644 --- a/src/Makefile.am +++ b/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) \ From 038eae1b4b928429a1a4b88c007a270ef48e2c0a Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 16 Jan 2020 15:41:32 -0500 Subject: [PATCH 04/13] Merge #17541: test: add functional test for non-standard bare multisig txs 1be0b1fb2adcf95d76f879195564c0bf84162e31 test: add functional test for non-standard bare multisig txs (Sebastian Falbesoner) Pull request description: Approaches another missing functional test of issue #17394 (counterpart to unit test in PR #17502): A transaction is rejected by the mempool with reason `"bare-multisig"` if any of the outputs' scriptPubKey has bare multisig format (`M ... N OP_CHECKSIG`) and bitcoind is started with the argument `-permitbaremultisig=0`. ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/17541/commits/1be0b1fb2adcf95d76f879195564c0bf84162e31 kristapsk: ACK 1be0b1fb2adcf95d76f879195564c0bf84162e31 Tree-SHA512: 2cade68c4454029b62278b38d0f137c2605a0e4450c435cdda2833667234edd4406f017ed12fa8df9730618654acbaeb68b16dcabb9f5aa84bad9f1c76c6d476 --- test/functional/mempool_accept.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 132b4fc9ef02..b170d95fef2f 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -8,6 +8,7 @@ import math from test_framework.test_framework import BitcoinTestFramework +from test_framework.key import ECKey from test_framework.messages import ( BIP125_SEQUENCE_NUMBER, COIN, @@ -21,6 +22,9 @@ hash160, CScript, OP_0, + OP_2, + OP_3, + OP_CHECKMULTISIG, OP_EQUAL, OP_HASH160, OP_RETURN, @@ -36,7 +40,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.extra_args = [[ - '-txindex', + '-txindex','-permitbaremultisig=0', ]] * self.num_nodes self.supports_cli = False @@ -263,6 +267,15 @@ def run_test(self): rawtxs=[tx.serialize().hex()], ) tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference))) + key = ECKey() + key.generate() + pubkey = key.get_pubkey().get_bytes() + tx.vout[0].scriptPubKey = CScript([OP_2, pubkey, pubkey, pubkey, OP_3, OP_CHECKMULTISIG]) # Some bare multisig script (2-of-3) + self.check_mempool_result( + result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: bare-multisig'}], + rawtxs=[tx.serialize().hex()], + ) + tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference))) tx.vin[0].scriptSig = CScript([OP_HASH160]) # Some not-pushonly scriptSig self.check_mempool_result( result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: scriptsig-not-pushonly'}], From 8c6fb5622de7fda3e039fd47a3f8e4f7607f6217 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 20 Jan 2020 20:24:27 +0100 Subject: [PATCH 05/13] Merge #17823: scripts: Read suspicious hosts from a file instead of hardcoding e1c582cbaa4c094d204da34c3b1fdd0d4c557519 contrib: makeseeds: Read suspicious hosts from a file instead of hardcoding (Sanjay K) Pull request description: referring to: https://github.com/bitcoin/bitcoin/issues/17020 good first issue: reading SUSPICIOUS_HOSTS from a file. I haven't changed the base hosts that were included in the original source, just made it readable from a file. ACKs for top commit: practicalswift: ACK e1c582cbaa4c094d204da34c3b1fdd0d4c557519 -- diff looks correct Tree-SHA512: 18684abc1c02cf52d63f6f6ecd98df01a9574a7c470524c37e152296504e2e3ffbabd6f3208214b62031512aeb809a6d37446af82c9f480ff14ce4c42c98e7c2 --- contrib/seeds/makeseeds.py | 5 +++-- contrib/seeds/suspicious_hosts.txt | 0 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 contrib/seeds/suspicious_hosts.txt diff --git a/contrib/seeds/makeseeds.py b/contrib/seeds/makeseeds.py index 79a243ba5253..e1aef34e85b2 100755 --- a/contrib/seeds/makeseeds.py +++ b/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+)$") diff --git a/contrib/seeds/suspicious_hosts.txt b/contrib/seeds/suspicious_hosts.txt new file mode 100644 index 000000000000..e69de29bb2d1 From f94a833add3b677d734b5643447ecf1582be37af Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 20 Jan 2020 20:35:12 +0100 Subject: [PATCH 06/13] Merge #17945: doc: Fix doxygen errors 297e09855793feb94c3229ed989bef8b1eac864e Fix doxygen errors (Ben Woosley) Pull request description: These are all the remaining errors identified via -Werror=documentation, e.g.: ``` ./rpc/rawtransaction_util.h:31:13: error: parameter 'prevTxs' not found in the function declaration [-Werror,-Wdocumentation] * @param prevTxs Array of previous txns outputs that tx depends on but may not yet be in the block chain ^~~~~~~ ./rpc/rawtransaction_util.h:31:13: note: did you mean 'prevTxsUnival'? * @param prevTxs Array of previous txns outputs that tx depends on but may not yet be in the block chain ^~~~~~~ prevTxsUnival netbase.cpp:766:11: error: parameter 'outProxyConnectionFailed[out]' not found in the function declaration [-Werror,-Wdocumentation] * @param outProxyConnectionFailed[out] Whether or not the connection to the ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ netbase.cpp:766:11: note: did you mean 'outProxyConnectionFailed'? * @param outProxyConnectionFailed[out] Whether or not the connection to the ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ outProxyConnectionFailed ``` You can use this to run with `-Wdocumentation` yourself: #14920 ACKs for top commit: laanwj: ACK 297e09855793feb94c3229ed989bef8b1eac864e Tree-SHA512: a232d893b170873d923e77fa56c56a6567e7fd120b5af1f52cfeeae1093eec55621604cc80a523678f6fedc8bbb31228c4aa8dc2a630ce9ffc91525988522af7 --- src/net_processing.cpp | 2 +- src/netbase.cpp | 2 +- src/node/transaction.h | 4 ++-- src/psbt.h | 6 +++--- src/rpc/rawtransaction_util.h | 1 + src/script/descriptor.h | 18 +++++++++--------- src/wallet/psbtwallet.h | 4 ++-- src/wallet/wallet.h | 6 +++--- src/wallet/walletdb.h | 2 +- 9 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b3f1ecff4501..3ee520b95475 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1181,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. diff --git a/src/netbase.cpp b/src/netbase.cpp index 1fe29a8e821f..73c15643eeab 100644 --- a/src/netbase.cpp +++ b/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. diff --git a/src/node/transaction.h b/src/node/transaction.h index d1ab741823c0..5ecb69518bce 100644 --- a/src/node/transaction.h +++ b/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); diff --git a/src/psbt.h b/src/psbt.h index 6017ddd61152..fbd066f1fd5d 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -533,7 +533,7 @@ 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); @@ -541,7 +541,7 @@ 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) */ diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index 2c37ab7b4999..7e1f41d2fd1f 100644 --- a/src/rpc/rawtransaction_util.h +++ b/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 diff --git a/src/script/descriptor.h b/src/script/descriptor.h index 34143b525b83..155e72792a9e 100644 --- a/src/script/descriptor.h +++ b/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& 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& output_scripts, FlatSigningProvider& out) const = 0; diff --git a/src/wallet/psbtwallet.h b/src/wallet/psbtwallet.h index b20b15d05ec5..bcdd700bce3f 100644 --- a/src/wallet/psbtwallet.h +++ b/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 diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6d151df5dfb4..788b80ebd3b2 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1020,9 +1020,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati * Should be called after CreateTransaction unless you want to abort * broadcasting the transaction. * - * @param tx[in] The transaction to be broadcast. - * @param mapValue[in] key-values to be set on the transaction. - * @param orderForm[in] BIP 70 / BIP 21 order form details to be set on the transaction. + * @param[in] tx The transaction to be broadcast. + * @param[in] mapValue key-values to be set on the transaction. + * @param[in] orderForm BIP 70 / BIP 21 order form details to be set on the transaction. */ void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index d09932a055ad..5c37590342dd 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -92,7 +92,7 @@ class CKeyMetadata int nVersion; int64_t nCreateTime; // 0 means unknown KeyOriginInfo key_origin; // Key origin info with path and fingerprint - bool has_key_origin = false; //< Whether the key_origin is useful + bool has_key_origin = false; //!< Whether the key_origin is useful CKeyMetadata() { From ed70a2fe78cc34215f1f3688b497e9a2e45af5a2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 15 Nov 2019 14:03:54 -0500 Subject: [PATCH 07/13] Merge #17480: test: add unit test for non-standard txs with too large scriptSig 5e8a56348b5e1026e9ddcae0b2fa2a68faf4439e test: add unit test for non-standard txs with too large scriptSig (Sebastian Falbesoner) Pull request description: Approaches the first missing test of issue #17394: Checks that the function `IsStandardTx()` returns rejection reason `"scriptsig-size"` if any one the inputs' scriptSig is larger than 1650 bytes. ACKs for top commit: MarcoFalke: ACK 5e8a56348b5e1026e9ddcae0b2fa2a68faf4439e instagibbs: ACK https://github.com/bitcoin/bitcoin/commit/5e8a56348b5e1026e9ddcae0b2fa2a68faf4439e Tree-SHA512: 79977b12ddea9438a37cefdbb48cc551e4ad02a8ccfaa2d2837ced9f3a185e2e07cc366c243b9e3c7736245e90e315d7b4110efc6b440c63dbef7ee2c9d78a73 --- src/test/transaction_tests.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 2ce49c61e20a..69fe888bb069 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -451,6 +451,19 @@ 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(1647, 0); // 1650 + BOOST_CHECK(IsStandardTx(CTransaction(t), reason)); + + t.vin[0].scriptSig = CScript() << std::vector(1648, 0); // 1651 + reason.clear(); + BOOST_CHECK(!IsStandardTx(CTransaction(t), reason)); + BOOST_CHECK_EQUAL(reason, "scriptsig-size"); } BOOST_AUTO_TEST_SUITE_END() From 10d3849271e30bfee65e6c21d829781820d41d2c Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 20 Nov 2019 11:14:37 -0500 Subject: [PATCH 08/13] Merge #17532: test: add functional test for non-standard txs with too large scriptSig 8f2d7737cc236b6122f30e31856eb3181960fba1 test: add functional test for non-standard txs with too large scriptSig (Sebastian Falbesoner) Pull request description: Approaches another missing functional test of issue #17394 (counterpart to unit test in PR #17480, Commit https://github.com/bitcoin/bitcoin/commit/5e8a56348b5e1026e9ddcae0b2fa2a68faf4439e): A transaction is rejected by the mempool with reason `"scriptsig-size"` if any of the inputs' scriptSig is larger than 1650 bytes. ACKs for top commit: MarcoFalke: ACK 8f2d7737cc236b6122f30e31856eb3181960fba1 instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/17532/commits/8f2d7737cc236b6122f30e31856eb3181960fba1 Tree-SHA512: 7a45b8a4181158be3e3b91756783ddf032f132ca8780dc35fac91b2df2149268f784d28ac56005135c4d86a357c57805c5a54b8155f0d049932844b18dc03992 --- test/functional/mempool_accept.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index b170d95fef2f..95aa7b16b25f 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -288,6 +288,12 @@ def run_test(self): rawtxs=[tx.serialize().hex()], ) tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference))) + tx.vin[0].scriptSig = CScript([b'a' * 1648]) # Some too large scriptSig (>1650 bytes) + self.check_mempool_result( + result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: scriptsig-size'}], + rawtxs=[tx.serialize().hex()], + ) + tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference))) output_p2sh_burn = CTxOut(nValue=540, scriptPubKey=CScript([OP_HASH160, hash160(b'burn'), OP_EQUAL])) num_scripts = 100000 // len(output_p2sh_burn.serialize()) # Use enough outputs to make the tx too large for our policy tx.vout = [output_p2sh_burn] * num_scripts From 44fec4395abb09924b3791ae8cca6c8c4f66a202 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 10 Dec 2019 12:49:29 -0500 Subject: [PATCH 09/13] Merge #17502: test: add unit test for non-standard bare multisig txs 1bb5d517aa616c1d5b5801d2ea36a2de5fb61eba test: add unit test for non-standard bare multisig txs (Sebastian Falbesoner) Pull request description: Approaches another missing unit test of issue #17394: Checks that the function `IsStandardTx()` returns rejection reason `"bare-multisig"` if any one of the outputs' scriptPubKey has bare multisignature format (i.e. `M ... N OP_CHECKSIG`, not P2SH!) and the policy flag `fIsBareMultisigStd` is set to false. ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/17502/commits/1bb5d517aa616c1d5b5801d2ea36a2de5fb61eba Tree-SHA512: d7c95e35da16520d6dcd2b4278e2426fedd13f68d1f23c90e85e929774e123fbfcfbccc26df6ad1c0dd61780896fa4b4b3d4e8280c647bb06df2bfcf2ba572fb --- src/test/transaction_tests.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 69fe888bb069..74074329c221 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -464,6 +464,17 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) 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(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() From 3b5b8d0a941a56e57ea517a937e3344b3b2172b9 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Wed, 15 Jan 2020 22:10:39 +1300 Subject: [PATCH 10/13] Merge #17843: wallet: Reset reused transactions cache 6fc554f591d8ea1681b8bb25aa12da8d4f023f66 wallet: Reset reused transactions cache (Fabian Jahr) Pull request description: Fixes #17603 (together with #17824) `getbalances` is using the cache within `GetAvailableCredit` under certain conditions [here](https://github.com/bitcoin/bitcoin/blob/35fff5be60e853455abc24713481544e91adfedb/src/wallet/wallet.cpp#L1826). For a wallet with `avoid_reuse` activated this can lead to inconsistent reporting of `used` transactions/balances between `getbalances` and `listunspent` as pointed out in #17603. When an address is reused before the first transaction is spending from this address, the cache is not updated even after the transaction is sent. This means the remaining outputs at the reused address are not showing up as `used` in `getbalances`. With this change, any newly incoming transaction belonging to the wallet marks all the other outputs at the same address as dirty. ACKs for top commit: kallewoof: Code review re-ACK 6fc554f591d8ea1681b8bb25aa12da8d4f023f66 promag: ACK 6fc554f591d8ea1681b8bb25aa12da8d4f023f66. achow101: Re-ACK 6fc554f591d8ea1681b8bb25aa12da8d4f023f66 meshcollider: Code review ACK 6fc554f591d8ea1681b8bb25aa12da8d4f023f66 Tree-SHA512: c4cad2c752176d16d77b4a4202291d20baddf9f27250896a40274d74a6945e0f6b34be04c2f9b1b2e756d3ac669b794969df8f82a98e0b16f10e92f276649ea2 --- src/wallet/wallet.cpp | 27 ++++++++++++++++++++--- src/wallet/wallet.h | 8 ++++++- test/functional/wallet_avoidreuse.py | 32 ++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0753bda0ec42..e58ea0e717ed 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -777,7 +777,7 @@ void CWallet::MarkDirty() fAnonymizableTallyCachedNonDenom = false; } -void CWallet::SetUsedDestinationState(const uint256& hash, unsigned int n, bool used) +void CWallet::SetUsedDestinationState(const uint256& hash, unsigned int n, bool used, std::set& tx_destinations) { const CWalletTx* srctx = GetWalletTx(hash); if (!srctx) return; @@ -787,7 +787,9 @@ void CWallet::SetUsedDestinationState(const uint256& hash, unsigned int n, bool if (IsMine(dst)) { LOCK(cs_wallet); if (used && !GetDestData(dst, "used", nullptr)) { - AddDestData(dst, "used", "p"); // p for "present", opposite of absent (null) + if (AddDestData(dst, "used", "p")) { // p for "present", opposite of absent (null) + tx_destinations.insert(dst); + } } else if (!used && GetDestData(dst, "used", nullptr)) { EraseDestData(dst, "used"); } @@ -818,10 +820,14 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) if (IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { // Mark used destinations + std::set tx_destinations; + for (const CTxIn& txin : wtxIn.tx->vin) { const COutPoint& op = txin.prevout; - SetUsedDestinationState(op.hash, op.n, true); + SetUsedDestinationState(op.hash, op.n, true, tx_destinations); } + + MarkDestinationsDirty(tx_destinations); } // Inserts only if not already there, returns tx inserted or tx found @@ -3785,6 +3791,21 @@ bool CWallet::GetNewChangeDestination(CTxDestination& dest, std::string& error) return true; } +void CWallet::MarkDestinationsDirty(const std::set& destinations) { + for (auto& entry : mapWallet) { + CWalletTx& wtx = entry.second; + + for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) { + CTxDestination dst; + + if (ExtractDestination(wtx.tx->vout[i].scriptPubKey, dst) && destinations.count(dst)) { + wtx.MarkDirty(); + break; + } + } + } +} + std::map CWallet::GetAddressBalances() { std::map balances; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 788b80ebd3b2..d3b681c12003 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -901,7 +901,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati // Whether this or any UTXO with the same CTxDestination has been spent. bool IsUsedDestination(const CTxDestination& dst) const; bool IsUsedDestination(const uint256& hash, unsigned int n) const; - void SetUsedDestinationState(const uint256& hash, unsigned int n, bool used); + void SetUsedDestinationState(const uint256& hash, unsigned int n, bool used, std::set& tx_destinations); std::vector GroupOutputs(const std::vector& outputs, bool single_coin) const; @@ -1066,6 +1066,12 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati std::set GetLabelAddresses(const std::string& label) const; + /** + * Marks all outputs in each one of the destinations dirty, so their cache is + * reset and does not return outdated information. + */ + void MarkDestinationsDirty(const std::set& destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool GetNewDestination(const std::string label, CTxDestination& dest, std::string& error); bool GetNewChangeDestination(CTxDestination& dest, std::string& error); diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 7359aabe4a63..f26f83db2434 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -86,6 +86,8 @@ def run_test(self): self.test_fund_send_fund_senddirty() reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) self.test_fund_send_fund_send() + reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) + self.test_getbalances_used() def test_persistence(self): '''Test that wallet files persist the avoid_reuse flag.''' @@ -221,5 +223,35 @@ def test_fund_send_fund_send(self): assert_approx(self.nodes[1].getbalance(), 1, 0.001) assert_approx(self.nodes[1].getbalance(avoid_reuse=False), 11, 0.001) + def test_getbalances_used(self): + ''' + getbalances and listunspent should pick up on reused addresses + immediately, even for address reusing outputs created before the first + transaction was spending from that address + ''' + self.log.info("Test getbalances used category") + + # node under test should be completely empty + assert_equal(self.nodes[1].getbalance(avoid_reuse=False), 0) + + new_addr = self.nodes[1].getnewaddress() + ret_addr = self.nodes[0].getnewaddress() + + # send multiple transactions, reusing one address + for _ in range(11): + self.nodes[0].sendtoaddress(new_addr, 1) + + self.nodes[0].generate(1) + self.sync_all() + + # send transaction that should not use all the available outputs + # per the current coin selection algorithm + self.nodes[1].sendtoaddress(ret_addr, 5) + + # getbalances and listunspent should show the remaining outputs + # in the reused address as used/reused + assert_unspent(self.nodes[1], total_count=2, total_sum=6, reused_count=1, reused_sum=1) + assert_balances(self.nodes[1], mine={"used": 1, "trusted": 5}) + if __name__ == '__main__': AvoidReuseTest().main() From ad68327f38b02475b8dba06ef0b9242a388422a1 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Fri, 17 Jan 2020 14:15:10 +1300 Subject: [PATCH 11/13] Merge #17889: wallet: Improve CWallet:MarkDestinationsDirty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 2b1641492fbf81e2c5a95f3e580811ca8700adc5 wallet: Improve CWallet:MarkDestinationsDirty (João Barbosa) Pull request description: Improve `CWallet:MarkDestinationsDirty` by skipping transactions that already have the cache invalidated. Skipping a transaction avoids at worst case extracting all output destinations. ACKs for top commit: meshcollider: re-utACK 2b1641492fbf81e2c5a95f3e580811ca8700adc5 Tree-SHA512: 479dc2dde4b653b856e3d6a0c59a34fe33e963eb131a2d88552a8b30471b8725a087888fe5d7db6e4ee19b74072fe64441497f033be7d1931637f756e0d8fef5 --- src/wallet/test/coinselector_tests.cpp | 1 + src/wallet/wallet.cpp | 5 +++-- src/wallet/wallet.h | 8 ++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index eabb4ba0b750..694d621b7352 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -73,6 +73,7 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa if (fIsFromMe) { wtx->m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1); + wtx->m_is_cache_empty = false; } COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); vCoins.push_back(output); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e58ea0e717ed..22168a110d08 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2035,6 +2035,7 @@ CAmount CWalletTx::GetCachableAmount(AmountType type, const isminefilter& filter auto& amount = m_amounts[type]; if (recalculate || !amount.m_cached[filter]) { amount.Set(filter, type == DEBIT ? pwallet->GetDebit(*tx, filter) : pwallet->GetCredit(*tx, filter)); + m_is_cache_empty = false; } return amount.m_value[filter]; } @@ -2112,6 +2113,7 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter if (allow_cache) { m_amounts[AVAILABLE_CREDIT].Set(filter, nCredit); + m_is_cache_empty = false; } return nCredit; @@ -3794,10 +3796,9 @@ bool CWallet::GetNewChangeDestination(CTxDestination& dest, std::string& error) void CWallet::MarkDestinationsDirty(const std::set& destinations) { for (auto& entry : mapWallet) { CWalletTx& wtx = entry.second; - + if (wtx.m_is_cache_empty) continue; for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) { CTxDestination dst; - if (ExtractDestination(wtx.tx->vout[i].scriptPubKey, dst) && destinations.count(dst)) { wtx.MarkDirty(); break; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d3b681c12003..c525da8a83f3 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -327,6 +327,13 @@ class CWalletTx enum AmountType { DEBIT, CREDIT, IMMATURE_CREDIT, AVAILABLE_CREDIT, ANON_CREDIT, DENOM_UCREDIT, DENOM_CREDIT, AMOUNTTYPE_ENUM_ELEMENTS }; CAmount GetCachableAmount(AmountType type, const isminefilter& filter, bool recalculate = false) const; mutable CachableAmount m_amounts[AMOUNTTYPE_ENUM_ELEMENTS]; + /** + * This flag is true if all m_amounts caches are empty. This is particularly + * useful in places where MarkDirty is conditionally called and the + * condition can be expensive and thus can be skipped if the flag is true. + * See MarkDestinationsDirty. + */ + mutable bool m_is_cache_empty{true}; mutable bool fChangeCached; mutable bool fInMempool; mutable CAmount nChangeCached; @@ -456,6 +463,7 @@ class CWalletTx m_amounts[IMMATURE_CREDIT].Reset(); m_amounts[AVAILABLE_CREDIT].Reset(); fChangeCached = false; + m_is_cache_empty = true; } void BindWallet(CWallet *pwalletIn) From 078094b2aabd809802b17817351b02c0f44c1447 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 6 May 2019 10:12:20 +0200 Subject: [PATCH 12/13] Merge #15930: rpc: Add balances RPC facfb4111d14a3b06c46690a2cca7ca91cea8a96 rpc: Deprecate getunconfirmedbalance and getwalletinfo balances (MarcoFalke) 999931cf8f167c7547f1015cdf05437a460c27f0 rpc: Add getbalances RPC (MarcoFalke) fad13e925e197163a942f3f0d1ba2c95a2b65a56 rpcwallet: Make helper methods const on CWallet (MarcoFalke) fad40ec9151248c6e8225e14980424f581d23e02 wallet: Use IsValidNumArgs in getwalletinfo rpc (MarcoFalke) Pull request description: This exposes the `CWallet::GetBalance()` struct over RPC. In the future, incorrectly named rpcs such as `getunconfirmedbalance` or rpcs redundant to this such as `getbalance` could be removed. ACKs for commit facfb4: jnewbery: utACK facfb4111d14a3b06c46690a2cca7ca91cea8a96 Tree-SHA512: 1f54fedce55df9a8ea82d2b6265354b39a956072621876ebaee2355aac0e23c7b64340c3279502415598c095858529e18b50789be956250aafda1cd3a8d948a5 --- src/wallet/rpcwallet.cpp | 79 +++++++++++++++++++++++++++---- src/wallet/rpcwallet.h | 2 +- test/functional/wallet_balance.py | 15 +++++- 3 files changed, 86 insertions(+), 10 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 2cd7bce98ac8..f6ad0ba7e2cc 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Copyright (c) 2014-2022 The Dash Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -20,10 +20,10 @@ #include #include #include // For MessageSign() -#include #include #include #include +#include #include #include #include @@ -123,7 +123,7 @@ std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& reques "Wallet file not specified (must request wallet RPC through /wallet/ uri-path)."); } -void EnsureWalletIsUnlocked(CWallet * const pwallet) +void EnsureWalletIsUnlocked(const CWallet* pwallet) { if (pwallet->IsLocked()) { throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Please enter the wallet passphrase with walletpassphrase first."); @@ -801,7 +801,7 @@ static UniValue getbalance(const JSONRPCRequest& request) static UniValue getunconfirmedbalance(const JSONRPCRequest &request) { RPCHelpMan{"getunconfirmedbalance", - "Returns the server's total unconfirmed balance\n", + "DEPRECATED\nIdentical to getbalances().mine.untrusted_pending\n", {}, RPCResult{RPCResult::Type::NUM, "", "The balance"}, RPCExamples{""}, @@ -2381,6 +2381,68 @@ static UniValue setcoinjoinamount(const JSONRPCRequest& request) return NullUniValue; } +static UniValue getbalances(const JSONRPCRequest& request) +{ + RPCHelpMan{"getbalances", + "Returns an object with all balances in " + CURRENCY_UNIT + ".\n", + {}, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::OBJ, "mine", "balances from outputs that the wallet can sign", + { + {RPCResult::Type::STR_AMOUNT, "trusted", " trusted balance (outputs created by the wallet or confirmed outputs)"}, + {RPCResult::Type::STR_AMOUNT, "untrusted_pending", " untrusted pending balance (outputs created by others that are in the mempool)"}, + {RPCResult::Type::STR_AMOUNT, "immature", " balance from immature coinbase outputs"}, + {RPCResult::Type::STR_AMOUNT, "coinjoin", " CoinJoin balance (outputs with enough rounds created by the wallet via mixing)"}, + }}, + {RPCResult::Type::OBJ, "watchonly", "watchonly balances (not present if wallet does not watch anything)", + { + {RPCResult::Type::STR_AMOUNT, "trusted", " trusted balance (outputs created by the wallet or confirmed outputs)"}, + {RPCResult::Type::STR_AMOUNT, "untrusted_pending", " untrusted pending balance (outputs created by others that are in the mempool)"}, + {RPCResult::Type::STR_AMOUNT, "immature", " balance from immature coinbase outputs"}, + }}, + }, + }, + RPCExamples{ + HelpExampleCli("getbalances", "") + + HelpExampleRpc("getbalances", "") + }, + }.Check(request); + + std::shared_ptr const rpc_wallet = GetWalletForJSONRPCRequest(request); + if (!rpc_wallet) return NullUniValue; + CWallet& wallet = *rpc_wallet; + + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + wallet.BlockUntilSyncedToCurrentChain(); + + LOCK(wallet.cs_wallet); + + UniValue obj(UniValue::VOBJ); + + const auto bal = wallet.GetBalance(); + UniValue balances{UniValue::VOBJ}; + { + UniValue balances_mine{UniValue::VOBJ}; + balances_mine.pushKV("trusted", ValueFromAmount(bal.m_mine_trusted)); + balances_mine.pushKV("untrusted_pending", ValueFromAmount(bal.m_mine_untrusted_pending)); + balances_mine.pushKV("immature", ValueFromAmount(bal.m_mine_immature)); + balances_mine.pushKV("coinjoin", ValueFromAmount(bal.m_anonymized)); + balances.pushKV("mine", balances_mine); + } + auto spk_man = wallet.GetLegacyScriptPubKeyMan(); + if (spk_man && spk_man->HaveWatchOnly()) { + UniValue balances_watchonly{UniValue::VOBJ}; + balances_watchonly.pushKV("trusted", ValueFromAmount(bal.m_watchonly_trusted)); + balances_watchonly.pushKV("untrusted_pending", ValueFromAmount(bal.m_watchonly_untrusted_pending)); + balances_watchonly.pushKV("immature", ValueFromAmount(bal.m_watchonly_immature)); + balances.pushKV("watchonly", balances_watchonly); + } + return balances; +} + static UniValue getwalletinfo(const JSONRPCRequest& request) { RPCHelpMan{"getwalletinfo", @@ -2391,10 +2453,10 @@ static UniValue getwalletinfo(const JSONRPCRequest& request) { {RPCResult::Type::STR, "walletname", "the wallet name"}, {RPCResult::Type::NUM, "walletversion", "the wallet version"}, - {RPCResult::Type::NUM, "balance", "the total confirmed balance of the wallet in " + CURRENCY_UNIT}, - {RPCResult::Type::NUM, "coinjoin_balance", "the CoinJoin balance in " + CURRENCY_UNIT}, - {RPCResult::Type::NUM, "unconfirmed_balance", "the total unconfirmed balance of the wallet in " + CURRENCY_UNIT}, - {RPCResult::Type::NUM, "immature_balance", "the total immature balance of the wallet in " + CURRENCY_UNIT}, + {RPCResult::Type::NUM, "balance", "DEPRECATED. Identical to getbalances().mine.trusted"}, + {RPCResult::Type::NUM, "coinjoin_balance", "DEPRECATED. Identical to getbalances().mine.coinjoin"}, + {RPCResult::Type::NUM, "unconfirmed_balance", "DEPRECATED. Identical to getbalances().mine.untrusted_pending"}, + {RPCResult::Type::NUM, "immature_balance", "DEPRECATED. Identical to getbalances().mine.immature"}, {RPCResult::Type::NUM, "txcount", "the total number of transactions in the wallet"}, {RPCResult::Type::NUM_TIME, "timefirstkey", "the " + UNIX_EPOCH_TIME + " of the oldest known key in the wallet"}, {RPCResult::Type::NUM_TIME, "keypoololdest", "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool"}, @@ -3985,6 +4047,7 @@ static const CRPCCommand commands[] = { "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf","addlocked"} }, { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly"} }, { "wallet", "getunconfirmedbalance", &getunconfirmedbalance, {} }, + { "wallet", "getbalances", &getbalances, {} }, { "wallet", "getwalletinfo", &getwalletinfo, {} }, { "wallet", "importaddress", &importaddress, {"address","label","rescan","p2sh"} }, { "wallet", "importelectrumwallet", &importelectrumwallet, {"filename", "index"} }, diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index 234937477186..6d66753aaee7 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -35,7 +35,7 @@ Span GetWalletRPCCommands(); */ std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& request); -void EnsureWalletIsUnlocked(CWallet *); +void EnsureWalletIsUnlocked(const CWallet*); WalletContext& EnsureWalletContext(const util::Ref& context); UniValue getaddressinfo(const JSONRPCRequest& request); diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 8aaa6853a631..b05fd0ba3272 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -62,14 +62,24 @@ def run_test(self): assert_equal(len(self.nodes[0].listunspent()), 0) assert_equal(len(self.nodes[1].listunspent()), 0) - self.log.info("Mining blocks ...") + self.log.info("Check that only node 0 is watching an address") + assert 'watchonly' in self.nodes[0].getbalances() + assert 'watchonly' not in self.nodes[1].getbalances() + self.log.info("Mining blocks ...") self.nodes[0].generate(1) self.sync_all() self.nodes[1].generate(1) self.nodes[1].generatetoaddress(101, ADDRESS_WATCHONLY) self.sync_all() + assert_equal(self.nodes[0].getbalances()['mine']['trusted'], 500) + assert_equal(self.nodes[0].getwalletinfo()['balance'], 500) + assert_equal(self.nodes[1].getbalances()['mine']['trusted'], 500) + + assert_equal(self.nodes[0].getbalances()['watchonly']['immature'], 50000) + assert 'watchonly' not in self.nodes[1].getbalances() + assert_equal(self.nodes[0].getbalance(), 500) assert_equal(self.nodes[1].getbalance(), 500) @@ -113,8 +123,11 @@ def test_balances(*, fee_node_1=0): assert_equal(self.nodes[1].getbalance(minconf=1), Decimal('0')) # getunconfirmedbalance assert_equal(self.nodes[0].getunconfirmedbalance(), Decimal('960')) # output of node 1's spend + assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], Decimal('960')) assert_equal(self.nodes[0].getwalletinfo()["unconfirmed_balance"], Decimal('960')) + assert_equal(self.nodes[1].getunconfirmedbalance(), Decimal('0')) # Doesn't include output of node 0's send since it was spent + assert_equal(self.nodes[1].getbalances()['mine']['untrusted_pending'], Decimal('0')) assert_equal(self.nodes[1].getwalletinfo()["unconfirmed_balance"], Decimal('0')) test_balances(fee_node_1=Decimal('0.01')) From ccfe4b76c6d9ea13d2441c41a9a748f59ddc3768 Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Sat, 22 Jun 2019 21:58:52 +1200 Subject: [PATCH 13/13] Merge #16239: wallet/rpc: follow-up clean-up/fixes to avoid_reuse 71d0344cf25d3aaf60112c5248198c444bc98105 docs: release note wording (Karl-Johan Alm) 3d2ff379131a01e4e9f9648b150e806104a23795 wallet/rpc: use static help text (Karl-Johan Alm) 53c3c1ea9e20f881c843a9219e48cec202e962f8 wallet/rpc/getbalances: add entry for 'mine.used' balance in results (Karl-Johan Alm) Pull request description: This addresses a few remaining issues pointed out in #13756: * First commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r284907468 * Second commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r294868973 Ping jnewbery and achow101 as they pointed out these issues. ACKs for commit 71d034: jnewbery: ACK 71d0344cf25d3aaf60112c5248198c444bc98105 meshcollider: re-utACK https://github.com/bitcoin/bitcoin/pull/16239/commits/71d0344cf25d3aaf60112c5248198c444bc98105 Tree-SHA512: 5e28822af0574ad07dbbed21aa2fe7866bf5770b4c0a1c150ad0da8af3152bcfb7170330a7497fa500326c594740ecf63733cf58325821e2811d7b911d5783a0 --- doc/release-notes-13756.md | 10 ++++++---- src/wallet/rpcwallet.cpp | 16 ++++++++++++---- test/functional/wallet_avoidreuse.py | 24 ++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/doc/release-notes-13756.md b/doc/release-notes-13756.md index 21006f46a08e..a500aceb0f08 100644 --- a/doc/release-notes-13756.md +++ b/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. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f6ad0ba7e2cc..f983b03414ea 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -366,7 +366,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) " \"UNSET\"\n" " \"ECONOMICAL\"\n" " \"CONSERVATIVE\""}, - {"avoid_reuse", RPCArg::Type::BOOL, /* default */ pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) ? "true" : "unavailable", "Avoid spending from dirty addresses; addresses are considered\n" + {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Avoid spending from dirty addresses; addresses are considered\n" " dirty if they have previously been used in a transaction."}, }, RPCResult{ @@ -753,7 +753,7 @@ static UniValue getbalance(const JSONRPCRequest& request) {"minconf", RPCArg::Type::NUM, /* default */ "0", "Only include transactions confirmed at least this many times."}, {"addlocked", RPCArg::Type::BOOL, /* default */ "false", "Whether to include transactions locked via InstantSend in the wallet's balance."}, {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also include balance in watch-only addresses (see 'importaddress')"}, - {"avoid_reuse", RPCArg::Type::BOOL, /* default */ pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) ? "true" : "unavailable", "Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."}, + {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."}, }, RPCResult{ RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " received for this wallet." @@ -2394,6 +2394,7 @@ static UniValue getbalances(const JSONRPCRequest& request) {RPCResult::Type::STR_AMOUNT, "trusted", " trusted balance (outputs created by the wallet or confirmed outputs)"}, {RPCResult::Type::STR_AMOUNT, "untrusted_pending", " untrusted pending balance (outputs created by others that are in the mempool)"}, {RPCResult::Type::STR_AMOUNT, "immature", " balance from immature coinbase outputs"}, + {RPCResult::Type::STR_AMOUNT, "used", "(only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)"}, {RPCResult::Type::STR_AMOUNT, "coinjoin", " CoinJoin balance (outputs with enough rounds created by the wallet via mixing)"}, }}, {RPCResult::Type::OBJ, "watchonly", "watchonly balances (not present if wallet does not watch anything)", @@ -2429,6 +2430,12 @@ static UniValue getbalances(const JSONRPCRequest& request) balances_mine.pushKV("trusted", ValueFromAmount(bal.m_mine_trusted)); balances_mine.pushKV("untrusted_pending", ValueFromAmount(bal.m_mine_untrusted_pending)); balances_mine.pushKV("immature", ValueFromAmount(bal.m_mine_immature)); + if (wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { + // If the AVOID_REUSE flag is set, bal has been set to just the un-reused address balance. Get + // the total balance, and then subtract bal to get the reused address balance. + const auto full_bal = wallet.GetBalance(0, false); + balances_mine.pushKV("used", ValueFromAmount(full_bal.m_mine_trusted + full_bal.m_mine_untrusted_pending - bal.m_mine_trusted - bal.m_mine_untrusted_pending)); + } balances_mine.pushKV("coinjoin", ValueFromAmount(bal.m_anonymized)); balances.pushKV("mine", balances_mine); } @@ -2965,7 +2972,6 @@ static UniValue listunspent(const JSONRPCRequest& request) if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); - bool avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); RPCHelpMan{"listunspent", "\nReturns array of unspent transaction outputs\n" "with between minconf and maxconf (inclusive) confirmations.\n" @@ -3008,7 +3014,7 @@ static UniValue listunspent(const JSONRPCRequest& request) {RPCResult::Type::BOOL, "spendable", "Whether we have the private keys to spend this output"}, {RPCResult::Type::BOOL, "solvable", "Whether we know how to spend this output, ignoring the lack of keys"}, {RPCResult::Type::STR, "desc", "(only when solvable) A descriptor for spending this output"}, - {RPCResult::Type::BOOL, "reused", /* optional*/ true, "Whether this output is reused/dirty (sent to an address that was previously spent from)"}, + {RPCResult::Type::BOOL, "reused", /* optional*/ true, "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"}, {RPCResult::Type::BOOL, "safe", "Whether this output is considered safe to spend. Unconfirmed transactions" " from outside keys and unconfirmed replacement transactions are considered unsafe\n" "and are not eligible for spending by fundrawtransaction and sendtoaddress."}, @@ -3121,6 +3127,8 @@ static UniValue listunspent(const JSONRPCRequest& request) LOCK(pwallet->cs_wallet); + const bool avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); + for (const COutput& out : vecOutputs) { CTxDestination address; const CScript& scriptPubKey = out.tx->tx->vout[out.i].scriptPubKey; diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index f26f83db2434..47d16c35a0e8 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -62,6 +62,12 @@ def assert_unspent(node, total_count=None, total_sum=None, reused_supported=None if reused_sum is not None: assert_approx(stats["reused"]["sum"], reused_sum, 0.001) +def assert_balances(node, mine): + '''Make assertions about a node's getbalances output''' + got = node.getbalances()["mine"] + for k,v in mine.items(): + assert_approx(got[k], v, 0.001) + class AvoidReuseTest(BitcoinTestFramework): def set_test_params(self): @@ -149,6 +155,10 @@ def test_fund_send_fund_senddirty(self): # listunspent should show 1 single, unused 10 btc output assert_unspent(self.nodes[1], total_count=1, total_sum=10, reused_supported=True, reused_count=0) + # getbalances should show no used, 10 btc trusted + assert_balances(self.nodes[1], mine={"used": 0, "trusted": 10}) + # node 0 should not show a used entry, as it does not enable avoid_reuse + assert("used" not in self.nodes[0].getbalances()["mine"]) self.nodes[1].sendtoaddress(retaddr, 5) self.nodes[0].generate(1) @@ -156,6 +166,8 @@ def test_fund_send_fund_senddirty(self): # listunspent should show 1 single, unused 5 btc output assert_unspent(self.nodes[1], total_count=1, total_sum=5, reused_supported=True, reused_count=0) + # getbalances should show no used, 5 btc trusted + assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5}) self.nodes[0].sendtoaddress(fundaddr, 10) self.nodes[0].generate(1) @@ -163,11 +175,15 @@ def test_fund_send_fund_senddirty(self): # listunspent should show 2 total outputs (5, 10 btc), one unused (5), one reused (10) assert_unspent(self.nodes[1], total_count=2, total_sum=15, reused_count=1, reused_sum=10) + # getbalances should show 10 used, 5 btc trusted + assert_balances(self.nodes[1], mine={"used": 10, "trusted": 5}) self.nodes[1].sendtoaddress(address=retaddr, amount=10, avoid_reuse=False) # listunspent should show 1 total outputs (5 btc), unused assert_unspent(self.nodes[1], total_count=1, total_sum=5, reused_count=0) + # getbalances should show no used, 5 btc trusted + assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5}) # node 1 should now have about 5 btc left (for both cases) assert_approx(self.nodes[1].getbalance(), 5, 0.001) @@ -193,6 +209,8 @@ def test_fund_send_fund_send(self): # listunspent should show 1 single, unused 10 btc output assert_unspent(self.nodes[1], total_count=1, total_sum=10, reused_supported=True, reused_count=0) + # getbalances should show no used, 10 btc trusted + assert_balances(self.nodes[1], mine={"used": 0, "trusted": 10}) self.nodes[1].sendtoaddress(retaddr, 5) self.nodes[0].generate(1) @@ -200,6 +218,8 @@ def test_fund_send_fund_send(self): # listunspent should show 1 single, unused 5 btc output assert_unspent(self.nodes[1], total_count=1, total_sum=5, reused_supported=True, reused_count=0) + # getbalances should show no used, 5 btc trusted + assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5}) self.nodes[0].sendtoaddress(fundaddr, 10) self.nodes[0].generate(1) @@ -207,6 +227,8 @@ def test_fund_send_fund_send(self): # listunspent should show 2 total outputs (5, 10 btc), one unused (5), one reused (10) assert_unspent(self.nodes[1], total_count=2, total_sum=15, reused_count=1, reused_sum=10) + # getbalances should show 10 used, 5 btc trusted + assert_balances(self.nodes[1], mine={"used": 10, "trusted": 5}) # node 1 should now have a balance of 5 (no dirty) or 15 (including dirty) assert_approx(self.nodes[1].getbalance(), 5, 0.001) @@ -218,6 +240,8 @@ def test_fund_send_fund_send(self): # listunspent should show 2 total outputs (1, 10 btc), one unused (1), one reused (10) assert_unspent(self.nodes[1], total_count=2, total_sum=11, reused_count=1, reused_sum=10) + # getbalances should show 10 used, 1 btc trusted + assert_balances(self.nodes[1], mine={"used": 10, "trusted": 1}) # node 1 should now have about 1 btc left (no dirty) and 11 (including dirty) assert_approx(self.nodes[1].getbalance(), 1, 0.001)