fix(wallet): report authoritative L1 broadcast outcomes#4181
Closed
llbartekll wants to merge 1 commit into
Closed
Conversation
llbartekll
requested review from
QuantumExplorer,
ZocoLini,
lklimek and
shumkov
as code owners
July 21, 2026 12:31
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughBroadcasting now treats Dash Core acceptance as authoritative, reconciles uncertain submissions, preserves local transaction IDs for network outcomes, exposes dedicated rejection results through FFI and Swift, and updates the example send flow and wallet wiring. ChangesBroadcast authority and relay
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant SendViewModel
participant ManagedCoreWallet
participant DapiBroadcaster
participant SpvRuntime
SendViewModel->>ManagedCoreWallet: broadcastTransactionWithOutcome
ManagedCoreWallet->>DapiBroadcaster: submit transaction
DapiBroadcaster->>DapiBroadcaster: reconcile uncertain Core response
DapiBroadcaster->>SpvRuntime: relay after Core acceptance
DapiBroadcaster-->>ManagedCoreWallet: broadcast result and txid
ManagedCoreWallet-->>SendViewModel: accepted, rejected, or unknown outcome
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Collaborator
|
🔍 Review in progress — actively reviewing now (commit e03f870) |
Member
|
This is the wrong approach, this should be fixed in rust dash core. |
Member
|
Actually there are parts here that are useful, reopening. |
This was referenced Jul 22, 2026
Member
|
Superseded by #4198, which carries this PR's DAPI classification, getTransaction reconciliation, FFI/Swift outcome plumbing, and UTXO reservation handling forward on top of dashpay/rust-dashcore#913 — dash-spv now provides a trustless peer-echo acceptance signal (send to a subset of peers, a withheld peer announcing the txid back proves network acceptance), which #4198 uses as the fallback authority when DAPI is ambiguous or unreachable. Closing rather than rebasing since the SPV-side rework replaces this branch's core relay logic. |
bfoss765
pushed a commit
to bfoss765/platform
that referenced
this pull request
Jul 23, 2026
…allback Network acceptance, not a successful peer socket write, now decides what a broadcast reports. Two authorities establish it: - DAPI/Core (primary): single-attempt submission through DAPI's Core sendrawtransaction bridge, classified accepted / already-known / rejected / uncertain from the gRPC code, with uncertain submissions reconciled via getTransaction. - SPV peer echo (trustless fallback): when DAPI is ambiguous or unreachable, the wallet falls back to dash-spv's broadcast acceptance detection (dashpay/rust-dashcore#913) — the transaction is sent to a subset of SPV peers and a withheld peer announcing the txid back proves it propagated into network mempools. A p2p reject resolves Rejected; no signal within 30s resolves MaybeSent. Rejected releases the transaction's UTXO reservation for immediate retry; MaybeSent keeps it (double-spend guard). Accepted transactions are relayed through SPV best-effort so the local mempool pipeline sees them immediately; a relay failure never downgrades an authoritative accept. The outcome surfaces through FFI as a dedicated rejection code (26) with the txid preserved on all network outcomes, and in Swift as CoreTransactionBroadcastOutcome (accepted/rejected/unknown) via broadcastTransactionWithOutcome, so the example app can no longer report payment success without network acceptance. Bumps rust-dashcore to 58f207e5 (dashpay/rust-dashcore#913). Supersedes dashpay#4181; carries over its DAPI classification, getTransaction reconciliation, FFI/Swift outcome plumbing, and reservation reconciliation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ABI note
The current v4.1-dev branch already assigns code 22 to CoreInsufficientFunds and codes 23 through 25 to asset-lock errors. TransactionBroadcastRejected therefore uses the next free stable code, 26. TransactionBroadcastUnconfirmed remains code 20.
Tests
Summary by CodeRabbit
New Features
Bug Fixes