diff options
| -rw-r--r-- | configure.ac | 67 | ||||
| -rw-r--r-- | doc/dependencies.md | 2 | ||||
| -rw-r--r-- | src/consensus/validation.h | 3 | ||||
| -rw-r--r-- | src/interfaces/node.cpp | 74 | ||||
| -rw-r--r-- | src/interfaces/node.h | 6 | ||||
| -rw-r--r-- | src/net_processing.cpp | 35 | ||||
| -rw-r--r-- | src/policy/policy.cpp | 8 | ||||
| -rw-r--r-- | src/qt/bitcoin.cpp | 4 | ||||
| -rw-r--r-- | src/qt/test/addressbooktests.cpp | 2 | ||||
| -rw-r--r-- | src/qt/test/test_main.cpp | 3 | ||||
| -rw-r--r-- | src/qt/test/wallettests.cpp | 3 | ||||
| -rw-r--r-- | src/test/util/setup_common.cpp | 4 | ||||
| -rw-r--r-- | src/validation.cpp | 5 | ||||
| -rwxr-xr-x | test/functional/p2p_segwit.py | 12 |
14 files changed, 117 insertions, 111 deletions
diff --git a/configure.ac b/configure.ac index 013e68a319..2381c5dd08 100644 --- a/configure.ac +++ b/configure.ac @@ -1180,9 +1180,9 @@ fi if test x$use_boost = xyes; then dnl Minimum required Boost version -define(MINIMUM_REQUIRED_BOOST, 1.47.0) +define(MINIMUM_REQUIRED_BOOST, 1.58.0) -dnl Check for boost libs +dnl Check for Boost libs AX_BOOST_BASE([MINIMUM_REQUIRED_BOOST]) if test x$want_boost = xno; then AC_MSG_ERROR([[only libbitcoinconsensus can be built without boost]]) @@ -1199,25 +1199,7 @@ dnl counter implementations. In 1.63 and later the std::atomic approach is defau m4_pattern_allow(DBOOST_AC_USE_STD_ATOMIC) dnl otherwise it's treated like a macro BOOST_CPPFLAGS="-DBOOST_SP_USE_STD_ATOMIC -DBOOST_AC_USE_STD_ATOMIC $BOOST_CPPFLAGS" -if test x$use_reduce_exports = xyes; then - AC_MSG_CHECKING([for working boost reduced exports]) - TEMP_CPPFLAGS="$CPPFLAGS" - CPPFLAGS="$BOOST_CPPFLAGS $CPPFLAGS" - AC_PREPROC_IFELSE([AC_LANG_PROGRAM([[ - @%:@include <boost/version.hpp> - ]], [[ - #if BOOST_VERSION >= 104900 - // Everything is okay - #else - # error Boost version is too old - #endif - ]])],[ - AC_MSG_RESULT(yes) - ],[ - AC_MSG_ERROR([boost versions < 1.49 are known to be broken with reduced exports. Use --disable-reduce-exports.]) - ]) - CPPFLAGS="$TEMP_CPPFLAGS" -fi +BOOST_LIBS="$BOOST_LDFLAGS $BOOST_SYSTEM_LIB $BOOST_FILESYSTEM_LIB $BOOST_THREAD_LIB" fi if test x$use_reduce_exports = xyes; then @@ -1231,7 +1213,6 @@ if test x$use_tests = xyes; then AC_MSG_ERROR(hexdump is required for tests) fi - if test x$use_boost = xyes; then AX_BOOST_UNIT_TEST_FRAMEWORK @@ -1257,48 +1238,6 @@ if test x$use_tests = xyes; then fi fi -if test x$use_boost = xyes; then - -BOOST_LIBS="$BOOST_LDFLAGS $BOOST_SYSTEM_LIB $BOOST_FILESYSTEM_LIB $BOOST_THREAD_LIB" - - -dnl If boost (prior to 1.57) was built without c++11, it emulated scoped enums -dnl using c++98 constructs. Unfortunately, this implementation detail leaked into -dnl the abi. This was fixed in 1.57. - -dnl When building against that installed version using c++11, the headers pick up -dnl on the native c++11 scoped enum support and enable it, however it will fail to -dnl link. This can be worked around by disabling c++11 scoped enums if linking will -dnl fail. -dnl BOOST_NO_SCOPED_ENUMS was changed to BOOST_NO_CXX11_SCOPED_ENUMS in 1.51. - -TEMP_LIBS="$LIBS" -LIBS="$BOOST_LIBS $LIBS" -TEMP_CPPFLAGS="$CPPFLAGS" -CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" -AC_MSG_CHECKING([for mismatched boost c++11 scoped enums]) -AC_LINK_IFELSE([AC_LANG_PROGRAM([[ - #include <boost/config.hpp> - #include <boost/version.hpp> - #if !defined(BOOST_NO_SCOPED_ENUMS) && !defined(BOOST_NO_CXX11_SCOPED_ENUMS) && BOOST_VERSION < 105700 - #define BOOST_NO_SCOPED_ENUMS - #define BOOST_NO_CXX11_SCOPED_ENUMS - #define CHECK - #endif - #include <boost/filesystem.hpp> - ]],[[ - #if defined(CHECK) - boost::filesystem::copy_file("foo", "bar"); - #else - choke; - #endif - ]])], - [AC_MSG_RESULT(mismatched); BOOST_CPPFLAGS="$BOOST_CPPFLAGS -DBOOST_NO_SCOPED_ENUMS -DBOOST_NO_CXX11_SCOPED_ENUMS"], [AC_MSG_RESULT(ok)]) -LIBS="$TEMP_LIBS" -CPPFLAGS="$TEMP_CPPFLAGS" - -fi - dnl libevent check if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench != xnonononono; then diff --git a/doc/dependencies.md b/doc/dependencies.md index 0cb5311e8b..92dea65309 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -6,7 +6,7 @@ These are the dependencies currently used by Bitcoin Core. You can find instruct | Dependency | Version used | Minimum required | CVEs | Shared | [Bundled Qt library](https://doc.qt.io/qt-5/configure-options.html#third-party-libraries) | | --- | --- | --- | --- | --- | --- | | Berkeley DB | [4.8.30](https://www.oracle.com/technetwork/database/database-technologies/berkeleydb/downloads/index.html) | 4.8.x | No | | | -| Boost | [1.70.0](https://www.boost.org/users/download/) | [1.47.0](https://github.com/bitcoin/bitcoin/pull/8920) | No | | | +| Boost | [1.70.0](https://www.boost.org/users/download/) | [1.58.0](https://github.com/bitcoin/bitcoin/pull/19667) | No | | | | Clang | | [3.3+](https://releases.llvm.org/download.html) (C++11 support) | | | | | Expat | [2.2.7](https://libexpat.github.io/) | | No | Yes | | | fontconfig | [2.12.1](https://www.freedesktop.org/software/fontconfig/release/) | | No | Yes | | diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 8de7a8f2d8..2a93a090d6 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -26,7 +26,8 @@ enum class TxValidationResult { * is uninteresting. */ TX_RECENT_CONSENSUS_CHANGE, - TX_NOT_STANDARD, //!< didn't meet our local policy rules + TX_INPUTS_NOT_STANDARD, //!< inputs (covered by txid) failed policy rules + TX_NOT_STANDARD, //!< otherwise didn't meet our local policy rules TX_MISSING_INPUTS, //!< transaction was missing some of its inputs TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks /** diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 33f0dac263..969767b90f 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -56,6 +56,7 @@ namespace { class NodeImpl : public Node { public: + NodeImpl(NodeContext* context) { setContext(context); } void initError(const bilingual_str& message) override { InitError(message); } bool parseParameters(int argc, const char* const argv[], std::string& error) override { @@ -81,13 +82,13 @@ public: } bool appInitMain() override { - m_context.chain = MakeChain(m_context); - return AppInitMain(m_context_ref, m_context); + m_context->chain = MakeChain(*m_context); + return AppInitMain(m_context_ref, *m_context); } void appShutdown() override { - Interrupt(m_context); - Shutdown(m_context); + Interrupt(*m_context); + Shutdown(*m_context); } void startShutdown() override { @@ -108,19 +109,19 @@ public: StopMapPort(); } } - void setupServerArgs() override { return SetupServerArgs(m_context); } + void setupServerArgs() override { return SetupServerArgs(*m_context); } bool getProxy(Network net, proxyType& proxy_info) override { return GetProxy(net, proxy_info); } size_t getNodeCount(CConnman::NumConnections flags) override { - return m_context.connman ? m_context.connman->GetNodeCount(flags) : 0; + return m_context->connman ? m_context->connman->GetNodeCount(flags) : 0; } bool getNodesStats(NodesStats& stats) override { stats.clear(); - if (m_context.connman) { + if (m_context->connman) { std::vector<CNodeStats> stats_temp; - m_context.connman->GetNodeStats(stats_temp); + m_context->connman->GetNodeStats(stats_temp); stats.reserve(stats_temp.size()); for (auto& node_stats_temp : stats_temp) { @@ -141,46 +142,46 @@ public: } bool getBanned(banmap_t& banmap) override { - if (m_context.banman) { - m_context.banman->GetBanned(banmap); + if (m_context->banman) { + m_context->banman->GetBanned(banmap); return true; } return false; } bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) override { - if (m_context.banman) { - m_context.banman->Ban(net_addr, ban_time_offset); + if (m_context->banman) { + m_context->banman->Ban(net_addr, ban_time_offset); return true; } return false; } bool unban(const CSubNet& ip) override { - if (m_context.banman) { - m_context.banman->Unban(ip); + if (m_context->banman) { + m_context->banman->Unban(ip); return true; } return false; } bool disconnectByAddress(const CNetAddr& net_addr) override { - if (m_context.connman) { - return m_context.connman->DisconnectNode(net_addr); + if (m_context->connman) { + return m_context->connman->DisconnectNode(net_addr); } return false; } bool disconnectById(NodeId id) override { - if (m_context.connman) { - return m_context.connman->DisconnectNode(id); + if (m_context->connman) { + return m_context->connman->DisconnectNode(id); } return false; } - int64_t getTotalBytesRecv() override { return m_context.connman ? m_context.connman->GetTotalBytesRecv() : 0; } - int64_t getTotalBytesSent() override { return m_context.connman ? m_context.connman->GetTotalBytesSent() : 0; } - size_t getMempoolSize() override { return m_context.mempool ? m_context.mempool->size() : 0; } - size_t getMempoolDynamicUsage() override { return m_context.mempool ? m_context.mempool->DynamicMemoryUsage() : 0; } + int64_t getTotalBytesRecv() override { return m_context->connman ? m_context->connman->GetTotalBytesRecv() : 0; } + int64_t getTotalBytesSent() override { return m_context->connman ? m_context->connman->GetTotalBytesSent() : 0; } + size_t getMempoolSize() override { return m_context->mempool ? m_context->mempool->size() : 0; } + size_t getMempoolDynamicUsage() override { return m_context->mempool ? m_context->mempool->DynamicMemoryUsage() : 0; } bool getHeaderTip(int& height, int64_t& block_time) override { LOCK(::cs_main); @@ -223,11 +224,11 @@ public: bool getImporting() override { return ::fImporting; } void setNetworkActive(bool active) override { - if (m_context.connman) { - m_context.connman->SetNetworkActive(active); + if (m_context->connman) { + m_context->connman->SetNetworkActive(active); } } - bool getNetworkActive() override { return m_context.connman && m_context.connman->GetNetworkActive(); } + bool getNetworkActive() override { return m_context->connman && m_context->connman->GetNetworkActive(); } CFeeRate estimateSmartFee(int num_blocks, bool conservative, int* returned_target = nullptr) override { FeeCalculation fee_calc; @@ -269,7 +270,7 @@ public: std::vector<std::unique_ptr<Wallet>> getWallets() override { std::vector<std::unique_ptr<Wallet>> wallets; - for (auto& client : m_context.chain_clients) { + for (auto& client : m_context->chain_clients) { auto client_wallets = client->getWallets(); std::move(client_wallets.begin(), client_wallets.end(), std::back_inserter(wallets)); } @@ -277,12 +278,12 @@ public: } std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override { - return MakeWallet(LoadWallet(*m_context.chain, name, error, warnings)); + return MakeWallet(LoadWallet(*m_context->chain, name, error, warnings)); } std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) override { std::shared_ptr<CWallet> wallet; - status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); + status = CreateWallet(*m_context->chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); return MakeWallet(wallet); } std::unique_ptr<Handler> handleInitMessage(InitMessageFn fn) override @@ -336,13 +337,22 @@ public: /* verification progress is unused when a header was received */ 0); })); } - NodeContext* context() override { return &m_context; } - NodeContext m_context; - util::Ref m_context_ref{m_context}; + NodeContext* context() override { return m_context; } + void setContext(NodeContext* context) override + { + m_context = context; + if (context) { + m_context_ref.Set(*context); + } else { + m_context_ref.Clear(); + } + } + NodeContext* m_context{nullptr}; + util::Ref m_context_ref; }; } // namespace -std::unique_ptr<Node> MakeNode() { return MakeUnique<NodeImpl>(); } +std::unique_ptr<Node> MakeNode(NodeContext* context) { return MakeUnique<NodeImpl>(context); } } // namespace interfaces diff --git a/src/interfaces/node.h b/src/interfaces/node.h index a9680c42b5..cd3cfe487d 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -268,12 +268,14 @@ public: std::function<void(SynchronizationState, interfaces::BlockTip tip, double verification_progress)>; virtual std::unique_ptr<Handler> handleNotifyHeaderTip(NotifyHeaderTipFn fn) = 0; - //! Return pointer to internal chain interface, useful for testing. + //! Get and set internal node context. Useful for testing, but not + //! accessible across processes. virtual NodeContext* context() { return nullptr; } + virtual void setContext(NodeContext* context) { } }; //! Return implementation of Node interface. -std::unique_ptr<Node> MakeNode(); +std::unique_ptr<Node> MakeNode(NodeContext* context = nullptr); //! Block tip (could be a header or not, depends on the subscribed signal). struct BlockTip { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c503a97be5..72c8e65c6b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -187,7 +187,7 @@ namespace { * million to make it highly unlikely for users to have issues with this * filter. * - * We only need to add wtxids to this filter. For non-segwit + * We typically only add wtxids to this filter. For non-segwit * transactions, the txid == wtxid, so this only prevents us from * re-downloading non-segwit transactions when communicating with * non-wtxidrelay peers -- which is important for avoiding malleation @@ -196,6 +196,12 @@ namespace { * the reject filter store wtxids is exactly what we want to avoid * redownload of a rejected transaction. * + * In cases where we can tell that a segwit transaction will fail + * validation no matter the witness, we may add the txid of such + * transaction to the filter as well. This can be helpful when + * communicating with txid-relay peers or if we were to otherwise fetch a + * transaction via txid (eg in our orphan handling). + * * Memory used: 1.3 MB */ std::unique_ptr<CRollingBloomFilter> recentRejects GUARDED_BY(cs_main); @@ -1161,6 +1167,7 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, } // Conflicting (but not necessarily invalid) data or different policy: case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE: + case TxValidationResult::TX_INPUTS_NOT_STANDARD: case TxValidationResult::TX_NOT_STANDARD: case TxValidationResult::TX_MISSING_INPUTS: case TxValidationResult::TX_PREMATURE_SPEND: @@ -2052,6 +2059,19 @@ void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::set<uin // if we start doing this too early. assert(recentRejects); recentRejects->insert(orphanTx.GetWitnessHash()); + // If the transaction failed for TX_INPUTS_NOT_STANDARD, + // then we know that the witness was irrelevant to the policy + // failure, since this check depends only on the txid + // (the scriptPubKey being spent is covered by the txid). + // Add the txid to the reject filter to prevent repeated + // processing of this transaction in the event that child + // transactions are later received (resulting in + // parent-fetching by txid via the orphan-handling logic). + if (orphan_state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && orphanTx.GetWitnessHash() != orphanTx.GetHash()) { + // We only add the txid if it differs from the wtxid, to + // avoid wasting entries in the rolling bloom filter. + recentRejects->insert(orphanTx.GetHash()); + } } EraseOrphanTx(orphanHash); done = true; @@ -2943,7 +2963,7 @@ void ProcessMessage( // We do the AlreadyHave() check using wtxid, rather than txid - in the // absence of witness malleation, this is strictly better, because the - // recent rejects filter may contain the wtxid but will never contain + // recent rejects filter may contain the wtxid but rarely contains // the txid of a segwit transaction that has been rejected. // In the presence of witness malleation, it's possible that by only // doing the check with wtxid, we could overlook a transaction which @@ -3035,6 +3055,17 @@ void ProcessMessage( // if we start doing this too early. assert(recentRejects); recentRejects->insert(tx.GetWitnessHash()); + // If the transaction failed for TX_INPUTS_NOT_STANDARD, + // then we know that the witness was irrelevant to the policy + // failure, since this check depends only on the txid + // (the scriptPubKey being spent is covered by the txid). + // Add the txid to the reject filter to prevent repeated + // processing of this transaction in the event that child + // transactions are later received (resulting in + // parent-fetching by txid via the orphan-handling logic). + if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && tx.GetWitnessHash() != tx.GetHash()) { + recentRejects->insert(tx.GetHash()); + } if (RecursiveDynamicUsage(*ptx) < 100000) { AddToCompactExtraTransactions(ptx); } diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index c56abaf6c9..0e9820da1e 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -152,6 +152,8 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR * script can be anything; an attacker could use a very * expensive-to-check-upon-redemption script like: * DUP CHECKSIG DROP ... repeated 100 times... OP_1 + * + * Note that only the non-witness portion of the transaction is checked here. */ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) { @@ -164,7 +166,11 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) std::vector<std::vector<unsigned char> > vSolutions; TxoutType whichType = Solver(prev.scriptPubKey, vSolutions); - if (whichType == TxoutType::NONSTANDARD) { + if (whichType == TxoutType::NONSTANDARD || whichType == TxoutType::WITNESS_UNKNOWN) { + // WITNESS_UNKNOWN failures are typically also caught with a policy + // flag in the script interpreter, but it can be helpful to catch + // this type of NONSTANDARD transaction earlier in transaction + // validation. return false; } else if (whichType == TxoutType::SCRIPTHASH) { std::vector<std::vector<unsigned char> > stack; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 523f5c429b..4f1e0056be 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -29,6 +29,7 @@ #include <interfaces/handler.h> #include <interfaces/node.h> +#include <node/context.h> #include <noui.h> #include <uint256.h> #include <util/system.h> @@ -430,7 +431,8 @@ int GuiMain(int argc, char* argv[]) SetupEnvironment(); util::ThreadSetInternalName("main"); - std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(); + NodeContext node_context; + std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(&node_context); // Subscribe to global signals from core std::unique_ptr<interfaces::Handler> handler_message_box = node->handleMessageBox(noui_ThreadSafeMessageBox); diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 9347ff9e42..035c8196bc 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -18,6 +18,7 @@ #include <key.h> #include <key_io.h> #include <wallet/wallet.h> +#include <walletinitinterface.h> #include <QApplication> #include <QTimer> @@ -59,6 +60,7 @@ void EditAddressAndSubmit( void TestAddAddressesToSendBook(interfaces::Node& node) { TestChain100Setup test; + node.setContext(&test.m_node); std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); bool firstRun; diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index 12efca2503..031913bd02 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -52,7 +52,8 @@ int main(int argc, char* argv[]) BasicTestingSetup dummy{CBaseChainParams::REGTEST}; } - std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(); + NodeContext node_context; + std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(&node_context); bool fInvalid = false; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 6648029bae..475fd589af 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -138,8 +138,7 @@ void TestGUI(interfaces::Node& node) for (int i = 0; i < 5; ++i) { test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } - node.context()->connman = std::move(test.m_node.connman); - node.context()->mempool = std::move(test.m_node.mempool); + node.setContext(&test.m_node); std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase()); bool firstRun; wallet->LoadWallet(firstRun); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 14f65dcb7c..b2ae1cb845 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -11,6 +11,7 @@ #include <consensus/validation.h> #include <crypto/sha256.h> #include <init.h> +#include <interfaces/chain.h> #include <miner.h> #include <net.h> #include <net_processing.h> @@ -32,6 +33,7 @@ #include <util/vector.h> #include <validation.h> #include <validationinterface.h> +#include <walletinitinterface.h> #include <functional> @@ -104,6 +106,8 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve SetupNetworking(); InitSignatureCache(); InitScriptExecutionCache(); + m_node.chain = interfaces::MakeChain(m_node); + g_wallet_init_interface.Construct(m_node); fCheckBlockIndex = true; static bool noui_connected = false; if (!noui_connected) { diff --git a/src/validation.cpp b/src/validation.cpp index 81af972777..84c106f064 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -688,8 +688,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } // Check for non-standard pay-to-script-hash in inputs - if (fRequireStandard && !AreInputsStandard(tx, m_view)) - return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-nonstandard-inputs"); + if (fRequireStandard && !AreInputsStandard(tx, m_view)) { + return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs"); + } // Check for non-standard witness in P2WSH if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 728212ca23..2233fa5998 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -1418,7 +1418,7 @@ class SegWitTest(BitcoinTestFramework): temp_utxo.pop() # last entry in temp_utxo was the output we just spent temp_utxo.append(UTXO(tx2.sha256, 0, tx2.vout[0].nValue)) - # Spend everything in temp_utxo back to an OP_TRUE output. + # Spend everything in temp_utxo into an segwit v1 output. tx3 = CTransaction() total_value = 0 for i in temp_utxo: @@ -1426,8 +1426,16 @@ class SegWitTest(BitcoinTestFramework): tx3.wit.vtxinwit.append(CTxInWitness()) total_value += i.nValue tx3.wit.vtxinwit[-1].scriptWitness.stack = [witness_program] - tx3.vout.append(CTxOut(total_value - 1000, CScript([OP_TRUE]))) + tx3.vout.append(CTxOut(total_value - 1000, script_pubkey)) tx3.rehash() + + # First we test this transaction against fRequireStandard=true node + # making sure the txid is added to the reject filter + self.std_node.announce_tx_and_wait_for_getdata(tx3) + test_transaction_acceptance(self.nodes[1], self.std_node, tx3, with_witness=True, accepted=False, reason="bad-txns-nonstandard-inputs") + # Now the node will no longer ask for getdata of this transaction when advertised by same txid + self.std_node.announce_tx_and_wait_for_getdata(tx3, timeout=5, success=False) + # Spending a higher version witness output is not allowed by policy, # even with fRequireStandard=false. test_transaction_acceptance(self.nodes[0], self.test_node, tx3, with_witness=True, accepted=False, reason="reserved for soft-fork upgrades") |