From c52cea9486cffe496056d4f00ca7cfeffcce8591 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 23 Jul 2019 10:03:22 +0800 Subject: [PATCH 01/19] Merge #16430: doc: Update bips 35, 37 and 111 status fa56b21c744577eb5d2085fd6104cbc3dc62d677 doc: Update bips 35, 37 and 111 status (MarcoFalke) Pull request description: Follow-up to * #16152: Disable bloom filtering by default ACKs for top commit: laanwj: ACK fa56b21c744577eb5d2085fd6104cbc3dc62d677, thanks for keeping this file up to date fanquake: ACK fa56b21c744577eb5d2085fd6104cbc3dc62d677 Tree-SHA512: 50c17067abbba096e8604b8abccbd0474ecc2dca973935751720c79a023a2d517bb025bb6c36d4fb0d29b7338322af7ae1c0d5a9aaf2712000d9d336c898c204 --- doc/bips.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/bips.md b/doc/bips.md index 31d77d1d85c8..cf1d633ff41e 100644 --- a/doc/bips.md +++ b/doc/bips.md @@ -12,7 +12,7 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v0.18.0**): * [`BIP 31`](https://github.com/bitcoin/bips/blob/master/bip-0031.mediawiki): The 'pong' protocol message (and the protocol version bump to 60001) has been implemented since **v0.6.1** ([PR #1081](https://github.com/bitcoin/bitcoin/pull/1081)). * [`BIP 32`](https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki): Hierarchical Deterministic Wallets has been implemented since **v0.13.0** ([PR #8035](https://github.com/bitcoin/bitcoin/pull/8035)). * [`BIP 34`](https://github.com/bitcoin/bips/blob/master/bip-0034.mediawiki): The rule that requires blocks to contain their height (number) in the coinbase input, and the introduction of version 2 blocks has been implemented since **v0.7.0**. The rule took effect for version 2 blocks as of *block 224413* (March 5th 2013), and version 1 blocks are no longer allowed since *block 227931* (March 25th 2013) ([PR #1526](https://github.com/bitcoin/bitcoin/pull/1526)). -* [`BIP 35`](https://github.com/bitcoin/bips/blob/master/bip-0035.mediawiki): The 'mempool' protocol message (and the protocol version bump to 60002) has been implemented since **v0.7.0** ([PR #1641](https://github.com/bitcoin/bitcoin/pull/1641)). +* [`BIP 35`](https://github.com/bitcoin/bips/blob/master/bip-0035.mediawiki): The 'mempool' protocol message (and the protocol version bump to 60002) has been implemented since **v0.7.0** ([PR #1641](https://github.com/bitcoin/bitcoin/pull/1641)). As of **v0.13.0**, this is only available for `NODE_BLOOM` (BIP 111) peers. * [`BIP 37`](https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki): The bloom filtering for transaction relaying, partial Merkle trees for blocks, and the protocol version bump to 70001 (enabling low-bandwidth SPV clients) has been implemented since **v0.8.0** ([PR #1795](https://github.com/bitcoin/bitcoin/pull/1795)). * [`BIP 42`](https://github.com/bitcoin/bips/blob/master/bip-0042.mediawiki): The bug that would have caused the subsidy schedule to resume after block 13440000 was fixed in **v0.9.2** ([PR #3842](https://github.com/bitcoin/bitcoin/pull/3842)). * [`BIP 61`](https://github.com/bitcoin/bips/blob/master/bip-0061.mediawiki): The 'reject' protocol message (and the protocol version bump to 70002) was added in **v0.9.0** ([PR #3185](https://github.com/bitcoin/bitcoin/pull/3185)). Starting *v0.16.0*, whether to send reject messages can be configured with the `-enablebip61` option. From 0ec883f34cedc6d49520e2aeb9d2d85fb0bc2fb9 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 25 Jul 2019 22:01:43 +0200 Subject: [PATCH 02/19] Merge #16386: depends: disable unused Qt features MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 248e22bbc0d7bc40ae3584d53a18507c46b0e553 depends: disable unused Qt features (fanquake) Pull request description: Related to #16354. Kept separate from #16370, because: > QT is a monster 😂 - dongcarl in #bitcoin-builds I've done some basic testing on `macOS 10.14` and `Debian 9.9` so far. Would be good to have someone test on Windows. I was thinking about adding some inline documentation, i.e info about where to find the lists of Qt features & libraries, as well as breaking the flags up so that it's clearer which libraries we are supplying, which we rely on Qt for etc. Could go towards addressing some of`2` in #16354. ACKs for top commit: sipsorcery: tACK 248e22bbc0d7bc40ae3584d53a18507c46b0e553 (Windows 10 test only) laanwj: ACK 248e22bbc0d7bc40ae3584d53a18507c46b0e553 Tree-SHA512: 2cdcea8d268de21d355a7625c4d352f65728df0b8d8cc0f396aca676f42099a819f95652dfbfc665c991ba12c52735c1e9b693df4b12e3ee178fd39356fba8e0 --- depends/packages/qt.mk | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 0fe25894e4c8..a7216e26e94f 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -40,6 +40,7 @@ $(package)_config_opts += -no-freetype $(package)_config_opts += -no-gif $(package)_config_opts += -no-glib $(package)_config_opts += -no-icu +$(package)_config_opts += -no-ico $(package)_config_opts += -no-iconv $(package)_config_opts += -no-kms $(package)_config_opts += -no-linuxfb @@ -74,19 +75,35 @@ $(package)_config_opts += -qt-harfbuzz $(package)_config_opts += -system-zlib $(package)_config_opts += -static $(package)_config_opts += -v +$(package)_config_opts += -no-feature-bearermanagement +$(package)_config_opts += -no-feature-colordialog +$(package)_config_opts += -no-feature-commandlineparser +$(package)_config_opts += -no-feature-concurrent $(package)_config_opts += -no-feature-dial +$(package)_config_opts += -no-feature-filesystemwatcher +$(package)_config_opts += -no-feature-fontcombobox $(package)_config_opts += -no-feature-ftp +$(package)_config_opts += -no-feature-image_heuristic_mask +$(package)_config_opts += -no-feature-keysequenceedit $(package)_config_opts += -no-feature-lcdnumber $(package)_config_opts += -no-feature-pdf -$(package)_config_opts += -no-feature-printer $(package)_config_opts += -no-feature-printdialog -$(package)_config_opts += -no-feature-concurrent +$(package)_config_opts += -no-feature-printer +$(package)_config_opts += -no-feature-printpreviewdialog +$(package)_config_opts += -no-feature-printpreviewwidget +$(package)_config_opts += -no-feature-sessionmanager $(package)_config_opts += -no-feature-sql $(package)_config_opts += -no-feature-statemachine $(package)_config_opts += -no-feature-syntaxhighlighter $(package)_config_opts += -no-feature-textbrowser $(package)_config_opts += -no-feature-textodfwriter +$(package)_config_opts += -no-feature-topleveldomain $(package)_config_opts += -no-feature-udpsocket +$(package)_config_opts += -no-feature-undocommand +$(package)_config_opts += -no-feature-undogroup +$(package)_config_opts += -no-feature-undostack +$(package)_config_opts += -no-feature-undoview +$(package)_config_opts += -no-feature-vnc $(package)_config_opts += -no-feature-wizard $(package)_config_opts += -no-feature-xml @@ -111,7 +128,6 @@ $(package)_config_opts_linux += -qt-xcb $(package)_config_opts_linux += -no-xcb-xlib $(package)_config_opts_linux += -no-feature-xlib $(package)_config_opts_linux += -system-freetype -$(package)_config_opts_linux += -no-feature-sessionmanager $(package)_config_opts_linux += -fontconfig $(package)_config_opts_linux += -no-opengl $(package)_config_opts_linux += -dbus-runtime From 12e2ad89ce578e02cc7a732a0610dd629abe3a4a Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 29 Jul 2019 14:26:17 +0200 Subject: [PATCH 03/19] Merge #16467: rpc: sendrawtransaction help privacy note 07e01d6258831fd2dfef959a405b3dfe4e88c3c1 rpc: sendrawtransaction unconditionality/privacy note (Jon Atack) Pull request description: In sendrawtransaction RPCHelpMan, mention unconditionality and privacy as per http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-25.html#l-522 before ``` $ bitcoin-cli help sendrawtransaction sendrawtransaction "hexstring" ( maxfeerate ) Submits raw transaction (serialized, hex-encoded) to local node and network. Also see createrawtransaction and signrawtransactionwithkey calls. (...) ``` after ``` $ bitcoin-cli help sendrawtransaction sendrawtransaction "hexstring" ( maxfeerate ) Submit a raw transaction (serialized, hex-encoded) to local node and network. Note that the transaction will be sent unconditionally to all peers, so using this for manual rebroadcast may degrade privacy by leaking the transaction's origin, as nodes will normally not rebroadcast non-wallet transactions already in their mempool. Also see createrawtransaction and signrawtransactionwithkey calls. (...) ``` ACKs for top commit: promag: ACK 07e01d6258831fd2dfef959a405b3dfe4e88c3c1. laanwj: ACK 07e01d6258831fd2dfef959a405b3dfe4e88c3c1 Tree-SHA512: 427b3ca29384eef271eb496b7b14e883220863543a536ddeb31940aaffd52ea0b607d929d50f2b7958514105ef7823fa05c1ee381d4a432808753c06bd97af58 --- src/rpc/rawtransaction.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index c203191caa4a..e2f0fc47a3db 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -771,7 +771,10 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request) UniValue sendrawtransaction(const JSONRPCRequest& request) { - const RPCHelpMan help{"sendrawtransaction", "\nSubmits raw transaction (serialized, hex-encoded) to local node and network.\n" + const RPCHelpMan help{"sendrawtransaction", "\nSubmit a raw transaction (serialized, hex-encoded) to local node and network.\n" + "\nNote that the transaction will be sent unconditionally to all peers, so using this\n" + "for manual rebroadcast may degrade privacy by leaking the transaction's origin, as\n" + "nodes will normally not rebroadcast non-wallet transactions already in their mempool.\n" "\nAlso see createrawtransaction and signrawtransactionwithkey calls.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, From 3f17cc9dd31c230bfcadd1a13edca8c4f1e1878a Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 29 Jul 2019 17:18:38 +0200 Subject: [PATCH 04/19] Merge #16436: gui: Do not create payment server if -disablewallet option provided 4057b7acb7125739537078d026ad96bb21708e3c wallet: Recognize -disablewallet option early (Hennadii Stepanov) Pull request description: This PR makes early check for the `-disablewallet` option. If `-disablewallet=1`, objects `PaymentServer` and `WalletController` are nor created. ACKs for top commit: jonasschnelli: utACK 4057b7acb7125739537078d026ad96bb21708e3c laanwj: ACK 4057b7acb7125739537078d026ad96bb21708e3c Tree-SHA512: 74633cd1eacd0914c73712e6dff190255b5378595cfee7eaeb91e17671fc9120928034739f4ae1c53b86f46c4b400390877241384376b2fc534de326d3ab0944 --- src/qt/dash.cpp | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/qt/dash.cpp b/src/qt/dash.cpp index 8ea38903f829..dabe7b961c3a 100644 --- a/src/qt/dash.cpp +++ b/src/qt/dash.cpp @@ -26,7 +26,8 @@ #ifdef ENABLE_WALLET #include #include -#endif +#include +#endif // ENABLE_WALLET #include #include @@ -219,12 +220,6 @@ BitcoinApplication::~BitcoinApplication() delete window; window = nullptr; -#ifdef ENABLE_WALLET - delete paymentServer; - paymentServer = nullptr; - delete m_wallet_controller; - m_wallet_controller = nullptr; -#endif // Delete Qt-settings if user clicked on "Reset Options" QSettings settings; if(optionsModel && optionsModel->resetSettingsOnShutdown){ @@ -344,24 +339,21 @@ void BitcoinApplication::initializeResult(bool success) { // Log this only after AppInitMain finishes, as then logging setup is guaranteed complete qInfo() << "Platform customization:" << gArgs.GetArg("-uiplatform", BitcoinGUI::DEFAULT_UIPLATFORM).c_str(); -#ifdef ENABLE_WALLET - m_wallet_controller = new WalletController(m_node, optionsModel, this); -#ifdef ENABLE_BIP70 - PaymentServer::LoadRootCAs(); -#endif - if (paymentServer) { - paymentServer->setOptionsModel(optionsModel); -#ifdef ENABLE_BIP70 - connect(m_wallet_controller, &WalletController::coinsSent, paymentServer, &PaymentServer::fetchPaymentACK); -#endif - } -#endif - clientModel = new ClientModel(m_node, optionsModel); window->setClientModel(clientModel); #ifdef ENABLE_WALLET - window->setWalletController(m_wallet_controller); + if (WalletModel::isWalletEnabled()) { + m_wallet_controller = new WalletController(m_node, optionsModel, this); + window->setWalletController(m_wallet_controller); + if (paymentServer) { + paymentServer->setOptionsModel(optionsModel); +#ifdef ENABLE_BIP70 + PaymentServer::LoadRootCAs(); + connect(m_wallet_controller, &WalletController::coinsSent, paymentServer, &PaymentServer::fetchPaymentACK); #endif + } + } +#endif // ENABLE_WALLET // If -min option passed, start window minimized (iconified) or minimized to tray if (!gArgs.GetBoolArg("-min", false)) { @@ -579,8 +571,10 @@ int GuiMain(int argc, char* argv[]) // Start up the payment server early, too, so impatient users that click on // dash: links repeatedly have their payment requests routed to this process: - app.createPaymentServer(); -#endif + if (WalletModel::isWalletEnabled()) { + app.createPaymentServer(); + } +#endif // ENABLE_WALLET /// 9. Main GUI initialization // Install global event filter that makes sure that out-of-focus labels do not contain text cursor. From dbdfa1b3d022bdf33011fd21bfa8dc767fdf420a Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 30 Jul 2019 11:40:39 +0800 Subject: [PATCH 05/19] Merge #16484: doc: update labels in CONTRIBUTING.md faa88d0b5c7f6e317d0e35daef28d320801f1bb5 doc: update labels in CONTRIBUTING.md (MarcoFalke) Pull request description: None of the examples in the "trivial" area are acceptable pull requests, unless they are acceptable in a different area (like "doc" or "log"). Fix that by removing the "trivial" area. ACKs for top commit: jonatack: ACK faa88d0b5c7f6e317d0e35daef28d320801f1bb5 fanquake: ACK faa88d0b5c7f6e317d0e35daef28d320801f1bb5 - agree that trivial was pretty useless and that the meaning was unclear. Other changes look fine. Surprised the white space linter hasn't been having a field day in this file. Tree-SHA512: 6208bcc7c84ad0ca6aeaa2de1901c9da8971aac332b5e7a1194ea7b24fb2d887f988aa22fdfa818e89cbcfd8cb8595ce312525f88c81c5ade484fd7c9bd13d1b --- CONTRIBUTING.md | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e8b2f053569f..c420eaab08ed 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -61,21 +61,15 @@ The title of the pull request should be prefixed by the component or area that the pull request affects. Valid areas as: - *Consensus* for changes to consensus critical code - - *Docs* for changes to the documentation + - *Doc* for changes to the documentation - *Qt* for changes to dash-qt + - *Log* Changes to log messages - *Mining* for changes to the mining code - *Net* or *P2P* for changes to the peer-to-peer network code + - *Refactor* for structural changes that do not change behavior - *RPC/REST/ZMQ* for changes to the RPC, REST or ZMQ APIs - *Scripts and tools* for changes to the scripts and tools - - *Tests* for changes to the unit tests or QA tests - - *Trivial* should **only** be used for PRs that do not change generated - executable code. Notably, refactors (change of function arguments and code - reorganization) and changes in behavior should **not** be marked as trivial. - Examples of trivial PRs are changes to: - - comments - - whitespace - - variable names - - logging and messages + - *Test* for changes to the unit tests or QA tests - *Utils and libraries* for changes to the utils and libraries - *Wallet* for changes to the wallet code @@ -84,10 +78,10 @@ Examples: Consensus: Add new opcode for BIP-XXXX OP_CHECKAWESOMESIG Net: Automatically create hidden service, listen on Tor Qt: Add feed bump button - Trivial: Fix typo in init.cpp + Log: Fix typo in log message Note that translations should not be submitted as pull requests, please see -[Translation Process](https://github.com/dashpay/dash/blob/master/doc/translation_process.md) +[Translation Process](https://github.com/dashpay/dash/blob/master/doc/translation_process.md) for more information on helping with translations. If a pull request is not to be considered for merging (yet), please @@ -424,7 +418,7 @@ The project leader is the release manager for each Dash Core release. Copyright --------- -By contributing to this repository, you agree to license your work under the -MIT license unless specified otherwise in `contrib/debian/copyright` or at -the top of the file itself. Any work contributed where you are not the original +By contributing to this repository, you agree to license your work under the +MIT license unless specified otherwise in `contrib/debian/copyright` or at +the top of the file itself. Any work contributed where you are not the original author must contain its license header with the original author(s) and source. From cae283fdd8ba7bf2ac97d44742c78b57da9eff6b Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 30 Jul 2019 15:37:24 +0200 Subject: [PATCH 06/19] Merge #16434: build: Specify AM_CPPFLAGS for ZMQ 29ee4c417d97dca29c4ef53b6c1a55caa902787a Specify AM_CPPFLAGS for ZMQ. (Daniel Kraft) Pull request description: When building the ZMQ static library, add `AM_CPPFLAGS` to the library `CPPFLAGS`. Otherwise, we may miss important flags that are specified elsewhere. For instance, if `--enable-debug` is passed and `-DDEBUG_LOCKORDER` set, then that would not apply to the ZMQ library before (causing potential for hard-to-find bugs). ACKs for top commit: laanwj: utACK 29ee4c417d97dca29c4ef53b6c1a55caa902787a Tree-SHA512: 64085d71ed3f435a6e4df6dc42bda8b6159a4d292d0547c5b38c09d6ac95e976ad1728cd65278bffdd57363f60a58eb762b1171dafbe055cf94ffcd4f66da877 --- src/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index fc8a965be557..4f785b897408 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -444,7 +444,7 @@ libdash_server_a_SOURCES += dummywallet.cpp endif if ENABLE_ZMQ -libdash_zmq_a_CPPFLAGS = $(BITCOIN_INCLUDES) $(ZMQ_CFLAGS) +libdash_zmq_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(ZMQ_CFLAGS) libdash_zmq_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libdash_zmq_a_SOURCES = \ zmq/zmqabstractnotifier.cpp \ From 02e5436bb880c8c1fdc6c0b0022e21b8758f133d Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 31 Jul 2019 08:57:08 +0200 Subject: [PATCH 07/19] Merge #16451: Remove CMerkleTx 05b56d1c937b7667ad51400d2f9fb674af72953f [wallet] Remove CMerkleTx serialization logic (John Newbery) 783a76f23ba4f33b6e6f609eaf3bf41afd9bcd6f [wallet] Flatten CWalletTx class hierarchy (John Newbery) b3a9d179f23f654b8fba63924bbca5fd31ad4bb0 [wallet] Move CMerkleTx functions into CWalletTx (John Newbery) Pull request description: CMerkleTx is only used as a base class for CWalletTx. It was previously also used for vtxPrev which was removed in 93a18a3650292afbb441a47d1fa1b94aeb0164e3. This PR moves all of the CMerkleTx members and logic into CWalletTx. The CMerkleTx class is kept for deserialization and serialization of old wallet files. This makes the refactor in #15931 cleaner. ACKs for top commit: laanwj: ACK 05b56d1c937b7667ad51400d2f9fb674af72953f. Looks good to me. Tree-SHA512: 3d3a0069ebb536b12a328f1261e7dc55158a71088d445ae4b4ace4142c432dc296f58c8183b1922e54a60b8cc77e9d17c3dce7478294cd68693594baacf2bab3 --- src/wallet/wallet.cpp | 14 ++-- src/wallet/wallet.h | 152 ++++++++++++++++++++---------------------- 2 files changed, 79 insertions(+), 87 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 79ec2bb14049..3799cab66846 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -169,7 +169,7 @@ std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& return LoadWallet(chain, WalletLocation(name), error, warning); } -const uint256 CMerkleTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); +const uint256 CWalletTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); /** @defgroup mapWallet * @@ -5510,7 +5510,7 @@ CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool fInternalIn) fInternal = fInternalIn; } -void CMerkleTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) +void CWalletTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) { // Update the tx's hashBlock hashBlock = block_hash; @@ -5519,7 +5519,7 @@ void CMerkleTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) nIndex = posInBlock; } -int CMerkleTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const +int CWalletTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const { if (hashUnset()) return 0; @@ -5527,7 +5527,7 @@ int CMerkleTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const return locked_chain.getBlockDepth(hashBlock) * (nIndex == -1 ? -1 : 1); } -bool CMerkleTx::IsLockedByInstantSend() const +bool CWalletTx::IsLockedByInstantSend() const { if (fIsChainlocked) { fIsInstantSendLocked = false; @@ -5537,7 +5537,7 @@ bool CMerkleTx::IsLockedByInstantSend() const return fIsInstantSendLocked; } -bool CMerkleTx::IsChainLocked() const +bool CWalletTx::IsChainLocked() const { if (!fIsChainlocked) { AssertLockHeld(cs_main); @@ -5549,7 +5549,7 @@ bool CMerkleTx::IsChainLocked() const return fIsChainlocked; } -int CMerkleTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const +int CWalletTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const { if (!IsCoinBase()) return 0; @@ -5558,7 +5558,7 @@ int CMerkleTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const return std::max(0, (COINBASE_MATURITY+1) - chain_depth); } -bool CMerkleTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const +bool CWalletTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const { // note GetBlocksToMaturity is 0 for non-coinbase tx return GetBlocksToMaturity(locked_chain) > 0; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 06e273f71f12..befc1e0830eb 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -229,82 +229,24 @@ struct COutputEntry int vout; }; -/** A transaction with a merkle branch linking it to the block chain. */ +/** Legacy class used for deserializing vtxPrev for backwards compatibility. + * vtxPrev was removed in commit 93a18a3650292afbb441a47d1fa1b94aeb0164e3, + * but old wallet.dat files may still contain vtxPrev vectors of CMerkleTxs. + * These need to get deserialized for field alignment when deserializing + * a CWalletTx, but the deserialized values are discarded.**/ class CMerkleTx { -private: - /** Constant used in hashBlock to indicate tx has been abandoned */ - static const uint256 ABANDON_HASH; - - mutable bool fIsChainlocked{false}; - mutable bool fIsInstantSendLocked{false}; - public: - CTransactionRef tx; - uint256 hashBlock; - - /* An nIndex == -1 means that hashBlock (in nonzero) refers to the earliest - * block in the chain we know this or any in-wallet dependency conflicts - * with. Older clients interpret nIndex == -1 as unconfirmed for backward - * compatibility. - */ - int nIndex; - - CMerkleTx() - { - SetTx(MakeTransactionRef()); - Init(); - } - - explicit CMerkleTx(CTransactionRef arg) - { - SetTx(std::move(arg)); - Init(); - } - - void Init() - { - hashBlock = uint256(); - nIndex = -1; - } - - void SetTx(CTransactionRef arg) + template + void Unserialize(Stream& s) { - tx = std::move(arg); - } + CTransactionRef tx; + uint256 hashBlock; + std::vector vMerkleBranch; + int nIndex; - SERIALIZE_METHODS(CMerkleTx, obj) - { - std::vector vMerkleBranch; // For compatibility with older versions. - READWRITE(obj.tx, obj.hashBlock, vMerkleBranch, obj.nIndex); + s >> tx >> hashBlock >> vMerkleBranch >> nIndex; } - - void SetMerkleBranch(const uint256& block_hash, int posInBlock); - - /** - * Return depth of transaction in blockchain: - * <0 : conflicts with a transaction this deep in the blockchain - * 0 : in memory pool, waiting to be included in a block - * >=1 : this many blocks deep in the main chain - */ - int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const; - bool IsInMainChain(interfaces::Chain::Lock& locked_chain) const { return GetDepthInMainChain(locked_chain) > 0; } - bool IsLockedByInstantSend() const; - bool IsChainLocked() const; - - /** - * @return number of blocks to maturity for this transaction: - * 0 : is not a coinbase transaction, or is a mature coinbase transaction - * >0 : is a coinbase transaction which matures in this many blocks - */ - int GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const; - bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } - bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } - void setAbandoned() { hashBlock = ABANDON_HASH; } - - const uint256& GetHash() const { return tx->GetHash(); } - bool IsCoinBase() const { return tx->IsCoinBase(); } - bool IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const; }; //Get the marginal bytes of spending the specified output @@ -314,11 +256,17 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, * A transaction with a bunch of additional info that only the owner cares about. * It includes any unrecorded transactions needed to link it back to the block chain. */ -class CWalletTx : public CMerkleTx +class CWalletTx { private: const CWallet* pwallet; + /** Constant used in hashBlock to indicate tx has been abandoned */ + static const uint256 ABANDON_HASH; + + mutable bool fIsChainlocked{false}; + mutable bool fIsInstantSendLocked{false}; + public: /** * Key/value map with information about the transaction. @@ -376,7 +324,10 @@ class CWalletTx : public CMerkleTx mutable bool fInMempool; mutable CAmount nChangeCached; - CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) : CMerkleTx(std::move(arg)) + CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) + : tx(std::move(arg)), + hashBlock(uint256()), + nIndex(-1) { Init(pwalletIn); } @@ -396,10 +347,18 @@ class CWalletTx : public CMerkleTx nOrderPos = -1; } + CTransactionRef tx; + uint256 hashBlock; + /* An nIndex == -1 means that hashBlock (in nonzero) refers to the earliest + * block in the chain we know this or any in-wallet dependency conflicts + * with. Older clients interpret nIndex == -1 as unconfirmed for backward + * compatibility. + */ + int nIndex; + template void Serialize(Stream& s) const { - char fSpent = false; mapValue_t mapValueCopy = mapValue; mapValueCopy["fromaccount"] = ""; @@ -408,20 +367,21 @@ class CWalletTx : public CMerkleTx mapValueCopy["timesmart"] = strprintf("%u", nTimeSmart); } - s << static_cast(*this); - std::vector vUnused; //!< Used to be vtxPrev - s << vUnused << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << fSpent; + std::vector dummy_vector1; //!< Used to be vMerkleBranch + std::vector dummy_vector2; //!< Used to be vtxPrev + char dummy_char = false; //!< Used to be fSpent + s << tx << hashBlock << dummy_vector1 << nIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_char; } template void Unserialize(Stream& s) { Init(nullptr); - char fSpent; - s >> static_cast(*this); - std::vector vUnused; //!< Used to be vtxPrev - s >> vUnused >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> fSpent; + std::vector dummy_vector1; //!< Used to be vMerkleBranch + std::vector dummy_vector2; //!< Used to be vtxPrev + char dummy_char; //! Used to be fSpent + s >> tx >> hashBlock >> dummy_vector1 >> nIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_char; ReadOrderPos(nOrderPos, mapValue); nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0; @@ -432,6 +392,11 @@ class CWalletTx : public CMerkleTx mapValue.erase("timesmart"); } + void SetTx(CTransactionRef arg) + { + tx = std::move(arg); + } + //! make sure balances are recalculated void MarkDirty() { @@ -503,6 +468,33 @@ class CWalletTx : public CMerkleTx // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" // in place. std::set GetConflicts() const NO_THREAD_SAFETY_ANALYSIS; + + void SetMerkleBranch(const uint256& block_hash, int posInBlock); + + /** + * Return depth of transaction in blockchain: + * <0 : conflicts with a transaction this deep in the blockchain + * 0 : in memory pool, waiting to be included in a block + * >=1 : this many blocks deep in the main chain + */ + int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const; + bool IsInMainChain(interfaces::Chain::Lock& locked_chain) const { return GetDepthInMainChain(locked_chain) > 0; } + bool IsLockedByInstantSend() const; + bool IsChainLocked() const; + + /** + * @return number of blocks to maturity for this transaction: + * 0 : is not a coinbase transaction, or is a mature coinbase transaction + * >0 : is a coinbase transaction which matures in this many blocks + */ + int GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const; + bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } + bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } + void setAbandoned() { hashBlock = ABANDON_HASH; } + + const uint256& GetHash() const { return tx->GetHash(); } + bool IsCoinBase() const { return tx->IsCoinBase(); } + bool IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const; }; struct WalletTxHasher From 356785cdaa1afe79ddd2278c7b05853b6e0a7351 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 31 Jul 2019 18:03:43 -0400 Subject: [PATCH 08/19] Merge #16293: test: Make test cases separate functions faf8318c55a6001270a6fc8ed2298767099bafba test: Split fundrawtx test into subtests (MarcoFalke) fa6fba3bc8013d7f813edd71f152d86eab907e4d test: Make local symbols in run_test members (MarcoFalke) Pull request description: This prevents scope-leak of symbols that are supposed to be local to one test case. Top commit has no ACKs. Tree-SHA512: 9b2a4ca2cdd631ef915d2f7e6cd62375df9a0919448350aa6e5ae4aa8a8fe3ba53870f7a9a25a57736894b4e3a45e861018253ed2d57d9a64c2bb65fa270fad8 --- test/functional/rpc_fundrawtransaction.py | 155 +++++++++++++--------- 1 file changed, 90 insertions(+), 65 deletions(-) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 4a60ab990070..0b4592c6b09a 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -42,11 +42,11 @@ def setup_network(self): connect_nodes(self.nodes[0], 3) def run_test(self): - min_relay_tx_fee = self.nodes[0].getnetworkinfo()['relayfee'] + self.min_relay_tx_fee = self.nodes[0].getnetworkinfo()['relayfee'] # This test is not meant to test fee estimation and we'd like # to be sure all txs are sent at a consistent desired feerate for node in self.nodes: - node.settxfee(min_relay_tx_fee) + node.settxfee(self.min_relay_tx_fee) # if the fee's positive delta is higher than this value tests will fail, # neg. delta always fail the tests. @@ -54,13 +54,42 @@ def run_test(self): # than a minimum sized signature. # = 2 bytes * minRelayTxFeePerByte - feeTolerance = 2 * min_relay_tx_fee/1000 + self.fee_tolerance = 2 * self.min_relay_tx_fee / 1000 self.nodes[2].generate(1) self.sync_all() self.nodes[0].generate(121) self.sync_all() + self.test_change_position() + self.test_simple() + self.test_simple_two_coins() + self.test_simple_two_outputs() + self.test_change() + self.test_no_change() + self.test_invalid_option() + self.test_invalid_change_address() + self.test_valid_change_address() + self.test_coin_selection() + self.test_two_vin() + self.test_two_vin_two_vout() + self.test_invalid_input() + self.test_fee_p2pkh() + self.test_fee_p2pkh_multi_out() + self.test_fee_p2sh() + self.test_fee_4of5() + self.test_spend_2of2() + self.test_locked_wallet() + self.test_many_inputs_fee() + self.test_many_inputs_send() + self.test_op_return() + self.test_watchonly() + self.test_all_watched_funds() + self.test_option_feerate() + self.test_address_reuse() + self.test_option_subtract_fee_from_outputs() + + def test_change_position(self): # ensure that setting changePosition in fundraw with an exact match is handled properly rawmatch = self.nodes[2].createrawtransaction([], {self.nodes[2].getnewaddress():500}) rawmatch = self.nodes[2].fundrawtransaction(rawmatch, {"changePosition":1, "subtractFeeFromOutputs":[0]}) @@ -68,15 +97,15 @@ def run_test(self): watchonly_address = self.nodes[0].getnewaddress() watchonly_pubkey = self.nodes[0].getaddressinfo(watchonly_address)["pubkey"] - watchonly_amount = Decimal(2000) + self.watchonly_amount = Decimal(2000) self.nodes[3].importpubkey(watchonly_pubkey, "", True) - watchonly_txid = self.nodes[0].sendtoaddress(watchonly_address, watchonly_amount) + self.watchonly_txid = self.nodes[0].sendtoaddress(watchonly_address, self.watchonly_amount) # Lock UTXO so nodes[0] doesn't accidentally spend it - watchonly_vout = find_vout_for_address(self.nodes[0], watchonly_txid, watchonly_address) - self.nodes[0].lockunspent(False, [{"txid": watchonly_txid, "vout": watchonly_vout}]) + self.watchonly_vout = find_vout_for_address(self.nodes[0], self.watchonly_txid, watchonly_address) + self.nodes[0].lockunspent(False, [{"txid": self.watchonly_txid, "vout": self.watchonly_vout}]) - self.nodes[0].sendtoaddress(self.nodes[3].getnewaddress(), watchonly_amount / 10) + self.nodes[0].sendtoaddress(self.nodes[3].getnewaddress(), self.watchonly_amount / 10) self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 15) self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 10) @@ -85,6 +114,7 @@ def run_test(self): self.nodes[0].generate(1) self.sync_all() + def test_simple(self): ############### # simple test # ############### @@ -93,10 +123,10 @@ def run_test(self): rawtx = self.nodes[2].createrawtransaction(inputs, outputs) dec_tx = self.nodes[2].decoderawtransaction(rawtx) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) assert len(dec_tx['vin']) > 0 #test that we have enough inputs + def test_simple_two_coins(self): ############################## # simple test with two coins # ############################## @@ -106,25 +136,11 @@ def run_test(self): dec_tx = self.nodes[2].decoderawtransaction(rawtx) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) assert len(dec_tx['vin']) > 0 #test if we have enough inputs - - ############################## - # simple test with two coins # - ############################## - inputs = [ ] - outputs = { self.nodes[0].getnewaddress() : 26 } - rawtx = self.nodes[2].createrawtransaction(inputs, outputs) - dec_tx = self.nodes[2].decoderawtransaction(rawtx) - - rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] - dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) - assert len(dec_tx['vin']) > 0 assert_equal(dec_tx['vin'][0]['scriptSig']['hex'], '') - + def test_simple_two_outputs(self): ################################ # simple test with two outputs # ################################ @@ -134,7 +150,6 @@ def run_test(self): dec_tx = self.nodes[2].decoderawtransaction(rawtx) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 for out in dec_tx['vout']: @@ -143,7 +158,7 @@ def run_test(self): assert len(dec_tx['vin']) > 0 assert_equal(dec_tx['vin'][0]['scriptSig']['hex'], '') - + def test_change(self): ######################################################################### # test a fundrawtransaction with a VIN greater than the required amount # ######################################################################### @@ -157,6 +172,7 @@ def run_test(self): rawtxfund = self.nodes[2].fundrawtransaction(rawtx) fee = rawtxfund['fee'] + self.test_no_change_fee = fee # Use the same fee for the next tx dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 for out in dec_tx['vout']: @@ -164,14 +180,14 @@ def run_test(self): assert_equal(fee + totalOut, utx['amount']) #compare vin total and totalout+fee - + def test_no_change(self): ##################################################################### # test a fundrawtransaction with which will not get a change output # ##################################################################### utx = get_unspent(self.nodes[2].listunspent(), 50) inputs = [ {'txid' : utx['txid'], 'vout' : utx['vout']}] - outputs = { self.nodes[0].getnewaddress() : Decimal(50) - fee - feeTolerance } + outputs = {self.nodes[0].getnewaddress(): Decimal(50) - self.test_no_change_fee - self.fee_tolerance} rawtx = self.nodes[2].createrawtransaction(inputs, outputs) dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) @@ -186,7 +202,7 @@ def run_test(self): assert_equal(rawtxfund['changepos'], -1) assert_equal(fee + totalOut, utx['amount']) #compare vin total and totalout+fee - + def test_invalid_option(self): #################################################### # test a fundrawtransaction with an invalid option # #################################################### @@ -203,6 +219,7 @@ def run_test(self): # reserveChangeKey was deprecated and is now removed assert_raises_rpc_error(-3, "Unexpected key reserveChangeKey", lambda: self.nodes[2].fundrawtransaction(hexstring=rawtx, options={'reserveChangeKey': True})) + def test_invalid_change_address(self): ############################################################ # test a fundrawtransaction with an invalid change address # ############################################################ @@ -216,6 +233,7 @@ def run_test(self): assert_raises_rpc_error(-5, "changeAddress must be a valid dash address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'}) + def test_valid_change_address(self): ############################################################ # test a fundrawtransaction with a provided change address # ############################################################ @@ -234,7 +252,7 @@ def run_test(self): out = dec_tx['vout'][0] assert_equal(change, out['scriptPubKey']['addresses'][0]) - + def test_coin_selection(self): ######################################################################### # test a fundrawtransaction with a VIN smaller than the required amount # ######################################################################### @@ -252,7 +270,6 @@ def run_test(self): assert_equal("00", dec_tx['vin'][0]['scriptSig']['hex']) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 @@ -269,7 +286,7 @@ def run_test(self): assert_equal(matchingOuts, 1) assert_equal(len(dec_tx['vout']), 2) - + def test_two_vin(self): ########################################### # test a fundrawtransaction with two VINs # ########################################### @@ -283,7 +300,6 @@ def run_test(self): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 @@ -303,6 +319,7 @@ def run_test(self): assert_equal(matchingIns, 2) #we now must see two vins identical to vins given as params + def test_two_vin_two_vout(self): ######################################################### # test a fundrawtransaction with two VINs and two vOUTs # ######################################################### @@ -316,7 +333,6 @@ def run_test(self): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - fee = rawtxfund['fee'] dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 @@ -328,16 +344,16 @@ def run_test(self): assert_equal(matchingOuts, 2) assert_equal(len(dec_tx['vout']), 3) + def test_invalid_input(self): ############################################## # test a fundrawtransaction with invalid vin # ############################################## inputs = [ {'txid' : "1c7f966dab21119bac53213a2bc7532bff1fa844c124fd750a7d0b1332440bd1", 'vout' : 0} ] #invalid vin! outputs = { self.nodes[0].getnewaddress() : 10} rawtx = self.nodes[2].createrawtransaction(inputs, outputs) - dec_tx = self.nodes[2].decoderawtransaction(rawtx) - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx) + def test_fee_p2pkh(self): ############################################################ #compare fee of a standard pubkeyhash transaction inputs = [] @@ -351,9 +367,10 @@ def run_test(self): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance ############################################################ + def test_fee_p2pkh_multi_out(self): ############################################################ #compare fee of a standard pubkeyhash transaction with multiple outputs inputs = [] @@ -366,10 +383,10 @@ def run_test(self): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance ############################################################ - + def test_fee_p2sh(self): ############################################################ #compare fee of a 2of2 multisig p2sh transaction @@ -393,10 +410,10 @@ def run_test(self): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance ############################################################ - + def test_fee_4of5(self): ############################################################ #compare fee of a standard pubkeyhash transaction @@ -426,10 +443,10 @@ def run_test(self): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance ############################################################ - + def test_spend_2of2(self): ############################################################ # spend a 2of2 multisig transaction over fundraw @@ -444,7 +461,7 @@ def run_test(self): # send 12 DASH to msig addr - txId = self.nodes[0].sendtoaddress(mSigObj, 12) + self.nodes[0].sendtoaddress(mSigObj, 12) self.sync_all() self.nodes[1].generate(1) self.sync_all() @@ -456,7 +473,7 @@ def run_test(self): fundedTx = self.nodes[2].fundrawtransaction(rawtx) signedTx = self.nodes[2].signrawtransactionwithwallet(fundedTx['hex']) - txId = self.nodes[2].sendrawtransaction(signedTx['hex']) + self.nodes[2].sendrawtransaction(signedTx['hex']) self.sync_all() self.nodes[1].generate(1) self.sync_all() @@ -464,6 +481,7 @@ def run_test(self): # make sure funds are received at node1 assert_equal(oldBalance+Decimal('11.0000000'), self.nodes[1].getbalance()) + def test_locked_wallet(self): ############################################################ # locked wallet test self.nodes[1].encryptwallet("test") @@ -473,7 +491,7 @@ def run_test(self): # This test is not meant to test fee estimation and we'd like # to be sure all txs are sent at a consistent desired feerate for node in self.nodes: - node.settxfee(min_relay_tx_fee) + node.settxfee(self.min_relay_tx_fee) connect_nodes(self.nodes[0], 1) connect_nodes(self.nodes[1], 2) @@ -481,7 +499,7 @@ def run_test(self): 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": watchonly_txid, "vout": watchonly_vout}]) + self.nodes[0].lockunspent(False, [{"txid": self.watchonly_txid, "vout": self.watchonly_vout}]) self.sync_all() # drain the keypool @@ -509,13 +527,14 @@ def run_test(self): #now we need to unlock self.nodes[1].walletpassphrase("test", 600) signedTx = self.nodes[1].signrawtransactionwithwallet(fundedTx['hex']) - txId = self.nodes[1].sendrawtransaction(signedTx['hex']) + self.nodes[1].sendrawtransaction(signedTx['hex']) self.nodes[1].generate(1) self.sync_all() # make sure funds are received at node1 assert_equal(oldBalance+Decimal('511.0000000'), self.nodes[0].getbalance()) + def test_many_inputs_fee(self): ############################################### # multiple (~19) inputs tx test | Compare fee # ############################################### @@ -543,9 +562,9 @@ def run_test(self): #compare fee feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) - assert feeDelta >= 0 and feeDelta <= feeTolerance*19 #~19 inputs - + assert feeDelta >= 0 and feeDelta <= self.fee_tolerance * 19 #~19 inputs + def test_many_inputs_send(self): ############################################# # multiple (~19) inputs tx test | sign/send # ############################################# @@ -569,12 +588,13 @@ def run_test(self): rawtx = self.nodes[1].createrawtransaction(inputs, outputs) fundedTx = self.nodes[1].fundrawtransaction(rawtx) fundedAndSignedTx = self.nodes[1].signrawtransactionwithwallet(fundedTx['hex']) - txId = self.nodes[1].sendrawtransaction(fundedAndSignedTx['hex']) + self.nodes[1].sendrawtransaction(fundedAndSignedTx['hex']) self.sync_all() self.nodes[0].generate(1) self.sync_all() assert_equal(oldBalance+Decimal('500.19000000'), self.nodes[0].getbalance()) #0.19+block reward + def test_op_return(self): ##################################################### # test fundrawtransaction with OP_RETURN and no vin # ##################################################### @@ -591,40 +611,41 @@ def run_test(self): assert_greater_than(len(dec_tx['vin']), 0) # at least one vin assert_equal(len(dec_tx['vout']), 2) # one change output added - + def test_watchonly(self): ################################################## # test a fundrawtransaction using only watchonly # ################################################## inputs = [] - outputs = {self.nodes[2].getnewaddress() : watchonly_amount / 2} + outputs = {self.nodes[2].getnewaddress(): self.watchonly_amount / 2} rawtx = self.nodes[3].createrawtransaction(inputs, outputs) result = self.nodes[3].fundrawtransaction(rawtx, {'includeWatching': True }) res_dec = self.nodes[0].decoderawtransaction(result["hex"]) assert_equal(len(res_dec["vin"]), 1) - assert_equal(res_dec["vin"][0]["txid"], watchonly_txid) + assert_equal(res_dec["vin"][0]["txid"], self.watchonly_txid) assert "fee" in result.keys() assert_greater_than(result["changepos"], -1) + def test_all_watched_funds(self): ############################################################### # test fundrawtransaction using the entirety of watched funds # ############################################################### inputs = [] - outputs = {self.nodes[2].getnewaddress() : watchonly_amount} + outputs = {self.nodes[2].getnewaddress(): self.watchonly_amount} rawtx = self.nodes[3].createrawtransaction(inputs, outputs) # Backward compatibility test (2nd param is includeWatching) result = self.nodes[3].fundrawtransaction(rawtx, True) res_dec = self.nodes[0].decoderawtransaction(result["hex"]) assert_equal(len(res_dec["vin"]), 2) - assert res_dec["vin"][0]["txid"] == watchonly_txid or res_dec["vin"][1]["txid"] == watchonly_txid + assert res_dec["vin"][0]["txid"] == self.watchonly_txid or res_dec["vin"][1]["txid"] == self.watchonly_txid assert_greater_than(result["fee"], 0) assert_greater_than(result["changepos"], -1) - assert_equal(result["fee"] + res_dec["vout"][result["changepos"]]["value"], watchonly_amount / 10) + assert_equal(result["fee"] + res_dec["vout"][result["changepos"]]["value"], self.watchonly_amount / 10) signedtx = self.nodes[3].signrawtransactionwithwallet(result["hex"]) assert not signedtx["complete"] @@ -634,6 +655,7 @@ def run_test(self): self.nodes[0].generate(1) self.sync_all() + def test_option_feerate(self): ####################### # Test feeRate option # ####################### @@ -644,18 +666,20 @@ def run_test(self): inputs = [] outputs = {self.nodes[3].getnewaddress() : 1} rawtx = self.nodes[3].createrawtransaction(inputs, outputs) - result = self.nodes[3].fundrawtransaction(rawtx) # uses min_relay_tx_fee (set by settxfee) - result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee}) - result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10*min_relay_tx_fee}) + result = self.nodes[3].fundrawtransaction(rawtx) # uses self.min_relay_tx_fee (set by settxfee) + result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee}) + result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10 * self.min_relay_tx_fee}) assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[3].fundrawtransaction, rawtx, {"feeRate": 1}) result_fee_rate = result['fee'] * 1000 / count_bytes(result['hex']) assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate) assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate) + def test_address_reuse(self): ################################ # Test no address reuse occurs # ################################ + rawtx = self.nodes[3].createrawtransaction(inputs=[], outputs={self.nodes[3].getnewaddress(): 1}) result3 = self.nodes[3].fundrawtransaction(rawtx) res_dec = self.nodes[0].decoderawtransaction(result3["hex"]) changeaddress = "" @@ -667,6 +691,7 @@ def run_test(self): # Now the change address key should be removed from the keypool assert changeaddress != nextaddr + def test_option_subtract_fee_from_outputs(self): ###################################### # Test subtractFeeFromOutputs option # ###################################### @@ -678,11 +703,11 @@ def run_test(self): outputs = {self.nodes[2].getnewaddress(): 1} rawtx = self.nodes[3].createrawtransaction(inputs, outputs) - result = [self.nodes[3].fundrawtransaction(rawtx), # uses min_relay_tx_fee (set by settxfee) - self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": []}), # empty subtraction list - self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0]}), # uses min_relay_tx_fee (set by settxfee) - self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee}), - self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee, "subtractFeeFromOutputs": [0]})] + result = [self.nodes[3].fundrawtransaction(rawtx), # uses self.min_relay_tx_fee (set by settxfee) + self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": []}), # empty subtraction list + self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0]}), # uses self.min_relay_tx_fee (set by settxfee) + self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee}), + self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee, "subtractFeeFromOutputs": [0]}),] dec_tx = [self.nodes[3].decoderawtransaction(tx_['hex']) for tx_ in result] output = [d['vout'][1 - r['changepos']]['value'] for d, r in zip(dec_tx, result)] From 41f9bfaee67bd4113afd40e46b25aafc85735571 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 1 Mar 2018 18:27:08 +0100 Subject: [PATCH 09/19] Merge #11882: Disable default fallbackfee on mainnet 3f592b8 [QA] add wallet-rbf test (Jonas Schnelli) 8222e05 Disable wallet fallbackfee by default on mainnet (Jonas Schnelli) Pull request description: Removes the default fallback fee on mainnet (but keeps it on testnet/regtest). Transactions using the fallbackfee in case the fallback fee has not been set are getting rejected. Tree-SHA512: e54d2594b7f954e640cc513a18b0bfbe189f15e15bdeed4fe02b7677f939bca1731fef781b073127ffd4ce08a595fb118259b8826cdaa077ff7d5ae9495810db --- src/chainparams.cpp | 8 +++++++ src/chainparams.h | 3 +++ src/qt/test/wallettests.cpp | 2 ++ src/wallet/fees.cpp | 3 +++ src/wallet/init.cpp | 1 + src/wallet/test/wallet_test_fixture.cpp | 2 ++ src/wallet/wallet.cpp | 7 +++++++ src/wallet/wallet.h | 2 ++ test/functional/test_runner.py | 1 + test/functional/wallet_fallbackfee.py | 28 +++++++++++++++++++++++++ 10 files changed, 57 insertions(+) create mode 100755 test/functional/wallet_fallbackfee.py diff --git a/src/chainparams.cpp b/src/chainparams.cpp index f8f16c907b71..995b233cc991 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -360,6 +360,9 @@ class CMainParams : public CChainParams { // (the tx=... number in the ChainStateFlushed debug.log lines) 0.3 // * estimated number of transactions per second after that timestamp }; + + /* disable fallback fee on mainnet */ + m_fallback_fee_enabled = false; } }; @@ -554,6 +557,8 @@ class CTestNetParams : public CChainParams { 0.01 // * estimated number of transactions per second after that timestamp }; + /* enable fallback fee on testnet */ + m_fallback_fee_enabled = true; } }; @@ -1020,6 +1025,9 @@ class CRegTestParams : public CChainParams { params->minSize = threshold; params->threshold = threshold; params->dkgBadVotesThreshold = threshold; + + /* enable fallback fee on regtest */ + m_fallback_fee_enabled = true; } void UpdateLLMQTestParametersFromArgs(const ArgsManager& args); }; diff --git a/src/chainparams.h b/src/chainparams.h index 48560caa4a1d..adb4b725f1b6 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -85,6 +85,8 @@ class CChainParams int LLMQConnectionRetryTimeout() const { return nLLMQConnectionRetryTimeout; } /** Return the BIP70 network string (main, test or regtest) */ std::string NetworkIDString() const { return strNetworkID; } + /** Return true if the fallback fee is by default enabled for this network */ + bool IsFallbackFeeEnabled() const { return m_fallback_fee_enabled; } /** Return the list of hostnames to look up for DNS seeds */ const std::vector& DNSSeeds() const { return vSeeds; } const std::vector& Base58Prefix(Base58Type type) const { return base58Prefixes[type]; } @@ -134,6 +136,7 @@ class CChainParams int nLLMQConnectionRetryTimeout; CCheckpointData checkpointData; ChainTxData chainTxData; + bool m_fallback_fee_enabled; int nPoolMinParticipants; int nPoolMaxParticipants; int nFulfilledRequestExpireTime; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 2289115d0d4a..aa656090c458 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -105,6 +105,8 @@ void TestGUI() test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } auto chain = interfaces::MakeChain(); + g_wallet_allow_fallback_fee = true; + std::shared_ptr wallet = std::make_shared(*chain, WalletLocation(), WalletDatabase::CreateMock()); AddWallet(wallet); bool firstRun; diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 038d7030ef12..1ad991115229 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -58,6 +58,9 @@ CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_contr // if we don't have enough data for estimateSmartFee, then use fallback fee feerate_needed = wallet.m_fallback_fee; if (feeCalc) feeCalc->reason = FeeReason::FALLBACK; + + // directly return if fallback fee is disabled (feerate 0 == disabled) + if (wallet.m_fallback_fee.GetFee(1000) == 0) return feerate_needed; } // Obey mempool min fee when using smart fee estimation CFeeRate min_mempool_feerate = wallet.chain().mempoolMinFee(); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 47d7d2a9997c..5c5a4068586c 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -3,6 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index f6db5196210f..090ad7392891 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -9,6 +9,8 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), m_wallet(*m_chain, WalletLocation(), WalletDatabase::CreateMock()) { + g_wallet_allow_fallback_fee = true; + bool fFirstRun; m_wallet.LoadWallet(fFirstRun); m_wallet.handleNotifications(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3799cab66846..87ec08080387 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -53,6 +53,7 @@ static const size_t OUTPUT_GROUP_MAX_ENTRIES = 10; static CCriticalSection cs_wallets; static std::vector> vpwallets GUARDED_BY(cs_wallets); +bool g_wallet_allow_fallback_fee = false; //& wallet) { @@ -3838,6 +3839,12 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std } } + if (feeCalc.reason == FeeReason::FALLBACK && !g_wallet_allow_fallback_fee) { + // eventually allow a fallback fee + strFailReason = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); + return false; + } + if (nAmountLeft == nFeeRet) { // We either added the change amount to nFeeRet because the change amount was considered // to be dust or the input exactly matches output + fee. diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index befc1e0830eb..5723b398e3cd 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -52,6 +52,7 @@ bool HasWallets(); std::vector> GetWallets(); std::shared_ptr GetWallet(const std::string& name); std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::string& warning); +extern bool g_wallet_allow_fallback_fee; static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000; //! -paytxfee default @@ -74,6 +75,7 @@ static const bool DEFAULT_AVOIDPARTIALSPENDS = false; static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 6; static const bool DEFAULT_WALLETBROADCAST = true; static const bool DEFAULT_DISABLE_WALLET = false; +static const bool DEFAULT_WALLET_ALLOW_FALLBACKFEE = true; //! -maxtxfee default static const CAmount DEFAULT_TRANSACTION_MAXFEE = COIN / 10; //! Discourage users to set fees higher than this amount (in duffs) per kB diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 95478c88cf79..c97e001b8426 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -203,6 +203,7 @@ 'feature_governance_objects.py', 'rpc_uptime.py', 'wallet_resendwallettransactions.py', + 'wallet_fallbackfee.py', 'feature_minchainwork.py', 'p2p_unrequested_blocks.py', # NOTE: needs dash_hash to pass 'feature_shutdown.py', diff --git a/test/functional/wallet_fallbackfee.py b/test/functional/wallet_fallbackfee.py new file mode 100755 index 000000000000..4ca52b836e9f --- /dev/null +++ b/test/functional/wallet_fallbackfee.py @@ -0,0 +1,28 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test wallet replace-by-fee capabilities in conjunction with the fallbackfee.""" +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_raises_rpc_error + +class WalletRBFTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.setup_clean_chain = True + + def run_test(self): + self.nodes[0].generate(101) + + # sending a transaction without fee estimations must be possible by default on regtest + self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) + + # test sending a tx with disabled fallback fee (must fail) + self.restart_node(0, extra_args=["-fallbackfee=0"]) + assert_raises_rpc_error(-4, "Fee estimation failed", lambda: self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)) + assert_raises_rpc_error(-4, "Fee estimation failed", lambda: self.nodes[0].fundrawtransaction(self.nodes[0].createrawtransaction([], {self.nodes[0].getnewaddress(): 1}))) + assert_raises_rpc_error(-4, "Fee estimation failed", lambda: self.nodes[0].sendfrom("", self.nodes[0].getnewaddress(), 1)) + assert_raises_rpc_error(-6, "Fee estimation failed", lambda: self.nodes[0].sendmany("", {self.nodes[0].getnewaddress(): 1})) + +if __name__ == '__main__': + WalletRBFTest().main() From 8ca90f3f99defa239ea1fe0f4d40a245dda71690 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 16 Jul 2019 16:07:14 -0400 Subject: [PATCH 10/19] Merge #15891: test: Require standard txs in regtest by default fa89badf887dcc01e5bdece248b5e7d234fee227 test: Require standard txs in regtest (MarcoFalke) fa9b4191609c3ef75e69d391eb91e4d5c1e0bcf5 test: Add test that mainnet requires standard txs (MarcoFalke) fa613ca0a8f99c4771859de9e571878530d3ecb5 chainparams: Remove unused fMineBlocksOnDemand (MarcoFalke) Pull request description: I don't see a reason why regtest should allow non-standard txs, as it makes testing mainnet behaviour such as #15846 unnecessarily hard and unintuitive. Of course, testnet policy remains unchanged to allow propagation of non-standard txs. ACKs for top commit: ajtowns: ACK fa89badf887dcc01e5bdece248b5e7d234fee227 Tree-SHA512: c4c675affb054868850bd2683aa07f4c741a448cbacb2ea8334191e105f426b0790fe6a468be61e9c5880d24154f7bf1c7075051697172dce92180c1bc3a1c90 --- src/chainparams.cpp | 10 +++++----- src/chainparams.h | 8 +++++--- src/init.cpp | 3 ++- test/functional/feature_bip68_sequence.py | 5 ++++- test/functional/feature_block.py | 2 +- test/functional/feature_cltv.py | 7 ++++++- test/functional/feature_config_args.py | 5 +++++ test/functional/feature_dip0020_activation.py | 3 ++- test/functional/feature_fee_estimation.py | 6 +++--- test/functional/feature_maxuploadtarget.py | 2 +- test/functional/mempool_accept.py | 1 - test/functional/mempool_limit.py | 6 +++++- test/functional/mining_prioritisetransaction.py | 5 ++++- test/functional/p2p_compactblocks.py | 5 ++++- test/functional/p2p_invalid_tx.py | 5 ++++- test/functional/wallet_basic.py | 5 ++++- 16 files changed, 55 insertions(+), 23 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 995b233cc991..f91cd17b9bef 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -309,7 +309,7 @@ class CMainParams : public CChainParams { fDefaultConsistencyChecks = false; fRequireStandard = true; fRequireRoutableExternalIP = true; - fMineBlocksOnDemand = false; + m_is_test_chain = false; fAllowMultipleAddressesFromGroup = false; fAllowMultiplePorts = false; nLLMQConnectionRetryTimeout = 60; @@ -525,7 +525,7 @@ class CTestNetParams : public CChainParams { fDefaultConsistencyChecks = false; fRequireStandard = false; fRequireRoutableExternalIP = true; - fMineBlocksOnDemand = false; + m_is_test_chain = true; fAllowMultipleAddressesFromGroup = false; fAllowMultiplePorts = true; nLLMQConnectionRetryTimeout = 60; @@ -726,7 +726,7 @@ class CDevNetParams : public CChainParams { fDefaultConsistencyChecks = false; fRequireStandard = false; fRequireRoutableExternalIP = true; - fMineBlocksOnDemand = false; + m_is_test_chain = true; fAllowMultipleAddressesFromGroup = true; fAllowMultiplePorts = true; nLLMQConnectionRetryTimeout = 60; @@ -908,9 +908,9 @@ class CRegTestParams : public CChainParams { vSeeds.clear(); //!< Regtest mode doesn't have any DNS seeds. fDefaultConsistencyChecks = true; - fRequireStandard = false; + fRequireStandard = true; fRequireRoutableExternalIP = false; - fMineBlocksOnDemand = true; + m_is_test_chain = true; fAllowMultipleAddressesFromGroup = true; fAllowMultiplePorts = true; nLLMQConnectionRetryTimeout = 1; // must be lower then the LLMQ signing session timeout so that tests have control over failing behavior diff --git a/src/chainparams.h b/src/chainparams.h index adb4b725f1b6..09808800e636 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -70,13 +70,15 @@ class CChainParams bool RequireStandard() const { return fRequireStandard; } /** Require addresses specified with "-externalip" parameter to be routable */ bool RequireRoutableExternalIP() const { return fRequireRoutableExternalIP; } + /** If this is a test chain */ + bool IsTestChain() const { return m_is_test_chain; } uint64_t PruneAfterHeight() const { return nPruneAfterHeight; } /** Minimum free space (in GB) needed for data directory */ uint64_t AssumedBlockchainSize() const { return m_assumed_blockchain_size; } /** Minimum free space (in GB) needed for data directory when pruned; Does not include prune target*/ uint64_t AssumedChainStateSize() const { return m_assumed_chain_state_size; } - /** Make miner stop after a block is found. In RPC, don't return until nGenProcLimit blocks are generated */ - bool MineBlocksOnDemand() const { return fMineBlocksOnDemand; } + /** Whether it is possible to mine blocks on demand (no retargeting) */ + bool MineBlocksOnDemand() const { return consensus.fPowNoRetargeting; } /** Allow multiple addresses to be selected from the same network group (e.g. 192.168.x.x) */ bool AllowMultipleAddressesFromGroup() const { return fAllowMultipleAddressesFromGroup; } /** Allow nodes with the same address and multiple ports */ @@ -130,7 +132,7 @@ class CChainParams bool fDefaultConsistencyChecks; bool fRequireStandard; bool fRequireRoutableExternalIP; - bool fMineBlocksOnDemand; + bool m_is_test_chain; bool fAllowMultipleAddressesFromGroup; bool fAllowMultiplePorts; int nLLMQConnectionRetryTimeout; diff --git a/src/init.cpp b/src/init.cpp index 4ceb288b7611..a6ae5ceaae80 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1494,8 +1494,9 @@ bool AppInitParameterInteraction() } fRequireStandard = !gArgs.GetBoolArg("-acceptnonstdtxn", !chainparams.RequireStandard()); - if (chainparams.RequireStandard() && !fRequireStandard) + if (!chainparams.IsTestChain() && !fRequireStandard) { return InitError(strprintf("acceptnonstdtxn is not currently supported for %s chain", chainparams.NetworkIDString())); + } nBytesPerSigOp = gArgs.GetArg("-bytespersigop", nBytesPerSigOp); if (!g_wallet_init_interface.ParameterInteraction()) return false; diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py index 5f29f5323745..eee3a67314c1 100755 --- a/test/functional/feature_bip68_sequence.py +++ b/test/functional/feature_bip68_sequence.py @@ -21,7 +21,10 @@ class BIP68Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 - self.extra_args = [[], ["-acceptnonstdtxn=0"]] + self.extra_args = [ + ["-acceptnonstdtxn=1"], + ["-acceptnonstdtxn=0"], + ] def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 8d4f6ca801b9..238acc449208 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -82,7 +82,7 @@ def set_test_params(self): # which causes RPC to hang, so we need to increase RPC timeouts self.rpc_timeout = 180 # Must set '-dip3params=2000:2000' to create pre-dip3 blocks only - self.extra_args = [['-dip3params=2000:2000']] + self.extra_args = [['-dip3params=2000:2000', '-acceptnonstdtxn=1']] # This is a consensus block test, we don't care about tx policy def setup_nodes(self): self.add_nodes(self.num_nodes, self.extra_args) diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index f4b1034e3ead..0d3de388f142 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -59,7 +59,12 @@ def cltv_validate(node, tx, height): class BIP65Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [['-whitelist=127.0.0.1', '-dip3params=9000:9000', '-par=1']] # Use only one script thread to get the exact reject reason for testing + self.extra_args = [[ + '-whitelist=127.0.0.1', + '-dip3params=9000:9000', + '-par=1', # Use only one script thread to get the exact reject reason for testing + '-acceptnonstdtxn=1', # cltv_invalidate is nonstandard + ]] self.setup_clean_chain = True self.rpc_timeout = 120 diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 16b659885326..450450d84203 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -40,6 +40,11 @@ def test_config_file_parser(self): conf.write("wallet=foo\n") self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Config setting for -wallet only applied on regtest network when in [regtest] section.') + with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: + conf.write('regtest=0\n') # mainnet + conf.write('acceptnonstdtxn=1\n') + self.nodes[0].assert_start_raises_init_error(expected_msg='Error: acceptnonstdtxn is not currently supported for main chain') + with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: conf.write('nono\n') self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 1: nono, if you intended to specify a negated option, use nono=1 instead') diff --git a/test/functional/feature_dip0020_activation.py b/test/functional/feature_dip0020_activation.py index 7359723f06ba..471e4fdc667a 100755 --- a/test/functional/feature_dip0020_activation.py +++ b/test/functional/feature_dip0020_activation.py @@ -19,6 +19,7 @@ class DIP0020ActivationTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 + self.extra_args = [["-acceptnonstdtxn=1"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -31,7 +32,7 @@ def run_test(self): utxos = self.node.listunspent() assert len(utxos) > 0 - # Send some coins to a P2SH address constructed using disabled opcodes + # Lock some coins using disabled opcodes utxo = utxos[len(utxos) - 1] value = int(satoshi_round(utxo["amount"] - self.relayfee) * COIN) tx = CTransaction() diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index c000df218236..692c71a2f658 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -126,9 +126,9 @@ def set_test_params(self): self.num_nodes = 3 # mine non-standard txs (e.g. txs with "dust" outputs) self.extra_args = [ - ["-maxorphantxsize=1000", "-whitelist=127.0.0.1"], - ["-blockmaxsize=17000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"], - ["-blockmaxsize=8000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"] + ["-acceptnonstdtxn=1", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"], + ["-acceptnonstdtxn=1", "-blockmaxsize=17000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"], + ["-acceptnonstdtxn=1", "-blockmaxsize=8000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"] ] def skip_test_if_missing_module(self): diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py index f12ec8a354d0..8341d149795e 100755 --- a/test/functional/feature_maxuploadtarget.py +++ b/test/functional/feature_maxuploadtarget.py @@ -35,7 +35,7 @@ class MaxUploadTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - self.extra_args = [["-maxuploadtarget=200", "-blockmaxsize=999000", "-maxtipage="+str(2*60*60*24*7)]] + self.extra_args = [["-maxuploadtarget=200", "-blockmaxsize=999000", "-maxtipage="+str(2*60*60*24*7), "-acceptnonstdtxn=1"]] # Cache for utxos, as the listunspent may take a long time later in the test self.utxo_cache = [] diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 5f994b983730..f58de536f14c 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -38,7 +38,6 @@ def set_test_params(self): self.extra_args = [[ '-txindex', '-reindex', # Need reindex for txindex - '-acceptnonstdtxn=0', # Try to mimic main-net ]] * self.num_nodes def skip_test_if_missing_module(self): diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index 00da7c5dbf72..ee43520ac34f 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -13,7 +13,11 @@ class MempoolLimitTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - self.extra_args = [["-maxmempool=5", "-spendzeroconfchange=0"]] + self.extra_args = [[ + "-acceptnonstdtxn=1", + "-maxmempool=5", + "-spendzeroconfchange=0", + ]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index 0c47c56bff43..6df4f44df6b5 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -12,7 +12,10 @@ class PrioritiseTransactionTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 - self.extra_args = [["-printpriority=1"]] * 2 + self.extra_args = [[ + "-printpriority=1", + "-acceptnonstdtxn=1", + ]] * self.num_nodes def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index 86a61c3abcb3..9c026277e2c7 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -93,7 +93,10 @@ def set_test_params(self): self.setup_clean_chain = True # both nodes has the same version self.num_nodes = 2 - self.extra_args = [["-txindex"]] * 2 + self.extra_args = [[ + "-txindex", + "-acceptnonstdtxn=1", + ]] * 2 self.utxos = [] def skip_test_if_missing_module(self): diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py index d13c18ded936..322bb5899877 100755 --- a/test/functional/p2p_invalid_tx.py +++ b/test/functional/p2p_invalid_tx.py @@ -27,6 +27,9 @@ class InvalidTxRequestTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 + self.extra_args = [[ + "-acceptnonstdtxn=1", + ]] self.setup_clean_chain = True def bootstrap_p2p(self, *, num_connections=1): @@ -100,7 +103,7 @@ def run_test(self): self.test_orphan_tx_handling(block1.vtx[0].sha256, False) self.log.info('Test orphan transaction handling, resolve via block') - self.restart_node(0, ['-persistmempool=0']) + self.restart_node(0, ["-acceptnonstdtxn=1", '-persistmempool=0']) self.reconnect_p2p(num_connections=2) self.test_orphan_tx_handling(block2.vtx[0].sha256, True) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index fe766073a57d..6d00e5ff77ee 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -21,8 +21,11 @@ class WalletTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 + self.extra_args = [[ + "-acceptnonstdtxn=1", + '-usehd={:d}'.format(i%2==0), + ] for i in range(self.num_nodes)] self.setup_clean_chain = True - self.extra_args = [['-usehd={:d}'.format(i%2==0)] for i in range(4)] def skip_test_if_missing_module(self): self.skip_if_no_wallet() From 0f6c7b98a5064671042bace38e3fc404f4bf15e5 Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Sat, 27 Jul 2019 22:28:24 +1200 Subject: [PATCH 11/19] Merge #16402: Remove wallet settings from chainparams fa4a605a4c611abe9af4c18aab20f4d1d039170f Remove wallet settings from chainparams (MarcoFalke) Pull request description: Feels a bit odd to have wallet setting in the chainparams, so remove them from there ACKs for top commit: promag: ACK fa4a605a4c611abe9af4c18aab20f4d1d039170f, missed s/2018/2019? practicalswift: utACK fa4a605a4c611abe9af4c18aab20f4d1d039170f darosior: ACK fa4a605a4c611abe9af4c18aab20f4d1d039170f Tree-SHA512: 2b3a5ee85d36af290d7db80bed1339e3c684607f1ce61cc65c906726e9174e40325fb1f67a34d8780f2a61fa39a1785e7c3a1cef5b6d6c364f38db5300cdbe3a --- src/chainparams.cpp | 9 --------- src/chainparams.h | 5 +---- src/chainparamsbase.h | 1 - src/wallet/wallet.cpp | 3 +-- src/wallet/wallet.h | 1 - 5 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index f91cd17b9bef..b6319015f6b5 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -360,9 +360,6 @@ class CMainParams : public CChainParams { // (the tx=... number in the ChainStateFlushed debug.log lines) 0.3 // * estimated number of transactions per second after that timestamp }; - - /* disable fallback fee on mainnet */ - m_fallback_fee_enabled = false; } }; @@ -556,9 +553,6 @@ class CTestNetParams : public CChainParams { // (the tx=... number in the ChainStateFlushed debug.log lines) 0.01 // * estimated number of transactions per second after that timestamp }; - - /* enable fallback fee on testnet */ - m_fallback_fee_enabled = true; } }; @@ -1025,9 +1019,6 @@ class CRegTestParams : public CChainParams { params->minSize = threshold; params->threshold = threshold; params->dkgBadVotesThreshold = threshold; - - /* enable fallback fee on regtest */ - m_fallback_fee_enabled = true; } void UpdateLLMQTestParametersFromArgs(const ArgsManager& args); }; diff --git a/src/chainparams.h b/src/chainparams.h index 09808800e636..a50f9cd7754d 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -70,7 +70,7 @@ class CChainParams bool RequireStandard() const { return fRequireStandard; } /** Require addresses specified with "-externalip" parameter to be routable */ bool RequireRoutableExternalIP() const { return fRequireRoutableExternalIP; } - /** If this is a test chain */ + /** If this chain is exclusively used for testing */ bool IsTestChain() const { return m_is_test_chain; } uint64_t PruneAfterHeight() const { return nPruneAfterHeight; } /** Minimum free space (in GB) needed for data directory */ @@ -87,8 +87,6 @@ class CChainParams int LLMQConnectionRetryTimeout() const { return nLLMQConnectionRetryTimeout; } /** Return the BIP70 network string (main, test or regtest) */ std::string NetworkIDString() const { return strNetworkID; } - /** Return true if the fallback fee is by default enabled for this network */ - bool IsFallbackFeeEnabled() const { return m_fallback_fee_enabled; } /** Return the list of hostnames to look up for DNS seeds */ const std::vector& DNSSeeds() const { return vSeeds; } const std::vector& Base58Prefix(Base58Type type) const { return base58Prefixes[type]; } @@ -138,7 +136,6 @@ class CChainParams int nLLMQConnectionRetryTimeout; CCheckpointData checkpointData; ChainTxData chainTxData; - bool m_fallback_fee_enabled; int nPoolMinParticipants; int nPoolMaxParticipants; int nFulfilledRequestExpireTime; diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h index ce48376e060d..47133e7e271f 100644 --- a/src/chainparamsbase.h +++ b/src/chainparamsbase.h @@ -7,7 +7,6 @@ #include #include -#include /** * CBaseChainParams defines the base parameters (shared between dash-cli and dashd) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 87ec08080387..16e66619f7da 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5103,8 +5103,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->m_min_fee = CFeeRate(n); } - // TODO: enable when IsFallbackFeeEnabled is backported - // walletInstance->m_allow_fallback_fee = Params().IsFallbackFeeEnabled(); + walletInstance->m_allow_fallback_fee = Params().IsTestChain(); if (gArgs.IsArgSet("-fallbackfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-fallbackfee", ""), nFeePerK)) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5723b398e3cd..afcfab9d1fec 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -12,7 +12,6 @@ #include #include #include -#include #include