From cf627fba6d74b527b38d6b5557adcef21b9db40c Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 6 Nov 2018 11:08:40 +0100 Subject: [PATCH 01/10] Merge #14522: tests: add invalid P2P message tests d20a9fa13d1c13f552e879798c0508be70190e71 tests: add tests for invalid P2P messages (James O'Beirne) 62f94d39f8de88a44bb0a8a2837d864f777aaacc tests: add P2PConnection.send_raw_message (James O'Beirne) 5aa31f6ef26f51ce461c917654dd1cfbbdd1409a tests: add utility to assert node memory usage hasn't increased (James O'Beirne) Pull request description: - Adds `p2p_invalid_messages.py`: tests based on behavior for dealing with invalid and malformed P2P messages. Includes a test verifying that we can't DoS a node by spamming it with large invalid messages. - Adds `TestNode.assert_memory_usage_stable`: a context manager that allows us to ensure memory usage doesn't significantly increase on a node during some test. - Adds `P2PConnection.send_raw_message`: which allows us to construct and send messages with tweaked headers. Tree-SHA512: 720a4894c1e6d8f1551b2ae710e5b06c9e4f281524623957cb01599be9afea82671dc26d6152281de0acb87720f0c53b61e2b27d40434d30e525dd9e31fa671f --- test/functional/p2p_invalid_messages.py | 175 ++++++++++++++++++++ test/functional/test_framework/mininode.py | 15 +- test/functional/test_framework/test_node.py | 45 +++++ test/functional/test_runner.py | 1 + 4 files changed, 230 insertions(+), 6 deletions(-) create mode 100755 test/functional/p2p_invalid_messages.py diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py new file mode 100755 index 000000000000..85f035628ff6 --- /dev/null +++ b/test/functional/p2p_invalid_messages.py @@ -0,0 +1,175 @@ +#!/usr/bin/env python3 +# Copyright (c) 2015-2018 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 node responses to invalid network messages.""" +import struct + +from test_framework import messages +from test_framework.mininode import P2PDataStore +from test_framework.test_framework import BitcoinTestFramework + + +class msg_unrecognized: + """Nonsensical message. Modeled after similar types in test_framework.messages.""" + + command = b'badmsg' + + def __init__(self, str_data): + self.str_data = str_data.encode() if not isinstance(str_data, bytes) else str_data + + def serialize(self): + return messages.ser_string(self.str_data) + + def __repr__(self): + return "{}(data={})".format(self.command, self.str_data) + + +class msg_nametoolong(msg_unrecognized): + + command = b'thisnameiswayyyyyyyyytoolong' + + +class InvalidMessagesTest(BitcoinTestFramework): + + def set_test_params(self): + self.num_nodes = 1 + self.setup_clean_chain = True + + def run_test(self): + """ + 0. Send a bunch of large (4MB) messages of an unrecognized type. Check to see + that it isn't an effective DoS against the node. + + 1. Send an oversized (4MB+) message and check that we're disconnected. + + 2. Send a few messages with an incorrect data size in the header, ensure the + messages are ignored. + + 3. Send an unrecognized message with a command name longer than 12 characters. + + """ + node = self.nodes[0] + self.node = node + node.add_p2p_connection(P2PDataStore()) + conn2 = node.add_p2p_connection(P2PDataStore()) + + msg_limit = 4 * 1000 * 1000 # 4MB, per MAX_PROTOCOL_MESSAGE_LENGTH + valid_data_limit = msg_limit - 5 # Account for the 4-byte length prefix + + # + # 0. + # + # Send as large a message as is valid, ensure we aren't disconnected but + # also can't exhaust resources. + # + msg_at_size = msg_unrecognized("b" * valid_data_limit) + assert len(msg_at_size.serialize()) == msg_limit + + with node.assert_memory_usage_stable(perc_increase_allowed=0.03): + self.log.info( + "Sending a bunch of large, junk messages to test " + "memory exhaustion. May take a bit...") + + # Run a bunch of times to test for memory exhaustion. + for _ in range(200): + node.p2p.send_message(msg_at_size) + + # Check that, even though the node is being hammered by nonsense from one + # connection, it can still service other peers in a timely way. + for _ in range(20): + conn2.sync_with_ping(timeout=2) + + # Peer 1, despite serving up a bunch of nonsense, should still be connected. + self.log.info("Waiting for node to drop junk messages.") + node.p2p.sync_with_ping(timeout=8) + assert node.p2p.is_connected + + # + # 1. + # + # Send an oversized message, ensure we're disconnected. + # + msg_over_size = msg_unrecognized("b" * (valid_data_limit + 1)) + assert len(msg_over_size.serialize()) == (msg_limit + 1) + + with node.assert_debug_log(["Oversized message from peer=0, disconnecting"]): + # An unknown message type (or *any* message type) over + # MAX_PROTOCOL_MESSAGE_LENGTH should result in a disconnect. + node.p2p.send_message(msg_over_size) + node.p2p.wait_for_disconnect(timeout=4) + + node.disconnect_p2ps() + conn = node.add_p2p_connection(P2PDataStore()) + conn.wait_for_verack() + + # + # 2. + # + # Send messages with an incorrect data size in the header. + # + actual_size = 100 + msg = msg_unrecognized("b" * actual_size) + + # TODO: handle larger-than cases. I haven't been able to pin down what behavior to expect. + for wrong_size in (2, 77, 78, 79): + self.log.info("Sending a message with incorrect size of {}".format(wrong_size)) + + # Unmodified message should submit okay. + node.p2p.send_and_ping(msg) + + # A message lying about its data size results in a disconnect when the incorrect + # data size is less than the actual size. + # + # TODO: why does behavior change at 78 bytes? + # + node.p2p.send_raw_message(self._tweak_msg_data_size(msg, wrong_size)) + + # For some reason unknown to me, we sometimes have to push additional data to the + # peer in order for it to realize a disconnect. + try: + node.p2p.send_message(messages.msg_ping(nonce=123123)) + except IOError: + pass + + node.p2p.wait_for_disconnect(timeout=10) + node.disconnect_p2ps() + node.add_p2p_connection(P2PDataStore()) + + # + # 3. + # + # Send a message with a too-long command name. + # + node.p2p.send_message(msg_nametoolong("foobar")) + node.p2p.wait_for_disconnect(timeout=4) + + # Node is still up. + conn = node.add_p2p_connection(P2PDataStore()) + conn.sync_with_ping() + + + def _tweak_msg_data_size(self, message, wrong_size): + """ + Return a raw message based on another message but with an incorrect data size in + the message header. + """ + raw_msg = self.node.p2p.build_message(message) + + bad_size_bytes = struct.pack(" str: """Return a modified msg that identifies this node by its index as a debugging aid.""" return "[node %d] %s" % (self.index, msg) @@ -298,6 +320,29 @@ def assert_debug_log(self, expected_msgs): if re.search(re.escape(expected_msg), log, flags=re.MULTILINE) is None: self._raise_assertion_error('Expected message "{}" does not partially match log:\n\n{}\n\n'.format(expected_msg, print_log)) + @contextlib.contextmanager + def assert_memory_usage_stable(self, perc_increase_allowed=0.03): + """Context manager that allows the user to assert that a node's memory usage (RSS) + hasn't increased beyond some threshold percentage. + """ + before_memory_usage = self.get_mem_rss() + + yield + + after_memory_usage = self.get_mem_rss() + + if not (before_memory_usage and after_memory_usage): + self.log.warning("Unable to detect memory usage (RSS) - skipping memory check.") + return + + perc_increase_memory_usage = 1 - (float(before_memory_usage) / after_memory_usage) + + if perc_increase_memory_usage > perc_increase_allowed: + self._raise_assertion_error( + "Memory usage increased over threshold of {:.3f}% from {} to {} ({:.3f}%)".format( + perc_increase_allowed * 100, before_memory_usage, after_memory_usage, + perc_increase_memory_usage * 100)) + @contextlib.contextmanager def profile_with_perf(self, profile_name): """ diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 7e62c6101341..2fa8978c6351 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -140,6 +140,7 @@ 'mining_prioritisetransaction.py', 'p2p_invalid_locator.py', 'p2p_invalid_block.py', + 'p2p_invalid_messages.py', 'p2p_invalid_tx.py', 'feature_assumevalid.py', 'example_test.py', From ea6def648d4257c5b89cca721b5925d4df96a10b Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 8 Aug 2021 21:58:24 +0700 Subject: [PATCH 02/10] Fixed test by updating length of message accordingly dash' MAX MESSAGE SIZE --- test/functional/p2p_invalid_messages.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index 85f035628ff6..b07636982f69 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -38,10 +38,10 @@ def set_test_params(self): def run_test(self): """ - 0. Send a bunch of large (4MB) messages of an unrecognized type. Check to see + 0. Send a bunch of large (3MB) messages of an unrecognized type. Check to see that it isn't an effective DoS against the node. - 1. Send an oversized (4MB+) message and check that we're disconnected. + 1. Send an oversized (3MB+) message and check that we're disconnected. 2. Send a few messages with an incorrect data size in the header, ensure the messages are ignored. @@ -54,7 +54,7 @@ def run_test(self): node.add_p2p_connection(P2PDataStore()) conn2 = node.add_p2p_connection(P2PDataStore()) - msg_limit = 4 * 1000 * 1000 # 4MB, per MAX_PROTOCOL_MESSAGE_LENGTH + msg_limit = 3 * 1024 * 1024 # 3MB, per MAX_PROTOCOL_MESSAGE_LENGTH valid_data_limit = msg_limit - 5 # Account for the 4-byte length prefix # From f8a5d6fc76345e053bb84f6cbf5cbad2c725fe4d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 6 Nov 2018 15:14:38 -0500 Subject: [PATCH 03/10] Merge #14672: tests: Send fewer spam messages in p2p_invalid_messages 3d305e3b89 Send fewer spam messages in p2p_invalid_messages (James O'Beirne) Pull request description: Builds on travis are failing because the test node isn't able to drop all the bad messages sent within the given timeout. Reduce the number of bad messages we're sending and increase the timeout to avoid failures on travis. Tree-SHA512: 11c389619d9590caf7eca74e0efe6d950469415d34220072770689024b350cc08a2d5ec90634237d87ff71ba8b638c1152b8a45ffbb2815a48bde6a88fbb8fc6 --- test/functional/p2p_invalid_messages.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index b07636982f69..1581525b8635 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -72,7 +72,7 @@ def run_test(self): "memory exhaustion. May take a bit...") # Run a bunch of times to test for memory exhaustion. - for _ in range(200): + for _ in range(80): node.p2p.send_message(msg_at_size) # Check that, even though the node is being hammered by nonsense from one @@ -82,7 +82,7 @@ def run_test(self): # Peer 1, despite serving up a bunch of nonsense, should still be connected. self.log.info("Waiting for node to drop junk messages.") - node.p2p.sync_with_ping(timeout=8) + node.p2p.sync_with_ping(timeout=30) assert node.p2p.is_connected # From a52460d709e8371f4c44951cd499e874dbaffea5 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 26 Nov 2018 16:14:50 -0500 Subject: [PATCH 04/10] Merge #14812: qa: fix p2p_invalid_messages on macOS 5a1f57646b qa: clean up assert_memory_usage_stable utility (James O'Beirne) 0cf1632f03 qa: fix p2p_invalid_messages on macOS (James O'Beirne) Pull request description: Infinite mea culpa for the number of problems with this test. This change bumps the acceptable RSS increase threshold from 3% to 50% when spamming the test node with junk 4MB messages. On [@MarcoFalke's macOS test build](https://travis-ci.org/MarcoFalke/btc_nightly) we see RSS grow ~14% from ~71MB to 81MB, so a 50% increase threshold should be more than sufficient to avoid spurious failures. Tree-SHA512: 150a7b88080fd883c7a5d0b9ffa470f61a97c4885fccc1a06fde6260aaef15640a7c1de7e89c581b245df7807d617ec3d86775330386ec5149ad567492fc5d31 --- test/functional/p2p_invalid_messages.py | 2 +- test/functional/test_framework/test_node.py | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index 1581525b8635..dbf9c057a525 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -66,7 +66,7 @@ def run_test(self): msg_at_size = msg_unrecognized("b" * valid_data_limit) assert len(msg_at_size.serialize()) == msg_limit - with node.assert_memory_usage_stable(perc_increase_allowed=0.03): + with node.assert_memory_usage_stable(increase_allowed=0.5): self.log.info( "Sending a bunch of large, junk messages to test " "memory exhaustion. May take a bit...") diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 3e48afdac991..6afb156f8c5b 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -134,7 +134,7 @@ def get_deterministic_priv_key(self): ] return PRIV_KEYS[self.index] - def get_mem_rss(self): + def get_mem_rss_kilobytes(self): """Get the memory usage (RSS) per `ps`. If process is stopped or `ps` is unavailable, return None. @@ -321,15 +321,19 @@ def assert_debug_log(self, expected_msgs): self._raise_assertion_error('Expected message "{}" does not partially match log:\n\n{}\n\n'.format(expected_msg, print_log)) @contextlib.contextmanager - def assert_memory_usage_stable(self, perc_increase_allowed=0.03): + def assert_memory_usage_stable(self, *, increase_allowed=0.03): """Context manager that allows the user to assert that a node's memory usage (RSS) hasn't increased beyond some threshold percentage. + + Args: + increase_allowed (float): the fractional increase in memory allowed until failure; + e.g. `0.12` for up to 12% increase allowed. """ - before_memory_usage = self.get_mem_rss() + before_memory_usage = self.get_mem_rss_kilobytes() yield - after_memory_usage = self.get_mem_rss() + after_memory_usage = self.get_mem_rss_kilobytes() if not (before_memory_usage and after_memory_usage): self.log.warning("Unable to detect memory usage (RSS) - skipping memory check.") @@ -337,10 +341,10 @@ def assert_memory_usage_stable(self, perc_increase_allowed=0.03): perc_increase_memory_usage = 1 - (float(before_memory_usage) / after_memory_usage) - if perc_increase_memory_usage > perc_increase_allowed: + if perc_increase_memory_usage > increase_allowed: self._raise_assertion_error( "Memory usage increased over threshold of {:.3f}% from {} to {} ({:.3f}%)".format( - perc_increase_allowed * 100, before_memory_usage, after_memory_usage, + increase_allowed * 100, before_memory_usage, after_memory_usage, perc_increase_memory_usage * 100)) @contextlib.contextmanager From 42d1b0ba71b55f8599f8d19699167f39d875ee32 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 12 Nov 2018 15:20:30 +0100 Subject: [PATCH 05/10] Merge #14494: Error if # is used in rpcpassword in conf 0385109444646561a718f34ae437b7e0e4d4d5bc Add test for rpcpassword hash error (MeshCollider) 13fe258e91e7a92326aedf151c571994166a06d4 Error if rpcpassword in conf contains a hash character (MeshCollider) Pull request description: Fixes #13143 now #13482 was merged Tree-SHA512: e7d00c8df1657f6b8d0eee1e06b9ce2b1b0a2de487377699382c1b057836e1571dac313ca878b5877c862f0461ba789a50b239d2a9f34accd8a6321f126e3d2a --- src/util/system.cpp | 6 ++++++ test/functional/feature_config_args.py | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/util/system.cpp b/src/util/system.cpp index ca3c8bb44631..e6d8588ccb09 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -885,8 +885,10 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect std::string::size_type pos; int linenr = 1; while (std::getline(stream, str)) { + bool used_hash = false; if ((pos = str.find('#')) != std::string::npos) { str = str.substr(0, pos); + used_hash = true; } const static std::string pattern = " \t\r\n"; str = TrimString(str, pattern); @@ -899,6 +901,10 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect } else if ((pos = str.find('=')) != std::string::npos) { std::string name = prefix + TrimString(str.substr(0, pos), pattern); std::string value = TrimString(str.substr(pos + 1), pattern); + if (used_hash && name == "rpcpassword") { + error = strprintf("parse error on line %i, using # in rpcpassword can be ambiguous and should be avoided", linenr); + return false; + } options.emplace_back(name, value); } else { error = strprintf("parse error on line %i: %s", linenr, str); diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index efe03829993b..feef4e7f5b74 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -39,6 +39,10 @@ def test_config_file_parser(self): conf.write('nono\n') self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 1: nono, if you intended to specify a negated option, use nono=1 instead') + with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: + conf.write('server=1\nrpcuser=someuser\nrpcpassword=some#pass') + self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 3, using # in rpcpassword can be ambiguous and should be avoided') + with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: conf.write('') # clear From 739c675f03343cb33ad3e0934b4a569e9a281b9e Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 13 Nov 2018 13:43:52 +0100 Subject: [PATCH 06/10] Merge #14690: Throw error if CPubKey is invalid during PSBT keypath serialization 4e4de10f69d5d705256cadfb15d76314dff16e77 Throw error if CPubKey is invalid during PSBT keypath serialization (Gregory Sanders) Pull request description: Related to https://github.com/bitcoin/bitcoin/pull/14689 We should catch this error before attempting to deserialize it later. Tree-SHA512: d2f3ea7f363818ac70c81ee988231b2bb50d055b6919f7bff3f27120c85a7048bfa183efae33e23e6b81d684bcb8bb81e5b209abb3acbcaff1d88014f4f1aa93 --- src/script/sign.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/script/sign.h b/src/script/sign.h index b80dd7848d36..4df9a9bb9249 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -180,6 +180,9 @@ template void SerializeHDKeypaths(Stream& s, const std::map& hd_keypaths, uint8_t type) { for (auto keypath_pair : hd_keypaths) { + if (!keypath_pair.first.IsValid()) { + throw std::ios_base::failure("Invalid CPubKey being serialized"); + } SerializeToVector(s, type, MakeSpan(keypath_pair.first)); WriteCompactSize(s, (keypath_pair.second.path.size() + 1) * sizeof(uint32_t)); s << keypath_pair.second.fingerprint; From 701e16ba5b3084d981eb7bf43f90067f24beb960 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 18 Sep 2018 01:19:45 +0200 Subject: [PATCH 07/10] Merge #14247: Fix crash bug with duplicate inputs within a transaction 9b4a36effcf642f3844c6696b757266686ece11a [qa] Test for duplicate inputs within a transaction (Suhas Daftuar) b8f801964f59586508ea8da6cf3decd76bc0e571 Fix crash bug with duplicate inputs within a transaction (Suhas Daftuar) Pull request description: Tree-SHA512: 8c7ea34c7fa44188d86c04a690a7cbf8e9deda71ab1f7ca6d11de1f2abb3dd7222627071f86d0d39689a8b302ba9af142f0202466a67e30cd54aed3a08d4eb14 --- test/functional/p2p_invalid_block.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/functional/p2p_invalid_block.py b/test/functional/p2p_invalid_block.py index 3225b33ccd97..de8e7d81b3ed 100755 --- a/test/functional/p2p_invalid_block.py +++ b/test/functional/p2p_invalid_block.py @@ -82,6 +82,16 @@ def run_test(self): node.p2p.send_blocks_and_test([block2], node, success=False, request_block=False, reject_code=16, reject_reason=b'bad-txns-duplicate') + # Check transactions for duplicate inputs + self.log.info("Test duplicate input block.") + + block2_orig.vtx[2].vin.append(block2_orig.vtx[2].vin[0]) + block2_orig.vtx[2].rehash() + block2_orig.hashMerkleRoot = block2_orig.calc_merkle_root() + block2_orig.rehash() + block2_orig.solve() + node.p2p.send_blocks_and_test([block2_orig], node, success=False, request_block=False, reject_reason='bad-txns-inputs-duplicate') + self.log.info("Test very broken block.") block3 = create_block(tip, create_coinbase(height), block_time) From 5bda9afcf6b148af684386bbd5bb66aa82c5157d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 13 Nov 2018 10:24:49 -0500 Subject: [PATCH 08/10] Partial merge #14700: qa: Avoid race in p2p_invalid_block by waiting for the block request fa21568208 qa: Avoid race in p2p_invalid_block by waiting for the block request (MarcoFalke) 6c787d340c tests: Make feature_block pass on centos (MarcoFalke) Pull request description: This hopefully fixes #14661, which I believe is caused by a race in `send_blocks_and_test`. By setting `request_block=False` we only effectively check `node.getbestblockhash() != blocks[-1].hash` before returning and checking the debug.log. By setting `request_block=True` (the default) we make sure that we send the block, then sync with a ping before asserting on the debug.log. Even if this patch doesn't fix the issue, it is good cleanup: There is no reason to not wait for the blocks to be requested, since in all these cases the header gives no indication that the block is consensus invalid. So this patch makes the test also a bit stricter and more useful. Unrelated to this, I also include a fix that makes the tests pass on latest CentOS. Tree-SHA512: c7abee3b7dc790a8af6c289159a7751bd962f6fa16c1537e7e21a0a0ef05b9596d1f4eb75319614603c05cb803e021314fa3596508ba443edd03046b25527e0f --- test/functional/p2p_invalid_block.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/functional/p2p_invalid_block.py b/test/functional/p2p_invalid_block.py index de8e7d81b3ed..a2e60a50d82a 100755 --- a/test/functional/p2p_invalid_block.py +++ b/test/functional/p2p_invalid_block.py @@ -78,9 +78,9 @@ def run_test(self): block2.vtx.append(tx2) assert_equal(block2.hashMerkleRoot, block2.calc_merkle_root()) assert_equal(orig_hash, block2.rehash()) - assert(block2_orig.vtx != block2.vtx) + assert block2_orig.vtx != block2.vtx - node.p2p.send_blocks_and_test([block2], node, success=False, request_block=False, reject_code=16, reject_reason=b'bad-txns-duplicate') + node.p2p.send_blocks_and_test([block2], node, success=False, reject_code=16, reject_reason=b'bad-txns-duplicate') # Check transactions for duplicate inputs self.log.info("Test duplicate input block.") @@ -90,7 +90,7 @@ def run_test(self): block2_orig.hashMerkleRoot = block2_orig.calc_merkle_root() block2_orig.rehash() block2_orig.solve() - node.p2p.send_blocks_and_test([block2_orig], node, success=False, request_block=False, reject_reason='bad-txns-inputs-duplicate') + node.p2p.send_blocks_and_test([block2_orig], node, success=False, reject_code=16, reject_reason=b'bad-txns-inputs-duplicate') self.log.info("Test very broken block.") @@ -103,7 +103,7 @@ def run_test(self): block3.rehash() block3.solve() - node.p2p.send_blocks_and_test([block3], node, success=False, request_block=False, reject_code=16, reject_reason=b'bad-cb-amount') + node.p2p.send_blocks_and_test([block3], node, success=False, reject_code=16, reject_reason=b'bad-cb-amount') if __name__ == '__main__': InvalidBlockRequestTest().main() From 6699b4d79a4556a24ba4a707aa4d35599d8bfd7b Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 13 Nov 2018 12:25:22 -0500 Subject: [PATCH 09/10] Merge #14705: travis: Avoid timeout on verify-commits check fa5a6ce102 qa: Raise ci test_runner timeout to 40 mins (MarcoFalke) fa3df025e1 travis: Avoid timeout on verify-commits check (MarcoFalke) Pull request description: The verify-commits check is too expensive to run in full (calculate Tree-SHA512 and clean-merge for every single merge commit in history) every day (the cron job runs every ~24h). Since the cron job is running every day, it is also redundant to redo most of the work on the next day. So, only check two days worth of commits and assume that travis checked the Tree-SHA512 and clean-merge for all other commits already. The script will still check all the signatures, since the check-result for them depends on external inputs such as current time or the public keys we got from the server. [Note that travis is not meant to do the verification for anyone or is meant to be trusted in any way. This check only serves as a belt-and-suspender to notify maintainers in case of a technical issue or script malfunction. But since the script is timing out for months now, its purpose is diminished right now.] Tree-SHA512: 336c5cbcc03cdf50be96cd61412471be9078d862da8ba2054f337441e062a6067c95fbbd03912e3de6a116f3caa75fd3f01a04864d34aae1489faa3154572815 --- .travis/lint_06_script.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis/lint_06_script.sh b/.travis/lint_06_script.sh index 6191d825716a..701e6d8005c8 100755 --- a/.travis/lint_06_script.sh +++ b/.travis/lint_06_script.sh @@ -19,6 +19,7 @@ test/lint/check-rpc-mappings.py . test/lint/lint-all.sh if [ "$TRAVIS_REPO_SLUG" = "bitcoin/bitcoin" -a "$TRAVIS_EVENT_TYPE" = "cron" ]; then + git log --merges --before="2 days ago" -1 --format='%H' > ./contrib/verify-commits/trusted-sha512-root-commit while read -r LINE; do travis_retry gpg --keyserver hkp://subset.pool.sks-keyservers.net --recv-keys $LINE; done < contrib/verify-commits/trusted-keys && - travis_wait 50 contrib/verify-commits/verify-commits.py; + travis_wait 50 contrib/verify-commits/verify-commits.py --clean-merge=2; fi From 1809b8f03830572a51d3153a9b94a9484a05504c Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 14 Nov 2018 11:26:51 -0500 Subject: [PATCH 10/10] Merge #14478: Show error to user when corrupt wallet unlock fails b4f6e58ca5 Better error message for user when corrupt wallet unlock fails (MeshCollider) Pull request description: Mentioned here: https://github.com/bitcoin/bitcoin/issues/14461#issuecomment-429183503 Current behavior is to assert(false) and crash, only info is printed in the log. This shows the message to the user before abort() instead. Tree-SHA512: 526f9ed9262257fca55caf7153ab913ed958b13b079d2f01db797485614d8c375815a1554276e8cf73d3838104b2691a9cf85c8d097973127ae8de9e111446bf --- src/qt/askpassphrasedialog.cpp | 20 ++++++++++++-------- src/wallet/crypter.cpp | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/qt/askpassphrasedialog.cpp b/src/qt/askpassphrasedialog.cpp index 5fc2d65d4f4e..2d4053e15f1a 100644 --- a/src/qt/askpassphrasedialog.cpp +++ b/src/qt/askpassphrasedialog.cpp @@ -183,14 +183,18 @@ void AskPassphraseDialog::accept() } break; case UnlockMixing: case Unlock: - if(!model->setWalletLocked(false, oldpass, mode == UnlockMixing)) - { - QMessageBox::critical(this, tr("Wallet unlock failed"), - tr("The passphrase entered for the wallet decryption was incorrect.")); - } - else - { - QDialog::accept(); // Success + try { + if(!model->setWalletLocked(false, oldpass, mode == UnlockMixing)) + { + QMessageBox::critical(this, tr("Wallet unlock failed"), + tr("The passphrase entered for the wallet decryption was incorrect.")); + } + else + { + QDialog::accept(); // Success + } + } catch (const std::runtime_error& e) { + QMessageBox::critical(this, tr("Wallet unlock failed"), e.what()); } break; case Decrypt: diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index c50c12f4d061..45914ce1bded 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -269,7 +269,7 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn, bool fForMixin if (keyPass && keyFail) { LogPrintf("The wallet is probably corrupted: Some keys decrypt but not all.\n"); - assert(false); + throw std::runtime_error("Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt."); } if (keyFail || (!keyPass && cryptedHDChain.IsNull())) return false;