From 2a5616edcb4b85dc3aa97d78c38793e9fdb5310b Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 24 Aug 2023 16:26:03 -0400 Subject: [PATCH 1/5] Merge bitcoin/bitcoin#27480: doc: Improve documentation of rpcallowip c8e066461b54d745b85411035fcc00a1a4044d76 doc: Improve documentation of rpcallowip rpchelp (willcl-ark) Pull request description: Closes #21070 v21.0 introduced a behaviour changed noted in #21070 where using a config value `rpcallowip=::0` no longer also permitted ipv4 ip addresses. The rpc_bind.py functional test covers this new behaviour already by checking that the list of bind addresses exactly matches what is expected so this commit only updates the documentation. ACKs for top commit: achow101: ACK c8e066461b54d745b85411035fcc00a1a4044d76 pinheadmz: ACK c8e066461b54d745b85411035fcc00a1a4044d76 jonatack: ACK c8e066461b54d745b85411035fcc00a1a4044d76 Tree-SHA512: 332060cf0df0427c6637a9fd1e0783ce0b0940abdb41b0df13f03bfbdc28af067cec8f0b1bbc4e47b3d54fa1b2f110418442b05b39d5e7c7e0b96744ddd7c003 --- src/init.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index 4b2d2af40d238..d3f5006795756 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -745,7 +745,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-blockversion=", "Override block version to test forking scenarios", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION); argsman.AddArg("-rest", strprintf("Accept public REST requests (default: %u)", DEFAULT_REST_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); - argsman.AddArg("-rpcallowip=", "Allow JSON-RPC connections from specified source. Valid for are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); + argsman.AddArg("-rpcallowip=", "Allow JSON-RPC connections from specified source. Valid values for are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), all ipv4 (0.0.0.0/0), or all ipv6 (::/0). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcauth=", "Username and HMAC-SHA-256 hashed password for JSON-RPC connections. The field comes in the format: :$. A canonical python script is included in share/rpcuser. The client then connects normally using the rpcuser=/rpcpassword= pair of arguments. This option can be specified multiple times", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC); argsman.AddArg("-rpcbind=[:port]", "Bind to given address to listen for JSON-RPC connections. Do not expose the RPC server to untrusted networks such as the public internet! This option is ignored unless -rpcallowip is also passed. Port is optional and overrides -rpcport. Use [host]:port notation for IPv6. This option can be specified multiple times (default: 127.0.0.1 and ::1 i.e., localhost, or if -rpcallowip has been specified, 0.0.0.0 and :: i.e., all addresses)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY | ArgsManager::SENSITIVE, OptionsCategory::RPC); argsman.AddArg("-rpccookiefile=", "Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); From 9a390dd93f077055e1dcd48b6b0b2a7ebd795db4 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 16 May 2023 10:05:42 +0100 Subject: [PATCH 2/5] Merge bitcoin/bitcoin#27664: docs: fix spelling errors e9dcac1ec7b4e00a08a943caa250c4ff00981b8b add `lief` to `spelling.ignore-words` (brunoerg) 258f93000b0522368a315100526ea8e59d0280cd test: fix spelling in `interface_usdt_utxocache` (brunoerg) Pull request description: Add `lief` to `spelling.ignore-words` since it's the name of a Python lib and fix spelling error in `interface_usdt_utxocache` (s/eariler/earlier) ACKs for top commit: fanquake: ACK e9dcac1ec7b4e00a08a943caa250c4ff00981b8b Tree-SHA512: f42cdbb74144c5d289d70bb9bac1179650bb32594678e15f4e18e4b2f68009999d60ff69494d4e68650d82fc1838e67515ed2f922ee84db98735ef906911ec40 --- test/functional/interface_usdt_utxocache.py | 2 +- test/lint/spelling.ignore-words.txt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py index 13e8d165c3ddc..3f28cde34b6a9 100755 --- a/test/functional/interface_usdt_utxocache.py +++ b/test/functional/interface_usdt_utxocache.py @@ -364,7 +364,7 @@ def handle_utxocache_flush(_, data, __): bpf["utxocache_flush"].open_perf_buffer(handle_utxocache_flush) self.log.info("stop the node to flush the UTXO cache") - UTXOS_IN_CACHE = 104 # might need to be changed if the eariler tests are modified + UTXOS_IN_CACHE = 104 # might need to be changed if the earlier tests are modified # A node shutdown causes two flushes. One that flushes UTXOS_IN_CACHE # UTXOs and one that flushes 0 UTXOs. Normally the 0-UTXO-flush is the # second flush, however it can happen that the order changes. diff --git a/test/lint/spelling.ignore-words.txt b/test/lint/spelling.ignore-words.txt index e3e672893277f..036c53ef44e4b 100644 --- a/test/lint/spelling.ignore-words.txt +++ b/test/lint/spelling.ignore-words.txt @@ -10,6 +10,7 @@ hights hist inout invokable +lief mor nin ser From 0ad41d17f319dffebaaf233c06c0000a6fe90a72 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 27 Jun 2023 15:33:13 -0400 Subject: [PATCH 3/5] Merge bitcoin/bitcoin#27334: util: implement `noexcept` move assignment & move ctor for `prevector` bfb9291a8661fe5b26c14ed755cfa89d27c37110 util: implement prevector's move ctor & move assignment (Martin Leitner-Ankerl) fffc86f49f4eeb811b8438bc1b7f8d9e05882c6f test: CScriptCheck is used a lot in std::vector, make sure that's efficient (Martin Leitner-Ankerl) 81f67977f543faca2dcc35846f73e2917375ae79 util: prevector's move ctor and move assignment is `noexcept` (Martin Leitner-Ankerl) d380d2877ed45cf1e75a87d822b30e4e1e21e3d4 bench: Add benchmark for prevector usage in std::vector (Martin Leitner-Ankerl) Pull request description: `prevector`'s move assignment and move constructor were not `noexcept`, which makes it inefficient to use inside STL containers like `std::vector`. That's the case e.g. for `CScriptCheck`. This PR adds `noexcept`, and also implements the move assignment & ctor, which makes it quite a bit more efficient to use prevector in an std::vector. The PR also adds a benchmark which grows an `std::vector` by adding `prevector` objects to it. merge-base: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,440.29 | 155,272.42 | 0.2% | 40,713.01 | 20,473.84 | 1.989 | 7,132.01 | 0.2% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 3,213.19 | 311,217.35 | 0.7% | 35,373.01 | 10,214.07 | 3.463 | 6,945.00 | 0.2% | 0.43 | `PrevectorFillVectorDirectTrivial` | 34,749.70 | 28,777.23 | 0.1% | 364,396.05 | 110,521.94 | 3.297 | 78,568.37 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 32,535.05 | 30,736.09 | 0.4% | 353,823.31 | 103,464.53 | 3.420 | 79,871.80 | 0.2% | 0.40 | `PrevectorFillVectorIndirectTrivial` util: prevector's move ctor and move assignment is `noexcept`: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,603.87 | 151,426.40 | 0.2% | 23,734.01 | 21,009.63 | 1.130 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,980.93 | 504,813.15 | 0.1% | 13,784.00 | 6,304.32 | 2.186 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 19,110.54 | 52,327.15 | 0.1% | 139,816.41 | 51,987.72 | 2.689 | 28,512.18 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 12,334.37 | 81,074.27 | 0.7% | 125,655.12 | 39,253.58 | 3.201 | 27,854.46 | 0.2% | 0.44 | `PrevectorFillVectorIndirectTrivial` util: implement prevector's move ctor & move assignment | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 5,262.66 | 190,018.01 | 0.2% | 20,157.01 | 16,745.26 | 1.204 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,687.07 | 592,744.35 | 0.2% | 12,742.00 | 5,368.02 | 2.374 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 17,930.80 | 55,769.95 | 0.1% | 136,237.69 | 47,903.31 | 2.844 | 28,512.02 | 0.2% | 0.42 | `PrevectorFillVectorIndirectNontrivial` | 11,893.75 | 84,077.78 | 0.2% | 126,182.02 | 37,852.91 | 3.333 | 28,152.01 | 0.1% | 0.44 | `PrevectorFillVectorIndirectTrivial` As can be seen, mostly thanks to just `noexcept` the benchmark becomes about 2 times faster because `std::vector` can now use move operations instead of having to fall back to copy everything I had a look at how this change affects the other benchmarks, and they are all pretty much the same, the only noticable difference is `CCheckQueueSpeedPrevectorJob` goes from 364.56ns down to 346.21ns. ACKs for top commit: achow101: ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 jonatack: > Tested Approach ACK [bfb9291](https://github.com/bitcoin/bitcoin/commit/bfb9291a8661fe5b26c14ed755cfa89d27c37110), ~ACK modulo re-verifying the implementation of the last commit. john-moffett: Approach ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 theStack: Tested and light code-review ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 Tree-SHA512: 242995d7cb2f8ebfa73177aa690a505f189d91edeb8cea3e34a41ca507c8cb17c65fe2a4e196fdafc5c6e89b2b2222627bfc9f5c316517de0857b7b5e9c58225 --- src/bench/prevector.cpp | 26 ++++++++++++++++++++++++++ src/prevector.h | 15 +++++++++++---- src/validation.h | 6 ++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/bench/prevector.cpp b/src/bench/prevector.cpp index 46accbae847d1..3b2682001fd8a 100644 --- a/src/bench/prevector.cpp +++ b/src/bench/prevector.cpp @@ -107,6 +107,30 @@ static void PrevectorAssignTo(benchmark::Bench& bench) }); } +template +static void PrevectorFillVectorDirect(benchmark::Bench& bench) +{ + bench.run([&] { + std::vector> vec; + for (size_t i = 0; i < 260; ++i) { + vec.emplace_back(); + } + }); +} + + +template +static void PrevectorFillVectorIndirect(benchmark::Bench& bench) +{ + bench.run([&] { + std::vector> vec; + for (size_t i = 0; i < 260; ++i) { + // force allocation + vec.emplace_back(29, T{}); + } + }); +} + #define PREVECTOR_TEST(name) \ static void Prevector##name##Nontrivial(benchmark::Bench& bench) \ { \ @@ -126,3 +150,5 @@ PREVECTOR_TEST(Deserialize) BENCHMARK(PrevectorAssign) BENCHMARK(PrevectorAssignTo) +PREVECTOR_TEST(FillVectorDirect) +PREVECTOR_TEST(FillVectorIndirect) diff --git a/src/prevector.h b/src/prevector.h index 81dc49978bba5..90495ca1a27d4 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -286,8 +286,10 @@ class prevector { fill(item_ptr(0), other.begin(), other.end()); } - prevector(prevector&& other) { - swap(other); + prevector(prevector&& other) noexcept + : _union(std::move(other._union)), _size(other._size) + { + other._size = 0; } prevector& operator=(const prevector& other) { @@ -298,8 +300,13 @@ class prevector { return *this; } - prevector& operator=(prevector&& other) { - swap(other); + prevector& operator=(prevector&& other) noexcept { + if (!is_direct()) { + free(_union.indirect_contents.indirect); + } + _union = std::move(other._union); + _size = other._size; + other._size = 0; return *this; } diff --git a/src/validation.h b/src/validation.h index 7edf390eccb9b..ab2a1841c1f16 100644 --- a/src/validation.h +++ b/src/validation.h @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -337,6 +338,11 @@ class CScriptCheck ScriptError GetScriptError() const { return error; } }; +// CScriptCheck is used a lot in std::vector, make sure that's efficient +static_assert(std::is_nothrow_move_assignable_v); +static_assert(std::is_nothrow_move_constructible_v); +static_assert(std::is_nothrow_destructible_v); + /** Initializes the script-execution cache */ void InitScriptExecutionCache(); From 3b3fdaadc44e32ff30e738212eb59c5efbd1952b Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 25 Jul 2023 18:44:15 -0400 Subject: [PATCH 4/5] Merge bitcoin/bitcoin#27930: util: Don't derive secure_allocator from std::allocator 07c59eda00841aafaafd8fd648217b56b1e907c9 Don't derive secure_allocator from std::allocator (Casey Carter) Pull request description: Giving the C++ Standard Committee control of the public interface of your type means they will break it. C++23 adds a new `allocate_at_least` member to `std::allocator`. Very bad things happen when, say, `std::vector` uses `allocate_at_least` from `secure_allocator`'s base to allocate memory which it then tries to free with `secure_allocator::deallocate`. (Discovered by microsoft/STL#3712, which will be reverted by microsoft/STL#3819 before it ships.) ACKs for top commit: jonatack: re-ACK 07c59eda00841aafaafd8fd648217b56b1e907c9 no change since my previous ACK apart from squashing the commits achow101: ACK 07c59eda00841aafaafd8fd648217b56b1e907c9 john-moffett: ACK 07c59eda00841aafaafd8fd648217b56b1e907c9 Reviewed and tested. Performance appears unaffected in my environment. Tree-SHA512: 23606c40414d325f5605a9244d4dd50907fdf5f2fbf70f336accb3a2cb98baa8acd2972f46eab1b7fdec1d28a843a96b06083cd2d09791cda7c90ee218e5bbd5 --- src/support/allocators/secure.h | 36 +++++++++++------------- src/support/allocators/zeroafterfree.h | 39 +++++++++++++++----------- 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/src/support/allocators/secure.h b/src/support/allocators/secure.h index 537ca2bb7874a..14676dbe4d8e7 100644 --- a/src/support/allocators/secure.h +++ b/src/support/allocators/secure.h @@ -18,27 +18,14 @@ // out of memory and clears its contents before deletion. // template -struct secure_allocator : public std::allocator { - using base = std::allocator; - using traits = std::allocator_traits; - using size_type = typename traits::size_type; - using difference_type = typename traits::difference_type; - using pointer = typename traits::pointer; - using const_pointer = typename traits::const_pointer; - using value_type = typename traits::value_type; - secure_allocator() noexcept {} - secure_allocator(const secure_allocator& a) noexcept : base(a) {} +struct secure_allocator { + using value_type = T; + + secure_allocator() = default; template - secure_allocator(const secure_allocator& a) noexcept : base(a) - { - } - ~secure_allocator() noexcept {} - template - struct rebind { - typedef secure_allocator<_Other> other; - }; + secure_allocator(const secure_allocator&) noexcept {} - T* allocate(std::size_t n, const void* hint = nullptr) + T* allocate(std::size_t n) { T* allocation = static_cast(LockedPoolManager::Instance().alloc(sizeof(T) * n)); if (!allocation) { @@ -54,6 +41,17 @@ struct secure_allocator : public std::allocator { } LockedPoolManager::Instance().free(p); } + + template + friend bool operator==(const secure_allocator&, const secure_allocator&) noexcept + { + return true; + } + template + friend bool operator!=(const secure_allocator&, const secure_allocator&) noexcept + { + return false; + } }; // This is exactly like std::string, but with a custom allocator. diff --git a/src/support/allocators/zeroafterfree.h b/src/support/allocators/zeroafterfree.h index 0befe0ffcdfe4..725ec11696da0 100644 --- a/src/support/allocators/zeroafterfree.h +++ b/src/support/allocators/zeroafterfree.h @@ -12,31 +12,36 @@ #include template -struct zero_after_free_allocator : public std::allocator { - using base = std::allocator; - using traits = std::allocator_traits; - using size_type = typename traits::size_type; - using difference_type = typename traits::difference_type; - using pointer = typename traits::pointer; - using const_pointer = typename traits::const_pointer; - using value_type = typename traits::value_type; - zero_after_free_allocator() noexcept {} - zero_after_free_allocator(const zero_after_free_allocator& a) noexcept : base(a) {} +struct zero_after_free_allocator { + using value_type = T; + + zero_after_free_allocator() noexcept = default; template - zero_after_free_allocator(const zero_after_free_allocator& a) noexcept : base(a) + zero_after_free_allocator(const zero_after_free_allocator&) noexcept + { + } + + T* allocate(std::size_t n) { + return std::allocator{}.allocate(n); } - ~zero_after_free_allocator() noexcept {} - template - struct rebind { - typedef zero_after_free_allocator<_Other> other; - }; void deallocate(T* p, std::size_t n) { if (p != nullptr) memory_cleanse(p, sizeof(T) * n); - std::allocator::deallocate(p, n); + std::allocator{}.deallocate(p, n); + } + + template + friend bool operator==(const zero_after_free_allocator&, const zero_after_free_allocator&) noexcept + { + return true; + } + template + friend bool operator!=(const zero_after_free_allocator&, const zero_after_free_allocator&) noexcept + { + return false; } }; From c781f1ad39708ae4b865300d57002a85a174fb7c Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 22 Jun 2023 15:43:18 +0100 Subject: [PATCH 5/5] (Partial) Merge bitcoin/bitcoin#27831: test: handle failed `assert_equal()` assertions in bcc callback functions 61f4b9b7ad6e992a9dbbbb091e9b7ba9abe529ac Manage exceptions in bcc callback functions (virtu) Pull request description: Address #27380 (and similar future issues) by handling failed `assert_equal()` assertions in bcc callback functions ### Problem Exceptions are not propagated in ctype callback functions used by bcc. This means an AssertionError exception raised by `assert_equal()` to signal a failed assertion is not getting caught and properly logged. Instead, the error is logged to stdout and execution of the callback stops. The current workaround to check whether all `assert_equal()` assertions in a callback succeeded is to increment a success counter after the assertions (which only gets incremented if none exception is raised and stops execution). Then, outside the callback, the success counter can be used to check whether a callback executed successfully. One issue with the described workaround is that when an exception occurs, there is no way of telling which of the `assert_equal()` statements caused the exception; moreover, there is no way of inspecting how the pieces of data that got compared in `assert_equal()` differed (often a crucial clue when debugging what went wrong). This problem is happening in #27380: Sporadically, in the `mempool:rejected` test, execution does not reach the end of the callback function and the success counter is not incremented. Thus, the test fails when comparing the counter to its expected value of one. Without knowing which of the asserts failed any why it failed, this issue is hard to debug. ### Solution Two fixes come to mind. The first involves having the callback function make event data accessible outside the callback and inspecting the event using `assert_equal()` outside the callback. This solution still requires a counter in the callback in order to tell whether a callback was actually executed or if instead the call to perf_buffer_poll() timed out. The second fix entails wrapping all relevant `assert_equal()` statements inside callback functions into try-catch blocks and manually logging AssertionErrors. While not as elegant in terms of design, this approach can be more pragmatic for more complex tests (e.g., ones involving multiple events, events of different types, or the order of events). The solution proposed here is to select the most pragmatic fix on a case-by-case basis: Tests in `interface_usdt_net.py`, `interface_usdt_mempool.py` and `interface_usdt_validation.py` have been refactored to use the first approach, while the second approach was chosen for `interface_usdt_utxocache.py` (partly to provide a reference for the second approach, but mainly because the utxocache tests are the most intricate tests, and refactoring them to use the first approach would negatively impact their readability). Lastly, `interface_usdt_coinselection.py` was kept unchanged because it does not use `assert_equal()` statements inside callback functions. ACKs for top commit: 0xB10C: Reviewed the changes since my last review. ACK 61f4b9b7ad6e992a9dbbbb091e9b7ba9abe529ac. I've tested that the combined log contains both exceptions by modifying `interface_usdt_utxocache.py`. willcl-ark: utACK 61f4b9b stickies-v: utACK 61f4b9b7a Tree-SHA512: 85cdaabf370d4f09a9eab6af9ce7c796cd9d08cb91f38f021f71adda34c5f643331022dd09cadb95be2185dad6016c95cbb8942e41e4fbd566a49bf431c5141a --- test/functional/interface_usdt_net.py | 15 +++--- test/functional/interface_usdt_utxocache.py | 49 ++++++++++++-------- test/functional/interface_usdt_validation.py | 23 ++++++--- 3 files changed, 54 insertions(+), 33 deletions(-) diff --git a/test/functional/interface_usdt_net.py b/test/functional/interface_usdt_net.py index 0debc7bdb1ca4..1c21bbfc219fa 100755 --- a/test/functional/interface_usdt_net.py +++ b/test/functional/interface_usdt_net.py @@ -116,13 +116,10 @@ def __repr__(self): fn_name="trace_outbound_message") bpf = BPF(text=net_tracepoints_program, usdt_contexts=[ctx], debug=0) - # The handle_* function is a ctypes callback function called from C. When - # we assert in the handle_* function, the AssertError doesn't propagate - # back to Python. The exception is ignored. We manually count and assert - # that the handle_* functions succeeded. EXPECTED_INOUTBOUND_VERSION_MSG = 1 checked_inbound_version_msg = 0 checked_outbound_version_msg = 0 + events = [] def check_p2p_message(event, inbound): nonlocal checked_inbound_version_msg, checked_outbound_version_msg @@ -142,12 +139,13 @@ def check_p2p_message(event, inbound): checked_outbound_version_msg += 1 def handle_inbound(_, data, __): + nonlocal events event = ctypes.cast(data, ctypes.POINTER(P2PMessage)).contents - check_p2p_message(event, True) + events.append((event, True)) def handle_outbound(_, data, __): event = ctypes.cast(data, ctypes.POINTER(P2PMessage)).contents - check_p2p_message(event, False) + events.append((event, False)) bpf["inbound_messages"].open_perf_buffer(handle_inbound) bpf["outbound_messages"].open_perf_buffer(handle_outbound) @@ -158,12 +156,15 @@ def handle_outbound(_, data, __): bpf.perf_buffer_poll(timeout=200) self.log.info( - "check that we got both an inbound and outbound version message") + "check receipt and content of in- and outbound version messages") + for event, inbound in events: + check_p2p_message(event, inbound) assert_equal(EXPECTED_INOUTBOUND_VERSION_MSG, checked_inbound_version_msg) assert_equal(EXPECTED_INOUTBOUND_VERSION_MSG, checked_outbound_version_msg) + bpf.cleanup() diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py index 3f28cde34b6a9..e757875cb9dfc 100755 --- a/test/functional/interface_usdt_utxocache.py +++ b/test/functional/interface_usdt_utxocache.py @@ -190,13 +190,16 @@ def handle_utxocache_uncache(_, data, __): nonlocal handle_uncache_succeeds event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents self.log.info(f"handle_utxocache_uncache(): {event}") - assert_equal(block_1_coinbase_txid, bytes(event.txid[::-1]).hex()) - assert_equal(0, event.index) # prevout index - assert_equal(EARLY_BLOCK_HEIGHT, event.height) - assert_equal(500 * COIN, event.value) - assert_equal(True, event.is_coinbase) - - handle_uncache_succeeds += 1 + try: + assert_equal(block_1_coinbase_txid, bytes(event.txid[::-1]).hex()) + assert_equal(0, event.index) # prevout index + assert_equal(EARLY_BLOCK_HEIGHT, event.height) + assert_equal(50 * COIN, event.value) + assert_equal(True, event.is_coinbase) + except AssertionError: + self.log.exception("Assertion failed") + else: + handle_uncache_succeeds += 1 bpf["utxocache_uncache"].open_perf_buffer(handle_utxocache_uncache) @@ -262,24 +265,32 @@ def handle_utxocache_add(_, data, __): event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents self.log.info(f"handle_utxocache_add(): {event}") add = expected_utxocache_adds.pop(0) - assert_equal(add["txid"], bytes(event.txid[::-1]).hex()) - assert_equal(add["index"], event.index) - assert_equal(add["height"], event.height) - assert_equal(add["value"], event.value) - assert_equal(add["is_coinbase"], event.is_coinbase) - handle_add_succeeds += 1 + try: + assert_equal(add["txid"], bytes(event.txid[::-1]).hex()) + assert_equal(add["index"], event.index) + assert_equal(add["height"], event.height) + assert_equal(add["value"], event.value) + assert_equal(add["is_coinbase"], event.is_coinbase) + except AssertionError: + self.log.exception("Assertion failed") + else: + handle_add_succeeds += 1 def handle_utxocache_spent(_, data, __): nonlocal handle_spent_succeeds event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents self.log.info(f"handle_utxocache_spent(): {event}") spent = expected_utxocache_spents.pop(0) - assert_equal(spent["txid"], bytes(event.txid[::-1]).hex()) - assert_equal(spent["index"], event.index) - assert_equal(spent["height"], event.height) - assert_equal(spent["value"], event.value) - assert_equal(spent["is_coinbase"], event.is_coinbase) - handle_spent_succeeds += 1 + try: + assert_equal(spent["txid"], bytes(event.txid[::-1]).hex()) + assert_equal(spent["index"], event.index) + assert_equal(spent["height"], event.height) + assert_equal(spent["value"], event.value) + assert_equal(spent["is_coinbase"], event.is_coinbase) + except AssertionError: + self.log.exception("Assertion failed") + else: + handle_spent_succeeds += 1 bpf["utxocache_add"].open_perf_buffer(handle_utxocache_add) bpf["utxocache_spent"].open_perf_buffer(handle_utxocache_spent) diff --git a/test/functional/interface_usdt_validation.py b/test/functional/interface_usdt_validation.py index 71e997fdca401..f28923bb3a970 100755 --- a/test/functional/interface_usdt_validation.py +++ b/test/functional/interface_usdt_validation.py @@ -85,13 +85,10 @@ def __repr__(self): self.sigops, self.duration) - # The handle_* function is a ctypes callback function called from C. When - # we assert in the handle_* function, the AssertError doesn't propagate - # back to Python. The exception is ignored. We manually count and assert - # that the handle_* functions succeeded. BLOCKS_EXPECTED = 2 blocks_checked = 0 expected_blocks = dict() + events = [] self.log.info("hook into the validation:block_connected tracepoint") ctx = USDT(path=str(self.options.bitcoind)) @@ -101,7 +98,7 @@ def __repr__(self): usdt_contexts=[ctx], debug=0) def handle_blockconnected(_, data, __): - nonlocal expected_blocks, blocks_checked + nonlocal events, blocks_checked event = ctypes.cast(data, ctypes.POINTER(Block)).contents self.log.info(f"handle_blockconnected(): {event}") block_hash = bytes(event.hash[::-1]).hex() @@ -126,12 +123,24 @@ def handle_blockconnected(_, data, __): expected_blocks[block_hash] = self.nodes[0].getblock(block_hash, 2) bpf.perf_buffer_poll(timeout=200) - bpf.cleanup() - self.log.info(f"check that we traced {BLOCKS_EXPECTED} blocks") + self.log.info(f"check that we correctly traced {BLOCKS_EXPECTED} blocks") + for event in events: + block_hash = bytes(event.hash[::-1]).hex() + block = expected_blocks[block_hash] + assert_equal(block["hash"], block_hash) + assert_equal(block["height"], event.height) + assert_equal(len(block["tx"]), event.transactions) + assert_equal(len([tx["vin"] for tx in block["tx"]]), event.inputs) + assert_equal(0, event.sigops) # no sigops in coinbase tx + # only plausibility checks + assert event.duration > 0 + del expected_blocks[block_hash] assert_equal(BLOCKS_EXPECTED, blocks_checked) assert_equal(0, len(expected_blocks)) + bpf.cleanup() + if __name__ == '__main__': ValidationTracepointTest().main()