fix(platform-wallet): use dash-spv's own acceptance timeout instead of a 30s override

[Claudius-Maginificent]

TL;DR

SpvBroadcaster handed dash-spv a 30s deadline that expired before dash-spv's own 60s acceptance timeout even fired, throwing away the entire late-echo window the mechanism was built around. Pass None and defer to dash-spv's configured window (60s + 5s grace).

User story

As a wallet user sending a transaction, I want a send that the network actually accepted to be reported as accepted — not as "outcome unknown" — so my inputs aren't held hostage by the reservation TTL and I'm not left wondering whether my money moved.

Scenario

1. User sends a transaction through an SPV-backed platform wallet.
2. dash-spv dispatches it to a subset of peers, withholding it from the rest, and waits for a withheld peer to echo the txid back (or an InstantSend lock / confirmation).
3. The echo arrives at, say, 38s — well inside dash-spv's designed window.
4. Before: platform-wallet had already given up at 30s. The send surfaced as TransactionBroadcastUnconfirmed, and the reserved inputs stayed locked until the reservation TTL backstop cleaned up.
5. After: the wait runs to dash-spv's own deadline, the echo lands, and the send is correctly reported as Accepted.

Detailed discussion

packages/rs-platform-wallet/src/broadcaster.rs defined a local SPV_ACCEPTANCE_TIMEOUT = 30s and passed Some(SPV_ACCEPTANCE_TIMEOUT) into broadcast_transaction_and_wait. Its stated rationale was that 30s is "shorter than dash-spv's own default so a user-facing send does not hang for a full minute" — but that reasoning misses how the await loop actually works.

In dash-spv (dash-spv/src/client/transactions.rs, pinned rev a97b32c):

let wait = timeout.unwrap_or(config_timeout + AWAIT_GRACE);

with config_timeout = broadcast_acceptance_timeout (60s default) and AWAIT_GRACE = 5s. The decisive detail is in the loop itself:

Ok(Ok(SyncEvent::TransactionBroadcastResult { txid: event_txid, result }))
    if event_txid == txid && !matches!(result, BroadcastResult::Uncertain) => { return Ok(result) }

The mempool manager's interim Uncertain event is deliberately filtered out and does not end the wait — a late echo can still upgrade the verdict to Accepted, so the wait deliberately runs to the caller's deadline. This was an explicit design choice in dashpay/rust-dashcore#913, precisely so that callers willing to wait longer keep waiting rather than bailing early.

That makes the 30s override strictly lossy. It doesn't merely trade latency for certainty — it expires before the manager's own 60s timeout has even fired, so the only verdict platform-wallet could ever observe past 30s was the deadline-elapsed Uncertain it manufactured itself. Every acceptance signal in the 30–65s band was discarded unread.

The fix is to pass None, which is already the API's "use dash-spv's own default" expression (Option<Duration> → unwrap_or(config_timeout + AWAIT_GRACE)). The now-unused constant is removed, and the Uncertain message no longer names a duration it no longer controls.

Provenance

Found while investigating a dash-evo-tool TransactionBroadcastUnconfirmed report — transactions that the network had in fact accepted were being reported back to the user as unconfirmed.

Scope

Deliberately minimal: 1 file, +4/-15, no behavioural changes beyond the deadline. The stale asset-lock test assertions noticed nearby are not touched here — they're landing separately in #3549.

Testing

Existing broadcaster::tests::spv_broadcast_maps_dash_spv_verdicts passes; the verdict-mapping contract is unchanged by this fix (only the deadline handed to dash-spv changes, and the test's channel spy ignores the timeout argument). No new test added: meaningfully exercising the 30s-vs-65s window needs a live P2P peer echo, which isn't available in unit scope.

cargo test -p platform-wallet --lib broadcaster   # exit 0, 1 passed
cargo clippy -p platform-wallet --lib --all-features   # exit 0, clean

---

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Updated transaction broadcasting to use the platform’s default acceptance timeout.
  • Improved timeout-related error messaging when transaction acceptance cannot be confirmed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0bf26779-4b38-401e-9a46-1e88a1523a46

📥 Commits

Reviewing files that changed from the base of the PR and between 1e26927 and cfc5c10.

📒 Files selected for processing (1)

• packages/rs-platform-wallet/src/broadcaster.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The broadcaster no longer defines a 30-second SPV acceptance timeout. It delegates timeout selection to dash-spv and updates the MaybeSent reason to reference dash-spv's acceptance timeout.

Changes

SPV timeout handling

Layer / File(s)	Summary
Delegate acceptance timeout packages/rs-platform-wallet/src/broadcaster.rs	The broadcaster removes its 30-second timeout, passes None to broadcast_and_wait, and uses a static MaybeSent reason for missing acceptance signals.

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

Merge Risk: ⚪ Minimal · up to cfc5c

This narrowly adjusts transaction acceptance waiting to use the wallet's configured timeout; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: quantumexplorer, lklimek

🚥 Pre-merge checks | ✅ 4 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: it replaces the local 30-second override with dash-spv’s own acceptance timeout.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/platform-wallet-broadcast-acceptance-timeout

---

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.}

[thepastaclaw]

✅ Final review complete — no blockers (commit cfc5c10)

[thepastaclaw]

Final validation — Codex/Sol only (Phase 2 disabled)

The production fix is correct: passing None delegates the deadline to dash-spv, whose pinned implementation uses the configured acceptance timeout plus its grace period. The existing spy discards that argument, however, so the sole behavioral regression fixed by this PR remains untested and can be covered without a live peer.
Source: reviewer backends gpt-5.6-sol (codex-general and codex-rust-quality); final verifier backend gpt-5.6-sol; orchestration-only and not reviewer evidence: openclaw-agent/cliproxy/gpt-5.6-sol.

Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — rust-quality (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
• Secondary pass: disabled (temporary_phase2_sonnet_disable)

🟡 1 suggestion(s)

1 additional finding(s) omitted (not in diff).

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/broadcaster.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/broadcaster.rs:236-247: Assert that the broadcaster delegates timeout policy to dash-spv
  This PR's behavioral fix consists entirely of changing the timeout passed through `SpvChannel` from `Some(30s)` to `None`. The pinned dash-spv implementation interprets `None` as its configured acceptance timeout plus the grace period, but `AcceptanceSpy` currently discards `_timeout`; therefore, `spv_broadcast_maps_dash_spv_verdicts` passes with both the old buggy implementation and the corrected one. Assert that the spy receives `None` to lock this delegation contract directly—no live peer or 65-second timing test is needed.