Skip to content

fix: wait for broadcast result - #113

Open
ovitrif wants to merge 35 commits into
mainfrom
codex/112-onchain-broadcast-result
Open

fix: wait for broadcast result#113
ovitrif wants to merge 35 commits into
mainfrom
codex/112-onchain-broadcast-result

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Closes #112

Summary

  • Wait for the configured backend result before explicit on-chain sends report acceptance.
  • Return transaction-keyed rejected, not-dispatched, acceptance-unknown failure, and acceptance-unknown timeout errors through Rust, Swift, Kotlin, and Python.
  • Persist every possibly dispatched transaction and its complete RBF lineage before broadcast, reserve its inputs across restart, and support exact-byte rebroadcast or explicit abandonment after independent reconciliation.
  • Expose durable Pending, Accepted, and Abandoned broadcast outcomes by any tracked lineage transaction ID.
  • Retain terminal outcomes without TTL until explicit consumer acknowledgement, including after restart, confirmation, payment-history cleanup, and abandonment.
  • Persist accepted and abandoned outcomes before removing their active intent; persistence failures stay queryable and fail closed.
  • Persist received-event delivery state so accepted and acceptance-unknown sends emit one recoverable OnchainTransactionReceived event across restart.
  • Serialize broadcast dispatch and abandonment, retain complete RBF lineage on predecessor re-entry, and defer cleanup until every participating wallet account is loaded.
  • Keep LDK-managed broadcasts fire-and-forget on a dedicated queue, coalesce identical live packages, and dispatch explicit backend requests independently.
  • Require backend evidence before reconciliation and classify Bitcoin Core, Electrum, and Esplora responses conservatively.
  • Publish the breaking binding contract as 0.7.0-rc.67.

Preview

N/A — no user-visible changes.

QA Notes

  • A deterministic backend rejection does not reach the consumer success path and releases the pending spend.
  • A timeout or operational failure after dispatch carries the transaction ID. Consumers must reconcile or call rebroadcast_transaction for that exact ID; they must not create a second payment.
  • broadcast_outcome accepts any lineage transaction ID and returns the canonical active or terminal transaction ID plus the complete lineage.
  • Only BroadcastOutcomeStatus::Accepted proves backend acceptance. Pending, an unknown result, and a query failure remain unresolved.
  • acknowledge_broadcast_outcome removes one root-keyed terminal record and every lineage alias after downstream handling is durable. It fails while the lineage remains active and is idempotent after removal.
  • list_pending_broadcasts returns every unresolved transaction and its full RBF lineage.
  • abandon_pending_broadcast is valid only after an independent source proves every lineage member absent from mempool and chain and no other process can rebroadcast it.
  • Swift unit NodeError cases no longer carry the legacy generated message value. The four broadcast-result cases carry txid.
  • Kotlin and Python fieldless exceptions have empty generated messages. The four broadcast-result cases carry txid.

Validation

  • Exact PR head: 6b60fe0df32362d6c6604393071110e94a2058b8; local HEAD, upstream, and PR head match.
  • Synced current main at 26664614696e82dc10e10f5c2042cc9ac1bb8f5e; the branch is zero commits behind and GitHub reports it mergeable.
  • cargo fmt completed before commit.
  • ./bindgen.sh generated Swift, Kotlin Android, Kotlin JVM, and Python sources and artifacts. Its required target compilation and built-in JVM/Android packaging completed.
  • Generated Swift archive checksum: 549f7a962f7b4262ef77b7e65abd6fe588be87801bee35da6295571c141f183b; Package.swift contains that value.
  • Source unit, integration, doc, lint, device, and consumer tests for this outcome-API delta were not run per owner direction.
  • Earlier source and consumer validation applies only through e1b0667 and does not certify this outcome-API delta.

Consumer contract

  • Android generated API: BroadcastOutcomeStatus.PENDING|ACCEPTED|ABANDONED, BroadcastOutcome(status, txid, lineage), broadcastOutcome(txid), and acknowledgeBroadcastOutcome(txid).
  • Swift generated API: BroadcastOutcomeStatus.pending|accepted|abandoned, BroadcastOutcome(status:txid:lineage:), broadcastOutcome(txid:), and acknowledgeBroadcastOutcome(txid:).
  • Android fix: require accepted onchain broadcasts bitkit-android#1225 and iOS fix: prevent false on-chain send success bitkit-ios#727 confirmed these exact names and semantics before generation.
  • Local Android artifact coordinates: com.synonym:ldk-node-android:0.7.0-rc.67.

Release

  • The previously generated rc.67 archive with checksum 07cf71e2a44ee901c0cb2c5d1062cc72fbb9fbbd56fb341a6ca1b531ec316219 does not contain this API and is superseded.
  • No tag or GitHub release was created for this head.

@chatgpt-codex-connector

This comment was marked as resolved.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif
ovitrif marked this pull request as draft September 2, 2026 08:47
@ovitrif
ovitrif marked this pull request as ready for review September 2, 2026 12:33
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif

This comment was marked as resolved.

@chatgpt-codex-connector

This comment was marked as resolved.

Comment thread src/chain/electrum.rs Outdated
Comment thread src/wallet/mod.rs
Comment thread src/wallet/mod.rs Outdated
Comment thread src/payment/onchain.rs Outdated
Comment thread src/wallet/mod.rs Outdated
@ovitrif
ovitrif requested a review from ben-kaufman September 2, 2026 21:50
Comment thread bindings/ldk_node.udl
Comment thread src/chain/mod.rs Outdated
Comment thread src/wallet/mod.rs Outdated
@ovitrif
ovitrif requested a review from ben-kaufman September 3, 2026 22:37
Comment thread src/wallet/mod.rs Outdated
@ovitrif
ovitrif requested a review from ben-kaufman September 3, 2026 23:01
Comment thread src/payment/onchain.rs Outdated
@ovitrif
ovitrif force-pushed the codex/112-onchain-broadcast-result branch from 3b1f49f to 2f6d7c8 Compare September 4, 2026 14:51
@ovitrif ovitrif mentioned this pull request Sep 4, 2026
@ovitrif
ovitrif force-pushed the codex/112-onchain-broadcast-result branch from 2f6d7c8 to 0bc67a9 Compare September 4, 2026 15:20
@ovitrif
ovitrif changed the base branch from main to chore/untrack-jni-drop-jvm September 4, 2026 15:20
@ovitrif ovitrif changed the title fix: wait for on-chain broadcast results fix: wait for broadcast results Sep 4, 2026
@ovitrif ovitrif changed the title fix: wait for broadcast results fix: wait for broadcast result Sep 4, 2026
@ovitrif ovitrif self-assigned this Sep 5, 2026
@ovitrif
ovitrif force-pushed the codex/112-onchain-broadcast-result branch from 0bc67a9 to 8e707af Compare September 5, 2026 01:57
@ovitrif
ovitrif force-pushed the codex/112-onchain-broadcast-result branch from 505b1a6 to 63b045f Compare September 7, 2026 14:29
@ovitrif
ovitrif requested a review from ben-kaufman September 7, 2026 14:31
Comment thread src/payment/onchain.rs Outdated
Comment thread src/wallet/mod.rs
@ben-kaufman

Copy link
Copy Markdown

Following up on the dispatch/abandonment issue in #113 (comment): shutdown still appears to release the dispatch lease before a running Electrum request finishes at 63b045f.

The stop branch in src/chain/mod.rs:1333–1346 returns and drops explicit_jobs, but the spawn_blocking RPC in src/chain/electrum.rs:850–853 can keep running. Dropping the result sender lets the caller return Failed and release its wallet lease while that RPC can still broadcast.

Could we retain the dispatch protection until the blocking RPC actually finishes, including during shutdown, and add a test that holds the RPC across stop and attempts abandonment? This is verified from the code path, not a runtime reproduction.

@ovitrif

ovitrif commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

@ben-kaufman The explicit request now carries a shared dispatch guard into the Electrum spawn_blocking closure. Canceling the worker can return an acceptance-unknown error to the caller, but abandonment remains blocked until the blocking RPC actually finishes. The regression test holds that backend guard across worker stop, verifies abandonment is rejected, then releases the backend and verifies abandonment can proceed.

@ovitrif

ovitrif commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips Follow-up on your review:

  • 63b045f fixed predecessor eviction, active-dispatch abandonment, persisted participant accounts, node-drop handling, RBF lineage re-entry, independent explicit dispatch, backend classification, restart-safe received events, timeout classification, Python migration notes, and LDK package coalescing.
  • 23652b2 closes the remaining shutdown gap by retaining the wallet dispatch lease in Electrum's blocking RPC. It also preserves txid-bearing outcomes across event-outbox failures and prevents late backend results from duplicating already-handled received events.
  • The legacy intent decoders remain intentionally because earlier rc.67 branch builds wrote those durable formats; retaining them keeps those existing test and consumer wallets recoverable through the final branch update.
  • Current-head focused tests and a fresh binding build passed. The XCFramework checksum is a5f64657c667232083b14aa658235683418f1282449344c3d7c48ed7dc396e65, and Package.swift matches.

@ovitrif
ovitrif requested a review from ben-kaufman September 7, 2026 16:15
Comment thread src/lib.rs Outdated
Comment thread src/tx_broadcaster.rs
Comment thread src/chain/mod.rs
Comment thread src/chain/mod.rs
Comment thread bindings/ldk_node.udl
@ovitrif
ovitrif requested a review from ben-kaufman September 7, 2026 23:36
Comment thread src/event.rs
@ovitrif
ovitrif requested a review from ben-kaufman September 8, 2026 13:04
@ovitrif
ovitrif requested a review from ben-kaufman September 8, 2026 16:43
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.

fix: On-chain send returns before broadcast result

3 participants