Repository navigation
backport: v0.26 bitcoin#26531, partial bitcoin#27944, bitcoin#28088 - #75
Closed
DCG-Claude wants to merge 3 commits into
Closed
DCG-Claude wants to merge 3 commits into
DCG-Claude wants to merge 3 commits into
Conversation
4b7aec2 Add mempool tracepoints (virtu) Pull request description: This PR adds multiple mempool tracepoints. | tracepoint | description | | ------------- | ------------- | | `mempool:added` | Is called when a transaction enters the mempool | | `mempool:removed` | ... when a transaction is removed from the mempool | | `mempool:replaced` | ... when a transaction is replaced in the mempool | | `mempool:rejected` | ... when a transaction is rejected from entering the mempool | The tracepoints are further documented in `docs/tracing.md`. Usage is demonstrated in the example script `contrib/tracing/mempool_monitor.py`. Interface tests are provided in `test/functional/interface_usdt_mempool.py`. The rationale for passing the removal reason as a string instead of numerically is that the benefits of not having to maintain a redundant enum-string mapping seem to outweigh the small cost of string generation. The reject reason is passed as string as well, although in this instance the string does not have to be generated but is readily available. ACKs for top commit: 0xB10C: ACK 4b7aec2 achow101: ACK 4b7aec2 Tree-SHA512: 6deb3ba2d1a061292fb9b0f885f7a5c4d11b109b838102d8a8f4828cd68f5cd03fa3fc64adc6fdf54a08a1eaccce261b0aa90c2b8c33cd5fd3828c8f74978958 Dash adaptations: - src/txmempool.cpp: `#include <util/trace.h>` added alone — upstream's adjacent `<util/result.h>`/`<util/translation.h>` lines are context this PR does not add and Dash's include block does not have. - src/txmempool.cpp: TRACE3(mempool, added) placed as the last statement of addUnchecked(), i.e. after Dash's `addUncheckedProTx(newit, tx)` call, preserving upstream's 'end of addUnchecked' position around the Dash-only ProTx bookkeeping. - src/txmempool.cpp: TRACE5(mempool, removed) applied verbatim; Dash's RemovalReasonToString() longest string is still 'sizelimit' (9 chars), so the documented MAX_REMOVAL_REASON_LENGTH=9 holds. - doc/tracing.md: the `mempool:replaced` subsection is not added (Dash emits no such tracepoint, see omitted_hunks); heading kept as Dash's 'Adding tracepoints to Dash Core'. - contrib/tracing/mempool_monitor.py: replaced_event struct, trace_replaced BPF fn, handle_replaced, the mempool:replaced enable_probe/perf buffer, the 'replaced' metric rows and the 'replaced' branch of parse_event are dropped — enable_probe() on a probe absent from the binary raises at startup, so keeping them would make the script unusable on dashd. Also bitcoind->dashd (path arg, USAGE string, variable names) and 'Bitcoin Core'->'Dash Core' in the docstring. - contrib/tracing/README.md: ./src/bitcoind -> ./src/dashd; the mempool_monitor.py description lists only added/removed/rejected; the sample dashboard drops the 'replaced' count/rate rows and the two replaced event-log lines, and the sample 'removed ...: replaced' line now shows ': expiry' (a reason Dash's RemovalReasonToString actually returns). - test/functional/interface_usdt_mempool.py: replaced_test(), the replaced_event struct, trace_replaced and the run_test() call are dropped for the same reason; doc URL retargeted to dashpay/dash blob/develop, matching the other interface_usdt_*.py files in Dash. Not applicable to Dash (intentionally omitted): - src/validation.cpp: The TRACE7(mempool, replaced) hunk lands inside MemPoolAccept::Finalize's 'Remove conflicting transactions from the mempool' loop. Dash has no RBF: Workspace has no m_all_conflicting/m_conflicting_fees/m_conflicting_size/m_replaced_transactions and MemPoolRemovalReason has no REPLACED member, so the entire enclosing loop is absent from Dash and there is nowhere for the tracepoint to fire. The mempool:replaced tracepoint therefore does not exist in dashd, and its doc/monitor/test counterparts were dropped to match (listed under adaptations). The other three tracepoints (added/removed/rejected) are landed in full.
…ups (27831 follow-ups) 9f55773 test: refactor: usdt_mempool: store all events (stickies-v) bc43270 test: refactor: remove unnecessary nonlocal (stickies-v) 326db63 test: log sanity check assertion failures (stickies-v) f5525ad test: store utxocache events (stickies-v) f1b99ac test: refactor: deduplicate handle_utxocache_* logic (stickies-v) ad90ba3 test: refactor: rename inbound to is_inbound (stickies-v) afc0224 test: refactor: remove unnecessary blocks_checked counter (stickies-v) Pull request description: Various cleanups to the USDT functional tests, largely (but not exclusively) follow-ups to bitcoin#27831 (review). Except for slightly different logging behaviour in "test: store utxocache events" and "test: log sanity check assertion failures", this is a refactor PR, removing unnecessary code and (imo) making it more readable and maintainable. The rationale for each change is in the corresponding commit message. Note: except for "test: store utxocache events" (which relies on its parent, and I separated into two commits because we may want the parent but not the child), all commits are stand-alone and I'm okay with dropping one/multiple commits if they turn out to be controversial or undesired. ACKs for top commit: 0xB10C: ACK 9f55773. Reviewed the code and ran the USDT interface tests. I stepped through the commits and think all changes are reasonable. Tree-SHA512: 6c37a0265b6c26d4f9552a056a690b8f86f7304bd33b4419febd8b17369cf6af799cb87c16df35d0c2a1b839ad31de24661d4384eafa88816c2051c522fd3bf5 Dash adaptations: - test/functional/interface_usdt_validation.py: the resolved hunk also deletes the in-callback assertion block that Dash's partial bitcoin#27831 backport (c781f1a) left behind alongside the new post-poll loop; upstream's callback body is just events.append(event), so the duplicate had to go and the file now matches upstream's post-image (Dash keeps its own cflags=["-Wno-error=implicit-function-declaration"] line from bitcoin#28629 and its dashpay doc URLs) Omitted (partial backport): - test/functional/interface_usdt_mempool.py: Dash's bitcoin#27831 backport (c781f1a) is marked (Partial) and covers only net/utxocache/validation; interface_usdt_mempool.py is still at the bitcoin#26531 state with assert_equal() calls inside each bcc callback, so the 'EXPECTED_*_EVENTS/handled_*_events/event = None' -> 'events = []' rewrite that bitcoin#27944 performs has nothing to rewrite here (needs bitcoin#27831)
faf8be7 test: Disable known broken USDT test (MarcoFalke) Pull request description: The failure is known and running into more failures doesn't help anyone. Not disabling the test would be a waste of CPU and developer time. bitcoin#27380 Top commit has no ACKs. Tree-SHA512: d0469153b00d6b30e10a21bcd52d508fcf9f796ff2468f59aff75020a82c718bcae85caf4b58397dea6fd9e210b501353fd51567f979c6b57d3b1bb23d318216 Dash adaptations: - test/functional/interface_usdt_mempool.py: Dash still has the pre-bitcoin#27679 structure, so the disabled assertion is `assert_equal(reason, event.reason.decode("UTF-8"))` inside handle_rejected_event() rather than upstream's inlined `assert_equal(event.reason.decode("UTF-8"), "min relay fee not met")` at the end of the function; the same three-line comment referencing bitcoin#27380 precedes it - test/functional/interface_usdt_mempool.py: also commented out `reason = "min relay fee not met"` — in Dash's structure it is a local of rejected_test() whose only reference was the now-disabled assertion, and test/lint/lint-python.py enables flake8 F841 (assigned but never used), so leaving it live would fail lint. Upstream had no such variable to deal with - test/functional/interface_usdt_mempool.py: the incoming hunk's trailing context (`bpf.cleanup()` / `self.generate(self.wallet, 1)` after the assertions) was dropped — Dash already calls bpf.cleanup() before the event-count assertion and has no post-test generate in this function; that context belongs to upstream's refactored layout, not to the change itself
This was referenced Sep 20, 2026
|
This pull request has conflicts, please rebase. |
Collaborator
Author
|
batch backport-0.26-b034-contrib is over: folded into backport-0.26-b058-misc (batch 58) |
Collaborator
Author
|
folded into backport-0.26-b058-misc: its commits ship there |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated Bitcoin Core v0.26 backports, batch
backport-0.26-b034-contrib.e636d51abbe3c8b35bbaa32288c5c3Provenance
Each commit passed: cherry-pick (adapted by an Opus lane only where conflicts existed), build, touched tests, a mechanical diff-of-diffs check (every upstream hunk landed; no added line without an upstream counterpart), and an independent Opus verification lane where anything was adapted. Gate rows and lane artifacts are in the backportsys DB.