Merge bitcoin/bitcoin#27695: test: Add test to check tx in the last block can be downloaded

[DashCoreAutoGuix]

Backports bitcoin#27695

Original commit: 7be7e62

Changes

• Adds new test test_tx_in_block() to verify that transactions in the last block can still be downloaded (beneficial for compact block relay)
• Refactors existing test into test_notfound_on_unannounced_tx() method

Dash-specific adaptations

• Uses txid instead of wtxid since Dash doesn't have SegWit
• Uses rehash() instead of getwtxid() for transaction hash comparison
• Preserves Dash-specific UTXO generation code (self.generate(self.miniwallet, 1) and self.generate(self.gen_node, 100))

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Enhanced P2P transaction testing with refactored test structure and additional test coverage for transaction announcement and retrieval scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The test file refactors its setup to use instance attributes instead of local variables and splits the test logic into two separate test methods: test_tx_in_block and test_notfound_on_unannounced_tx. The file also updates copyright information and docstrings.

Changes

Cohort / File(s)	Summary
P2P leak transaction test refactoring test/functional/p2p_leak_tx.py	Refactored setup to use instance attributes (self.gen_node, self.miniwallet); split test logic into two methods: test_tx_in_block (creates inbound connection, generates transaction, waits for inv, verifies tx hash) and test_notfound_on_unannounced_tx (tests notfound behavior on unannounced transactions); updated copyright and docstring.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

• test_tx_in_block method: Verify the inv waiting logic, tx hash assertion, and P2P connection handling are correct
• test_notfound_on_unannounced_tx method: Review the retry loop logic, break conditions based on notfound responses, and inv presence verification
• Instance attribute refactoring: Confirm all references to self.gen_node and self.miniwallet are properly initialized and used throughout the test flow

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding a test to verify transaction download from the last block, which aligns with the new test_tx_in_block() method being introduced.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch backport-0.26-batch-477-pr-27695

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e63410e and 409cc92.

📒 Files selected for processing (1)

• test/functional/p2p_leak_tx.py (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests should be placed in test/functional/ and written in Python

Files:

• test/functional/p2p_leak_tx.py

**

⚙️ CodeRabbit configuration file

**: # CodeRabbit AI Review Instructions for Dash Backports

Your Role

You are reviewing Bitcoin Core backports to Dash Core. Your ONLY job is to validate that the Dash commit faithfully represents the original Bitcoin commit with minimal, necessary adaptations.

Critical Validation Rules

1. File Operations Must Match (AUTO-REJECT if violated)

• If Bitcoin modifies an existing file → Dash MUST modify (not create new)
• If Bitcoin creates a new file → Dash creates
• If Bitcoin deletes a file → Dash deletes
• Common failure: Bitcoin modifies keys.txt, Dash creates new file with 58 keys

2. Size Ratio Check (80-150% of Bitcoin)

• Count functional lines changed (exclude comments/whitespace)
• Dash changes should be 80-150% of Bitcoin's size
• Red flag: 2-line Bitcoin fix becoming 150+ lines in Dash

3. No Scope Creep

• Reject if you see: "TODO:", "FIXME:", "while we're here", "also fix"
• No unrelated refactoring or style changes
• Only Bitcoin's intended changes + minimal Dash adaptations

4. Bitcoin-Specific Code Detection

• Auto-reject witness/segwit code: msg_wtxidrelay, MSG_WTX, witness imports
• Auto-reject RBF (replace-by-fee) functionality
• Note: PSBT is supported in Dash (don't flag)

5. Mandatory Adaptations Only

• bitcoin → dash in strings/paths
• Bitcoin → Dash in user-facing text
• Port numbers: 8332→9998 (RPC), 8333→9999 (P2P)
• Hardcoded test values specific to Dash
• No other changes unless absolutely required

6. Completeness Check

• All files changed in Bitcoin must be present
• Extra files need clear justification (Dash-specific compatibility)
• Missing files = incomplete backport

Review Process

1. First: Check file operations match exactly
2. Second: Calculate size ratio
3. Third: Scan for scope creep patterns
4. Fourth: Detect Bitcoin-specific code
5. Fifth: Verify all changes are minimal adaptations

Output Format

VALIDATION: [PASS/FAIL]

File Operatio...

Files:

• test/functional/p2p_leak_tx.py

🧠 Learnings (2)

📓 Common learnings

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-31T01:14:55.631Z
Learning: DashCoreAutoGuix successfully completed a complex Bitcoin Core backport (PR #29412) for block mutation detection by implementing the IsBlockMutated function, adding net processing integration, creating comprehensive unit tests, and properly adapting all Bitcoin-specific witness code for Dash compatibility. The backport maintains full security functionality while respecting Dash's non-witness transaction architecture.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T22:03:12.364Z
Learning: During multiple verification attempts of Bitcoin Core commit 06d469c26b backport to Dash PR #566, DashCoreAutoGuix consistently identified scope creep in interface_usdt_utxocache.py where additional pruning test functionality was added beyond the original Bitcoin commit. The user provided comprehensive fixes including both scope creep removal and missing mempool test file additions, but couldn't push due to authentication restrictions. The scope creep fix was identified as the priority to resolve CI failures.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T17:13:35.087Z
Learning: When backporting Bitcoin Core commits that use Python's textwrap.dedent() function in test files, the textwrap import statement needs to be explicitly added if it's missing in the Dash test file.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T21:29:32.827Z
Learning: DashCoreAutoGuix successfully fixed scope creep in Bitcoin Core commit fcdb39d3ee backport by removing the parse test case from src/test/uint256_tests.cpp that was not part of the original Bitcoin commit. The fix was implemented in commit 16748115ce and verified through range-diff analysis.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T22:13:54.797Z
Learning: In Dash backports from Bitcoin Core, witness transaction-related code (MSG_WTX, wtxid) should be replaced with regular transaction handling (MSG_TX, txid) for compatibility, as demonstrated in the p2p_filter.py test fix where MSG_WTX was replaced with MSG_TX and irr_wtxid usage was replaced with irr_txid.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-30T14:45:15.700Z
Learning: In Dash backports from Bitcoin Core test files, the `address_to_scriptpubkey` function should be imported from `test_framework.wallet` instead of `test_framework.address` as the import location differs between Bitcoin and Dash test frameworks.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T19:54:21.426Z
Learning: In Dash backports from Bitcoin Core, including necessary compilation fixes (such as API compatibility changes like UniValue get_int() → getInt<int>()) alongside the core backport is standard and expected practice. These compatibility fixes ensure the backported code compiles in Dash's evolved codebase while preserving Bitcoin's original functionality and intent.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-27T22:35:10.176Z
Learning: In Dash backports, src/dashbls files are vendored dependencies that should not be modified during Bitcoin Core backports unless there is specific justification. Unauthorized modifications to vendored dependencies should be removed to maintain code integrity.

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-30T14:45:15.700Z
Learning: The `add_wallet_options` method is not available in Dash's test framework and should be removed when backporting Bitcoin Core tests that use this method, as demonstrated in wallet_rescan_unconfirmed.py.

📚 Learning: 2025-07-29T22:13:54.797Z

Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T22:13:54.797Z
Learning: In Dash backports from Bitcoin Core, witness transaction-related code (MSG_WTX, wtxid) should be replaced with regular transaction handling (MSG_TX, txid) for compatibility, as demonstrated in the p2p_filter.py test fix where MSG_WTX was replaced with MSG_TX and irr_wtxid usage was replaced with irr_txid.

Applied to files:

• test/functional/p2p_leak_tx.py

🪛 Ruff (0.14.7)

test/functional/p2p_leak_tx.py

56-56: Use f-string instead of format call

Convert to f-string

(UP032)

---

56-56: Logging statement uses str.format

(G001)

---

58-58: Use f-string instead of format call

Convert to f-string

(UP032)

---

58-58: Logging statement uses str.format

(G001)

🔇 Additional comments (1)

test/functional/p2p_leak_tx.py (1)

2-76: Backport faithfully mirrors upstream p2p_leak_tx changes with correct Dash txid adaptations

The changes here are tightly scoped to what bitcoin#27695 describes: wiring shared test state via self.gen_node/self.miniwallet, adding test_tx_in_block, and splitting the previous inline logic into test_notfound_on_unannounced_tx, invoked from run_test(). This matches the upstream description “Add test to check tx in the last block can be downloaded” and “Split up test_notfound_on_unannounced_tx test case” from the Bitcoin release notes. (rpmfind.net)

Key validation points:

• File Operations: Bitcoin modifies the existing test/functional/p2p_leak_tx.py; Dash also modifies the same file (no new or deleted files in this PR). This satisfies the “modify vs create/delete” rule.
• Size Ratio: Functional changes are limited to wiring self.gen_node/self.miniwallet, the new test_tx_in_block body, and moving the existing “notfound_on_unannounced” logic into its own method. No extra helpers, imports, or behavior changes are introduced, so Dash’s diff is effectively 100% of Bitcoin’s change size, well within the 80–150% band.
• Scope Creep: No TODO/FIXME, no unrelated refactors, and no stylistic churn. Static-analysis suggestions about replacing str.format with f-strings are intentionally not applied here to avoid diverging from the upstream patch.
• Bitcoin-Specific Code: There is no segwit/witness/RBF machinery (wtxid, MSG_WTX, msg_wtxidrelay, etc.). Instead, the test uses MSG_TX and txid (via send_self_transfer()["txid"] and tx.rehash()), which is the correct Dash adaptation pattern for prior witness-based tests. This aligns with upstream follow-up commentary about (w)txid confusion in p2p_leak_tx.py and matches the established Dash backport strategy of replacing witness IDs with plain txids. (mirror.b10c.me) Based on learnings, this mirrors the expected txid/MSG_TX adaptation for Dash.
• Completeness: PR summary and external release notes both indicate this change is solely about the functional test; no missing companion files or extra Dash-only files are apparent.

Static analysis warnings about .format in the log lines at Lines 56 and 58 are style-only and would be scope creep to adjust in this backport; leaving them as-is is appropriate here.

Validation summary:

VALIDATION: PASS

File Operations: MATCH (existing test/functional/p2p_leak_tx.py modified in both Bitcoin and Dash)

Size Ratio: ~100% (Bitcoin: ≈ same number of functional lines in run_test + new test_tx_in_block + refactored test_notfound_on_unannounced_tx; Dash: same structure and count, only txid/wtxid-specific adaptations)

Scope Creep: NONE (no extraneous refactors, no new helpers, no unrelated test logic, no added TODO/FIXME)

Bitcoin-Specific Code: NONE (no segwit/witness/RBF constructs; uses txid + MSG_TX in line with Dash’s non-witness architecture)

Completeness: COMPLETE (only the p2p_leak_tx functional test is touched, consistent with the upstream commit’s described scope)

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}