diff options
47 files changed, 1000 insertions, 497 deletions
diff --git a/ci/lint/04_install.sh b/ci/lint/04_install.sh index 01322f61e6..6c3019a1aa 100755 --- a/ci/lint/04_install.sh +++ b/ci/lint/04_install.sh @@ -8,7 +8,7 @@ export LC_ALL=C travis_retry pip3 install codespell==1.15.0 travis_retry pip3 install flake8==3.7.8 -travis_retry pip3 install vulture==0.29 +travis_retry pip3 install vulture==1.0 SHELLCHECK_VERSION=v0.6.0 curl -s "https://storage.googleapis.com/shellcheck/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/ diff --git a/configure.ac b/configure.ac index e1b7281c30..2445b72683 100644 --- a/configure.ac +++ b/configure.ac @@ -777,6 +777,39 @@ fi AC_CHECK_HEADERS([endian.h sys/endian.h byteswap.h stdio.h stdlib.h unistd.h strings.h sys/types.h sys/stat.h sys/select.h sys/prctl.h]) +# FD_ZERO may be dependent on a declaration of memcpy, e.g. in SmartOS +# check that it fails to build without memcpy, then that it builds with +AC_MSG_CHECKING(FD_ZERO memcpy dependence) +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ + #include <cstddef> + #if HAVE_SYS_SELECT_H + #include <sys/select.h> + #endif + ]],[[ + #if HAVE_SYS_SELECT_H + fd_set fds; + FD_ZERO(&fds); + #endif + ]])], + [ AC_MSG_RESULT(no) ], + [ + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ + #include <cstring> + #if HAVE_SYS_SELECT_H + #include <sys/select.h> + #endif + ]], [[ + #if HAVE_SYS_SELECT_H + fd_set fds; + FD_ZERO(&fds); + #endif + ]])], + [ AC_MSG_RESULT(yes); AC_DEFINE(HAVE_CSTRING_DEPENDENT_FD_ZERO, 1, [Define this symbol if FD_ZERO is dependent of a memcpy declaration being available]) ], + [ AC_MSG_ERROR(failed with cstring include) ] + ) + ] +) + AC_CHECK_DECLS([getifaddrs, freeifaddrs],,, [#include <sys/types.h> #include <ifaddrs.h>] diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 57208a678a..56045ade50 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -156,9 +156,7 @@ define $(package)_preprocess_cmds sed -i.old "s|updateqm.commands = \$$$$\$$$$LRELEASE|updateqm.commands = $($(package)_extract_dir)/qttools/bin/lrelease|" qttranslations/translations/translations.pro && \ sed -i.old "/updateqm.depends =/d" qttranslations/translations/translations.pro && \ sed -i.old "s/src_plugins.depends = src_sql src_network/src_plugins.depends = src_network/" qtbase/src/src.pro && \ - sed -i.old "s|X11/extensions/XIproto.h|X11/X.h|" qtbase/src/plugins/platforms/xcb/qxcbxsettings.cpp && \ sed -i.old -e 's/if \[ "$$$$XPLATFORM_MAC" = "yes" \]; then xspecvals=$$$$(macSDKify/if \[ "$$$$BUILD_ON_MAC" = "yes" \]; then xspecvals=$$$$(macSDKify/' -e 's|/bin/pwd|pwd|' qtbase/configure && \ - sed -i.old 's/CGEventCreateMouseEvent(0, kCGEventMouseMoved, pos, 0)/CGEventCreateMouseEvent(0, kCGEventMouseMoved, pos, kCGMouseButtonLeft)/' qtbase/src/plugins/platforms/cocoa/qcocoacursor.mm && \ mkdir -p qtbase/mkspecs/macx-clang-linux &&\ cp -f qtbase/mkspecs/macx-clang/Info.plist.lib qtbase/mkspecs/macx-clang-linux/ &&\ cp -f qtbase/mkspecs/macx-clang/Info.plist.app qtbase/mkspecs/macx-clang-linux/ &&\ diff --git a/depends/packages/zlib.mk b/depends/packages/zlib.mk index 1600b11a01..168f85e65e 100644 --- a/depends/packages/zlib.mk +++ b/depends/packages/zlib.mk @@ -5,23 +5,26 @@ $(package)_file_name=$(package)-$($(package)_version).tar.gz $(package)_sha256_hash=c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1 define $(package)_set_vars -$(package)_build_opts= CC="$($(package)_cc)" -$(package)_build_opts+=CFLAGS="$($(package)_cflags) $($(package)_cppflags) -fPIC" -$(package)_build_opts+=RANLIB="$($(package)_ranlib)" -$(package)_build_opts+=AR="$($(package)_ar)" -$(package)_build_opts_darwin+=AR="$($(package)_libtool)" -$(package)_build_opts_darwin+=ARFLAGS="-o" +$(package)_config_opts= CC="$($(package)_cc)" +$(package)_config_opts+=CFLAGS="$($(package)_cflags) $($(package)_cppflags) -fPIC" +$(package)_config_opts+=RANLIB="$($(package)_ranlib)" +$(package)_config_opts+=AR="$($(package)_ar)" +$(package)_config_opts_darwin+=AR="$($(package)_libtool)" +$(package)_config_opts_darwin+=ARFLAGS="-o" endef +# zlib has its own custom configure script that takes in options like CC, +# CFLAGS, RANLIB, AR, and ARFLAGS from the environment rather than from +# command-line arguments. define $(package)_config_cmds - ./configure --static --prefix=$(host_prefix) + env $($(package)_config_opts) ./configure --static --prefix=$(host_prefix) endef define $(package)_build_cmds - $(MAKE) $($(package)_build_opts) libz.a + $(MAKE) libz.a endef define $(package)_stage_cmds - $(MAKE) DESTDIR=$($(package)_staging_dir) install $($(package)_build_opts) + $(MAKE) DESTDIR=$($(package)_staging_dir) install endef diff --git a/doc/release-notes-16512.md b/doc/release-notes-16512.md new file mode 100644 index 0000000000..9aa9cf36f9 --- /dev/null +++ b/doc/release-notes-16512.md @@ -0,0 +1,4 @@ +RPC changes +----------- +The RPC `joinpsbts` will shuffle the order of the inputs and outputs of the resulting joined psbt. +Previously inputs and outputs were added in the order that the PSBTs were provided which makes correlating inputs to outputs extremely easy. diff --git a/src/Makefile.am b/src/Makefile.am index 8fc7f61d4b..1ef62a656d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -484,6 +484,7 @@ libbitcoin_util_a_SOURCES = \ support/lockedpool.cpp \ chainparamsbase.cpp \ clientversion.cpp \ + compat/glibc_sanity_fdelt.cpp \ compat/glibc_sanity.cpp \ compat/glibcxx_sanity.cpp \ compat/strnlen.cpp \ diff --git a/src/compat/glibc_sanity.cpp b/src/compat/glibc_sanity.cpp index 1ef66e27b4..cc74f28899 100644 --- a/src/compat/glibc_sanity.cpp +++ b/src/compat/glibc_sanity.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-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. @@ -9,7 +9,7 @@ #include <cstddef> #if defined(HAVE_SYS_SELECT_H) -#include <sys/select.h> +bool sanity_test_fdelt(); #endif extern "C" void* memcpy(void* a, const void* b, size_t c); @@ -41,21 +41,6 @@ bool sanity_test_memcpy() } return true; } - -#if defined(HAVE_SYS_SELECT_H) -// trigger: Call FD_SET to trigger __fdelt_chk. FORTIFY_SOURCE must be defined -// as >0 and optimizations must be set to at least -O2. -// test: Add a file descriptor to an empty fd_set. Verify that it has been -// correctly added. -bool sanity_test_fdelt() -{ - fd_set fds; - FD_ZERO(&fds); - FD_SET(0, &fds); - return FD_ISSET(0, &fds); -} -#endif - } // namespace bool glibc_sanity_test() diff --git a/src/compat/glibc_sanity_fdelt.cpp b/src/compat/glibc_sanity_fdelt.cpp new file mode 100644 index 0000000000..87140d0c71 --- /dev/null +++ b/src/compat/glibc_sanity_fdelt.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2009-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. + +#if defined(HAVE_CONFIG_H) +#include <config/bitcoin-config.h> +#endif + +#if defined(HAVE_SYS_SELECT_H) +#ifdef HAVE_CSTRING_DEPENDENT_FD_ZERO +#include <cstring> +#endif +#include <sys/select.h> + +// trigger: Call FD_SET to trigger __fdelt_chk. FORTIFY_SOURCE must be defined +// as >0 and optimizations must be set to at least -O2. +// test: Add a file descriptor to an empty fd_set. Verify that it has been +// correctly added. +bool sanity_test_fdelt() +{ + fd_set fds; + FD_ZERO(&fds); + FD_SET(0, &fds); + return FD_ISSET(0, &fds); +} +#endif diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index 23ed3ecb53..00ebbbd1ab 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -18,7 +18,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-oversize"); - // Check for negative or overflow output values + // Check for negative or overflow output values (see CVE-2010-5139) CAmount nValueOut = 0; for (const auto& txout : tx.vout) { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 93a98974eb..34d349e8e9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2559,7 +2559,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } AddOrphanTx(ptx, pfrom->GetId()); - // DoS prevention: do not allow mapOrphanTransactions to grow unbounded + // DoS prevention: do not allow mapOrphanTransactions to grow unbounded (see CVE-2012-3789) unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, gArgs.GetArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS)); unsigned int nEvicted = LimitOrphanTxSize(nMaxOrphanTx); if (nEvicted > 0) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index fb8ea8c227..461e8025da 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -14,9 +14,11 @@ #include <node/coin.h> #include <node/psbt.h> #include <node/transaction.h> +#include <policy/policy.h> #include <policy/rbf.h> #include <primitives/transaction.h> #include <psbt.h> +#include <random.h> #include <rpc/rawtransaction_util.h> #include <rpc/server.h> #include <rpc/util.h> @@ -37,11 +39,11 @@ #include <univalue.h> -/** High fee for sendrawtransaction and testmempoolaccept. - * By default, transaction with a fee higher than this will be rejected by the - * RPCs. This can be overridden with the maxfeerate argument. +/** High fee rate for sendrawtransaction and testmempoolaccept. + * By default, transaction with a fee rate higher than this will be rejected by + * the RPCs. This can be overridden with the maxfeerate argument. */ -constexpr static CAmount DEFAULT_MAX_RAW_TX_FEE{COIN / 10}; +static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry) { @@ -774,7 +776,7 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) "\nAlso see createrawtransaction and signrawtransactionwithkey calls.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, - {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE), + {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK()), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB.\nSet to 0 to accept any fee rate.\n"}, }, @@ -804,19 +806,17 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); CTransactionRef tx(MakeTransactionRef(std::move(mtx))); - CAmount max_raw_tx_fee = DEFAULT_MAX_RAW_TX_FEE; + CFeeRate max_raw_tx_fee_rate = DEFAULT_MAX_RAW_TX_FEE_RATE; // TODO: temporary migration code for old clients. Remove in v0.20 if (request.params[1].isBool()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Second argument must be numeric (maxfeerate) and no longer supports a boolean. To allow a transaction with high fees, set maxfeerate to 0."); } else if (!request.params[1].isNull()) { - size_t weight = GetTransactionWeight(*tx); - CFeeRate fr(AmountFromValue(request.params[1])); - // the +3/4 part rounds the value up, and is the same formula used when - // calculating the fee for a transaction - // (see GetVirtualTransactionSize) - max_raw_tx_fee = fr.GetFee((weight+3)/4); + max_raw_tx_fee_rate = CFeeRate(AmountFromValue(request.params[1])); } + int64_t virtual_size = GetVirtualTransactionSize(*tx); + CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); + std::string err_string; AssertLockNotHeld(cs_main); const TransactionError err = BroadcastTransaction(tx, err_string, max_raw_tx_fee, /*relay*/ true, /*wait_callback*/ true); @@ -840,7 +840,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, }, }, - {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB\n"}, + {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK()), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB\n"}, }, RPCResult{ "[ (array) The result of the mempool acceptance test for each raw transaction in the input array.\n" @@ -880,19 +880,17 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) CTransactionRef tx(MakeTransactionRef(std::move(mtx))); const uint256& tx_hash = tx->GetHash(); - CAmount max_raw_tx_fee = DEFAULT_MAX_RAW_TX_FEE; + CFeeRate max_raw_tx_fee_rate = DEFAULT_MAX_RAW_TX_FEE_RATE; // TODO: temporary migration code for old clients. Remove in v0.20 if (request.params[1].isBool()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Second argument must be numeric (maxfeerate) and no longer supports a boolean. To allow a transaction with high fees, set maxfeerate to 0."); } else if (!request.params[1].isNull()) { - size_t weight = GetTransactionWeight(*tx); - CFeeRate fr(AmountFromValue(request.params[1])); - // the +3/4 part rounds the value up, and is the same formula used when - // calculating the fee for a transaction - // (see GetVirtualTransactionSize) - max_raw_tx_fee = fr.GetFee((weight+3)/4); + max_raw_tx_fee_rate = CFeeRate(AmountFromValue(request.params[1])); } + int64_t virtual_size = GetVirtualTransactionSize(*tx); + CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); + UniValue result(UniValue::VARR); UniValue result_0(UniValue::VOBJ); result_0.pushKV("txid", tx_hash.GetHex()); @@ -1615,8 +1613,30 @@ UniValue joinpsbts(const JSONRPCRequest& request) merged_psbt.unknown.insert(psbt.unknown.begin(), psbt.unknown.end()); } + // Generate list of shuffled indices for shuffling inputs and outputs of the merged PSBT + std::vector<int> input_indices(merged_psbt.inputs.size()); + std::iota(input_indices.begin(), input_indices.end(), 0); + std::vector<int> output_indices(merged_psbt.outputs.size()); + std::iota(output_indices.begin(), output_indices.end(), 0); + + // Shuffle input and output indicies lists + Shuffle(input_indices.begin(), input_indices.end(), FastRandomContext()); + Shuffle(output_indices.begin(), output_indices.end(), FastRandomContext()); + + PartiallySignedTransaction shuffled_psbt; + shuffled_psbt.tx = CMutableTransaction(); + shuffled_psbt.tx->nVersion = merged_psbt.tx->nVersion; + shuffled_psbt.tx->nLockTime = merged_psbt.tx->nLockTime; + for (int i : input_indices) { + shuffled_psbt.AddInput(merged_psbt.tx->vin[i], merged_psbt.inputs[i]); + } + for (int i : output_indices) { + shuffled_psbt.AddOutput(merged_psbt.tx->vout[i], merged_psbt.outputs[i]); + } + shuffled_psbt.unknown.insert(merged_psbt.unknown.begin(), merged_psbt.unknown.end()); + CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); - ssTx << merged_psbt; + ssTx << shuffled_psbt; return EncodeBase64((unsigned char*)ssTx.data(), ssTx.size()); } diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index f8701b6d01..20fae2eebf 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -334,7 +334,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& opcode == OP_MOD || opcode == OP_LSHIFT || opcode == OP_RSHIFT) - return set_error(serror, SCRIPT_ERR_DISABLED_OPCODE); // Disabled opcodes. + return set_error(serror, SCRIPT_ERR_DISABLED_OPCODE); // Disabled opcodes (CVE-2010-5137). // With SCRIPT_VERIFY_CONST_SCRIPTCODE, OP_CODESEPARATOR in non-segwit script is rejected even in an unexecuted branch if (opcode == OP_CODESEPARATOR && sigversion == SigVersion::BASE && (flags & SCRIPT_VERIFY_CONST_SCRIPTCODE)) @@ -1483,6 +1483,8 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const C return set_error(serror, SCRIPT_ERR_SIG_PUSHONLY); } + // scriptSig and scriptPubKey must be evaluated sequentially on the same stack + // rather than being simply concatenated (see CVE-2010-5141) std::vector<std::vector<unsigned char> > stack, stackCopy; if (!EvalScript(stack, scriptSig, flags, checker, SigVersion::BASE, serror)) // serror is set diff --git a/src/test/data/script_tests.json b/src/test/data/script_tests.json index 9b320b6943..3241f32f56 100644 --- a/src/test/data/script_tests.json +++ b/src/test/data/script_tests.json @@ -829,15 +829,16 @@ ["NOP", "2SWAP 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], ["1", "2 3 2SWAP 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], + +["NOP", "SIZE 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], + +["TEST DISABLED OP CODES (CVE-2010-5137)"], ["'a' 'b'", "CAT", "P2SH,STRICTENC", "DISABLED_OPCODE", "CAT disabled"], ["'a' 'b' 0", "IF CAT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "CAT disabled"], ["'abc' 1 1", "SUBSTR", "P2SH,STRICTENC", "DISABLED_OPCODE", "SUBSTR disabled"], ["'abc' 1 1 0", "IF SUBSTR ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "SUBSTR disabled"], ["'abc' 2 0", "IF LEFT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "LEFT disabled"], ["'abc' 2 0", "IF RIGHT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "RIGHT disabled"], - -["NOP", "SIZE 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], - ["'abc'", "IF INVERT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "INVERT disabled"], ["1 2 0 IF AND ELSE 1 ENDIF", "NOP", "P2SH,STRICTENC", "DISABLED_OPCODE", "AND disabled"], ["1 2 0 IF OR ELSE 1 ENDIF", "NOP", "P2SH,STRICTENC", "DISABLED_OPCODE", "OR disabled"], diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index 1684258c9f..dc38a1a818 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -249,4 +249,104 @@ BOOST_AUTO_TEST_CASE(merkle_test) } } + +BOOST_AUTO_TEST_CASE(merkle_test_empty_block) +{ + bool mutated = false; + CBlock block; + uint256 root = BlockMerkleRoot(block, &mutated); + + BOOST_CHECK_EQUAL(root.IsNull(), true); + BOOST_CHECK_EQUAL(mutated, false); +} + +BOOST_AUTO_TEST_CASE(merkle_test_oneTx_block) +{ + bool mutated = false; + CBlock block; + + block.vtx.resize(1); + CMutableTransaction mtx; + mtx.nLockTime = 0; + block.vtx[0] = MakeTransactionRef(std::move(mtx)); + uint256 root = BlockMerkleRoot(block, &mutated); + BOOST_CHECK_EQUAL(root, block.vtx[0]->GetHash()); + BOOST_CHECK_EQUAL(mutated, false); +} + +BOOST_AUTO_TEST_CASE(merkle_test_OddTxWithRepeatedLastTx_block) +{ + bool mutated; + CBlock block, blockWithRepeatedLastTx; + + block.vtx.resize(3); + + for (std::size_t pos = 0; pos < block.vtx.size(); pos++) { + CMutableTransaction mtx; + mtx.nLockTime = pos; + block.vtx[pos] = MakeTransactionRef(std::move(mtx)); + } + + blockWithRepeatedLastTx = block; + blockWithRepeatedLastTx.vtx.push_back(blockWithRepeatedLastTx.vtx.back()); + + uint256 rootofBlock = BlockMerkleRoot(block, &mutated); + BOOST_CHECK_EQUAL(mutated, false); + + uint256 rootofBlockWithRepeatedLastTx = BlockMerkleRoot(blockWithRepeatedLastTx, &mutated); + BOOST_CHECK_EQUAL(rootofBlock, rootofBlockWithRepeatedLastTx); + BOOST_CHECK_EQUAL(mutated, true); +} + +BOOST_AUTO_TEST_CASE(merkle_test_LeftSubtreeRightSubtree) +{ + CBlock block, leftSubtreeBlock, rightSubtreeBlock; + + block.vtx.resize(4); + std::size_t pos; + for (pos = 0; pos < block.vtx.size(); pos++) { + CMutableTransaction mtx; + mtx.nLockTime = pos; + block.vtx[pos] = MakeTransactionRef(std::move(mtx)); + } + + for (pos = 0; pos < block.vtx.size() / 2; pos++) + leftSubtreeBlock.vtx.push_back(block.vtx[pos]); + + for (pos = block.vtx.size() / 2; pos < block.vtx.size(); pos++) + rightSubtreeBlock.vtx.push_back(block.vtx[pos]); + + uint256 root = BlockMerkleRoot(block); + uint256 rootOfLeftSubtree = BlockMerkleRoot(leftSubtreeBlock); + uint256 rootOfRightSubtree = BlockMerkleRoot(rightSubtreeBlock); + std::vector<uint256> leftRight; + leftRight.push_back(rootOfLeftSubtree); + leftRight.push_back(rootOfRightSubtree); + uint256 rootOfLR = ComputeMerkleRoot(leftRight); + + BOOST_CHECK_EQUAL(root, rootOfLR); +} + +BOOST_AUTO_TEST_CASE(merkle_test_BlockWitness) +{ + CBlock block; + + block.vtx.resize(2); + for (std::size_t pos = 0; pos < block.vtx.size(); pos++) { + CMutableTransaction mtx; + mtx.nLockTime = pos; + block.vtx[pos] = MakeTransactionRef(std::move(mtx)); + } + + uint256 blockWitness = BlockWitnessMerkleRoot(block); + + std::vector<uint256> hashes; + hashes.resize(block.vtx.size()); + hashes[0].SetNull(); + hashes[1] = block.vtx[1]->GetHash(); + + uint256 merkelRootofHashes = ComputeMerkleRoot(hashes); + + BOOST_CHECK_EQUAL(merkelRootofHashes, blockWitness); +} BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.cpp b/src/validation.cpp index 3a35fd510a..4caee52424 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -428,21 +428,134 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationSt return CheckInputs(tx, state, view, flags, cacheSigStore, true, txdata); } -/** - * @param[out] coins_to_uncache Return any outpoints which were not previously present in the - * coins cache, but were added as a result of validating the tx - * for mempool acceptance. This allows the caller to optionally - * remove the cache additions if the associated transaction ends - * up being rejected by the mempool. - */ -static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, - bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced, - bool bypass_limits, const CAmount& nAbsurdFee, std::vector<COutPoint>& coins_to_uncache, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +namespace { + +class MemPoolAccept { - const CTransaction& tx = *ptx; - const uint256 hash = tx.GetHash(); - AssertLockHeld(cs_main); - LOCK(pool.cs); // mempool "read lock" (held through GetMainSignals().TransactionAddedToMempool()) +public: + MemPoolAccept(CTxMemPool& mempool) : m_pool(mempool), m_view(&m_dummy), m_viewmempool(&::ChainstateActive().CoinsTip(), m_pool), + m_limit_ancestors(gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT)), + m_limit_ancestor_size(gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000), + m_limit_descendants(gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT)), + m_limit_descendant_size(gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000) {} + + // We put the arguments we're handed into a struct, so we can pass them + // around easier. + struct ATMPArgs { + const CChainParams& m_chainparams; + CValidationState &m_state; + bool* m_missing_inputs; + const int64_t m_accept_time; + std::list<CTransactionRef>* m_replaced_transactions; + const bool m_bypass_limits; + const CAmount& m_absurd_fee; + /* + * Return any outpoints which were not previously present in the coins + * cache, but were added as a result of validating the tx for mempool + * acceptance. This allows the caller to optionally remove the cache + * additions if the associated transaction ends up being rejected by + * the mempool. + */ + std::vector<COutPoint>& m_coins_to_uncache; + const bool m_test_accept; + }; + + // Single transaction acceptance + bool AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + +private: + // All the intermediate state that gets passed between the various levels + // of checking a given transaction. + struct Workspace { + Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {} + std::set<uint256> m_conflicts; + CTxMemPool::setEntries m_all_conflicting; + CTxMemPool::setEntries m_ancestors; + std::unique_ptr<CTxMemPoolEntry> m_entry; + + bool m_replacement_transaction; + CAmount m_modified_fees; + CAmount m_conflicting_fees; + size_t m_conflicting_size; + + const CTransactionRef& m_ptx; + const uint256& m_hash; + }; + + // Run the policy checks on a given transaction, excluding any script checks. + // Looks up inputs, calculates feerate, considers replacement, evaluates + // package limits, etc. As this function can be invoked for "free" by a peer, + // only tests that are fast should be done here (to avoid CPU DoS). + bool PreChecks(ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + + // Run the script checks using our policy flags. As this can be slow, we should + // only invoke this on transactions that have otherwise passed policy checks. + bool PolicyScriptChecks(ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + // Re-run the script checks, using consensus flags, and try to cache the + // result in the scriptcache. This should be done after + // PolicyScriptChecks(). This requires that all inputs either be in our + // utxo set or in the mempool. + bool ConsensusScriptChecks(ATMPArgs& args, Workspace& ws, PrecomputedTransactionData &txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + // Try to add the transaction to the mempool, removing any conflicts first. + // Returns true if the transaction is in the mempool after any size + // limiting is performed, false otherwise. + bool Finalize(ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + + // Compare a package's feerate against minimum allowed. + bool CheckFeeRate(size_t package_size, CAmount package_fee, CValidationState& state) + { + CAmount mempoolRejectFee = m_pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(package_size); + if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); + } + + if (package_fee < ::minRelayTxFee.GetFee(package_size)) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "min relay fee not met", strprintf("%d < %d", package_fee, ::minRelayTxFee.GetFee(package_size))); + } + return true; + } + +private: + CTxMemPool& m_pool; + CCoinsViewCache m_view; + CCoinsViewMemPool m_viewmempool; + CCoinsView m_dummy; + + // The package limits in effect at the time of invocation. + const size_t m_limit_ancestors; + const size_t m_limit_ancestor_size; + // These may be modified while evaluating a transaction (eg to account for + // in-mempool conflicts; see below). + size_t m_limit_descendants; + size_t m_limit_descendant_size; +}; + +bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) +{ + const CTransactionRef& ptx = ws.m_ptx; + const CTransaction& tx = *ws.m_ptx; + const uint256& hash = ws.m_hash; + + // Copy/alias what we need out of args + CValidationState &state = args.m_state; + bool* pfMissingInputs = args.m_missing_inputs; + const int64_t nAcceptTime = args.m_accept_time; + const bool bypass_limits = args.m_bypass_limits; + const CAmount& nAbsurdFee = args.m_absurd_fee; + std::vector<COutPoint>& coins_to_uncache = args.m_coins_to_uncache; + + // Alias what we need out of ws + std::set<uint256>& setConflicts = ws.m_conflicts; + CTxMemPool::setEntries& allConflicting = ws.m_all_conflicting; + CTxMemPool::setEntries& setAncestors = ws.m_ancestors; + std::unique_ptr<CTxMemPoolEntry>& entry = ws.m_entry; + bool& fReplacementTransaction = ws.m_replacement_transaction; + CAmount& nModifiedFees = ws.m_modified_fees; + CAmount& nConflictingFees = ws.m_conflicting_fees; + size_t& nConflictingSize = ws.m_conflicting_size; + if (pfMissingInputs) { *pfMissingInputs = false; } @@ -461,7 +574,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Do not work on transactions that are too small. // A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes. - // Transactions smaller than this are not relayed to reduce unnecessary malloc overhead. + // Transactions smaller than this are not relayed to mitigate CVE-2017-12842 by not relaying + // 64-byte transactions. if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) < MIN_STANDARD_TX_NONWITNESS_SIZE) return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "tx-size-small"); @@ -472,15 +586,14 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-final"); // is it already in the memory pool? - if (pool.exists(hash)) { + if (m_pool.exists(hash)) { return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-in-mempool"); } // Check for conflicts with in-memory transactions - std::set<uint256> setConflicts; for (const CTxIn &txin : tx.vin) { - const CTransaction* ptxConflicting = pool.GetConflictTx(txin.prevout); + const CTransaction* ptxConflicting = m_pool.GetConflictTx(txin.prevout); if (ptxConflicting) { if (!setConflicts.count(ptxConflicting->GetHash())) { @@ -514,395 +627,436 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool } } - { - CCoinsView dummy; - CCoinsViewCache view(&dummy); - - LockPoints lp; - CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); - CCoinsViewMemPool viewMemPool(&coins_cache, pool); - view.SetBackend(viewMemPool); - - // do all inputs exist? - for (const CTxIn& txin : tx.vin) { - if (!coins_cache.HaveCoinInCache(txin.prevout)) { - coins_to_uncache.push_back(txin.prevout); - } + LockPoints lp; + m_view.SetBackend(m_viewmempool); - // Note: this call may add txin.prevout to the coins cache - // (CoinsTip().cacheCoins) by way of FetchCoin(). It should be removed - // later (via coins_to_uncache) if this tx turns out to be invalid. - if (!view.HaveCoin(txin.prevout)) { - // Are inputs missing because we already have the tx? - for (size_t out = 0; out < tx.vout.size(); out++) { - // Optimistically just do efficient check of cache for outputs - if (coins_cache.HaveCoinInCache(COutPoint(hash, out))) { - return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-known"); - } - } - // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet - if (pfMissingInputs) { - *pfMissingInputs = true; + CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); + // do all inputs exist? + for (const CTxIn& txin : tx.vin) { + if (!coins_cache.HaveCoinInCache(txin.prevout)) { + coins_to_uncache.push_back(txin.prevout); + } + + // Note: this call may add txin.prevout to the coins cache + // (coins_cache.cacheCoins) by way of FetchCoin(). It should be removed + // later (via coins_to_uncache) if this tx turns out to be invalid. + if (!m_view.HaveCoin(txin.prevout)) { + // Are inputs missing because we already have the tx? + for (size_t out = 0; out < tx.vout.size(); out++) { + // Optimistically just do efficient check of cache for outputs + if (coins_cache.HaveCoinInCache(COutPoint(hash, out))) { + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-known"); } - return false; // fMissingInputs and !state.IsInvalid() is used to detect this condition, don't set state.Invalid() } + // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet + if (pfMissingInputs) { + *pfMissingInputs = true; + } + return false; // fMissingInputs and !state.IsInvalid() is used to detect this condition, don't set state.Invalid() } + } - // Bring the best block into scope - view.GetBestBlock(); + // Bring the best block into scope + m_view.GetBestBlock(); - // we have all inputs cached now, so switch back to dummy, so we don't need to keep lock on mempool - view.SetBackend(dummy); + // we have all inputs cached now, so switch back to dummy (to protect + // against bugs where we pull more inputs from disk that miss being added + // to coins_to_uncache) + m_view.SetBackend(m_dummy); - // Only accept BIP68 sequence locked transactions that can be mined in the next - // block; we don't want our mempool filled up with transactions that can't - // be mined yet. - // Must keep pool.cs for this unless we change CheckSequenceLocks to take a - // CoinsViewCache instead of create its own - if (!CheckSequenceLocks(pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-BIP68-final"); + // Only accept BIP68 sequence locked transactions that can be mined in the next + // block; we don't want our mempool filled up with transactions that can't + // be mined yet. + // Must keep pool.cs for this unless we change CheckSequenceLocks to take a + // CoinsViewCache instead of create its own + if (!CheckSequenceLocks(m_pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-BIP68-final"); - CAmount nFees = 0; - if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees)) { - return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); - } + CAmount nFees = 0; + if (!Consensus::CheckTxInputs(tx, state, m_view, GetSpendHeight(m_view), nFees)) { + return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); + } - // Check for non-standard pay-to-script-hash in inputs - if (fRequireStandard && !AreInputsStandard(tx, view)) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); + // Check for non-standard pay-to-script-hash in inputs + if (fRequireStandard && !AreInputsStandard(tx, m_view)) + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); - // Check for non-standard witness in P2WSH - if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, view)) - return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_NONSTANDARD, "bad-witness-nonstandard"); + // Check for non-standard witness in P2WSH + if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) + return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_NONSTANDARD, "bad-witness-nonstandard"); - int64_t nSigOpsCost = GetTransactionSigOpCost(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS); + int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS); - // nModifiedFees includes any fee deltas from PrioritiseTransaction - CAmount nModifiedFees = nFees; - pool.ApplyDelta(hash, nModifiedFees); + // nModifiedFees includes any fee deltas from PrioritiseTransaction + nModifiedFees = nFees; + m_pool.ApplyDelta(hash, nModifiedFees); - // Keep track of transactions that spend a coinbase, which we re-scan - // during reorgs to ensure COINBASE_MATURITY is still met. - bool fSpendsCoinbase = false; - for (const CTxIn &txin : tx.vin) { - const Coin &coin = view.AccessCoin(txin.prevout); - if (coin.IsCoinBase()) { - fSpendsCoinbase = true; - break; - } + // Keep track of transactions that spend a coinbase, which we re-scan + // during reorgs to ensure COINBASE_MATURITY is still met. + bool fSpendsCoinbase = false; + for (const CTxIn &txin : tx.vin) { + const Coin &coin = m_view.AccessCoin(txin.prevout); + if (coin.IsCoinBase()) { + fSpendsCoinbase = true; + break; } + } - CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, ::ChainActive().Height(), - fSpendsCoinbase, nSigOpsCost, lp); - unsigned int nSize = entry.GetTxSize(); + entry.reset(new CTxMemPoolEntry(ptx, nFees, nAcceptTime, ::ChainActive().Height(), + fSpendsCoinbase, nSigOpsCost, lp)); + unsigned int nSize = entry->GetTxSize(); - if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops", + if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); - CAmount mempoolRejectFee = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nSize); - if (!bypass_limits && mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", strprintf("%d < %d", nModifiedFees, mempoolRejectFee)); - } - - // No transactions are allowed below minRelayTxFee except from disconnected blocks - if (!bypass_limits && nModifiedFees < ::minRelayTxFee.GetFee(nSize)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "min relay fee not met", strprintf("%d < %d", nModifiedFees, ::minRelayTxFee.GetFee(nSize))); - } + // No transactions are allowed below minRelayTxFee except from disconnected + // blocks + if (!bypass_limits && !CheckFeeRate(nSize, nModifiedFees, state)) return false; - if (nAbsurdFee && nFees > nAbsurdFee) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, + if (nAbsurdFee && nFees > nAbsurdFee) + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_HIGHFEE, "absurdly-high-fee", strprintf("%d > %d", nFees, nAbsurdFee)); - const CTxMemPool::setEntries setIterConflicting = pool.GetIterSet(setConflicts); - // Calculate in-mempool ancestors, up to a limit. - CTxMemPool::setEntries setAncestors; - size_t nLimitAncestors = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); - size_t nLimitAncestorSize = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; - size_t nLimitDescendants = gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT); - size_t nLimitDescendantSize = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000; - - if (setConflicts.size() == 1) { - // In general, when we receive an RBF transaction with mempool conflicts, we want to know whether we - // would meet the chain limits after the conflicts have been removed. However, there isn't a practical - // way to do this short of calculating the ancestor and descendant sets with an overlay cache of - // changed mempool entries. Due to both implementation and runtime complexity concerns, this isn't - // very realistic, thus we only ensure a limited set of transactions are RBF'able despite mempool - // conflicts here. Importantly, we need to ensure that some transactions which were accepted using - // the below carve-out are able to be RBF'ed, without impacting the security the carve-out provides - // for off-chain contract systems (see link in the comment below). - // - // Specifically, the subset of RBF transactions which we allow despite chain limits are those which - // conflict directly with exactly one other transaction (but may evict children of said transaction), - // and which are not adding any new mempool dependencies. Note that the "no new mempool dependencies" - // check is accomplished later, so we don't bother doing anything about it here, but if BIP 125 is - // amended, we may need to move that check to here instead of removing it wholesale. - // - // Such transactions are clearly not merging any existing packages, so we are only concerned with - // ensuring that (a) no package is growing past the package size (not count) limits and (b) we are - // not allowing something to effectively use the (below) carve-out spot when it shouldn't be allowed - // to. - // - // To check these we first check if we meet the RBF criteria, above, and increment the descendant - // limits by the direct conflict and its descendants (as these are recalculated in - // CalculateMempoolAncestors by assuming the new transaction being added is a new descendant, with no - // removals, of each parent's existing dependant set). The ancestor count limits are unmodified (as - // the ancestor limits should be the same for both our new transaction and any conflicts). - // We don't bother incrementing nLimitDescendants by the full removal count as that limit never comes - // into force here (as we're only adding a single transaction). - assert(setIterConflicting.size() == 1); - CTxMemPool::txiter conflict = *setIterConflicting.begin(); - - nLimitDescendants += 1; - nLimitDescendantSize += conflict->GetSizeWithDescendants(); - } - - std::string errString; - if (!pool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) { - setAncestors.clear(); - // If CalculateMemPoolAncestors fails second time, we want the original error string. - std::string dummy_err_string; - // Contracting/payment channels CPFP carve-out: - // If the new transaction is relatively small (up to 40k weight) - // and has at most one ancestor (ie ancestor limit of 2, including - // the new transaction), allow it if its parent has exactly the - // descendant limit descendants. - // - // This allows protocols which rely on distrusting counterparties - // being able to broadcast descendants of an unconfirmed transaction - // to be secure by simply only having two immediately-spendable - // outputs - one for each counterparty. For more info on the uses for - // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html - if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || - !pool.CalculateMemPoolAncestors(entry, setAncestors, 2, nLimitAncestorSize, nLimitDescendants + 1, nLimitDescendantSize + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too-long-mempool-chain", errString); - } - } - - // A transaction that spends outputs that would be replaced by it is invalid. Now - // that we have the set of all ancestors we can detect this - // pathological case by making sure setConflicts and setAncestors don't - // intersect. - for (CTxMemPool::txiter ancestorIt : setAncestors) + const CTxMemPool::setEntries setIterConflicting = m_pool.GetIterSet(setConflicts); + // Calculate in-mempool ancestors, up to a limit. + if (setConflicts.size() == 1) { + // In general, when we receive an RBF transaction with mempool conflicts, we want to know whether we + // would meet the chain limits after the conflicts have been removed. However, there isn't a practical + // way to do this short of calculating the ancestor and descendant sets with an overlay cache of + // changed mempool entries. Due to both implementation and runtime complexity concerns, this isn't + // very realistic, thus we only ensure a limited set of transactions are RBF'able despite mempool + // conflicts here. Importantly, we need to ensure that some transactions which were accepted using + // the below carve-out are able to be RBF'ed, without impacting the security the carve-out provides + // for off-chain contract systems (see link in the comment below). + // + // Specifically, the subset of RBF transactions which we allow despite chain limits are those which + // conflict directly with exactly one other transaction (but may evict children of said transaction), + // and which are not adding any new mempool dependencies. Note that the "no new mempool dependencies" + // check is accomplished later, so we don't bother doing anything about it here, but if BIP 125 is + // amended, we may need to move that check to here instead of removing it wholesale. + // + // Such transactions are clearly not merging any existing packages, so we are only concerned with + // ensuring that (a) no package is growing past the package size (not count) limits and (b) we are + // not allowing something to effectively use the (below) carve-out spot when it shouldn't be allowed + // to. + // + // To check these we first check if we meet the RBF criteria, above, and increment the descendant + // limits by the direct conflict and its descendants (as these are recalculated in + // CalculateMempoolAncestors by assuming the new transaction being added is a new descendant, with no + // removals, of each parent's existing dependant set). The ancestor count limits are unmodified (as + // the ancestor limits should be the same for both our new transaction and any conflicts). + // We don't bother incrementing m_limit_descendants by the full removal count as that limit never comes + // into force here (as we're only adding a single transaction). + assert(setIterConflicting.size() == 1); + CTxMemPool::txiter conflict = *setIterConflicting.begin(); + + m_limit_descendants += 1; + m_limit_descendant_size += conflict->GetSizeWithDescendants(); + } + + std::string errString; + if (!m_pool.CalculateMemPoolAncestors(*entry, setAncestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, m_limit_descendant_size, errString)) { + setAncestors.clear(); + // If CalculateMemPoolAncestors fails second time, we want the original error string. + std::string dummy_err_string; + // Contracting/payment channels CPFP carve-out: + // If the new transaction is relatively small (up to 40k weight) + // and has at most one ancestor (ie ancestor limit of 2, including + // the new transaction), allow it if its parent has exactly the + // descendant limit descendants. + // + // This allows protocols which rely on distrusting counterparties + // being able to broadcast descendants of an unconfirmed transaction + // to be secure by simply only having two immediately-spendable + // outputs - one for each counterparty. For more info on the uses for + // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html + if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || + !m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too-long-mempool-chain", errString); + } + } + + // A transaction that spends outputs that would be replaced by it is invalid. Now + // that we have the set of all ancestors we can detect this + // pathological case by making sure setConflicts and setAncestors don't + // intersect. + for (CTxMemPool::txiter ancestorIt : setAncestors) + { + const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); + if (setConflicts.count(hashAncestor)) { - const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); - if (setConflicts.count(hashAncestor)) - { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-spends-conflicting-tx", - strprintf("%s spends conflicting transaction %s", - hash.ToString(), - hashAncestor.ToString())); - } + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-spends-conflicting-tx", + strprintf("%s spends conflicting transaction %s", + hash.ToString(), + hashAncestor.ToString())); } + } - // Check if it's economically rational to mine this transaction rather - // than the ones it replaces. - CAmount nConflictingFees = 0; - size_t nConflictingSize = 0; - uint64_t nConflictingCount = 0; - CTxMemPool::setEntries allConflicting; - - // If we don't hold the lock allConflicting might be incomplete; the - // subsequent RemoveStaged() and addUnchecked() calls don't guarantee - // mempool consistency for us. - const bool fReplacementTransaction = setConflicts.size(); - if (fReplacementTransaction) - { - CFeeRate newFeeRate(nModifiedFees, nSize); - std::set<uint256> setConflictsParents; - const int maxDescendantsToVisit = 100; - for (const auto& mi : setIterConflicting) { - // Don't allow the replacement to reduce the feerate of the - // mempool. - // - // We usually don't want to accept replacements with lower - // feerates than what they replaced as that would lower the - // feerate of the next block. Requiring that the feerate always - // be increased is also an easy-to-reason about way to prevent - // DoS attacks via replacements. - // - // We only consider the feerates of transactions being directly - // replaced, not their indirect descendants. While that does - // mean high feerate children are ignored when deciding whether - // or not to replace, we do require the replacement to pay more - // overall fees too, mitigating most cases. - CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize()); - if (newFeeRate <= oldFeeRate) - { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", - strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", - hash.ToString(), - newFeeRate.ToString(), - oldFeeRate.ToString())); - } - - for (const CTxIn &txin : mi->GetTx().vin) - { - setConflictsParents.insert(txin.prevout.hash); - } + // Check if it's economically rational to mine this transaction rather + // than the ones it replaces. + nConflictingFees = 0; + nConflictingSize = 0; + uint64_t nConflictingCount = 0; - nConflictingCount += mi->GetCountWithDescendants(); - } - // This potentially overestimates the number of actual descendants - // but we just want to be conservative to avoid doing too much - // work. - if (nConflictingCount <= maxDescendantsToVisit) { - // If not too many to replace, then calculate the set of - // transactions that would have to be evicted - for (CTxMemPool::txiter it : setIterConflicting) { - pool.CalculateDescendants(it, allConflicting); - } - for (CTxMemPool::txiter it : allConflicting) { - nConflictingFees += it->GetModifiedFee(); - nConflictingSize += it->GetTxSize(); - } - } else { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too many potential replacements", - strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", + // If we don't hold the lock allConflicting might be incomplete; the + // subsequent RemoveStaged() and addUnchecked() calls don't guarantee + // mempool consistency for us. + fReplacementTransaction = setConflicts.size(); + if (fReplacementTransaction) + { + CFeeRate newFeeRate(nModifiedFees, nSize); + std::set<uint256> setConflictsParents; + const int maxDescendantsToVisit = 100; + for (const auto& mi : setIterConflicting) { + // Don't allow the replacement to reduce the feerate of the + // mempool. + // + // We usually don't want to accept replacements with lower + // feerates than what they replaced as that would lower the + // feerate of the next block. Requiring that the feerate always + // be increased is also an easy-to-reason about way to prevent + // DoS attacks via replacements. + // + // We only consider the feerates of transactions being directly + // replaced, not their indirect descendants. While that does + // mean high feerate children are ignored when deciding whether + // or not to replace, we do require the replacement to pay more + // overall fees too, mitigating most cases. + CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize()); + if (newFeeRate <= oldFeeRate) + { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", + strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", hash.ToString(), - nConflictingCount, - maxDescendantsToVisit)); + newFeeRate.ToString(), + oldFeeRate.ToString())); } - for (unsigned int j = 0; j < tx.vin.size(); j++) + for (const CTxIn &txin : mi->GetTx().vin) { - // We don't want to accept replacements that require low - // feerate junk to be mined first. Ideally we'd keep track of - // the ancestor feerates and make the decision based on that, - // but for now requiring all new inputs to be confirmed works. - // - // Note that if you relax this to make RBF a little more useful, - // this may break the CalculateMempoolAncestors RBF relaxation, - // above. See the comment above the first CalculateMempoolAncestors - // call for more info. - if (!setConflictsParents.count(tx.vin[j].prevout.hash)) - { - // Rather than check the UTXO set - potentially expensive - - // it's cheaper to just check if the new input refers to a - // tx that's in the mempool. - if (pool.exists(tx.vin[j].prevout.hash)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "replacement-adds-unconfirmed", - strprintf("replacement %s adds unconfirmed input, idx %d", - hash.ToString(), j)); - } - } + setConflictsParents.insert(txin.prevout.hash); } - // The replacement must pay greater fees than the transactions it - // replaces - if we did the bandwidth used by those conflicting - // transactions would not be paid for. - if (nModifiedFees < nConflictingFees) - { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", - strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", - hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees))); + nConflictingCount += mi->GetCountWithDescendants(); + } + // This potentially overestimates the number of actual descendants + // but we just want to be conservative to avoid doing too much + // work. + if (nConflictingCount <= maxDescendantsToVisit) { + // If not too many to replace, then calculate the set of + // transactions that would have to be evicted + for (CTxMemPool::txiter it : setIterConflicting) { + m_pool.CalculateDescendants(it, allConflicting); } - - // Finally in addition to paying more fees than the conflicts the - // new transaction must pay for its own bandwidth. - CAmount nDeltaFees = nModifiedFees - nConflictingFees; - if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) - { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", - strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", - hash.ToString(), - FormatMoney(nDeltaFees), - FormatMoney(::incrementalRelayFee.GetFee(nSize)))); + for (CTxMemPool::txiter it : allConflicting) { + nConflictingFees += it->GetModifiedFee(); + nConflictingSize += it->GetTxSize(); } + } else { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too many potential replacements", + strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", + hash.ToString(), + nConflictingCount, + maxDescendantsToVisit)); } - constexpr unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; - - // Check against previous transactions - // The first loop above does all the inexpensive checks. - // Only if ALL inputs pass do we perform expensive ECDSA signature checks. - // Helps prevent CPU exhaustion denial-of-service attacks. - PrecomputedTransactionData txdata(tx); - if (!CheckInputs(tx, state, view, scriptVerifyFlags, true, false, txdata)) { - // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we - // need to turn both off, and compare against just turning off CLEANSTACK - // to see if the failure is specifically due to witness validation. - CValidationState stateDummy; // Want reported failures to be from first CheckInputs - if (!tx.HasWitness() && CheckInputs(tx, stateDummy, view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && - !CheckInputs(tx, stateDummy, view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { - // Only the witness is missing, so the transaction itself may be fine. - state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, - state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); + for (unsigned int j = 0; j < tx.vin.size(); j++) + { + // We don't want to accept replacements that require low + // feerate junk to be mined first. Ideally we'd keep track of + // the ancestor feerates and make the decision based on that, + // but for now requiring all new inputs to be confirmed works. + // + // Note that if you relax this to make RBF a little more useful, + // this may break the CalculateMempoolAncestors RBF relaxation, + // above. See the comment above the first CalculateMempoolAncestors + // call for more info. + if (!setConflictsParents.count(tx.vin[j].prevout.hash)) + { + // Rather than check the UTXO set - potentially expensive - + // it's cheaper to just check if the new input refers to a + // tx that's in the mempool. + if (m_pool.exists(tx.vin[j].prevout.hash)) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "replacement-adds-unconfirmed", + strprintf("replacement %s adds unconfirmed input, idx %d", + hash.ToString(), j)); + } } - assert(IsTransactionReason(state.GetReason())); - return false; // state filled in by CheckInputs - } - - // Check again against the current block tip's script verification - // flags to cache our script execution flags. This is, of course, - // useless if the next block has different script flags from the - // previous one, but because the cache tracks script flags for us it - // will auto-invalidate and we'll just have a few blocks of extra - // misses on soft-fork activation. - // - // This is also useful in case of bugs in the standard flags that cause - // transactions to pass as valid when they're actually invalid. For - // instance the STRICTENC flag was incorrectly allowing certain - // CHECKSIG NOT scripts to pass, even though they were invalid. - // - // There is a similar check in CreateNewBlock() to prevent creating - // invalid blocks (using TestBlockValidity), however allowing such - // transactions into the mempool can be exploited as a DoS attack. - unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(::ChainActive().Tip(), chainparams.GetConsensus()); - if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, currentBlockScriptVerifyFlags, true, txdata)) { - return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed against latest-block but not STANDARD flags %s, %s", - __func__, hash.ToString(), FormatStateMessage(state)); } - if (test_accept) { - // Tx was accepted, but not added - return true; + // The replacement must pay greater fees than the transactions it + // replaces - if we did the bandwidth used by those conflicting + // transactions would not be paid for. + if (nModifiedFees < nConflictingFees) + { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", + strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", + hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees))); } - // Remove conflicting transactions from the mempool - for (CTxMemPool::txiter it : allConflicting) + // Finally in addition to paying more fees than the conflicts the + // new transaction must pay for its own bandwidth. + CAmount nDeltaFees = nModifiedFees - nConflictingFees; + if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) { - LogPrint(BCLog::MEMPOOL, "replacing tx %s with %s for %s BTC additional fees, %d delta bytes\n", - it->GetTx().GetHash().ToString(), - hash.ToString(), - FormatMoney(nModifiedFees - nConflictingFees), - (int)nSize - (int)nConflictingSize); - if (plTxnReplaced) - plTxnReplaced->push_back(it->GetSharedTx()); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", + strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", + hash.ToString(), + FormatMoney(nDeltaFees), + FormatMoney(::incrementalRelayFee.GetFee(nSize)))); } - pool.RemoveStaged(allConflicting, false, MemPoolRemovalReason::REPLACED); + } + return true; +} + +bool MemPoolAccept::PolicyScriptChecks(ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata) +{ + const CTransaction& tx = *ws.m_ptx; - // This transaction should only count for fee estimation if: - // - it isn't a BIP 125 replacement transaction (may not be widely supported) - // - it's not being re-added during a reorg which bypasses typical mempool fee limits - // - the node is not behind - // - the transaction is not dependent on any other transactions in the mempool - bool validForFeeEstimation = !fReplacementTransaction && !bypass_limits && IsCurrentForFeeEstimation() && pool.HasNoInputsOf(tx); + CValidationState &state = args.m_state; - // Store transaction in memory - pool.addUnchecked(entry, setAncestors, validForFeeEstimation); + constexpr unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; - // trim mempool and check if tx was trimmed - if (!bypass_limits) { - LimitMempoolSize(pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); - if (!pool.exists(hash)) - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool full"); + // Check against previous transactions + // This is done last to help prevent CPU exhaustion denial-of-service attacks. + if (!CheckInputs(tx, state, m_view, scriptVerifyFlags, true, false, txdata)) { + // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we + // need to turn both off, and compare against just turning off CLEANSTACK + // to see if the failure is specifically due to witness validation. + CValidationState stateDummy; // Want reported failures to be from first CheckInputs + if (!tx.HasWitness() && CheckInputs(tx, stateDummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && + !CheckInputs(tx, stateDummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { + // Only the witness is missing, so the transaction itself may be fine. + state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, + state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); } + assert(IsTransactionReason(state.GetReason())); + return false; // state filled in by CheckInputs + } + + return true; +} + +bool MemPoolAccept::ConsensusScriptChecks(ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata) +{ + const CTransaction& tx = *ws.m_ptx; + const uint256& hash = ws.m_hash; + + CValidationState &state = args.m_state; + const CChainParams& chainparams = args.m_chainparams; + + // Check again against the current block tip's script verification + // flags to cache our script execution flags. This is, of course, + // useless if the next block has different script flags from the + // previous one, but because the cache tracks script flags for us it + // will auto-invalidate and we'll just have a few blocks of extra + // misses on soft-fork activation. + // + // This is also useful in case of bugs in the standard flags that cause + // transactions to pass as valid when they're actually invalid. For + // instance the STRICTENC flag was incorrectly allowing certain + // CHECKSIG NOT scripts to pass, even though they were invalid. + // + // There is a similar check in CreateNewBlock() to prevent creating + // invalid blocks (using TestBlockValidity), however allowing such + // transactions into the mempool can be exploited as a DoS attack. + unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(::ChainActive().Tip(), chainparams.GetConsensus()); + if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, true, txdata)) { + return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed against latest-block but not STANDARD flags %s, %s", + __func__, hash.ToString(), FormatStateMessage(state)); + } + + return true; +} + +bool MemPoolAccept::Finalize(ATMPArgs& args, Workspace& ws) +{ + const CTransaction& tx = *ws.m_ptx; + const uint256& hash = ws.m_hash; + CValidationState &state = args.m_state; + const bool bypass_limits = args.m_bypass_limits; + + CTxMemPool::setEntries& allConflicting = ws.m_all_conflicting; + CTxMemPool::setEntries& setAncestors = ws.m_ancestors; + const CAmount& nModifiedFees = ws.m_modified_fees; + const CAmount& nConflictingFees = ws.m_conflicting_fees; + const size_t& nConflictingSize = ws.m_conflicting_size; + const bool fReplacementTransaction = ws.m_replacement_transaction; + std::unique_ptr<CTxMemPoolEntry>& entry = ws.m_entry; + + // Remove conflicting transactions from the mempool + for (CTxMemPool::txiter it : allConflicting) + { + LogPrint(BCLog::MEMPOOL, "replacing tx %s with %s for %s BTC additional fees, %d delta bytes\n", + it->GetTx().GetHash().ToString(), + hash.ToString(), + FormatMoney(nModifiedFees - nConflictingFees), + (int)entry->GetTxSize() - (int)nConflictingSize); + if (args.m_replaced_transactions) + args.m_replaced_transactions->push_back(it->GetSharedTx()); + } + m_pool.RemoveStaged(allConflicting, false, MemPoolRemovalReason::REPLACED); + + // This transaction should only count for fee estimation if: + // - it isn't a BIP 125 replacement transaction (may not be widely supported) + // - it's not being re-added during a reorg which bypasses typical mempool fee limits + // - the node is not behind + // - the transaction is not dependent on any other transactions in the mempool + bool validForFeeEstimation = !fReplacementTransaction && !bypass_limits && IsCurrentForFeeEstimation() && m_pool.HasNoInputsOf(tx); + + // Store transaction in memory + m_pool.addUnchecked(*entry, setAncestors, validForFeeEstimation); + + // trim mempool and check if tx was trimmed + if (!bypass_limits) { + LimitMempoolSize(m_pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); + if (!m_pool.exists(hash)) + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool full"); } + return true; +} + +bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs& args) +{ + AssertLockHeld(cs_main); + LOCK(m_pool.cs); // mempool "read lock" (held through GetMainSignals().TransactionAddedToMempool()) + + Workspace workspace(ptx); + + if (!PreChecks(args, workspace)) return false; + + // Only compute the precomputed transaction data if we need to verify + // scripts (ie, other policy checks pass). We perform the inexpensive + // checks first and avoid hashing and signature verification unless those + // checks pass, to mitigate CPU exhaustion denial-of-service attacks. + PrecomputedTransactionData txdata(*ptx); + + if (!PolicyScriptChecks(args, workspace, txdata)) return false; + + if (!ConsensusScriptChecks(args, workspace, txdata)) return false; + + // Tx was accepted, but not added + if (args.m_test_accept) return true; + + if (!Finalize(args, workspace)) return false; GetMainSignals().TransactionAddedToMempool(ptx); return true; } +} // anon namespace + /** (try to) add transaction to memory pool with a specified acceptance time **/ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::vector<COutPoint> coins_to_uncache; - bool res = AcceptToMemoryPoolWorker(chainparams, pool, state, tx, pfMissingInputs, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept); + MemPoolAccept::ATMPArgs args { chainparams, state, pfMissingInputs, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept }; + bool res = MemPoolAccept(pool).AcceptSingleTransaction(tx, args); if (!res) { // Remove coins that were not present in the coins cache before calling ATMPW; // this is to prevent memory DoS in case we receive a large number of @@ -1828,7 +1982,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // If such overwrites are allowed, coinbases and transactions depending upon those // can be duplicated to remove the ability to spend the first instance -- even after // being sent to another address. - // See BIP30 and http://r6.ca/blog/20120206T005236Z.html for more information. + // See BIP30, CVE-2012-1909, and http://r6.ca/blog/20120206T005236Z.html for more information. // This logic is not necessary for memory pool transactions, as AcceptToMemoryPool // already refuses previously-known transaction ids entirely. // This rule was originally applied to all blocks with a timestamp after March 15, 2012, 0:00 UTC. @@ -3100,6 +3254,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-multiple", "more than one coinbase"); // Check transactions + // Must check for duplicate inputs (see CVE-2018-17144) for (const auto& tx : block.vtx) if (!CheckTransaction(*tx, state, true)) return state.Invalid(state.GetReason(), false, state.GetRejectCode(), state.GetRejectReason(), diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py index 454eb583f7..fd69bbd2c7 100644 --- a/test/functional/data/invalid_txs.py +++ b/test/functional/data/invalid_txs.py @@ -24,7 +24,24 @@ import abc from test_framework.messages import CTransaction, CTxIn, CTxOut, COutPoint from test_framework import script as sc from test_framework.blocktools import create_tx_with_script, MAX_BLOCK_SIGOPS - +from test_framework.script import ( + CScript, + OP_CAT, + OP_SUBSTR, + OP_LEFT, + OP_RIGHT, + OP_INVERT, + OP_AND, + OP_OR, + OP_XOR, + OP_2MUL, + OP_2DIV, + OP_MUL, + OP_DIV, + OP_MOD, + OP_LSHIFT, + OP_RSHIFT +) basic_p2sh = sc.CScript([sc.OP_HASH160, sc.hash160(sc.CScript([sc.OP_0])), sc.OP_EQUAL]) @@ -82,6 +99,8 @@ class InputMissing(BadTxTemplate): return tx +# The following check prevents exploit of lack of merkle +# tree depth commitment (CVE-2017-12842) class SizeTooSmall(BadTxTemplate): reject_reason = "tx-size-small" expect_disconnect = False @@ -178,7 +197,44 @@ class TooManySigops(BadTxTemplate): script_pub_key=lotsa_checksigs, amount=1) +def getDisabledOpcodeTemplate(opcode): + """ Creates disabled opcode tx template class""" + def get_tx(self): + tx = CTransaction() + vin = self.valid_txin + vin.scriptSig = CScript([opcode]) + tx.vin.append(vin) + tx.vout.append(CTxOut(1, basic_p2sh)) + tx.calc_sha256() + return tx + + return type('DisabledOpcode_' + str(opcode), (BadTxTemplate,), { + 'reject_reason': "disabled opcode", + 'expect_disconnect': True, + 'get_tx': get_tx, + 'valid_in_block' : True + }) + +# Disabled opcode tx templates (CVE-2010-5137) +DisabledOpcodeTemplates = [getDisabledOpcodeTemplate(opcode) for opcode in [ + OP_CAT, + OP_SUBSTR, + OP_LEFT, + OP_RIGHT, + OP_INVERT, + OP_AND, + OP_OR, + OP_XOR, + OP_2MUL, + OP_2DIV, + OP_MUL, + OP_DIV, + OP_MOD, + OP_LSHIFT, + OP_RSHIFT]] + def iter_all_templates(): """Iterate through all bad transaction template types.""" return BadTxTemplate.__subclasses__() + diff --git a/test/functional/feature_assumevalid.py b/test/functional/feature_assumevalid.py index 420a3a7688..1b434c4485 100755 --- a/test/functional/feature_assumevalid.py +++ b/test/functional/feature_assumevalid.py @@ -40,7 +40,7 @@ from test_framework.messages import ( CTxIn, CTxOut, msg_block, - msg_headers + msg_headers, ) from test_framework.mininode import P2PInterface from test_framework.script import (CScript, OP_TRUE) @@ -180,7 +180,7 @@ class AssumeValidTest(BitcoinTestFramework): for i in range(2202): p2p1.send_message(msg_block(self.blocks[i])) # Syncing 2200 blocks can take a while on slow systems. Give it plenty of time to sync. - p2p1.sync_with_ping(200) + p2p1.sync_with_ping(960) assert_equal(self.nodes[1].getblock(self.nodes[1].getbestblockhash())['height'], 2202) # Send blocks to node2. Block 102 will be rejected. diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 12a52935ef..c74270febc 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -806,7 +806,7 @@ class FullBlockTest(BitcoinTestFramework): # # Blocks are not allowed to contain a transaction whose id matches that of an earlier, # not-fully-spent transaction in the same chain. To test, make identical coinbases; - # the second one should be rejected. + # the second one should be rejected. See also CVE-2012-1909. # self.log.info("Reject a block with a transaction with a duplicate hash of a previous transaction (BIP30)") self.move_tip(60) @@ -1261,7 +1261,7 @@ class FullBlockTest(BitcoinTestFramework): self.save_spendable_output() spend = self.get_spendable_output() - self.send_blocks(blocks, True, timeout=480) + self.send_blocks(blocks, True, timeout=960) chain1_tip = i # now create alt chain of same length @@ -1273,14 +1273,14 @@ class FullBlockTest(BitcoinTestFramework): # extend alt chain to trigger re-org block = self.next_block("alt" + str(chain1_tip + 1), version=4) - self.send_blocks([block], True, timeout=480) + self.send_blocks([block], True, timeout=960) # ... and re-org back to the first chain self.move_tip(chain1_tip) block = self.next_block(chain1_tip + 1, version=4) self.send_blocks([block], False, force_send=True) block = self.next_block(chain1_tip + 2, version=4) - self.send_blocks([block], True, timeout=480) + self.send_blocks([block], True, timeout=960) self.log.info("Reject a block with an invalid block header version") b_v1 = self.next_block('b_v1', version=1) diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index b86f6af4ca..6bd6bb5b8c 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -50,7 +50,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = False - self.rpc_timeout = 180 + self.rpc_timeout = 480 # Set -maxmempool=0 to turn off mempool memory sharing with dbcache # Set -rpcservertimeout=900 to reduce socket disconnects in this diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index 13cf951550..da00b773ad 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -7,7 +7,11 @@ import os from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, wait_until, connect_nodes_bi +from test_framework.util import ( + assert_equal, + wait_until, + connect_nodes, +) class NotificationsTest(BitcoinTestFramework): @@ -58,7 +62,7 @@ class NotificationsTest(BitcoinTestFramework): self.log.info("test -walletnotify after rescan") # restart node to rescan to force wallet notifications self.start_node(1) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index 727f4b9589..51523f13e7 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -92,6 +92,7 @@ class PruneTest(BitcoinTestFramework): ["-maxreceivebuffer=20000"], ["-prune=550"], ] + self.rpc_timeout = 120 def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index b9db618575..d2826dd1b7 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -257,7 +257,7 @@ class SegWitTest(BitcoinTestFramework): tx.vin.append(CTxIn(COutPoint(int(txid2, 16), 0), b"")) tx.vout.append(CTxOut(int(49.95 * COIN), CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE]))) # Huge fee tx.calc_sha256() - txid3 = self.nodes[0].sendrawtransaction(ToHex(tx)) + txid3 = self.nodes[0].sendrawtransaction(ToHex(tx), 0) assert tx.wit.is_null() assert txid3 in self.nodes[0].getrawmempool() diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 209a222004..dee7a04516 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -183,6 +183,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): self.check_mempool_result( result_expected=[{'txid': tx.rehash(), 'allowed': True}], rawtxs=[tx.serialize().hex()], + maxfeerate=0, ) self.log.info('A transaction with no outputs') @@ -211,6 +212,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): rawtxs=[tx.serialize().hex()], ) + # The following two validations prevent overflow of the output amounts (see CVE-2010-5139). self.log.info('A transaction with too large output value') tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference))) tx.vout[0].nValue = 21000000 * COIN + 1 diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index 788aabc192..f9231614ce 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -27,7 +27,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, ) from test_framework.script import CScriptNum @@ -54,7 +54,7 @@ class MiningTest(BitcoinTestFramework): assert_equal(mining_info['currentblocktx'], 0) assert_equal(mining_info['currentblockweight'], 4000) self.restart_node(0) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) def run_test(self): self.mine_chain() diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index 1b11a2a294..23dea4b729 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -9,7 +9,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, wait_until, ) @@ -18,6 +18,10 @@ class DisconnectBanTest(BitcoinTestFramework): self.num_nodes = 2 def run_test(self): + self.log.info("Connect nodes both way") + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 0) + self.log.info("Test setban and listbanned RPCs") self.log.info("setban: successfully ban single IP address") @@ -74,7 +78,9 @@ class DisconnectBanTest(BitcoinTestFramework): # Clear ban lists self.nodes[1].clearbanned() - connect_nodes_bi(self.nodes, 0, 1) + self.log.info("Connect nodes both way") + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 0) self.log.info("Test disconnectnode RPCs") @@ -93,7 +99,7 @@ class DisconnectBanTest(BitcoinTestFramework): assert not [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1] self.log.info("disconnectnode: successfully reconnect node") - connect_nodes_bi(self.nodes, 0, 1) # reconnect the node + connect_nodes(self.nodes[0], 1) # reconnect the node assert_equal(len(self.nodes[0].getpeerinfo()), 2) assert [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1] diff --git a/test/functional/p2p_invalid_block.py b/test/functional/p2p_invalid_block.py index 1e0b876593..905534b862 100755 --- a/test/functional/p2p_invalid_block.py +++ b/test/functional/p2p_invalid_block.py @@ -53,10 +53,11 @@ class InvalidBlockRequestTest(BitcoinTestFramework): block_time = best_block["time"] + 1 # Use merkle-root malleability to generate an invalid block with - # same blockheader. + # same blockheader (CVE-2012-2459). # Manufacture a block with 3 transactions (coinbase, spend of prior # coinbase, spend of that spend). Duplicate the 3rd transaction to # leave merkle root and blockheader unchanged but invalidate the block. + # For more information on merkle-root malleability see src/consensus/merkle.cpp. self.log.info("Test merkle root malleability.") block2 = create_block(tip, create_coinbase(height), block_time) @@ -81,15 +82,16 @@ class InvalidBlockRequestTest(BitcoinTestFramework): node.p2p.send_blocks_and_test([block2], node, success=False, reject_reason='bad-txns-duplicate') - # Check transactions for duplicate inputs + # Check transactions for duplicate inputs (CVE-2018-17144) self.log.info("Test duplicate input block.") - block2_orig.vtx[2].vin.append(block2_orig.vtx[2].vin[0]) - block2_orig.vtx[2].rehash() - block2_orig.hashMerkleRoot = block2_orig.calc_merkle_root() - block2_orig.rehash() - block2_orig.solve() - node.p2p.send_blocks_and_test([block2_orig], node, success=False, reject_reason='bad-txns-inputs-duplicate') + block2_dup = copy.deepcopy(block2_orig) + block2_dup.vtx[2].vin.append(block2_dup.vtx[2].vin[0]) + block2_dup.vtx[2].rehash() + block2_dup.hashMerkleRoot = block2_dup.calc_merkle_root() + block2_dup.rehash() + block2_dup.solve() + node.p2p.send_blocks_and_test([block2_dup], node, success=False, reject_reason='bad-txns-inputs-duplicate') self.log.info("Test very broken block.") @@ -105,5 +107,31 @@ class InvalidBlockRequestTest(BitcoinTestFramework): node.p2p.send_blocks_and_test([block3], node, success=False, reject_reason='bad-cb-amount') + # Complete testing of CVE-2012-2459 by sending the original block. + # It should be accepted even though it has the same hash as the mutated one. + + self.log.info("Test accepting original block after rejecting its mutated version.") + node.p2p.send_blocks_and_test([block2_orig], node, success=True, timeout=5) + + # Update tip info + height += 1 + block_time += 1 + tip = int(block2_orig.hash, 16) + + # Complete testing of CVE-2018-17144, by checking for the inflation bug. + # Create a block that spends the output of a tx in a previous block. + block4 = create_block(tip, create_coinbase(height), block_time) + tx3 = create_tx_with_script(tx2, 0, script_sig=b'\x51', amount=50 * COIN) + + # Duplicates input + tx3.vin.append(tx3.vin[0]) + tx3.rehash() + block4.vtx.append(tx3) + block4.hashMerkleRoot = block4.calc_merkle_root() + block4.rehash() + block4.solve() + self.log.info("Test inflation by duplicating input") + node.p2p.send_blocks_and_test([block4], node, success=False, reject_reason='bad-txns-inputs-duplicate') + if __name__ == '__main__': InvalidBlockRequestTest().main() diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index d5c68e622b..58c72f89d8 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -85,7 +85,7 @@ class InvalidMessagesTest(BitcoinTestFramework): # Peer 1, despite serving up a bunch of nonsense, should still be connected. self.log.info("Waiting for node to drop junk messages.") - node.p2p.sync_with_ping(timeout=120) + node.p2p.sync_with_ping(timeout=320) assert node.p2p.is_connected # diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py index a4650df8ee..e6451d9f18 100755 --- a/test/functional/p2p_node_network_limited.py +++ b/test/functional/p2p_node_network_limited.py @@ -14,7 +14,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, disconnect_nodes, - connect_nodes_bi, + connect_nodes, wait_until, ) @@ -64,7 +64,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): assert_equal(int(self.nodes[0].getnetworkinfo()['localservices'], 16), expected_services) self.log.info("Mine enough blocks to reach the NODE_NETWORK_LIMITED range.") - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) blocks = self.nodes[1].generatetoaddress(292, self.nodes[1].get_deterministic_priv_key().address) self.sync_blocks([self.nodes[0], self.nodes[1]]) @@ -90,7 +90,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): # connect unsynced node 2 with pruned NODE_NETWORK_LIMITED peer # because node 2 is in IBD and node 0 is a NODE_NETWORK_LIMITED peer, sync must not be possible - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 2) try: self.sync_blocks([self.nodes[0], self.nodes[2]], timeout=5) except: @@ -99,7 +99,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): assert_equal(self.nodes[2].getblockheader(self.nodes[2].getbestblockhash())['height'], 0) # now connect also to node 1 (non pruned) - connect_nodes_bi(self.nodes, 1, 2) + connect_nodes(self.nodes[1], 2) # sync must be possible self.sync_blocks() @@ -111,7 +111,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): self.nodes[0].generatetoaddress(10, self.nodes[0].get_deterministic_priv_key().address) # connect node1 (non pruned) with node0 (pruned) and check if the can sync - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) # sync must be possible, node 1 is no longer in IBD and should therefore connect to node 0 (NODE_NETWORK_LIMITED) self.sync_blocks([self.nodes[0], self.nodes[1]]) diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py index 19d78ff303..aada04f66f 100755 --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -121,6 +121,7 @@ class TxDownloadTest(BitcoinTestFramework): # peer, plus # * the first time it is re-requested from the outbound peer, plus # * 2 seconds to avoid races + assert self.nodes[1].getpeerinfo()[0]['inbound'] == False timeout = 2 + (MAX_GETDATA_RANDOM_DELAY + INBOUND_PEER_TX_DELAY) + ( GETDATA_TX_INTERVAL + MAX_GETDATA_RANDOM_DELAY) self.log.info("Tx should be received at node 1 after {} seconds".format(timeout)) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index b621081752..c956af1cbe 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -12,7 +12,7 @@ from test_framework.util import ( assert_greater_than, assert_greater_than_or_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, count_bytes, find_vout_for_address, ) @@ -35,10 +35,10 @@ class RawTransactionsTest(BitcoinTestFramework): def setup_network(self): self.setup_nodes() - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 1, 2) - connect_nodes_bi(self.nodes, 0, 2) - connect_nodes_bi(self.nodes, 0, 3) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) + connect_nodes(self.nodes[0], 3) def run_test(self): self.min_relay_tx_fee = self.nodes[0].getnetworkinfo()['relayfee'] @@ -508,10 +508,10 @@ class RawTransactionsTest(BitcoinTestFramework): for node in self.nodes: node.settxfee(self.min_relay_tx_fee) - connect_nodes_bi(self.nodes,0,1) - connect_nodes_bi(self.nodes,1,2) - connect_nodes_bi(self.nodes,0,2) - connect_nodes_bi(self.nodes,0,3) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) + connect_nodes(self.nodes[0], 3) # Again lock the watchonly UTXO or nodes[0] may spend it, because # lockunspent is memory-only and thus lost on restart self.nodes[0].lockunspent(False, [{"txid": self.watchonly_txid, "vout": self.watchonly_vout}]) diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 104c66aaa7..e24bf3111b 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -15,7 +15,7 @@ from test_framework.util import ( assert_greater_than_or_equal, assert_greater_than, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, p2p_port, wait_until, ) @@ -54,6 +54,10 @@ class NetTest(BitcoinTestFramework): self.extra_args = [["-minrelaytxfee=0.00001000"],["-minrelaytxfee=0.00000500"]] def run_test(self): + self.log.info('Connect nodes both way') + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 0) + self._test_connection_count() self._test_getnettotals() self._test_getnetworkinfo() @@ -62,7 +66,7 @@ class NetTest(BitcoinTestFramework): self._test_getnodeaddresses() def _test_connection_count(self): - # connect_nodes_bi connects each node to the other + # connect_nodes connects each node to the other assert_equal(self.nodes[0].getconnectioncount(), 2) def _test_getnettotals(self): @@ -105,7 +109,10 @@ class NetTest(BitcoinTestFramework): wait_until(lambda: self.nodes[0].getnetworkinfo()['connections'] == 0, timeout=3) self.nodes[0].setnetworkactive(state=True) - connect_nodes_bi(self.nodes, 0, 1) + self.log.info('Connect nodes both way') + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 0) + assert_equal(self.nodes[0].getnetworkinfo()['networkactive'], True) assert_equal(self.nodes[0].getnetworkinfo()['connections'], 2) diff --git a/test/functional/rpc_preciousblock.py b/test/functional/rpc_preciousblock.py index 2d5631bb27..0663ffdf5b 100755 --- a/test/functional/rpc_preciousblock.py +++ b/test/functional/rpc_preciousblock.py @@ -7,7 +7,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - connect_nodes_bi, + connect_nodes, ) def unidirectional_node_sync_via_rpc(node_src, node_dest): @@ -60,7 +60,7 @@ class PreciousTest(BitcoinTestFramework): self.log.info("Connect nodes and check no reorg occurs") # Submit competing blocks via RPC so any reorg should occur before we proceed (no way to wait on inaction for p2p sync) node_sync_via_rpc(self.nodes[0:2]) - connect_nodes_bi(self.nodes,0,1) + connect_nodes(self.nodes[0], 1) assert_equal(self.nodes[0].getbestblockhash(), hashC) assert_equal(self.nodes[1].getbestblockhash(), hashG) self.log.info("Make Node0 prefer block G") @@ -97,8 +97,8 @@ class PreciousTest(BitcoinTestFramework): hashL = self.nodes[2].getbestblockhash() self.log.info("Connect nodes and check no reorg occurs") node_sync_via_rpc(self.nodes[1:3]) - connect_nodes_bi(self.nodes,1,2) - connect_nodes_bi(self.nodes,0,2) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) assert_equal(self.nodes[0].getbestblockhash(), hashH) assert_equal(self.nodes[1].getbestblockhash(), hashH) assert_equal(self.nodes[2].getbestblockhash(), hashL) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 5a04e0c8d8..61572654e0 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -11,7 +11,7 @@ from test_framework.util import ( assert_equal, assert_greater_than, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, disconnect_nodes, find_output, ) @@ -72,8 +72,8 @@ class PSBTTest(BitcoinTestFramework): assert_equal(online_node.gettxout(txid,0)["confirmations"], 1) # Reconnect - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[0], 2) def run_test(self): # Create and fund a raw tx for sending 10 BTC @@ -382,6 +382,16 @@ class PSBTTest(BitcoinTestFramework): joined_decoded = self.nodes[0].decodepsbt(joined) assert len(joined_decoded['inputs']) == 4 and len(joined_decoded['outputs']) == 2 and "final_scriptwitness" not in joined_decoded['inputs'][3] and "final_scriptSig" not in joined_decoded['inputs'][3] + # Check that joining shuffles the inputs and outputs + # 10 attempts should be enough to get a shuffled join + shuffled = False + for i in range(0, 10): + shuffled_joined = self.nodes[0].joinpsbts([psbt, psbt2]) + shuffled |= joined != shuffled_joined + if shuffled: + break + assert shuffled + # Newly created PSBT needs UTXOs and updating addr = self.nodes[1].getnewaddress("", "p2sh-segwit") txid = self.nodes[0].sendtoaddress(addr, 7) diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 4338675270..eb98334988 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -17,7 +17,13 @@ from decimal import Decimal from io import BytesIO from test_framework.messages import CTransaction, ToHex from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, connect_nodes_bi, hex_str_to_bytes +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, + connect_nodes, + hex_str_to_bytes, +) + class multidict(dict): """Dictionary that allows duplicate keys. @@ -53,7 +59,7 @@ class RawTransactionsTest(BitcoinTestFramework): def setup_network(self): super().setup_network() - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 2) def run_test(self): self.log.info('prepare some coins for multiple *rawtransaction commands') @@ -432,17 +438,18 @@ class RawTransactionsTest(BitcoinTestFramework): self.log.info('sendrawtransaction/testmempoolaccept with maxfeerate') + # Test a transaction with small fee txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) rawTx = self.nodes[0].getrawtransaction(txId, True) vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) self.sync_all() inputs = [{ "txid" : txId, "vout" : vout['n'] }] - outputs = { self.nodes[0].getnewaddress() : Decimal("0.99999000") } # 1000 sat fee + outputs = { self.nodes[0].getnewaddress() : Decimal("0.999990000") } # 10000 sat fee rawTx = self.nodes[2].createrawtransaction(inputs, outputs) rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) assert_equal(rawTxSigned['complete'], True) - # 1000 sat fee, ~100 b transaction, fee rate should land around 10 sat/b = 0.00010000 BTC/kB + # 10000 sat fee, ~100 b transaction, fee rate should land around 100 sat/b = 0.00100000 BTC/kB # Thus, testmempoolaccept should reject testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']], 0.00001000)[0] assert_equal(testres['allowed'], False) @@ -450,9 +457,32 @@ class RawTransactionsTest(BitcoinTestFramework): # and sendrawtransaction should throw assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000) # And below calls should both succeed - testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.00070000')[0] + testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0] + assert_equal(testres['allowed'], True) + self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex']) + + # Test a transaction with large fee + txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) + rawTx = self.nodes[0].getrawtransaction(txId, True) + vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) + + self.sync_all() + inputs = [{ "txid" : txId, "vout" : vout['n'] }] + outputs = { self.nodes[0].getnewaddress() : Decimal("0.98000000") } # 2000000 sat fee + rawTx = self.nodes[2].createrawtransaction(inputs, outputs) + rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) + assert_equal(rawTxSigned['complete'], True) + # 2000000 sat fee, ~100 b transaction, fee rate should land around 20000 sat/b = 0.20000000 BTC/kB + # Thus, testmempoolaccept should reject + testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']])[0] + assert_equal(testres['allowed'], False) + assert_equal(testres['reject-reason'], '256: absurdly-high-fee') + # and sendrawtransaction should throw + assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex']) + # And below calls should both succeed + testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.20000000')[0] assert_equal(testres['allowed'], True) - self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.00070000') + self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.20000000') if __name__ == '__main__': diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 9aff08fdc7..780aa5fe03 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -25,7 +25,7 @@ from .util import ( PortSeed, assert_equal, check_json_precision, - connect_nodes_bi, + connect_nodes, disconnect_nodes, get_datadir_path, initialize_datadir, @@ -281,8 +281,18 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # Connect the nodes as a "chain". This allows us # to split the network between nodes 1 and 2 to get # two halves that can work on competing chains. + # + # Topology looks like this: + # node0 <-- node1 <-- node2 <-- node3 + # + # If all nodes are in IBD (clean chain from genesis), node0 is assumed to be the source of blocks (miner). To + # ensure block propagation, all nodes will establish outgoing connections toward node0. + # See fPreferredDownload in net_processing. + # + # If further outbound connections are needed, they can be added at the beginning of the test with e.g. + # connect_nodes(self.nodes[1], 2) for i in range(self.num_nodes - 1): - connect_nodes_bi(self.nodes, i, i + 1) + connect_nodes(self.nodes[i + 1], i) self.sync_all() def setup_nodes(self): @@ -423,7 +433,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): """ Join the (previously split) network halves together. """ - connect_nodes_bi(self.nodes, 1, 2) + connect_nodes(self.nodes[1], 2) self.sync_all() def sync_blocks(self, nodes=None, **kwargs): diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index f9f5fe553e..3175872b00 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -377,10 +377,6 @@ def connect_nodes(from_connection, node_num): # with transaction relaying wait_until(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo())) -def connect_nodes_bi(nodes, a, b): - connect_nodes(nodes[a], b) - connect_nodes(nodes[b], a) - def sync_blocks(rpc_connections, *, wait=1, timeout=60): """ Wait until everybody has the same tip. diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 28a65f7823..355cd7af75 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -19,6 +19,7 @@ class ToolWalletTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True + self.rpc_timeout = 120 def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_address_types.py b/test/functional/wallet_address_types.py index 4e4ed8f26b..c41b31e2e1 100755 --- a/test/functional/wallet_address_types.py +++ b/test/functional/wallet_address_types.py @@ -62,9 +62,12 @@ from test_framework.util import ( assert_equal, assert_greater_than, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, +) +from test_framework.segwit_addr import ( + encode, + decode, ) - class AddressTypeTest(BitcoinTestFramework): def set_test_params(self): @@ -87,7 +90,7 @@ class AddressTypeTest(BitcoinTestFramework): # Fully mesh-connect nodes for faster mempool sync for i, j in itertools.product(range(self.num_nodes), repeat=2): if i > j: - connect_nodes_bi(self.nodes, i, j) + connect_nodes(self.nodes[i], j) self.sync_all() def get_balances(self, confirmed=True): @@ -97,6 +100,13 @@ class AddressTypeTest(BitcoinTestFramework): else: return [self.nodes[i].getunconfirmedbalance() for i in range(4)] + # Quick test of python bech32 implementation + def test_python_bech32(self, addr): + hrp = addr[:4] + assert_equal(hrp, "bcrt") + (witver, witprog) = decode(hrp, addr) + assert_equal(encode(hrp, witver, witprog), addr) + def test_address(self, node, address, multisig, typ): """Run sanity checks on an address.""" info = self.nodes[node].getaddressinfo(address) @@ -121,6 +131,7 @@ class AddressTypeTest(BitcoinTestFramework): assert_equal(info['witness_version'], 0) assert_equal(len(info['witness_program']), 40) assert 'pubkey' in info + self.test_python_bech32(info["address"]) elif typ == 'legacy': # P2SH-multisig assert info['isscript'] @@ -146,6 +157,7 @@ class AddressTypeTest(BitcoinTestFramework): assert_equal(info['witness_version'], 0) assert_equal(len(info['witness_program']), 64) assert 'pubkeys' in info + self.test_python_bech32(info["address"]) else: # Unknown type assert False diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 58ad835d39..e3aa6705e5 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -8,7 +8,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, ) # TODO: Copied from wallet_groups.py -- should perhaps move into util.py @@ -103,7 +103,7 @@ class AvoidReuseTest(BitcoinTestFramework): # Stop and restart node 1 self.stop_node(1) self.start_node(1) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) # Flags should still be node1.avoid_reuse=false, node2.avoid_reuse=true assert_equal(self.nodes[0].getwalletinfo()["avoid_reuse"], False) diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 55c517e92f..93178c5ab2 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -49,6 +49,7 @@ class WalletBackupTest(BitcoinTestFramework): self.setup_clean_chain = True # nodes 1, 2,3 are spenders, let's give them a keypool=100 self.extra_args = [["-keypool=100"], ["-keypool=100"], ["-keypool=100"], []] + self.rpc_timeout = 120 def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 137c77a51e..c50dcd987a 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -11,7 +11,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, sync_blocks, ) @@ -211,7 +211,7 @@ class WalletTest(BitcoinTestFramework): # Now confirm tx_orig self.restart_node(1, ['-persistmempool=0']) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) sync_blocks(self.nodes) self.nodes[1].sendrawtransaction(tx_orig) self.nodes[1].generatetoaddress(1, ADDRESS_WATCHONLY) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index ce04110240..5b1c672a48 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -12,7 +12,7 @@ from test_framework.util import ( assert_equal, assert_fee_amount, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, wait_until, ) @@ -32,9 +32,9 @@ class WalletTest(BitcoinTestFramework): self.setup_nodes() # Only need nodes 0-2 running at start of test self.stop_node(3) - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 1, 2) - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) self.sync_all(self.nodes[0:3]) def check_fee_amount(self, curr_balance, balance_with_fee, fee_per_byte, tx_size): @@ -218,7 +218,7 @@ class WalletTest(BitcoinTestFramework): node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) self.start_node(3) - connect_nodes_bi(self.nodes, 0, 3) + connect_nodes(self.nodes[0], 3) self.sync_all() # check if we can list zero value tx as available coins @@ -253,9 +253,9 @@ class WalletTest(BitcoinTestFramework): self.start_node(0, ["-walletbroadcast=0"]) self.start_node(1, ["-walletbroadcast=0"]) self.start_node(2, ["-walletbroadcast=0"]) - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 1, 2) - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) self.sync_all(self.nodes[0:3]) txid_not_broadcast = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2) @@ -280,9 +280,9 @@ class WalletTest(BitcoinTestFramework): self.start_node(0) self.start_node(1) self.start_node(2) - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 1, 2) - connect_nodes_bi(self.nodes, 0, 2) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[0], 2) self.sync_blocks(self.nodes[0:3]) self.nodes[0].generate(1) @@ -433,7 +433,7 @@ class WalletTest(BitcoinTestFramework): # Split into two chains rawtx = self.nodes[0].createrawtransaction([{"txid": singletxid, "vout": 0}], {chain_addrs[0]: node0_balance / 2 - Decimal('0.01'), chain_addrs[1]: node0_balance / 2 - Decimal('0.01')}) signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx) - singletxid = self.nodes[0].sendrawtransaction(signedtx["hex"]) + singletxid = self.nodes[0].sendrawtransaction(signedtx["hex"], 0) self.nodes[0].generate(1) # Make a long chain of unconfirmed payments without hitting mempool limit diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index bfc01e3f5e..a7c79ec916 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -23,7 +23,7 @@ from test_framework.util import ( assert_equal, assert_greater_than, assert_raises_rpc_error, - connect_nodes_bi, + connect_nodes, hex_str_to_bytes, ) @@ -48,7 +48,7 @@ class BumpFeeTest(BitcoinTestFramework): self.nodes[1].encryptwallet(WALLET_PASSPHRASE) self.nodes[1].walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) self.sync_all() peer_node, rbf_node = self.nodes diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index 97172d8b82..fa5d5a8878 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -10,7 +10,7 @@ import shutil from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - connect_nodes_bi, + connect_nodes, assert_raises_rpc_error ) @@ -82,7 +82,7 @@ class WalletHDTest(BitcoinTestFramework): assert_equal(hd_info_2["hdkeypath"], "m/0'/0'/"+str(i)+"'") assert_equal(hd_info_2["hdseedid"], masterkeyid) assert_equal(hd_add, hd_add_2) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) self.sync_all() # Needs rescan @@ -96,7 +96,7 @@ class WalletHDTest(BitcoinTestFramework): shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "chainstate")) shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat")) self.start_node(1, extra_args=self.extra_args[1]) - connect_nodes_bi(self.nodes, 0, 1) + connect_nodes(self.nodes[0], 1) self.sync_all() # Wallet automatically scans blocks older than key on startup assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + 1) diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py index 0014555ade..2e70a9e0a5 100755 --- a/test/functional/wallet_keypool_topup.py +++ b/test/functional/wallet_keypool_topup.py @@ -16,7 +16,7 @@ import shutil from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - connect_nodes_bi, + connect_nodes, ) @@ -38,9 +38,9 @@ class KeypoolRestoreTest(BitcoinTestFramework): self.stop_node(1) shutil.copyfile(wallet_path, wallet_backup_path) self.start_node(1, self.extra_args[1]) - connect_nodes_bi(self.nodes, 0, 1) - connect_nodes_bi(self.nodes, 0, 2) - connect_nodes_bi(self.nodes, 0, 3) + connect_nodes(self.nodes[0], 1) + connect_nodes(self.nodes[0], 2) + connect_nodes(self.nodes[0], 3) for i, output_type in enumerate(["legacy", "p2sh-segwit", "bech32"]): @@ -72,7 +72,7 @@ class KeypoolRestoreTest(BitcoinTestFramework): self.stop_node(idx) shutil.copyfile(wallet_backup_path, wallet_path) self.start_node(idx, self.extra_args[idx]) - connect_nodes_bi(self.nodes, 0, idx) + connect_nodes(self.nodes[0], idx) self.sync_all() self.log.info("Verify keypool is restored and balance is correct") diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 021a29d4ac..4aeb393255 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -5,9 +5,15 @@ """Test the listsincelast RPC.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_array_result, assert_raises_rpc_error +from test_framework.util import ( + assert_array_result, + assert_equal, + assert_raises_rpc_error, + connect_nodes, +) -class ListSinceBlockTest (BitcoinTestFramework): + +class ListSinceBlockTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True @@ -16,6 +22,9 @@ class ListSinceBlockTest (BitcoinTestFramework): self.skip_if_no_wallet() def run_test(self): + # All nodes are in IBD from genesis, so they'll need the miner (node2) to be an outbound connection, or have + # only one connection. (See fPreferredDownload in net_processing) + connect_nodes(self.nodes[1], 2) self.nodes[2].generate(101) self.sync_all() diff --git a/test/lint/lint-python-dead-code-whitelist b/test/lint/lint-python-dead-code-whitelist index 2522c8fa1c..33bb7b44fa 100644 --- a/test/lint/lint-python-dead-code-whitelist +++ b/test/lint/lint-python-dead-code-whitelist @@ -6,6 +6,7 @@ _.converter # unused attribute (test/functional/test_framework/test_framework.p _.daemon # unused attribute (test/functional/test_framework/socks5.py) data_received # unused function (test/functional/test_framework/mininode.py) DuplicateInput # unused class (test/functional/data/invalid_txs.py) +DisabledOpcodeTemplates # unused class (test/functional/data/invalid_txs.py) _.filename # unused attribute (contrib/macdeploy/custom_dsstore.py) InvalidOPIFConstruction # unused class (test/functional/data/invalid_txs.py) _.is_compressed # unused property (test/functional/test_framework/key.py) |