fix: prevent false on-chain send success

[ovitrif]

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

Description

The published rc68 dependency matches the canonical artifact. The latest accepted-funding fix passed 169 focused unit tests and compilation. The earlier feedback batch at 0ef4613 passed 3,079 unit tests and two emulator component tests after resolving the master conflict. Native send and component Preview evidence below predates the latest fix.

• 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.
• Saves one bounded attempt guard before calling the node and blocks another send after refusal, unknown outcome, cancellation or unreadable state. Shop request protection also covers switching payment methods.
• 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.
• Pins published LDK 0.7.0-rc.68; independent remote and resolved AAR hashes match, with Maven local excluded from validation.

Out of Scope

• Transaction recovery: journals, raw transaction storage, automatic rebroadcast, replacement, RBF recovery and abandonment.

• Chain handling: reorg and event-delivery redesign.

• Unresolved sends: no timeout/reset escape. A refused or unknown result can block further on-chain sends indefinitely when the transaction never becomes positively observed; 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

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.

Current Pending preview: 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

Journeys

• 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

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

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

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.

[github-advanced-security]

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

[github-actions]

Regtest APK

Built from 6c75463 (run).

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

[ovitrif]

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]

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]

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]

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.

[jvsena42]

A refused broadcast locks on-chain sends for good.

Unknown and Rejected both block the next send, and the only exit is observing that exact txid on chain. There is no release on sync and no user action; only a wallet reset or reinstall clears the stored attempt. Ordinary sends, Shop payments and Blocktank funding all go through the same guard.

In ldk-node rc.68, classify_electrum_broadcast (src/chain/electrum.rs) returns Rejected only for -26 non-final. Every other daemon refusal becomes Unknown, including mempool min fee not met, min relay fee not met, txn-mempool-conflict and bad-txns-inputs-missingorspent. Those mean the backend did not take the transaction, and nothing rebroadcasts a user send, so no Received/Confirmed event ever arrives. A Slow or Custom fee during a mempool purge, or a send built on stale UTXO state, leaves the wallet unable to send on-chain while its funds stay in BDK.

Out of Scope accepts indefinite blocking for a lost result. This is a definite, common refusal that lands in the same state.

Fix: allow a release once a post-refusal sync shows the txid unknown to the backend and the attempt's inputs still unspent. If it could surface later, require the next send to spend at least one of those inputs so it conflicts rather than double-pays.

Same in synonymdev/bitkit-ios#844.

[ovitrif]

The lockout is real when a refused transaction never surfaces. It is an explicit limitation of this bounded change, including refusal as well as a lost response.

rc.68 Rejected reports a recognized refusal and explicitly allows earlier delivery: outcome contract. The pinned electrum-client 0.24.1 retries transport failures up to three times after the original call. A later protocol refusal returns without the earlier transport-error history; a request may already have been written before a receive failure. The final refusal therefore does not prove that earlier delivery or acceptance never occurred. See the retry wrapper. This is a possibility supported by the transport contract, not a claim that it happened in the tested journeys.

Only a proven pre-dispatch failure releases this guard. Relabelling fee/conflict/missing-input responses as Rejected would preserve diagnostics but would not prove retry safety. One backend reporting absence and unspent inputs cannot retract previously delivered bytes or an in-flight request.

Requiring a later send to conflict with the original adds a replacement/recovery policy, which the agreed scope excludes. I’m keeping the guard and clarifying the refusal limitation. Does this remain a blocker for the bounded acceptance-reporting change, or can the recovery policy be considered separately?

[coreyphillips]

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]

Pushed 0ef4613 with the current master conflict resolved. Accepted/observed funding now resumes from its saved original order/balance context at startup and node events, persists transfer/paid-order/local activity idempotently, and completes without another native send. Proven pre-admission errors use existing error handling; post-dispatch uncertainty remains guarded. Pending shows the candidate transaction ID/refusal and exact original-wallet local Details without treating that row as acceptance.

Local verification: compile, 3,079 unit tests and two emulator component tests passed against the published rc68 AAR; app/test APK builds passed. Detekt exited successfully under ignoreFailures with 570 existing findings, none on changed feedback lines. Two semantic regressions failed before their fixes. The installed native bytes match rc68.

Current Preview is explicitly synthetic component UI; prior funded fixed/Max media predates this batch. Native refusal/response-loss, storage-fault and hardware Shop journeys remain unrun. Refusal lockout and pre-upgrade software-proof migration remain open review discussions; no release, new schema or payment recovery was added.

[jvsena42]

Re-checked the fixes in merge 0ef4613. The transfer-resume HIGH, the pre-admission MEDIUM and the Pending LOW are resolved (replied on each). One new LOW inline, introduced with the auto-resume.

Checked and clean:

• The resume never broadcasts, and needs positive evidence plus an exact order/txid match.
• persistAcceptedFunding is serialized and idempotent.
• Observation promotion happens before the nodeEvents emit, so there is no race.

Still open, waiting on my decision: the refusal lockout thread and the legacy-proof migration thread.

[ovitrif]

Pushed 6c75463 to preserve paid-success navigation when local activity follow-up fails after funding and the paid order are durably saved. Both fresh Accepted and resumed Accepted/Observed paths keep the original transaction; existing resumption retries the local work. Funding-persistence failures remain guarded.

Three duplicate-funding regressions failed on the old source; compilation and 169 focused tests now pass against unchanged published rc68. Actual storage-fault device QA remains unrun. The description retains that limit, earlier validation provenance and the single-row image table.

[jvsena42]

Re-checked 6c75463. It applies exactly the suggested fix, with a test, so the second-order LOW is resolved. The refusal lockout thread is still waiting on my decision.