Merge bitcoin/bitcoin#27944: test: various USDT functional test cleanups (27831 follow-ups)#926
Conversation
…31 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
WalkthroughVariable renaming and event handling logic were refactored in three functional test files. Counters and inline assertions in event handlers were replaced with event collection and post-processing assertions. Variable names related to message direction were standardized, and event comparison logic was moved outside of callbacks for clarity and robustness. No public interfaces were changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (2)test/functional/**/*.py📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
**⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (8)📓 Common learnings📚 Learning: in dash backports from bitcoin core, witness transaction-related code (msg_wtx, wtxid) should be rep...Applied to files:
📚 Learning: in dash backports from bitcoin core test files, the `address_to_scriptpubkey` function should be imp...Applied to files:
📚 Learning: during dash backport verification of bitcoin core commit 06d469c, scope creep was detected when a...Applied to files:
📚 Learning: during multiple verification attempts of bitcoin core commit 06d469c backport to dash pr #566, da...Applied to files:
📚 Learning: dashcoreautoguix successfully completed a complex bitcoin core backport (pr bitcoin#29412) for block mutati...Applied to files:
📚 Learning: dashcoreautoguix successfully fixed scope creep in bitcoin core commit fcdb39d backport by removi...Applied to files:
📚 Learning: the `add_wallet_options` method is not available in dash's test framework and should be removed when...Applied to files:
🧬 Code Graph Analysis (2)test/functional/interface_usdt_net.py (2)
test/functional/interface_usdt_validation.py (1)
🪛 Ruff (0.12.2)test/functional/interface_usdt_net.py124-124: Missing return type annotation for private function Add return type annotation: (ANN202) 124-124: Missing type annotation for function argument (ANN001) 124-124: Missing type annotation for function argument (ANN001) 128-128: Logging statement uses f-string (G004) test/functional/interface_usdt_utxocache.py264-264: Missing return type annotation for private function Add return type annotation: (ANN202) 264-264: Missing type annotation for function argument (ANN001) 264-264: Missing type annotation for function argument (ANN001) 272-272: Missing return type annotation for private function Add return type annotation: (ANN202) 272-272: Missing type annotation for function argument (ANN001) 272-272: Missing type annotation for function argument (ANN001) 272-272: Missing type annotation for function argument (ANN001) 274-274: Logging statement uses f-string (G004) 277-277: Missing return type annotation for private function Add return type annotation: (ANN202) 277-277: Missing type annotation for function argument (ANN001) 277-277: Missing type annotation for function argument (ANN001) 277-277: Missing type annotation for function argument (ANN001) 279-279: Logging statement uses f-string (G004) 322-322: Logging statement uses f-string (G004) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
🔇 Additional comments (8)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Backport Verification SuccessfulNo issues found! This backport is complete and faithful to the original Bitcoin change. Original Bitcoin commit: Verification Details:
Reviewer Feedback Analysis:PastaPastaPasta comments reviewed: 0 No reviewer feedback to address - PR is ready for review. Technical Analysis:This PR successfully backports Bitcoin's USDT functional test cleanups. The missing This PR is ready for merge. ✅ |
✅ Verification Complete - APPROVEDThis backport of Bitcoin Core PR bitcoin#27944 has been verified and approved. Verification SummaryBitcoin Commit: c5a63ea Changes VerifiedAll three modified test files correctly implement the Bitcoin changes:
Intentionally Excluded
Dash Adaptations
The backport faithfully implements the USDT test cleanup refactoring from Bitcoin Core with appropriate handling of Dash's test suite differences. |
Backports bitcoin#27944
Original commit: c5a63ea
Backported from Bitcoin Core v0.26
Note: The interface_usdt_mempool.py test was not included in this backport as it doesn't exist in Dash.
Summary by CodeRabbit