Repository navigation
fix: retain exact fees for payment recovery - #915
ben-kaufman wants to merge 2 commits into
Conversation
|
talosmachina
left a comment
There was a problem hiding this comment.
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.miningFeeSatsalready written tofollowupContext.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'sattempt.recoveryContext == recoveryguard: 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,
restoreBackupnils 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
Codablemember, so existing keychain JSON decodes withniland falls back towinningFee.
Merge confidence: 4/5, no findings, but the unit and integration test jobs had not finished and the change could not be built here.
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
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
Design
N/A - no UI changes.
Preview
N/A - recovery bookkeeping only.
QA Notes
Journeys
onchain-original-payment-retry.xml- retains the original guarded operation through retry; its description includes exact-fee recovery checks requiring manual fault controls.Manual Tests
Automated Checks
OnchainSendAttemptServiceTests.swift- accepted successor retains its exact fee through JSON restart without transaction history; conflicting fees for one transaction fail closed.OnchainSendAttemptServiceTests.swift- unknown outcomes, original-winner selection, pre-dispatch persistence, absent-fee fallback and canceled candidate cleanup remain guarded.PaykitPaymentStateBackupTests.swift- identical replay preserves local fees without importing new fee metadata; changed candidates/progress remain rejected.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 controlled funded/native journey remains unrun; no new payment, funding, broadcast or protected-wallet interaction was performed. Readiness is for peer review, not a device-payment or merchant-unlock sign-off.