From d9c31d6817f148a487ee83a82d3a7c37fa88c2d5 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 11 Mar 2021 19:08:26 +0100 Subject: [PATCH 01/11] Merge #21411: test: add logging, reduce blocks, move sync_all in wallet_ groups c62f9bc0e931f65eef63041d2c53f9a294c0e8d6 test: use fewer blocks in wallet_groups and move sync call (Jon Atack) 3a16b5ef95c1c25f8b78e591f985e80b41a6dbdd test: add missing logging to wallet_groups.py (Jon Atack) Pull request description: - add logging (particularly useful as the tests are somewhat slow) - generate 101 blocks instead of 110 - move `sync_all` call into the loop, so fewer blocks are synced on each call, to hopefully see fewer CI timeouts as in https://bitcoinbuilds.org/index.php?ansilog=88eee99e-1727-44ed-b778-3b9c75c33928.log ``` L2742 File "/home/ubuntu/src/test/functional/wallet_groups.py", line 162, in run_test L2743 self.sync_all() test_framework.authproxy.JSONRPCException: 'syncwithvalidationinterfacequeue' RPC took longer than 960.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344) ``` ACKs for top commit: MarcoFalke: cr ACK c62f9bc0e931f65eef63041d2c53f9a294c0e8d6 Tree-SHA512: 711deafcd589cb8196cb207ff882e0f2ab6b70828a6abad91f83f535974cc430a56b9e8a960fd233d31d610932a0d48b49ee681aae564d145a3040288ecda8f8 --- test/functional/wallet_groups.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index 9568326196..d30a7c0a58 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -32,8 +32,9 @@ class WalletGroupTest(BitcoinTestFramework): self.skip_if_no_wallet() def run_test(self): + self.log.info("Setting up") # Mine some coins - self.nodes[0].generate(COINBASE_MATURITY + 10) + self.nodes[0].generate(COINBASE_MATURITY + 1) # Get some addresses from the two nodes addr1 = [self.nodes[1].getnewaddress() for _ in range(3)] @@ -51,6 +52,7 @@ class WalletGroupTest(BitcoinTestFramework): # - node[1] should pick one 0.5 UTXO and leave the rest # - node[2] should pick one (1.0 + 0.5) UTXO group corresponding to a # given address, and leave the rest + self.log.info("Test sending transactions picks one UTXO group and leaves the rest") txid1 = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 0.2) tx1 = self.nodes[1].getrawtransaction(txid1, True) # txid1 should have 1 input and 2 outputs @@ -73,7 +75,7 @@ class WalletGroupTest(BitcoinTestFramework): assert_approx(v[0], vexp=0.2, vspan=0.0001) assert_approx(v[1], vexp=1.3, vspan=0.0001) - # Test 'avoid partial if warranted, even if disabled' + self.log.info("Test avoiding partial spends if warranted, even if avoidpartialspends is disabled") self.sync_all() self.nodes[0].generate(1) # Nodes 1-2 now have confirmed UTXOs (letters denote destinations): @@ -107,7 +109,7 @@ class WalletGroupTest(BitcoinTestFramework): assert_equal(input_addrs[0], input_addrs[1]) # Node 2 enforces avoidpartialspends so needs no checking here - # Test wallet option maxapsfee with Node 3 + self.log.info("Test wallet option maxapsfee") addr_aps = self.nodes[3].getnewaddress() self.nodes[0].sendtoaddress(addr_aps, 1.0) self.nodes[0].sendtoaddress(addr_aps, 1.0) @@ -134,6 +136,7 @@ class WalletGroupTest(BitcoinTestFramework): # Test wallet option maxapsfee with node 4, which sets maxapsfee # 1 sat higher, crossing the threshold from non-grouped to grouped. + self.log.info("Test wallet option maxapsfee threshold from non-grouped to grouped") addr_aps3 = self.nodes[4].getnewaddress() [self.nodes[0].sendtoaddress(addr_aps3, 1.0) for _ in range(5)] self.nodes[0].generate(1) @@ -150,8 +153,7 @@ class WalletGroupTest(BitcoinTestFramework): self.sync_all() self.nodes[0].generate(1) - # Fill node2's wallet with 10000 outputs corresponding to the same - # scriptPubKey + self.log.info("Fill a wallet with 10,000 outputs corresponding to the same scriptPubKey") for _ in range(5): raw_tx = self.nodes[0].createrawtransaction([{"txid":"0"*64, "vout":0}], [{addr2[0]: 0.05}]) tx = tx_from_hex(raw_tx) @@ -161,12 +163,12 @@ class WalletGroupTest(BitcoinTestFramework): signed_tx = self.nodes[0].signrawtransactionwithwallet(funded_tx['hex']) self.nodes[0].sendrawtransaction(signed_tx['hex']) self.nodes[0].generate(1) - - self.sync_all() + self.sync_all() # Check that we can create a transaction that only requires ~100 of our # utxos, without pulling in all outputs and creating a transaction that # is way too big. + self.log.info("Test creating txn that only requires ~100 of our UTXOs without pulling in all outputs") assert self.nodes[2].sendtoaddress(address=addr2[0], amount=5) From e10eec249b9516655566dacdb5a3df36b7e56871 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 24 Mar 2021 08:26:57 +0100 Subject: [PATCH 02/11] Merge #21338: test: add functional test for anchors.dat 581791c62067403fbeb4e1fd88c1d80549627c94 test: add functional test for anchors.dat (bruno) Pull request description: This PR adds a functional test for anchors.dat. It creates a node and adds 2 outbound block-relay-only connections and 5 inbound connections. When the node is down, anchors.dat should contain the 2 addresses from the outbound block-relay-only connections. ACKs for top commit: MarcoFalke: Concept ACK 581791c62067403fbeb4e1fd88c1d80549627c94 hebasto: ACK 581791c62067403fbeb4e1fd88c1d80549627c94 Tree-SHA512: 77038b09e36ee5ae473a26d6f566c0ed283af258c34df8486706a24f72b05abab621a293ac886d03849bc45bc28be7336137252225b25aff393baa6b5238688c --- test/functional/feature_anchors.py | 85 ++++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 86 insertions(+) create mode 100755 test/functional/feature_anchors.py diff --git a/test/functional/feature_anchors.py b/test/functional/feature_anchors.py new file mode 100755 index 0000000000..a60a723b3e --- /dev/null +++ b/test/functional/feature_anchors.py @@ -0,0 +1,85 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 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 Anchors functionality""" + +import os + +from test_framework.p2p import P2PInterface +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +def check_node_connections(*, node, num_in, num_out): + info = node.getnetworkinfo() + assert_equal(info["connections_in"], num_in) + assert_equal(info["connections_out"], num_out) + + +class AnchorsTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def setup_network(self): + self.setup_nodes() + + def run_test(self): + self.log.info("Add 2 block-relay-only connections to node 0") + for i in range(2): + self.log.debug(f"block-relay-only: {i}") + self.nodes[0].add_outbound_p2p_connection( + P2PInterface(), p2p_idx=i, connection_type="block-relay-only" + ) + + self.log.info("Add 5 inbound connections to node 0") + for i in range(5): + self.log.debug(f"inbound: {i}") + self.nodes[0].add_p2p_connection(P2PInterface()) + + self.log.info("Check node 0 connections") + check_node_connections(node=self.nodes[0], num_in=5, num_out=2) + + # 127.0.0.1 + ip = "7f000001" + + # Since the ip is always 127.0.0.1 for this case, + # we store only the port to identify the peers + block_relay_nodes_port = [] + inbound_nodes_port = [] + for p in self.nodes[0].getpeerinfo(): + addr_split = p["addr"].split(":") + if p["connection_type"] == "block-relay-only": + block_relay_nodes_port.append(hex(int(addr_split[1]))[2:]) + else: + inbound_nodes_port.append(hex(int(addr_split[1]))[2:]) + + self.log.info("Stop node 0") + self.stop_node(0) + + node0_anchors_path = os.path.join( + self.nodes[0].datadir, "regtest", "anchors.dat" + ) + + # It should contain only the block-relay-only addresses + self.log.info("Check the addresses in anchors.dat") + + with open(node0_anchors_path, "rb") as file_handler: + anchors = file_handler.read().hex() + + for port in block_relay_nodes_port: + ip_port = ip + port + assert ip_port in anchors + for port in inbound_nodes_port: + ip_port = ip + port + assert ip_port not in anchors + + self.log.info("Start node 0") + self.start_node(0) + + self.log.info("When node starts, check if anchors.dat doesn't exist anymore") + assert not os.path.exists(node0_anchors_path) + + +if __name__ == "__main__": + AnchorsTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2d3818609b..60c0a62209 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -318,6 +318,7 @@ BASE_SCRIPTS = [ 'p2p_ping.py', 'rpc_scantxoutset.py', 'feature_logging.py', + 'feature_anchors.py', 'feature_coinstatsindex.py', 'wallet_orphanedreward.py', 'p2p_node_network_limited.py', From 6674ee85ab2ec30c5b4f866bb9fd7a0dc2bc6d9c Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 26 Mar 2021 08:52:51 +0100 Subject: [PATCH 03/11] Merge #21390: test: Test improvements for UTXO set hash tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 4f2653a89018fa4d24bd2a551832a7410b682600 test: Use deterministic chain in utxo set hash test (Fabian Jahr) 4973c5175c5fd1f4791ea26e8ddefd6fb11ac1c3 test: Remove wallet dependency of utxo set hash test (Fabian Jahr) 1a27af1d7b5ec18b4248ead1eaf0f381047b4b24 rpc: Improve gettxoutsetinfo help (Fabian Jahr) Pull request description: Follow-ups to #19145: - Small improvement on the help text of RPC gettxoutsetinfo - Using deterministic blockchain in the test `functional/feature_utxo_set_hash.py` - Removing wallet dependency in the test `functional/feature_utxo_set_hash.py` Split out of #19521. ACKs for top commit: MarcoFalke: review ACK 4f2653a89018fa4d24bd2a551832a7410b682600 👲 Tree-SHA512: 92927b3aa22b6324eb4fc9d346755313dec44d973aa69a0ebf80a8569b5f3a7cf3539721ebdba183737534b9e29b3e33f412515890f0d0b819878032a3bba8f9 --- src/node/coinstats.cpp | 1 + src/rpc/blockchain.cpp | 6 ++--- test/functional/feature_utxo_set_hash.py | 30 ++++++++++-------------- test/functional/test_framework/wallet.py | 3 +++ 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/node/coinstats.cpp b/src/node/coinstats.cpp index f41adb160f..d86e6ae316 100644 --- a/src/node/coinstats.cpp +++ b/src/node/coinstats.cpp @@ -18,6 +18,7 @@ #include +// Database-independent metric indicating the UTXO set size uint64_t GetBogoSize(const CScript& script_pub_key) { return 32 /* txid */ + diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 26b6b3746b..c13deb58a0 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1360,15 +1360,15 @@ static RPCHelpMan gettxoutsetinfo() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::NUM, "height", "The current block height (index)"}, - {RPCResult::Type::STR_HEX, "bestblock", "The hash of the block at the tip of the chain"}, + {RPCResult::Type::NUM, "height", "The block height (index) of the returned statistics"}, + {RPCResult::Type::STR_HEX, "bestblock", "The hash of the block at which these statistics are calculated"}, {RPCResult::Type::NUM, "txouts", "The number of unspent transaction outputs"}, {RPCResult::Type::NUM, "bogosize", "Database-independent, meaningless metric indicating the UTXO set size"}, {RPCResult::Type::STR_HEX, "hash_serialized_2", /* optional */ true, "The serialized hash (only present if 'hash_serialized_2' hash_type is chosen)"}, {RPCResult::Type::STR_HEX, "muhash", /* optional */ true, "The serialized hash (only present if 'muhash' hash_type is chosen)"}, {RPCResult::Type::NUM, "transactions", "The number of transactions with unspent outputs (not available when coinstatsindex is used)"}, {RPCResult::Type::NUM, "disk_size", "The estimated size of the chainstate on disk (not available when coinstatsindex is used)"}, - {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount"}, + {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount of coins in the UTXO set"}, {RPCResult::Type::STR_AMOUNT, "total_unspendable_amount", "The total amount of coins permanently excluded from the UTXO set (only available if coinstatsindex is used)"}, {RPCResult::Type::OBJ, "block_info", "Info on amounts in the block at this block height (only available if coinstatsindex is used)", { diff --git a/test/functional/feature_utxo_set_hash.py b/test/functional/feature_utxo_set_hash.py index b21b34b906..8b0e39fc5c 100755 --- a/test/functional/feature_utxo_set_hash.py +++ b/test/functional/feature_utxo_set_hash.py @@ -6,7 +6,6 @@ import struct -from test_framework.blocktools import create_transaction from test_framework.messages import ( CBlock, COutPoint, @@ -15,38 +14,30 @@ from test_framework.messages import ( from test_framework.crypto.muhash import MuHash3072 from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal +from test_framework.wallet import MiniWallet class UTXOSetHashTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - - def test_deterministic_hash_results(self): - self.log.info("Test deterministic UTXO set hash results") - - # These depend on the setup_clean_chain option, the chain loaded from the cache - assert_equal(self.nodes[0].gettxoutsetinfo()['hash_serialized_2'], "b61ee2cb582d2f4f94493f3d480e9a59d064706e98a12be0f335a3eeadd5678a") - assert_equal(self.nodes[0].gettxoutsetinfo("muhash")['muhash'], "dd5ad2a105c2d29495f577245c357409002329b9f4d6182c0af3dc2f462555c8") - def test_muhash_implementation(self): self.log.info("Test MuHash implementation consistency") node = self.nodes[0] + wallet = MiniWallet(node) + mocktime = node.getblockheader(node.getblockhash(0))['time'] + 1 + node.setmocktime(mocktime) # Generate 100 blocks and remove the first since we plan to spend its # coinbase - block_hashes = node.generate(100) + block_hashes = wallet.generate(1) + node.generate(99) blocks = list(map(lambda block: from_hex(CBlock(), node.getblock(block, False)), block_hashes)) - spending = blocks.pop(0) + blocks.pop(0) # Create a spending transaction and mine a block which includes it - tx = create_transaction(node, spending.vtx[0].rehash(), node.getnewaddress(), amount=49) - txid = node.sendrawtransaction(hexstring=tx.serialize().hex(), maxfeerate=0) - - tx_block = node.generateblock(node.getnewaddress(), [txid])['hash'] + txid = wallet.send_self_transfer(from_node=node)['txid'] + tx_block = node.generateblock(output=wallet.get_address(), transactions=[txid])['hash'] blocks.append(from_hex(CBlock(), node.getblock(tx_block, False))) # Serialize the outputs that should be in the UTXO set and add them to @@ -77,8 +68,11 @@ class UTXOSetHashTest(BitcoinTestFramework): assert_equal(finalized[::-1].hex(), node_muhash) + self.log.info("Test deterministic UTXO set hash results") + assert_equal(node.gettxoutsetinfo()['hash_serialized_2'], "4eb23b9673b7472e33ebf6e6aefee849fe5bc5e598fd387c2f9222070cc2f711") + assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "dd74d7f1fb047577915d490e82d063e843f7853ae7543c9980a28c67326e1a2c") + def run_test(self): - self.test_deterministic_hash_results() self.test_muhash_implementation() diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index e0dd06027e..98d0dcdcc1 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -39,6 +39,9 @@ class MiniWallet: self._utxos.append({'txid': cb_tx['txid'], 'vout': 0, 'value': cb_tx['vout'][0]['value']}) return blocks + def get_address(self): + return self._address + def get_utxo(self, *, txid=''): """ Returns a utxo and marks it as spent (pops it from the internal list) From ad947099a08736a49b73d4cbba6d35f065194e13 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 5 Apr 2024 01:35:55 +0700 Subject: [PATCH 04/11] test: remove exception for util::Ref which doesn't exist more This PR is follow-up for dash#5055 and based on bitcoin#21366 which is DNM --- test/lint/extended-lint-cppcheck.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lint/extended-lint-cppcheck.sh b/test/lint/extended-lint-cppcheck.sh index e2391b11fe..3b66d3d433 100755 --- a/test/lint/extended-lint-cppcheck.sh +++ b/test/lint/extended-lint-cppcheck.sh @@ -55,7 +55,6 @@ IGNORED_WARNINGS=( "src/test/checkqueue_tests.cpp:.* Struct 'UniqueCheck' has a constructor with 1 argument that is not explicit." "src/test/fuzz/util.h:.* Class 'FuzzedFileProvider' has a constructor with 1 argument that is not explicit." "src/test/fuzz/util.h:.* Class 'FuzzedAutoFileProvider' has a constructor with 1 argument that is not explicit." - "src/util/ref.h:.* Class 'Ref' has a constructor with 1 argument that is not explicit." "src/wallet/db.h:.* Class 'BerkeleyEnvironment' has a constructor with 1 argument that is not explicit." ) From a224b800e45b82acd4b0421128e4a1accf583081 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 6 Apr 2021 07:57:26 +0200 Subject: [PATCH 05/11] Merge #21609: ci: increase CPU count of sanitizer job to increase memory limit de3ae78eff257302dd45e57e38067f72e970a123 ci: increase CPU count of sanitizer job to increase memory limit (fanquake) Pull request description: According to the [docs](https://cirrus-ci.org/guide/linux/#linux-containers): > For each CPU you can't get more than 4G of memory. thus if we want this job to have 24GB of memory, we need to increase the CPU count to 6. It's currently [failing with](https://github.com/bitcoin/bitcoin/runs/2273962280): > Requested memory is too high! You can request at most 4G per CPU Top commit has no ACKs. Tree-SHA512: 0a4da5649d061425190a373859274c78ca5587cd2d6e27905ec548f124ed114a0133215cb2eff22ffc182f50c3a53df58e7c9832b44db6e37d7ea59ec96a4775 --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 103fed7409..5470229002 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -102,7 +102,7 @@ task: << : *GLOBAL_TASK_TEMPLATE container: image: ubuntu:lunar - cpu: 4 # Double CPU and increase Memory to avoid timeout + cpu: 6 # Increase CPU and Memory to avoid timeout memory: 24G env: MAKEJOBS: "-j8" From 233fb245f7644e1b62566a5146d1dd84630b4efb Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 9 Apr 2021 09:52:07 +0800 Subject: [PATCH 06/11] Merge #21445: cirrus: Use SSD cluster for speedup fa212391ce0bb04669bc5205553a71e653f5ad48 cirrus: Use SSD cluster for speedup (MarcoFalke) Pull request description: Haven't tested, but this might be faster: https://twitter.com/fedor/status/1354505744293502980 ACKs for top commit: fanquake: ACK fa212391ce0bb04669bc5205553a71e653f5ad48 - going to merge this now given there is a speedup, and it's enough to fix the fuzz task the is continually timing out. Tree-SHA512: b24161ba2ed16fcf23ac6098100b04f7f6eb8936bb312a35fcd8e632568b877bfc09a1e522836fe298a2cd87a53a91e3b501a7f2e1c81cc0edb57c59aecffb0d --- .cirrus.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 5470229002..342be2f0c6 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -29,7 +29,6 @@ global_task_template: &GLOBAL_TASK_TEMPLATE # Each project has 16 CPU in total, assign 2 to each container, so that 8 tasks run in parallel cpu: 2 memory: 8G # Set to 8GB to avoid OOM. https://cirrus-ci.org/guide/linux/#linux-containers - kvm: true # Use kvm to avoid spurious CI failures in the default virtualization cluster, see https://github.com/bitcoin/bitcoin/issues/20093 ccache_cache: folder: "/tmp/ccache_dir" depends_built_cache: From bc6e3ed6e4493ac044c86b4505992ef2777455eb Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 9 Apr 2021 18:54:10 +0200 Subject: [PATCH 07/11] Merge #21606: fuzz: Extend psbt fuzz target a bit faaf3954e2f0089b6c6b9965f15e7f9af09c6fb0 fuzz: Extend psbt fuzz target a bit (MarcoFalke) Pull request description: Previously it only merged the psbt with itself, now it tries to merge another. ACKs for top commit: practicalswift: Tested ACK faaf3954e2f0089b6c6b9965f15e7f9af09c6fb0 Tree-SHA512: e1b1d31a47d35e1767285bc2fda176c79cb0550d6d383fe467104272e61e1c83f6cbc0c7d6bbc0c3027729eec13ae1f289f8950117ee91e0fb3703e66d5e6918 --- src/test/fuzz/psbt.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/fuzz/psbt.cpp b/src/test/fuzz/psbt.cpp index de0f706243..b932385e1c 100644 --- a/src/test/fuzz/psbt.cpp +++ b/src/test/fuzz/psbt.cpp @@ -46,6 +46,7 @@ FUZZ_TARGET(psbt) (void)PSBTInputSigned(input); (void)input.IsNull(); } + (void)CountPSBTUnsignedInputs(psbt); for (const PSBTOutput& output : psbt.outputs) { (void)output.IsNull(); From 7e023c394f7f4944bad3fcc82280652b834427ec Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 13 Apr 2021 21:11:35 +0800 Subject: [PATCH 08/11] Merge #17934: doc: Use CONFIG_SITE variable instead of --prefix option 223b1ba7d90509a47ea07af46f4b9c3b8efbc9f8 doc: Use CONFIG_SITE instead of --prefix (Hennadii Stepanov) Pull request description: The current examples of `--prefix=...` option usage to point `configure` script to appropriate `depends` directory is not [standard](https://www.gnu.org/prep/standards/html_node/Directory-Variables.html). This causes some [confusion](https://github.com/bitcoin/bitcoin/pull/16691) and a bit of inconvenience. Consider a CentOS 7 32 bit system. Packages `libdb4-devel`, `libdb4-cxx-devel`, `miniupnpc-devel` and `zeromq-devel` are unavailable from repos. After recommended build with depends: ``` cd depends make cd .. ./autogen.sh ./configure --prefix=$PWD/depends/i686-pc-linux-gnu make ``` a user is unable to `make install` compiled binaries neither locally (to `~/.local`) nor system-wide (to `/usr/local`) as `--prefix` is set already. Meanwhile, the standard approach with using [`config.site`](https://www.gnu.org/software/automake/manual/html_node/config_002esite.html) files allows both possibilities: ``` cd depends make cd .. ./autogen.sh CONFIG_SITE=$PWD/depends/i686-pc-linux-gnu/share/config.site ./configure --prefix ~/.local make make install ``` or ``` CONFIG_SITE=$PWD/depends/i686-pc-linux-gnu/share/config.site ./configure make sudo make install # install to /usr/local ``` Moreover, this approach is used in [Gitian descriptors](https://github.com/bitcoin/bitcoin/tree/master/contrib/gitian-descriptors) already. ACKs for top commit: practicalswift: ACK 223b1ba7d90509a47ea07af46f4b9c3b8efbc9f8: patch looks correct fanquake: ACK 223b1ba7d90509a47ea07af46f4b9c3b8efbc9f8 Tree-SHA512: 46d97924f0fc7e95ee4566737cf7c2ae805ca500e5c49af9aa99ecc3acede4b00329bc727a110aa1b62618dfbf5d1ca2234e736f16fbdf96d6ece5f821712f54 --- doc/multiprocess.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/multiprocess.md b/doc/multiprocess.md index 9c46e68d4f..e315343a4a 100644 --- a/doc/multiprocess.md +++ b/doc/multiprocess.md @@ -24,7 +24,7 @@ The multiprocess feature requires [Cap'n Proto](https://capnproto.org/) and [lib ``` cd make -C depends NO_QT=1 MULTIPROCESS=1 -./configure --prefix=$PWD/depends/x86_64-pc-linux-gnu +CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure make src/dash-node -regtest -printtoconsole -debug=ipc DASHD=dash-node test/functional/test_runner.py From adea52a5fe8530a6260d10246ab7443d143e9790 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 14 Apr 2021 10:22:57 +0200 Subject: [PATCH 09/11] Merge bitcoin-core/gui#260: Handle exceptions instead of crash b8e5d0d3fe3386807d47f50d13ac34fcd2a538fd qt: Handle exceptions in SendCoinsDialog::sendButtonClicked slot (Hennadii Stepanov) 1ac2bc7ac070dfd1df1872d759540b0c92495301 qt: Handle exceptions in TransactionView::bumpFee slot (Hennadii Stepanov) bc00e13bc800863641b3e1e64732a38418d3022f qt: Handle exceptions in WalletModel::pollBalanceChanged slot (Hennadii Stepanov) eb6156ba1b4c303eb597e3fc4a9e42ce45e6e78d qt: Handle exceptions in BitcoinGUI::addWallet slot (Hennadii Stepanov) f7e260a471010e2d656fbc5ea8c310f6d94c26b9 qt: Add GUIUtil::ExceptionSafeConnect function (Hennadii Stepanov) 64a8755af396f1c2791018510e22b58114e68594 qt: Add BitcoinApplication::handleNonFatalException function (Hennadii Stepanov) af7e365b1516d660d271475fdfe0c20ae09e66a8 qt: Make PACKAGE_BUGREPORT link clickable (Hennadii Stepanov) Pull request description: This PR is an alternative to https://github.com/bitcoin/bitcoin/pull/18897, and is based on Russ' [idea](https://github.com/bitcoin/bitcoin/pull/18897#pullrequestreview-418703664): > IMO it would be nice to have a followup PR that eliminated the one-line forwarding methods ... Related issues - #91 - https://github.com/bitcoin/bitcoin/issues/18643 Qt docs: https://doc.qt.io/qt-5.12/exceptionsafety.html#exceptions-in-client-code With this PR the GUI handles the wallet-related exception, and: - display it to a user: ![Screenshot from 2021-04-01 02-55-59](https://user-images.githubusercontent.com/32963518/113226183-33ff8480-9298-11eb-8fe6-2168834ab09a.png) - prints a message to `stderr`: ``` ************************ EXCEPTION: 18NonFatalCheckError wallet/wallet.cpp:2677 (IsCurrentForAntiFeeSniping) Internal bug detected: '!chain.findBlock(block_hash, FoundBlock().time(block_time))' You may report this issue here: https://github.com/bitcoin/bitcoin/issues bitcoin in QPushButton->SendCoinsDialog ``` - writes a message to the `debug.log` - and, if the exception is a non-fatal error, leaves the main window running. ACKs for top commit: laanwj: Code review ACK b8e5d0d3fe3386807d47f50d13ac34fcd2a538fd ryanofsky: Code review ACK b8e5d0d3fe3386807d47f50d13ac34fcd2a538fd. This is great! I think more improvements are possible but implementation is very clean and I love how targeted each commit is. Changes since last review: adding more explanatory text, making links clickable, reorganizing. Tree-SHA512: a9f2a2ee8e64b993b0dbc454edcbc39c68c8852abb5dc1feb58f601c0e0e8014dca81c72733aa3fb07b619c6f49b823ed20c7d79cc92088a3abe040ed2149727 --- src/qt/bitcoin.cpp | 17 ++++++++++- src/qt/bitcoin.h | 6 ++++ src/qt/bitcoingui.cpp | 2 +- src/qt/guiutil.cpp | 20 +++++++++++++ src/qt/guiutil.h | 57 +++++++++++++++++++++++++++++++++++++ src/qt/sendcoinsdialog.cpp | 4 ++- src/qt/sendcoinsdialog.h | 2 +- src/qt/test/wallettests.cpp | 2 +- src/qt/walletmodel.cpp | 6 +++- src/qt/walletmodel.h | 2 ++ 10 files changed, 112 insertions(+), 6 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 8884086013..a9be5c5e30 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -47,11 +47,13 @@ #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -491,10 +493,23 @@ void BitcoinApplication::shutdownResult() void BitcoinApplication::handleRunawayException(const QString &message) { - QMessageBox::critical(nullptr, "Runaway exception", BitcoinGUI::tr("A fatal error occurred. %1 can no longer continue safely and will quit.").arg(PACKAGE_NAME) + QString("

") + message); + QMessageBox::critical( + nullptr, tr("Runaway exception"), + tr("A fatal error occurred. %1 can no longer continue safely and will quit.").arg(PACKAGE_NAME) % + QLatin1String("

") % GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT)); ::exit(EXIT_FAILURE); } +void BitcoinApplication::handleNonFatalException(const QString& message) +{ + assert(QThread::currentThread() == thread()); + QMessageBox::warning( + nullptr, tr("Internal error"), + tr("An internal error occurred. %1 will attempt to continue safely. This is " + "an unexpected bug which can be reported as described below.").arg(PACKAGE_NAME) % + QLatin1String("

") % GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT)); +} + WId BitcoinApplication::getMainWinId() const { if (!window) diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index 52ec1cc280..76ebdb4486 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -96,6 +96,12 @@ public Q_SLOTS: /// Handle runaway exceptions. Shows a message box with the problem and quits the program. void handleRunawayException(const QString &message); + /** + * A helper function that shows a message box + * with details about a non-fatal exception. + */ + void handleNonFatalException(const QString& message); + Q_SIGNALS: void requestedInitialize(); void requestedRestart(QStringList args); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index eb0accfdb3..561fefefe1 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -891,7 +891,7 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller) m_open_wallet_action->setEnabled(true); m_open_wallet_action->setMenu(m_open_wallet_menu); - connect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet); + GUIUtil::ExceptionSafeConnect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet); connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet); for (WalletModel* wallet_model : m_wallet_controller->getOpenWallets()) { diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 705e3a952a..e8c13d8906 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -51,6 +51,7 @@ #include #include #include +#include #include #include #include @@ -63,6 +64,7 @@ #include #include #include +#include #include // for Qt::mightBeRichText #include #include @@ -1858,4 +1860,22 @@ QImage GetImage(const QLabel* label) #endif } +QString MakeHtmlLink(const QString& source, const QString& link) +{ + return QString(source).replace( + link, + QLatin1String("") % link % QLatin1String("")); +} + +void PrintSlotException( + const std::exception* exception, + const QObject* sender, + const QObject* receiver) +{ + std::string description = sender->metaObject()->className(); + description += "->"; + description += receiver->metaObject()->className(); + PrintExceptionContinue(std::make_exception_ptr(exception), description.c_str()); +} + } // namespace GUIUtil diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index b5095becd2..d53ac4522c 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -9,18 +9,23 @@ #include #include #include +#include +#include #include #include #include #include +#include #include #include #include #include #include +#include #include +#include class QValidatedLineEdit; class OptionsModel; @@ -520,6 +525,58 @@ namespace GUIUtil QObject::connect(&source, &QObject::destroyed, object, std::forward(function), connection); } + /** + * Replaces a plain text link with an HTML tagged one. + */ + QString MakeHtmlLink(const QString& source, const QString& link); + + void PrintSlotException( + const std::exception* exception, + const QObject* sender, + const QObject* receiver); + + /** + * A drop-in replacement of QObject::connect function + * (see: https://doc.qt.io/qt-5/qobject.html#connect-3), that + * guaranties that all exceptions are handled within the slot. + * + * NOTE: This function is incompatible with Qt private signals. + */ + template + auto ExceptionSafeConnect( + Sender sender, Signal signal, Receiver receiver, Slot method, + Qt::ConnectionType type = Qt::AutoConnection) + { + return QObject::connect( + sender, signal, receiver, + [sender, receiver, method](auto&&... args) { + bool ok{true}; + try { + (receiver->*method)(std::forward(args)...); + } catch (const NonFatalCheckError& e) { + PrintSlotException(&e, sender, receiver); + ok = QMetaObject::invokeMethod( + qApp, "handleNonFatalException", + blockingGUIThreadConnection(), + Q_ARG(QString, QString::fromStdString(e.what()))); + } catch (const std::exception& e) { + PrintSlotException(&e, sender, receiver); + ok = QMetaObject::invokeMethod( + qApp, "handleRunawayException", + blockingGUIThreadConnection(), + Q_ARG(QString, QString::fromStdString(e.what()))); + } catch (...) { + PrintSlotException(nullptr, sender, receiver); + ok = QMetaObject::invokeMethod( + qApp, "handleRunawayException", + blockingGUIThreadConnection(), + Q_ARG(QString, "Unknown failure occurred.")); + } + assert(ok); + }, + type); + } + } // namespace GUIUtil #endif // BITCOIN_QT_GUIUTIL_H diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 6c350a797f..f52b45b17e 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -153,6 +153,8 @@ SendCoinsDialog::SendCoinsDialog(bool _fCoinJoin, QWidget* parent) : } m_coin_control->UseCoinJoin(_fCoinJoin); + + GUIUtil::ExceptionSafeConnect(ui->sendButton, &QPushButton::clicked, this, &SendCoinsDialog::sendButtonClicked); } void SendCoinsDialog::setClientModel(ClientModel *_clientModel) @@ -459,7 +461,7 @@ bool SendCoinsDialog::send(const QList& recipients, QString& return true; } -void SendCoinsDialog::on_sendButton_clicked() +void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked) { if(!model || !model->getOptionsModel()) return; diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h index c8766be9e8..ece928b84b 100644 --- a/src/qt/sendcoinsdialog.h +++ b/src/qt/sendcoinsdialog.h @@ -83,7 +83,7 @@ private: void updateCoinControlState(CCoinControl& ctrl); private Q_SLOTS: - void on_sendButton_clicked(); + void sendButtonClicked(bool checked); void on_buttonChooseFee_clicked(); void on_buttonMinimizeFee_clicked(); void removeEntry(SendCoinsEntry* entry); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 8cae9eaf2d..1da85c9803 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -68,7 +68,7 @@ uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDe if (status == CT_NEW) txid = hash; })); ConfirmSend(); - bool invoked = QMetaObject::invokeMethod(&sendCoinsDialog, "on_sendButton_clicked"); + bool invoked = QMetaObject::invokeMethod(&sendCoinsDialog, "sendButtonClicked", Q_ARG(bool, false)); assert(invoked); return txid; } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 08e783ad91..2b33db9a66 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -68,7 +69,10 @@ WalletModel::~WalletModel() void WalletModel::startPollBalance() { // This timer will be fired repeatedly to update the balance - connect(timer, &QTimer::timeout, this, &WalletModel::pollBalanceChanged); + // Since the QTimer::timeout is a private signal, it cannot be used + // in the GUIUtil::ExceptionSafeConnect directly. + connect(timer, &QTimer::timeout, this, &WalletModel::timerTimeout); + GUIUtil::ExceptionSafeConnect(this, &WalletModel::timerTimeout, this, &WalletModel::pollBalanceChanged); timer->start(MODEL_UPDATE_DELAY); } diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 816a745c6c..8194c141a8 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -233,6 +233,8 @@ Q_SIGNALS: // Notify that there are now keys in the keypool void canGetAddressesChanged(); + void timerTimeout(); + public Q_SLOTS: /* Starts a timer to periodically update the balance */ void startPollBalance(); From 76a41eb24509684d8e79f9397d4e64d5ecf41015 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 11 Apr 2021 13:36:25 +0200 Subject: [PATCH 10/11] Merge #21602: rpc: add additional ban time fields to listbanned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec doc: release notes for new listbanned fields (Jarol Rodriguez) 60290d3f5ec8e7e3b8cb1ebae02d5d72f6005184 test: increase listbanned unit test coverage (Jon Atack) 3e978d1a5dbd43f85bd03e759984ab1f209d6e34 rpc: add time_remaining field to listbanned (Jarol Rodriguez) 5456b345312857981cb426712f0665800c682e09 rpc: add ban_duration field to listbanned (Jarol Rodriguez) c95c61657afd058b46549fb3d65633d7c736f5fc doc: improve listbanned help (Jarol Rodriguez) dd3c8eaa3399b28dc78a883ff78cbe7cc5c31b5b rpc: swap position of banned_until and ban_created fields (Jarol Rodriguez) Pull request description: This PR adds a `ban_duration` and `time_remaining` field to the `listbanned` RPC command. Thanks to jonatack, this PR also expands the `listbanned` test coverage to include these new fields It's useful to keep track of `ban_duration` as this is another data point on which to sort banned peers. I found this helpful in adding additional context columns to the GUI `bantablemodel` as part of a follow-up PR. As [suggested](https://github.com/bitcoin/bitcoin/pull/21602#issuecomment-813486134) by jonatack, `time_remaining` is another useful user-centric data point. Since a ban always expires after its created, the `ban_created` field is now placed before the `banned_until` field. This new ordering is more logical. This PR also improves the `help listbanned` output by providing additional context to the descriptions of the `address`, `ban_created`, and `banned_until` fields. **Master: listbanned** ``` [ { "address": "1.2.3.4/32", "banned_until": 1617691101, "ban_created": 1617604701 }, { "address": "135.181.41.129/32", "banned_until": 1649140716, "ban_created": 1617604716 } ] ``` **PR: listbanned** ``` [ { "address": "1.2.3.4/32", "ban_created": 1617775773, "banned_until": 1617862173, "ban_duration": 86400, "time_remaining": 86392 }, { "address": "3.114.211.172/32", "ban_created": 1617753165, "banned_until": 1618357965, "ban_duration": 604800, "time_remaining": 582184 } ] ``` ACKs for top commit: jonatack: re-ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec hebasto: ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec, tested on Linux Mint 20.1 (x86_64). MarcoFalke: review ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec 🕙 Tree-SHA512: 5b83ed2483344e546d57e43adc8a1ed7a1fff292124b14c86ca3a1aa2aec8b0f7198212fabff2c5145e7f726ca04ae567fe667b141254c7519df290cf63774e5 --- doc/release-notes-21602.md | 7 +++++++ src/rpc/net.cpp | 13 +++++++++---- src/test/rpc_tests.cpp | 17 +++++++++++------ 3 files changed, 27 insertions(+), 10 deletions(-) create mode 100644 doc/release-notes-21602.md diff --git a/doc/release-notes-21602.md b/doc/release-notes-21602.md new file mode 100644 index 0000000000..656b501a12 --- /dev/null +++ b/doc/release-notes-21602.md @@ -0,0 +1,7 @@ +Updated RPCs +------------ + + +- The `listbanned` RPC now returns two new numeric fields: `ban_duration` and `time_remaining`. + Respectively, these new fields indicate the duration of a ban and the time remaining until a ban expires, + both in seconds. Additionally, the `ban_created` field is repositioned to come before `banned_until`. (#5976) diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e906e4759a..1b83b7d6ab 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -789,9 +789,11 @@ static RPCHelpMan listbanned() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "address", ""}, - {RPCResult::Type::NUM_TIME, "banned_until", ""}, - {RPCResult::Type::NUM_TIME, "ban_created", ""}, + {RPCResult::Type::STR, "address", "The IP/Subnet of the banned node"}, + {RPCResult::Type::NUM_TIME, "ban_created", "The " + UNIX_EPOCH_TIME + " the ban was created"}, + {RPCResult::Type::NUM_TIME, "banned_until", "The " + UNIX_EPOCH_TIME + " the ban expires"}, + {RPCResult::Type::NUM_TIME, "ban_duration", "The ban duration, in seconds"}, + {RPCResult::Type::NUM_TIME, "time_remaining", "The time remaining until the ban expires, in seconds"}, }}, }}, RPCExamples{ @@ -808,6 +810,7 @@ static RPCHelpMan listbanned() banmap_t banMap; node.banman->GetBanned(banMap); + const int64_t current_time{GetTime()}; UniValue bannedAddresses(UniValue::VARR); for (const auto& entry : banMap) @@ -815,8 +818,10 @@ static RPCHelpMan listbanned() const CBanEntry& banEntry = entry.second; UniValue rec(UniValue::VOBJ); rec.pushKV("address", entry.first.ToString()); - rec.pushKV("banned_until", banEntry.nBanUntil); rec.pushKV("ban_created", banEntry.nCreateTime); + rec.pushKV("banned_until", banEntry.nBanUntil); + rec.pushKV("ban_duration", (banEntry.nBanUntil - banEntry.nCreateTime)); + rec.pushKV("time_remaining", (banEntry.nBanUntil - current_time)); bannedAddresses.push_back(rec); } diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 3eeb868de9..3208da076b 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -317,9 +317,9 @@ BOOST_AUTO_TEST_CASE(rpc_ban) ar = r.get_array(); o1 = ar[0].get_obj(); adr = find_value(o1, "address"); - UniValue banned_until = find_value(o1, "banned_until"); + int64_t banned_until{find_value(o1, "banned_until").get_int64()}; BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/24"); - BOOST_CHECK_EQUAL(banned_until.get_int64(), 9907731200); // absolute time check + BOOST_CHECK_EQUAL(banned_until, 9907731200); // absolute time check BOOST_CHECK_NO_THROW(CallRPC(std::string("clearbanned"))); @@ -328,11 +328,16 @@ BOOST_AUTO_TEST_CASE(rpc_ban) ar = r.get_array(); o1 = ar[0].get_obj(); adr = find_value(o1, "address"); - banned_until = find_value(o1, "banned_until"); + banned_until = find_value(o1, "banned_until").get_int64(); + const int64_t ban_created{find_value(o1, "ban_created").get_int64()}; + const int64_t ban_duration{find_value(o1, "ban_duration").get_int64()}; + const int64_t time_remaining{find_value(o1, "time_remaining").get_int64()}; + const int64_t now{GetTime()}; BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/24"); - int64_t now = GetTime(); - BOOST_CHECK(banned_until.get_int64() > now); - BOOST_CHECK(banned_until.get_int64()-now <= 200); + BOOST_CHECK(banned_until > now); + BOOST_CHECK(banned_until - now <= 200); + BOOST_CHECK_EQUAL(ban_duration, banned_until - ban_created); + BOOST_CHECK_EQUAL(time_remaining, banned_until - now); // must throw an exception because 127.0.0.1 is in already banned subnet range BOOST_CHECK_THROW(r = CallRPC(std::string("setban 127.0.0.1 add")), std::runtime_error); From 21ad71c5788017a9aad90205ebb4f79811d3f9ac Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 15 Apr 2021 10:05:13 +0200 Subject: [PATCH 11/11] Merge #21676: test: Use mocktime to avoid intermittent failure in rpc_tests fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1 test: Reset mocktime in the common setup (MarcoFalke) fa78590a8fffdfc7e98ddb1f81218f05b1935a0a test: Use mocktime to avoid intermittent failure (MarcoFalke) Pull request description: See https://github.com/bitcoin/bitcoin/pull/21602#discussion_r611176103 ACKs for top commit: jonatack: Code review ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1 jarolrod: ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1 Tree-SHA512: 4967e006f3d2c4eb92f03c9086a6abe3190ad54755d251c30d20422c574bb1a154c06f3d5bcb0d4deaa3c4abfd3864d743b71d84897edd358e829bb42233ad12 --- src/test/denialofservice_tests.cpp | 1 - src/test/logging_tests.cpp | 3 --- src/test/mempool_tests.cpp | 2 -- src/test/rpc_tests.cpp | 10 ++++++---- src/test/util/setup_common.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 5 ----- 6 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 0ae1ad5c31..30e05e4ee1 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -113,7 +113,6 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); // should result in disconnect } BOOST_CHECK(dummyNode1.fDisconnect == true); - SetMockTime(0); peerLogic->FinalizeNode(dummyNode1); } diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index 09a05c3701..e2e31c62d7 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -14,7 +14,6 @@ BOOST_FIXTURE_TEST_SUITE(logging_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(logging_timer) { - SetMockTime(1); auto sec_timer = BCLog::Timer("tests", "end_msg"); SetMockTime(2); @@ -29,8 +28,6 @@ BOOST_AUTO_TEST_CASE(logging_timer) auto micro_timer = BCLog::Timer("tests", "end_msg"); SetMockTime(2); BOOST_CHECK_EQUAL(micro_timer.LogMsg("test micros"), "tests: test micros (1000000μs)"); - - SetMockTime(0); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 3ea1c9fac1..e66bedef05 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -571,8 +571,6 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) SetMockTime(42 + 8*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), 0); // ... unless it has gone all the way to 0 (after getting past 1000/2) - - SetMockTime(0); } inline CTransactionRef make_tx(std::vector&& output_values, std::vector&& inputs=std::vector(), std::vector&& input_indices=std::vector()) diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 3208da076b..44b3c8fae6 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -323,7 +323,11 @@ BOOST_AUTO_TEST_CASE(rpc_ban) BOOST_CHECK_NO_THROW(CallRPC(std::string("clearbanned"))); + auto now = 10'000s; + SetMockTime(now); BOOST_CHECK_NO_THROW(r = CallRPC(std::string("setban 127.0.0.0/24 add 200"))); + SetMockTime(now += 2s); + const int64_t time_remaining_expected{198}; BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned"))); ar = r.get_array(); o1 = ar[0].get_obj(); @@ -332,12 +336,10 @@ BOOST_AUTO_TEST_CASE(rpc_ban) const int64_t ban_created{find_value(o1, "ban_created").get_int64()}; const int64_t ban_duration{find_value(o1, "ban_duration").get_int64()}; const int64_t time_remaining{find_value(o1, "time_remaining").get_int64()}; - const int64_t now{GetTime()}; BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/24"); - BOOST_CHECK(banned_until > now); - BOOST_CHECK(banned_until - now <= 200); + BOOST_CHECK_EQUAL(banned_until, time_remaining_expected + now.count()); BOOST_CHECK_EQUAL(ban_duration, banned_until - ban_created); - BOOST_CHECK_EQUAL(time_remaining, banned_until - now); + BOOST_CHECK_EQUAL(time_remaining, time_remaining_expected); // must throw an exception because 127.0.0.1 is in already banned subnet range BOOST_CHECK_THROW(r = CallRPC(std::string("setban 127.0.0.1 add")), std::runtime_error); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index d40b0ecb7a..2780624056 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -195,6 +195,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve BasicTestingSetup::~BasicTestingSetup() { + SetMockTime(0s); // Reset mocktime for following tests connman.reset(); llmq::quorumSnapshotManager.reset(); m_node.cpoolman.reset(); @@ -498,7 +499,6 @@ TestChainSetup::~TestChainSetup() g_txindex->Stop(); SyncWithValidationInterfaceQueue(); g_txindex.reset(); - SetMockTime(0); } CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) const diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 6fc3fd3006..c470c33428 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -315,8 +315,6 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) BOOST_CHECK_EQUAL(found, expected); } } - - SetMockTime(0); } // Verify getaddressinfo RPC produces more or less expected results @@ -473,9 +471,6 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart) // If there are future entries, new transaction should use time of the // newest entry that is no more than 300 seconds ahead of the clock time. BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 5, 50, 600), 300); - - // Reset mock time for other tests. - SetMockTime(0); } BOOST_AUTO_TEST_CASE(LoadReceiveRequests)