From 0de4d686e5591878969fe9dac215620870ef94fe Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 11 Oct 2026 00:11:16 -0500 Subject: [PATCH 1/3] fix: retain exact fees for payment recovery --- .../Services/OnchainSendAttemptService.swift | 25 +++++++-- .../OnchainSendAttemptServiceTests.swift | 51 +++++++++++++++++++ .../PaykitPaymentStateBackupTests.swift | 21 ++++++++ .../TransferServiceActivityTests.swift | 2 + changelog.d/next/prepared-fees.fixed.md | 1 + .../send/onchain-original-payment-retry.xml | 5 +- 6 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 changelog.d/next/prepared-fees.fixed.md diff --git a/Bitkit/Services/OnchainSendAttemptService.swift b/Bitkit/Services/OnchainSendAttemptService.swift index d2c6a3e82..a763b9134 100644 --- a/Bitkit/Services/OnchainSendAttemptService.swift +++ b/Bitkit/Services/OnchainSendAttemptService.swift @@ -85,6 +85,7 @@ struct OnchainSendRecoveryContext: Codable, Equatable { let paymentIdentity: String? var candidateTxids: [String] var candidateFeeRates: [String: UInt32]? = nil + var candidateFeeSats: [String: UInt64]? = nil func feeRate(for txid: String) -> UInt32? { if let rate = candidateFeeRates?[txid.lowercased()] { @@ -367,9 +368,13 @@ actor OnchainSendAttemptService { func restoreBackup(_ attempt: OnchainSendAttempt?) throws { guard nativeDispatchInProgress == nil else { throw OnchainSendAttemptError.unresolved } - if let previous = try currentAttempt(), previous.blocksNewSend { + if var previous = try currentAttempt(), previous.blocksNewSend { // A restore must not erase a newer unresolved candidate or another original operation. - guard let attempt, previous == attempt else { throw OnchainSendAttemptError.unresolved } + guard var incoming = attempt else { throw OnchainSendAttemptError.unresolved } + // Exact fee evidence is local and absent from the shared backup record. + previous.recoveryContext?.candidateFeeSats = nil + incoming.recoveryContext?.candidateFeeSats = nil + guard previous == incoming else { throw OnchainSendAttemptError.unresolved } return } try store.save(attempt.map { [$0] } ?? []) @@ -445,7 +450,8 @@ actor OnchainSendAttemptService { } attempt.recoveryContext = OnchainSendRecoveryContext( inputs: prepared.inputs, satsPerVbyte: satsPerVbyte, paymentIdentity: paymentIdentity, - candidateTxids: [prepared.txid], candidateFeeRates: [prepared.txid.lowercased(): satsPerVbyte] + candidateTxids: [prepared.txid], candidateFeeRates: [prepared.txid.lowercased(): satsPerVbyte], + candidateFeeSats: prepared.miningFeeSats.map { [prepared.txid.lowercased(): $0] } ) try store.save([attempt]) knownAttempt = attempt @@ -556,6 +562,15 @@ actor OnchainSendAttemptService { } } attempt.recoveryContext?.candidateFeeRates?[prepared.txid.lowercased()] = authorizedFeeRate + if let fee = prepared.miningFeeSats { + if let retained = attempt.recoveryContext?.candidateFeeSats?[prepared.txid.lowercased()], retained != fee { + throw OnchainSendAttemptError.unresolved + } + if attempt.recoveryContext?.candidateFeeSats == nil { + attempt.recoveryContext?.candidateFeeSats = [:] + } + attempt.recoveryContext?.candidateFeeSats?[prepared.txid.lowercased()] = fee + } // Keep the original rejected/unknown state until a real new outcome arrives. // A crash here still recognizes both possible payments and cannot unlock the wallet. try store.save([attempt]) @@ -614,6 +629,7 @@ actor OnchainSendAttemptService { else { return } recovery.candidateTxids.removeAll { $0.caseInsensitiveCompare(txid) == .orderedSame } recovery.candidateFeeRates?.removeValue(forKey: txid.lowercased()) + recovery.candidateFeeSats?.removeValue(forKey: txid.lowercased()) current.recoveryContext = recovery try store.save([current]) knownAttempt = current @@ -797,7 +813,8 @@ actor OnchainSendAttemptService { throw OnchainSendAttemptError.localFollowupNotSaved } let isOriginal = recovery.candidateTxids.first?.caseInsensitiveCompare(txid) == .orderedSame - let observed = isOriginal && context.feeSats > 0 ? context.feeSats : try await winningFee(original) + let retainedFee = recovery.candidateFeeSats?[txid.lowercased()] ?? (isOriginal && context.feeSats > 0 ? context.feeSats : nil) + let observed: UInt64? = if let retainedFee { retainedFee } else { try await winningFee(original) } guard let fee = observed, !original.amountSats.addingReportingOverflow(fee).overflow, var attempt = try currentAttempt(), attempt.id == original.id, diff --git a/BitkitTests/OnchainSendAttemptServiceTests.swift b/BitkitTests/OnchainSendAttemptServiceTests.swift index 6444913cd..52abbf030 100644 --- a/BitkitTests/OnchainSendAttemptServiceTests.swift +++ b/BitkitTests/OnchainSendAttemptServiceTests.swift @@ -328,6 +328,7 @@ final class OnchainSendAttemptServiceTests: XCTestCase { followupContext: .init(feeSats: 143, feeRate: 1, tags: [], contact: nil, createdAt: 123)) let original = try XCTUnwrap(store.snapshot().first) sender.txid = String(repeating: "cd", count: 32) + sender.miningFee = nil sender.result = .accepted(txid: sender.txid) _ = try await service.retrySamePayment(using: sender, context: .init(attemptId: original.id, walletId: original.walletId, txid: original.txid), @@ -339,6 +340,32 @@ final class OnchainSendAttemptServiceTests: XCTestCase { XCTAssertEqual(store.snapshot().first?.localFollowupComplete, false) } + func testAcceptedSuccessorUsesPreparedFeeAfterRestartWithoutWalletHistory() async throws { + let store = MemoryAttemptStore() + let service = OnchainSendAttemptService(store: store) + let sender = PreparedAttemptNodeMock() + sender.miningFee = 143 + _ = try await service.send(using: sender, address: "original", amountSats: sender.amount, + satsPerVbyte: 1, utxosToSpend: nil, isMaxAmount: false, + followupContext: .init(feeSats: 0, feeRate: 1, tags: [], contact: nil, createdAt: 123)) + let original = try XCTUnwrap(store.snapshot().first) + sender.txid = String(repeating: "cd", count: 32) + sender.miningFee = 281 + sender.result = .accepted(txid: sender.txid) + _ = try await service.retrySamePayment(using: sender, + context: .init(attemptId: original.id, walletId: original.walletId, txid: original.txid), + satsPerVbyte: 2, authorize: { _, _ in }) + let restored = MemoryAttemptStore() + try restored.save(JSONDecoder().decode([OnchainSendAttempt].self, from: JSONEncoder().encode(store.snapshot()))) + let followup = CapturingWinningFeeFollowup() + let restarted = OnchainSendAttemptService(store: restored, localFollowup: followup, winningFee: { _ in nil }) + do { _ = try await restarted.resumeAcceptedOrdinarySend(walletId: original.walletId) } catch {} + XCTAssertEqual(followup.attempt?.txid, sender.txid) + XCTAssertEqual(followup.attempt?.followupContext?.feeSats, 281) + XCTAssertEqual(followup.attempt?.followupContext?.feeRate, 2) + XCTAssertEqual(restored.snapshot().first?.blocksNewSend, true, "Failed activity persistence must remain guarded") + } + func testWinningFeeCalculationUsesExactPreviousOutputAndRejectsMissingOrInvalidDetails() throws { let parentId = String(repeating: "ef", count: 32) let details = LDKNode.TransactionDetails(amountSats: -1000, @@ -420,6 +447,7 @@ final class OnchainSendAttemptServiceTests: XCTestCase { let original = try XCTUnwrap(store.snapshot().first) let originalTxid = try XCTUnwrap(original.txid) sender.txid = String(repeating: "cd", count: 32) + sender.miningFee = 281 _ = try await service.retrySamePayment(using: sender, context: .init(attemptId: original.id, walletId: original.walletId, txid: originalTxid), satsPerVbyte: 2, authorize: { _, _ in }) @@ -441,6 +469,7 @@ final class OnchainSendAttemptServiceTests: XCTestCase { followupContext: .init(feeSats: 143, feeRate: 1, tags: [], contact: nil, createdAt: 123)) let original = try XCTUnwrap(store.snapshot().first) sender.txid = String(repeating: "cd", count: 32) + sender.miningFee = nil sender.result = .accepted(txid: sender.txid) _ = try await service.retrySamePayment(using: sender, context: .init(attemptId: original.id, walletId: original.walletId, txid: original.txid), @@ -469,12 +498,33 @@ final class OnchainSendAttemptServiceTests: XCTestCase { XCTAssertEqual(prepared.recipientAmountSats, original.amountSats) XCTAssertEqual(prepared.txid, sender.txid) XCTAssertEqual(sender.broadcasts, 1) + XCTAssertEqual(store.snapshot().first?.recoveryContext?.candidateFeeSats?[prepared.txid], 800) approved = true }, authorize: { _, _ in XCTAssertTrue(approved) } ) XCTAssertEqual(sender.preparations, 2) XCTAssertEqual(sender.broadcasts, 2) XCTAssertEqual(store.snapshot().first?.txid, sender.txid) + XCTAssertEqual(store.snapshot().first?.status, .unknown) + XCTAssertEqual(store.snapshot().first?.blocksNewSend, true) + } + + func testRetainedCandidateRejectsConflictingPreparedFee() async throws { + let store = MemoryAttemptStore() + let service = OnchainSendAttemptService(store: store) + let sender = PreparedAttemptNodeMock() + _ = try await service.send(using: sender, address: "original", amountSats: sender.amount, + satsPerVbyte: 1, utxosToSpend: nil, isMaxAmount: false) + let original = try XCTUnwrap(store.snapshot().first) + sender.miningFee = 700 + do { + _ = try await service.retrySamePayment(using: sender, + context: .init(attemptId: original.id, walletId: original.walletId, txid: original.txid), + authorize: { _, _ in XCTFail("Conflicting receipt must not reach authorization") }) + XCTFail("A transaction ID cannot acquire another mining fee") + } catch OnchainSendAttemptError.unresolved {} + XCTAssertEqual(sender.broadcasts, 1) + XCTAssertEqual(store.snapshot(), [original]) } func testCancelledFeeApprovalDoesNotSubmitOrRetainNewCandidate() async throws { @@ -710,6 +760,7 @@ final class OnchainSendAttemptServiceTests: XCTestCase { XCTAssertEqual(retained.recoveryContext?.inputs, original.recoveryContext?.inputs) XCTAssertEqual(retained.recoveryContext?.candidateTxids, [originalTxid]) XCTAssertNil(retained.recoveryContext?.candidateFeeRates?[sender.txid]) + XCTAssertNil(retained.recoveryContext?.candidateFeeSats?[sender.txid]) XCTAssertEqual(sender.broadcasts, 1) XCTAssertTrue(retained.blocksNewSend) let restarted = OnchainSendAttemptService(store: store) diff --git a/BitkitTests/PaykitPaymentStateBackupTests.swift b/BitkitTests/PaykitPaymentStateBackupTests.swift index d5a7f4c29..5edc13514 100644 --- a/BitkitTests/PaykitPaymentStateBackupTests.swift +++ b/BitkitTests/PaykitPaymentStateBackupTests.swift @@ -455,6 +455,27 @@ final class PaykitPaymentStateBackupTests: XCTestCase { XCTAssertEqual(store.snapshot(), [existing]) do { try await service.restoreBackup(nil); XCTFail("Absent backup guard erased unresolved local operation") } catch {} XCTAssertEqual(store.snapshot(), [existing]) + + var local = restored + let candidate = try XCTUnwrap(local.recoveryContext?.candidateTxids.last) + local.recoveryContext?.candidateFeeSats = [candidate: 281] + try store.save([local]) + let replay = OnchainSendAttemptService(store: store) + try await replay.restoreBackup(restored) + XCTAssertEqual(store.snapshot(), [local], "Identical backup replay must retain local fee evidence") + var incoming = restored + incoming.recoveryContext?.candidateFeeSats = [candidate: 999] + try await replay.restoreBackup(incoming) + XCTAssertEqual(store.snapshot(), [local], "Backup replay must not import fee metadata") + + var differentCandidates = restored + differentCandidates.recoveryContext?.candidateTxids.append(String(repeating: "ef", count: 32)) + var differentProgress = restored + differentProgress.localFollowupComplete.toggle() + for changed in [differentCandidates, differentProgress] { + do { try await replay.restoreBackup(changed); XCTFail("Restore replaced different candidate or progress state") } catch {} + XCTAssertEqual(store.snapshot(), [local]) + } } func testWalletEnvelopeRetainsActiveAttemptWireAcrossDecodeAndEncode() throws { diff --git a/BitkitTests/TransferServiceActivityTests.swift b/BitkitTests/TransferServiceActivityTests.swift index a8369829f..addb797af 100644 --- a/BitkitTests/TransferServiceActivityTests.swift +++ b/BitkitTests/TransferServiceActivityTests.swift @@ -698,11 +698,13 @@ final class TransferServiceActivityTests: XCTestCase { let store = MemoryAttemptStore() let attempts = OnchainSendAttemptService(store: store, winningFee: { _ in 281 }) let sender = PreparedAttemptNodeMock() + sender.miningFee = 143 _ = try await attempts.send(using: sender, address: "bcrt1qoriginal", amountSats: sender.amount, satsPerVbyte: 1, utxosToSpend: nil, isMaxAmount: false, followupContext: .init(feeSats: 143, feeRate: 1, tags: ["original"], contact: nil, createdAt: 100)) let original = try XCTUnwrap(store.snapshot().first) sender.txid = String(repeating: "da", count: 32) + sender.miningFee = 281 sender.result = .accepted(txid: sender.txid) _ = try await attempts.retrySamePayment(using: sender, context: .init(attemptId: original.id, walletId: original.walletId, txid: original.txid), diff --git a/changelog.d/next/prepared-fees.fixed.md b/changelog.d/next/prepared-fees.fixed.md new file mode 100644 index 000000000..c68a806b4 --- /dev/null +++ b/changelog.d/next/prepared-fees.fixed.md @@ -0,0 +1 @@ +Retain exact Bitcoin retry fees so accepted payments can finish local recovery while wallet history is still catching up. diff --git a/journeys/send/onchain-original-payment-retry.xml b/journeys/send/onchain-original-payment-retry.xml index 7b4d8fd7b..6d28da4db 100644 --- a/journeys/send/onchain-original-payment-retry.xml +++ b/journeys/send/onchain-original-payment-retry.xml @@ -8,7 +8,10 @@ as state injection, never as proof of a native fault. Manual Tests must verify the exact backend winning transaction retains the complete original input set and recipient amount, and any original Shop request or Blocktank order completes once without a new request or order. Local activity alone - cannot prove these checks. The funded native fault journey remains unrun. + cannot prove these checks. Also use a controlled wallet-history delay after a positively accepted + retry: restart before local follow-up and verify the winner's exact prepared fee appears in Sent + details without waiting for transaction history. An unknown outcome must remain Pending, and an + original winner must never inherit a replacement's fee. The funded native fault journey remains unrun. iOS adaptations: Max recovery keeps the original fee rate without an editable field because the original drain has no higher-fee headroom. The UI also checks wallet.getFeeLimits().maxFee (at most 999). For channel funding, manually verify reopening the original Pending after expiry or failed local completion creates no new order or broadcast, and that accepted original funding resumes its own order watcher. During retry preparation/authentication, check original-winner races and wallet/payer changes; backup must retain the original guarded amount, inputs and candidates. These extra channel-funding and backup checks extend the common actions rather than replace their wording. From 6973637a71f54489f3ffd8eae23e0127fc08a972 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 11 Oct 2026 00:12:39 -0500 Subject: [PATCH 2/3] chore: rename changelog fragment --- changelog.d/next/{prepared-fees.fixed.md => 915.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{prepared-fees.fixed.md => 915.fixed.md} (100%) diff --git a/changelog.d/next/prepared-fees.fixed.md b/changelog.d/next/915.fixed.md similarity index 100% rename from changelog.d/next/prepared-fees.fixed.md rename to changelog.d/next/915.fixed.md From f56b0fb74151f3fac3598bae782aa9b889c68bb6 Mon Sep 17 00:00:00 2001 From: benk10 Date: Mon, 12 Oct 2026 05:24:29 -0500 Subject: [PATCH 3/3] docs: align prepared fee recovery checks --- journeys/README.md | 1 + journeys/send/onchain-original-payment-retry.xml | 6 ++---- journeys/send/prepared-fee-recovery.md | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 journeys/send/prepared-fee-recovery.md diff --git a/journeys/README.md b/journeys/README.md index 17c03c1b3..0a290501f 100644 --- a/journeys/README.md +++ b/journeys/README.md @@ -124,6 +124,7 @@ Their descriptions record extra iOS manual checks and necessary platform mechani | Journey | iOS adaptation | | --- | --- | | Original payment retry | Max retains its original rate; custom fees use `getFeeLimits()`. Additional channel funding, payer/wallet race and backup checks are in the description. | +| `send/prepared-fee-recovery.md` | Same four manual fault cases and file on both platforms; the recipe records original-send versus replacement fee retention and iOS Max/identity-flow adaptations. These faults are not journey capabilities. | | Accepted result | `xcrun simctl openurl`; confirm change before the Max leg, use Manual coin selection and restore Autopilot afterward. The existing Autopilot Max limitation is not claimed fixed. | | Hardware Shop proof | iOS identifiers and Bridge setup; extra durable returning-payer and later tag-edit replay checks are in the description. | diff --git a/journeys/send/onchain-original-payment-retry.xml b/journeys/send/onchain-original-payment-retry.xml index 6d28da4db..7551e8788 100644 --- a/journeys/send/onchain-original-payment-retry.xml +++ b/journeys/send/onchain-original-payment-retry.xml @@ -8,10 +8,8 @@ as state injection, never as proof of a native fault. Manual Tests must verify the exact backend winning transaction retains the complete original input set and recipient amount, and any original Shop request or Blocktank order completes once without a new request or order. Local activity alone - cannot prove these checks. Also use a controlled wallet-history delay after a positively accepted - retry: restart before local follow-up and verify the winner's exact prepared fee appears in Sent - details without waiting for transaction history. An unknown outcome must remain Pending, and an - original winner must never inherit a replacement's fee. The funded native fault journey remains unrun. + cannot prove these checks. See prepared-fee-recovery.md in this directory for the four shared + manual fee-recovery cases and platform adaptations. Record native fault coverage separately. iOS adaptations: Max recovery keeps the original fee rate without an editable field because the original drain has no higher-fee headroom. The UI also checks wallet.getFeeLimits().maxFee (at most 999). For channel funding, manually verify reopening the original Pending after expiry or failed local completion creates no new order or broadcast, and that accepted original funding resumes its own order watcher. During retry preparation/authentication, check original-winner races and wallet/payer changes; backup must retain the original guarded amount, inputs and candidates. These extra channel-funding and backup checks extend the common actions rather than replace their wording. diff --git a/journeys/send/prepared-fee-recovery.md b/journeys/send/prepared-fee-recovery.md new file mode 100644 index 000000000..d4dcbcbfc --- /dev/null +++ b/journeys/send/prepared-fee-recovery.md @@ -0,0 +1,16 @@ +# Prepared transaction fee recovery + +These checks need a disposable funded regtest wallet and controlled native transaction-history, broadcast-response or persistence faults. The journey capabilities do not provide those faults; ordinary payment success alone does not cover them. Never use an existing wallet or start a new payment after an unknown outcome. An explicit retry must retain the original payment's inputs, recipient and amount, with normal fee approval and spend authentication. + +1. Prepare a software-wallet payment and record the candidate transaction ID and exact mining fee. Allow the original broadcast to return Accepted while withholding transaction-history details. Verify local activity records that candidate's exact fee and the original Paykit proof can finish without another send. +2. Interrupt the app after the receipt is persisted. Verify restart retains the fee but does not finish an Unknown payment until the existing exact-transaction evidence identifies the winner. Separately interrupt after replacement acceptance but before local follow-up; reopen with history still withheld and verify the accepted winner's exact fee in Sent details, without another broadcast. +3. Authorize a same-input replacement with a different exact fee. Verify completion uses the actual winner's fee, including when the older candidate wins, and never borrows the other candidate's fee. Confirm the winning transaction and original inputs/recipient amount against the backend; local activity alone is not proof. +4. Fail the prepared-receipt save before dispatch. Verify no broadcast occurs. Separately replay an identical wallet backup over the same local operation and verify retained fees survive; altered operation, candidate or progress data must not overwrite it. Switch wallet or payer identity during retry preparation/authentication and verify the original identity and duplicate-payment guards still apply. + +If the winning fee is not already known, an attempt without a retained candidate fee must still wait for verified transaction-history data. The private candidate-fee map is not exported in the shared backup: reset or a new installation uses the existing history fallback, not an imported fee. A retained fee is bookkeeping evidence, never payment-acceptance evidence. + +## Platform adaptations + +Android retains candidate fees for original sends and replacements in `candidateMiningFees`. iOS already retains the original prepared fee; `candidateFeeSats` covers successors/replacements. Run the same four cases on both platforms. Use each platform's normal identity-selection flow; iOS Max recovery keeps the original rate without an editable field. The common UI actions are in [onchain-original-payment-retry.xml](onchain-original-payment-retry.xml). + +Record the platform, exact build, fixture/fault controls, observed results and untested cases separately. Injected Pending state is not proof of a native broadcast fault, and one platform's results do not establish the other's. These checks do not authorize changing acceptance, safety waits, backup formats or proof delivery semantics.