From afc0224cdbe73e326addf5fb98a3e95d941f2104 Mon Sep 17 00:00:00 2001
From: stickies-v <stickies-v@protonmail.com>
Date: Fri, 23 Jun 2023 11:02:59 +0100
Subject: test: refactor: remove unnecessary blocks_checked counter

Since we already store all the blocks in `events`, keeping an
additional counter is redundant.
---
 test/functional/interface_usdt_validation.py | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/test/functional/interface_usdt_validation.py b/test/functional/interface_usdt_validation.py
index f9d9b525cd..f32d83a50c 100755
--- a/test/functional/interface_usdt_validation.py
+++ b/test/functional/interface_usdt_validation.py
@@ -86,7 +86,6 @@ class ValidationTracepointTest(BitcoinTestFramework):
                     self.duration)
 
         BLOCKS_EXPECTED = 2
-        blocks_checked = 0
         expected_blocks = dict()
         events = []
 
@@ -98,11 +97,10 @@ class ValidationTracepointTest(BitcoinTestFramework):
                   usdt_contexts=[ctx], debug=0)
 
         def handle_blockconnected(_, data, __):
-            nonlocal events, blocks_checked
+            nonlocal events
             event = ctypes.cast(data, ctypes.POINTER(Block)).contents
             self.log.info(f"handle_blockconnected(): {event}")
             events.append(event)
-            blocks_checked += 1
 
         bpf["block_connected"].open_perf_buffer(
             handle_blockconnected)
@@ -127,7 +125,7 @@ class ValidationTracepointTest(BitcoinTestFramework):
             # only plausibility checks
             assert event.duration > 0
             del expected_blocks[block_hash]
-        assert_equal(BLOCKS_EXPECTED, blocks_checked)
+        assert_equal(BLOCKS_EXPECTED, len(events))
         assert_equal(0, len(expected_blocks))
 
         bpf.cleanup()
-- 
cgit v1.2.3


From ad90ba36bd930f00753643cd1fe0af72d1c828c2 Mon Sep 17 00:00:00 2001
From: stickies-v <stickies-v@protonmail.com>
Date: Fri, 23 Jun 2023 11:05:18 +0100
Subject: test: refactor:  rename inbound to is_inbound

Makes it easier to recognize this variable represents a flag.
---
 test/functional/interface_usdt_net.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/functional/interface_usdt_net.py b/test/functional/interface_usdt_net.py
index d1f94637c9..921af71aa9 100755
--- a/test/functional/interface_usdt_net.py
+++ b/test/functional/interface_usdt_net.py
@@ -121,11 +121,11 @@ class NetTracepointTest(BitcoinTestFramework):
         checked_outbound_version_msg = 0
         events = []
 
-        def check_p2p_message(event, inbound):
+        def check_p2p_message(event, is_inbound):
             nonlocal checked_inbound_version_msg, checked_outbound_version_msg
             if event.msg_type.decode("utf-8") == "version":
                 self.log.info(
-                    f"check_p2p_message(): {'inbound' if inbound else 'outbound'} {event}")
+                    f"check_p2p_message(): {'inbound' if is_inbound else 'outbound'} {event}")
                 peer = self.nodes[0].getpeerinfo()[0]
                 msg = msg_version()
                 msg.deserialize(BytesIO(bytes(event.msg[:event.msg_size])))
@@ -133,7 +133,7 @@ class NetTracepointTest(BitcoinTestFramework):
                 assert_equal(peer["addr"], event.peer_addr.decode("utf-8"))
                 assert_equal(peer["connection_type"],
                              event.peer_conn_type.decode("utf-8"))
-                if inbound:
+                if is_inbound:
                     checked_inbound_version_msg += 1
                 else:
                     checked_outbound_version_msg += 1
@@ -157,8 +157,8 @@ class NetTracepointTest(BitcoinTestFramework):
 
         self.log.info(
             "check receipt and content of in- and outbound version messages")
-        for event, inbound in events:
-            check_p2p_message(event, inbound)
+        for event, is_inbound in events:
+            check_p2p_message(event, is_inbound)
         assert_equal(EXPECTED_INOUTBOUND_VERSION_MSG,
                      checked_inbound_version_msg)
         assert_equal(EXPECTED_INOUTBOUND_VERSION_MSG,
-- 
cgit v1.2.3


From f1b99ac94fb77340c4d3a5b4bbc3df28009bc773 Mon Sep 17 00:00:00 2001
From: stickies-v <stickies-v@protonmail.com>
Date: Fri, 23 Jun 2023 11:32:10 +0100
Subject: test: refactor: deduplicate handle_utxocache_* logic

Carve out the comparison logic into a helper function to avoid
code duplication.
---
 test/functional/interface_usdt_utxocache.py | 36 +++++++++++++----------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py
index 5f2ba49026..7988ff2364 100755
--- a/test/functional/interface_usdt_utxocache.py
+++ b/test/functional/interface_usdt_utxocache.py
@@ -258,37 +258,33 @@ class UTXOCacheTracepointTest(BitcoinTestFramework):
         expected_utxocache_spents = []
         expected_utxocache_adds = []
 
+        def compare_utxo_with_event(utxo, event):
+            """Returns 1 if a utxo is identical to the event produced by BPF, otherwise"""
+            try:
+                assert_equal(utxo["txid"], bytes(event.txid[::-1]).hex())
+                assert_equal(utxo["index"], event.index)
+                assert_equal(utxo["height"], event.height)
+                assert_equal(utxo["value"], event.value)
+                assert_equal(utxo["is_coinbase"], event.is_coinbase)
+            except AssertionError:
+                self.log.exception("Assertion failed")
+                return 0
+            else:
+                return 1
+
         def handle_utxocache_add(_, data, __):
             nonlocal handle_add_succeeds
             event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents
             self.log.info(f"handle_utxocache_add(): {event}")
             add = expected_utxocache_adds.pop(0)
-            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
+            handle_add_succeeds += compare_utxo_with_event(add, event)
 
         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)
-            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
+            handle_spent_succeeds += compare_utxo_with_event(spent, event)
 
         bpf["utxocache_add"].open_perf_buffer(handle_utxocache_add)
         bpf["utxocache_spent"].open_perf_buffer(handle_utxocache_spent)
-- 
cgit v1.2.3


From f5525ad6808df6afc38e5c6e4767ab577e30629c Mon Sep 17 00:00:00 2001
From: stickies-v <stickies-v@protonmail.com>
Date: Fri, 23 Jun 2023 11:57:18 +0100
Subject: test: store utxocache events

By storing the events instead of doing the comparison inside the
handle_utxocache_* functions, we simplify the overall logic and
potentially making debugging easier, by allowing pdb to access the
events.

Mostly a refactor, but changes logging behaviour slightly by not
raising and not calling self.log.exception("Assertion failed")
---
 test/functional/interface_usdt_utxocache.py | 50 ++++++++++++-----------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py
index 7988ff2364..aa4216942d 100755
--- a/test/functional/interface_usdt_utxocache.py
+++ b/test/functional/interface_usdt_utxocache.py
@@ -252,39 +252,30 @@ class UTXOCacheTracepointTest(BitcoinTestFramework):
         # that the handle_* functions succeeded.
         EXPECTED_HANDLE_ADD_SUCCESS = 2
         EXPECTED_HANDLE_SPENT_SUCCESS = 1
-        handle_add_succeeds = 0
-        handle_spent_succeeds = 0
 
-        expected_utxocache_spents = []
         expected_utxocache_adds = []
+        expected_utxocache_spents = []
+
+        actual_utxocache_adds = []
+        actual_utxocache_spents = []
 
         def compare_utxo_with_event(utxo, event):
-            """Returns 1 if a utxo is identical to the event produced by BPF, otherwise"""
-            try:
-                assert_equal(utxo["txid"], bytes(event.txid[::-1]).hex())
-                assert_equal(utxo["index"], event.index)
-                assert_equal(utxo["height"], event.height)
-                assert_equal(utxo["value"], event.value)
-                assert_equal(utxo["is_coinbase"], event.is_coinbase)
-            except AssertionError:
-                self.log.exception("Assertion failed")
-                return 0
-            else:
-                return 1
+            """Compare a utxo dict to the event produced by BPF"""
+            assert_equal(utxo["txid"], bytes(event.txid[::-1]).hex())
+            assert_equal(utxo["index"], event.index)
+            assert_equal(utxo["height"], event.height)
+            assert_equal(utxo["value"], event.value)
+            assert_equal(utxo["is_coinbase"], event.is_coinbase)
 
         def handle_utxocache_add(_, data, __):
-            nonlocal handle_add_succeeds
             event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents
             self.log.info(f"handle_utxocache_add(): {event}")
-            add = expected_utxocache_adds.pop(0)
-            handle_add_succeeds += compare_utxo_with_event(add, event)
+            actual_utxocache_adds.append(event)
 
         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)
-            handle_spent_succeeds += compare_utxo_with_event(spent, event)
+            actual_utxocache_spents.append(event)
 
         bpf["utxocache_add"].open_perf_buffer(handle_utxocache_add)
         bpf["utxocache_spent"].open_perf_buffer(handle_utxocache_spent)
@@ -320,19 +311,18 @@ class UTXOCacheTracepointTest(BitcoinTestFramework):
                         "is_coinbase": block_index == 0,
                     })
 
-        assert_equal(EXPECTED_HANDLE_ADD_SUCCESS, len(expected_utxocache_adds))
-        assert_equal(EXPECTED_HANDLE_SPENT_SUCCESS,
-                     len(expected_utxocache_spents))
-
         bpf.perf_buffer_poll(timeout=200)
-        bpf.cleanup()
+
+        assert_equal(EXPECTED_HANDLE_ADD_SUCCESS, len(expected_utxocache_adds), len(actual_utxocache_adds))
+        assert_equal(EXPECTED_HANDLE_SPENT_SUCCESS, len(expected_utxocache_spents), len(actual_utxocache_spents))
 
         self.log.info(
             f"check that we successfully traced {EXPECTED_HANDLE_ADD_SUCCESS} adds and {EXPECTED_HANDLE_SPENT_SUCCESS} spent")
-        assert_equal(0, len(expected_utxocache_adds))
-        assert_equal(0, len(expected_utxocache_spents))
-        assert_equal(EXPECTED_HANDLE_ADD_SUCCESS, handle_add_succeeds)
-        assert_equal(EXPECTED_HANDLE_SPENT_SUCCESS, handle_spent_succeeds)
+        for expected_utxo, actual_event in zip(expected_utxocache_adds + expected_utxocache_spents,
+                                               actual_utxocache_adds + actual_utxocache_spents):
+            compare_utxo_with_event(expected_utxo, actual_event)
+
+        bpf.cleanup()
 
     def test_flush(self):
         """ Tests the utxocache:flush tracepoint API.
-- 
cgit v1.2.3


From 326db63a6819813db55ba0d01ab4fe80f7a0d818 Mon Sep 17 00:00:00 2001
From: stickies-v <stickies-v@protonmail.com>
Date: Fri, 23 Jun 2023 12:00:39 +0100
Subject: test: log sanity check assertion failures

---
 test/functional/interface_usdt_utxocache.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py
index aa4216942d..2fc5981451 100755
--- a/test/functional/interface_usdt_utxocache.py
+++ b/test/functional/interface_usdt_utxocache.py
@@ -353,9 +353,13 @@ class UTXOCacheTracepointTest(BitcoinTestFramework):
                 "size": event.size
             })
             # sanity checks only
-            assert event.memory > 0
-            assert event.duration > 0
-            handle_flush_succeeds += 1
+            try:
+                assert event.memory > 0
+                assert event.duration > 0
+            except AssertionError:
+                self.log.exception("Assertion error")
+            else:
+                handle_flush_succeeds += 1
 
         bpf["utxocache_flush"].open_perf_buffer(handle_utxocache_flush)
 
-- 
cgit v1.2.3


From bc432704505eb165dd86de39ea3434c6fb7a2514 Mon Sep 17 00:00:00 2001
From: stickies-v <stickies-v@protonmail.com>
Date: Fri, 23 Jun 2023 12:01:12 +0100
Subject: test: refactor: remove unnecessary nonlocal

Since we're only mutating, and not reassigning, we don't need to
declare `events` as `nonlocal`.
---
 test/functional/interface_usdt_net.py        | 1 -
 test/functional/interface_usdt_validation.py | 1 -
 2 files changed, 2 deletions(-)

diff --git a/test/functional/interface_usdt_net.py b/test/functional/interface_usdt_net.py
index 921af71aa9..e15ac3c1f2 100755
--- a/test/functional/interface_usdt_net.py
+++ b/test/functional/interface_usdt_net.py
@@ -139,7 +139,6 @@ class NetTracepointTest(BitcoinTestFramework):
                     checked_outbound_version_msg += 1
 
         def handle_inbound(_, data, __):
-            nonlocal events
             event = ctypes.cast(data, ctypes.POINTER(P2PMessage)).contents
             events.append((event, True))
 
diff --git a/test/functional/interface_usdt_validation.py b/test/functional/interface_usdt_validation.py
index f32d83a50c..e29b2c46eb 100755
--- a/test/functional/interface_usdt_validation.py
+++ b/test/functional/interface_usdt_validation.py
@@ -97,7 +97,6 @@ class ValidationTracepointTest(BitcoinTestFramework):
                   usdt_contexts=[ctx], debug=0)
 
         def handle_blockconnected(_, data, __):
-            nonlocal events
             event = ctypes.cast(data, ctypes.POINTER(Block)).contents
             self.log.info(f"handle_blockconnected(): {event}")
             events.append(event)
-- 
cgit v1.2.3


From 9f55773a370a0d039e727445ccee6b84e05f562a Mon Sep 17 00:00:00 2001
From: stickies-v <stickies-v@protonmail.com>
Date: Fri, 23 Jun 2023 12:05:34 +0100
Subject: test: refactor: usdt_mempool: store all events

Even though we expect these functions to only produce one event,
we still keep a counter to check if that's true. By simply storing
all the events, we can remove the counters and make debugging
easier, by allowing pdb to access the events.
---
 test/functional/interface_usdt_mempool.py | 44 +++++++++++--------------------
 1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/test/functional/interface_usdt_mempool.py b/test/functional/interface_usdt_mempool.py
index f138fa44cc..208b065c34 100755
--- a/test/functional/interface_usdt_mempool.py
+++ b/test/functional/interface_usdt_mempool.py
@@ -137,9 +137,7 @@ class MempoolTracepointTest(BitcoinTestFramework):
         """Add a transaction to the mempool and make sure the tracepoint returns
         the expected txid, vsize, and fee."""
 
-        EXPECTED_ADDED_EVENTS = 1
-        handled_added_events = 0
-        event = None
+        events = []
 
         self.log.info("Hooking into mempool:added tracepoint...")
         node = self.nodes[0]
@@ -148,9 +146,7 @@ class MempoolTracepointTest(BitcoinTestFramework):
         bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0)
 
         def handle_added_event(_, data, __):
-            nonlocal event, handled_added_events
-            event = bpf["added_events"].event(data)
-            handled_added_events += 1
+            events.append(bpf["added_events"].event(data))
 
         bpf["added_events"].open_perf_buffer(handle_added_event)
 
@@ -165,7 +161,8 @@ class MempoolTracepointTest(BitcoinTestFramework):
         self.generate(node, 1)
 
         self.log.info("Ensuring mempool:added event was handled successfully...")
-        assert_equal(EXPECTED_ADDED_EVENTS, handled_added_events)
+        assert_equal(1, len(events))
+        event = events[0]
         assert_equal(bytes(event.hash)[::-1].hex(), tx["txid"])
         assert_equal(event.vsize, tx["tx"].get_vsize())
         assert_equal(event.fee, fee)
@@ -177,9 +174,7 @@ class MempoolTracepointTest(BitcoinTestFramework):
         """Expire a transaction from the mempool and make sure the tracepoint returns
         the expected txid, expiry reason, vsize, and fee."""
 
-        EXPECTED_REMOVED_EVENTS = 1
-        handled_removed_events = 0
-        event = None
+        events = []
 
         self.log.info("Hooking into mempool:removed tracepoint...")
         node = self.nodes[0]
@@ -188,9 +183,7 @@ class MempoolTracepointTest(BitcoinTestFramework):
         bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0)
 
         def handle_removed_event(_, data, __):
-            nonlocal event, handled_removed_events
-            event = bpf["removed_events"].event(data)
-            handled_removed_events += 1
+            events.append(bpf["removed_events"].event(data))
 
         bpf["removed_events"].open_perf_buffer(handle_removed_event)
 
@@ -212,7 +205,8 @@ class MempoolTracepointTest(BitcoinTestFramework):
         bpf.perf_buffer_poll(timeout=200)
 
         self.log.info("Ensuring mempool:removed event was handled successfully...")
-        assert_equal(EXPECTED_REMOVED_EVENTS, handled_removed_events)
+        assert_equal(1, len(events))
+        event = events[0]
         assert_equal(bytes(event.hash)[::-1].hex(), txid)
         assert_equal(event.reason.decode("UTF-8"), "expiry")
         assert_equal(event.vsize, tx["tx"].get_vsize())
@@ -226,9 +220,7 @@ class MempoolTracepointTest(BitcoinTestFramework):
         """Replace one and two transactions in the mempool and make sure the tracepoint
         returns the expected txids, vsizes, and fees."""
 
-        EXPECTED_REPLACED_EVENTS = 1
-        handled_replaced_events = 0
-        event = None
+        events = []
 
         self.log.info("Hooking into mempool:replaced tracepoint...")
         node = self.nodes[0]
@@ -237,9 +229,7 @@ class MempoolTracepointTest(BitcoinTestFramework):
         bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0)
 
         def handle_replaced_event(_, data, __):
-            nonlocal event, handled_replaced_events
-            event = bpf["replaced_events"].event(data)
-            handled_replaced_events += 1
+            events.append(bpf["replaced_events"].event(data))
 
         bpf["replaced_events"].open_perf_buffer(handle_replaced_event)
 
@@ -261,7 +251,8 @@ class MempoolTracepointTest(BitcoinTestFramework):
         bpf.perf_buffer_poll(timeout=200)
 
         self.log.info("Ensuring mempool:replaced event was handled successfully...")
-        assert_equal(EXPECTED_REPLACED_EVENTS, handled_replaced_events)
+        assert_equal(1, len(events))
+        event = events[0]
         assert_equal(bytes(event.replaced_hash)[::-1].hex(), original_tx["txid"])
         assert_equal(event.replaced_vsize, original_tx["tx"].get_vsize())
         assert_equal(event.replaced_fee, original_fee)
@@ -277,9 +268,7 @@ class MempoolTracepointTest(BitcoinTestFramework):
         """Create an invalid transaction and make sure the tracepoint returns
         the expected txid, rejection reason, peer id, and peer address."""
 
-        EXPECTED_REJECTED_EVENTS = 1
-        handled_rejected_events = 0
-        event = None
+        events = []
 
         self.log.info("Adding P2P connection...")
         node = self.nodes[0]
@@ -291,9 +280,7 @@ class MempoolTracepointTest(BitcoinTestFramework):
         bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0)
 
         def handle_rejected_event(_, data, __):
-            nonlocal event, handled_rejected_events
-            event = bpf["rejected_events"].event(data)
-            handled_rejected_events += 1
+            events.append(bpf["rejected_events"].event(data))
 
         bpf["rejected_events"].open_perf_buffer(handle_rejected_event)
 
@@ -305,7 +292,8 @@ class MempoolTracepointTest(BitcoinTestFramework):
         bpf.perf_buffer_poll(timeout=200)
 
         self.log.info("Ensuring mempool:rejected event was handled successfully...")
-        assert_equal(EXPECTED_REJECTED_EVENTS, handled_rejected_events)
+        assert_equal(1, len(events))
+        event = events[0]
         assert_equal(bytes(event.hash)[::-1].hex(), tx["tx"].hash)
         assert_equal(event.reason.decode("UTF-8"), "min relay fee not met")
 
-- 
cgit v1.2.3