-
Notifications
You must be signed in to change notification settings - Fork 0
Merge bitcoin/bitcoin#25528: ci: run USDT interface tests in the CI #1201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: backport-0.24-batch-463
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -174,7 +174,7 @@ def test_uncache(self): | |||||
| invalid_tx.vin[0].prevout.hash = int(block_1_coinbase_txid, 16) | ||||||
|
|
||||||
| self.log.info("hooking into the utxocache:uncache tracepoint") | ||||||
| ctx = USDT(path=str(self.options.bitcoind)) | ||||||
| ctx = USDT(pid=self.nodes[0].process.pid) | ||||||
| ctx.enable_probe(probe="utxocache:uncache", | ||||||
| fn_name="trace_utxocache_uncache") | ||||||
| bpf = BPF(text=utxocache_changes_program, usdt_contexts=[ctx], debug=0, cflags=["-Wno-error=implicit-function-declaration"]) | ||||||
|
|
@@ -242,7 +242,7 @@ def test_add_spent(self): | |||||
|
|
||||||
| self.log.info( | ||||||
| "hook into the utxocache:add and utxocache:spent tracepoints") | ||||||
| ctx = USDT(path=str(self.options.bitcoind)) | ||||||
| ctx = USDT(pid=self.nodes[0].process.pid) | ||||||
| ctx.enable_probe(probe="utxocache:add", fn_name="trace_utxocache_add") | ||||||
| ctx.enable_probe(probe="utxocache:spent", | ||||||
| fn_name="trace_utxocache_spent") | ||||||
|
|
@@ -346,7 +346,7 @@ def test_flush(self): | |||||
|
|
||||||
| self.log.info("test the utxocache:flush tracepoint API") | ||||||
| self.log.info("hook into the utxocache:flush tracepoint") | ||||||
| ctx = USDT(path=str(self.options.bitcoind)) | ||||||
| ctx = USDT(pid=self.nodes[0].process.pid) | ||||||
| ctx.enable_probe(probe="utxocache:flush", | ||||||
| fn_name="trace_utxocache_flush") | ||||||
| bpf = BPF(text=utxocache_flushes_program, usdt_contexts=[ctx], debug=0, cflags=["-Wno-error=implicit-function-declaration"]) | ||||||
|
|
@@ -357,16 +357,17 @@ def test_flush(self): | |||||
| # that the handle_* functions succeeded. | ||||||
| EXPECTED_HANDLE_FLUSH_SUCCESS = 3 | ||||||
| handle_flush_succeeds = 0 | ||||||
| possible_cache_sizes = set() | ||||||
| expected_flushes = [] | ||||||
| expected_flushes = list() | ||||||
|
|
||||||
| def handle_utxocache_flush(_, data, __): | ||||||
| nonlocal handle_flush_succeeds | ||||||
| event = ctypes.cast(data, ctypes.POINTER(UTXOCacheFlush)).contents | ||||||
| self.log.info(f"handle_utxocache_flush(): {event}") | ||||||
| expected = expected_flushes.pop(0) | ||||||
| assert_equal(expected["mode"], FLUSHMODE_NAME[event.mode]) | ||||||
| possible_cache_sizes.remove(event.size) # fails if size not in set | ||||||
| expected_flushes.remove({ | ||||||
| "mode": FLUSHMODE_NAME[event.mode], | ||||||
| "for_prune": event.for_prune, | ||||||
| "size": event.size | ||||||
| }) | ||||||
| # sanity checks only | ||||||
| assert(event.memory > 0) | ||||||
| assert(event.duration > 0) | ||||||
|
|
@@ -375,33 +376,37 @@ 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 earlier tests are modified | ||||||
| UTXOS_IN_CACHE = 2 # 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. | ||||||
| possible_cache_sizes = {UTXOS_IN_CACHE, 0} | ||||||
| flush_for_shutdown = {"mode": "ALWAYS", "for_prune": False} | ||||||
| expected_flushes.extend([flush_for_shutdown, flush_for_shutdown]) | ||||||
| expected_flushes.append({"mode": "ALWAYS", "for_prune": False, "size": UTXOS_IN_CACHE}) | ||||||
| expected_flushes.append({"mode": "ALWAYS", "for_prune": False, "size": 0}) | ||||||
| self.stop_node(0) | ||||||
|
|
||||||
| bpf.perf_buffer_poll(timeout=200) | ||||||
| bpf.cleanup() | ||||||
|
|
||||||
| self.log.info("check that we don't expect additional flushes") | ||||||
| assert_equal(0, len(expected_flushes)) | ||||||
| assert_equal(0, len(possible_cache_sizes)) | ||||||
|
|
||||||
| self.log.info("restart the node with -prune") | ||||||
| self.start_node(0, ["-fastprune=1", "-prune=1"]) | ||||||
|
|
||||||
| BLOCKS_TO_MINE = 450 | ||||||
| self.log.info(f"mine {BLOCKS_TO_MINE} blocks to be able to prune") | ||||||
| self.generate(self.wallet, BLOCKS_TO_MINE) | ||||||
| # we added BLOCKS_TO_MINE coinbase UTXOs to the cache | ||||||
| possible_cache_sizes = {BLOCKS_TO_MINE} | ||||||
| expected_flushes.append( | ||||||
| {"mode": "NONE", "for_prune": True, "size_fn": lambda x: x == BLOCKS_TO_MINE}) | ||||||
|
|
||||||
| self.log.info("test the utxocache:flush tracepoint API with pruning") | ||||||
| self.log.info("hook into the utxocache:flush tracepoint") | ||||||
| ctx = USDT(pid=self.nodes[0].process.pid) | ||||||
| ctx.enable_probe(probe="utxocache:flush", | ||||||
| fn_name="trace_utxocache_flush") | ||||||
| bpf = BPF(text=utxocache_flushes_program, usdt_contexts=[ctx], debug=0) | ||||||
| bpf["utxocache_flush"].open_perf_buffer(handle_utxocache_flush) | ||||||
|
|
||||||
| self.log.info(f"prune blockchain to trigger a flush for pruning") | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Minor: Remove f-string prefix without placeholders. The f-string on line 408 has no placeholders and can be a regular string. - self.log.info(f"prune blockchain to trigger a flush for pruning")
+ self.log.info("prune blockchain to trigger a flush for pruning")📝 Committable suggestion
Suggested change
🧰 Tools🪛 Flake8 (7.3.0)[error] 408-408: f-string is missing placeholders (F541) 🪛 Ruff (0.14.6)408-408: f-string without any placeholders Remove extraneous (F541) 408-408: Logging statement uses f-string (G004) 🤖 Prompt for AI Agents |
||||||
| expected_flushes.append({"mode": "NONE", "for_prune": True, "size": 0}) | ||||||
| self.nodes[0].pruneblockchain(415) | ||||||
|
Comment on lines
+399
to
410
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get current file and check for Bitcoin commit references
echo "=== Current repository info ==="
git log --oneline -1
echo "=== Check git show for the interface_usdt_utxocache.py file ==="
git log --oneline test/functional/interface_usdt_utxocache.py | head -5
echo "=== Get the current Dash pruning test section ==="
git show HEAD:test/functional/interface_usdt_utxocache.py | sed -n '395,415p'Repository: DashCoreAutoGuix/dash Length of output: 1264 🏁 Script executed: #!/bin/bash
# Search for Bitcoin commit references in commit messages or files
echo "=== Search for Bitcoin commit reference in recent commits ==="
git log --oneline -20 | grep -i bitcoin
echo "=== Check the commit message for this file ==="
git log -1 --format="%B" -- test/functional/interface_usdt_utxocache.pyRepository: DashCoreAutoGuix/dash Length of output: 320 🏁 Script executed: #!/bin/bash
# Look for the original Bitcoin USDT interface test file
echo "=== Checking if there's info about the upstream Bitcoin commit ==="
git log --all --oneline -- test/functional/interface_usdt_utxocache.py | head -10
echo "=== Get full context of lines 399-410 in current file ==="
git show HEAD:test/functional/interface_usdt_utxocache.py | sed -n '390,420p'Repository: DashCoreAutoGuix/dash Length of output: 2363 🏁 Script executed: #!/bin/bash
# Search for Bitcoin Core upstream information
echo "=== Looking for Bitcoin commit details in the backport ==="
git log d063103a2 --format="%B" -1 | head -20
echo "=== Check if there are any notes about the Bitcoin commit hash ==="
git show d063103a2 | head -50Repository: DashCoreAutoGuix/dash Length of output: 1396 🌐 Web query:
💡 Result: I couldn't find a Bitcoin Core PR numbered 25528 that touches interface_usdt_utxocache.py. The relevant change is PR bitcoin#32101 ("Accept unordered tracepoints in interface_usdt_utxocache.py"), which was merged into master on 2025-03-27 — it updates test/functional/interface_usdt_utxocache.py to accept unordered USDT tracepoints (and related test changes). [1][2] Sources:
If you actually meant PR bitcoin#25528, tell me the repository/owner or paste a link and I’ll look it up. References: 🏁 Script executed: #!/bin/bash
# Check the full diff to see what was changed in this merge
echo "=== Check files modified in this merge ==="
git show d063103a2 --stat | grep interface_usdt
echo "=== Check if there are related commits that reference Bitcoin commits ==="
git log --all --oneline -- test/functional/interface_usdt_utxocache.py | grep -i bitcoin | head -10
echo "=== Look for any comments indicating Bitcoin commit in the file ==="
git show d063103a2:test/functional/interface_usdt_utxocache.py | head -50Repository: DashCoreAutoGuix/dash Length of output: 2727 🏁 Script executed: #!/bin/bash
# Check if there's a reference commit or documentation
echo "=== Search for commit message patterns that might indicate Bitcoin commit ==="
git log -50 --oneline | grep -i usdt
echo "=== Check earlier commits that touched this file ==="
git log --all --oneline test/functional/interface_usdt_utxocache.py | tail -20Repository: DashCoreAutoGuix/dash Length of output: 1021 🏁 Script executed: #!/bin/bash
# Let's look for any scope creep indicators - look for unrelated changes
echo "=== Check the pruning section for any TODO/FIXME/while we're here comments ==="
git show HEAD:test/functional/interface_usdt_utxocache.py | sed -n '390,430p' | grep -E "TODO|FIXME|while we're here|also fix"
echo "=== Check if there are uncommitted changes or notes about scope creep ==="
git diff HEAD test/functional/interface_usdt_utxocache.pyRepository: DashCoreAutoGuix/dash Length of output: 217 🏁 Script executed: #!/bin/bash
# Check the history of the pruning section specifically
echo "=== Check git blame for the pruning test section ==="
git blame -L 399,410 HEAD -- test/functional/interface_usdt_utxocache.py
echo "=== Check if this section has been modified since the initial merge ==="
git log --follow -p test/functional/interface_usdt_utxocache.py | grep -B5 -A5 "prune blockchain to trigger a flush for pruning" | head -30Repository: DashCoreAutoGuix/dash Length of output: 2036 🌐 Web query:
💡 Result: Answer: interface_usdt_utxocache.py is a Bitcoin Core functional test that exercises the UTXO-cache USDT tracepoints (used by BPF/bcc/bpftrace tracing). The test watches tracepoints such as utxocache:add / utxocache:spent and the utxocache:flush event to ensure the tracepoint interface and arguments remain stable for external tracers. The utxocache:flush tracepoint corresponds to CCoinsViewCache::Flush() — i.e., the moment the in‑memory UTXO cache is written to disk (used for durability and triggered by events like pruning or periodic durability flushes). [1][2][3] Sources:
🏁 Script executed: #!/bin/bash
# Check the earlier fix commit
echo "=== Check the earlier fix in commit 1204dc0f8 ==="
git show 1204dc0f8 --stat | head -20
echo "=== Look at what was changed in that fix ==="
git show 1204dc0f8 | grep -A 20 "prune" | head -40Repository: DashCoreAutoGuix/dash Length of output: 1297 🏁 Script executed: #!/bin/bash
# Compare the file size and structure across versions
echo "=== Check file line counts across versions ==="
echo "Current (d063103a2):"
git show d063103a2:test/functional/interface_usdt_utxocache.py | wc -l
echo "Earlier fix (1204dc0f8):"
git show 1204dc0f8:test/functional/interface_usdt_utxocache.py | wc -l 2>/dev/null || echo "File may not exist in this commit"
echo "=== Look for pruning section in earlier fix commit ==="
git show 1204dc0f8:test/functional/interface_usdt_utxocache.py 2>/dev/null | grep -n "prune" | head -10Repository: DashCoreAutoGuix/dash Length of output: 951 🏁 Script executed: #!/bin/bash
# Check if the pruning test initialization was added or was already there
echo "=== Check the full pruning test section structure ==="
git show HEAD:test/functional/interface_usdt_utxocache.py | sed -n '385,425p'Repository: DashCoreAutoGuix/dash Length of output: 1701 🌐 Web query:
💡 Result: I can't find a file named interface_usdt_utxocache.py in the main Bitcoin Core test tree or common forks. Do you have a link or the repository/branch where that test lives? If you want, I can search a specific repo or scan GitHub for that filename. (Background: the Bitcoin functional tests live under test/functional — docs). [1][2] Sources: 🏁 Script executed: #!/bin/bash
# Let's look at what changed between the earlier version and current
echo "=== Compare pruning test structure ==="
echo "=== Earlier version (1204dc0f8) pruning section ==="
git show 1204dc0f8:test/functional/interface_usdt_utxocache.py 2>/dev/null | sed -n '375,410p'
echo ""
echo "=== Current version (d063103a2) pruning section ==="
git show d063103a2:test/functional/interface_usdt_utxocache.py | sed -n '390,425p'Repository: DashCoreAutoGuix/dash Length of output: 3035 🏁 Script executed: #!/bin/bash
# Check if there's a commit between these two that modified the pruning section
echo "=== Commits between 1204dc0f8 and d063103a2 that touched this file ==="
git log 1204dc0f8..d063103a2 --oneline -- test/functional/interface_usdt_utxocache.pyRepository: DashCoreAutoGuix/dash Length of output: 213 Remove USDT context re-initialization from pruning test section. Lines 399–406 re-initialize the USDT context and BPF program after the node restart, but this duplicates the probe setup pattern and deviates from the earlier test structure which used cache size validation ( Remove the USDT re-initialization block (lines 399–406) and revert to the cache size validation approach with lambda functions for proper test coverage of the pruning flush behavior. 🧰 Tools🪛 Flake8 (7.3.0)[error] 408-408: f-string is missing placeholders (F541) 🪛 Ruff (0.14.6)408-408: f-string without any placeholders Remove extraneous (F541) 408-408: Logging statement uses f-string (G004) 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| bpf.perf_buffer_poll(timeout=500) | ||||||
|
|
@@ -410,7 +415,6 @@ def handle_utxocache_flush(_, data, __): | |||||
| self.log.info( | ||||||
| f"check that we don't expect additional flushes and that the handle_* function succeeded") | ||||||
| assert_equal(0, len(expected_flushes)) | ||||||
| assert_equal(0, len(possible_cache_sizes)) | ||||||
| assert_equal(EXPECTED_HANDLE_FLUSH_SUCCESS, handle_flush_succeeds) | ||||||
|
|
||||||
| self.stop_node(0, expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE) | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Minor: Use literal list syntax.
The static analysis correctly suggests using
[]instead oflist()for clarity.📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.14.6)
360-360: Unnecessary
list()call (rewrite as a literal)Rewrite as a literal
(C408)
🤖 Prompt for AI Agents