test: stabilize asset locks functional test

[PastaPastaPasta]

Issue being fixed or feature implemented

Fixes #7310.

feature_asset_locks.py could fail intermittently under high concurrency while waiting for a freshly mined llmq_test_platform quorum or while syncing mempools during an asset-unlock reorg path.

What was done?

• Wait for the actual mining node to have the non-null quorum commitment before mining the final-commitment block in mine_quorum().
• Allow generate_batch() to use block-only sync for the asset-unlock invalidation padding where mempool equality is not part of the assertion.
• Make asset-unlock request ID handling reusable and deterministic.
• Select an asset-unlock index that maps to the intended oldest active platform quorum instead of relying on a bounded probabilistic search.
• Fix the asset-unlock quorum-selection assertion to use the transaction's actual request ID.

How Has This Been Tested?

• Built from latest upstream/develop with make -j8.
• Reproduced the issue locally before the fix with 20 concurrent feature_asset_locks.py runs.
• Ran python3.9 test/functional/feature_asset_locks.py --tmpdir=/private/tmp/dash_assetlocks_fixed_solo2 --portseed=301.
• Ran 20 concurrent feature_asset_locks.py instances using seeds 100-119 with --timeout-factor=4; all 20 passed.

Breaking Changes

None.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone

This pull request was created by Codex.

[github-actions]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• feat: asset lock tx v2 with basic RPC support + tests #7294 - feat: asset lock tx v2 with basic RPC support + tests

Please coordinate with the authors of these PRs to avoid merge conflicts.

[thepastaclaw]

✅ Review complete (commit d9d7695)

[coderabbitai]

Walkthrough

This PR refactors functional test helper code for asset-unlock transactions and quorum commitment waiting. In feature_asset_locks.py, request-id computation is extracted into a reusable method, a new method selects an asset-unlock transaction targeting the oldest matching active quorum, generate_batch gains an optional sync callback, and several tests are updated to use these helpers instead of hard-coded assertions and manual retry loops. In test_framework.py, the minableCommitments scan logic is extracted into node_has_quorum_commitment, used by wait_for_quorum_commitment and mine_quorum.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test as AssetLocksTest
  participant Helper as create_assetunlock_for_oldest_quorum
  participant Node as selectquorum

  Test->>Helper: start_index, withdrawal, pubkey
  loop candidate indices
    Helper->>Helper: create_assetunlock_request_id(index)
    Helper->>Node: selectquorum(request_id)
    Node-->>Helper: quorum hash
    Helper->>Helper: compare with expected quorum
  end
  Helper-->>Test: tx, payload, quorumHash

Loading

sequenceDiagram
  participant MineQuorum as mine_quorum
  participant WaitUntil as wait_until
  participant Node0 as nodes[0]
  participant Helper as node_has_quorum_commitment

  MineQuorum->>WaitUntil: wait for final commitment
  WaitUntil->>Helper: node_has_quorum_commitment(nodes[0], quorumHash, llmqType)
  Helper->>Node0: check minableCommitments
  Node0-->>Helper: commitment data
  Helper-->>WaitUntil: True/False
  WaitUntil-->>MineQuorum: commitment confirmed

Loading

Suggested reviewers: kwvg

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: stabilizing the asset locks functional test.
Description check	✅ Passed	The description matches the changeset and describes the test stability fixes accurately.
Linked Issues check	✅ Passed	The changes address #7310 by removing quorum-commitment timing flakiness and improving asset-lock test determinism.
Out of Scope Changes check	✅ Passed	The diff stays within test-stability work for asset locks and shared test helpers, with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

test/functional/test_framework/test_framework.py (1)

2205-2207: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Hardcoded timeout=15 diverges from surrounding parameterized waits.

Every other wait_for_* helper in this class accepts a configurable timeout (and mine_quorum itself doesn't expose one either, so this is consistent with the file's overall lack of a mine_quorum timeout param). Still, baking 15 directly into this new wait_until call makes it harder to tune independently of wait_for_quorum_commitment's existing timeout=15 default if the two ever need to diverge.

♻️ Optional: extract a local timeout variable

-        self.log.info("Waiting final commitment on mining node")
-        self.wait_until(lambda: self.node_has_quorum_commitment(self.nodes[0], q, llmq_type), timeout=15)
+        self.log.info("Waiting final commitment on mining node")
+        self.wait_until(lambda: self.node_has_quorum_commitment(self.nodes[0], q, llmq_type), timeout=15)  # keep in sync with wait_for_quorum_commitment default timeout

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/functional/test_framework/test_framework.py` around lines 2205 - 2207,
The new wait in mine_quorum uses a hardcoded timeout that should stay
configurable like the other wait helpers in TestFramework. Update the final
commitment wait around node_has_quorum_commitment to use a local timeout
variable or pass through an existing parameter pattern, and keep it aligned with
wait_for_quorum_commitment so the two can diverge later if needed. Use the
mine_quorum and wait_for_quorum_commitment symbols to locate the change.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/functional/test_framework/test_framework.py`:
- Around line 2205-2207: The new wait in mine_quorum uses a hardcoded timeout
that should stay configurable like the other wait helpers in TestFramework.
Update the final commitment wait around node_has_quorum_commitment to use a
local timeout variable or pass through an existing parameter pattern, and keep
it aligned with wait_for_quorum_commitment so the two can diverge later if
needed. Use the mine_quorum and wait_for_quorum_commitment symbols to locate the
change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f1407c66-146c-44bd-8811-e5b05e334653

📥 Commits

Reviewing files that changed from the base of the PR and between 4af395d and d9d7695.

📒 Files selected for processing (2)

• test/functional/feature_asset_locks.py
• test/functional/test_framework/test_framework.py

[thepastaclaw]

Code Review

Small, well-scoped test stabilization PR limited to two functional-test files. Adds an explicit mining-node commitment wait to close a race in mine_quorum, threads an optional sync_fun through generate_batch, and factors out oldest-quorum asset-unlock selection with a bounded (100-index) deterministic search. All changes preserve semantics and target real intermittency.

Source: reviewers: claude-general (opus), codex-general (gpt-5.5), claude-dash-core-commit-history (opus), codex-dash-core-commit-history (gpt-5.5); verifier: claude (opus).