Skip to content

fix: separate usdt recovery backups from paykit - #1118

Merged
ovitrif merged 14 commits into
masterfrom
fix/usdt-independent-recovery
Oct 28, 2026
Merged

ovitrif merged 14 commits into
masterfrom
fix/usdt-independent-recovery

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Twin: synonymdev/bitkit-android#1681

This PR separates acknowledged USDT recovery backups from the full wallet backup, so an ordinary transfer does not export Paykit state or acquire a Pubky lock before broadcast.

Description

  • Adds a USDT (Tether) row in Settings > Backup for the independent recovery-backup status and retry. After React Native migration, a failed USDT VSS recovery phase keeps the wallet on its restore failure screen; Retry or restart resumes that phase without replacing the migrated wallet metadata. This describes the implemented gate; the recorded installed-app acceptance does not claim a separate RN migration or Backup-row device run.
  • Adopts published Paykit 0.1.1-rc2, including acknowledged lease lifetime/renewal and pre-callback rejected-lock cleanup. Pubky remains 0.16.2; the separate homeserver unexpected-401 repair is not included.
  • Adopts published VSS 0.5.26-rc1. Existing backup APIs remain in use; the conditional API does not activate a split full-wallet format or exclude legacy writers.
  • Adopts published Core 0.8.0-rc11, including the restore-order fix from fix: preserve USDT application identity across backup merges bitkit-core#179 and its regenerated bindings.
  • Writes encrypted, immutable, content-addressed USDT recovery records and acknowledges the required backup before broadcasting. Failed uploads retain the original signed transfer.
  • Keeps complete wallet backups separate, retains pending work through temporary failures and retries capture with exponential delays capped at five minutes.
  • Gates network recovery during restore, retains the selected legacy payload before journal import, validates every selected record and resumes interrupted migration. Concurrent writers cannot replace one another's acknowledged records or overwrite a complete wallet backup with partial data.
  • Advances recovery feedback only after an acknowledged backup callback, retaining Retry after a successful no-op refresh.
  • Reconciles automatic recovery and manual Retry against the retained transfer ID. Retry never creates another quote/payment, and feedback explains that closing the screen does not cancel the operation.
  • Preserves genuine Paykit authorization, request/deadline checks and started-payment guards. Imported proof evidence must be revalidated before it can satisfy a request.

Out of Scope

  • LOW recovery-export amplification and restore scaling are tracked for 3.2.0 in #1157 from this review. No unsafe callback acknowledgement shortcut, outgoing-only history filter or record pruning is included.

Native Pubky authentication recovery is tracked in pubky/pubky-homeserver#741. This PR does not release SDK artifacts, replay uncertain contact mutations or establish the historical contact-error cause.

Design

N/A — no new design supplied; Retry uses the existing send-screen controls.

Preview

Owned installed-app screenshots, native database comparisons, authenticated VSS logs and original chain receipts now cover the seven J1 actions below. External videos remain attributed to their recorded revisions.

QA Notes

Setup

Use the committed app dependency pins for automated tests. The complete device journey requires an owned disposable test wallet and recipient, controlled USDT RPC/bundler, encrypted VSS endpoint and Pubky homeserver with fault injection. Standard staging does not provide this combined fixture. Do not use production funds or reset an existing wallet. Record operation IDs, signed bytes, acknowledgement and broadcast counts; see the committed USDT recovery backup document and journey prerequisites.

For the native protocol subset, follow the pinned loopback fixture recipe in synonymdev/bitkit-core#180. It uses a public test mnemonic without funds, real native signing and encrypted VSS, and separate prepare/resume/restore processes. The earlier combined run below used local Core c393f8e0, Paykit e79b7bbb and Pubky 65a331e0 bindings; these overrides are not the committed release pins.

The previously used fork block 517943110 is no longer served by the upstream RPC. Preserve that fixture and its wallets; future runs require a separate healthy fork with account/code/nonce preflight. The commands below describe the recorded historical run, not a currently usable fixture.

For the deployed-contract UI subset, use the fork provider at Core c393f8e0 with Anvil v1.8.6 on loopback port18545, chain42161, Prague hardfork and Arbitrum block517943110; run its Node provider on18546 using the public fixture mnemonic. Run UsdtWalletUITests/testPaymentUsesUsdtOnLocalArbitrumFork on a newly created dedicated simulator. All funds and transactions stay in the local fork. This subset uses the normal VSS endpoint and does not inject backup or Pubky faults.

Journeys

  • J1: new usdt-independent-recovery.xml — all seven existing actions are covered on the final combined head within the owned disposable fixture scope below. This includes Pubky outage, rejected/lost backup acknowledgements, manual/automatic retry, Close/restart, real restore interruption boundaries, original pending-payment continuation and cross-platform writer/restore checks.

Manual Tests

N/A — the controlled device cases are specified in J1; backend fault injection is an explicit prerequisite.

Automated Checks

  • added UsdtNativeRecoveryIntegrationTests.swift — real encrypted backup rejection/lost acknowledgements, separate-process restart, fresh-database restore and the original signed operation before and after retry.
  • added UsdtRecoveryBackupTests.swift — independent uploads, immutable concurrent writers, lost acknowledgements, byte-order/hash compatibility and fail-closed restore.
  • updated UsdtWalletUITests.swift — targets the current address and swipe controls while preserving the local-fork payment, cancellation and offline restart assertions.
  • updated BackupServiceTests.swift, PaykitPaymentStateBackupTests.swift, UsdtWalletManagerTests.swift and UsdtSendFailureTests.swift — migration checkpoints, retained work, proof revalidation and same-transfer recovery feedback.

Final readiness and acceptance — e5aa5776

Current head e5aa5776ab334cdee2b3f44e5a8c4c082401f2ff includes validated master 543d9389e2453fa9e5e3386a915819ac78ee3965 through signed additive merges. Live master subsequently advanced to 85e9d4c6d795a0e83f164a2afe432029c4e218d7 with foreground subscription/allowance notification presentation changes. The complete upstream diff and callers were independently checked: no USDT recovery, backup/restore, payment execution or profile-admission dependency changed. The merge is conflict-free and the actual-base range remains the same 14 commits; all were locally reverified. Per the approved dependency-efficiency policy, this unrelated change does not trigger another full stack merge/build. The existing app/native/hosted evidence remains scoped to e5aa5776, not the unbuilt hypothetical combined tree or the new notification journeys. Core 0.8.0-rc11, Paykit 0.1.1-rc2 and VSS 0.5.26-rc1 remain the normal published dependencies. All 14 current-base commits verify locally and on GitHub (true/valid). The PR is ready for review; Ben's ready transition is preserved.

The actual installed app matches all 105 files of the recorded final-head build. It uses real native signing, LNURL authorization, encrypted VSS and an owned Pubky homeserver with scoped faults, plus an owned non-fork synthetic EVM/provider. No production funds, wallet reset, released artifact replacement or production deployment was used. Earlier component/protocol and external results below are not substituted for these app observations.

Existing J1 action Verified installed-app result
1. Ordinary payment during Pubky outage Payment confirms after its own independent recovery ACK; complete historical WALLET remains readable while unrelated full-wallet work defers.
2. Rejected backup and recovery feedback One approved signed plan remains pending with Error/Pending/Details/Retry/Close and no pre-ACK send. German same-payment/non-cancellation wording was also inspected.
3. Manual/automatic retry and restart Two manual retries, Close/cold restart and committed writes with lost acknowledgements preserve the original operation. Recovery submits only after a new acknowledged write.
4. Close during delayed manual Retry The actual manual continuation reaches the native retry and later UI error handler after a fresh Send form opens; that form stays empty and unchanged. No extra payment is approved. The separately recorded fresh-form cancellation is reused explicitly.
5. Genuine Paykit request and lease A real peer request receives explicit app authorization, retains its exact request-bound payment under rejection/native lease and cold restart, then delivers one genuine peer proof. Canonical proof and original chain receipt validate offline. The peer runtime chain verifier was not separately invoked.
6. Interrupted restore and pending continuation Actual stops after Core import/before the corresponding independent association callback, and after all association callbacks/before marker clearing, retain both gates and all plans. A fresh restore subsequently resumes one still-valid, originally unsubmitted payment without a new quote, signature or approval.
7. Writers, association/proof state and metadata Recorded two-writer/cross-platform restores preserve the operation union, started guards and proof behavior. Both selected historical METADATA records match all 11 fields after actual restoration. Tags are empty; no nonempty-tag or blanket full-wallet comparison is claimed.

The final pending-restore case retains operation bfd0a064-8577-4d85-8328-0cd89e5ee36f and signed-plan SHA256 13082e5d9a29937ab65fe1cab69ddc69dd90b4048068c0e3d019cde3d26de95a through rejection, 56 committed writes with lost acknowledgements, source shutdown and fresh-device restore. The restored process receives native backup ACK at 22:26:36.251 UTC, then its sole provider submission at 22:26:38.490. Provider sends increase 12→13, all 14 signed plans remain exact, and the original receipt contains one successful operation and one 6-USDT recipient transfer, 229 seconds before its original expiry. The earlier slow restore expired safely with no extra send; it remains a failed successful-continuation attempt, not erased evidence.

The final independent evidence audit verifies 280 frozen inputs, complete app/proxy/provider/RPC records, selected encrypted records and original chain receipt/header. Final seven-action matrix SHA256: 0a77d9db33016fad6b0f3f1fafe17fb5ed88d7118238495480ac69b3947e6ec8; final N inspection receipt SHA256: 675579c56831063ee78d79715063654f21b5a7ec4d8c88c31fe646e13a9bbca9. Debugger auto-continue hit counts and original output bounds establish the manual path; attempted timestamp commands did not install, so precise breakpoint timestamps are not claimed. Earlier unsuccessful controls and all private artifacts remain preserved locally without exposing seeds or signed payloads.

Final combined-tree validation

  • Local: 2,962 selected units, zero failures/skips; 39 UI executions, zero failures and three disabled Trezor skips. The expired historical fork case is explicitly excluded, not passed. Full SwiftFormat 0.64: 0/852; translations: zero errors, 15 warnings verified against the actual base. Complete raw/nested diagnostics inspected; AppIntents, simulator and QoS observations retained without a timing-cause claim.
  • Two independent final source reviews are clean on all 1,678 unchanged inputs. Later acceptance captures do not change those inputs.
  • Hosted unit/UI: 2,962 units + 39 UI passes, one native-fixture unit skip and four UI skips. Integration: 30 passes / four explicit staging-dependent skips. SwiftFormat: 0/852. All complete logs, 3,040 nested execution summaries and verified artifacts inspected.
  • Hosted E2E: 13 unique eventual passes / one explicit hardware skip. First attempt had 12 passes and a 30-second Lightning Transfer Initiated timeout; the workflow's second attempt passed that case and reused the prior 12 results. No author rerun. Failure screenshot/video were not uploaded after eventual success, so the cause remains unproven.
  • Hosted synthetic merge 64c83669 has exactly the head tree. The 286 distinct duplicate-symbol linker warnings are retained; no matching complete live-base build establishes their introduction, so they are not labelled pre-existing or fixed. No source-located Swift finding or runtime-warning object was reported in those hosted results.

Remaining limits

J1's finite acceptance is complete within the stated local fixture scope. This does not claim production paymaster pricing, every complete-wallet field, simultaneous multi-device spending coordination, an atomic multi-page listing or a split full-wallet migration. The legacy complete wallet remains supported; older apps recover journal-only operations only once included in a complete wallet backup. No records are pruned. Pubky homeserver #741 remains separate and unmerged; RC2 does not include it or expose a new CredentialRefreshError classification. Native contact CAS and future full-wallet redesign are not prerequisites for this PR.

Models used

  • Planning/scoping and implementation: gpt-6-astra (reasoning: xhigh)
  • Independent source/acceptance review: gpt-6-astra (reasoning: xhigh)
  • Complete hosted diagnostic inspection: gpt-6.1-sol (reasoning: high)
Preserved historical revision evidence and author notes (superseded status; not current readiness)

The following authored notes are retained at their original revisions. Statements about pending checks, unchecked J1 or draft status describe those earlier checkpoints; the current result is above.

Earlier released VSS validation

At signed f82258ef, normal SPM resolution adopts VSS 0.5.26-rc1 at eba41ef6; all nine resolved framework files match the published archive (SHA256 8bd5a46b6a0a132a76f8da59b2fc6acc7196e432bb138ed8e5666fcff4389b21). An additive signed parent merge also retains both independent recovery Retry and the typed expiry explanation from #1115, without rewriting history. Final 2,891 selected units passed with zero failures/skips, and SwiftFormat 0.64 reported zero violations across 847 files. Complete nested diagnostics include two AppIntents notices and five QoS observations whose timing causes remain unproven.

Both platforms passed fresh rejected-backup and lost-acknowledgement prepare/resume/restore phases, plus encrypted restore in both directions: 14 native phases total. Every retry preserved the original signed operation, each process acknowledged its own recovery backup before a broadcast attempt, there was one unique operation, and the complete WALLET sentinel remained unchanged. iOS calls the production BackupService handler; Android calls the recovery store directly. These tests use native signing/encryption with a loopback protocol/RPC fixture, not deployed contracts or the full UI/Paykit J1 journey. Two consecutive final independent reviews were clean. All eight commits against the live base are locally signed and GitHub reports verified=true, reason=valid. Hosted checks for this head are pending; draft-skipped jobs will not be counted as passes.

Earlier revision evidence

At aeeaa020, Xcode 26.1 / iPhone 17 / iOS 26.5 ran the CI-equivalent selection: 2,889 unit tests passed; UI results were 38 passed, four fixture-dependent skips and one failure. The failing comma-region manual-entry case waited on the Bitcoin node's startup screen, with both recovery gates absent. It reproduced on the unchanged signed parent 008b2e5e; a diagnostic 90-second wait also failed and was discarded. The underlying node-startup delay remains unknown; that historical run did not pass the full UI target. The later historical-head run below passed with explicit fixture skips.

SwiftFormat 0.64 full-tree lint has an empty report (0/846 files, 77 excluded). Translation validation has zero errors; all 15 warnings were verified against the parent after line-number normalization. Journey XML parses. Independent review and bounded rechecks found no remaining actionable issues.

At test commit 9a763eb4, the combined local SDK build passed 2,889 unit tests and changed-file SwiftFormat lint. Native rejection and lost-acknowledgement sequences passed prepare, resume and restore; bidirectional iOS/Android encrypted snapshot restore also passed. iOS calls the actual BackupService recovery handler, checks zero unrelated backup captures and counts every attempted broadcast. The initial interrupted rejection run is not counted; a fresh complete sequence passed.

Each native failure phase verifies zero broadcast attempts before its own recovery acknowledgement, unchanged signed operation bytes on retry and one unique payment. Full-wallet sentinel bytes remain unchanged. This verifies the native protocol subset, not the complete J1 journey.

At adoption commit 646670df, the normal published Core RC11 package resolves to 22e0acc3; its downloaded archive matches the manifest SHA256 f7ef2ffb625498c651111cb0f5a177893c5abd91c89b077e11abf0ee289cd544. With no local SDK overrides, all 2,889 unit tests passed and full SwiftFormat 0.64 lint reported zero violations across 847 files. Native rejected-backup and lost-acknowledgement cases each passed prepare, resume and restore on the owned iOS simulator. The final documentation-only wording correction was independently reviewed and checked; production and package inputs were unchanged after the unit run. All four PR commits are signed and GitHub reports verified=true, reason=valid.

With Android 258bf9e5b, fresh encrypted restore passed in both directions using each app's normal published Core RC11 dependency. Across both platforms, all 16 native failure/recovery/restore phases preserved the same signed operation and acknowledgement-before-submission ordering. This remains protocol-subset evidence, not completion of J1.

At acknowledgement fix 5851185c, a successful refresh with no backup callback no longer clears recovery feedback. Progress advances only after the native backup callback is acknowledged; failed persistence retains the error and Retry action. The focused regression fails on unchanged 646670df and passes with the repair. Final CI-equivalent unit selection passed 2,890 tests, zero failures/skips; full SwiftFormat 0.64 reported zero violations across 847 files. Complete diagnostics were inspected: compiler metadata warnings and negative-fixture runtime diagnostics are inherited. Independent review found no remaining issue. All five commits against the live parent are signed and GitHub reports verified=true, reason=valid. This follow-up has component/adapter coverage; the earlier native protocol evidence remains tied to the commits above.

A later diagnostic launch at 5851185c completed LDK construction in 1.08 seconds and subsequently synchronized and uploaded recovery data. The unchanged comma-region manual-entry UI test then passed in 35 seconds on the owned simulator. This does not establish the earlier delay's cause. A subsequent complete UI target at that historical revision finished with exit code zero; the finalized XCTest bundle confirms 39 passing executions (ten test methods, including 30 dynamic runs), four opt-in fixture skips and zero failures. The skips are three Trezor emulator cases and the local Arbitrum fork payment case. Compiler metadata and simulator accessibility warnings match the prior run. Hosted SwiftFormat for this head also passed with zero violations across 847 files.

At signed test repair 6ead1895, the previously skipped local Arbitrum fork payment test ran on a new dedicated iPhone 17 / iOS 26.5 simulator and passed with zero skips. The test now targets the actual address button and draggable confirmation knob, retaining the exact recipient assertion and cancel-then-confirm checks. It sent one synthetic USDT, confirmed, retained zero native ETH and displayed saved activity after an offline restart. The local Anvil receipt contains one successful user operation and exactly one recipient token transfer. This executes deployed contract code in a fork; it does not validate production paymaster pricing or the complete Paykit/fault-injection J1 journey. The final unit selection passed 2,890 tests with no failures/skips; full SwiftFormat reported zero violations across 847 files. Complete diagnostics and independent review were clean apart from unchanged AppIntents metadata warnings. All six actual-base PR commits are signed and GitHub reports verified=true, reason=valid.

Gaps

J1 has not run as a complete UI/Paykit/deployed-contract journey. The protocol fixture does not execute deployed contracts or exercise a real Pubky outage. Full wallet migration and concurrent state changes remain covered by component tests rather than this native fixture; Android's native store test does not exercise all BackupRepo orchestration.

This remains draft pending its own remaining J1 acceptance: final-composition production-app backup failures, restart/restore/migration, concurrent recovery records, and preserved Paykit authorization/proof behavior. The ordinary required-backup path uses released Core RC11, released VSS RC1 and local payment associations; it does not call Paykit export or a Pubky lease. The separate future full-wallet split and further Pubky upgrades are not blanket prerequisites. A dependency will be recorded only if a required case demonstrates it. Core #179 and RC11 preparation #181 are merged, and the published Core RC11 dependency is adopted and validated here. The Android twin adopted the published Maven package in 258bf9e5b and now includes the acknowledgement repair at dc5a47d90; its native libraries match the published debug symbols. Local combination builds and native tests passed with Paykit lease-expiry safety from pubky/paykit-rs#211 and authentication recovery from pubky/pubky-homeserver#741; the Paykit lease and lock-cleanup fixes are now adopted through released RC2 below, while the separate Pubky homeserver repair remains unmerged and unavailable. No app release or deployment is part of this PR.

The earlier device-sleep lease repair at Paykit 461864a4 passed 1,557 Rust tests, both native binding builds and full generated Android checks. The later complete RC2 includes additional reviewed lease/cleanup repairs and is adopted below. The combined app/native evidence above used Paykit e79b7bbb. Core RC11 does not contain the Pubky authentication or Paykit lease fixes.

The legacy complete wallet format remains supported. Older apps cannot read journal-only operations; downgrade recovery is limited to the last complete wallet backup. Records are not pruned, and immutable writes do not make multi-page listing atomic or coordinate simultaneous spending by multiple devices.

Authenticated local backup fixture configuration (d1f5ce5)

The recovery branch now accepts a paired local VSS/auth configuration only in an explicitly enabled development E2E build. Set E2E=true, E2E_VSS_SERVER_URL=http://localhost:18657/vss and E2E_LNURL_AUTH_SERVER_URL=http://localhost:18655/auth before building/launching; Android additionally requires device-local port forwarding for both ports. Both values are required together, must use localhost with explicit ports and paths, and reject credentials, query strings and fragments. Normal production builds retain their original endpoints. The existing native LNURL authentication and encrypted storage remain in use.

Final validation at d1f5ce5d: 2,893 selected unit tests passed with zero failures/skips; full SwiftFormat (847 files) passed with no introduced findings. Complete logs and structured diagnostics were inspected, and two consecutive independent final reviews were clean. This configuration enables the controlled fixture; it does not establish J1 or a full authenticated app failure journey. The existing unchecked journey cases and draft status remain applicable.

The final iOS run also recorded two AppIntents build notices and four QoS observations in unchanged contact/SDK/profile tests. Their timing cause remains unproven; they are not reported as fixed.

Paired close/reopen journey clarification

At c6dac32d0, the journey adds the same delayed recovery Retry → Close → fresh Send → old failure action as Android #1681. This commit changes only the journey XML; XML parsing and git diff --check pass, and two consecutive independent reviews found no issue. iOS keeps send feedback in presentation-local state, so no matching shared-view-model production repair is claimed. The new device action remains unchecked; prior iOS native results remain scoped to their recorded revisions. Android's new authenticated fixture has partial results and a failed expired-paymaster restart settlement, not a full J1 or iOS acceptance pass.

Published Paykit RC2 adoption (2b387b28)

Normal SPM resolution now uses Paykit 0.1.1-rc2 at 01de5dd2. All seven resolved XCFramework members match the published archive SHA256 886ef57dc75938f2713d2a7e649c6557ea6ba91afed92ef6030554b015d89322; other dependency pins are unchanged. Final 2,893 selected unit tests passed with zero failures/skips, full SwiftFormat checked 847 files with zero violations, and two consecutive independent final reviews were clean. Complete logs and nested XCResult issues were inspected. Two AppIntents notices and four QoS observations remain; three warning/test identities overlap the preceding run and one differs, so no runtime-warning cause or fix is claimed.

Six fresh released-native rejected-backup/lost-acknowledgement prepare/resume/restore phases passed. Each retained the original signed operation, acknowledged its own recovery backup before submission, produced one unique operation, and preserved the complete WALLET sentinel. These use real native signing/encryption with loopback protocol and mock authorization; they are not authenticated UI, deployed-contract, or full J1 acceptance. The paired Android RC2 phases and fresh encrypted restores in both directions have now also passed, bringing the total to sixteen native phases. Repeated refresh deliberately makes two attempts with the same signed operation; there is still only one unique operation. Earlier Android instrumentation initialization and expired original cross-fixture failures are preserved separately; a new independent public-seed scenario passed the opposite-direction restore without resetting or re-signing the old operation. All eleven commits in the current-base range are locally verified and GitHub reports verified=true, reason=valid. The PR remains draft, with J1 unchanged and unchecked. RC2 does not include Pubky #741 or expose CredentialRefreshError; no new error classification or split full-wallet writer is activated.

External authenticated app evidence at 2b387b28 (October 27)

Ovi's automated QA review reports current-head iPhone 17 / iOS 26.5 execution with disposable local-fork funds, real LNURL authentication and encrypted VSS. It reports successful ordinary USDT transfer during Pubky LOCK 503 failures; rejected-backup recovery through two manual retries and cold restart; committed recovery writes with lost acknowledgements followed by automatic recovery; and Close during a delayed Retry without the old failure entering a new Send form. The review reports unchanged signed bytes/operation identity, one successful recipient transfer for each completed payment, and readable complete-WALLET data alongside the journal. Its native protocol cases retain mock-authorization and loopback limits.

This is attributed external evidence: the author inspected the complete review, but has not independently inspected its raw proxy ledger, SQLite comparisons or contract receipts. It is not a human approval or a complete J1 pass. Genuine Paykit request/authorization/proof recovery, interrupted full-wallet/RN restoration, concurrent production-app writers/cross-platform restoration and non-English presentation remain unexecuted in that review. J1 stays unchecked and this PR remains draft; the successful subsets do not erase the author's earlier expired-paymaster fixture failure.

Models used

  • Planning/scoping: gpt-6-astra (reasoning: xhigh)
  • Implementation: gpt-6-astra (reasoning: xhigh)
  • Review: gpt-6-astra (reasoning: xhigh)

Latest hosted validation (2026-10-26)

Hosted SwiftFormat passed on current 2b387b28, with zero violations across 847 files and the complete log inspected. Synthetic merge 89a2a298 has the same tree as the published head. Native/unit/migration jobs were draft-skipped; this is not a hosted native or J1 pass.

@ovitrif ovitrif added this to the 3.1.0 milestone Oct 24, 2026
@ovitrif

ovitrif commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Triage: 3.1.0. It fixes shipped USDT recovery backups, and 3.1.0 is due 28 Oct.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ⚠️ Not verified — the complete J1 journey remains partial.

Findings: None.

QA for 2b387b2:

Tested the committed dependency pins on iPhone 17 / iOS 26.5. The app cases used disposable local-fork funds, real LNURL authentication and encrypted VSS. Native protocol cases below use mock authorization and do not execute deployed contracts.

Test 1: J1a — Pubky outage with working VSS ✅ The app confirmed an ordinary USDT payment while its profile’s Pubky LOCK requests returned 503.

The full-wallet backup remained pending and logged increasing retry delays. Recovery writes were acknowledged independently. The deployed-contract receipt contains one successful original operation and exactly one 1-USDT recipient transfer. Acknowledgement-before-submission ordering was checked in the native cases and source; the full-wallet readback is reported in Test 8.

Approval-to-sent excerpt (2×); the fault and receipt assertions above were checked in request logs and the contract receipt.

J1a-excerpt-2x.2x.mp4

Test 2: J1b — Rejected backup, Retry and cold restart ✅ The failure explanation appeared, and recovery confirmed the original signed payment after acknowledgement.

The screen showed Error, the same-payment explanation, Pending, Retry, Details and Close. Two manual retries and a cold restart preserved the application ID, operation hash and exact signed-plan bytes. The bundler count stayed unchanged while uploads were rejected, then increased once after acknowledgement. SQLite comparison and the successful receipt verified the original operation and one 1-USDT recipient transfer.

Failure-screen excerpt (2×) Cold restart and recovery (2×)
J1b-1-excerpt-2x.2x.mp4
J1b-2-2x.2x.mp4
Test 3: J1c — Lost acknowledgement and automatic recovery ✅ A committed recovery record survived dropped responses, and automatic recovery confirmed the same signed operation.

The VSS proxy recorded successful commits followed by dropped acknowledgements. The bundler count did not increase until acknowledgements were allowed. Recovery completed without pressing Retry; SQLite retained identical signed-plan bytes and the operation hash. Its receipt contains one successful operation and one 1-USDT recipient transfer.

Recovery-to-sent excerpt (2×); commit/acknowledgement ordering and operation identity were checked in the proxy ledger and SQLite.

J1c-excerpt-2x.2x.mp4

Test 4: J1d — Close during Retry ✅ The old failure stayed out of a fresh recipient form.

I started a delayed Retry, closed its screen and opened a fresh Send form. The form stayed empty after the old failure returned. The excerpt shows the fresh form; the callback log and SQLite comparison verify the original payment remained pending with unchanged signed bytes and no additional operation.

Fresh-form excerpt after the old callback returned (2×).

J1d-excerpt-2x.2x.mp4

Test 5: N1 — Native rejected-backup recovery ✅ XCTest assertions passed across separate prepare, restart and fresh-database restore processes.

Each process verified its own rejected upload and no pre-acknowledgement submission. Accepted recovery retained the original signed operation and complete-wallet sentinel bytes. Repeated submission attempts used one unique operation. This checks the production recovery handler with native signing/encryption and the loopback protocol fixture; it does not validate LNURL signatures or full-wallet migration.

N1-2x.2x.mp4

Test 6: N2 — Native lost-acknowledgement recovery ✅ XCTest assertions verified committed encrypted records restore and retry the original operation.

Separate prepare, restart and restore processes each required a fresh acknowledgement before submission. Signed bytes and complete-wallet sentinel bytes were preserved. The same protocol-fixture limits as Test 5 apply.

N2-2x.2x.mp4

Test 7: U1 — Affected component regressions ✅ The selected backup, manager, payment-state, presentation and endpoint XCTest classes passed without failures or skips.

The assertions include interrupted migration gates, immutable concurrent records, conflicting bindings, proof revalidation, acknowledgement-driven feedback and bounded backup retry. These component results do not complete the corresponding app journeys.

U1-2x.2x.mp4

Test 8: R1 — Complete-wallet and journal readback ✅ Native assertions decoded the existing complete WALLET and validated seven recovery records while the Pubky fault remained active.

Read-only test instrumentation used the published native VSS client and production decoders on the disposable wallet. The complete envelope remained readable alongside the independent journal; this is readback evidence rather than a full restoration journey.

R1-outage-2x.2x.mp4

Test 9: J1e — Genuine Paykit payment and lock recovery ⏭️ The complete payment, authorization and proof-reconciliation case was not driven.

The Pubky-failure/full-wallet-backoff subset ran in J1a. The available peer fixture proposes BTC requests through amount_sats; it does not supply the required USDT Paykit request/proof flow. Source review and component assertions do not establish the complete held-lock and restart journey.

Test 10: J1f — Interrupted full restoration and migration ⏭️ Production-app restoration was not interrupted after bitkit-core and association commits.

The native fixture exercised USDT restart/restore, and R1 exercised complete-envelope readback. Neither drives full-wallet/RN metadata migration or its interruption points after a Pubky failure. Those paths retain component coverage from U1.

Test 11: J1g — Concurrent app writers and cross-platform restoration ⏭️ Simultaneous production-app writers with changing Paykit associations were not driven.

Immutable-store concurrency and both envelope encodings passed component assertions. The author’s recorded cross-platform native results remain protocol-subset evidence; they do not establish this concurrent app/proof journey.

Check 1: Build and static checks ✅ The pinned head built, and changed-file SwiftFormat, translation validation, journey XML parsing and diff checks passed.

I used xcodebuild build-for-testing, swiftformat --lint, node scripts/validate-translations.js, XML parsing and git diff --check. The native fixture recipe defines the protocol tests’ narrower scope.

Twin: bitkit-android#1681 fix: separate usdt recovery backups from paykit matches the recovery format, restore guards and proof-revalidation behavior at 7c218843; iOS keeps send feedback in presentation-local state.

Full review of 10c2912..2b387b2, including every changed file and reached backup, restore, retry and proof path. Genuine Paykit recovery, full-wallet/RN interruption, concurrent app writers and non-English presentation were not driven on devices.

Reviewer model: gpt-6.1-sol
Reasoning effort: high


Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ✅ Approve

Posted as a comment: not every planned case passed.

Findings: None.

QA for 4d1c0ba:

I tested the pinned dependencies on iPhone 17 / iOS 26.5, with Android API 36 for cross-platform recovery. Payments used disposable local-fork funds, deployed contracts, native LNURL authentication and encrypted VSS. Timing hooks paused real restoration commits; read-only exports and an opt-in native snapshot upload supplied the state comparisons below.

Test 1: U2 — Component regressions ✅ The affected backup, restore, proof, authorization, send-lifecycle, activity and receive tests passed without failures or skips.

This includes authorization/expiry rejection, conflicting bindings, stale-proof validation and the merged send-lifecycle changes.

Functional recording (2×); state, ordering and receipt assertions above were checked in native exports and request logs.

Test 2: R2 — Complete WALLET and journal readback ✅ Native assertions decoded the complete WALLET and independent recovery records during the Pubky outage.

The existing envelope remained readable alongside the independent journal. This case checks readback; full restoration is covered below.

Functional recording (2×); state, ordering and receipt assertions above were checked in native exports and request logs.

Test 3: J1a — Ordinary payment during Pubky outage ✅ An ordinary USDT payment confirmed while Pubky LOCK requests returned 503.

Request logs show independent VSS writes and increasing full-wallet retry delays. The deployed-contract receipt contains exactly one 1-USDT recipient transfer.

Functional recording (2×); state, ordering and receipt assertions above were checked in native exports and request logs.

Test 4: J1b — Rejected backup, Retry and restart ✅ Two retries and a cold restart retained the original signed payment until acknowledgement allowed submission.

The error explained backup failure and same-payment retry, with Pending, Details, Retry and Close available. SQLite comparisons preserve the application ID, operation hash and exact signed bytes; request counts show no pre-acknowledgement submission. The successful receipt contains one 1-USDT recipient transfer.

Functional recording (2×); state, ordering and receipt assertions above were checked in native exports and request logs.

Test 5: J1c — Lost acknowledgement and automatic recovery ✅ A committed record survived dropped acknowledgements and recovered without pressing Retry.

The proxy ledger shows successful writes followed by dropped responses. The bundler count stayed unchanged until acknowledgement was allowed; SQLite and the receipt verify the original signed operation and one recipient transfer.

Functional recording (2×); state, ordering and receipt assertions above were checked in native exports and request logs.

Test 6: J1d — Close during held Retry ✅ An old failure stayed out of a fresh, empty recipient form.

I held the actual recovery upload, closed its payment screen, opened a fresh manual form, then released the old request into failure. The form remained empty. Request counters and SQLite show no new approval or operation; the original later confirmed once.

Functional recording (2×); state, ordering and receipt assertions above were checked in native exports and request logs.

Test 7: J1e — Genuine Paykit payment and lock recovery ✅ A request-bound USDT payment survived Pubky HTTP 423 and restart, then delivered one verified proof.

A second Bitkit requested $1 with USDT as its only accepted method. Native logs report SharedStateBusy while the signed payment and started guard remain retained. After restart and lock release, the bundler and authorization counts stayed unchanged. Recipient state contains one unique verified proof receipt for the request, and its contract receipt has one 1-USDT transfer. Authorization and expiry rejection were checked in U2.

Functional recording (2×); state, ordering and receipt assertions above were checked in native exports and request logs.

Test 8: J1f — Interrupted full restoration ✅ Restoration resumed after interruptions at both real import boundaries and kept sending gated until completion.

At the core-import and association-commit holds, native logs show the restore gate active and the UI remains on setup. Restart resumes the complete WALLET plus independent records. A separate RN-format restoration recovered the original lost-acknowledgement payment without another quote or approval; its unchanged signed operation confirmed once.

Functional recording (2×); state, ordering and receipt assertions above were checked in native exports and request logs.

Test 9: J1g — Concurrent writers and both platforms ✅ Overlapping app uploads retained all seven signed plans and all three request bindings across recovery.

An Android app uploaded its actual native wallet and association snapshot while older HTTP copies remained in flight. iOS added a genuine request binding and signed payment during that overlap. Proxy ordering and app logs establish the concurrent writes. Fresh iOS recovery and Android’s production recovery entry point preserve every application ID, hash and signed byte; started guards survive and restored proof flags are cleared for revalidation. U2 checks stale-proof rejection. The RN-format restore retains its selected EUR setting and tag metadata instead of replacing them with native-wallet metadata.

RN-selected iOS recovery and Android restoration (2×). Native exports and request logs establish the upload overlap and exact state comparisons.

iOS Android
Check 1: Build and static checks ✅ The pinned head built and the changed-file checks passed.

xcodebuild build-for-testing, changed SwiftFormat lint, translation validation, journey XML parsing and git diff --check passed. Translation validation reported existing warnings and no errors.

The unchanged native protocol cases N1/N2 and earlier complete-envelope readback retain their previous passing evidence; I verified their relevant inputs are unchanged.

Twin: bitkit-android#1681 fix: separate usdt recovery backups from paykit at 416c1e3 matches the recovery format, request bindings, started guards and proof-reset behavior exercised here.

Delta review of 2b387b2..4d1c0ba, plus the prior open recovery cases; merged BTC, subscription, receive-layout and non-English presentation changes were source/component reviewed but not separately driven on devices. RN fixture coverage is selected metadata and USDT recovery, not legacy Lightning-channel migration.

Reviewer model: gpt-6.1-sol
Reasoning effort: high


Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@ovitrif
ovitrif changed the base branch from fix/usdt-backup-failure-feedback to master October 27, 2026 15:05

Copy link
Copy Markdown
Contributor Author

I independently fetched and decoded all nine recordings from review 5668366289 at 4d1c0ba. The fresh iOS Restore → recovered history → confirmed detail is verified, along with the visible send/error/retry-form flows; the earlier media-access limitation is resolved. To finish acceptance without repeating those cases, please attach the existing redacted receipts:

  1. Diagnostic hook diffs/build identity and a compact case table of operation IDs/signed-byte hashes, VSS commit/drop/ack and submission times/counts, successful contract transfer counts, and complete-WALLET readback assertions. Hashes/assertions are sufficient; no keys, session material, decrypted backups or full databases.
  2. For J1e, whether a second native holder actually acquired the same lease, with hold/release, bounded retry and post-release reconciliation-before-write events plus duplicate counts across restart. At the pinned SDK, injected HTTP 423 alone can produce SharedStateBusy before any lease is acquired; it is useful busy-path coverage but does not establish the real-held-lease segment.
  3. J1f/g’s existing two import-boundary interruption traces, upload/association ordering and post-restore proof verification before use, mapped to the retained failed-backup/lost-ack/lock records.
  4. The Android restore invocation/opt-in patch and pre-restore local-state summary. Its clip shows resulting Home/history but not fresh Restore entry, so I cannot yet distinguish fresh app restoration from a production helper applied to an existing wallet.

This requests existing evidence first, not another broad run or a wallet reset. Any actually missing segment can be isolated on a new disposable fixture. Full-wallet redesign and native contact CAS are not blockers for this PR.

@ben-kaufman
ben-kaufman marked this pull request as ready for review October 27, 2026 19:44
@ovitrif
ovitrif removed the request for review from jvsena42 October 27, 2026 19:50

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ⚠️ Not verified — the combined app recovery journeys remain untested.

Findings: None.

QA for e5aa577:

Test 1: Foreground backup and recovery regressions ✅ Foreground recovery, failure notification and independent recovery store assertions passed without skips.

The XCTest result bundle confirms the assertions. The recording shows the simulator during the run.

U2.2x.mp4

Test 2: J1a — Ordinary payment during Pubky outage ⏭️ The combined-head journey was not driven.

The controlled payment fixture did not reach a reliable app payment.

Test 3: J1b — Rejected backup, Retry and restart ⏭️ The funded wallet could not reach payment review.

The local fork returned provider-unavailable responses during user-operation gas estimation, including after retrying with VSS healthy; no payment was approved. The backup-rejection and restart assertions were not reached.

Test 4: J1c — Lost acknowledgement and automatic recovery ⏭️ The combined-head journey was not driven.

The controlled payment fixture did not reach a reliable app payment.

Test 5: J1d — Close during held Retry ⏭️ The combined-head journey was not driven.

A retained payment could not be prepared with the controlled fixture.

Test 6: J1e — Genuine Paykit payment and lock recovery ⏭️ The combined-head journey was not driven.

The controlled payment fixture did not reach a reliable app payment, and a second simulator could not complete app installation.

Test 7: J1f — Interrupted full restoration and migration ⏭️ The combined-head journey was not driven.

A second simulator could not complete app installation, so the interruption and restart boundaries were not exercised.

Test 8: J1g — Concurrent writers and cross-platform recovery ⏭️ The combined-head journey was not driven.

The controlled payment fixture and second simulator did not reach the required two-writer setup.

Check 1: Build and static checks ✅ The pinned head built for testing; diff and translation checks passed.

The first broader component run printed passing assertions but its result bundle did not finalize, so it is not counted as a pass. The focused rerun above produced a completed XCTest result.

The earlier review at 4d1c0ba records the prior app journeys; the foreground-backup composition changed since then. The unchanged native protocol cases and complete-wallet readback retain their earlier evidence; those checks do not replace the full app journeys. The PR description also reports combined-head component and UI checks.

Twin: bitkit-android#1681 fix: separate usdt recovery backups from paykit matched the independent recovery format and proof-revalidation design at 416c1e3 in the prior review; its later delta was not fully reviewed here.

Delta review of 4d1c0ba..e5aa577, including backup scheduling, restore guards, cached balance and receive changes. The inherited cached-balance and receive presentation changes were not separately driven in this run.


Reviewed by gpt-6.1-terra-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No findings. The iOS twin: USDT recovery snapshots move out of the WALLET backup into immutable, content-addressed USDT_RECOVERY_V1_<sha256> VSS records (UsdtRecoveryBackupStore), acknowledged before broadcast, with UsdtRecoveryRestoreGate in the keychain gating network operations until restore completes, and temporary-failure backoff in BackupService. Reviewed e5aa577, oversized tier (40 files, +1276/-107).

What I checked, and 5 candidates I ruled out

Read in full: the production diff (BackupService.swift, UsdtRecoveryBackupStore.swift, UsdtWalletManager.swift, PaykitUsdtPaymentService.swift, UsdtSendView.swift, AppScene.swift, Env.swift, BackupCategory.swift, Keychain.swift, the package pins). Tests, journeys, docs and translations were not reviewed: the oversized tier.
Not built here: iOS does not build on this machine, so this is a static review. CI is the gate: SwiftFormat and validate green, Run Tests, Run Integration Tests and build-local pending when I posted.

Ruled out

  • Record replay order: bitkit-core restore_backup merges with local precedence (src/modules/usdt/backup.rs), so sorted-key replay is safe.
  • pauseForRecoveryRestore deadlocking against an in-flight send: idleWaiters is now a list resumed in full at activeOperations == 0, and backupSnapshot/restoreBackup enter with requiresNetwork: false, so the restore itself is not blocked by the gate it set.
  • .usdt hitting the generic upload path: runBackup routes it to backupUsdtRecoveryIfPresent, getBackupDataBytes(.usdt) throws, and getLatestBackupTime skips it.
  • Removing .dropFirst() in startBackupStatusObservers firing a backup storm at startup: the prepended value only schedules categories already isRequired && !running, which is the pending work the change is meant to resume.
  • A keychain read error locking USDT forever: UsdtRecoveryRestoreGate.isPending fails closed by design, the same posture as hasPendingWalletRestore.

Merge confidence: 3/5, the oversized-tier ceiling (tests and journeys not read), and unit and integration CI had not finished when I posted.

Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed in this run's metadata)

@ben-kaufman
ben-kaufman requested a review from jvsena42 October 27, 2026 22:43

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code scan: no HIGH or MEDIUM. One LOW inline: every USDT transfer change writes a new full-history recovery record.

Checked and clean:

  • The records hold signed userOps and Paykit associations, and no keys. They are encrypted with the unchanged seed-derived VSS client.
  • The v3.0.0 WALLET USDT state is replayed before the journal, so a restore from 3.0.x loses nothing.
  • The pre-broadcast persist never touches Paykit or Pubky.
  • Cross-identity contamination is ruled out: the store is scoped to the seed, the core checks the identity, and quote ids are UUIDv4.
  • The formats are interoperable between the twins: same envelope, prefix, hash over the stored bytes, and replay order.

Not raised as a finding: on the RN-backup restore path, a VSS outage now keeps the whole wallet closed until VSS returns. Master already does this for every other restore during an outage, and your tests assert it. A line in docs/usdt-recovery-backups.md saying the wallet stays closed until that phase succeeds would document it.

Device gate (partial): not run yet. Pending: usdt-independent-recovery.xml with the local usdt-fixture, and the new "USDT (Tether)" row in Settings › Backup. Neither PR body mentions that row.

Comment thread Bitkit/Services/BackupService.swift

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review

Scope: Reviewed the full PR diff against its merge base, at e5aa577 (merge base 543d938). No earlier assessment baseline was available.

No new actionable code findings.

An ordinary USDT send acknowledges an immutable USDT_RECOVERY_V1_<sha256> record before broadcast and does not take the full Paykit or Pubky wallet-backup path. The keychain restore gate stays set until the retained legacy wallet snapshot and every selected journal record are merged. Core restore keeps an existing local transfer outcome and adds new ids. Retry uses refreshTransfers for the retained pending id, and closing the send screen drops only the view-local error. Compared with bitkit-android#1681 at 6fc2267: the recovery record, restore order, and proof invalidation match. Android's journey adds a deadline-transition action that this iOS journey does not repeat; journeys/README.md records that, and UsdtSendFailureTests covers the expiration copy.

The open performance comment on BackupService.swift was checked and not adopted. The pre-broadcast callback leaves the USDT category required, and the observer then schedules one follow-up upload; a scheduled run that does not observe a newer version marks the category synced. Distinct snapshots are retained on purpose, which the recovery document already states.

Validation: hosted Run Tests, Run Integration Tests, SwiftFormat, and e2e-tests-local completed successfully on this commit. This review inspected the changed recovery, restore, retry, and test source and did not re-run those checks. J1 was not executed here; it still depends on the controlled fixture described in the PR.

Device testing: not performed in this review.

Ready for device testing.

Reviewer model: grok-4.7
Reasoning effort: high

@jvsena42

Copy link
Copy Markdown
Member

Device gate (partial): e5aa577, iPhone 17 simulator, local usdt-fixture (Arbitrum fork) and a local VSS proxy.

  • The new "USDT (Tether)" row in Settings › Backup shows its last backup time, and it is still there after a restore.
  • One USDT change, receiving 25 USDT, wrote exactly one record (USDT_RECOVERY_V1_45b164…), seen in both the app log and the VSS proxy.
  • Reset wallet, then a seed restore, brought back the USDT balance ($25.00) and the +25.00 history entry with no Paykit state. It took 9.2 s to Get Started and under 10 s more for the USDT row; the VSS reads took about 0.25 s. I have no master baseline yet.

⚠️ Not run yet: a USDT payment and journey actions 1–7. The fixture's Arbitrum node ran out of memory mid-run, and after the restart the bundler's gas estimation returned 502, so no payment could go through. That is an environment failure, not the app. I'll retry it on the next pass.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The code scan had no HIGH or MEDIUM, and the device gate passed. The approval stands with one LOW open, inline at BackupService.swift:776 (a full-history record per transfer change): fix it here or in a follow-up issue, your choice; reply on the thread saying which.

Device gate: e5aa577, iPhone 17 simulator, local usdt-fixture (Arbitrum fork) and a VSS fault proxy.

  • A payment's backup is acknowledged (13:11:16.503) before the bundler submit (13:11:16.688Z).
  • A rejected backup and a lost ack (stored, but answered with a 500) both show the retry error with Retry/Details/Close, and nothing is broadcast.
  • Retry, close and relaunch while still rejected keep one signed payment. Once VSS returns, the same op is submitted automatically with no Retry needed (same id, userOpHash and operation sha256), and it confirms.
  • The backup row shows its last backup time and is still there after a restore. One USDT change writes one record.
  • Seed restore: the balance is right ($21.87). Sends come back Pending, showing the amount without the fee, and reconcile to Confirmed with their original tx hashes in about 36 s with no resubmission. That matches the Android twin and Android master.
  • Perf: the restore took about 55 s here because two staging backups-ldk requests took about 24 s each (environment). In round 1 it took 9.2 s.

⚠️ Not run: Pubky write-lock control, an interrupted restore, two writers, and the backup row's Failed/Retry state. They rest on the code scan and unit tests. There is no iOS master baseline: Xcode 27.1's license isn't accepted on this machine, so nothing could be rebuilt.

@ovitrif
ovitrif merged commit 681f5aa into master Oct 28, 2026
41 checks passed
@ovitrif

ovitrif commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

@ben-kaufman Merged, as it was approved and green.

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.

5 participants