Skip to content

fix: prevent false on-chain send success - #1384

Open
ovitrif wants to merge 73 commits into
masterfrom
codex/1211-explicit-broadcast-outcome
Open

ovitrif wants to merge 73 commits into
masterfrom
codex/1211-explicit-broadcast-outcome

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1211
Twin: synonymdev/bitkit-ios#844
Refs:

Description

Hardware Shop payments persist the signed receipt and original private boundary before endpoint consumption. Interrupted unsigned software preparations release the saved boundary before removing their proof.

  • Preserves the prior consumed private payment-list boundary with the original receipt and backup, so cancelling an unsent version cannot reopen older payment details.
  • Preserves the original consumed private payment-list version in proof backups and releases it durably before removing a definitely unsent hardware Shop proof; failed cleanup remains retryable after restart.
  • Restores whether the original hardware Shop transaction ever reached dispatch; interrupted authorization or expiry before dispatch clears only its exact unsent proof, while attempted payments stay guarded.
  • Keeps a failed hardware Shop broadcast guarded through Back/cancel so an explicit retry reuses the original signed transaction, payer and request.
  • Defers wallet backups while any local on-chain attempt lacks a signed receipt, preventing restoration of an unrecoverable unsigned guard.
  • Keeps the exact signed hardware payment guarded when candidate persistence fails, allowing a save retry without another signature.
  • Keeps an accepted retry Pending when its acceptance cannot be saved, unless the exact operation already has durable positive evidence.
  • Revalidates the original funding address, client balance and service fee before confirmation resumes an accepted transfer.
  • Preserves same-operation progress when retrying an interrupted backup restore without replacing its original payment context.
  • Keeps manual contact detachment protected when a contact assignment fails to persist.
  • Integrates shared-state Paykit 0.1.0-rc69 with LDK 0.7.0-rc.70, retaining original payment guards, captured proof app IDs and cross-platform backup state.
  • Checks payment deadlines at native preparation and dispatch; expiry after a prior broadcast retains the original guarded payment.
  • Original-payment retry preserves the original request deadline through native preparation and dispatch, and rejects a changed deadline after authentication. Expiry before dispatch retains the original guarded payment; the inclusive deadline remains valid.
  • Saves the Shop payment guard before consuming request details. Interrupted unsigned preparation can be retried after restart only after durable cleanup of its original proof; live preparations, restored guards and signed candidates stay protected.
  • Fixes unsent private Shop request retries by releasing consumed details on proven pre-dispatch failures; subscription proof errors retain their retry screen.

Required for Bitkit 2.6.0 Shop support. Open for review so app and native dependency reviews can proceed in parallel. Dependency approval, remaining feedback and funded Paykit/server acceptance are still required before merging. The merged broadcast-result API is extended by LDK rc70 so the original signed transaction ID, actual inputs and recipient amount are durable before submission.

  • Uses explicit LDK broadcast outcomes for normal sends, send-all, transfers and Shop payments so a transaction ID alone never produces success or payment proof.
  • Persists one active payment guard and its signed receipts before submission. Pending offers an explicit authenticated retry of the original recipient amount using only the original inputs; all candidate IDs remain guarded across restart and wallet backup, and an accepted or observed original winner cannot be overwritten by a later failure.
  • Preserves accepted transaction IDs after local storage/activity/proof failures and resumes local follow-up without creating another payment. Reconciliation requires independent observation of the exact transaction ID.
  • Preserves original transfer amount and balance context, keeps a new unsent payment separate from an earlier accepted attempt, and saves verified acceptance with Shop proofs so delivery can resume after the guard is replaced.
  • Requires fresh observation of the exact outgoing hardware transaction in the original wallet before Shop proof, Sent activity or Success. Missing observation keeps the original request pending; its saved identity, wallet and transaction cannot be replaced by current screen context.
  • Retains a started hardware Shop guard after a candidate-save failure or a Core exception with no returned transaction ID, so dismissing and reopening cannot authorize a replacement payment.
  • Resumes accepted or exactly observed funding at startup and node events from the saved original order and balance context. Transfer and paid-order persistence are idempotent; guard completion requires durable local activity and never broadcasts again.
  • Preserves paid-success navigation after the original transfer and paid order are durably saved, even if subsequent local activity completion fails. Existing resumption repairs that follow-up without funding a second order; failed funding persistence still retains the original attempt.
  • Routes proven pre-admission failures to existing error handling while retaining unresolved protection after dispatch starts.
  • Shows candidate transaction IDs and refusal reasons in Pending, with Details for an exact original-wallet local activity. That local record never proves acceptance or releases the guard.
  • Uses matching published LDK 0.7.0-rc.70 prepared-send bindings; independent Maven resolution and current built APK native bytes were verified; earlier rc69 installation was verified before the backup follow-up.
  • Includes the active payment guard in the shared wallet backup, validates its original wallet/network and proof association before restore, and resets local acknowledgement so restoration cannot silently unlock another send. Restored contact attribution is saved before acknowledgement, without overwriting a later contact edit. Missing or invalid follow-up remains guarded.
  • Updates Pending from the exact durable original payment after positive evidence and local completion. Transfer recovery opens the original funded order only after its saved follow-up completes; a late uncertain retry result cannot replace the winner.
  • Keeps recovery errors scrollable while Retry, Details and Close remain visible and separated.

Why

Required for Shop support in Bitkit 2.6.0: buyers must be able to tell whether an on-chain payment was accepted and recover an uncertain checkout without paying twice.

  • Prevent false success: Bitkit must not show “sent” or deliver payment proof when the backend rejected the transaction or acceptance is still unknown; the Shop order may remain unpaid.
  • Prevent accidental double payment: reopening or retrying an uncertain checkout must retain the original payment instead of starting a separate payment.
  • Preserve the merchant amount on retry: use only the original inputs and recipient amount; a Max payment without enough fee headroom must fail safely rather than reduce what the merchant receives.
  • Resolve stale Pending state: once the exact original payment succeeds and its local follow-up is saved, the app must reflect that result.
  • Preserve protection after wallet restore: restoring a backup must retain the unresolved payment guard so it cannot silently authorize another send.

These are release acceptance requirements. The app PRs include the selected Paykit updates for 2.6.0. Final validation must pay a Shop order with these app builds and verify that the merchant receives the payment proof and the order becomes paid on the existing Shop server.

  • Defers the complete wallet backup while a Shop preparation has no signed receipt, keeping private payment-list consumption and payment guards together. Upload completion acknowledges its captured snapshot; newer changes remain queued.
  • Clears the exact hardware candidate after a definite first queued dispatch expiry before releasing its preparation; prior uncertain submissions and accepted proofs remain guarded.
  • Rejects mixed private-consumption/SDK wallet snapshots and resumes positively evidenced Shop proof follow-up after complete restore, without waiting for another node event.
  • Consumes a matching completed hardware result after asynchronous reconciliation so future sends do not replay the previous payment.

Out of Scope

  • Legacy private Paykit cache/backup receiver-path migration: bug: opted-in private payment state is lost on upgrade #1431; raised in fix: prevent false on-chain send success #1384 (comment).
  • Transaction recovery: historical journals, raw transaction storage, automatic retries, replacement input selection, abandonment and general RBF recovery.
  • Chain handling: reorg and event-delivery redesign.
  • Unresolved sends: no timeout/reset escape. Missing original provenance or insufficient fee headroom keeps the payment guarded. A higher-fee retry cannot reduce the merchant amount or add other inputs; backend acceptance does not guarantee confirmation.
  • Legacy software proofs: queued-era transaction IDs without positive acceptance and original wallet provenance are not promoted or delivered. Opted-in released users can have these records; migration is excluded from this PR, with the documented limitation accepted in review.

Design

N/A — no design available for the new unresolved-send state.

Preview

Prepared-send Pending preview before the backup follow-up: actual published rc69 native preparation in a funded regtest wallet, with Unknown injected before broadcasting. This shows the retained 1,000-sat original receipt and explicit retry action; it does not reproduce backend refusal or response loss.

Original payment retained (rc69 fixture)

Historical rc68 candidate before the current feedback batch: fixed 1,000-sat and Max 98,749-sat regtest sends using the actual published and resolved LDK package. Both exact transaction IDs matched native Accepted logs and independent backend observation.

Earlier Pending preview at 0ef4613: synthetic component UI only, with dummy transaction IDs/refusal text and mocked fiat value. It shows refusal copy, a selectable candidate ID and local Details without success; it does not reproduce backend refusal or persistent reopening.

Fixed: Sent (historical) Fixed: Details (historical) Max: Sent (historical) Max: Details (historical) Pending (component) Details (component)

QA Notes

Current master integration

  • updated shared-state Paykit to 0.1.0-rc69 from the merged Paykit PR; LDK remains 0.7.0-rc.70.
  • ran local verification of the merged payment recovery, hardware coordinator, backup and Paykit scheduling changes.
  • Fresh-wallet Shop request/proof VSS restore and final funded merchant order/proof checkout on these heads.
  • Physical hardware payment path and current Android expiry device journey.

Private hardware cleanup (b71aff7)

  • Encoded proof backup preserves the original consumed private payment-list version alongside the signed receipt and dispatch marker.
  • Definite pre-dispatch cleanup releases that exact version before deleting the proof. Failed private-state storage or proof removal retains the original proof for safe, idempotent retry after reopening.
  • Local verification: affected proof, hardware-send, app send-flow, wallet-backup and wire-format checks and Android build completed. The regression failed before correction; physical hardware, fresh-wallet Shop VSS and funded merchant checkout remain unrun.

Hardware dispatch-state recovery (91351de)

  • The original signed receipt starts with a durable false dispatch marker. The marker is saved before native submission and restored together with its original payer/request/wallet context; missing dispatch evidence remains guarded.
  • Definite authorization denial or expiry before the first dispatch removes only the matching unsent signed proof. Attempted and uncertain payments retain their original receipt and require explicit authorized recovery.
  • Local verification: affected hardware/proof checks and application build completed. Encoded backup/reopen covers both unattempted cleanup and attempted preservation; restored authorization cannot sign another transaction. Physical hardware, fresh-wallet Shop VSS and funded merchant checkout remain unrun.

Hardware authorization recovery (3925ef2)

  • The original signed hardware receipt, derived transaction ID and fee metadata are persisted in the same write that starts the Shop proof, before authorization can suspend.
  • Preparation receives the exact signed object; a Shop request without it cannot start. Restart or backup during authorization retains the original receipt for authenticated retry without another signature.
  • Updated PaykitPaymentProofRepoTest, HwSendViewModelTest and AppViewModelSendFlowTest; the authorization-boundary backup regression reproduced the missing receipt before correction. Local verification: affected checks and Android build completed; physical hardware and fresh-wallet Shop VSS acceptance remain unrun.

Current review corrections at 59df4f8 / 5843da2: direct-send fallback validates original attempt and wallet; hardware Shop Pending retains its result until proof completion. Missing proof app IDs remain readable and retained, and cannot be submitted with invented provenance. Focused repository, hardware-send and send-flow checks and the app build completed. Physical hardware proof-save failure, remote restore and final Shop order/proof checkout remain unvalidated.

Journeys

  • new onchain-original-payment-retry.xml — Pending → approve fee and normal payment authentication → retry the same recipient amount and input set; persist both candidates, keep uncertainty guarded and finish only the accepted/observed winner. Spec parsed; current funded retry journey remains unrun.
  • new shop-onchain-proof.xml — linked issuer and funded Bridge hardware wallet: exact original transaction and delivered proof, with no new signing/payment while observation is pending. Spec parsed; native hardware journey unrun.
  • new onchain-accepted-result.xml — accepted fixed-amount and send-all finish local activity and expose distinct exact transaction IDs in Details.

Manual Tests

  • Controlled non-final refusal → Send Confirm → Pending → no success/proof/paid order and no fresh send after reopening — controlled native broadcast-refusal fixture not in Capabilities.
  • Lost response or process death after dispatch → reopen the same Shop request and switch payment methods → no second payment — native response-loss/dispatch-synchronization fixture not in Capabilities.
  • regression: fail local storage/activity/proof/transfer follow-up after Accepted → reopen and finish the original local operation → same transaction ID without another broadcast — storage-fault injection at the send boundary not in Capabilities.
  • regression: fail local metadata write/readback after accepted funding and durable paid-order save → Transfer → Setting Up without a payment-failure toast or another confirm swipe → original activity resumes without a second funded order — deterministic SQLite fault injection not in Capabilities; unrun.
  • regression: physical Trezor Send → approve transaction → original hardware payment and proof survive transport changes — physical USB permissions/enumeration and BLE transport not in Capabilities.

Automated Checks

  • updated AppViewModelSendFlowTest.kt — recovering an older blocked payment never assigns the new recipient’s contact to it.

  • updated BackupRepoTest.kt — an older unresolved backup resumes retained local acceptance after original context is restored, without requiring another transaction event.

  • updated ActiveOnchainAttemptBackupTest.kt and BackupRepoTest.kt — missing original follow-up context rejects restore before installing a blocking payment guard.

  • updated ActiveOnchainAttemptBackupTest.kt, BackupRepoTest.kt and OnchainSendAttemptStoreTest.kt — accepted replacement backups require the winning candidate’s fee rate; missing/partial maps fail restore, valid replacement rates and original-candidate fallback remain supported.

  • updated HwSendViewModelTest.kt — exact-wallet completion after a hardware Shop broadcast timeout clears the retained send guard and preserves the original amount/request/payer; unrelated observations cannot clear it. Build, unit and lint checks completed; the funded hardware journey remains unrun.

  • updated LightningRepoTest.kt — a retained accepted result after a broadcast failure stays Pending while durable repair fails, including after reopening; no second native send is prepared.

  • updated ActivityServiceTest.kt — accepted-payment restoration preserves a deliberately removed contact and allows local follow-up to finish. The previous behavior reattached the original contact in the regression. Local verification: Debug build, unit tests and lint. Device and merchant acceptance remain outstanding.

  • updated ActiveOnchainAttemptBackupTest.kt and LightningRepoTest.kt — unsigned backup attempts are rejected; retry preserves Pending when the accepted winner is not durable, without authorizing or dispatching another payment. Both defects reproduced before correction. Local verification: Debug build, unit tests and lint. Fresh-wallet Shop restoration and funded merchant acceptance remain outstanding.

  • ran PaykitPaymentProofRepoTest.kt and HwSendViewModelTest.kt regressions: definite software preparation failure retains the proof until its original private boundary is released; a restored signed hardware receipt blocks ordinary and other-request signing from the same wallet. Required local build, unit tests and lint passed at bcc22b1.

  • ran LightningRepoTest.kt and ActivityRepoTest.kt regressions: failed accepted-outcome persistence and failed repair stay Pending across restart; contact-assignment retries finish marker cleanup and replacement propagation. Required local build, unit tests and lint passed at 4306d22.

  • updated HwSendViewModelTest.kt and PaykitPaymentProofRepoTest.kt — signed hardware Shop receipts survive encoded backup/repository restoration; explicit authorized retry broadcasts the original bytes without another signature. Wrong payer/wallet/address/amount cannot load the receipt. Restoration itself does not broadcast or prove acceptance.

  • Hardware Shop connectivity failure retains its signed payment across cancellation and explicit retry, with one signature and the same original payer/request. The regression failed before correction; affected hardware-send/proof tests, app build and changed-line static analysis completed. Physical hardware remains unrun.

  • Non-connectivity hardware Shop broadcast errors also retain the original signed payment through cancellation and retry. The invalid-transaction regression failed before correction; affected hardware-send/proof checks, app build and changed-line static analysis completed. Physical hardware remains unrun.

  • added BackupRepoTest.kt — unsigned ordinary sends and transfers defer the entire wallet snapshot before any remote write.

  • added HwSendViewModelTest.kt — save denial and storage exceptions preserve the signed payment across cancellation and retry it without signing again.

  • added OnchainSendCoordinatorTest.kt — accepted retry storage failure stays Pending with the original candidate family after restart.

  • added TransferViewModelTest.kt — changed funding address, client balance or service fee cannot complete the retained transfer or trigger another send.

  • updated OnchainSendAttemptStoreTest.kt — repeated restore retains an imported operation’s progressed fee and completion; changed payer or recipient is rejected, and a first import without follow-up context remains guarded.

  • updated ActivityRepoTest.kt — a failed Core contact write leaves the durable manual-detachment marker intact. Both new regressions failed before correction.

  • updated OnchainBackupRestoreDeviceTest.kt — verifies the remote wallet payload contains the accepted attempt, deletes its local attempt record and checks it is absent, then restores the original wallet, transaction, amount, exact inputs, candidate IDs and completed follow-up from VSS. Corrected device replay passed. The earlier completed-attempt replay was a false positive because backups excluded its completed receipt. This remains an ordinary-payment test on the same wallet, not fresh-wallet Shop request/proof recovery.

  • ran repaired shared-state Paykit device fixtures at 5f800c2: app/test APK compilation and drawer/Pending component checks passed on Android 16, with exact installed app APK bytes verified. Removed duplicate profile arguments and obsolete settings dependencies. These are component checks, not funded PIN/recovery or merchant checkout.

  • updated PaymentDeadlineSubmissionTest.kt, AppViewModelSendFlowTest.kt — queue-time expiry, expiry after immutable preparation, and exact original hardware cancellation with no release after dispatch.

  • ran local JVM suite and detekt against Paykit rc65 and hosted LDK rc70; current funded device, hardware and Shop checkout journeys remain unrun. Detekt retains its existing ignoreFailures=true configuration.

  • ran native rc70 consumer compilation and broadcast-outcome/recovery checks against the hosted Maven artifact with local Maven repositories excluded.

  • updated OnchainSendAttemptStoreTest.kt, PaykitPaymentProofRepoTest.kt — durable admission precedes request consumption; failed writes preserve request details; restart cleanup preserves live, legacy and signed attempts, and proof-removal failure retains the guard.

  • added ActiveOnchainAttemptBackupTest.kt, updated PaykitPaymentStateBackupTest.kt, BackupRepoTest.kt — shared golden wire, wallet/network/proof rejection and guard-before-proof restore.

  • added OnchainSendCoordinatorTest.kt — exact original amount/input retries, original winner races, payer change and UInt32 fee bounds.

  • added OnchainSendAttemptStoreTest.kt — serialized admission, durable guards, outcome persistence and exact transaction observation.

  • added SendPendingScreenTest.kt — refusal/candidate visibility and enabled local Details callback while remaining Pending; component checks do not reload durable state.

  • updated LightningRepoTest.kt, LightningServiceTest.kt — explicit accepted/rejected/unknown mapping and pre-dispatch boundaries.

  • updated AppViewModelSendFlowTest.kt, PaykitPaymentProofRepoTest.kt, TransferViewModelTest.kt — proven pre-admission errors, accepted follow-up failures, request protection and original hardware proof identity.

  • updated AppViewModelSendFlowTest.kt — permission denial, captured request/identity across asynchronous hardware authorization, and started-proof preservation after retry denial/cancellation.

  • updated TransferViewModelTest.kt — fresh Accepted and resumed Accepted/Observed funding retain one order/send and paid-success navigation after local activity failure; failed funding persistence retains the original order without success.

  • updated TransferRepoTest.kt, SendPendingViewModelTest.kt — startup/event funding resumption, partial storage idempotency and original-wallet Details without acceptance inference.

  • updated HwWalletRepoTest.kt, HwSendViewModelTest.kt, ActivityRepoTest.kt — exact outgoing original-account observation, Sent activity durability and one native broadcast while proof completion is pending.

  • removed PaykitOnchainPaymentProofLookupTest.kt — address/amount matching no longer establishes that an interrupted attempt was accepted.

  • ran remote dependency validation with Maven local excluded — 0.7.0-rc.68 resolved AAR SHA-256 0cee2079291260aea12bf60ac9c8af8f459d9f24c3327d4cb021e5686035aa2f, identical to the published canonical artifact.

Recovery completion and identity (7984a64)

  • A retry returning Accepted keeps Pending visible until the exact original operation has durable local completion; both the immediate result and update observer apply the same identity/completion checks.
  • Restoring an already accepted ordinary payment resumes its original local follow-up after all restore context is installed, without requiring a new node lifecycle/event. Unknown and Shop request attempts do not enter this ordinary repair route.
  • Hardware observation verifies the original recipient outputs and exact amount before creating Sent activity or permitting a Shop proof; unrelated outgoing transactions are refused.
  • Each reported defect was reproduced by its focused regression before correction. Pending, backup, hardware-observation checks and application compilation completed afterward. Remote VSS and physical hardware acceptance remain unrun.

Restart-safe Shop preparation (5a07a0f)

  • Wallet backups exclude a never-dispatched preparation and its exact empty started proof together, while preserving signed candidates and unrelated proofs. Snapshot capture serializes with receipt retention.
  • Hardware Shop payments persist their exact signed candidate ID before broadcast. Reopening can reconcile that original candidate; persistence failure prevents dispatch, and candidate retention alone cannot create Sent activity or a delivered proof.
  • Focused backup, receipt ordering, response-loss, persistence-failure and repository-reopen checks completed. Physical hardware and funded merchant/channel validation remain unrun.
Historical validation before the shared-state Paykit integration

Local verification at 56398e3: master b29956a was integrated without manual edits; compilation and 484 focused tests passed (0 failures/errors/skips). Published rc68 AAR identity was verified with Maven local excluded. Detekt completed with ignoreFailures=true and 577 reported findings; it is not warning-free. All finding locations map to existing parent lines, which does not prove identical prior structural findings. Full-suite, device, native-fault and physical-hardware checks were not repeated; heavy hosted integration CI remains deferred to release.

Earlier verification at 62c32cc: compilation and 470 focused tests passed (0 failures/errors) against unchanged published rc68, with Maven local excluded. The identity-switch-during-authorization regression failed before the captured identity/context checks and passes now; captured callbacks cannot authorize a replacement request. No device, native fault, physical hardware or contact-deletion journey was run on this merged source.

Earlier verification at 6c75463: compilation and 169 focused tests passed (0 failures/errors). All three new duplicate-funding regressions failed on the preceding production source by funding a second order; the persistence-failure regression remained fail closed. Maven local was excluded and the resolved AAR hash matches unchanged published rc68. No device or storage-fault journey was run for this fix.

Earlier verification at 0ef4613: compile, 3,079 unit tests (0 failures/errors/skips), app/test APK builds and two emulator component tests passed. Startup funding and pre-admission regressions failed before their fixes. Detekt exited successfully with ignoreFailures: 570 findings remained, none on added or changed feedback lines. The installed APK’s native library matched rc68. Full-suite, Detekt and device checks were not repeated for 6c75463 or 62c32cc.

Prior rc68 validation: 95 affected tests and funded fixed-amount/Max native journeys passed with independently verified exact backend transactions. Those native journeys were not repeated for this batch. Controlled native refusal, response-loss, storage-fault and hardware Shop journeys remain unrun; the new Preview is synthetic component coverage only.

Current integration verification at d252bcb: 925 focused tests passed; strengthened Shop rerun (203 cases), 96 Jade/Paykit integration tests and 13 emulator component tests passed. The regression for acknowledgement without durable original Shop activity failed on old behavior and passes after repair. Exact request/txid mismatches and identity switching cannot deliver or acknowledge another proof, and recovery never broadcasts. Test API integration repairs and final test-only formatting compiled successfully. Detekt exits successfully with ignoreFailures=true; existing findings remain, so it is not warning-free. Component refusal screens use synthetic fixtures, not a real native refusal. Final matched release-pair Shop, response-loss and physical hardware validation remain required.

Feedback verification at 5f7839a: both new pre-dispatch regression cases failed before the fix; 334 affected send-flow tests and compilation passed afterward. Detekt completed with existing findings and none on added lines. No devices or native fault fixtures were used in this batch; final recovery and release-pair validation remain outstanding.

Current recovery and backup verification: 307 affected checks passed against the published remote rc69 AAR before backup additions. Then 57 focused backup/metadata checks passed, including shared wire round-trip, wallet binding, original proof authorization/completion and restore ordering. The final UInt32 fee-boundary regression failed before its correction and passed afterward, along with current app/test APK builds. These overlapping counts are not additive. Built app native bytes match the hosted rc69 arm64 library; the current backup batch has not been installed or driven on a device. Detekt exited with ignoreFailures=true: one changed-line complexity finding remains, zero changed-line formatting findings. Hosted Maven publication completed successfully. Earlier funded preparation and Pending/fee/PIN observations seeded uncertainty before submission; they do not prove backend refusal, response loss or a successful retry. Current funded fixed/Max retry, independent Android native VSS vector, physical hardware and final Shop release-pair QA remain unrun.

Pending UI verification at 15f45fa: 16 focused JVM checks and 3 emulator component checks passed, along with app/test APK builds. The preceding view model failed the exact observed-successor regression; the preceding error layout failed the non-overlap assertion. Detekt exited successfully with ignoreFailures=true and no changed-line findings; existing findings remain. Component checks use synthetic state. Funded auto-navigation on this UI batch has not been repeated.

Native rc69 validation on preceding production head152b43d: five native fixture checks passed with installed hosted package bytes. A fixed1,000-sat retry with withheld backend acknowledgements remained Pending until independent exact successor observation and durable local completion. Native transport repeated the same transaction; no distinct second payment was observed. Max retained99,890sats and exactinputs: a higher fee failed before broadcast, then original-fee retry succeeded. Original uncertainty was seeded before the first broadcast, so this does not prove process-death recovery after initial dispatch. Native VSS derivation vectors passed. Physical hardware, native refusal and final production Shop merchant pairing remain unrun.

Published 5f8e105:

  • Accepted funding resumes only with the original order amount, fee and wallet; changed or missing terms leave it guarded.
  • Backup restore resumes the accepted transfer after the original guard and transfer state are restored, including an already-running node.
  • Definite authorization denial before the first hardware broadcast releases only the exact matching prepared proof after durable storage; attempted or uncertain payments remain guarded.
  • Attempt timestamps use the injected clock.

Validation: four regressions failed against the previous production behavior. 209 focused tests passed; final overlapping 31-case and 62-case checks passed, alongside app/test APK builds against published rc69 and detekt. Lint reported existing findings with zero introduced-line findings. Physical hardware, live backup restore and final selected-version Shop checkout remain unrun. Draft status remains unchanged.

Published 6c1a9cb: added the Pending observation device integration fixture. Two checks passed using the current production APK and published rc69. The shared backup reader preserved the original wallet, inputs and candidate family; injected exact-positive observation completed the production Pending callback with the original txid and amount. The fixture restored its original saved operation afterward. This validates the store/ViewModel/screen callback, not a native event, full-app success route, remote VSS restore or merchant checkout.

Lint/detekt and fixture APK compilation completed. Full accepted funding/VSS device restore, authenticated hardware Shop rejection, physical hardware and final merchant pairing remain unrun. Five new review findings are being addressed; draft status was retained at that validation point.

Published 66d32dd: preparation cancellation releases only an empty pre-dispatch guard; completing an older payment does not present the blocked new send as successful; uncertain transfer funding opens the original recovery surface; each candidate retains its authorized fee rate; proof reconciliation avoids the reversed attempt/proof lock order. The original order, payer, amount and exact inputs remain protected.

Validation: 96 focused tests passed, with meaningful prior-behavior failures for cancellation, misleading navigation, transfer routing, fee metadata, shared restore and lock ordering. App/test APK builds passed against published rc69. Detekt completed with 515 existing findings and zero introduced-line findings (ignoreFailures=true). The shared optional candidate fee-rate map passed the same golden vector as iOS. Current transfer recovery device routing/PIN, funded restore, physical hardware and final selected-version merchant checkout remain outstanding.

Published c7745fc: Accepted successors use the actual signed transaction input/prevout fee instead of the original fee. Missing or invalid evidence keeps follow-up guarded; verified fee/rate repair also updates an existing activity placeholder.

Validation: 174 affected JVM tests passed (0 failures/errors/skips), including exact previous-output calculation, missing/substituted inputs, invalid values and arithmetic overflow. The earlier stale-fee regression failed before the writer fix. Current transfer/PIN/device validation, four new review findings and final selected-version Shop checkout remain outstanding.

Published d3f3bec: both retry fallback paths verify original attempt and wallet identity. The prior behavior failed the later-winner regression; all 17 coordinator tests passed afterward. Three remaining review findings and current device/release-pair validation remain outstanding.

Published b9cf568: restored shared/iOS contact attribution no longer prevents completion of an accepted payment. The original contact fills only an empty activity contact; later edits remain unchanged. A failed contact write retains the payment guard.

Validation: the shared golden backup failed acknowledgement before the fix. All 175 affected JVM tests passed afterward (13 attempt-store, 159 send-repository, 3 activity tests; no failures/errors/skips), including contact write failure and later-edit preservation. Two remaining findings concern pre-prepare proof crash ordering and transfer Pending completion. Both apps still need matching rc70 consumer and current funded/Shop validation.

Published df05937: transfer Pending now observes durable completion of its exact original order, wallet, input set and candidate family, then opens the funded order. Accepted funding stays Pending until local follow-up is saved, and this navigation does not dispatch another payment.

Validation: the stale transfer Pending regression failed before the fix. Compilation and all 3,465 JVM tests passed before formatting; the final 355 affected tests and detekt passed after formatting. Detekt has 577 existing findings with zero findings on introduced lines (ignoreFailures=true). No device or merchant checkout was run for this batch.

The signed receipt and private boundary are persisted before hardware endpoint consumption. Fresh-wallet Shop request/proof recovery, current expiry device coverage, physical hardware and final merchant order/proof checkout remain outstanding; this PR remained draft at that validation point.

Original retry deadline verification (6686f9a)

  • The expiry-after-authentication regression failed on the previous retry behavior and passes with the original deadline forwarded into native preparation/dispatch.
  • Affected coordinator, send-flow, native service and repository tests passed; the inclusive deadline remains valid and expiry retains the original amount, inputs and payment guard.
  • Funded recovery, actual PIN navigation, hardware/restore and final selected-version Shop checkout remain outstanding.

Original input validation (fea3f42)

  • Original retries pass only the saved signed receipt’s exact outpoints to native preparation. A fresh output-list snapshot no longer prevents authoritative native validation.
  • Native preparation still resolves actual values and requires eligible wallet inputs; this change does not allow spent inputs, input substitution or general replacement recovery.
  • The absent-output regression failed before the fix. Affected repository, coordinator and native-service tests and app/test APK builds completed. Native refusal remains before authentication or candidate dispatch.
  • Current funded Android PIN/recovery is documented below; final Shop merchant checkout remains unvalidated.

Funded Android payment-PIN verification (fea3f42, native rc70, Paykit rc65)

  • A disposable funded wallet retained a real signed native receipt as deliberately seeded Pending; the original transaction was not broadcast by the setup fixture.
  • Attempting another payment opened the original Pending screen rather than showing success. Retry displayed the original recipient and 1,000-sat amount; cancelling payment PIN kept the same Pending transaction.
  • Explicit retry with the normal payment PIN reached Bitcoin Sent. Details showed a 1,000-sat payment and 141-sat fee.
  • After stopping the app, persistence verification retained the original input set, both candidate IDs, the accepted successor and completed local follow-up. The backend independently returned that exact successor with the saved inputs and 1,000-sat recipient output.
  • This verifies the seeded-Pending UI/authentication/persistence path, not real acknowledgement loss, remote VSS restoration, hardware signing or merchant order/proof completion. Those checks remain outstanding.

Real acknowledgement-loss recovery (2cb41e0, native rc70, Paykit rc65)

  • A funded native broadcast was accepted by the backend while an app-specific proxy suppressed the acknowledgement. Native returned Unknown and the original signed receipt remained persisted. This case did not inject the outcome.
  • The device reproduced a sync-ordering defect: blocked observation failed sync before opening recovery. The regression failed before the fix; the current app checks the retained guard before sync and opens that exact original Pending payment.
  • Restoring observation completed the original 1,000-sat payment to Bitcoin Sent without authorizing a retry, dispatching another transaction or adding a candidate. Persistence retained the original exact inputs/transaction ID and completed local follow-up. The accepted transaction was independently decoded from the backend.
  • Focused repository/send-flow tests and app/test APK builds completed. Remote VSS restoration, hardware signing and final selected-version Shop merchant order/proof completion remain unvalidated; both app PRs retained their draft gate at that validation point.

First-submission expiry (875a3cb)

  • A deadline failure before the first native broadcast is returned as definitely not dispatched. The original Shop proof is removed before the expired preparation guard can be cleared.
  • Previously submitted, uncertain, recovered and reopened signed candidates retain their guard; no absence-based release is added.
  • The initial-expiry regression failed before the fix. Focused attempt-store, repository and send-flow checks and the app build completed; the proof-cleanup failure case preserves the guard. Device validation of this new expiry path remains outstanding.

Current-head device component verification at875a3cb7: rebuilt and installed APK identity verified; SendPendingScreenTest.kt and DrawerMenuWidgetsTest.kt ran on Android emulator. This covers uncertainty presentation, guarded retry controls and exact-candidate Details availability; it does not certify funded expiry/replacement, remote restore, hardware or merchant proof/order completion. Those acceptance checks remain open.

Original-winner recovery accounting (a6ddeb0)

  • A funded PIN-authorized replay on 875a3cb exercised real backend acknowledgement loss, an authenticated same-input/1,000-sat retry rejected by the backend, and recovery of the independently observed original after restart.
  • That replay exposed a zero-fee Details entry despite its actual 141-sat fee. The correction requires the exact winning fee before local completion and repairs the activity entry; missing fee evidence keeps Pending guarded.
  • The accounting regression failed before correction; repository recovery checks and application compilation completed afterward. The corrected funded Details readback at a6ddeb0 shows the original 1,000-sat payment and exact 141-sat fee, also retained in the durable receipt. Expiry, remote VSS restore, hardware and merchant order/proof acceptance remain open.

Restart-safe hardware expiry cleanup (d463af9)

  • A definite first-dispatch denial retains the exact signed candidate and a durable cleanup marker. Reopening releases only the captured original private payment-list version before removing its proof; failed storage retains the marker for retry.
  • Denied operations cannot submit a proof or export a partial wallet backup. Prior broadcast uncertainty and accepted candidates remain protected.
  • The process-reopen regression failed before correction. Proof/private-state repositories, hardware send and app send-flow checks plus the Android build completed. This is local repository restart coverage; physical hardware and remote restore remain unvalidated.

Current validation gaps: the rc65 SDK fresh-grant remote request fixture and same-wallet ordinary-payment remote VSS restore passed. Fresh-wallet Shop request/proof recovery, funded channel recovery, current expiry device coverage, physical hardware and final merchant order/proof checkout remain unvalidated.

Payment storage recovery (8f4b85e)

  • Transfer records are captured under the guarded-attempt snapshot lock, so accepted follow-up cannot leave a backup missing both its transfer and active guard.
  • Native acceptance whose outcome cannot be durably saved remains Pending until the exact signed candidate is persisted or independently observed; stale requested amount and zero-fee metadata cannot produce success.
  • Contact cleanup retains immediate attribution invalidation and retries the durable removal after storage failure, including when its filtered cache already matches.
  • All three regressions failed before correction. Affected backup, send-repository and private-reservation checks and the Android build completed. Remote restore, physical hardware, funded channel and merchant order/proof acceptance remain outstanding.

Retained hardware completion (2592aa2)

  • Reconciled hardware completions are retained by exact wallet and transaction ID until the matching pending hardware result consumes them. Later completions cannot overwrite an earlier one while the sheet is closed.
  • A mismatched candidate cannot consume an entry; lookup verifies the original payer identity. Completion resumes local follow-up without another signing or broadcast.
  • The multiple-completion regression failed before correction. Affected app send-flow and hardware-send checks and the Android build completed. Physical hardware and final merchant proof/order acceptance remain unvalidated.

Prior private payment boundary (e38369f)

  • Local verification: affected private-state, proof, send-flow and backup unit checks; app build and zero changed-line detekt findings.
    • Reproduced consumed version 6 being lost when cancelling an unsent version 7.
    • The original receipt and encoded backup retain both versions; definite cancellation restores 6, while newer consumption remains protected.
    • Bound requests without a payment-list version retain their existing behavior.
  • Funded merchant checkout, fresh-wallet Shop VSS and physical hardware acceptance remain outstanding.

Unsent private preparation cleanup (5214de9)

  • ran PaykitPaymentProofRepoTest, AppViewModelSendFlowTest, private payment and backup regressions, dev app build and changed-line formatting checks.
  • reproduced both prior failures: consumption preceded a failing hardware proof write, and interrupted unsigned cleanup deleted the proof without releasing version 7 back to 6.
  • cleanup retains the original proof and operation when private storage or proof deletion fails; a later retry completes the same cleanup.
  • fresh-wallet Shop VSS, current expiry device journey, physical hardware and funded merchant checkout acceptance remain unrun.

Retained private payment version (08d35b7)

  • updated PaykitPaymentProofRepoTest.kt — restored signed receipts consume their saved private version before retry; failed private storage blocks dispatch, existing consumption remains valid, and another order cannot reuse the same contact while its signed payment is retained.
  • Local verification: affected private/proof, send, hardware, backup and deadline checks; app build and static analysis.
  • Fresh-wallet Shop VSS, physical hardware and funded merchant checkout remain unrun for this head.

@ovitrif ovitrif self-assigned this Sep 30, 2026

@github-advanced-security github-advanced-security AI 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.

detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@ovitrif
ovitrif marked this pull request as ready for review September 30, 2026 01:17
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from f616693 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@greptile-apps

This comment has been minimized.

greptile-apps[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed 6966111 with the scoped review fixes: original-send follow-up, exact-observation acknowledgement, running-process Accepted persistence repair, verified Shop proof delivery after guard replacement, and original transfer accounting. The visibility component test now states its actual coverage.

The source batch passed 71 focused tests. Process loss before Accepted is durable still leaves the attempt guarded. This PR remains draft while corrected node artifacts, consumer validation and current native journeys are pending.

@ovitrif

ovitrif commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed ae0be37 with the hardware Shop proof correction. A Core txid is retained only as the original lookup candidate; proof and Success require fresh observation of that exact outgoing transaction in the original wallet. Identity/request/wallet context is captured, preparation failures block dispatch, and pending proof work cannot start another payment.

The frozen batch passed 58 focused tests, including the guard, wrong-identity, inbound/mismatched lookup and original-context regressions. The hardware journey spec is parsed but unrun; physical hardware and native fault fixtures remain explicit QA gaps.

This remains draft while both apps validate the published rc68 dependency and current native fixed/Max journeys. Prior rc67 media is labelled historical.

@ovitrif

ovitrif commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed 9de645a with the final hardware Shop guards and the production rc68 dependency pin. A candidate-save failure or a Core exception with no returned txid keeps the started request protected after dismissal/reopening. Shop Sent activity now requires fresh exact outgoing observation and durable local follow-up in the original wallet.

The affected run passed 95 tests and app/test APK builds. The actual installed APK embeds the published rc68 native bytes; funded fixed 1,000-sat and Max 98,749-sat native sends passed, and both exact UI transaction IDs were independently observed on the backend. Current Preview replaces the historical accepted-send media.

Controlled native refusal/response-loss and hardware Shop execution remain unrun; their QA entries remain explicit. No recovery scope was added.

@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.

Two HIGH, one MEDIUM and two LOW inline. I reviewed this as a funds change. The refusal lockout and the legacy-proof gap are shared with synonymdev/bitkit-ios#844.

Checked and clean:

  • No failure is reported for a tx that broadcast: NodeException/NodeNotSetup release the guard only before dispatch in rc.68, and everything else keeps the guard.
  • Success is shown only on Accepted. The replays at :4018/:4172 are for the same request with its original txid. HW Shop needs a fresh exact observation.
  • admit blocks the same requestId/orderId, and Lightning proof association is refused while an on-chain attempt exists.
  • Pending is persisted before dispatch.
  • Old backups decode with the new field defaulting to false, and newer backups decode on older builds via ignoreUnknownKeys.
  • The Keychain change is storage-only, with no seed material.
  • The rc.66 → rc.68 bump carries the broadcast-result API.

Pre-existing: RBF/CPFP remain fire-and-forget.

Also LOW: the new toasts at TransferViewModel.kt:394/436 are hardcoded English.

Comment thread app/src/main/java/to/bitkit/repositories/LightningRepo.kt
Comment thread app/src/main/java/to/bitkit/repositories/OnchainSendAttemptStore.kt
Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/send/SendPendingScreen.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PaykitPaymentProofRepo.kt
@coreyphillips

Copy link
Copy Markdown
Contributor

Two independent reviews.

needs changing before merge

  • Preflight send failures now show the unresolved Pending screen (app/src/main/java/to/bitkit/repositories/LightningRepo.kt:1487). If sendOnChain fails before admission (sync, fee rate, coin selection or node not running), the app now shows the on-chain Pending screen instead of the error, even though no transaction or guard exists. AppViewModel.handleOnchainPaymentFailure treats every error except OnchainSendNotDispatchedError as unresolved and calls showUnresolvedOnchainSend. LightningRepo.sendOnChain only wraps failures from admit and the node call. Earlier failures come back raw: - ensureSyncedBeforeSend() returns SyncUnhealthyError (LightningRepoTest sendOnChain should fail when sync is unhealthy asserts the raw type). - getFeeRateForSpeed(...).getOrThrow() and determineUtxosToSpend throw through executeOperation unwrapped. - executeWhenNodeRunning returns NodeNotRunningError or NodeRunTimeoutError. The new VM test generic outer error after ordinary send remains unresolved pins that a plain IllegalStateException produces NavigateToPending("", amount, false, isOnchain = true). Net effect: with Electrum unreachable, an ordinary send that master rejected with an error toast now shows a Pending screen. The screen uses the new wallet__send_pending__onchain_description copy ("Bitkit will block another send while this outcome is unresolved") and has no txid, and nothing ever resolves it. That is the reverse of the issue: a payment that was never created looks like it might have been sent. The VM test onchain payment failure before send attempt cancels prepared proof wraps preflight errors in OnchainSendNotDispatchedError, which the repo never does, so the intended contract and the code disagree. Confirmed by tracing: LightningRepo.kt:1487-1505 returns or throws before admit, and AppViewModel.kt:4162 routes anything that is not OnchainSendNotDispatchedError to Pending. Fix: wrap every failure before admit in OnchainSendNotDispatchedError, or classify by "guard was persisted" rather than by error type.
  • Preflight failures are shown as unresolved payments (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:4162). sendOnChain returns SyncUnhealthyError at lines 1488 to 1490 before OnchainSendAttemptStore.admit at line 1508. Node-state and fee lookup errors can escape at the same stage. handleOnchainPaymentFailure then classifies every error except OnchainSendNotDispatchedError as unresolved and navigates to Pending. I confirmed this against the existing unhealthy-sync repository test and the exact call path: neither the guard nor native send runs. For Shop payments, this also leaves the consumed private payment state unreleased, so an unpaid request can be stranded instead of retried.

worth doing, does not block

  • Accepted transfer guard can only be cleared by reopening the same order (app/src/main/java/to/bitkit/viewmodels/TransferViewModel.kt:375). If transfer bookkeeping fails after LDK accepts a funding transaction, the send guard can only be cleared from the same Blocktank order, so leaving the flow blocks all later on-chain sends. The guard is cleared only by completeAcceptedTransferFollowup, and only TransferViewModel.paySpendingConfirmOrder calls it when previous.orderId == order.id. The onEvent observation path skips transfers (!attempt.isTransfer). The failure case: fundPaidOrder(requireTransferPersisted = true) throws, for example because findLspOrderIdByFundingTxId or createTransfer fails. The user then sees an error with paid = false. The order lives in _spendingUiState. If the user leaves the flow, the VM is cleared or the app restarts, the next attempt uses a new order. The attempt stays Accepted with localFollowupComplete = false, so blocksNextSend rejects every later on-chain send, ordinary ones included. Those show the Pending screen for the old transfer txid. Accepted Shop sends have a similar dependency. completeOnchainPayment returns early when currentIdentity() is null, and reconcile needs a live Pubky session, so signing out of Pubky before the proof persists leaves the guard blocking all sends. The PR says local follow-up resumes "without creating another payment". That holds, but the only resume route for transfers is the original order object. Consider letting reconciliation, such as onEvent or startup, finish accepted transfer attempts from the persisted transferContext/orderId.
  • Shop completion discards activity recovery state (app/src/main/java/to/bitkit/repositories/LightningRepo.kt:1623). completeAcceptedShopFollowup marks the guard complete after proof completion without rerunning or validating finishOnchainSendLocally. Since sendOnChain swallows that function's metadata or activity failure at line 1557, and event recovery excludes Shop attempts at lines 568 to 570, a transient local write failure can be forgotten and the guard overwritten by the next send. The payment remains protected, but missing activity metadata or tags are no longer recoverable from the saved attempt.

nits

  • Accepted ordinary sends write local metadata and activity twice (app/src/main/java/to/bitkit/repositories/LightningRepo.kt:1602). sendOnChain runs finishOnchainSendLocally(recorded) after an accepted outcome, and AppViewModel then calls completeAcceptedOrdinaryFollowup, which runs finishOnchainSendLocally a second time before marking the follow-up complete. createSentOnchainActivityFromSendResult skips existing activity, so this is harmless. Still, every successful send makes a second addPreActivityMetadata write and a second Core read-back. Marking the follow-up complete in sendOnChain when the first finish succeeds would avoid it.

@ovitrif

ovitrif commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Published cd07c3c. I rejected unsigned active backup attempts that could leave a restored wallet permanently blocked, and kept recovery Pending when acceptance exists only in memory after a failed write. The retry preserves the original transaction and does not authorize another payment. Both defects were reproduced before correction; local build, unit tests and lint completed.

The dispatch-boundary, contact-attribution and fresh-wallet Shop restore findings remain open, alongside device and merchant acceptance. This PR stays draft.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: cd07c3c084

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ovitrif

ovitrif commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Published 964ef89. I fixed restored-payment contact attribution so recovery preserves the user's durable manual detachment instead of reattaching the original contact or blocking completion. The regression reproduced the unwanted reattachment before correction; local build, unit tests and lint completed.

The dispatch-boundary finding and fresh-wallet Shop restoration remain open, together with device and funded merchant acceptance. This PR stays draft.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 964ef89f16

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ovitrif
ovitrif marked this pull request as ready for review October 7, 2026 20:57
@ovitrif
ovitrif requested a review from a team October 7, 2026 20:58
Comment thread app/src/main/java/to/bitkit/repositories/LightningRepo.kt

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 964ef89f16

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendViewModel.kt Outdated
@@ -868,17 +891,12 @@ class AppViewModel @Inject constructor(

private fun handlePaykitOnchainPaymentResolution(resolution: PaykitOnchainPaymentProofResolution) {
if (!PubkyPublicKeyFormat.matches(pubkyRepo.publicKey.value, resolution.identity)) return

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reprocess resolutions when the matching identity becomes active

If reconciliation publishes a resolution while pubkyRepo.publicKey is temporarily null or points to another profile, this return leaves the resolution in the repository flow, but nothing retries it when the matching identity becomes active. onchainPaymentResolutions is a StateFlow, duplicate publication keeps the same list without re-emitting, and identity activation can additionally clear the list, so a completed hardware payment is never added to resolvedHardwarePayments and a software completion never updates the current send/contact state. Observe the identity together with the resolution list or explicitly replay retained resolutions after activation.

Useful? React with 👍 / 👎.

@ovitrif
ovitrif requested review from piotr-iohk and removed request for a team October 7, 2026 21:15
@ovitrif

ovitrif commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 7ec4d56. I kept acceptance recovered after a broadcast error Pending until the exact winner is durably saved. This addresses the retained-only success finding.

Local verification: Android build, unit tests and lint completed; the new regression failed before the fix and passes afterward. Funded Shop acceptance remains unrun.

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7ec4d56753

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/src/main/java/to/bitkit/models/ActiveOnchainAttemptBackup.kt
proofs[index] = verified
return persistAndSubmit(listOf(verified), proofs)
}
submitReady(proof)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Rebuild hardware activity before submitting a restored proof

When a wallet backup restores a hardware proof with onchainAcceptanceVerified == true, this branch submits and removes the proof without calling observeExactTransaction(), which is also the only path that recreates and verifies the local sent activity. Because wallet and activity backup categories upload independently, the restored wallet envelope can be newer than the activity envelope; in that case the app publishes payment success and discards its retryable proof while the corresponding activity is still absent. Verify or recreate the hardware activity before submitting an already-verified restored proof.

Useful? React with 👍 / 👎.

Comment on lines +65 to +66
lightningRepo.lightningState.map { it.nodeLifecycleState.isRunning() }.distinctUntilChanged()
.collect { running -> if (running) resumeAcceptedFunding() }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Retry accepted funding after connectivity recovers

If an accepted transfer still needs its paid-order/local follow-up and fetchOrders() fails because Blocktank is temporarily unreachable, the only automatic retries installed here are another node-running transition or an on-chain event. Once the node is already running and the transaction event has already been consumed—for example, restoring an already-confirmed transfer while the API is offline—network recovery triggers neither, so the accepted attempt remains incomplete and blocks subsequent software sends until a process restart or manual order flow. Observe connectivity recovery or otherwise schedule a retry after this failure.

Useful? React with 👍 / 👎.

@ovitrif

ovitrif commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 2836a9b. I fixed hardware Shop recovery after a broadcast timeout: exact transaction observation clears the retained send guard and opens Success with the original payment details, without requiring a returned Core result or another send.

Local verification: Android build, unit tests and lint completed. The regression failed before the fix; it also checks original request/payer/amount, unrelated observations and admission of the next send. Funded hardware/merchant validation remains unrun.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 2836a9b0b8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ovitrif

ovitrif commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 3ca8a30. I made restore reject an accepted replacement transaction whose backup omits that candidate’s fee rate, preventing installation of a payment guard that cannot complete. The original candidate can still use its original fee rate; a valid replacement preserves its own rate.

Local verification: Android build, unit tests and lint completed. The new regression failed before the fix; accepted restore fixtures now include the winning replacement rate. Funded Shop/hardware acceptance remains unrun.

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ca8a3093f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

walletIndex = walletIndex,
transferContext = transferContext,
payerIdentity = payerIdentity,
preparationPending = requestId != null,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Recover unsigned guards after process death

If the process dies after this persistence and before retainPreparedReceipt() finishes, an ordinary send or transfer restarts with a durable blocking attempt but no inputs or candidate txid. retryOriginal() rejects that state, while the only restart cleanup (releaseInterruptedShopPreparation) requires preparationPending, a requestId, and a Shop proof, so these non-Shop attempts permanently block every later software on-chain send. The backup deferral fixes remote snapshots but does not repair this local keychain state; all definitely unsigned, non-restored attempts need a restart-safe cleanup path.

Useful? React with 👍 / 👎.

Comment thread app/src/main/java/to/bitkit/models/ActiveOnchainAttemptBackup.kt Outdated
).also {
persist(it)
inFlightPreparations -= attemptId
if (!isRecovery) firstSubmissions += attemptId

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Persist whether a prepared software candidate was dispatched

If the process dies after the signed receipt is persisted here but before broadcastPreparedCandidate() reaches prepared.broadcast(), the only proof that this candidate was never submitted is lost with the process-local firstSubmissions set. After restart the candidate cannot be cleared as definitely unsent; for a Shop request or transfer whose payment/order deadline has since expired, every recovery attempt fails authorization before dispatch and the retained guard permanently blocks all later software sends. Persist a recoverable pre-dispatch phase or the exact signed transaction instead of relying on this in-memory marker.

Useful? React with 👍 / 👎.

@ovitrif

ovitrif commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed 96a9230 to reject an active on-chain backup without its original local follow-up context. Restore now fails before installing a guard that cannot finish the payment, addressing the missing-context finding.

Local verification: model restore and wallet restore regressions reproduced the gap before the fix; build, unit tests and lint passed after the fix. Funded Shop checkout validation remains unrun.

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 96a9230e8e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/src/main/java/to/bitkit/repositories/BackupRepo.kt Outdated
Comment on lines +196 to +200
backupFollowup = ActiveOnchainAttemptBackup.Followup(
feeSats = "0",
tags = tags,
createdAtMillis = nowMillis(clock).toString(),
channelId = channelId,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Capture the contact in the active follow-up context

For an ordinary on-chain payment initiated from a contact, admit() creates every backupFollowup without the contact and no later production path populates it. If the wallet snapshot is uploaded before the separately launched contact-assignment write or its independently uploaded activity backup, restoring the accepted attempt recreates the Sent activity without attribution because finishOnchainSendLocally() can restore only followup.contact. Pass the selected contact into this durable follow-up context when admitting the send.

Useful? React with 👍 / 👎.

Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Outdated
attempt: OnchainSendAttempt?,
): Boolean {
if (proof.onchainWalletId != WalletScope.default) return reconcileHardwareOnchainProof(proof)
if (!attempt.matchesPositiveShopProof(proof)) return false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve reconciliation for pre-upgrade on-chain proofs

Existing installations can contain a software on-chain proof written by the previous version with paymentStarted == true, a null paymentIdentifier, and only onchainMatchingTransactionIdsBeforeAttempt for identifying the eventual transaction. Such a record has no OnchainSendAttempt, so this check now rejects it unconditionally, whereas the removed reconciliation path searched the wallet by its original address and amount while excluding the saved baseline. After upgrading, an already-broadcast payment can therefore never produce or deliver its proof and the started record permanently blocks the request; retain a verified legacy lookup or migrate these records before requiring an active attempt.

Useful? React with 👍 / 👎.

@ovitrif

ovitrif commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed 9f73e83 to resume payment follow-up from the merged local guard after wallet restore. An older unresolved snapshot no longer hides acceptance already retained locally. Recovery remains tied to the original attempt and wallet.

Local verification: the stale-snapshot regression failed before correction; build, unit tests and lint passed after the fix. Funded Shop checkout validation remains unrun.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 9f73e8389e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ovitrif

ovitrif commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed f616693 to clear the blocked new confirmation’s captured contact context before recovery of an older payment. Pending retains the older operation’s original request, and later completion cannot assign the new recipient’s contact to its transaction.

Local verification: the contact-attribution regression failed before correction; build, unit tests and lint passed after the fix. Funded Shop checkout validation remains unrun.

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6166930f9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 4644 to +4647
paykitPaymentProofRepo.onchainPaymentResolutions.value
.firstOrNull { it.requestId == incomingPaymentRequest.id }
.firstOrNull { it.requestId == unresolvedRequestId }
?.let(::handlePaykitOnchainPaymentResolution)
if (uncertainOnchainPaymentRequestId == null) return
setSendEffect(
SendEffect.NavigateToPending(
paymentHash = incomingPaymentRequest.paymentRequestId,
amount = amount.toLong(),
observeResolution = false,
)
showUnresolvedOnchainSend(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Stop after consuming an existing payment resolution

When the exact Shop resolution is already present by the time an uncertain send reaches this branch, handlePaykitOnchainPaymentResolution clears the uncertainty and queues PaymentSuccess, but execution then unconditionally calls showUnresolvedOnchainSend, restores the uncertain request ID, and queues Pending as well. This can leave an already accepted payment displayed as unresolved; return when the matching resolution was consumed before presenting Pending.

Useful? React with 👍 / 👎.

Comment on lines +4642 to +4643
val unresolvedRequestId = if (previous != null) previous.requestId else incomingPaymentRequest?.id
uncertainOnchainPaymentRequestId = unresolvedRequestId

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bind recovery UI to the older Shop request

When Shop request B is blocked by an unresolved Shop request A, this routes Pending using A's request ID while SendUiState.incomingPaymentRequestId remains B. If A resolves afterward, handlePaykitOnchainPaymentResolution consumes the resolution but refuses success because the state still names B, and SendPendingViewModel deliberately does not observe request-bound attempts, so the screen remains Pending after A's proof completes. Update the displayed request context to A or otherwise deliver A's exact resolution to this Pending route.

Useful? React with 👍 / 👎.

This branch has not been deployed

No deployments
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.

fix: prevent false success for rejected on-chain sends

4 participants