fix: retain exact fees for payment recovery

[ben-kaufman]

This PR retains the exact prepared fee for each Bitcoin retry candidate so a positively accepted replacement can finish local recovery while wallet history catches up.

Description

• Persists the prepared fee with its transaction before dispatch and uses only the proven winner's fee for local activity and follow-up.
• Preserves unknown-outcome, original-input, wallet/identity and write-failure guards; canceled unsubmitted candidates lose only their own metadata.
• Allows identical backup replay to preserve local fee evidence without importing fee metadata or accepting changes to the original operation, candidates or progress.

Ordinary iOS sends already retain their original fee. This fixes the narrower replacement-recovery gap; Android's matching original/replacement fix is synonymdev/bitkit-android#1459.

Out of Scope

• Shared backup schema, SDK/native APIs, dependencies, Homeserver, Noise and safety waits: unchanged. Reset/cross-device restores without local evidence retain the existing wallet-history fallback.
• Payment acceptance and remote proof delivery: unchanged. Knowing a fee never proves payment acceptance.
• Performance PR fix: refresh Paykit views and local completion #914 and minimum-amount PR fix: validate minimum paykit request amounts #913 remain separate. This is not a confirmed cause of James's report, an Electrum fix or a measured latency improvement.

Design

N/A - no UI changes.

Preview

N/A - recovery bookkeeping only.

QA Notes

Journeys

• updated onchain-original-payment-retry.xml - retains the original guarded operation through retry; its description links to journeys/send/prepared-fee-recovery.md, the same four manual fault cases carried on Android #1459.

Manual Tests

• In a disposable funded regtest fixture, delay wallet history after an accepted replacement and restart before local follow-up -> Sent details use that winner's prepared fee without another send; native history fault injection is not in Capabilities.
• Exercise an unknown outcome, original-winner race, failed receipt save and identical backup replay -> unresolved payments remain guarded, only the winner's fee is used, persistence failure prevents dispatch and local fee evidence survives replay; controlled native outcomes and storage faults are not in Capabilities.

Automated Checks

• added OnchainSendAttemptServiceTests.swift - accepted successor retains its exact fee through JSON restart without transaction history; conflicting fees for one transaction fail closed.
• updated OnchainSendAttemptServiceTests.swift - unknown outcomes, original-winner selection, pre-dispatch persistence, absent-fee fallback and canceled candidate cleanup remain guarded.
• updated PaykitPaymentStateBackupTests.swift - identical replay preserves local fees without importing new fee metadata; changed candidates/progress remain rejected.
• updated TransferServiceActivityTests.swift - prepared original/replacement fixtures provide consistent exact fees for winner-specific activity details.

Final local validation on master 4e0c8ba5: 180 focused native tests and all 2,365 CI-selected unit tests passed, with the six unchanged repository integration-suite exclusions. Two tests added, none removed. Complete reports retain all 102 formatting findings and 1,898 translation warnings from base with no introduced findings. The focused rebuild's 49 compiler records map to existing baseline contexts; the final full-suite incremental build has no diagnostics. All three requested hardening skills and one fresh frozen combined correctness/quality review completed clean.

The author validation above did not run a controlled funded/native journey. Piotr subsequently reported bounded iOS device results in review 5488752990: original/replacement exact-fee recovery with controlled history delay and service recreation, response-loss Pending across restart, normal retry/PIN cancellation and authorization, pre-dispatch receipt-save failure, missing-fee fallback, and selected backup/guard cases. These are reviewer-reported results, not a new run by this documentation follow-up.

The full manual checklist remains incomplete: interruption after replacement acceptance before local follow-up, a genuinely confirmed original-winner race, actual wallet/Pubky-payer switching during authorization, and the full altered-candidate/progress replay matrix were not exercised on device. Live Shop proof delivery and full VSS/fresh-device restore were not exercised. The normal Pending/PIN journey used a disclosed same-installation disposable fixture-state transfer. This is not Android coverage or a full device-payment/merchant-unlock sign-off.

The shared manual recipe and cross-platform README notes now match Android #1459. This follow-up changes only documentation; source/test hashes are unchanged. Complete formatting and translation reports exactly match the prior verified baseline.

[greptile-apps]

Confidence Score: 5/5

[Critical impact] This PR appears safe to merge; no actionable issues were found.

Summary

This PR saves each prepared Bitcoin transaction’s exact fee before dispatch. Accepted recovery uses the winning transaction’s fee without waiting for wallet history.

• Accepted retries keep their exact fee for Sent details.
• Matching backup replays preserve local fee evidence.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Prepare transaction and exact fee] --> B[Save candidate and fee locally]
  B --> C[Authorize and dispatch]
  C --> D{Payment accepted?}
  D -->|No or unknown| E[Keep payment guarded]
  D -->|Yes| F[Select accepted transaction]
  F --> G{Local fee available?}
  G -->|Yes| H[Use winning prepared fee]
  G -->|No| I[Read fee from wallet history]
  H --> J[Save local activity]
  I --> J
  J --> K[Mark local follow-up complete]

Loading

_{Reviews (1) · Last reviewed commit: "chore: rename changelog fragment" · Reviewed by Greptile}

[talosmachina]

No findings. Stores each retry candidate's exact prepared fee in the recovery context before dispatch, prefers it over wallet history in winningFollowupAttempt, and ignores the local-only field when comparing a backup replay. Reviewed 6973637, full tier on the changed paths. Not built here: iOS does not build on this box, so this is reasoned from the code and from CI.

What I checked, and 5 candidates I ruled out

Read: the diff whole; in OnchainSendAttemptService.swift, the initial send receipt write, retrySamePayment, discardUnsubmittedRetryCandidate, restoreBackup, winningFollowupAttempt, OnchainSendRecoveryContext; PaykitPaymentStateBackup.ActiveOnchainAttempt.init.
Call sites traced: miningFeeSats producers (the prepareOnchainSend wrapper reads it from the same prepared tx as txid); every recoveryContext equality check.
CI: validate, detect-changes green; Run Tests, Run Integration Tests and build-local still pending at review time.

Ruled out

• Original winner fee changes: for the original txid the retained fee is the same prepared.miningFeeSats already written to followupContext.feeSats, so the result matches the old branch.
• Successor inheriting the original's fee: lookup is by the winner's lowercased txid; no fallback to another candidate.
• retrySamePayment's attempt.recoveryContext == recovery guard: both sides are read from the same stored record, so the new field cannot make it differ.
• Backup replay: the backup wire format has no fee field, restoreBackup nils it on both sides only for the comparison, and on match returns without saving, so local evidence survives; any other difference still throws.
• Older records: the field is an optional Codable member, so existing keychain JSON decodes with nil and falls back to winningFee.

Merge confidence: 4/5, no findings, but the unit and integration test jobs had not finished and the change could not be built here.

[piotr-iohk]

QA review

Scope: Reviewed the full PR diff against its merge base, at 6973637.

No new actionable code findings.

Each prepared candidate's mining fee is stored on the local recovery record before dispatch, and local follow-up uses only the accepted transaction's retained fee. A missing successor fee does not borrow the original fee, a conflicting fee for one transaction fails closed before broadcast, and canceled unsubmitted candidates drop only their own fee. Identical backup replay keeps local fee evidence and does not import fee metadata; a reset or cross-device restore still has no shared fee field and uses the existing wallet-history fallback. Payment acceptance is unchanged.

The same contract matches bitkit-android#1459 at 10e00654: per-candidate fees, fail-closed conflicts, local replay, and no backup import. That was a source comparison of this flow, not a full Android review.

Validation: unit-tests and integration-tests succeeded on this commit. This review inspected those tests and did not execute them (no iOS toolchain). The passing e2e run does not cover the history-delay or storage-fault path; the author's two manual cases remain the runtime check and were not run.

Device testing: not performed in this review.

Ready for device testing.

[jvsena42]

No HIGH or MEDIUM finding at 6973637. Approving with 1 LOW open (inline, on the journey file). It can be fixed here or moved to a follow-up issue, your choice; please reply on the thread saying which, with the issue link if it is a follow-up.

Checked and clean:

• Approved fee = broadcast fee = recorded fee. SendPendingScreen shows prepared.miningFeeSats of the same PreparedOnchainSendDispatch whose closure broadcasts it; candidateFeeSats[prepared.txid] is stored from that receipt before approvePrepared/authorize, and winningFollowupAttempt reads it back by the winner's lowercased txid. The original's fallback is its own followupContext.feeSats, never another candidate's entry, so an original winner cannot take a replacement's fee.
• Staleness. The fee is keyed by txid, and a txid commits to its inputs and outputs, so a fee-rate, UTXO or RBF change produces a new txid with its own entry. A second, different fee for the same txid fails closed as unresolved before store.save, with no broadcast (testRetainedCandidateRejectsConflictingPreparedFee).
• Persisted format. candidateFeeSats: [String: UInt64]? = nil with synthesized Codable, so keychain JSON written by master decodes to nil and falls through to the previous lookup. The Paykit backup wire has no fee field and restored() builds the context without it. v2.5.0 has no OnchainSendAttemptService at all, so there is no installed base for this state.
• Backup replay. restoreBackup strips candidateFeeSats on local copies only and returns without saving on a match, so local fee evidence survives an identical replay; any other difference still throws unresolved.
• Cancel and Back. Task.checkCancellation in retrySamePayment and the CancellationErrors thrown by SendPendingScreen all land in discardUnsubmittedRetryCandidate, which removes the candidate's txid, fee rate and fee when it was added by this retry. A pre-broadcast PreparedOnchainSendNotSubmitted takes the same path. In send(), a cancel in beforeBroadcastAttempt clears the whole pending attempt.
• Reentrancy. winningFollowupAttempt awaits winningFee only when no retained fee exists, and re-reads and re-checks the attempt (id, wallet, status, txid, follow-up flag) after the await.
• A retained fee of 0 would now be used where it previously fell through to history, but a real PreparedOnchainSend cannot have one: the adapter floors the fee rate at 253 sat/kwu.

CI is green at this head.

⚠️ Device gate: not run — onchain-original-payment-retry.xml needs a retained unresolved prepared-send receipt and a controlled wallet-history delay after an accepted retry (native fault injection), which is not available on my setup and is not in the Capabilities table. The approval rests on the code scan, the unit tests and CI; the journey and both Manual Tests are still unrun.

⚠️ perf: not measured — needs the same delayed-history fixture. The PR removes a wait (Sent details no longer block on Electrum history when the fee is retained); that claim rests on reading the code.

[piotr-iohk]

Approved for iOS at 6973637a71f54489f3ffd8eae23e0127fc08a972.

Device validation on a disposable iOS simulator wallet against staging regtest:

• Original accepted native send: 1,000 sat recipient amount and exact 143 sat fee. Real Keychain receipt survived service recreation; local activity used the retained fee with history withheld, and repeated follow-up did not resend.
• Accepted native same-input replacement: retained its exact 703 sat fee instead of the original 143 sat fee across service recreation with history unavailable.
• Unknown outcome and Pending restart: an actual dropped broadcast response left the original guarded. Full normal-app stop/relaunch retained Pending and the original transaction.
• Real retry authorization: valid fee selection displayed 1,000 sat + 703 sat fee, followed by the payment PIN before dispatch. Canceling PIN forwarded nothing. Reopening and authorizing produced one accepted same-input replacement, independently verified against backend transaction data.
• Post-retry restart: one activity row retained the correct amount/fee after full normal-app restart, with one forwarded broadcast and no duplicate payment.
• Prepared-receipt save failure: an injected storage failure prevented dispatch.
• Missing-fee fallback: withheld history kept local follow-up guarded; exposing real native history recovered the exact 703 sat fee.
• Unrelated request context was rejected. Identical local recovery replay preserved fee evidence, while an altered original amount was rejected.

Coverage limits: native checks used controlled history/storage faults with the real native sender, Keychain and SQLite. The normal-app Pending/PIN journey used a disclosed private transfer of disposable fixture state within the same installation.

Still unverified on device: interruption specifically after replacement acceptance but before local follow-up; a genuinely confirmed original-winner race; an actual wallet/Pubky payer switch during authentication; and the full altered-candidate/progress backup replay matrix. These recovery areas have unit coverage, but that does not replace the missing device evidence. Live Shop proof delivery and full VSS/fresh-device restore were not exercised.

No confirmed iOS product defect was established. The full paired device plan remains incomplete; this approval records the iOS evidence above and does not claim an Android payment-test pass.

[piotr-iohk]

QA review

Scope: Follow-up review of the documentation delta since 6973637, at f56b0fb. Inherited coverage is the completed baseline QA review: on-chain send, retry, unsubmitted-candidate discard, and winner-fee selection; backup replay that ignores local candidate fees and does not import them; ordinary, Paykit-request, and transfer follow-up; unit tests for restart, conflicting fees, canceled candidates, and backup replay; and the Android prepared-fee recovery contract at 10e00654. This pass re-read the journey description, the new shared recipe, and the README parity row.

No new actionable code findings.

The new journeys/send/prepared-fee-recovery.md matches the unchanged recovery behavior: the prepared fee is stored before dispatch, follow-up uses the accepted transaction's retained fee, a failed receipt save returns before broadcast, and an identical unresolved backup replay keeps local fees. The file is byte-identical to bitkit-android#1459 at 46a9a9de. That check covers the shared manual recipe only. The journey-file comment is addressed at this revision: both platforms point at the same four cases, including receipt-save failure and wallet or payer switching, and both READMEs record the row (discussion).

Validation: this review did not execute the unit tests or a simulator. unit-tests and integration-tests succeeded on 6973637, and the Swift and test blobs are unchanged here. unit-tests and integration-tests were still in progress on f56b0fb.

Device testing: not performed in this review. The device notes in review 5488752990 apply to 6973637.