Skip to content

test: fix wait_for_instantlock calls in test_instantsend_after_restart#7252

Merged
PastaPastaPasta merged 1 commit intodashpay:developfrom
thepastaclaw:fix/p2p-instantsend-wait-api
Mar 24, 2026
Merged

test: fix wait_for_instantlock calls in test_instantsend_after_restart#7252
PastaPastaPasta merged 1 commit intodashpay:developfrom
thepastaclaw:fix/p2p-instantsend-wait-api

Conversation

@thepastaclaw
Copy link
Copy Markdown

Summary

Fix two broken wait_for_instantlock call sites in test_instantsend_after_restart that were missed during the merge of #7240 and #7241.

Problem

The merge of #7240 (added test_instantsend_after_restart with old-style calls) and #7241 (refactored wait_for_instantlock(txid, node)wait_for_instantlock(*txids, nodes=None)) left two call sites using the old positional signature:

for node in self.nodes:
    self.wait_for_instantlock(txid, node)  # node consumed as second *txid

The node object gets silently swallowed into *txids as a second "txid", causing getrawtransaction(node_object, True) to fail and the wait to time out after ~282 seconds. This breaks p2p_instantsend.py on every CI run.

Fix

Replace both loops with the new consolidated API which handles mempool sync and checks all nodes by default:

self.wait_for_instantlock(txid)

The merge of dashpay#7240 and dashpay#7241 left two call sites in
test_instantsend_after_restart using the old positional signature
wait_for_instantlock(txid, node). After dashpay#7241 changed the signature
to *txids with keyword-only nodes=, the node object was silently
consumed as a second txid, causing the wait to always time out.

Use the new consolidated API which handles mempool sync and checks
all nodes by default.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7fd8230e-12f3-42d0-bbd9-fa9c2f391c1a

📥 Commits

Reviewing files that changed from the base of the PR and between 3957e09 and c885b55.

📒 Files selected for processing (1)
  • test/functional/p2p_instantsend.py

Walkthrough

The change modifies the test_instantsend_after_restart test in the InstantSend test suite. Previously, the test explicitly advanced mock time by 30 blocks, synchronized mempools across nodes, and then iterated over each node to wait for InstantSend locks with explicit node arguments. The updated approach removes these explicit time advancement and synchronization steps, instead directly calling wait_for_instantlock() without node iteration, relying on default behavior for lock resolution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing wait_for_instantlock calls in a specific test function.
Description check ✅ Passed The description is directly related to the changeset, providing clear context about the problem (mismatched API signatures from merged PRs) and the fix applied.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

@github-actions
Copy link
Copy Markdown

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c885b55

@PastaPastaPasta PastaPastaPasta merged commit 2f1d59e into dashpay:develop Mar 24, 2026
42 of 43 checks passed
Copy link
Copy Markdown
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post-ACK c885b55

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants