Repository navigation
fix: separate usdt recovery backups from paykit - #1681
ben-kaufman wants to merge 11 commits into
Conversation
|
Triage: 3.1.0. It fixes shipped USDT recovery backups, and 3.1.0 is due 28 Oct. |
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — a delayed Retry failure leaks into a new send form.
The PR moved to f496f43 after this run; these results are for 2493dbb.
The PR moved to f496f43 during this run. These findings and results cover 2493dbb; the new head has not been reviewed here.
Findings:
- LOW — Discard retry feedback after closing the payment screen (
app/src/main/java/to/bitkit/ui/screens/wallets/usdt/UsdtViewModel.kt:275). Closing while Retry awaits VSS resets the same view model. The old failure then puts the preceding payment’s backup explanation into a fresh recipient form. A focused deferred-callback regression and the device reproduce this; the pending-transfer guard still prevents another payment.
QA for 2493dbb:
Tested on Pixel 8 / Android 16 with published Core 0.8.0-rc11 and VSS 0.5.26-rc1. The funded app run used a disposable Arbitrum fork and a test-only override for app backup endpoints; payment and recovery logic were unchanged. The encrypted backup protocol fixture does not verify LNURL-auth signatures.
Test 1: Rejected recovery uploads ✅ Instrumentation assertions verified the production callback preserves the original signed operation through restart and migration restore.
The extended protocol harness calls the production UsdtService backup adapter and BackupRepo, rather than the direct-store callback in the committed native test. Prepare, resume and restore each verified their own upload fault and no submission before acknowledgement. One unique operation and the complete-wallet sentinel were retained. These assertions exercise native signing and encrypted VSS, not deployed contracts or Paykit authorization.
T1.2x.mp4
Test 2: Lost recovery acknowledgements ✅ Instrumentation assertions verified committed encrypted records restore and retry the same signed operation.
The same production callback and migration path passed separate prepare, resume and restore processes with dropped acknowledgements. Each process required its own successful acknowledgement before submission; signed operation bytes and the complete-wallet sentinel were preserved.
T2.2x.mp4
Test 3: Funded app failure and recovery ✅ The app showed the backup error, then confirmed the original payment after restart and acknowledgement.
Rejecting VSS after approval showed Error, the same-payment explanation, Retry, Details and Close; Details showed Pending. The bundler count stayed unchanged while uploads failed, including after restart. After acknowledgement, the app showed Confirmed. SQLite comparisons preserved the application ID, operation hash and exact signed-plan bytes. The deployed-contract receipt contains one successful user operation and exactly one 1-USDT recipient transfer. A native readback then decrypted and decoded the complete WALLET envelope successfully.
T3-initial-evidence.2x.mp4
T3-relaunch.2x.mp4
T3-confirmation.2x.mp4
Test 4: Close during Retry ❌ The old retry failure enters a fresh recipient draft.
With a rejected upload in flight, I tapped Retry, Close and Send. The fresh Send USDT form first had no error, then displayed the preceding payment’s backup explanation as UsdtRecipientError. The focused deferred-callback regression fails on the same update. No duplicate payment was observed.
T4-evidence.2x.mp4
Test J1: Complete recovery journey ⏭️ The full authenticated, concurrent recovery journey remains incomplete.
The funded subset above covers ordinary backup failure and restart recovery. The running fork and VSS fixtures do not provide authenticated Paykit/Pubky fault controls. Genuine Paykit authorization/proof faults, interrupted full restore, concurrent production-app writers and the deadline-transition journey were not driven together. Protocol/component results do not complete J1.
Check 1: Build and static checks ✅ The unmodified head passed compilation, unit tests and lint.
compileDevDebugKotlin, testDevDebugUnitTest, assembleDevDebug, assembleDevDebugAndroidTest and just lint passed. Structured Detekt and Android Lint reports contain no findings. The added stale-retry regression was run separately and fails as described above.
Twin: bitkit-ios#1118 fix: separate usdt recovery backups from paykit matches the independent record format, restore order and same-operation Retry behavior at f82258e. Android additionally carries the explicit deadline journey action. I relied on the recorded earlier native cross-platform and screen-component results in the PR descriptions; those are not new full-J1 results.
Full review of 2b0c8a3..2493dbb, all 43 changed files and affected callers. Full-wallet retry scheduling, Paykit proof revalidation and concurrent-writer behavior retain component coverage; no complete device journey verified them here.
Reviewed by gh-pr-review-loop.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
Twin: synonymdev/bitkit-ios#1118
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
0.1.1-rc2with the validated lease, suspension and acquired-lock cleanup fixes. Pubky remains0.16.2; unmerged fix: recover native grant requests after 401 pubky/pubky-homeserver#741 is excluded.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.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.Out of Scope
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
The owned synthetic-funds fixture now has captured backup-failure UI and transaction evidence at
f496f436; the scoped results and failed restart settlement are recorded below. No production funds were used. Screenshots are retained with the test evidence, not embedded here.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, Paykite79b7bbband Pubky65a331e0bindings; these overrides are not the committed release pins.The previously used fork block
517943110is 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 Core fork provider at
c393f8e0with Anvil v1.8.6, chain42161, Prague hardfork and fork block517943110. Build withUSDT_RPC_URL=http://127.0.0.1:18545 USDT_BUNDLER_URL=http://127.0.0.1:18546; install on a new dedicated emulator and reverse only its ports18545/18546. Create a disposable wallet, use the provider'stest_fundWalleton its receive address, then send one synthetic USDT to0x1111111111111111111111111111111111111111. Cancel once, review again, confirm and inspect the receipt. Remove only that emulator's18545 reverse mapping before restarting to check saved activity offline. All funds and mutations remain in the local fork; normal VSS is used and no backup/Pubky faults are injected in this subset.Journeys
usdt-independent-recovery.xml— Pubky outage with working VSS, actual backup failure before broadcast, lost acknowledgements, repeated/automatic retry, closing/restarting, interrupted restore/migration and concurrent writers; shares the recovery expectations across platforms; Android also has an explicit deadline-transition action.Manual Tests
N/A — the controlled device cases are specified in J1; backend fault injection is an explicit prerequisite.
Automated Checks
UsdtNativeRecoveryIntegrationTest.kt— real encrypted backup rejection/lost acknowledgements, separate-process restart, fresh-database restore and the original signed operation before and after retry.UsdtRecoveryBackupStoreTest.kt— independent uploads, immutable concurrent writers, lost acknowledgements, byte-order/hash compatibility and fail-closed restore.BackupRepoTest.kt,PaykitUsdtPaymentRepoRecoveryTest.kt,UsdtRepoTest.kt,WalletViewModelTest.ktandUsdtViewModelTest.kt— restart/migration gates, retained payment progress, proof revalidation and same-transfer Retry/automatic recovery.Earlier released VSS validation
At signed
c1748c876, normal Gradle resolution adopts published VSS0.5.26-rc1. AAR SHA25633bc7f5ef9202fb183c95ebaf85c83b1afac06235d17d4d48ad20c53b810724ematches the cache and installed app native bytes; all four ABI build IDs match the published symbol archive. Final 4,604 fresh units across 254 suites passed with zero failures/errors/skips; compile, formatting, full Detekt and Android Lint passed with zero findings. Complete logs and structured reports were inspected; the JVM and Gradle notices match recorded base evidence. Both installed APK hashes were verified without clearing wallet data.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 seven 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
cc14496ed, requiredjust compile,just testandjust lintpassed on macOS ARM64 with the committed library coordinates. Complete JUnit reports contain 4,602 passing tests, zero failures or skips. Detekt SARIF and Android Lint XML contain zero findings. The final test/lint run had unchanged source content.Validation caught and repaired a restore-gate early-return type error and a stale test expectation that trusted imported proof. The repaired tests preserve payment-started guards while requiring proof revalidation. Journey XML parses. Independent review and bounded repair rechecks found no remaining actionable issues.
At test commit
2d8f8747, requiredjust compile,just testandjust lintpassed again with committed dependency coordinates: 4,602 tests, zero failures/skips, Detekt SARIF zero findings and Android Lint XML zero findings. With the combined local SDK build, native rejection and lost-acknowledgement sequences passed prepare, resume and restore; bidirectional iOS/Android encrypted snapshot restore also passed. The instrumented native case calls the recovery store directly; BackupRepo orchestration is covered separately by unit tests.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
258bf9e5b, normal Gradle resolution uses published Core RC11. The downloaded AAR SHA256 is9d5d54f5761b7773d2cfaaf0be77eeb914348d52e31ebf81684d7e761ce68b5b, matching Gradle's resolved artifact and the installed app's native library. All four ABI build IDs match the published debug-symbol archive. Required compile/lint checks passed with zero Detekt SARIF or Android Lint XML findings. The initial unit invocation reused cached output, so an explicit fresh execution was run: 4,602 passed, zero failures/errors/skips across 253 suites. Existing Gradle plugin deprecation and JVM CDS diagnostics are recorded separately.The normal published-dependency app and instrumentation APKs were installed on the owned Android emulator and their installed bytes verified. Native rejected-backup and lost-acknowledgement cases each passed prepare, resume and restore. Fresh iOS-to-Android and Android-to-iOS encrypted restores passed with iOS
646670df; all 16 released-dependency native phases preserved the original operation and acknowledgement-before-submission ordering. All four PR commits are signed and GitHub reportsverified=true,reason=valid. Current-head hosted Detekt and Android Lint also passed; their uploaded SARIF/XML reports contain zero findings. Draft-only build/E2E jobs were skipped and are not counted as executed tests.At acknowledgement fix
dc5a47d90, a successful refresh with no backup callback no longer clears recovery feedback. The UI reads live acknowledgements from the native backup adapter, including when a prior payment was already acknowledged. Failed persistence and no-op automatic/manual retry retain the error and Retry action. Regressions fail on unchanged258bf9e5band pass with the repair. Final requiredjust compile,just testandjust lintpassed: 4,604 tests across 254 suites, zero failures/errors/skips; complete Detekt SARIF and Android Lint XML/SARIF reports contain zero findings. Source inputs were unchanged through validation and independent re-review is clean. All five commits against the live base are signed and GitHub reportsverified=true,reason=valid. This follow-up has component/adapter coverage; the earlier native protocol evidence remains tied to the commits above.Hosted lint for
dc5a47d90passed in run40123893660. The downloaded artifact checksum matches GitHub metadata; complete Detekt SARIF and Android Lint XML/SARIF contain zero findings. Build and E2E jobs remain skipped by the draft guards and are not counted as passes.A fresh normal-dependency app and instrumentation build at
dc5a47d90also passed. Both installed APK hashes were verified on the owned Android emulator, without clearing wallet data. All tenUsdtPaymentContentTestdevice cases passed, including submitted/confirmed transitions, closing the payment screen and comma-region input. This is screen-component coverage; it does not execute a full payment against the backend.At
dc5a47d90, a separate full-app run on a newly created Pixel 8 / API 36 ARM emulator used published dependencies with only USDT RPC/bundler URLs pointed to the owned loopback Anvil fork. The exact installed APK SHA256 was02606962f5493af2e32c05219df6888e42f0c10e7ea9d2789aebafb490902cc7. After synthetic funding, canceling the first fee warning submitted no operation. Repeating review and explicitly confirming sent one USDT; the UI showed Confirmed. The local deployed-contract receipt contains one successful user operation and exactly one recipient token transfer, with zero native ETH in the sender. Restarting with the chain RPC unavailable preserved the saved activity. The full build log had no compiler diagnostics; the sole structured Gradle deprecation exactly matched the prior normal-dependency build. This exercises the real app and BackupRepo success path, but does not inject VSS/Pubky faults or complete J1. Android returns to Amount after canceling the fee warning; iOS retains Review. Both preserve the amount and submit nothing before approval.At signed
071fe7d7, the typed deadline-expiry explanation and title survive the same saved transfer’s Pending → Failed transition. Confirmation, replacement and later bridge states still supersede stale feedback. The pre-fix regression failed and all 28 focused cases pass after repair. Final compile, formatting, full Detekt/Android Lint and 4,604 unit tests across 254 suites passed with zero failures/errors/skips and zero structured lint findings. An actual installed Compose case passed the pending/failed/replaced/confirmed title and body transitions; both APK hashes were verified without clearing app data. Two consecutive independent final reviews are clean. All six current-base commits are locally verified and GitHub true/valid. iOS parity is in synonymdev/bitkit-ios#1115. This is component coverage, not a new complete J1 pass.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. An unreleased Paykit/Pubky upgrade and the separate future full-wallet split are not blanket prerequisites. A dependency will be recorded only if a required case demonstrates it. Core #179 and RC11 preparation #181 are merged; published Core RC11 is adopted and validated in both native recovery PRs. 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 portion is now released and adopted as described below, while the separate Pubky #741 artifact remains unavailable. No app release or deployment is part of this PR.
The later device-sleep lease repair at Paykit
461864a4passed 1,557 Rust tests, both native binding builds and full generated Android checks, and is included in the later published RC2 adoption below. The combined app/native evidence above used Paykite79b7bbb. 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 (f496f43)
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/vssandE2E_LNURL_AUTH_SERVER_URL=http://localhost:18655/authbefore building/launching; Android additionally requires device-local port forwarding for both ports. Both values are required together, must uselocalhostwith 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
f496f436: 4,617 fresh unit tests across 255 suites passed with zero failures/skips; compile, full formatting, Detekt and Android Lint 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.Latest retry-screen repair and authenticated fixture evidence
At
5640fad6e, a recovery Retry that finishes after Close/Send no longer writes its old backup failure into the fresh recipient form. The result and loading cleanup remain bound to the original submitted quote. The repository retry continues for the retained signed operation; no new payment or cancellation is introduced. A deterministic deferred-result regression failed before the fix. Finaljust format,just compile,just testandjust lintpassed: 4,618 tests across 255 suites, zero failures/errors/skips, and zero findings in the complete fresh Detekt and Android Lint SARIF/XML reports. Two consecutive independent final reviews were clean. The matching journey action is included in both native apps. This regression is component evidence; its new device step remains unchecked.At earlier
f496f436(installed APK SHA-25623979eef90fe4eafc3cb7df6497a0cdf6203201233d7b6db7a2ba69cace14ac0), the disposable Android API 36 fixture used the committed released Core RC11, VSS 0.5.26-rc1 and Paykit 0.1.1-rc1, real native LNURL authorization/encryption, an owned RS256 VSS/PostgreSQL service and a separate Anvil fork at Arbitrum block 518486328. Its Pubky proxy tunnels were blocked while the UI sent one synthetic USDT. Recovery acknowledgement preceded the sole bundler submission; the deployed-contract receipt confirmed the payment. The complete WALLET ciphertext sentinel remained unchanged.A second signed payment was rejected at its exact authenticated recovery-record selector. Two manual retries, Close and a cold app restart retained identical operation bytes, with no submission before acknowledgement. Once acknowledgement was permitted, that same operation was submitted once. Settlement failed: its synthetic paymaster authorization had expired and the receipt reverted with
AA32 paymaster expired or not due. This is not a successful restart-settlement pass. The original pending operation, wallet and fork are preserved; no replacement quote or re-signing was performed. All fault controls were restored healthy after capture.These observed portions do not complete J1: authenticated lost-acknowledgement, fresh remote restore/migration, concurrent writers, preserved Paykit authorization/proof behavior and the new close/reopen action still need their own final-head acceptance. The first successful outage transfer is not evidence that the second transfer settled. Local gates above validate the repaired branch; the later live master layout merge was inspected but is not claimed as a tested synthetic merge.
Published Paykit RC2 adoption — 7c21884
Normal Gradle resolution now uses published
com.synonym:paykit-android:0.1.1-rc2. Its AAR SHA-256eba55ef4b1895f1c9e615440579be3a94f14bfed26c73509be45bd5d57c9ae2ematches the downloaded publication and resolved cache. Installed app/test APKs match the built bytes; installed arm64 Paykit matches the published AAR. Core RC11 and VSS RC1 remain unchanged. This one-line pin change does not add the new payment-request preparation API or broader credential-error classification.Final 4,618 fresh unit tests across 255 suites passed with zero failures/errors/skips; compile, full formatting, Detekt and Android Lint passed with zero findings. Complete logs and structured reports were inspected. Three JVM notices, four cursor CloseGuard records and the Detekt Gradle deprecation match verified prior evidence. Two consecutive independent final reviews are clean. All eleven commits against the live base are locally verified and GitHub reports
verified=true,reason=valid.Both released apps passed 16 fresh native phases: rejection/lost-acknowledgement prepare/resume/restore and encrypted restore in both directions. Each process acknowledged its own record before its first submission attempt, retained the original signed bytes and exactly one unique operation, and preserved the complete WALLET sentinel. Repeated refresh deliberately exercises two attempts with that same operation; this is not a single-attempt claim. These use real native signing/encryption and loopback protocol/mock authorization, not a complete authenticated J1 or deployed-contract settlement.
The initial Android install failed because the package service was unavailable; one scoped cold boot preserved userdata, and unchanged APKs were then installed and verified. An earlier cross-restore crashed during ART instrumentation initialization before test execution. Its bounded retry reached an old fixture operation after expiry and correctly marked proven nonexecution Failed; the expected-backup-failure assertion was no longer applicable. Both failures and the original operation remain preserved. A separate fresh public-seed scenario passed the opposite-platform restore; no old operation was re-signed or wallet reset. Published artifact provenance also records NDK r27d despite the release workflow's r28c setting; no replacement or reproducible-binary claim is made.
This PR remains draft with its existing J1 gaps. Current-head hosted checks are pending; draft-skipped jobs are not passes.
Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)Latest hosted validation (2026-10-26)
Hosted lint passed on current
7c218843; the complete logs and checksum/CRC-verified artifact12682111680contain zero Detekt and Android Lint SARIF/XML findings. Its synthetic merge54df56e7includes mastereb902f52, and lint compiled that composition, whose tree differs from the head. Native/unit/E2E jobs were draft-skipped, so no hosted native execution of that merge or completed J1 is claimed.