From d34c0009eeb49290d3512d34814d983327479513 Mon Sep 17 00:00:00 2001 From: merge-script Date: Sat, 25 Sep 2021 09:39:42 +0200 Subject: [PATCH 01/10] Merge bitcoin/bitcoin#23086: test: Add -testactivationheight tests to rpc_blockchain fa4ca8d5797adb9ad6aa0f150a5f187ebfb714c7 test: Add -testactivationheight tests to rpc_blockchain (MarcoFalke) Pull request description: Suggested: https://github.com/bitcoin/bitcoin/pull/22818#discussion_r712513991 ACKs for top commit: laanwj: Code review ACK fa4ca8d5797adb9ad6aa0f150a5f187ebfb714c7 theStack: Concept and code-review ACK fa4ca8d5797adb9ad6aa0f150a5f187ebfb714c7 Tree-SHA512: 41304db1a15c0c705a9cc2808c9f1d7831a321a8a7948a28ec5d3ee1ed3da6a0ce67cd50c99a33aaed86830c59608eb6ffadbeaba67d95245c490f9b6c277912 --- src/chainparams.cpp | 1 + test/functional/rpc_blockchain.py | 59 +++++++++++++++++++++++-------- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 90b43915583e..6eeea80802e9 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -1028,6 +1028,7 @@ static void MaybeUpdateHeights(const ArgsManager& args, Consensus::Params& conse } else if (name == "dip0008") { consensus.DIP0008Height = int{height}; } else if (name == "dip0024") { + consensus.DIP0024Height = int{height}; consensus.DIP0024QuorumsHeight = int{height}; } else if (name == "v19") { consensus.V19Height = int{height}; diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 3ef3017cf2ea..219d93fa1d14 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -157,7 +157,38 @@ def _test_getblockchaininfo(self): # should have exact keys assert_equal(sorted(res.keys()), keys) - self.restart_node(0, ['-stopatheight=207', '-prune=550', '-txindex=0']) + self.stop_node(0) + self.nodes[0].assert_start_raises_init_error( + extra_args=['-testactivationheight=name@2'], + expected_msg='Error: Invalid name (name@2) for -testactivationheight=name@height.', + ) + self.nodes[0].assert_start_raises_init_error( + extra_args=['-testactivationheight=bip34@-2'], + expected_msg='Error: Invalid height value (bip34@-2) for -testactivationheight=name@height.', + ) + self.nodes[0].assert_start_raises_init_error( + extra_args=['-testactivationheight='], + expected_msg='Error: Invalid format () for -testactivationheight=name@height.', + ) + self.start_node(0, extra_args=[ + '-stopatheight=207', + '-prune=550', + '-txindex=0', + '-testactivationheight=bip34@2', + '-testactivationheight=dersig@3', + '-testactivationheight=cltv@4', + '-testactivationheight=csv@5', + '-testactivationheight=bip147@6', + '-testactivationheight=dip0001@10', + '-dip3params=411:511', + '-testactivationheight=dip0008@12', + '-testactivationheight=dip0024@13', + '-testactivationheight=brr@14', + '-testactivationheight=v19@15', + '-testactivationheight=v20@901', + '-testactivationheight=mn_rr@902', + ]) + res = self.nodes[0].getblockchaininfo() # result should have these additional pruning keys if prune=550 assert_equal(sorted(res.keys()), sorted(['pruneheight', 'automatic_pruning', 'prune_target_size'] + keys)) @@ -169,20 +200,20 @@ def _test_getblockchaininfo(self): assert_equal(res['prune_target_size'], 576716800) assert_greater_than(res['size_on_disk'], 0) assert_equal(res['softforks'], { - 'bip34': {'type': 'buried', 'active': True, 'height': 1}, - 'bip66': {'type': 'buried', 'active': True, 'height': 1}, - 'bip65': {'type': 'buried', 'active': True, 'height': 1}, - 'bip147': { 'type': 'buried', 'active': True, 'height': 1}, - 'csv': {'type': 'buried', 'active': True, 'height': 1}, - 'dip0001': { 'type': 'buried', 'active': True, 'height': 1}, - 'dip0003': { 'type': 'buried', 'active': False, 'height': 432}, - 'dip0008': { 'type': 'buried', 'active': True, 'height': 1}, + 'bip34': {'type': 'buried', 'active': True, 'height': 2}, + 'bip66': {'type': 'buried', 'active': True, 'height': 3}, + 'bip65': {'type': 'buried', 'active': True, 'height': 4}, + 'csv': {'type': 'buried', 'active': True, 'height': 5}, + 'bip147': {'type': 'buried', 'active': True, 'height': 6}, + 'dip0001': { 'type': 'buried', 'active': True, 'height': 10}, + 'dip0003': { 'type': 'buried', 'active': False, 'height': 411}, + 'dip0008': { 'type': 'buried', 'active': True, 'height': 12}, 'dip0020': { 'type': 'buried', 'active': True, 'height': 1}, - 'dip0024': { 'type': 'buried', 'active': True, 'height': 1}, - 'realloc': { 'type': 'buried', 'active': True, 'height': 1}, - 'v19': { 'type': 'buried', 'active': True, 'height': 1}, - 'v20': { 'type': 'buried', 'active': False, 'height': 900}, - 'mn_rr': { 'type': 'buried', 'active': False, 'height': 900}, + 'dip0024': { 'type': 'buried', 'active': True, 'height': 13}, + 'realloc': { 'type': 'buried', 'active': True, 'height': 14}, + 'v19': { 'type': 'buried', 'active': True, 'height': 15}, + 'v20': { 'type': 'buried', 'active': False, 'height': 901}, + 'mn_rr': { 'type': 'buried', 'active': False, 'height': 902}, 'withdrawals': { 'type': 'bip9', 'bip9': { From 078a79114666064cd090b3c61319116d964bd673 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 13 Mar 2022 10:20:10 +0100 Subject: [PATCH 02/10] Merge bitcoin/bitcoin#24527: test: set segwit height back to 0 on regtest 5ce3057c8e8f192921fd5e4bdb95bb15e3f7dbad test: set segwit height back to 0 on regtest (Martin Zumsande) Pull request description: The change of `consensus.SegwitHeight` from 0 to 1 for regtest in #22818 had the effect that if I create a regtest enviroment with current master (or 23.x), and then try to load this chain with an older version (22.x), I get an InitError `Witness data for blocks after height 0 requires validation. Please restart with -reindex` and have to reindex because `BLOCK_OPT_WITNESS` is no longer set for the Genesis block and `NeedsRedownload()` in validation returns `true` with an older version. That might be a bit annoying for tests that use a shared regtest dir with different versions. If people think this is enough of an issue to be worth fixing, I think it should also make it into 23.x ACKs for top commit: theStack: Concept and code-review ACK 5ce3057c8e8f192921fd5e4bdb95bb15e3f7dbad Tree-SHA512: b0e89ff7fc953bc0ae929d2da44cde7149321d987fb4763934f6c9635d00d807129a50b459cc5e69e86bb1819e4b063b969486e8016a1cb8db8f905fa315653d --- src/chainparams.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 6eeea80802e9..66ead16b3011 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -786,7 +786,7 @@ class CRegTestParams : public CChainParams { consensus.BIP34Hash = uint256(); consensus.BIP65Height = 1; // Always active unless overridden consensus.BIP66Height = 1; // Always active unless overridden - consensus.BIP147Height = 1; // Always active unless overridden + consensus.BIP147Height = 0; // Always active unless overridden consensus.CSVHeight = 1; // Always active unless overridden consensus.DIP0001Height = 1; // Always active unless overridden consensus.DIP0003Height = 432; // Always active for DashTestFramework in functional tests (see dip3params) From 7f53669671b5de6ee7b304738738892357b5577b Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 20 Aug 2021 17:38:38 +0200 Subject: [PATCH 03/10] Merge bitcoin/bitcoin#22707: test: refactor use of getrawmempool in functional tests for efficiency 47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz) 77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz) 86dbd54ae8a8f9c693c0ea67114bbff24a0754df test: improve mempool_updatefrom efficiency by using getmempoolentry for specific txns (Michael Dietz) Pull request description: I don't think this changes the intention of the test. But it does shave ~30 seconds off the time it takes to run. From what I've seen our CI `macOS 11 native [gui] [no depends]` runs `mempool_updatefrom.py` in ~135 seconds. After this PR it should run in ~105 seconds I noticed this improvement should probably be made when testing performance/runtimes of https://github.com/bitcoin/bitcoin/pull/22698. But I wanted to separate this out from that PR so the affects of each is decoupled Edit: The major change in this PR is improving mempool_updatefrom.py's runtime as this is a very long running test. Then made the same efficiency improvements across all the functional tests as it made since to do that here ACKs for top commit: theStack: Tested ACK 47c48b5f35b4910fcf87caa6e37407e67d879c80 Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b --- test/functional/mempool_unbroadcast.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/functional/mempool_unbroadcast.py b/test/functional/mempool_unbroadcast.py index 45abcc5a6461..1a190667b156 100755 --- a/test/functional/mempool_unbroadcast.py +++ b/test/functional/mempool_unbroadcast.py @@ -90,9 +90,7 @@ def test_broadcast(self): self.log.info("Rebroadcast transaction and ensure it is not added to unbroadcast set when already in mempool") rpc_tx_hsh = node.sendrawtransaction(txFS["hex"]) - mempool = node.getrawmempool(True) - assert rpc_tx_hsh in mempool - assert not mempool[rpc_tx_hsh]['unbroadcast'] + assert not node.getmempoolentry(rpc_tx_hsh)['unbroadcast'] def test_txn_removal(self): self.log.info("Test that transactions removed from mempool are removed from unbroadcast set") From fec52a2c51708dae8ea3b6664f82e2673f9568d2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 23 Aug 2021 09:34:22 +0200 Subject: [PATCH 04/10] Merge bitcoin/bitcoin#22641: test: Split rpc_signmessage test for disabled wallet a3b559c970ada3c123ae24f21e45892734e7d494 test: added test for disabled wallet (Shubhankar Gambhir) Pull request description: This PR enables a part of the non-wallet functional test (rpc_signmessage.py) to be run even with the Bitcoin Core wallet disabled, it is inspired by #20078. Divided tests in rpc_signmessage.py into 2 files wallet_signmessagewithaddress.py and rpc_signmessagewithprivkey.py, latter one can run even when wallet is disabled that provides extra test which was not performed earlier. * we need bitcoincore wallet to run rpc_signmessage.py, but it is olny required for signing messages with address and not for signing messages wih private key, so latter one can be in a seperate test which can run without wallet * verifying message doesn't require wallet, so it can be used in both tests without any problem * 2 tests are named as wallet_signmessagewithaddress.py and rpc_signmessagewithprivkey.py to provide clarity of what they are testing. ACKs for top commit: vasild: ACK a3b559c970ada3c123ae24f21e45892734e7d494 theStack: Code-review ACK a3b559c970ada3c123ae24f21e45892734e7d494 Tree-SHA512: 1bfca3baf3123a02f0a2389e55e141d64430c3bed40ff5a5fb97ef2c66e2853c46e4b2dff62b948eb94dc574cb89d061769330f0535e2d5d1be76b60101136ac --- ...ssage.py => rpc_signmessagewithprivkey.py} | 27 +++--------- test/functional/test_runner.py | 3 +- .../wallet_signmessagewithaddress.py | 44 +++++++++++++++++++ 3 files changed, 52 insertions(+), 22 deletions(-) rename test/functional/{rpc_signmessage.py => rpc_signmessagewithprivkey.py} (62%) create mode 100755 test/functional/wallet_signmessagewithaddress.py diff --git a/test/functional/rpc_signmessage.py b/test/functional/rpc_signmessagewithprivkey.py similarity index 62% rename from test/functional/rpc_signmessage.py rename to test/functional/rpc_signmessagewithprivkey.py index 9c1dab889355..34df3bd4d4ab 100755 --- a/test/functional/rpc_signmessage.py +++ b/test/functional/rpc_signmessagewithprivkey.py @@ -2,7 +2,7 @@ # Copyright (c) 2016 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 RPC commands for signing and verifying messages.""" +"""Test RPC commands for signing messages with private key.""" from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -10,14 +10,11 @@ assert_raises_rpc_error, ) -class SignMessagesTest(BitcoinTestFramework): +class SignMessagesWithPrivTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - def run_test(self): message = 'This is just a test message' @@ -29,33 +26,21 @@ def run_test(self): assert_equal(expected_signature, signature) assert self.nodes[0].verifymessage(address, signature, message) - self.log.info('test signing with an address with wallet') - address = self.nodes[0].getnewaddress() - signature = self.nodes[0].signmessage(address, message) - assert self.nodes[0].verifymessage(address, signature, message) - - self.log.info('test verifying with another address should not work') - other_address = self.nodes[0].getnewaddress() - other_signature = self.nodes[0].signmessage(other_address, message) - assert not self.nodes[0].verifymessage(other_address, signature, message) - assert not self.nodes[0].verifymessage(address, other_signature, message) - self.log.info('test parameter validity and error codes') - # signmessage(withprivkey) have two required parameters + # signmessagewithprivkey has two required parameters for num_params in [0, 1, 3, 4, 5]: param_list = ["dummy"]*num_params assert_raises_rpc_error(-1, "signmessagewithprivkey", self.nodes[0].signmessagewithprivkey, *param_list) - assert_raises_rpc_error(-1, "signmessage", self.nodes[0].signmessage, *param_list) # verifymessage has three required parameters for num_params in [0, 1, 2, 4, 5]: param_list = ["dummy"]*num_params assert_raises_rpc_error(-1, "verifymessage", self.nodes[0].verifymessage, *param_list) # invalid key or address provided assert_raises_rpc_error(-5, "Invalid private key", self.nodes[0].signmessagewithprivkey, "invalid_key", message) - assert_raises_rpc_error(-5, "Invalid address", self.nodes[0].signmessage, "invalid_addr", message) assert_raises_rpc_error(-5, "Invalid address", self.nodes[0].verifymessage, "invalid_addr", signature, message) # malformed signature provided - assert_raises_rpc_error(-3, "Malformed base64 encoding", self.nodes[0].verifymessage, self.nodes[0].getnewaddress(), "invalid_sig", message) + + assert_raises_rpc_error(-3, "Malformed base64 encoding", self.nodes[0].verifymessage, 'yeMpGzMj3rhtnz48XsfpB8itPHhHtgxLc3', "invalid_sig", message) if __name__ == '__main__': - SignMessagesTest().main() + SignMessagesWithPrivTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index d3735a50fda8..531c71e884c4 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -279,7 +279,8 @@ 'p2p_ibd_stalling.py --v2transport', 'p2p_net_deadlock.py --v1transport', 'p2p_net_deadlock.py --v2transport', - 'rpc_signmessage.py', + 'wallet_signmessagewithaddress.py', + 'rpc_signmessagewithprivkey.py', 'rpc_generateblock.py', 'rpc_generate.py', 'wallet_balance.py --legacy-wallet', diff --git a/test/functional/wallet_signmessagewithaddress.py b/test/functional/wallet_signmessagewithaddress.py new file mode 100755 index 000000000000..df5e36bf3dde --- /dev/null +++ b/test/functional/wallet_signmessagewithaddress.py @@ -0,0 +1,44 @@ +#!/usr/bin/env python3 +# Copyright (c) 2016-2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test Wallet commands for signing and verifying messages.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_raises_rpc_error, +) + +class SignMessagesWithAddressTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + message = 'This is just a test message' + + self.log.info('test signing with an address with wallet') + address = self.nodes[0].getnewaddress() + signature = self.nodes[0].signmessage(address, message) + assert self.nodes[0].verifymessage(address, signature, message) + + self.log.info('test verifying with another address should not work') + other_address = self.nodes[0].getnewaddress() + other_signature = self.nodes[0].signmessage(other_address, message) + assert not self.nodes[0].verifymessage(other_address, signature, message) + assert not self.nodes[0].verifymessage(address, other_signature, message) + + self.log.info('test parameter validity and error codes') + # signmessage has two required parameters + for num_params in [0, 1, 3, 4, 5]: + param_list = ["dummy"]*num_params + assert_raises_rpc_error(-1, "signmessage", self.nodes[0].signmessage, *param_list) + # invalid key or address provided + assert_raises_rpc_error(-5, "Invalid address", self.nodes[0].signmessage, "invalid_addr", message) + + +if __name__ == '__main__': + SignMessagesWithAddressTest().main() From 2dbb2c57f48e9e7778848434dd825dd1cf4aab05 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 31 Aug 2021 20:45:23 +0800 Subject: [PATCH 05/10] Merge bitcoin/bitcoin#22744: ci: Re-enable verify-commits.py check fa001602cd5ac61b9258e998ee2b236688c19ef7 ci: Re-enable verify-commits.py check (MarcoFalke) fa880b10d67542b8eb476a0e1f3ffb67e88d5e53 ci: Unconditionally set the global git author name in cirrys.yml (MarcoFalke) Pull request description: Might be useful to detect bugs in the script itself or an accidentally missed signature. ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/pull/22744/commits/fa001602cd5ac61b9258e998ee2b236688c19ef7 Zero-1729: tACK fa001602cd5ac61b9258e998ee2b236688c19ef7 fanquake: untested ACK fa001602cd5ac61b9258e998ee2b236688c19ef7 Tree-SHA512: 8a13a67d325f2477f4088d1034f0d5e4e04937a01ee3c738435fe66394c02b9f33225529952ad331b0ba19b63ca4b2f26911cb5d264890159840cf3e09085969 --- .cirrus.yml | 6 ++++-- ci/lint/06_script.sh | 11 ++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index e9c39edca518..a2f720b6af63 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -13,11 +13,13 @@ env: base_template: &BASE_TEMPLATE skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == "" # No need to run on the read-only mirror, unless it is a PR. https://cirrus-ci.org/guide/writing-tasks/#conditional-task-execution merge_base_script: - - if [ "$CIRRUS_PR" = "" ]; then exit 0; fi + # Unconditionally install git (used in fingerprint_script) and set the + # default git author name (used in verify-commits.py) - bash -c "$PACKAGE_MANAGER_INSTALL git" - - git fetch $CIRRUS_REPO_CLONE_URL $CIRRUS_BASE_BRANCH - git config --global user.email "ci@ci.ci" - git config --global user.name "ci" + - if [ "$CIRRUS_PR" = "" ]; then exit 0; fi + - git fetch $CIRRUS_REPO_CLONE_URL $CIRRUS_BASE_BRANCH - git merge FETCH_HEAD # Merge base to detect silent merge conflicts stateful: false # https://cirrus-ci.org/guide/writing-tasks/#stateful-tasks diff --git a/ci/lint/06_script.sh b/ci/lint/06_script.sh index 02dd9e17f71b..ee926132e282 100755 --- a/ci/lint/06_script.sh +++ b/ci/lint/06_script.sh @@ -28,9 +28,14 @@ test/lint/git-subtree-check.sh src/leveldb test/lint/check-doc.py test/lint/all-lint.py -if [ "$CIRRUS_REPO_FULL_NAME" = "dashpay/dash" ] && [ -n "$CIRRUS_CRON" ]; then - git log --merges --before="2 days ago" -1 --format='%H' > ./contrib/verify-commits/trusted-sha512-root-commit +if [ "$CIRRUS_REPO_FULL_NAME" = "dashpay/dash" ] && [ "$CIRRUS_PR" = "" ] ; then + # Sanity check only the last few commits to get notified of missing sigs, + # missing keys, or expired keys. Usually there is only one new merge commit + # per push on the master branch and a few commits on release branches, so + # sanity checking only a few (10) commits seems sufficient and cheap. + git log HEAD~10 -1 --format='%H' > ./contrib/verify-commits/trusted-sha512-root-commit + git log HEAD~10 -1 --format='%H' > ./contrib/verify-commits/trusted-git-root mapfile -t KEYS < contrib/verify-commits/trusted-keys ${CI_RETRY_EXE} gpg --keyserver hkps://keys.openpgp.org --recv-keys "${KEYS[@]}" && - ./contrib/verify-commits/verify-commits.py --clean-merge=2; + ./contrib/verify-commits/verify-commits.py; fi From 27ddee7a509978ca3e737aa05ee7358fe4c77a66 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 1 Sep 2021 18:16:13 +0200 Subject: [PATCH 06/10] Merge bitcoin/bitcoin#22437: test, refactor: add GetTransaction() coverage, improve rpc_rawtransaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 387355bb9482a09c1fc9b137bea56745a93b7dfd test, refactor: rpc_rawtransaction PEP8 (Jon Atack) 7d5cec2e498dc059ff1d74a2b60764db45923264 refactor: separate the rpc_rawtransaction tests into functions (Jon Atack) 409779df95f886b08dbf6d44219e2fbeb3405a43 move-only: regroup similar rpc_rawtransaction tests together (Jon Atack) d861040dd24a321e1ceec1f07c7bb80d59779081 test: remove no longer needed (ASCII art) comments (Jon Atack) 14398b30d6242db14670b3286f988b2badda83fb test: add and harmonize getrawtransaction logging (Jon Atack) 85d8869cf89fedf243748e3e15b3ed39de1b0385 test: run 2nd getrawtransaction section with/without -txindex (Jon Atack) 00977407732969593800d15de39abbb7e0250abc refactor: txid to constant in rpc_rawtransaction to isolate tests (Jon Atack) 8c19d1329f1f28000ca32d826cecf04680c6be69 refactor: dedup/reorg createrawtransaction sequence number tests (Jon Atack) 7f073594c9f5b518dc1fb66dfb0189e8803e3545 Test src/node/transaction::GetTransaction() without -txindex (Jon Atack) Pull request description: Following up on https://github.com/bitcoin/bitcoin/pull/22383#pullrequestreview-698583510, this pull adds missing `src/node/transaction::GetTransaction()` test coverage for combinations of `-txindex` and `blockhash` and does some refactoring of the test file. ACKs for top commit: mjdietzx: reACK 387355bb9482a09c1fc9b137bea56745a93b7dfd josibake: reACK https://github.com/bitcoin/bitcoin/pull/22437/commits/387355bb9482a09c1fc9b137bea56745a93b7dfd MarcoFalke: Approach ACK 387355bb9482a09c1fc9b137bea56745a93b7dfd 🔆 Tree-SHA512: b47c4ff87d69c61434e5729c954b338bc13744eddaba0879ca9f5f42243ba2cb4640d94c5f74de9f2735a8bf5e66b3d1c3bd3b7c26cd7324da7d3270ce87c6fd --- test/functional/rpc_rawtransaction.py | 601 ++++++++++++++------------ 1 file changed, 333 insertions(+), 268 deletions(-) diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index db8da986f177..ed0ee1d87e33 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -5,11 +5,11 @@ """Test the rawtransaction RPCs. Test the following RPCs: + - getrawtransaction - createrawtransaction - signrawtransactionwithwallet - sendrawtransaction - decoderawtransaction - - getrawtransaction """ from collections import OrderedDict @@ -28,6 +28,9 @@ ) +TXID = "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000" + + class multidict(dict): """Dictionary that allows duplicate keys. @@ -46,15 +49,15 @@ def items(self): return self.x -# Create one-input, one-output, no-fee transaction: class RawTransactionsTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 3 + self.num_nodes = 4 self.extra_args = [ + [], # by default Tx Index is enabled ["-txindex"], ["-txindex"], - ["-txindex"], + ["-notxindex"], ] # whitelist all peers to speed up tx relay / mempool sync for args in self.extra_args: @@ -70,20 +73,112 @@ def setup_network(self): self.connect_nodes(0, 2) def run_test(self): - self.log.info('prepare some coins for multiple *rawtransaction commands') + self.log.info("Prepare some coins for multiple *rawtransaction commands") self.generate(self.nodes[2], 1) self.generate(self.nodes[0], COINBASE_MATURITY + 1) - self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),1.5) - self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),1.0) - self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),5.0) + for amount in [1.5, 1.0, 5.0]: + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), amount) self.sync_all() self.generate(self.nodes[0], 5) - self.log.info('Test getrawtransaction on genesis block coinbase returns an error') + self.getrawtransaction_tests() + self.createrawtransaction_tests() + self.signrawtransactionwithwallet_tests() + self.sendrawtransaction_tests() + self.sendrawtransaction_testmempoolaccept_tests() + self.transaction_version_number_tests() + if not self.options.descriptors: + self.raw_multisig_transaction_legacy_tests() + + def getrawtransaction_tests(self): + addr = self.nodes[1].getnewaddress() + txid = self.nodes[0].sendtoaddress(addr, 10) + self.generate(self.nodes[0], 1) + vout = find_vout_for_address(self.nodes[1], txid, addr) + rawTx = self.nodes[1].createrawtransaction([{'txid': txid, 'vout': vout}], {self.nodes[1].getnewaddress(): 9.999}) + rawTxSigned = self.nodes[1].signrawtransactionwithwallet(rawTx) + txId = self.nodes[1].sendrawtransaction(rawTxSigned['hex']) + self.generate(self.nodes[0], 1) + + for n in [0, 3]: + self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex") + # 1. valid parameters - only supply txid + assert_equal(self.nodes[n].getrawtransaction(txId), rawTxSigned['hex']) + assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]})[txId], rawTxSigned['hex']) + + # 2. valid parameters - supply txid and 0 for non-verbose + assert_equal(self.nodes[n].getrawtransaction(txId, 0), rawTxSigned['hex']) + assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]}, 0)[txId], rawTxSigned['hex']) + + # 3. valid parameters - supply txid and False for non-verbose + assert_equal(self.nodes[n].getrawtransaction(txId, False), rawTxSigned['hex']) + assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]}, False)[txId], rawTxSigned['hex']) + + # 4. valid parameters - supply txid and 1 for verbose. + # We only check the "hex" field of the output so we don't need to update this test every time the output format changes. + assert_equal(self.nodes[n].getrawtransaction(txId, 1)["hex"], rawTxSigned['hex']) + assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]}, 1)[txId]['hex'], rawTxSigned['hex']) + + # 5. valid parameters - supply txid and True for non-verbose + assert_equal(self.nodes[n].getrawtransaction(txId, True)["hex"], rawTxSigned['hex']) + assert_equal(self.nodes[n].getrawtransactionmulti(verbose=True, transactions={"0":[txId]})[txId]['hex'], rawTxSigned['hex']) + + # 6. invalid parameters - supply txid and invalid boolean values (strings) for verbose + for value in ["True", "False"]: + assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txid=txId, verbose=value) + assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransactionmulti, transactions={"0":[txId]}, verbose=value) + + # 7. invalid parameters - supply txid and empty array + assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, []) + + # 8. invalid parameters - supply txid and empty dict + assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, {}) + + # Make a tx by sending, then generate 2 blocks; block1 has the tx in it + tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1) + block1, block2 = self.generate(self.nodes[2], 2) + for n in [0, 3]: + self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex, with blockhash") + # We should be able to get the raw transaction by providing the correct block + gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1) + assert_equal(gottx['txid'], tx) + assert_equal(gottx['in_active_chain'], True) + if n == 0: + self.log.info("Test getrawtransaction with -txindex, without blockhash: 'in_active_chain' should be absent") + gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True) + assert_equal(gottx['txid'], tx) + assert 'in_active_chain' not in gottx + else: + self.log.info("Test getrawtransaction without -txindex, without blockhash: expect the call to raise") + err_msg = ( + "No such mempool transaction. Use -txindex or provide a block hash to enable" + " blockchain transaction queries. Use gettransaction for wallet transactions." + ) + assert_raises_rpc_error(-5, err_msg, self.nodes[n].getrawtransaction, txid=tx, verbose=True) + # We should not get the tx if we provide an unrelated block + assert_raises_rpc_error(-5, "No such transaction found", self.nodes[n].getrawtransaction, txid=tx, blockhash=block2) + # An invalid block hash should raise the correct errors + assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[n].getrawtransaction, txid=tx, blockhash=True) + assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[n].getrawtransaction, txid=tx, blockhash="foobar") + assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[n].getrawtransaction, txid=tx, blockhash="abcd1234") + foo = "ZZZ0000000000000000000000000000000000000000000000000000000000000" + assert_raises_rpc_error(-8, f"parameter 3 must be hexadecimal string (not '{foo}')", self.nodes[n].getrawtransaction, txid=tx, blockhash=foo) + bar = "0000000000000000000000000000000000000000000000000000000000000000" + assert_raises_rpc_error(-5, "Block hash not found", self.nodes[n].getrawtransaction, txid=tx, blockhash=bar) + # Undo the blocks and verify that "in_active_chain" is false. + self.nodes[n].invalidateblock(block1) + gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1) + assert_equal(gottx['in_active_chain'], False) + self.nodes[n].reconsiderblock(block1) + assert_equal(self.nodes[n].getbestblockhash(), block2) + + + self.log.info("Test getrawtransaction on genesis block coinbase returns an error") block = self.nodes[0].getblock(self.nodes[0].getblockhash(0)) assert_raises_rpc_error(-5, "The genesis block coinbase is not considered an ordinary transaction", self.nodes[0].getrawtransaction, block['merkleroot']) - self.log.info('Check parameter types and required parameters of createrawtransaction') + def createrawtransaction_tests(self): + self.log.info("Test createrawtransaction") # Test `createrawtransaction` required parameters assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction) assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, []) @@ -92,16 +187,28 @@ def run_test(self): assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, [], {}, 0, False, 'foo') # Test `createrawtransaction` invalid `inputs` - txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000' assert_raises_rpc_error(-3, "Expected type array", self.nodes[0].createrawtransaction, 'foo', {}) assert_raises_rpc_error(-1, "JSON value is not an object as expected", self.nodes[0].createrawtransaction, ['foo'], {}) assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].createrawtransaction, [{}], {}) assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {}) - assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", self.nodes[0].createrawtransaction, [{'txid': 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844'}], {}) - assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid}], {}) - assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': 'foo'}], {}) - assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': -1}], {}) - assert_raises_rpc_error(-8, "Invalid parameter, sequence number is out of range", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': 0, 'sequence': -1}], {}) + txid = "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844" + assert_raises_rpc_error(-8, f"txid must be hexadecimal string (not '{txid}')", self.nodes[0].createrawtransaction, [{'txid': txid}], {}) + assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': TXID}], {}) + assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': TXID, 'vout': 'foo'}], {}) + assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", self.nodes[0].createrawtransaction, [{'txid': TXID, 'vout': -1}], {}) + # sequence number out of range + for invalid_seq in [-1, 4294967296]: + inputs = [{'txid': TXID, 'vout': 1, 'sequence': invalid_seq}] + outputs = {self.nodes[0].getnewaddress(): 1} + assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range', + self.nodes[0].createrawtransaction, inputs, outputs) + # with valid sequence number + for valid_seq in [1000, 4294967294]: + inputs = [{'txid': TXID, 'vout': 1, 'sequence': valid_seq}] + outputs = {self.nodes[0].getnewaddress(): 1} + rawtx = self.nodes[0].createrawtransaction(inputs, outputs) + decrawtx = self.nodes[0].decoderawtransaction(rawtx) + assert_equal(decrawtx['vin'][0]['sequence'], valid_seq) # Test `createrawtransaction` invalid `outputs` address = self.nodes[0].getnewaddress() @@ -125,269 +232,89 @@ def run_test(self): assert_raises_rpc_error(-8, "Invalid parameter, locktime out of range", self.nodes[0].createrawtransaction, [], {}, -1) assert_raises_rpc_error(-8, "Invalid parameter, locktime out of range", self.nodes[0].createrawtransaction, [], {}, 4294967296) - self.log.info('Check that createrawtransaction accepts an array and object as outputs') + # Test that createrawtransaction accepts an array and object as outputs # One output - tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs={address: 99})) + tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs={address: 99})) assert_equal(len(tx.vout), 1) assert_equal( tx.serialize().hex(), - self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}]), + self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}]), ) # Two outputs - tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=OrderedDict([(address, 99), (address2, 99)]))) + tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=OrderedDict([(address, 99), (address2, 99)]))) assert_equal(len(tx.vout), 2) assert_equal( tx.serialize().hex(), - self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}, {address2: 99}]), + self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}, {address2: 99}]), ) # Multiple mixed outputs - tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=multidict([(address, 99), (address2, 99), ('data', '99')]))) + tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=multidict([(address, 99), (address2, 99), ('data', '99')]))) assert_equal(len(tx.vout), 3) assert_equal( tx.serialize().hex(), - self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}, {address2: 99}, {'data': '99'}]), + self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}, {address2: 99}, {'data': '99'}]), ) - self.log.info('sendrawtransaction with missing input') - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1}] #won't exists - outputs = { self.nodes[0].getnewaddress() : 4.998 } - rawtx = self.nodes[2].createrawtransaction(inputs, outputs) - rawtx = self.nodes[2].signrawtransactionwithwallet(rawtx) - - # This will raise an exception since there are missing inputs + def signrawtransactionwithwallet_tests(self): + for type in ["legacy"]: + self.log.info(f"Test signrawtransactionwithwallet with missing prevtx info ({type})") + addr = self.nodes[0].getnewaddress("") + addrinfo = self.nodes[0].getaddressinfo(addr) + pubkey = addrinfo["scriptPubKey"] + inputs = [{'txid': TXID, 'vout': 3, 'sequence': 1000}] + outputs = {self.nodes[0].getnewaddress(): 1} + rawtx = self.nodes[0].createrawtransaction(inputs, outputs) + + prevtx = dict(txid=TXID, scriptPubKey=pubkey, vout=3, amount=1) + succ = self.nodes[0].signrawtransactionwithwallet(rawtx, [prevtx]) + assert succ["complete"] + + if type == "legacy": + del prevtx["amount"] + succ = self.nodes[0].signrawtransactionwithwallet(rawtx, [prevtx]) + assert succ["complete"] + else: + assert_raises_rpc_error(-3, "Missing amount", self.nodes[0].signrawtransactionwithwallet, rawtx, [ + { + "txid": TXID, + "scriptPubKey": pubkey, + "vout": 3, + } + ]) + + assert_raises_rpc_error(-3, "Missing vout", self.nodes[0].signrawtransactionwithwallet, rawtx, [ + { + "txid": TXID, + "scriptPubKey": pubkey, + "amount": 1, + } + ]) + assert_raises_rpc_error(-3, "Missing txid", self.nodes[0].signrawtransactionwithwallet, rawtx, [ + { + "scriptPubKey": pubkey, + "vout": 3, + "amount": 1, + } + ]) + assert_raises_rpc_error(-3, "Missing scriptPubKey", self.nodes[0].signrawtransactionwithwallet, rawtx, [ + { + "txid": TXID, + "vout": 3, + "amount": 1 + } + ]) + + def sendrawtransaction_tests(self): + self.log.info("Test sendrawtransaction with missing input") + inputs = [{'txid': TXID, 'vout': 1}] # won't exist + outputs = {self.nodes[0].getnewaddress(): 4.998} + rawtx = self.nodes[2].createrawtransaction(inputs, outputs) + rawtx = self.nodes[2].signrawtransactionwithwallet(rawtx) assert_raises_rpc_error(-25, "bad-txns-inputs-missingorspent", self.nodes[2].sendrawtransaction, rawtx['hex']) - ##################################### - # getrawtransaction with block hash # - ##################################### - - # make a tx by sending then generate 2 blocks; block1 has the tx in it - tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1) - block1, block2 = self.generate(self.nodes[2], 2) - # We should be able to get the raw transaction by providing the correct block - gottx = self.nodes[0].getrawtransaction(tx, True, block1) - assert_equal(gottx['txid'], tx) - assert_equal(gottx['in_active_chain'], True) - # We should not have the 'in_active_chain' flag when we don't provide a block - gottx = self.nodes[0].getrawtransaction(tx, True) - assert_equal(gottx['txid'], tx) - assert 'in_active_chain' not in gottx - # We should not get the tx if we provide an unrelated block - assert_raises_rpc_error(-5, "No such transaction found", self.nodes[0].getrawtransaction, tx, True, block2) - # An invalid block hash should raise the correct errors - assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getrawtransaction, tx, True, True) - assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[0].getrawtransaction, tx, True, "foobar") - assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[0].getrawtransaction, tx, True, "abcd1234") - assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getrawtransaction, tx, True, "ZZZ0000000000000000000000000000000000000000000000000000000000000") - assert_raises_rpc_error(-5, "Block hash not found", self.nodes[0].getrawtransaction, tx, True, "0000000000000000000000000000000000000000000000000000000000000000") - # Undo the blocks and check in_active_chain - self.nodes[0].invalidateblock(block1) - gottx = self.nodes[0].getrawtransaction(txid=tx, verbose=True, blockhash=block1) - assert_equal(gottx['in_active_chain'], False) - self.nodes[0].reconsiderblock(block1) - assert_equal(self.nodes[0].getbestblockhash(), block2) - - if not self.options.descriptors: - # The traditional multisig workflow does not work with descriptor wallets so these are legacy only. - # The multisig workflow with descriptor wallets uses PSBTs and is tested elsewhere, no need to do them here. - ######################### - # RAW TX MULTISIG TESTS # - ######################### - # 2of2 test - addr1 = self.nodes[2].getnewaddress() - addr2 = self.nodes[2].getnewaddress() - - addr1Obj = self.nodes[2].getaddressinfo(addr1) - addr2Obj = self.nodes[2].getaddressinfo(addr2) - - # Tests for createmultisig and addmultisigaddress - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"]) - self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # createmultisig can only take public keys - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here. - - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address'] - - #use balance deltas instead of absolute values - bal = self.nodes[2].getbalance() - - # send 1.2 BTC to msig adr - txId = self.nodes[0].sendtoaddress(mSigObj, 1.2) - self.sync_all() - self.generate(self.nodes[0], 1) - assert_equal(self.nodes[2].getbalance(), bal+Decimal('1.20000000')) #node2 has both keys of the 2of2 ms addr., tx should affect the balance - - - # 2of3 test from different nodes - bal = self.nodes[2].getbalance() - addr1 = self.nodes[1].getnewaddress() - addr2 = self.nodes[2].getnewaddress() - addr3 = self.nodes[2].getnewaddress() - - addr1Obj = self.nodes[1].getaddressinfo(addr1) - addr2Obj = self.nodes[2].getaddressinfo(addr2) - addr3Obj = self.nodes[2].getaddressinfo(addr3) - - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']])['address'] - - txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) - decTx = self.nodes[0].gettransaction(txId) - rawTx = self.nodes[0].decoderawtransaction(decTx['hex']) - self.sync_all() - self.generate(self.nodes[0], 1) - - #THIS IS AN INCOMPLETE FEATURE - #NODE2 HAS TWO OF THREE KEY AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION - assert_equal(self.nodes[2].getbalance(), bal) #for now, assume the funds of a 2of3 multisig tx are not marked as spendable - - txDetails = self.nodes[0].gettransaction(txId, True) - rawTx = self.nodes[0].decoderawtransaction(txDetails['hex']) - vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('2.20000000')) - - bal = self.nodes[0].getbalance() - inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex']}] - outputs = { self.nodes[0].getnewaddress() : 2.19 } - rawTx = self.nodes[2].createrawtransaction(inputs, outputs) - rawTxPartialSigned = self.nodes[1].signrawtransactionwithwallet(rawTx, inputs) - assert_equal(rawTxPartialSigned['complete'], False) #node1 only has one key, can't comp. sign the tx - - rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx, inputs) - assert_equal(rawTxSigned['complete'], True) #node2 can sign the tx compl., own two of three keys - self.nodes[2].sendrawtransaction(rawTxSigned['hex']) - rawTx = self.nodes[0].decoderawtransaction(rawTxSigned['hex']) - self.sync_all() - self.generate(self.nodes[0], 1) - assert_equal(self.nodes[0].getbalance(), bal+Decimal('500.00000000')+Decimal('2.19000000')) #block reward + tx - - # 2of2 test for combining transactions - bal = self.nodes[2].getbalance() - addr1 = self.nodes[1].getnewaddress() - addr2 = self.nodes[2].getnewaddress() - - addr1Obj = self.nodes[1].getaddressinfo(addr1) - addr2Obj = self.nodes[2].getaddressinfo(addr2) - - self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] - mSigObjValid = self.nodes[2].getaddressinfo(mSigObj) - - txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) - decTx = self.nodes[0].gettransaction(txId) - rawTx2 = self.nodes[0].decoderawtransaction(decTx['hex']) - self.sync_all() - self.generate(self.nodes[0], 1) - - assert_equal(self.nodes[2].getbalance(), bal) # the funds of a 2of2 multisig tx should not be marked as spendable - - txDetails = self.nodes[0].gettransaction(txId, True) - rawTx2 = self.nodes[0].decoderawtransaction(txDetails['hex']) - vout = next(o for o in rawTx2['vout'] if o['value'] == Decimal('2.20000000')) - - bal = self.nodes[0].getbalance() - inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "redeemScript" : mSigObjValid['hex']}] - outputs = { self.nodes[0].getnewaddress() : 2.19 } - rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs) - rawTxPartialSigned1 = self.nodes[1].signrawtransactionwithwallet(rawTx2, inputs) - self.log.debug(rawTxPartialSigned1) - assert_equal(rawTxPartialSigned1['complete'], False) #node1 only has one key, can't comp. sign the tx - - rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet(rawTx2, inputs) - self.log.debug(rawTxPartialSigned2) - assert_equal(rawTxPartialSigned2['complete'], False) #node2 only has one key, can't comp. sign the tx - rawTxComb = self.nodes[2].combinerawtransaction([rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']]) - self.log.debug(rawTxComb) - self.nodes[2].sendrawtransaction(rawTxComb) - rawTx2 = self.nodes[0].decoderawtransaction(rawTxComb) - self.sync_all() - self.generate(self.nodes[0], 1) - assert_equal(self.nodes[0].getbalance(), bal+Decimal('500.00000000')+Decimal('2.19000000')) #block reward + tx - - - # Basic signrawtransaction test - addr = self.nodes[1].getnewaddress() - txid = self.nodes[0].sendtoaddress(addr, 10) - self.generate(self.nodes[0], 1) - vout = find_vout_for_address(self.nodes[1], txid, addr) - rawTx = self.nodes[1].createrawtransaction([{'txid': txid, 'vout': vout}], {self.nodes[1].getnewaddress(): 9.999}) - rawTxSigned = self.nodes[1].signrawtransactionwithwallet(rawTx) - txId = self.nodes[1].sendrawtransaction(rawTxSigned['hex']) - self.generate(self.nodes[0], 1) - - # getrawtransaction tests - # 1. valid parameters - only supply txid - assert_equal(self.nodes[0].getrawtransaction(txId), rawTxSigned['hex']) - assert_equal(self.nodes[0].getrawtransactionmulti({"0":[txId]})[txId], rawTxSigned['hex']) - - # 2. valid parameters - supply txid and 0 for non-verbose - assert_equal(self.nodes[0].getrawtransaction(txId, 0), rawTxSigned['hex']) - assert_equal(self.nodes[0].getrawtransactionmulti({"0":[txId]}, 0)[txId], rawTxSigned['hex']) - - # 3. valid parameters - supply txid and False for non-verbose - assert_equal(self.nodes[0].getrawtransaction(txId, False), rawTxSigned['hex']) - assert_equal(self.nodes[0].getrawtransactionmulti({"0":[txId]}, False)[txId], rawTxSigned['hex']) - - # 4. valid parameters - supply txid and 1 for verbose. - # We only check the "hex" field of the output so we don't need to update this test every time the output format changes. - assert_equal(self.nodes[0].getrawtransaction(txId, 1)["hex"], rawTxSigned['hex']) - assert_equal(self.nodes[0].getrawtransactionmulti({"0":[txId]}, 1)[txId]['hex'], rawTxSigned['hex']) - - # 5. valid parameters - supply txid and True for non-verbose - assert_equal(self.nodes[0].getrawtransaction(txId, True)["hex"], rawTxSigned['hex']) - assert_equal(self.nodes[0].getrawtransactionmulti(verbose=True, transactions={"0":[txId]})[txId]['hex'], rawTxSigned['hex']) - - # 6. invalid parameters - supply txid and string "Flase" - assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, "Flase") - - # 7. invalid parameters - supply txid and empty array - assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, []) - - # 8. invalid parameters - supply txid and empty dict - assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, {}) - - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 1000}] - outputs = { self.nodes[0].getnewaddress() : 1 } - rawtx = self.nodes[0].createrawtransaction(inputs, outputs) - decrawtx= self.nodes[0].decoderawtransaction(rawtx) - assert_equal(decrawtx['vin'][0]['sequence'], 1000) - - # 9. invalid parameters - sequence number out of range - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : -1}] - outputs = { self.nodes[0].getnewaddress() : 1 } - assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range', self.nodes[0].createrawtransaction, inputs, outputs) - - # 10. invalid parameters - sequence number out of range - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 4294967296}] - outputs = { self.nodes[0].getnewaddress() : 1 } - assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range', self.nodes[0].createrawtransaction, inputs, outputs) - - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 4294967294}] - outputs = { self.nodes[0].getnewaddress() : 1 } - rawtx = self.nodes[0].createrawtransaction(inputs, outputs) - decrawtx= self.nodes[0].decoderawtransaction(rawtx) - assert_equal(decrawtx['vin'][0]['sequence'], 4294967294) - - #################################### - # TRANSACTION VERSION NUMBER TESTS # - #################################### - - # Test the minimum transaction version number that fits in a signed 16-bit integer. - # Note, this is different to bitcoin. Bitcoin has a 32 bit integer - # representing the version, we have 16 bits of version and 16 bits of - # type. - # As transaction version is unsigned, this should convert to its unsigned equivalent. - tx = CTransaction() - tx.nVersion = -0x8000 - rawtx = tx.serialize().hex() - decrawtx = self.nodes[0].decoderawtransaction(rawtx) - assert_equal(decrawtx['version'], 0x8000) - - # Test the maximum transaction version number that fits in a signed 32-bit integer. - tx = CTransaction() - tx.nVersion = 0x7fff - rawtx = tx.serialize().hex() - decrawtx = self.nodes[0].decoderawtransaction(rawtx) - assert_equal(decrawtx['version'], 0x7fff) - - self.log.info('sendrawtransaction/testmempoolaccept with maxfeerate') + def sendrawtransaction_testmempoolaccept_tests(self): + self.log.info("Test sendrawtransaction/testmempoolaccept with maxfeerate") + fee_exceeds_max = "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)" # Test a transaction with a small fee. txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) @@ -395,9 +322,9 @@ def run_test(self): vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) self.sync_all() - inputs = [{ "txid" : txId, "vout" : vout['n'] }] + inputs = [{"txid": txId, "vout": vout['n']}] # Fee 10,000 satoshis, (1 - (10000 sat * 0.00000001 BTC/sat)) = 0.9999 - outputs = { self.nodes[0].getnewaddress() : Decimal("0.99990000") } + outputs = {self.nodes[0].getnewaddress(): Decimal("0.99990000")} rawTx = self.nodes[2].createrawtransaction(inputs, outputs) rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) assert_equal(rawTxSigned['complete'], True) @@ -407,7 +334,7 @@ def run_test(self): assert_equal(testres['allowed'], False) assert_equal(testres['reject-reason'], 'max-fee-exceeded') # and sendrawtransaction should throw - assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000) + assert_raises_rpc_error(-25, fee_exceeds_max, self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000) # and the following calls should both succeed testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0] assert_equal(testres['allowed'], True) @@ -419,9 +346,9 @@ def run_test(self): vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) self.sync_all() - inputs = [{ "txid" : txId, "vout" : vout['n'] }] + inputs = [{"txid": txId, "vout": vout['n']}] # Fee 2,000,000 satoshis, (1 - (2000000 sat * 0.00000001 BTC/sat)) = 0.98 - outputs = { self.nodes[0].getnewaddress() : Decimal("0.98000000") } + outputs = {self.nodes[0].getnewaddress() : Decimal("0.98000000")} rawTx = self.nodes[2].createrawtransaction(inputs, outputs) rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) assert_equal(rawTxSigned['complete'], True) @@ -431,13 +358,13 @@ def run_test(self): assert_equal(testres['allowed'], False) assert_equal(testres['reject-reason'], 'max-fee-exceeded') # and sendrawtransaction should throw - assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex']) + assert_raises_rpc_error(-25, fee_exceeds_max, self.nodes[2].sendrawtransaction, rawTxSigned['hex']) # and the following calls should both succeed testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.20000000')[0] assert_equal(testres['allowed'], True) self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.20000000') - self.log.info('sendrawtransaction/testmempoolaccept with tx that is already in the chain') + self.log.info("Test sendrawtransaction/testmempoolaccept with tx already in the chain") self.generate(self.nodes[2], 1) self.sync_blocks() for node in self.nodes: @@ -446,6 +373,144 @@ def run_test(self): assert_equal(testres['reject-reason'], 'txn-already-known') assert_raises_rpc_error(-27, 'Transaction already in block chain', node.sendrawtransaction, rawTxSigned['hex']) + def transaction_version_number_tests(self): + self.log.info("Test transaction version numbers") + # Test the minimum transaction version number that fits in a signed 16-bit integer. + # Note, this is different to bitcoin. Bitcoin has a 32 bit integer + # representing the version, we have 16 bits of version and 16 bits of + # type. + + # As transaction version is unsigned, this should convert to its unsigned equivalent. + tx = CTransaction() + tx.nVersion = -0x8000 + rawtx = tx.serialize().hex() + decrawtx = self.nodes[0].decoderawtransaction(rawtx) + assert_equal(decrawtx['version'], 0x8000) + + # Test the maximum transaction version number that fits in a signed 32-bit integer. + tx = CTransaction() + tx.nVersion = 0x7fff + rawtx = tx.serialize().hex() + decrawtx = self.nodes[0].decoderawtransaction(rawtx) + assert_equal(decrawtx['version'], 0x7fff) + + + def raw_multisig_transaction_legacy_tests(self): + self.log.info("Test raw multisig transactions (legacy)") + # The traditional multisig workflow does not work with descriptor wallets so these are legacy only. + # The multisig workflow with descriptor wallets uses PSBTs and is tested elsewhere, no need to do them here. + + # 2of2 test + addr1 = self.nodes[2].getnewaddress() + addr2 = self.nodes[2].getnewaddress() + + addr1Obj = self.nodes[2].getaddressinfo(addr1) + addr2Obj = self.nodes[2].getaddressinfo(addr2) + + # Tests for createmultisig and addmultisigaddress + assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"]) + # createmultisig can only take public keys + self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) + # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here + assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) + + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address'] + + # use balance deltas instead of absolute values + bal = self.nodes[2].getbalance() + + # send 1.2 BTC to msig adr + txId = self.nodes[0].sendtoaddress(mSigObj, 1.2) + self.sync_all() + self.generate(self.nodes[0], 1) + # node2 has both keys of the 2of2 ms addr, tx should affect the balance + assert_equal(self.nodes[2].getbalance(), bal + Decimal('1.20000000')) + + + # 2of3 test from different nodes + bal = self.nodes[2].getbalance() + addr1 = self.nodes[1].getnewaddress() + addr2 = self.nodes[2].getnewaddress() + addr3 = self.nodes[2].getnewaddress() + + addr1Obj = self.nodes[1].getaddressinfo(addr1) + addr2Obj = self.nodes[2].getaddressinfo(addr2) + addr3Obj = self.nodes[2].getaddressinfo(addr3) + + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']])['address'] + + txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) + decTx = self.nodes[0].gettransaction(txId) + rawTx = self.nodes[0].decoderawtransaction(decTx['hex']) + self.sync_all() + self.generate(self.nodes[0], 1) + + # THIS IS AN INCOMPLETE FEATURE + # NODE2 HAS TWO OF THREE KEYS AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION + assert_equal(self.nodes[2].getbalance(), bal) # for now, assume the funds of a 2of3 multisig tx are not marked as spendable + + txDetails = self.nodes[0].gettransaction(txId, True) + rawTx = self.nodes[0].decoderawtransaction(txDetails['hex']) + vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('2.20000000')) + + bal = self.nodes[0].getbalance() + inputs = [{"txid": txId, "vout": vout['n'], "scriptPubKey": vout['scriptPubKey']['hex']}] + outputs = {self.nodes[0].getnewaddress(): 2.19} + rawTx = self.nodes[2].createrawtransaction(inputs, outputs) + rawTxPartialSigned = self.nodes[1].signrawtransactionwithwallet(rawTx, inputs) + assert_equal(rawTxPartialSigned['complete'], False) # node1 only has one key, can't comp. sign the tx + + rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx, inputs) + assert_equal(rawTxSigned['complete'], True) # node2 can sign the tx compl., own two of three keys + self.nodes[2].sendrawtransaction(rawTxSigned['hex']) + rawTx = self.nodes[0].decoderawtransaction(rawTxSigned['hex']) + self.sync_all() + self.generate(self.nodes[0], 1) + assert_equal(self.nodes[0].getbalance(), bal + Decimal('500.00000000') + Decimal('2.19000000')) # block reward + tx + + # 2of2 test for combining transactions + bal = self.nodes[2].getbalance() + addr1 = self.nodes[1].getnewaddress() + addr2 = self.nodes[2].getnewaddress() + + addr1Obj = self.nodes[1].getaddressinfo(addr1) + addr2Obj = self.nodes[2].getaddressinfo(addr2) + + self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] + mSigObjValid = self.nodes[2].getaddressinfo(mSigObj) + + txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) + decTx = self.nodes[0].gettransaction(txId) + rawTx2 = self.nodes[0].decoderawtransaction(decTx['hex']) + self.sync_all() + self.generate(self.nodes[0], 1) + + assert_equal(self.nodes[2].getbalance(), bal) # the funds of a 2of2 multisig tx should not be marked as spendable + + txDetails = self.nodes[0].gettransaction(txId, True) + rawTx2 = self.nodes[0].decoderawtransaction(txDetails['hex']) + vout = next(o for o in rawTx2['vout'] if o['value'] == Decimal('2.20000000')) + + bal = self.nodes[0].getbalance() + inputs = [{"txid": txId, "vout": vout['n'], "scriptPubKey": vout['scriptPubKey']['hex'], "redeemScript": mSigObjValid['hex']}] + outputs = {self.nodes[0].getnewaddress(): 2.19} + rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs) + rawTxPartialSigned1 = self.nodes[1].signrawtransactionwithwallet(rawTx2, inputs) + self.log.debug(rawTxPartialSigned1) + assert_equal(rawTxPartialSigned1['complete'], False) # node1 only has one key, can't comp. sign the tx + + rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet(rawTx2, inputs) + self.log.debug(rawTxPartialSigned2) + assert_equal(rawTxPartialSigned2['complete'], False) # node2 only has one key, can't comp. sign the tx + rawTxComb = self.nodes[2].combinerawtransaction([rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']]) + self.log.debug(rawTxComb) + self.nodes[2].sendrawtransaction(rawTxComb) + rawTx2 = self.nodes[0].decoderawtransaction(rawTxComb) + self.sync_all() + self.generate(self.nodes[0], 1) + assert_equal(self.nodes[0].getbalance(), bal + Decimal('500.00000000') + Decimal('2.19000000')) # block reward + tx + if __name__ == '__main__': RawTransactionsTest().main() From 5d0ea79a3337e421228b0340f4e32916e967cd41 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 6 Sep 2021 09:59:47 +0200 Subject: [PATCH 07/10] Merge bitcoin/bitcoin#22841: ci: Fuzz with -ftrivial-auto-var-init=pattern fa0a5fa744108d81bee9600c80bfda6ca11e5255 ci: Fuzz with -ftrivial-auto-var-init=pattern (MarcoFalke) Pull request description: This makes memory bugs deterministic. `-ftrivial-auto-var-init=pattern` is incompatible with other memory sanitizers (like valgrind and msan), but that is irrelevant here, because the address sanitizer in this fuzz CI config is already incompatible with them. `-ftrivial-auto-var-init=pattern` goes well with `-fsanitize=bool` and `-fsanitize=enum`, but those are already enabled via `-fsanitize=undefined`. See https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks ACKs for top commit: practicalswift: cr ACK fa0a5fa744108d81bee9600c80bfda6ca11e5255 Tree-SHA512: ed6be953cd99eadb1ba245ba30170747eff66be54d2773c8d26a3a6aee0fdcd6967c596f4f4ab1d238de6a6526623dac5211f0ba77f1986639395d7921bdc19f --- ci/test/00_setup_env_native_fuzz.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh index 485ca639b2ee..9c6c7030bc13 100755 --- a/ci/test/00_setup_env_native_fuzz.sh +++ b/ci/test/00_setup_env_native_fuzz.sh @@ -16,4 +16,4 @@ export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false export RUN_FUZZ_TESTS=true export GOAL="install" -export BITCOIN_CONFIG="--enable-zmq --disable-ccache --enable-fuzz --with-sanitizers=fuzzer,address,undefined,integer --enable-suppress-external-warnings CC=clang-18 CXX=clang++-18 --with-boost-process" +export BITCOIN_CONFIG="--enable-zmq --disable-ccache --enable-fuzz --with-sanitizers=fuzzer,address,undefined,integer --enable-suppress-external-warnings CC='clang-18 -ftrivial-auto-var-init=pattern' CXX='clang++-18 -ftrivial-auto-var-init=pattern' --with-boost-process" From 8356044bf9a5d618aeaff228b47a7dc5719f622c Mon Sep 17 00:00:00 2001 From: merge-script Date: Fri, 10 Sep 2021 10:57:38 +0200 Subject: [PATCH 08/10] Merge bitcoin/bitcoin#22880: doc: Added hyperlink for doc/build 67c85bdb2f0d200b8e1669a6839ffd6325c663a6 doc: Added hyperlink for doc/build (pradumnasaraf) Pull request description: Added hyperlink for the doc/build directory (https://github.com/bitcoin/bitcoin/tree/master/doc). This will be convenient for visitors to redirect. ![Screenshot (84)](https://user-images.githubusercontent.com/51878265/132031506-9f5f3935-b1ef-4b70-a097-cd453540108d.png) Top commit has no ACKs. Tree-SHA512: e920487d5b5bc77ae4ad63f47c400834b8ca5b4b199392ff9b8f2166a620e12b842b7dcc4b04e9b2f65903dfbf421625aae41e7d59ae23306a94604106013eca --- INSTALL.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index 54762587fe71..013a099c95c2 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -1,5 +1 @@ -Building Dash -============= - -See doc/build-*.md for instructions on building the various -elements of the Dash Core reference implementation of Dash. +See [doc/build-\*.md](/doc) From 029572d4ab5761e537d63db949e75067a98731e0 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 16 Sep 2021 08:23:04 +0800 Subject: [PATCH 09/10] Merge bitcoin/bitcoin#22219: multiprocess: Start using init makeNode, makeChain, etc methods e4709c7b56612553fb7cbf16ef2d5099c5b732d0 Start using init makeNode, makeChain, etc methods (Russell Yanofsky) Pull request description: Use `interfaces::Init::make*` methods instead of `interfaces::Make*` functions, so interfaces can be constructed differently in different executable without having to change any code. (So for example `bitcoin-gui` can make an `interfaces::Node` pointer that communicates with a `bitcoin-node` subprocess, while `bitcoin-qt` can make an `interfaces::Node` pointer that controls node code in the same process.) --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102. ACKs for top commit: jamesob: reACK https://github.com/bitcoin/bitcoin/commit/e4709c7b56612553fb7cbf16ef2d5099c5b732d0 achow101: ACK e4709c7b56612553fb7cbf16ef2d5099c5b732d0 benthecarman: utACK e4709c7b56612553fb7cbf16ef2d5099c5b732d0 Tree-SHA512: 580c1979dbb2ef444157c8e53041e70d15ddeee77e5cbdb34f70b6d228cc2d2fe3843825f172da84e506200c58f7e0932f7cd4c006bb5058c1f4e43259394834 --- src/Makefile.qt.include | 4 ++-- src/Makefile.qttest.include | 1 + src/dummywallet.cpp | 10 ++++++++++ src/init.cpp | 3 ++- src/init/bitcoin-node.cpp | 9 +++++++++ src/init/bitcoind.cpp | 11 +++++++++++ src/interfaces/init.cpp | 2 +- src/interfaces/init.h | 7 ++++++- src/interfaces/node.h | 2 +- src/node/interfaces.cpp | 4 ++-- src/qt/bitcoin.cpp | 16 +++++++++------- src/qt/bitcoin.h | 8 ++++++-- src/qt/test/test_main.cpp | 8 ++++---- src/qt/test/wallettests.cpp | 2 ++ src/qt/transactionrecord.cpp | 5 ++--- src/qt/transactionrecord.h | 2 +- src/qt/transactiontablemodel.cpp | 4 ++-- src/rpc/misc.cpp | 5 +++-- src/test/util/setup_common.cpp | 4 ---- src/wallet/init.cpp | 3 ++- 20 files changed, 76 insertions(+), 34 deletions(-) diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index 2d3342b1d4de..6a6987fdc830 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -416,14 +416,14 @@ bitcoin_qt_libtoolflags = $(AM_LIBTOOLFLAGS) --tag CXX qt_dash_qt_CPPFLAGS = $(bitcoin_qt_cppflags) qt_dash_qt_CXXFLAGS = $(bitcoin_qt_cxxflags) -qt_dash_qt_SOURCES = $(bitcoin_qt_sources) +qt_dash_qt_SOURCES = $(bitcoin_qt_sources) init/bitcoind.cpp qt_dash_qt_LDADD = $(bitcoin_qt_ldadd) qt_dash_qt_LDFLAGS = $(bitcoin_qt_ldflags) qt_dash_qt_LIBTOOLFLAGS = $(bitcoin_qt_libtoolflags) dash_gui_CPPFLAGS = $(bitcoin_qt_cppflags) dash_gui_CXXFLAGS = $(bitcoin_qt_cxxflags) -dash_gui_SOURCES = $(bitcoin_qt_sources) +dash_gui_SOURCES = $(bitcoin_qt_sources) init/bitcoind.cpp dash_gui_LDADD = $(bitcoin_qt_ldadd) dash_gui_LDFLAGS = $(bitcoin_qt_ldflags) dash_gui_LIBTOOLFLAGS = $(bitcoin_qt_libtoolflags) diff --git a/src/Makefile.qttest.include b/src/Makefile.qttest.include index 8c4a950a5707..f8335c3121b4 100644 --- a/src/Makefile.qttest.include +++ b/src/Makefile.qttest.include @@ -33,6 +33,7 @@ qt_test_test_dash_qt_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BITCOIN_QT_ $(QT_INCLUDES) $(QT_TEST_INCLUDES) qt_test_test_dash_qt_SOURCES = \ + init/bitcoind.cpp \ qt/test/apptests.cpp \ qt/test/optiontests.cpp \ qt/test/rpcnestedtests.cpp \ diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index a9e5ae9486a8..10cd516a00f3 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -6,12 +6,17 @@ #include #include +class ArgsManager; class CWallet; namespace interfaces { class Chain; class Handler; class Wallet; +class WalletClient; +namespace CoinJoin { +class Loader; +} // namespcae CoinJoin } class DummyWalletInit : public WalletInitInterface { @@ -80,4 +85,9 @@ std::unique_ptr MakeWallet(const std::shared_ptr& wallet, const throw std::logic_error("Wallet function called in non-wallet build."); } +std::unique_ptr MakeWalletLoader(Chain& chain, const std::unique_ptr& coinjoin_loader, ArgsManager& args) +{ + throw std::logic_error("Wallet function called in non-wallet build."); +} + } // namespace interfaces diff --git a/src/init.cpp b/src/init.cpp index c88569d73220..7be800f83fb2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -1431,7 +1432,7 @@ bool AppInitLockDataDirectory() bool AppInitInterfaces(NodeContext& node) { - node.chain = interfaces::MakeChain(node); + node.chain = node.init->makeChain(); return true; } diff --git a/src/init/bitcoin-node.cpp b/src/init/bitcoin-node.cpp index 1468d509ba03..7300b36731fc 100644 --- a/src/init/bitcoin-node.cpp +++ b/src/init/bitcoin-node.cpp @@ -2,9 +2,12 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include +#include +#include #include #include @@ -24,6 +27,12 @@ class BitcoinNodeInit : public interfaces::Init m_node.args = &gArgs; m_node.init = this; } + std::unique_ptr makeNode() override { return interfaces::MakeNode(m_node); } + std::unique_ptr makeChain() override { return interfaces::MakeChain(m_node); } + std::unique_ptr makeWalletLoader(interfaces::Chain& chain, const std::unique_ptr& loader) override + { + return MakeWalletLoader(chain, loader, *Assert(m_node.args)); + } std::unique_ptr makeEcho() override { return interfaces::MakeEcho(); } interfaces::Ipc* ipc() override { return m_ipc.get(); } NodeContext& m_node; diff --git a/src/init/bitcoind.cpp b/src/init/bitcoind.cpp index 1d4504c24f6e..1a084227ec1f 100644 --- a/src/init/bitcoind.cpp +++ b/src/init/bitcoind.cpp @@ -2,7 +2,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include +#include #include +#include +#include #include #include @@ -18,6 +22,13 @@ class BitcoindInit : public interfaces::Init m_node.args = &gArgs; m_node.init = this; } + std::unique_ptr makeNode() override { return interfaces::MakeNode(m_node); } + std::unique_ptr makeChain() override { return interfaces::MakeChain(m_node); } + std::unique_ptr makeWalletLoader(interfaces::Chain& chain, const std::unique_ptr& loader) override + { + return MakeWalletLoader(chain, loader, *Assert(m_node.args)); + } + std::unique_ptr makeEcho() override { return interfaces::MakeEcho(); } NodeContext& m_node; }; } // namespace diff --git a/src/interfaces/init.cpp b/src/interfaces/init.cpp index f0f8aa5fedb6..8c51948fe8b2 100644 --- a/src/interfaces/init.cpp +++ b/src/interfaces/init.cpp @@ -11,7 +11,7 @@ namespace interfaces { std::unique_ptr Init::makeNode() { return {}; } std::unique_ptr Init::makeChain() { return {}; } -std::unique_ptr Init::makeWalletLoader(Chain& chain) { return {}; } +std::unique_ptr Init::makeWalletLoader(Chain& chain, const std::unique_ptr&) { return {}; } std::unique_ptr Init::makeEcho() { return {}; } Ipc* Init::ipc() { return nullptr; } } // namespace interfaces diff --git a/src/interfaces/init.h b/src/interfaces/init.h index a4ecf4b5d14a..75779e6e1b1d 100644 --- a/src/interfaces/init.h +++ b/src/interfaces/init.h @@ -16,6 +16,11 @@ class Ipc; class Node; class WalletLoader; +namespace CoinJoin +{ + class Loader; +} + //! Initial interface created when a process is first started, and used to give //! and get access to other interfaces (Node, Chain, Wallet, etc). //! @@ -29,7 +34,7 @@ class Init virtual ~Init() = default; virtual std::unique_ptr makeNode(); virtual std::unique_ptr makeChain(); - virtual std::unique_ptr makeWalletLoader(Chain& chain); + virtual std::unique_ptr makeWalletLoader(interfaces::Chain&, const std::unique_ptr&); virtual std::unique_ptr makeEcho(); virtual Ipc* ipc(); }; diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 9950762afd71..3d147686ca18 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -362,7 +362,7 @@ class Node }; //! Return implementation of Node interface. -std::unique_ptr MakeNode(NodeContext* context = nullptr); +std::unique_ptr MakeNode(NodeContext& context); //! Block tip (could be a header or not, depends on the subscribed signal). struct BlockTip { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 1a25271431f0..08a430d47d9b 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -305,7 +305,7 @@ class NodeImpl : public Node MasternodeSyncImpl m_masternodeSync; CoinJoinOptionsImpl m_coinjoin; - explicit NodeImpl(NodeContext* context) { setContext(context); } + explicit NodeImpl(NodeContext& context) { setContext(&context); } void initLogging() override { InitLogging(*Assert(m_context->args)); } void initParameterInteraction() override { InitParameterInteraction(*Assert(m_context->args)); } bilingual_str getWarnings() override { return GetWarnings(true); } @@ -1026,6 +1026,6 @@ class ChainImpl : public Chain } // namespace node namespace interfaces { -std::unique_ptr MakeNode(NodeContext* context) { return std::make_unique(context); } +std::unique_ptr MakeNode(NodeContext& context) { return std::make_unique(context); } std::unique_ptr MakeChain(NodeContext& node) { return std::make_unique(node); } } // namespace interfaces diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 36674d8e275b..bf1a76b3d9ed 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -269,10 +270,10 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle) connect(this, &BitcoinApplication::requestedShutdown, m_splash, &QWidget::close); } -void BitcoinApplication::setNode(interfaces::Node& node) +void BitcoinApplication::createNode(interfaces::Init& init) { assert(!m_node); - m_node = &node; + m_node = init.makeNode(); if (optionsModel) optionsModel->setNode(*m_node); if (m_splash) m_splash->setNode(*m_node); } @@ -478,11 +479,13 @@ int GuiMain(int argc, char* argv[]) util::WinCmdLineArgs winArgs; std::tie(argc, argv) = winArgs.get(); #endif - SetupEnvironment(); - util::ThreadSetInternalName("main"); NodeContext node_context; - std::unique_ptr node = interfaces::MakeNode(&node_context); + int unused_exit_status; + std::unique_ptr init = interfaces::MakeNodeInit(node_context, argc, argv, unused_exit_status); + + SetupEnvironment(); + util::ThreadSetInternalName("main"); // Subscribe to global signals from core boost::signals2::scoped_connection handler_message_box = ::uiInterface.ThreadSafeMessageBox_connect(noui_ThreadSafeMessageBox); @@ -504,7 +507,6 @@ int GuiMain(int argc, char* argv[]) /// 2. Parse command-line options. We do this after qt in order to show an error if there are problems parsing these // Command-line options take precedence: - node_context.args = &gArgs; SetupServerArgs(gArgs); SetupUIArgs(gArgs); std::string error; @@ -729,7 +731,7 @@ int GuiMain(int argc, char* argv[]) if (gArgs.GetBoolArg("-splash", DEFAULT_SPLASHSCREEN) && !gArgs.GetBoolArg("-min", false)) app.createSplashScreen(networkStyle.data()); - app.setNode(*node); + app.createNode(*init); int rv = EXIT_SUCCESS; try diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index 934b84179525..df6e4f0687e0 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -26,6 +26,9 @@ class PaymentServer; class SplashScreen; class WalletController; class WalletModel; +namespace interfaces { +class Init; +} // namespace interfaces /** Main Bitcoin application object */ @@ -50,6 +53,8 @@ class BitcoinApplication: public QApplication void createWindow(const NetworkStyle *networkStyle); /// Create splash screen void createSplashScreen(const NetworkStyle *networkStyle); + /// Create or spawn node + void createNode(interfaces::Init& init); /// Basic initialization, before starting initialization/shutdown thread. Return true on success. bool baseInitialize(); @@ -65,7 +70,6 @@ class BitcoinApplication: public QApplication WId getMainWinId() const; interfaces::Node& node() const { assert(m_node); return *m_node; } - void setNode(interfaces::Node& node); public Q_SLOTS: void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info); @@ -102,7 +106,7 @@ public Q_SLOTS: int returnValue; std::unique_ptr shutdownWindow; SplashScreen* m_splash = nullptr; - interfaces::Node* m_node = nullptr; + std::unique_ptr m_node; void startThread(); }; diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index af7389079942..4d36ed32c9f7 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -7,6 +7,7 @@ #include #endif +#include #include #include #include @@ -62,7 +63,8 @@ int main(int argc, char* argv[]) } NodeContext node_context; - std::unique_ptr node = interfaces::MakeNode(&node_context); + int unused_exit_status; + std::unique_ptr init = interfaces::MakeNodeInit(node_context, argc, argv, unused_exit_status); gArgs.ForceSetArg("-listen", "0"); gArgs.ForceSetArg("-listenonion", "0"); gArgs.ForceSetArg("-discover", "0"); @@ -81,10 +83,8 @@ int main(int argc, char* argv[]) #endif BitcoinApplication app; - app.setNode(*node); app.setApplicationName("Dash-Qt-test"); - - app.node().context()->args = &gArgs; // Make gArgs available in the NodeContext + app.createNode(*init); int num_test_failures{0}; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index effb354fd18e..4425b91b1111 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -108,6 +108,8 @@ void TestGUI(interfaces::Node& node) for (int i = 0; i < 5; ++i) { test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } + auto wallet_loader = interfaces::MakeWalletLoader(*test.m_node.chain, test.m_node.coinjoin_loader, *Assert(test.m_node.args)); + test.m_node.wallet_loader = wallet_loader.get(); node.setContext(&test.m_node); const std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase()); AddWallet(wallet); diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 46a5ffb6e815..688b34bcd395 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -27,7 +27,7 @@ bool TransactionRecord::showTransaction() /* * Decompose CWallet transaction to model transaction records. */ -QList TransactionRecord::decomposeTransaction(interfaces::Wallet& wallet, const interfaces::WalletTx& wtx) +QList TransactionRecord::decomposeTransaction(interfaces::Node& node, interfaces::Wallet& wallet, const interfaces::WalletTx& wtx) { QList parts; int64_t nTime = wtx.time; @@ -36,8 +36,7 @@ QList TransactionRecord::decomposeTransaction(interfaces::Wal CAmount nNet = nCredit - nDebit; uint256 hash = wtx.tx->GetHash(); std::map mapValue = wtx.value_map; - auto node = interfaces::MakeNode(); - auto& coinJoinOptions = node->coinJoinOptions(); + auto& coinJoinOptions = node.coinJoinOptions(); if (nNet > 0 || wtx.is_coinbase || wtx.is_platform_transfer) { diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index fe2064907ea1..1c16ac6c7304 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -129,7 +129,7 @@ class TransactionRecord /** Decompose CWallet transaction to model transaction records. */ static bool showTransaction(); - static QList decomposeTransaction(interfaces::Wallet& wallet, const interfaces::WalletTx& wtx); + static QList decomposeTransaction(interfaces::Node& node, interfaces::Wallet& wallet, const interfaces::WalletTx& wtx); /** @name Immutable transaction attributes @{*/ diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index ecb40062f9e3..d07b144fd928 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -114,7 +114,7 @@ class TransactionTablePriv try { for (const auto& wtx : wallet.getWalletTxs()) { if (TransactionRecord::showTransaction()) { - cachedWallet.append(TransactionRecord::decomposeTransaction(wallet, wtx)); + cachedWallet.append(TransactionRecord::decomposeTransaction(parent->walletModel->node(), wallet, wtx)); } } } catch(const std::exception& e) { @@ -174,7 +174,7 @@ class TransactionTablePriv } // Added -- insert at the right position QList toInsert = - TransactionRecord::decomposeTransaction(wallet, wtx); + TransactionRecord::decomposeTransaction(parent->walletModel->node(), wallet, wtx); if(!toInsert.isEmpty()) /* only if something to insert */ { parent->beginInsertRows(QModelIndex(), lowerIndex, lowerIndex+toInsert.size()-1); diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 64775878d7c6..7900e99cd1f9 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -1380,8 +1380,9 @@ static RPCHelpMan echoipc() RPCExamples{HelpExampleCli("echo", "\"Hello world\"") + HelpExampleRpc("echo", "\"Hello world\"")}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + interfaces::Init& local_init = *EnsureAnyNodeContext(request.context).init; std::unique_ptr echo; - if (interfaces::Ipc* ipc = Assert(EnsureAnyNodeContext(request.context).init)->ipc()) { + if (interfaces::Ipc* ipc = local_init.ipc()) { // Spawn a new bitcoin-node process and call makeEcho to get a // client pointer to a interfaces::Echo instance running in // that process. This is just for testing. A slightly more @@ -1399,7 +1400,7 @@ static RPCHelpMan echoipc() // interfaces::Echo object and return it so the `echoipc` RPC // method will work, and the python test calling `echoipc` // can expect the same result. - echo = interfaces::MakeEcho(); + echo = local_init.makeEcho(); } return echo->echo(request.params[0].get_str()); }, diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 99cfc3796c7f..4014ad6847fe 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -192,10 +192,6 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve m_node.args->GetIntArg("-checkaddrman", 0)); m_node.connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); // Deterministic randomness for tests. - // while g_wallet_init_interface is init here at very early stage - // we can't get rid of unique_ptr from wallet/context.h - // TODO: remove unique_ptr from wallet/context.h after bitcoin/bitcoin#22219 - g_wallet_init_interface.Construct(m_node); fCheckBlockIndex = true; m_node.evodb = std::make_unique(1 << 20, true, true); m_node.mnhf_manager = std::make_unique(*m_node.evodb); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index ee8430395b2a..a141317d9f2f 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -187,7 +188,7 @@ void WalletInit::Construct(NodeContext& node) const LogPrintf("Wallet disabled!\n"); return; } - auto wallet_loader = interfaces::MakeWalletLoader(*node.chain, node.coinjoin_loader, args); + auto wallet_loader = node.init->makeWalletLoader(*node.chain, node.coinjoin_loader); node.wallet_loader = wallet_loader.get(); node.chain_clients.emplace_back(std::move(wallet_loader)); } From bc51716351c6e7543cf1c10b92a70ddcc53c1cc5 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 16 Sep 2021 12:09:33 +0800 Subject: [PATCH 10/10] Merge bitcoin/bitcoin#22992: Fix Qt test broken by #22219 865ee1af201238f48671e3b79b8215f896db7a05 Fix Qt test broken by #22219 (Russell Yanofsky) Pull request description: It looks like this should have been caught by CI but there might have been a conflict with a recently merged PR like #19101. Failure was reported by fanquake https://github.com/bitcoin/bitcoin/pull/22219#issuecomment-920496509 Fix avoids null WalletClient pointer dereference in address book test by adding MakeWalletClient call and making address book test initialization more consistent with wallet test initialization: https://github.com/bitcoin/bitcoin/blob/865ee1af201238f48671e3b79b8215f896db7a05/src/qt/test/addressbooktests.cpp#L63-L66 https://github.com/bitcoin/bitcoin/blob/865ee1af201238f48671e3b79b8215f896db7a05/src/qt/test/wallettests.cpp#L141-L144 ACKs for top commit: fanquake: ACK 865ee1af201238f48671e3b79b8215f896db7a05 - I'm merging this now to unbreak the build. Tree-SHA512: 1f32b7fc79fa79fcf8600d23063896cbc7f8bbcff39d95747ecd546e754581f0f36ece3098ddecded175afccbb3709b4232da39a400dda23b7e550f361b515fb --- src/qt/test/addressbooktests.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index c4a47847aa1c..ede8755afffc 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -63,6 +63,8 @@ void EditAddressAndSubmit( void TestAddAddressesToSendBook(interfaces::Node& node) { TestChain100Setup test; + auto wallet_loader = interfaces::MakeWalletLoader(*test.m_node.chain, test.m_node.coinjoin_loader, *Assert(test.m_node.args)); + test.m_node.wallet_loader = wallet_loader.get(); node.setContext(&test.m_node); const std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan();