From 959092990016e9d0dbb18d524eee8b9191e5d87b Mon Sep 17 00:00:00 2001 From: MacroFake Date: Tue, 12 Jul 2022 17:28:23 +0200 Subject: [PATCH 01/11] Merge bitcoin/bitcoin#24944: rpc: add getblockfrompeer RPCTypeCheck and invalid input test coverage 2ef5294a5bb68ceb3797d2638567a172cc21699f rpc: add RPCTypeCheck for getblockfrompeer inputs (Jon Atack) 734b9669ff7b2f5e2820993443a6f868f6b0b20a test: add getblockfrompeer coverage of invalid inputs (Jon Atack) Pull request description: The new getblockfrompeer RPC lacks test coverage for invalid arguments, and its error messages are not harmonized with the existing RPCs. Fix all issues. ACKs for top commit: brunoerg: ACK 2ef5294a5bb68ceb3797d2638567a172cc21699f Tree-SHA512: 454782cf6a44fd0e05483bb152153667ef5c8021358385ddcf89724fbbbd35e187362bdff757e00c99319527bc4c0b20c7187f67241d4585d767a29787142f25 --- src/rpc/blockchain.cpp | 5 +++++ test/functional/rpc_getblockfrompeer.py | 9 ++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index fc1d8367ea..59a73c7dd3 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -790,6 +790,11 @@ static RPCHelpMan getblockfrompeer() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + RPCTypeCheck(request.params, { + UniValue::VSTR, // blockhash + UniValue::VNUM, // peer_id + }); + const NodeContext& node = EnsureAnyNodeContext(request.context); ChainstateManager& chainman = EnsureChainman(node); PeerManager& peerman = EnsurePeerman(node); diff --git a/test/functional/rpc_getblockfrompeer.py b/test/functional/rpc_getblockfrompeer.py index 71ed87293e..2988e0d967 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -49,14 +49,17 @@ class GetBlockFromPeerTest(BitcoinTestFramework): assert_equal(len(peers), 1) peer_0_peer_1_id = peers[0]["id"] - self.log.info("Arguments must be sensible") - assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", 0) + self.log.info("Arguments must be valid") + assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", peer_0_peer_1_id) + assert_raises_rpc_error(-3, "Expected type string, got number", self.nodes[0].getblockfrompeer, 1234, peer_0_peer_1_id) + assert_raises_rpc_error(-3, "Expected type number, got string", self.nodes[0].getblockfrompeer, short_tip, "0") self.log.info("We must already have the header") assert_raises_rpc_error(-1, "Block header missing", self.nodes[0].getblockfrompeer, "00" * 32, 0) self.log.info("Non-existent peer generates error") - assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_0_peer_1_id + 1) + for peer_id in [-1, peer_0_peer_1_id + 1]: + assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_id) self.log.info("Successful fetch") result = self.nodes[0].getblockfrompeer(short_tip, peer_0_peer_1_id) From df2f533aaf6db2f2054e2d7e9e811c5b3d077ba5 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 28 Dec 2022 18:02:44 +0100 Subject: [PATCH 02/11] Merge bitcoin/bitcoin#26759: test: Drop no longer needed `race:epoll_ctl` TSan suppression a3f5e541523a843e834df1858e16f89188fe19a2 test: Drop no longer needed `race:epoll_ctl` TSan suppression (Hennadii Stepanov) Pull request description: The removed suppression seems no needed. I cannot point the exact commit/PR which makes this change possible. Top commit has no ACKs. Tree-SHA512: 8ee79cbdb2bc62796d72c69be4a818379132eae47be33951e8b9d224b049ff77e867004801c7cb0cc564a5374f318dafd9142b5231e9bd428f80acc75253933e --- test/sanitizer_suppressions/tsan | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index 2cf4851c6c..a472190187 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -44,8 +44,5 @@ race:libzmq # https://github.com/bitcoin/bitcoin/issues/20618 race:CZMQAbstractPublishNotifier::SendZmqMessage -# https://github.com/bitcoin/bitcoin/pull/20218, https://github.com/bitcoin/bitcoin/pull/20745 -race:epoll_ctl - # https://github.com/bitcoin/bitcoin/issues/23366 race:std::__1::ios_base::* From 7f2b9340899a53739a45ee9b66438e5a0321ca63 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 4 Jan 2023 10:19:57 +0000 Subject: [PATCH 03/11] Merge bitcoin/bitcoin#26772: contrib: fix sha256 check in install_db4.sh for FreeBSD 22e9afe40d987f4f90bc8469f9475df138fe6261 use sha256 command instead of sha256sum on FreeBSD (Murray Nesbitt) Pull request description: The FreeBSD version of `sha256sum` takes different arguments than the GNU version. The `sha256_check` function in `contrib/install_db4.sh` has code specific to FreeBSD, however it doesn't get reached because while the `sha256sum` command does exist on FreeBSD, it is incompatible and results in an error: ``` sha256sum: option requires an argument -- c usage: sha256sum [-pqrtx] [-c file] [-s string] [files ...] ``` This change moves the FreeBSD-specific code before the check for the `sha256sum` command. Fixes: #26774 Top commit has no ACKs. Tree-SHA512: 2485e2e7d8fdca3b072b29fb22bbdfd69e520740537b331b33c64cc645b63da712cfa63a23bdf039bbc92a6558fc7bf03323a51784bf601ff360ff0ef59506c8 --- contrib/install_db4.sh | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/contrib/install_db4.sh b/contrib/install_db4.sh index 560eaa00ed..0f6f9d8d51 100755 --- a/contrib/install_db4.sh +++ b/contrib/install_db4.sh @@ -32,16 +32,15 @@ check_exists() { sha256_check() { # Args: # - if check_exists sha256sum; then - echo "${1} ${2}" | sha256sum -c + if [ "$(uname)" = "FreeBSD" ]; then + # sha256sum exists on FreeBSD, but takes different arguments than the GNU version + sha256 -c "${1}" "${2}" + elif check_exists sha256sum; then + echo "${1} ${2}" | sha256sum -c elif check_exists sha256; then - if [ "$(uname)" = "FreeBSD" ]; then - sha256 -c "${1}" "${2}" - else - echo "${1} ${2}" | sha256 -c - fi + echo "${1} ${2}" | sha256 -c else - echo "${1} ${2}" | shasum -a 256 -c + echo "${1} ${2}" | shasum -a 256 -c fi } From 092ddc3a3e1d55bf957596f63a9e0dbd9ca3ce21 Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 4 Jan 2023 18:01:52 +0000 Subject: [PATCH 04/11] Merge bitcoin/bitcoin#26603: doc: CalculateSequenceLocks: prevHeights entries are set to 0, not removed f537127271b1f22ee4651915b7b9266e0df72841 doc: fix: prevHeights entries are set to 0, not removed (stickies-v) Pull request description: In [`CalculateSequenceLocks`](https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.h#L69) no items are removed from `prevHeights`, they are just set to 0: https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.cpp#L69-L73 This PR updates the docs to reflect the actual implementation. Seems to have been wrongly documented since introduction in #7184 already ([implementation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R742-R749) and [documentation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R712-R713)) ACKs for top commit: hebasto: ACK f537127271b1f22ee4651915b7b9266e0df72841 Tree-SHA512: 3661501660f6832b2116fd83466ffe95a60b341c14cb09a37489e2a587bea3290b0528690120a0f644c3eea02177aa1fb8968258482fa43b0303e016abb17418 --- src/consensus/tx_verify.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h index 87ac66e46d..199f8c94a7 100644 --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -63,8 +63,8 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime); /** * Calculates the block height and previous block's median time past at * which the transaction will be considered final in the context of BIP 68. - * Also removes from the vector of input heights any entries which did not - * correspond to sequence locked inputs as they do not affect the calculation. + * For each input that is not sequence locked, the corresponding entries in + * prevHeights are set to 0 as they do not affect the calculation. */ std::pair CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector& prevHeights, const CBlockIndex& block); From 864d02e4a96fb4e33f8e719354214d6c1ba21114 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 4 Jan 2023 17:25:24 -0500 Subject: [PATCH 05/11] Merge bitcoin/bitcoin#26809: compat: use STDIN_FILENO over 0 585c6722128537f772043ef4c87238e283669b8a compat: use STDIN_FILENO over 0 (fanquake) Pull request description: This is already used throughout this file, and is self-documenting. ACKs for top commit: john-moffett: ACK 585c6722128537f772043ef4c87238e283669b8a achow101: ACK 585c6722128537f772043ef4c87238e283669b8a hebasto: ACK 585c6722128537f772043ef4c87238e283669b8a, I have reviewed the code and it looks OK, I agree it can be merged. kristapsk: utACK 585c6722128537f772043ef4c87238e283669b8a aureleoules: ACK 585c6722128537f772043ef4c87238e283669b8a Tree-SHA512: c0114ae896ba5404be70b804ee9f454d213f1d789c8f5a578c422dd15a308a214e6851fee76c0ec736a212bc86fb33ec17af1b22e5d23422c375ca4458251356 --- src/compat/stdin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compat/stdin.cpp b/src/compat/stdin.cpp index 0fc4e0fcf2..b6975a4cd3 100644 --- a/src/compat/stdin.cpp +++ b/src/compat/stdin.cpp @@ -62,7 +62,7 @@ bool StdinReady() return false; #else struct pollfd fds; - fds.fd = 0; /* this is STDIN */ + fds.fd = STDIN_FILENO; fds.events = POLLIN; return poll(&fds, 1, 0) == 1; #endif From d1b93c78b7f49a69190dc5e00afad8313ce0eaf0 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 5 Jan 2023 17:22:24 +0100 Subject: [PATCH 06/11] Merge bitcoin/bitcoin#26818: test: Fix feature_startupnotify intermittent issue fac810bb0a524e79014882f8fc694efade35de9f test: Fix feature_startupnotify intermittent issue (MarcoFalke) Pull request description: Might fix #25644 ACKs for top commit: aureleoules: ACK fac810bb0a524e79014882f8fc694efade35de9f brunoerg: ACK fac810bb0a524e79014882f8fc694efade35de9f Tree-SHA512: 870bf65da8120b6897d02e3bb70eea018d4761396abe64c3533bbc5237e65be9f77d35f62cd5d08cf7132dd53b504bf58229c33e18833c191495ad229c84d7c2 --- test/functional/feature_startupnotify.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_startupnotify.py b/test/functional/feature_startupnotify.py index c6aa837768..19b8130748 100755 --- a/test/functional/feature_startupnotify.py +++ b/test/functional/feature_startupnotify.py @@ -29,9 +29,14 @@ class StartupNotifyTest(BitcoinTestFramework): self.wait_until(lambda: os.path.exists(tmpdir_file)) self.log.info("Test -startupnotify is executed once") - with open(tmpdir_file, "r", encoding="utf8") as f: - file_content = f.read() - assert_equal(file_content.count(FILE_NAME), 1) + + def get_count(): + with open(tmpdir_file, "r", encoding="utf8") as f: + file_content = f.read() + return file_content.count(FILE_NAME) + + self.wait_until(lambda: get_count() > 0) + assert_equal(get_count(), 1) self.log.info("Test node is fully started") assert_equal(self.nodes[0].getblockcount(), 200) From 6fe46fc02af626375e735655c4f1cf06a4ab40fb Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 10 Jan 2023 17:12:35 +0100 Subject: [PATCH 07/11] Merge bitcoin/bitcoin#26864: doc: net: fix link to onion address encoding scheme [ONIONADDRESS] 3076f1815d64f448aa9dff6e48e07004f42ac0fc doc: net: fix link to onion address encoding scheme [ONIONADDRESS] (Sebastian Falbesoner) Pull request description: Instead of referring to a fixed line number to a file in master (which is obviously always quickly outdated), use a permalink tied to the latest commit. ACKs for top commit: vasild: ACK 3076f1815d64f448aa9dff6e48e07004f42ac0fc Tree-SHA512: 7070a7e47d683b1539f33daa4c67093a87d6121a84430a3b24afee139a7f2b3cab32fcdf0bc561f8e177b5ba864a98e4491e08dac90cdd4bd2e4e6b8fa7e4b14 --- src/netaddress.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 3fb668df6d..c4b639df3e 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -184,7 +184,7 @@ bool CNetAddr::SetInternal(const std::string &name) } namespace torv3 { -// https://gitweb.torproject.org/torspec.git/tree/rend-spec-v3.txt#n2135 +// https://gitweb.torproject.org/torspec.git/tree/rend-spec-v3.txt?id=7116c9cdaba248aae07a3f1d0e15d9dd102f62c5#n2175 static constexpr size_t CHECKSUM_LEN = 2; static const unsigned char VERSION[] = {3}; static constexpr size_t TOTAL_LEN = ADDR_TORV3_SIZE + CHECKSUM_LEN + sizeof(VERSION); From 07f4c39c44ab1c16dd978ba9ce4f86fefe6905a2 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 11 Jan 2023 14:55:33 +0100 Subject: [PATCH 08/11] Merge bitcoin/bitcoin#26730: test: add coverage for `purpose` arg in `listlabels` c467cfffcebb30f829eeb8160166a6b941d97ed6 test: add coverage for `purpose` arg in `listlabels` (brunoerg) Pull request description: This PR adds test coverage for `listlabels` command when specifying the `purpose` (send and receive). https://github.com/bitcoin/bitcoin/blob/dcdfd72861c09a7945b9facc3726177a2d06fa64/src/wallet/rpc/addresses.cpp#L698-L704 ACKs for top commit: kristapsk: ACK c467cfffcebb30f829eeb8160166a6b941d97ed6 Tree-SHA512: 7e7143c1264692f7b22952e7c70dbe9ed3f5dcd2e3b69962a47be9f9c21b3f4a9089ca87962fbc8ff9116e7d2dbeb7f36d6a132c9ac13724a255cfe1b32373a8 --- test/functional/wallet_labels.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py index 5275abe80b..f57a16d9c3 100755 --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -20,7 +20,7 @@ from test_framework.wallet_util import test_address class WalletLabelsTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 1 + self.num_nodes = 2 def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -82,8 +82,14 @@ class WalletLabelsTest(BitcoinTestFramework): label.add_receive_address(address) label.verify(node) + # Check listlabels when passing 'purpose' + node2_addr = self.nodes[1].getnewaddress() + node.setlabel(node2_addr, "node2_addr") + assert_equal(node.listlabels(purpose="send"), ["node2_addr"]) + assert_equal(node.listlabels(purpose="receive"), sorted(['coinbase'] + [label.name for label in labels])) + # Check all labels are returned by listlabels. - assert_equal(node.listlabels(), sorted(['coinbase'] + [label.name for label in labels])) + assert_equal(node.listlabels(), sorted(['coinbase'] + [label.name for label in labels] + ["node2_addr"])) # Send a transaction to each label. for label in labels: From 93c4652a0578cb2cf0363410617392a405b534d4 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 11 Jan 2023 16:39:08 -0500 Subject: [PATCH 09/11] Merge bitcoin/bitcoin#26821: refactor: Make `ThreadHTTP` return void 45553e11c965db218733f9ad32ecde391b393443 refactor: Make `ThreadHTTP` return void (Hennadii Stepanov) Pull request description: The `bool` return value was introduced in 755aa05174e06effd758eeb78c5af9fb465e9611 (https://github.com/bitcoin/bitcoin/pull/8421). It has been not used since 8d3f46ec3938e2ba17654fecacd1d2629f9915fd (https://github.com/bitcoin/bitcoin/pull/14670). No behavior change. ACKs for top commit: achow101: ACK 45553e11c965db218733f9ad32ecde391b393443 brunoerg: crACK 45553e11c965db218733f9ad32ecde391b393443 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26821/commits/45553e11c965db218733f9ad32ecde391b393443 stickies-v: ACK 45553e11c Tree-SHA512: 1593a5740e729967fbe1363235cd5b77ecf431b29bc740a89a6c70fc838ad97a2e4a2cd7cd63aa482f7c50bc2ffabc8cd53e8f64d6032603cb3b662229bc3dc2 --- src/httpserver.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index a7dc2cfaa7..1d11955d38 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -315,14 +315,13 @@ static void http_reject_request_cb(struct evhttp_request* req, void*) } /** Event dispatcher thread */ -static bool ThreadHTTP(struct event_base* base) +static void ThreadHTTP(struct event_base* base) { util::ThreadRename("http"); LogPrint(BCLog::HTTP, "Entering http event loop\n"); event_base_dispatch(base); // Event loop will be interrupted by InterruptHTTPServer() LogPrint(BCLog::HTTP, "Exited http event loop\n"); - return event_base_got_break(base) == 0; } /** Bind HTTP server to specified addresses */ From b6bde7322c5a80795f2d3f86ee68c382247d70e7 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 12 Jan 2023 16:49:25 +0100 Subject: [PATCH 10/11] Merge bitcoin/bitcoin#26827: doc: use "std lib clock" over "C++11 clock" 672f7ad747ecc6e04472f96fa88332be1f39d39b doc: remove usages of C++11 (fanquake) Pull request description: These were new in C++11, and now they are just our standard library. ACKs for top commit: jarolrod: re-ACK 672f7ad747ecc6e04472f96fa88332be1f39d39b hebasto: re-ACK 672f7ad747ecc6e04472f96fa88332be1f39d39b Tree-SHA512: 7e3b8b0346ba29b19e6d8536700ca510e2b543cdeecd9e740bba71ea6d0133dd96cdaeaa00f371f8ef85913ff5aaabe12878255f393dac7d354a8b89b58d050a --- .github/ISSUE_TEMPLATE/good_first_issue.md | 2 +- src/random.cpp | 4 ++-- src/random.h | 2 +- src/randomenv.cpp | 2 +- src/reverse_iterator.h | 2 +- src/test/random_tests.cpp | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/good_first_issue.md b/.github/ISSUE_TEMPLATE/good_first_issue.md index 952e5b3c35..42d7cbbbed 100644 --- a/.github/ISSUE_TEMPLATE/good_first_issue.md +++ b/.github/ISSUE_TEMPLATE/good_first_issue.md @@ -15,7 +15,7 @@ assignees: '' #### Useful skills: - + #### Want to work on this issue? diff --git a/src/random.cpp b/src/random.cpp index ef4bfce2c0..614ddeb11c 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -64,7 +64,7 @@ static inline int64_t GetPerformanceCounter() noexcept __asm__ volatile ("rdtsc" : "=a"(r1), "=d"(r2)); // Constrain r1 to rax and r2 to rdx. return (r2 << 32) | r1; #else - // Fall back to using C++11 clock (usually microsecond or nanosecond precision) + // Fall back to using standard library clock (usually microsecond or nanosecond precision) return std::chrono::high_resolution_clock::now().time_since_epoch().count(); #endif } @@ -444,7 +444,7 @@ public: RNGState& GetRNGState() noexcept { - // This C++11 idiom relies on the guarantee that static variable are initialized + // This idiom relies on the guarantee that static variable are initialized // on first call, even when multiple parallel calls are permitted. static std::vector> g_rng(1); return g_rng[0]; diff --git a/src/random.h b/src/random.h index cae1ea6ce9..d461318e6c 100644 --- a/src/random.h +++ b/src/random.h @@ -232,7 +232,7 @@ public: /* interval [0..0] */ Dur{0}; }; - // Compatibility with the C++11 UniformRandomBitGenerator concept + // Compatibility with the UniformRandomBitGenerator concept typedef uint64_t result_type; static constexpr uint64_t min() { return 0; } static constexpr uint64_t max() { return std::numeric_limits::max(); } diff --git a/src/randomenv.cpp b/src/randomenv.cpp index bf23ea4a12..05aa0c2d2c 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -251,7 +251,7 @@ void RandAddDynamicEnv(CSHA512& hasher) gettimeofday(&tv, nullptr); hasher << tv; #endif - // Probably redundant, but also use all the clocks C++11 provides: + // Probably redundant, but also use all the standard library clocks: hasher << std::chrono::system_clock::now().time_since_epoch().count(); hasher << std::chrono::steady_clock::now().time_since_epoch().count(); hasher << std::chrono::high_resolution_clock::now().time_since_epoch().count(); diff --git a/src/reverse_iterator.h b/src/reverse_iterator.h index 729d8c11cc..4db001c04b 100644 --- a/src/reverse_iterator.h +++ b/src/reverse_iterator.h @@ -4,7 +4,7 @@ #define BITCOIN_REVERSE_ITERATOR_H /** - * Template used for reverse iteration in C++11 range-based for loops. + * Template used for reverse iteration in range-based for loops. * * std::vector v = {1, 2, 3, 4, 5}; * for (auto x : reverse_iterate(v)) diff --git a/src/test/random_tests.cpp b/src/test/random_tests.cpp index 258a3eafef..bdb6b9650e 100644 --- a/src/test/random_tests.cpp +++ b/src/test/random_tests.cpp @@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE(fastrandom_randbits) } } -/** Does-it-compile test for compatibility with standard C++11 RNG interface. */ +/** Does-it-compile test for compatibility with standard library RNG interface. */ BOOST_AUTO_TEST_CASE(stdrandom_test) { FastRandomContext ctx; From 0dd997c4e565e81d20de0a04354a3fd361b4ec78 Mon Sep 17 00:00:00 2001 From: fanquake Date: Sat, 14 Jan 2023 12:18:23 +0000 Subject: [PATCH 11/11] Merge bitcoin/bitcoin#26835: contrib: add PE Canary check to security-check 6ba17d4955b75b4f4064a817dd55427f25b194bf scripts: add PE Canary check to security-check (fanquake) Pull request description: We should be checking this, same as ELF & MACHO. Guix Build: ```bash 6334c001b276ca5f0278092be68bf6d49d9b755bcac893bbd4aa58df57356e40 guix-build-6ba17d4955b7/output/dist-archive/bitcoin-6ba17d4955b7.tar.gz e27ad7fffb377bc6264477933859ab47c7283a68fbf86124d3801bc4c8b790dd guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/SHA256SUMS.part ef7b61bd854f0d3c39f356ef85ac18d37c5740874111f5ce46f7ce3381e714ca guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/bitcoin-6ba17d4955b7-win64-debug.zip c419324597487f248143a076d6eb2a56b0dbf5ce690ca89afaaee5c6b352e1a1 guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/bitcoin-6ba17d4955b7-win64-setup-unsigned.exe a18ff1e3026cd9fc08dd7b500c06a343462aef4a37538608d940d1845bcdb94a guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/bitcoin-6ba17d4955b7-win64-unsigned.tar.gz 7e4ee0669940f4b8c1a12dab836898511a60f06a62057ac03beaca8bb693bfb4 guix-build-6ba17d4955b7/output/x86_64-w64-mingw32/bitcoin-6ba17d4955b7-win64.zip ``` ACKs for top commit: sipsorcery: ACK 6ba17d4955b75b4f4064a817dd55427f25b194bf. Tree-SHA512: 1acc24c0cb36dbc30311f4eee64e3d4737c828b97039be0f72cfe061bcb8c4d5c830d7792f503e711e219a62d85b7e07cdff3510cbd4f8d46895a7cb66b88219 --- contrib/devtools/security-check.py | 7 +++++++ contrib/devtools/test-security-check.py | 16 ++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py index e0ea46c3bc..16ef3a0c7b 100755 --- a/contrib/devtools/security-check.py +++ b/contrib/devtools/security-check.py @@ -146,6 +146,12 @@ def check_PE_control_flow(binary) -> bool: return True return False +def check_PE_Canary(binary) -> bool: + ''' + Check for use of stack canary + ''' + return binary.has_symbol('__stack_chk_fail') + def check_MACHO_NOUNDEFS(binary) -> bool: ''' Check for no undefined references. @@ -203,6 +209,7 @@ BASE_PE = [ ('NX', check_NX), ('RELOC_SECTION', check_PE_RELOC_SECTION), ('CONTROL_FLOW', check_PE_control_flow), + ('Canary', check_PE_Canary), ] BASE_MACHO = [ diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index fc8dd92057..9f45365c3b 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -94,19 +94,19 @@ class TestSecurityChecks(unittest.TestCase): cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc') write_testcode(source) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']), - (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fno-stack-protector']), + (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION CONTROL_FLOW Canary')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA RELOC_SECTION CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW')) # -pie -fPIE does nothing unless --dynamicbase is also supplied - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), (1, executable+': failed HIGH_ENTROPY_VA CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), (1, executable+': failed CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full','-fstack-protector-all', '-lssp']), (0, '')) clean_files(source, executable)