From 2b247866b681398f84f15a2a44ab859eb2b56a55 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 26 Jul 2018 18:26:23 -0400 Subject: [PATCH 01/26] Merge #13773: wallet: Fix accidental use of the comma operator aecd615ad7 wallet: Fix accidental use of the comma operator (practicalswift) Pull request description: Fix accidental use of the comma operator. Tree-SHA512: 035bf497343996c67a2fe25f367d1a416811b518cb9c7a18ce92355627871614e40db699e869881f941bc51e47fb94022f5ae13e7f01462ef35249b2dd3647a0 --- src/interfaces/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 359b975407..b74b93f3e7 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -104,7 +104,7 @@ WalletTxStatus MakeWalletTxStatus(const CWalletTx& wtx) WalletTxStatus result; auto mi = ::mapBlockIndex.find(wtx.hashBlock); CBlockIndex* block = mi != ::mapBlockIndex.end() ? mi->second : nullptr; - result.block_height = (block ? block->nHeight : std::numeric_limits::max()), + result.block_height = (block ? block->nHeight : std::numeric_limits::max()); result.blocks_to_maturity = wtx.GetBlocksToMaturity(); result.depth_in_main_chain = wtx.GetDepthInMainChain(); result.time_received = wtx.nTimeReceived; From 5698ebb7fef6903308094bd1bf3ae49dc333f390 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 30 Jul 2018 11:36:46 -0400 Subject: [PATCH 02/26] Merge #13803: doc: add note to contributor docs about warranted PR's db213aa47f doc: add note to contributor docs about warranted PR's (Karl-Johan Alm) Pull request description: Tree-SHA512: 39d4085ec0217c56b0d6a34e95d7b7a18e0373ec25549e6460cb8ef16218a6060f15e539ec2f8cceccd1188d2769e14fc276071f214ceb80db9b08ec5c24ccef --- CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cc8421fc6c..76633c50ea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -101,6 +101,8 @@ At this stage one should expect comments and review from other contributors. You can add more commits to your pull request by committing them locally and pushing to your fork until you have satisfied all feedback. +Note: Code review is a burdensome but important part of the development process, and as such, certain types of pull requests are rejected. In general, if the **improvements** do not warrant the **review effort** required, the PR has a high chance of being rejected. It is up to the PR author to convince the reviewers that the changes warrant the review effort, and if reviewers are "Concept NAK'ing" the PR, the author may need to present arguments and/or do research backing their suggested changes. + Squashing Commits --------------------------- If your pull request is accepted for merging, you may be asked by a maintainer From 1ecce00405cc2cb97ba55a4ab0633b97fba434fc Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 7 Aug 2018 16:40:49 +0200 Subject: [PATCH 03/26] Merge #13843: [trivial] Add doxygen-compatible comments to CAffectedKeysVisitor 3339d845354c9c357ec90505192748d9d639e72e [trivial] add doxygen-compatible comments to CAffectedKeysVisitor (Pierre Rochard) Pull request description: Tree-SHA512: 0003fde198a6977d0c8988efc8f76428f9e095009fddf131b07bd9809ef76a778c86bb2b1305e33df16101b6b703cf43eb6193462bb9f3687f98c1d9b109dd96 --- src/wallet/wallet.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6da5cadea9..f40122ebd5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -118,14 +118,25 @@ std::string COutput::ToString() const return strprintf("COutput(%s, %d, %d) [%s]", tx->GetHash().ToString(), i, nDepth, FormatMoney(tx->tx->vout[i].nValue)); } +/** A class to identify which pubkeys a script and a keystore have in common. */ class CAffectedKeysVisitor : public boost::static_visitor { private: const CKeyStore &keystore; std::vector &vKeys; public: + /** + * @param[in] keystoreIn The CKeyStore that is queried for the presence of a pubkey. + * @param[out] vKeysIn A vector to which a script's pubkey identifiers are appended if they are in the keystore. + */ CAffectedKeysVisitor(const CKeyStore &keystoreIn, std::vector &vKeysIn) : keystore(keystoreIn), vKeys(vKeysIn) {} + /** + * Apply the visitor to each destination in a script, recursively to the redeemscript + * in the case of p2sh destinations. + * @param[in] script The CScript from which destinations are extracted. + * @post Any CKeyIDs that script and keystore have in common are appended to the visitor's vKeys. + */ void Process(const CScript &script) { txnouttype type; std::vector vDest; From b04090df107c2f0184e93edf5992221e51787cde Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 11 Aug 2018 06:45:29 -0400 Subject: [PATCH 04/26] Merge #13913: qa: Remove redundant checkmempool/checkblockindex extra_args fa31ca0c22 qa: Remove redundant checkmempool/checkblockindex extra_args (MarcoFalke) Pull request description: They are already enabled by default for regtest: https://github.com/bitcoin/bitcoin/blob/df9f71274645a917e2578c52a1c59745bce8112d/src/init.cpp#L1002-L1007 Closes #13912. CC #12138 Tree-SHA512: b11a3e8cc4715569f917ab89132f8c8dcae64aebcd7a34182675f86cf7f6e207e3187b7ea01a56c92c8c3af76122b6b995e84f533e134676863f8953dc1f0574 --- test/functional/feature_reindex.py | 2 +- test/functional/mempool_accept.py | 1 - test/functional/mempool_reorg.py | 3 +-- test/functional/mempool_resurrect.py | 3 +-- test/functional/mempool_spend_coinbase.py | 3 +-- 5 files changed, 4 insertions(+), 8 deletions(-) diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py index 9dd9e331e8..daecdde4c2 100755 --- a/test/functional/feature_reindex.py +++ b/test/functional/feature_reindex.py @@ -22,7 +22,7 @@ class ReindexTest(BitcoinTestFramework): self.nodes[0].generate(3) blockcount = self.nodes[0].getblockcount() self.stop_nodes() - extra_args = [["-reindex-chainstate" if justchainstate else "-reindex", "-checkblockindex=1"]] + extra_args = [["-reindex-chainstate" if justchainstate else "-reindex"]] self.start_nodes(extra_args) wait_until(lambda: self.nodes[0].getblockcount() == blockcount) self.log.info("Success") diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 2e8736025f..57f81e1973 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -36,7 +36,6 @@ class MempoolAcceptanceTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.extra_args = [[ - '-checkmempool', '-txindex', '-reindex', # Need reindex for txindex '-acceptnonstdtxn=0', # Try to mimic main-net diff --git a/test/functional/mempool_reorg.py b/test/functional/mempool_reorg.py index 9321fa4bae..6d5e116df9 100755 --- a/test/functional/mempool_reorg.py +++ b/test/functional/mempool_reorg.py @@ -11,11 +11,10 @@ that spend (directly or indirectly) coinbase transactions. from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * -# Create one-input, one-output, no-fee transaction: + class MempoolCoinbaseTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 - self.extra_args = [["-checkmempool"]] * 2 alert_filename = None # Set by setup_network diff --git a/test/functional/mempool_resurrect.py b/test/functional/mempool_resurrect.py index 088cef5bac..90e106be3c 100755 --- a/test/functional/mempool_resurrect.py +++ b/test/functional/mempool_resurrect.py @@ -7,11 +7,10 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * -# Create one-input, one-output, no-fee transaction: + class MempoolCoinbaseTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [["-checkmempool"]] def run_test(self): node0_address = self.nodes[0].getnewaddress() diff --git a/test/functional/mempool_spend_coinbase.py b/test/functional/mempool_spend_coinbase.py index ea297f150a..c775cda39b 100755 --- a/test/functional/mempool_spend_coinbase.py +++ b/test/functional/mempool_spend_coinbase.py @@ -15,11 +15,10 @@ but less mature coinbase spends are NOT. from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * -# Create one-input, one-output, no-fee transaction: + class MempoolSpendCoinbaseTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [["-checkmempool"]] def run_test(self): chain_height = self.nodes[0].getblockcount() From 69c601c2c3aba47992f9bc6179537910cf553b94 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 13 Aug 2018 07:07:23 -0400 Subject: [PATCH 05/26] Merge #13939: lint: Make format string linter understand basic template parameter syntax 4441ad677a Make format string linter understand basic template parameter syntax (practicalswift) Pull request description: Make format string linter understand basic template parameter syntax. Fixes issue described in https://github.com/bitcoin/bitcoin/pull/13705#issuecomment-412046126. Thanks to @ken2812221 for reporting! Tree-SHA512: 8fb995bc6b29d8b9926ef5969e02cf71c494e829434fcdeb4ed5fabad5ab96e86e5b8eea705e8a416927757b4fa4e58abc0fd4f483daa58c94e2c6fdcb8ee822 --- test/lint/lint-format-strings.py | 38 +++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index 5a1adf6199..1d49992dab 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -131,6 +131,32 @@ def parse_function_call_and_arguments(function_name, function_call): ['foo(', '123', ')'] >>> parse_function_call_and_arguments("foo", 'foo("foo")') ['foo(', '"foo"', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", std::wstring_convert,wchar_t>().to_bytes(buf), err);') + ['strprintf(', '"%s (%d)",', ' std::wstring_convert,wchar_t>().to_bytes(buf),', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo().to_bytes(buf), err);') + ['strprintf(', '"%s (%d)",', ' foo().to_bytes(buf),', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo().to_bytes(buf), err);') + ['strprintf(', '"%s (%d)",', ' foo().to_bytes(buf),', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo << 1, err);') + ['strprintf(', '"%s (%d)",', ' foo << 1,', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo() >> 1, err);') + ['strprintf(', '"%s (%d)",', ' foo() >> 1,', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo < 1 ? bar : foobar, err);') + ['strprintf(', '"%s (%d)",', ' foo < 1 ? bar : foobar,', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo < 1, err);') + ['strprintf(', '"%s (%d)",', ' foo < 1,', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo > 1 ? bar : foobar, err);') + ['strprintf(', '"%s (%d)",', ' foo > 1 ? bar : foobar,', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo > 1, err);') + ['strprintf(', '"%s (%d)",', ' foo > 1,', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo <= 1, err);') + ['strprintf(', '"%s (%d)",', ' foo <= 1,', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo <= bar<1, 2>(1, 2), err);') + ['strprintf(', '"%s (%d)",', ' foo <= bar<1, 2>(1, 2),', ' err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo>foo<1,2>(1,2)?bar:foobar,err)'); + ['strprintf(', '"%s (%d)",', ' foo>foo<1,2>(1,2)?bar:foobar,', 'err', ')'] + >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo>foo<1,2>(1,2),err)'); + ['strprintf(', '"%s (%d)",', ' foo>foo<1,2>(1,2),', 'err', ')'] """ assert(type(function_name) is str and type(function_call) is str and function_name) remaining = normalize(escape(function_call)) @@ -139,9 +165,10 @@ def parse_function_call_and_arguments(function_name, function_call): parts = [expected_function_call] remaining = remaining[len(expected_function_call):] open_parentheses = 1 + open_template_arguments = 0 in_string = False parts.append("") - for char in remaining: + for i, char in enumerate(remaining): parts.append(parts.pop() + char) if char == "\"": in_string = not in_string @@ -159,6 +186,15 @@ def parse_function_call_and_arguments(function_name, function_call): parts.append(parts.pop()[:-1]) parts.append(char) break + prev_char = remaining[i - 1] if i - 1 >= 0 else None + next_char = remaining[i + 1] if i + 1 <= len(remaining) - 1 else None + if char == "<" and next_char not in [" ", "<", "="] and prev_char not in [" ", "<"]: + open_template_arguments += 1 + continue + if char == ">" and next_char not in [" ", ">", "="] and prev_char not in [" ", ">"] and open_template_arguments > 0: + open_template_arguments -= 1 + if open_template_arguments > 0: + continue if char == ",": parts.append("") return parts From 08835d37c8c27791f3215693fe40871449554410 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 13 Aug 2018 07:33:32 -0400 Subject: [PATCH 06/26] Merge #13905: docs: fixed bitcoin-cli -help output for help2man 869193f5a6 docs: fixed bitcoin-cli -help output for help2man (Hennadii Stepanov) Pull request description: Currently `bitcon-cli -help` output forces help2man to produce `.TP` and `.IP` commands instead of a single `.IP` command for `-stdinrpcpass` option. Removing an extra space fixes this issue. This pull request is rebased from #13879 Tree-SHA512: 1c5b25ed2ef7b7de42bc6210165bdbabe63f045699487f2db4790e0d3176f6493dfd3e8e19f4ddc38b551539465d7b41aea570f20dccbc0609f00fdfee1b5180 --- src/dash-cli.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dash-cli.cpp b/src/dash-cli.cpp index cf97031aa0..12960b4cd9 100644 --- a/src/dash-cli.cpp +++ b/src/dash-cli.cpp @@ -49,8 +49,8 @@ static void SetupCliArgs() gArgs.AddArg("-rpcuser=", "Username for JSON-RPC connections", false, OptionsCategory::OPTIONS); gArgs.AddArg("-rpcwait", "Wait for RPC server to start", false, OptionsCategory::OPTIONS); gArgs.AddArg("-rpcwallet=", "Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind)", false, OptionsCategory::OPTIONS); - gArgs.AddArg("-stdin", "Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.", false, OptionsCategory::OPTIONS); - gArgs.AddArg("-stdinrpcpass", strprintf("Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password."), false, OptionsCategory::OPTIONS); + gArgs.AddArg("-stdin", "Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.", false, OptionsCategory::OPTIONS); + gArgs.AddArg("-stdinrpcpass", "Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password.", false, OptionsCategory::OPTIONS); SetupChainParamsBaseOptions(); From 326592470af254d0d3eb1b132db787d7d298024f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 13 Aug 2018 09:53:06 -0400 Subject: [PATCH 07/26] Merge #13953: fix deprecation in bitcoin-util-test.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 49e56625aa fix deprecation in bitcoin-util-test.py (Isidoro Ghezzi) Pull request description: To avoid: $ make check {…omissis…} Running test/util/bitcoin-util-test.py... /usr/local/bin/python3.7 ../test/util/bitcoin-util-test.py ../test/util/bitcoin-util-test.py:31: DeprecationWarning: This method will be removed in future versions. Use 'parser.read_file()' instead. config.readfp(open(os.path.join(os.path.dirname(__file__), "../config.ini"), encoding="utf8")) $ python3 --version Python 3.7.0 Tree-SHA512: eafed629b64ae32b0b84520bb9b430204cba38d426dab1b3946a92c758c7d599aacc2798ab6e126808a6c7515ff20eb4ecc635b3e424f4c8903105438f817297 --- test/util/bitcoin-util-test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/util/bitcoin-util-test.py b/test/util/bitcoin-util-test.py index 9bba9e4194..268d1d7a41 100755 --- a/test/util/bitcoin-util-test.py +++ b/test/util/bitcoin-util-test.py @@ -28,7 +28,7 @@ import sys def main(): config = configparser.ConfigParser() config.optionxform = str - config.readfp(open(os.path.join(os.path.dirname(__file__), "../config.ini"), encoding="utf8")) + config.read_file(open(os.path.join(os.path.dirname(__file__), "../config.ini"), encoding="utf8")) env_conf = dict(config.items('environment')) parser = argparse.ArgumentParser(description=__doc__) From aa5e2c211128e847704044a99d9ec15b80241d36 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 14 Aug 2018 09:36:57 +0200 Subject: [PATCH 08/26] Merge #13963: tests: Replace usage of tostring() with tobytes() 8845c8aea65897637c330f5893461c0da180eaf8 tests: Replace usage of tostring() with tobytes() (Carl Dong) Pull request description: tostring() is deprecated as of python 3.7 and results in stderr output causing tests to fail Tree-SHA512: 8c5bbd6c6127490922add98543ee7719d19e11200e081784adef2f026ddf90d7735da7d0fb41fa4307d0d3450a27e126752c2b01cbd79b0c8a695855aed080ac --- test/functional/test_framework/netutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py index 1c7b7a1af3..195ec1c5e8 100644 --- a/test/functional/test_framework/netutil.py +++ b/test/functional/test_framework/netutil.py @@ -107,7 +107,7 @@ def all_interfaces(): max_possible *= 2 else: break - namestr = names.tostring() + namestr = names.tobytes() return [(namestr[i:i+16].split(b'\0', 1)[0], socket.inet_ntoa(namestr[i+20:i+24])) for i in range(0, outbytes, struct_size)] From 5e0feca0bd12a26b0136d9cc8d819b3cbf6971a6 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 15 Aug 2018 07:59:13 -0400 Subject: [PATCH 09/26] Merge #13974: [trivial] Fix typo in CDiskBlockPos struct's ToString 8bd98a3846 [trivial] Fix typo in CDiskBlockPos struct's ToString (Jon Layton) Pull request description: (Logging) Tree-SHA512: 5c0334fda15b1d668b251107772ae527e6b5f63d10e6c75330107eec0db7195845fdb9e92781591bcad6720bc8ef5af5a77cccf883170c4dfd2090b8c7ce16bd --- src/chain.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chain.h b/src/chain.h index 5962f335f4..3c8f5b1604 100644 --- a/src/chain.h +++ b/src/chain.h @@ -112,7 +112,7 @@ struct CDiskBlockPos std::string ToString() const { - return strprintf("CBlockDiskPos(nFile=%i, nPos=%i)", nFile, nPos); + return strprintf("CDiskBlockPos(nFile=%i, nPos=%i)", nFile, nPos); } }; From a4bf001d8d288e5300028ed3dbbb3e879686e0d5 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 21 Aug 2018 17:27:36 +0200 Subject: [PATCH 10/26] Merge #14006: Add const modifier to HTTPRequest methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 18c49eb8877d8b11f763083a79a7b8250e060106 http: Add const modifier to HTTPRequest methods (João Barbosa) Pull request description: Tree-SHA512: 233617425ff3abc7419817a95337056c190640197c6c4d8b1a0810967d960c0968d02967e16ffbc1af1a2b3117fdc98722bf05e270504d59548e6838fa7f5ffb --- src/httpserver.cpp | 8 ++++---- src/httpserver.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 65ede63eab..f856302dee 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -535,7 +535,7 @@ HTTPRequest::~HTTPRequest() // evhttpd cleans up the request, as long as a reply was sent. } -std::pair HTTPRequest::GetHeader(const std::string& hdr) +std::pair HTTPRequest::GetHeader(const std::string& hdr) const { const struct evkeyvalq* headers = evhttp_request_get_input_headers(req); assert(headers); @@ -608,7 +608,7 @@ void HTTPRequest::WriteReply(int nStatus, const std::string& strReply) req = nullptr; // transferred back to main thread } -CService HTTPRequest::GetPeer() +CService HTTPRequest::GetPeer() const { evhttp_connection* con = evhttp_request_get_connection(req); CService peer; @@ -622,12 +622,12 @@ CService HTTPRequest::GetPeer() return peer; } -std::string HTTPRequest::GetURI() +std::string HTTPRequest::GetURI() const { return evhttp_request_get_uri(req); } -HTTPRequest::RequestMethod HTTPRequest::GetRequestMethod() +HTTPRequest::RequestMethod HTTPRequest::GetRequestMethod() const { switch (evhttp_request_get_command(req)) { case EVHTTP_REQ_GET: diff --git a/src/httpserver.h b/src/httpserver.h index 50dffdb577..84b3931a48 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -74,21 +74,21 @@ public: /** Get requested URI. */ - std::string GetURI(); + std::string GetURI() const; /** Get CService (address:ip) for the origin of the http request. */ - CService GetPeer(); + CService GetPeer() const; /** Get request method. */ - RequestMethod GetRequestMethod(); + RequestMethod GetRequestMethod() const; /** * Get the request header specified by hdr, or an empty string. * Return a pair (isPresent,string). */ - std::pair GetHeader(const std::string& hdr); + std::pair GetHeader(const std::string& hdr) const; /** * Read request body. From 32b571bd13d7e4c614a66119d801ce7a95513409 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 26 Aug 2018 06:37:45 -0400 Subject: [PATCH 11/26] Merge #14063: Move cs_main locking annotations from .cpp to .h 9e2de6b9d0 Move cs_main locking annotations from .cpp to .h (practicalswift) Pull request description: Move `cs_main` locking annotations from `.cpp` to `.h`. Tree-SHA512: 591bdc408a7a04c8208530fb6992b7535f30d2473e6c33fe39920330319a40e8dfb40407f2ea2d4c6c0d4624c24e34ffbdf57271b332e82f98339792372be84e --- src/txmempool.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/txmempool.h b/src/txmempool.h index bd428e9419..f3bd17d332 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -34,6 +34,7 @@ #include class CBlockIndex; +extern CCriticalSection cs_main; /** Fake height value used in Coin to signify they are only in the memory pool (since 0.8) */ static const uint32_t MEMPOOL_HEIGHT = 0x7FFFFFFF; @@ -580,7 +581,7 @@ public: bool removeSpentIndex(const uint256 txhash); void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); - void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); + void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeProTxPubKeyConflicts(const CTransaction &tx, const CKeyID &keyId); void removeProTxPubKeyConflicts(const CTransaction &tx, const CBLSPublicKey &pubKey); From ee68324db5a9575e559e128ad3957a4ae84b8dd8 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 26 Aug 2018 10:29:29 -0400 Subject: [PATCH 12/26] Merge #14069: qa: Use assert not BOOST_CHECK_* from multithreaded tests 737670c036 Use assert when running from multithreaded code as BOOST_CHECK_* are not thread safe (Jesse Cohen) Pull request description: Resolves thread sanitizer failure @MarcoFalke found in #14058 Tree-SHA512: 24d86c2cdae21fee029ee4b06f633de4b3e655d3371d97f09db6fd3f24b29388a78110996712249c49e7fefa7bbc3d3c405d8b480382174831fe2f9a042a557e --- src/test/scheduler_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp index c055eb91ee..fddfb760a3 100644 --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -138,11 +138,11 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered) // the callbacks should run in exactly the order in which they were enqueued for (int i = 0; i < 100; ++i) { queue1.AddToProcessQueue([i, &counter1]() { - BOOST_CHECK_EQUAL(i, counter1++); + assert(i == counter1++); }); queue2.AddToProcessQueue([i, &counter2]() { - BOOST_CHECK_EQUAL(i, counter2++); + assert(i == counter2++); }); } From 6bf1fbbf091373f9bc0d5c76aa797bfb94d161aa Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 26 Aug 2018 22:04:30 -0400 Subject: [PATCH 13/26] Merge #14071: qa: Stop txindex thread before calling destructor faf4a9b674 qa: Stop txindex thread before calling destructor (MarcoFalke) Pull request description: Same as #13894, but for the tests. Tree-SHA512: a21d9f8ad8dc9703217d1808cb14bd969903c364fe30bbdc0dd2df170ddc0cbaba98b0bde28bc21ff1319222aaf6cb4f1b2c45cd6b236fe3c645a92eab6bacba --- src/test/txindex_tests.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp index aec776e904..aef8694163 100644 --- a/src/test/txindex_tests.cpp +++ b/src/test/txindex_tests.cpp @@ -61,6 +61,8 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) BOOST_ERROR("Read incorrect tx"); } } + + txindex.Stop(); // Stop thread before calling destructor } BOOST_AUTO_TEST_SUITE_END() From 70519e04951ff1ab3686ff8e3442bdf31d27a15d Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 27 Aug 2018 11:59:42 +0200 Subject: [PATCH 14/26] Merge #14031: Make IS_TRIVIALLY_CONSTRUCTIBLE consistent on GCC < 5, don't patch clang f1640d093fa682c98b000e377916cc32b2267e23 Make IS_TRIVIALLY_CONSTRUCTIBLE consistent on GCC < 5 (Ben Woosley) Pull request description: `std::is_trivially_constructible` is equivalent to `std::is_trivially_default_constructible` `std::has_trivial_default_constructor` is the GCC < 5 name for `std::is_trivially_default_constructible` https://en.cppreference.com/w/cpp/types/is_default_constructible https://www.gnu.org/software/gcc/gcc-5/changes.html `std::is_trivial` was also used when compiling with clang, due to clang's use of `__GNUC__`. Test `__clang__` to target the intended implementations. https://stackoverflow.com/a/28166605 All callers currently only pass one template argument to IS_TRIVIALLY_CONSTRUCTIBLE, with this change the build would fail if someone attempted passing more. Tree-SHA512: 3e36ddf20a1c0d76ad94d7c95f3fe5b90f4ee00389d5516b35c657136205e7a3ddff60789b0b0b2375624631f15a51eaad3570ef19a7b9df1469a50ba28415d1 --- src/compat.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compat.h b/src/compat.h index 7f62df8db9..d6bd5bfabb 100644 --- a/src/compat.h +++ b/src/compat.h @@ -14,10 +14,10 @@ // GCC 4.8 is missing some C++11 type_traits, // https://www.gnu.org/software/gcc/gcc-5/changes.html -#if defined(__GNUC__) && __GNUC__ < 5 -#define IS_TRIVIALLY_CONSTRUCTIBLE std::is_trivial +#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ < 5 +#define IS_TRIVIALLY_CONSTRUCTIBLE std::has_trivial_default_constructor #else -#define IS_TRIVIALLY_CONSTRUCTIBLE std::is_trivially_constructible +#define IS_TRIVIALLY_CONSTRUCTIBLE std::is_trivially_default_constructible #endif #ifdef WIN32 From 5773399cc380a615feebeb27037be052d9d5b134 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 27 Aug 2018 12:05:10 +0200 Subject: [PATCH 15/26] Merge #14030: Remove ambiguity in construction of prevector 497e90c02b96e8739e8faf3d43e41ba1ff0627b7 Remove default argument to prevector constructor to remove ambiguity (Ben Woosley) Pull request description: The call with this default argument is redundant with `prevector(size_type)` on line 251. Tree-SHA512: 4d22e6f4cd56e4b700596d7f5afc945ec6684636a94690fa16a1bbb34e4f53b6340f53a6c314fea213359426474125228ba7193388789f8a13308506358e92db --- src/prevector.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/prevector.h b/src/prevector.h index fbbc86fc57..e176d8d410 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -269,7 +269,7 @@ public: resize(n); } - explicit prevector(size_type n, const T& val = T()) : _size(0) { + explicit prevector(size_type n, const T& val) : _size(0) { change_capacity(n); _size += n; fill(item_ptr(0), n, val); From b619f993e7793804d72d7915f11d5b09192c46a7 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 28 Aug 2018 10:43:24 +0200 Subject: [PATCH 16/26] Merge #14051: [Tests] Make combine_logs.py handle multi-line logs 16e288acdd61fa5fa5e39f3936fb50499f82c085 test padding non micro timestamps (John Newbery) 995dd89d884bda3fb5ca1885c5887d989cd2cad3 [Tests] Make combine_logs.py handle multi-line logs (John Newbery) Pull request description: combine_logs.py currently inserts additional newlines into multi-line log messages, and doesn't color them properly. Fix both of those. Tree-SHA512: dbe2f3ecc7cfbc95ee4350e648d127538c79cb6555257d4aeec12fe3d159366742b68e90e620c8ed7219a44b973395c7e5929ba374fae115fbee25560db645f6 --- test/functional/combine_logs.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/functional/combine_logs.py b/test/functional/combine_logs.py index caa8c9c5bc..4d80399b37 100755 --- a/test/functional/combine_logs.py +++ b/test/functional/combine_logs.py @@ -14,7 +14,7 @@ import re import sys # Matches on the date format at the start of the log event -TIMESTAMP_PATTERN = re.compile(r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{6}Z") +TIMESTAMP_PATTERN = re.compile(r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d{6})?Z") LogEvent = namedtuple('LogEvent', ['timestamp', 'source', 'event']) @@ -84,11 +84,17 @@ def get_log_events(source, logfile): if time_match: if event: yield LogEvent(timestamp=timestamp, source=source, event=event.rstrip()) - event = line timestamp = time_match.group() + if time_match.group(1) is None: + # timestamp does not have microseconds. Add zeroes. + timestamp_micro = timestamp.replace("Z", ".000000Z") + line = line.replace(timestamp, timestamp_micro) + timestamp = timestamp_micro + event = line # if it doesn't have a timestamp, it's a continuation line of the previous log. else: - event += "\n" + line + # Add the line. Prefix with space equivalent to the source + timestamp so log lines are aligned + event += " " + line # Flush the final event yield LogEvent(timestamp=timestamp, source=source, event=event.rstrip()) except FileNotFoundError: @@ -107,7 +113,11 @@ def print_logs(log_events, color=False, html=False): colors["reset"] = "\033[0m" # Reset font color for event in log_events: - print("{0} {1: <5} {2} {3}".format(colors[event.source.rstrip()], event.source, event.event, colors["reset"])) + lines = event.event.splitlines() + print("{0} {1: <5} {2} {3}".format(colors[event.source.rstrip()], event.source, lines[0], colors["reset"])) + if len(lines) > 1: + for line in lines[1:]: + print("{0}{1}{2}".format(colors[event.source.rstrip()], line, colors["reset"])) else: try: From 2d91eb7b77cc3441cd5813deb9b005e89565dd94 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 29 Aug 2018 15:01:29 +0200 Subject: [PATCH 17/26] Merge #14028: Explicitly initialize prevector _union 1d9aa008d6e043c29c3c5b030a6d04278aea233b Explicitly initialize prevector _union (Ben Woosley) Pull request description: Tree-SHA512: 3037a5d63b840a4cb0c3c26593ce1b7e1a6ba273a4ee5072563b20169be9783dbdfe3a38c9651d73b2d18ed9668deaf65f994eca7f225c70f875716f05eda3a6 --- src/prevector.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/prevector.h b/src/prevector.h index e176d8d410..1f00576030 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -265,32 +265,32 @@ public: prevector() : _size(0), _union{{}} {} - explicit prevector(size_type n) : _size(0) { + explicit prevector(size_type n) : prevector() { resize(n); } - explicit prevector(size_type n, const T& val) : _size(0) { + explicit prevector(size_type n, const T& val) : prevector() { change_capacity(n); _size += n; fill(item_ptr(0), n, val); } template - prevector(InputIterator first, InputIterator last) : _size(0) { + prevector(InputIterator first, InputIterator last) : prevector() { size_type n = last - first; change_capacity(n); _size += n; fill(item_ptr(0), first, last); } - prevector(const prevector& other) : _size(0) { + prevector(const prevector& other) : prevector() { size_type n = other.size(); change_capacity(n); _size += n; fill(item_ptr(0), other.begin(), other.end()); } - prevector(prevector&& other) : _size(0) { + prevector(prevector&& other) : prevector() { swap(other); } From 12adb05b1262491ebb2063f8ab427eefc85ab407 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 29 Aug 2018 09:33:14 -0400 Subject: [PATCH 18/26] Merge #14020: Add tests for RPC help MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 6af6d9b23d test: Add tests for RPC help (João Barbosa) Pull request description: At the moment the new test checks for: - invalid usages - expected output for unknown command - current RPC command titles (derived from command categories) — this prevents adding wrong RPC categories and new categories must be added to the test Tree-SHA512: f987535d001b1cd300656588602b1634099ea68a1dd2282180c30fa56caf7f990be9e2dc86c7431dfcf7fd686d0299a8d4935df178a2c9f0fb6fbebcba748eb5 --- test/functional/rpc_help.py | 31 +++++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 32 insertions(+) create mode 100755 test/functional/rpc_help.py diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py new file mode 100755 index 0000000000..e878ded258 --- /dev/null +++ b/test/functional/rpc_help.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 +# Copyright (c) 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 RPC help output.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, assert_raises_rpc_error + +class HelpRpcTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def run_test(self): + node = self.nodes[0] + + # wrong argument count + assert_raises_rpc_error(-1, 'help', node.help, 'foo', 'bar') + + # invalid argument + assert_raises_rpc_error(-1, 'JSON value is not a string as expected', node.help, 0) + + # help of unknown command + assert_equal(node.help('foo'), 'help: unknown command: foo') + + # command titles + titles = [line[3:-3] for line in node.help().splitlines() if line.startswith('==')] + assert_equal(titles, ['Blockchain', 'Control', 'Generating', 'Mining', 'Network', 'Rawtransactions', 'Util', 'Wallet', 'Zmq']) + +if __name__ == '__main__': + HelpRpcTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index d957149474..c14eda35ca 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -190,6 +190,7 @@ BASE_SCRIPTS = [ 'p2p_node_network_limited.py', 'feature_blocksdir.py', 'feature_config_args.py', + 'rpc_help.py', 'feature_help.py', # Don't append tests at the end to avoid merge conflicts # Put them in a random line within the section that fits their approximate run-time From 2de80ac210cafc85f597909f65f2988d9db9677a Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 31 Aug 2018 13:05:10 +0200 Subject: [PATCH 19/26] Merge #14037: Add README.md to linux release tarballs 8550f1fb28292a081aab3f49ba2fb561710c4572 Add README.md to linux release tarballs (Hennadii Stepanov) Pull request description: fix #8160 Gitian building report for 0.17.0rc2: ``` 7d89d7dc3488915ec2380253a69fb3b8f8065592e24c5b2a99a91da30f2142cc bitcoin-0.17.0-aarch64-linux-gnu-debug.tar.gz fcb292fd2c4fca88e5cc5a97ee7fa3390d3c7221aada166fe7822d64a2ee9dfa bitcoin-0.17.0-aarch64-linux-gnu.tar.gz 0ec6f979a823a6b6084d2e80605dffd3ccdda359e8459cebec25092c1087348f bitcoin-0.17.0-arm-linux-gnueabihf-debug.tar.gz 45af8757a2315125afe2f4d4f276d9b9cf616b8ab814284ce2f82b9a345971d8 bitcoin-0.17.0-arm-linux-gnueabihf.tar.gz b37b6d9bda864af968dfab6eebb245e75ecc56eb18b139b946270933381ea288 bitcoin-0.17.0-i686-pc-linux-gnu-debug.tar.gz 20c96a5509eeb3e8ec505f18914ef9231beef1fec5e9e1c4b33ec6c6b613d146 bitcoin-0.17.0-i686-pc-linux-gnu.tar.gz d505888594a04dab2b34ccd6863b8f25eb97d9cb76650e39d93f4d6c09d4c55a bitcoin-0.17.0-x86_64-linux-gnu-debug.tar.gz f55b16716c3295e309c816e170911380a5a26e9be3a336b213f2f412f0b159b3 bitcoin-0.17.0-x86_64-linux-gnu.tar.gz 01c6b5ce15b9f3fcdcce96baae14eb04ab2605f2294d333e96b66e004594eea6 src/bitcoin-0.17.0.tar.gz ``` Release tarball content: ``` $ tar -tf bitcoin-binaries/0.17.0rc2/bitcoin-0.17.0-x86_64-linux-gnu.tar.gz bitcoin-0.17.0/ bitcoin-0.17.0/bin/ bitcoin-0.17.0/bin/bitcoin-cli bitcoin-0.17.0/bin/bitcoind bitcoin-0.17.0/bin/bitcoin-qt bitcoin-0.17.0/bin/bitcoin-tx bitcoin-0.17.0/bin/test_bitcoin bitcoin-0.17.0/include/ bitcoin-0.17.0/include/bitcoinconsensus.h bitcoin-0.17.0/lib/ bitcoin-0.17.0/lib/libbitcoinconsensus.so bitcoin-0.17.0/lib/libbitcoinconsensus.so.0 bitcoin-0.17.0/lib/libbitcoinconsensus.so.0.0.0 bitcoin-0.17.0/README.md bitcoin-0.17.0/share/ bitcoin-0.17.0/share/man/ bitcoin-0.17.0/share/man/man1/ bitcoin-0.17.0/share/man/man1/bitcoin-cli.1 bitcoin-0.17.0/share/man/man1/bitcoind.1 bitcoin-0.17.0/share/man/man1/bitcoin-qt.1 bitcoin-0.17.0/share/man/man1/bitcoin-tx.1 ``` Tree-SHA512: 2a0c069d6533502a95a83eaba57b9828bddd03ab4a4fc47027b0068c9f04837f107abc448d82c929aa1f45441d2459cf6f2ad74b97a4d953f66dc81031bd521a --- contrib/gitian-descriptors/gitian-linux.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/gitian-descriptors/gitian-linux.yml b/contrib/gitian-descriptors/gitian-linux.yml index ff158a227a..9ab3bc0221 100755 --- a/contrib/gitian-descriptors/gitian-linux.yml +++ b/contrib/gitian-descriptors/gitian-linux.yml @@ -213,6 +213,7 @@ script: | rm -rf ${DISTNAME}/lib/pkgconfig find ${DISTNAME}/bin -type f -executable -exec ../contrib/devtools/split-debug.sh {} {} {}.dbg \; find ${DISTNAME}/lib -type f -exec ../contrib/devtools/split-debug.sh {} {} {}.dbg \; + cp ../doc/README.md ${DISTNAME}/ find ${DISTNAME} -not -name "*.dbg" | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz find ${DISTNAME} -name "*.dbg" | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}-debug.tar.gz cd ../../ From cd5a39c8af7821899e875fdf810c4c1cb319d583 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 31 Aug 2018 14:59:36 +0200 Subject: [PATCH 20/26] Merge #14088: tests: Don't assert(...) with side effects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ca1a093127c11bb2aea10bf96c38dbfb40f8d170 Add regression test: Don't assert(...) with side effects (practicalswift) 4c3c9c38699360f93d3c52a01a90ff15ee5e1a62 Don't assert(...) with side effects (practicalswift) Pull request description: Don't `assert(...)` with side effects. From the developer notes: > **Assertions should not have side-effects** > > Rationale: Even though the source code is set to refuse to compile with assertions disabled, having side-effects in assertions is unexpected and makes the code harder to understand These assertions were introduced quite recently (in #14069 which was merged two days ago) and since this is a recurring thing (see #13534 – "Don't assert(foo()) where foo() has side effects" from May) I added a simple regression test for the most obvious common side effect. Tree-SHA512: be65db9d8d5d0f5752152ba73fe3fbb0531880f156d3cd7dfdf1752709979b63214e46ae64b1adbe1e09fa121278f4087f4ae49bff16cf8f5aec16ea6bde3650 --- src/test/scheduler_tests.cpp | 6 ++++-- test/lint/lint-assertions.sh | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) create mode 100755 test/lint/lint-assertions.sh diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp index fddfb760a3..8dac13d8fd 100644 --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -138,11 +138,13 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered) // the callbacks should run in exactly the order in which they were enqueued for (int i = 0; i < 100; ++i) { queue1.AddToProcessQueue([i, &counter1]() { - assert(i == counter1++); + bool expectation = i == counter1++; + assert(expectation); }); queue2.AddToProcessQueue([i, &counter2]() { - assert(i == counter2++); + bool expectation = i == counter2++; + assert(expectation); }); } diff --git a/test/lint/lint-assertions.sh b/test/lint/lint-assertions.sh new file mode 100755 index 0000000000..5bbcae79eb --- /dev/null +++ b/test/lint/lint-assertions.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# +# Copyright (c) 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. +# +# Check for assertions with obvious side effects. + +export LC_ALL=C + +EXIT_CODE=0 + +# PRE31-C (SEI CERT C Coding Standard): +# "Assertions should not contain assignments, increment, or decrement operators." +OUTPUT=$(git grep -E '[^_]assert\(.*(\+\+|\-\-|[^=!<>]=[^=!<>]).*\);' -- "*.cpp" "*.h") +if [[ ${OUTPUT} != "" ]]; then + echo "Assertions should not have side effects:" + echo + echo "${OUTPUT}" + EXIT_CODE=1 +fi + +exit ${EXIT_CODE} From 7c120a7124468497a9c5288c6e1e92872331ac59 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 31 Aug 2018 09:03:00 -0400 Subject: [PATCH 21/26] Merge #10605: Add AssertLockHeld assertions in CWallet::ListCoins 62b6f0f21e Add EXCLUSIVE_LOCKS_REQUIRED to CWallet::ListCoins (Russell Yanofsky) 545e85eccc Add AssertLockHeld assertions in CWallet::ListCoins (Russell Yanofsky) Pull request description: Fixes TODO from #10295 Tree-SHA512: 2dd03a8217e5e1313aa2119cb530e0c0daf3ae3a751b6fdec611df57b8090201a90b52ff05f8f696e978a1344aaf21989d67a03beb5ef6ef79b77be38d04b451 --- src/wallet/test/wallet_tests.cpp | 16 +++++++++++++--- src/wallet/wallet.cpp | 12 ++---------- src/wallet/wallet.h | 2 +- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 5481a63645..50d3d3b607 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -324,7 +324,11 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) // Confirm ListCoins initially returns 1 coin grouped under coinbaseKey // address. - auto list = wallet->ListCoins(); + std::map> list; + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 1U); @@ -337,7 +341,10 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) // coinbaseKey pubkey, even though the change address has a different // pubkey. AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */}); - list = wallet->ListCoins(); + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U); @@ -363,7 +370,10 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) } // Confirm ListCoins still returns same result as before, despite coins // being locked. - list = wallet->ListCoins(); + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f40122ebd5..2b0162904e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3000,20 +3000,12 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const std::map> CWallet::ListCoins() const { - // TODO: Add AssertLockHeld(cs_wallet) here. - // - // Because the return value from this function contains pointers to - // CWalletTx objects, callers to this function really should acquire the - // cs_wallet lock before calling it. However, the current caller doesn't - // acquire this lock yet. There was an attempt to add the missing lock in - // https://github.com/bitcoin/bitcoin/pull/10340, but that change has been - // postponed until after https://github.com/bitcoin/bitcoin/pull/10244 to - // avoid adding some extra complexity to the Qt code. + AssertLockHeld(cs_main); + AssertLockHeld(cs_wallet); std::map> result; std::vector availableCoins; - LOCK2(cs_main, cs_wallet); AvailableCoins(availableCoins); for (auto& coin : availableCoins) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d85743f80c..3731d80675 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -877,7 +877,7 @@ public: /** * Return list of available coins and locked coins grouped by non-change output address. */ - std::map> ListCoins() const; + std::map> ListCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_main, cs_wallet); /** * Find non-change parent output. From 1a8f904a0b7f208b4779f98683cab8462c61b91d Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 4 Sep 2018 12:27:46 +0200 Subject: [PATCH 22/26] Merge #14135: doc: correct GetDifficulty doc after #13288 68bfc0bce349954b2e0fb82aed2f47e213fff9e4 doc: correct GetDifficulty doc after #13288 (fanquake) Pull request description: `chain` is no longer passed to GetDifficulty, and we just return `1.0` if no `blockindex`. Tree-SHA512: 701375d732f343200c4abfaf9039d5c12b10abff97b022e84564f81b26b5ba552f1eb0c0d0fd5370b29b53319eafcf39773a36e1c2dd04ee77e61c18c7b183fa --- src/rpc/blockchain.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index 3aa8de2d2b..4d3fe5534a 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -10,8 +10,7 @@ class CBlockIndex; class UniValue; /** - * Get the difficulty of the net wrt to the given block index, or the chain tip if - * not provided. + * Get the difficulty of the net wrt to the given block index. * * @return A floating point number that is a multiple of the main net minimum * difficulty (4295032833 hashes). From fa7ef879ac449fb06348c942b009b565b9bd1f2a Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 4 Sep 2018 12:29:13 +0200 Subject: [PATCH 23/26] Merge #14129: Trivial: update clang thread-safety docs url c7f7fa467ec571d3b3fd7ae1d32429b74b18ad9c Trivial: update clang thread-safety docs url (Ben Woosley) Pull request description: From the defunct http://clang.llvm.org/docs/LanguageExtensions.html#threadsafety to https://clang.llvm.org/docs/ThreadSafetyAnalysis.html Tree-SHA512: 5113c3933fccee7b45cace5d8dffa38b46ed9ad1422795d57843a20b276ed0e513bbf8d3d2bd28f55a46baf14a349871d7a635485785b93cb21baf8b8720c15a --- src/threadsafety.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/threadsafety.h b/src/threadsafety.h index b0ab1f315d..f52d5a0367 100644 --- a/src/threadsafety.h +++ b/src/threadsafety.h @@ -12,7 +12,7 @@ // TL;DR Add GUARDED_BY(mutex) to member variables. The others are // rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo); // -// See http://clang.llvm.org/docs/LanguageExtensions.html#threadsafety +// See https://clang.llvm.org/docs/ThreadSafetyAnalysis.html // for documentation. The clang compiler can do advanced static analysis // of locking when given the -Wthread-safety option. #define LOCKABLE __attribute__((lockable)) From f9a16ff05d1155d5bbcb3e5ae6e471399b8a867d Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 28 Jun 2021 21:32:37 -0500 Subject: [PATCH 24/26] prevent clean / accidental merging of bitcoin#13782 Signed-off-by: pasta --- contrib/gitian-descriptors/gitian-win-signer.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/gitian-descriptors/gitian-win-signer.yml b/contrib/gitian-descriptors/gitian-win-signer.yml index f068c6464a..e3afda64b8 100644 --- a/contrib/gitian-descriptors/gitian-win-signer.yml +++ b/contrib/gitian-descriptors/gitian-win-signer.yml @@ -5,7 +5,7 @@ suites: architectures: - "amd64" packages: -- "libssl-dev" +- "libssl-dev" # do not merge bitcoin#13782, see https://github.com/dashpay/dash/pull/3894 - "autoconf" - "automake" - "libtool" From e01d766fd1332a5a71cb3983372620834ce52bc9 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 1 Jul 2021 10:55:35 -0500 Subject: [PATCH 25/26] fix macos compilation by including an extern to cs_main Signed-off-by: pasta --- src/wallet/wallet.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3731d80675..cdbf09914f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -83,6 +83,8 @@ class CWalletTx; struct FeeCalculation; enum class FeeEstimateMode; +extern CCriticalSection cs_main; + /** (client) version numbers for particular wallet features */ enum WalletFeature { From 6fae0326657f20c4b0ef51b25ca46c51a2cd4255 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 1 Jul 2021 11:02:07 -0500 Subject: [PATCH 26/26] fix rpc_help.py Signed-off-by: pasta --- test/functional/rpc_help.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py index e878ded258..b122abc2cf 100755 --- a/test/functional/rpc_help.py +++ b/test/functional/rpc_help.py @@ -14,8 +14,8 @@ class HelpRpcTest(BitcoinTestFramework): def run_test(self): node = self.nodes[0] - # wrong argument count - assert_raises_rpc_error(-1, 'help', node.help, 'foo', 'bar') + # wrong argument count, note: Dash's help allows for two options since we utilize subcommands + assert_raises_rpc_error(-1, 'help', node.help, 'foo', 'bar', 'foobar') # invalid argument assert_raises_rpc_error(-1, 'JSON value is not a string as expected', node.help, 0) @@ -25,7 +25,7 @@ class HelpRpcTest(BitcoinTestFramework): # command titles titles = [line[3:-3] for line in node.help().splitlines() if line.startswith('==')] - assert_equal(titles, ['Blockchain', 'Control', 'Generating', 'Mining', 'Network', 'Rawtransactions', 'Util', 'Wallet', 'Zmq']) + assert_equal(titles, ['Addressindex', 'Blockchain', 'Control', 'Dash', 'Evo', 'Generating', 'Mining', 'Network', 'Rawtransactions', 'Util', 'Wallet', 'Zmq']) if __name__ == '__main__': HelpRpcTest().main()