Skip to content

Abandoning a failed hardware send orphans the Paykit payment proof #730

Description

@jvsena42

What happened?

Abandoning a hardware-wallet send after its broadcast fails leaves the incoming Paykit payment request accepted and its prepared on-chain proof orphaned. The payer sees nothing more; the requester waits for a payment that will never arrive.

The proof is prepared before the device is asked to sign, in beforeBroadcast (HwSendSignView.swift:114SendSheet.prepareHardwareContactPayment, SendSheet.swift:687). When the broadcast then fails on connectivity, the signed transaction is deliberately retained so Retry can reuse it without a second signature, so cancelContactPaymentIfBroadcastIsRetryable() (HwSendSignView.swift:144) correctly declines to cancel the proof — a retry still needs it.

Nothing cancels it on the way out, though. Leaving the screen runs hwSend.cancel() (SendSheet.swift:248), which discards the signed transaction, and app.contactPaymentContext = nil (SendSheet.swift:253), which drops the request handle. The prepared proof row stays behind.

PaykitPaymentProofService.reconcile() never recovers it. It only submits proofs that already carry proofData, and otherwise handles lightning alone:

guard proof.kind == PaykitPaymentProofKind.lightning, let paymentHash = proof.paymentIdentifier else { continue }

(PaykitPaymentProofService.swift:286) — an on-chain row with no paymentIdentifier and no proofData is skipped every time, so it persists indefinitely.

Expected behavior

Abandoning a hardware-wallet send whose transaction was never broadcast should return the payment request to an actionable state on both sides, rather than leaving it accepted with an orphaned proof.

Steps to Reproduce

  1. Enable Paykit on two devices and pair them as contacts.
  2. Pair a Trezor on the payer and fund its native-segwit account.
  3. On the payee: request a payment from the payer.
  4. On the payer: open the request, set the funding source to the Trezor, and swipe to pay.
  5. Approve every prompt on the device, cutting connectivity so the broadcast fails (stop the local electrs container, or drop Wi-Fi).
  6. Instead of retrying, leave via the back arrow or dismiss the sheet.
  7. The request stays accepted and the requester never sees a rejection; the pending proof row remains forever.

Additional context

  • Not introduced by fix: unlock send sheet after failed broadcast #729, but made common by it. The same orphan happened before, when force-quitting was the only way out of the broadcast lockout. fix: unlock send sheet after failed broadcast #729 makes the exit graceful, so this path is now ordinary rather than rare.
  • The naive fix is unsafe and was rejected during review of fix: unlock send sheet after failed broadcast #729. Cancelling the proof when the signed transaction is discarded deletes it for a transaction that may still confirm: .broadcastUncertain is a broadcast timeout, and BroadcastError+Connectivity.swift classifies timeout / network / dns as connectivity alongside "failed to connect to electrum:", so "definitely unsent" cannot currently be told apart from "may already be on the network". completeOnchainPayment then finds no pending row (PaykitPaymentProofService.swift:229) and the proof is lost with no way back.
  • Android has the safe shape. The sibling issue is synonymdev/bitkit-android#1227, whose fix (commit ae762113f on PR #1186) adds PaykitPaymentProofRepo.failOnchainPayment and routes the cancellation paths through a shared helper. Marking the payment failed rather than deleting the prepared proof rolls the request back without destroying evidence for a transaction that might still confirm. iOS has no failOnchainPayment equivalent; adding one is the suggested direction here.
  • Scope: only reachable with Paykit payment requests enabled and a hardware funding source. Regular sends unwind correctly via shouldCancelPaymentProof (SendConfirmationView.swift:616), which is unambiguous there because that path has no signed-but-unbroadcast middle state.
  • Found while reviewing fix: unlock send sheet after failed broadcast #729 for release readiness; raised there by the Codex reviewer and deliberately deferred rather than fixed unsafely in that PR.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions