Skip to content

fix(wallet-lib): fix broadcast retry not matching DAPI error message#3434

Merged
QuantumExplorer merged 1 commit into
v3.1-devfrom
fix/wallet-lib-broadcast-retry-message
Apr 3, 2026
Merged

fix(wallet-lib): fix broadcast retry not matching DAPI error message#3434
QuantumExplorer merged 1 commit into
v3.1-devfrom
fix/wallet-lib-broadcast-retry-message

Conversation

@QuantumExplorer
Copy link
Copy Markdown
Member

@QuantumExplorer QuantumExplorer commented Apr 3, 2026

Issue Being Fixed

Since ~March 16, 7 nightly test suite tests fail consistently with bad-txns-inputs-missingorspent. This has been tracked in NIGHTLY_STATUS.md.

Root cause

The wallet-lib has retry logic for UTXO conflicts in broadcastTransaction.js:181, but the error message check was wrong:

// What wallet-lib checked for:
error.message === 'invalid transaction: bad-txns-inputs-missingorspent'

// What DAPI actually returns (both JS and Rust DAPI):
'Transaction is rejected: bad-txns-inputs-missingorspent'

The retry never triggered. When the faucet wallet tried to fund multiple test wallets in quick succession, the second transaction would reference UTXOs still in the mempool from the first — Core rejects with bad-txns-inputs-missingorspent, and without the retry, the test fails immediately.

Failing tests (7 total, from 2 broadcastTransaction failures)

  1. Platform > Data Contract > "before all" hook — faucet funding fails
    2-7. e2e > Contacts > Alice/Bob — cascade from Alice's wallet funding failure

Investigation timeline

  • Last passing nightly: March 15 (run 23121472332)
  • First failing nightly: March 16 (run 23170189658)
  • The UTXO race condition was always latent; timing changes from commits landing March 15-16 made it consistently reproducible
  • The retry logic message mismatch predates the failure — it was never matching against the DAPI error format

What was changed

packages/wallet-lib/src/types/Account/methods/broadcastTransaction.js

- if (error.message === 'invalid transaction: bad-txns-inputs-missingorspent') {
+ if (error.message && error.message.includes('bad-txns-inputs-missingorspent')) {

Using .includes() matches regardless of the message prefix ("invalid transaction:", "Transaction is rejected:", or any future format). The error.message && guard prevents TypeError on errors without a message property.

NIGHTLY_STATUS.md — Updated the known issue section with correct failure count (7, not 2), correct root cause (faucet funding, not withdrawals), and a link to this PR.

Breaking Changes

None. The retry behavior is strictly more permissive (matches errors it previously missed).

Tests

The fix should resolve the 7 nightly test failures. The retry logic itself doesn't have a unit test in broadcastTransaction.spec.js.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved transaction retry logic to more reliably detect and handle certain broadcast failures, enabling proper retry behavior for transaction conflicts that previously failed to retry.

The wallet-lib retry logic for UTXO conflicts checked for an exact
match against 'invalid transaction: bad-txns-inputs-missingorspent',
but DAPI (both JS and Rust) returns 'Transaction is rejected:
bad-txns-inputs-missingorspent'. The retry never triggered, causing
faucet wallet funding to fail when UTXOs were still in the mempool.

This caused 7 nightly test failures since ~Mar 16: 1 Data Contract
funding failure + 6 cascading Contacts test failures.

Fix: use .includes('bad-txns-inputs-missingorspent') which matches
regardless of the message prefix.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added this to the v3.1.0 milestone Apr 3, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 3, 2026

📝 Walkthrough

Walkthrough

This PR updates documentation for a known test failure issue related to transaction broadcast errors and implements the corresponding code fix. The error handling logic is made more robust by changing from exact string matching to substring matching to properly trigger retry behavior.

Changes

Cohort / File(s) Summary
Documentation Update
NIGHTLY_STATUS.md
Updated known issue entry for bad-txns-inputs-missingorspent: adjusted test failure count from 2 to 7 failures, narrowed scope to Data Contract and Contacts groups, documented root cause of string mismatch in broadcastTransaction.js:181, and added reference to fix PR #3434.
Error Handling Fix
packages/wallet-lib/src/types/Account/methods/broadcastTransaction.js
Modified mempool-broadcast error condition to use substring matching (.includes()) instead of exact string matching, allowing retry logic to properly detect and handle bad-txns-inputs-missingorspent errors regardless of message format variations from DAPI.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A transaction once stumbled and fell,
Error messages not matching? We could tell!
Now .includes() catches the slip,
No more UTXO conflicts will trip,
Retry logic flows with a bound-to-succeed yell! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(wallet-lib): fix broadcast retry not matching DAPI error message' is specific, concise, and directly summarizes the main change: fixing a bug where broadcast retry logic fails to match DAPI error messages.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/wallet-lib-broadcast-retry-message

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.

@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented Apr 3, 2026

Review Gate

Commit: ee59fa07

  • Debounce: 2m ago (need 30m)

  • CI checks: checks still running (3 pending)

  • CodeRabbit review: comment found

  • Off-peak hours: off-peak (12:42 PM PT Friday)

  • Run review now (check to override)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@NIGHTLY_STATUS.md`:
- Around line 40-42: Update the inconsistent test summary by replacing the
outdated phrase "2 of 65 tests fail" with the current result "65 tests pass, 7
fail (1 Data Contract funding + 6 Contacts cascade)" so the line currently
showing "2 of 65 tests fail" matches the detailed report that now states "65
tests pass, 7 fail"; ensure the summary text exactly mirrors that detailed
status.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 14492381-a49f-46f3-819c-7e920c00c18d

📥 Commits

Reviewing files that changed from the base of the PR and between f131258 and ee59fa0.

📒 Files selected for processing (2)
  • NIGHTLY_STATUS.md
  • packages/wallet-lib/src/types/Account/methods/broadcastTransaction.js

Comment thread NIGHTLY_STATUS.md
Comment on lines +40 to +42
Seven tests fail because Core rejects faucet wallet funding transactions whose inputs are already in the mempool. The failures are in the Data Contract and Contacts test groups -- 1 `before all` hook failure cascades into 6 dependent Contacts tests.

- **63 tests pass**, 2 fail
- **65 tests pass**, 7 fail (1 Data Contract funding + 6 Contacts cascade)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Inconsistent test counts with line 21.

Line 21 still states "2 of 65 tests fail" while line 42 now says "65 tests pass, 7 fail". These should be reconciled to avoid confusion.

📝 Suggested fix for line 21
-| [Test Suite](https://github.com/dashpay/platform/actions/workflows/tests-test-suite.yml) (`test:suite`) | **Failing** | 2 of 65 tests fail with `bad-txns-inputs-missingorspent`. See [known issues](`#test-suite-bad-txns-inputs-missingorspent-since-mar-16`). |
+| [Test Suite](https://github.com/dashpay/platform/actions/workflows/tests-test-suite.yml) (`test:suite`) | **Failing** | 7 of 72 tests fail with `bad-txns-inputs-missingorspent`. See [known issues](`#test-suite-bad-txns-inputs-missingorspent-since-mar-16`). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@NIGHTLY_STATUS.md` around lines 40 - 42, Update the inconsistent test summary
by replacing the outdated phrase "2 of 65 tests fail" with the current result
"65 tests pass, 7 fail (1 Data Contract funding + 6 Contacts cascade)" so the
line currently showing "2 of 65 tests fail" matches the detailed report that now
states "65 tests pass, 7 fail"; ensure the summary text exactly mirrors that
detailed status.

@QuantumExplorer QuantumExplorer merged commit 4ae9054 into v3.1-dev Apr 3, 2026
14 checks passed
@QuantumExplorer QuantumExplorer deleted the fix/wallet-lib-broadcast-retry-message branch April 3, 2026 19:50
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.

2 participants