Skip to content

fix: keep pending boosts across navigation - #1915

Merged
piotr-iohk merged 8 commits into
masterfrom
codex/320-boost-pending
Nov 13, 2026
Merged

piotr-iohk merged 8 commits into
masterfrom
codex/320-boost-pending

Conversation

@piotr-iohk

@piotr-iohk piotr-iohk commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1821
Closes #1822

This PR keeps an approved replacement attempt available when Activity Detail closes and shows pending feedback if the replacement takes longer than two seconds to publish.

Description

  • The wallet owns the approved fee quote and replacement TXID across Activity Detail and Boost sheet recreation. A publication retry uses that same replacement. The reopened sheet displays the confirmation estimate for the approved fee rate and keeps the approved custom layout when used.
  • Slow publication shows pending feedback in the sheet while the attempt continues. Success follows publication of the exact replacement and its metadata.
  • Confirmation and wallet generation checks protect the active attempt. UI effects go to the sheet that still owns them.

Out of Scope

  • Persisting pending attempts across process death.

Design

The change uses the existing Boost sheet and localized pending copy. The screens map identifies the confirmed transaction detail frame. The pending sheet frame is unverified.

Preview

Pending device capture.

QA Notes

Setup

Use an isolated local regtest wallet and miner. Start Bitcoin Core, Electrum and LND with docker compose up -d from the docker directory at bitkit-e2e-tests 7f61681. Build the Android dev app with E2E=true E2E_BACKEND=local ./gradlew assembleDevDebug. Create a fresh wallet, fund it with 100,000 confirmed sats and send 10,000 sats to a separate regtest address. Use the same miner to confirm the replacement.

Journeys

  • J1: existing rbf-own-send.xml — open the replacement immediately after Boost success, then check its fee, TXID, replacement history and confirmation.

Manual Tests

  • 1. Suspend Electrum on a dedicated regtest fixture after preparing the original send → swipe Boost and observe pending feedback → dismiss the sheet, leave Activity Detail, reopen it and check that the pending attempt retains its approved fee → resume Electrum and retry if publication is still pending. Verify exactly one native bump and the same replacement TXID, with success only after metadata is available. Controlled Electrum fault injection is absent from the journey capabilities.

Automated Checks

  • updated BoostTransactionViewModelTest.kt — covers a recreated view model, preserved custom quote layout and confirmation estimate, single native bump, slow feedback, terminal and retry paths, wallet reset and stale UI effects.
  • updated ActivityServiceRbfTest.kt — covers original confirmation before publication commits.

Evidence

At 2c9dadc74781b6e5b024f40ffe29f5f6d6328832, just compile, just test and just lint passed. The recreated-view-model test detected that the prior head reopened an approved custom quote in the recommended layout and passed with the fix. The controlled Android 13 emulator run passed on this head. Reopening kept the locked 4 sats/vbyte rate, total fee, and ±10-minute estimate. The run observed one native bump, the exact replacement TXID, success after publication, and confirmation after mining.

At 18692107ab4dd38d10f603a95e48fce4d6098d7d, just compile, just test and just lint passed. The recreated-view-model test detected the missing estimate on the previous head and passed with the fix. Case 1 and the immediate-open assertions passed on the Android 13 emulator with the dedicated Electrum proxy at this head. Both sheets showed ±10 minutes, the approved fee matched, and the logs showed one native bump and the exact replacement. Success followed publication, and the replacement confirmed after mining.

At dae03d9fe4a1e8284fafe43ea7a87facdeacaa8e, just compile, just test and just lint passed. The unit suite had 5,930 passes and two existing skips. Independent review resolved all findings.

On master 3798a007f654b1faf4042d63cf743365e7ded18e, local Android 13 regtest logs showed 3.86 s, 1.64 s and 1.95 s from native return to the matching publication marker. The pinned @boost_2 run passed in the first sample. The two immediate-open probes published the replacement and reached Home. Appium did not observe the toast in those runs.

J1 passed twice at dae03d9 on the Android 13 emulator with local regtest. Opening the row immediately after the success toast found the replacement, a 10,000-sat payment and a fee increase from 141 to 422 sats. The exact replacement TXID and history were visible, and the replacement confirmed after mining. Native return to publication was 1.18 s and 0.91 s, with success observed after publication in both runs. These are local samples.

Case 1 passed on the same candidate with a dedicated Electrum proxy. The pending message appeared with the connection paused. The app logged pending at 2.002 s. Leaving and reopening Activity Detail preserved the fee quote. A second swipe produced no additional native bump, and the exact replacement published at 7.201 s after the connection resumed. Appium observed success afterward, and mining confirmed the replacement.

Gaps

The local runs cover J1 and controlled pending navigation. The device case exercised an in-flight retry. Deterministic unit tests also exercise retry after a publication failure and wallet reset.

Models used

  • Planning/scoping: GPT-6.1 (reasoning: Not exposed)
  • Implementation: GPT-6.1 (reasoning: Not exposed)
  • Review: GPT-6.1 (reasoning: Not exposed)
  • Review rounds: 5

Reasoning effort was unavailable in the session and reviewer metadata.

@piotr-iohk piotr-iohk added this to the 3.2.0 milestone Nov 13, 2026
@ovitrif ovitrif removed this from the 3.2.0 milestone Nov 13, 2026
@github-actions

github-actions Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 2c9dadc (run).

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

@piotr-iohk
piotr-iohk marked this pull request as ready for review November 13, 2026 14:16
@greptile-apps

greptile-apps Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issues were found.

Summary

Keeps approved fee boosts available after leaving and reopening transaction details.

  • Approved fee boosts keep the same replacement when users reopen the sheet.
  • Slow fee boosts show pending feedback while the attempt keeps running.
  • RBF publication stops when the original transaction confirms first.

The previous estimate finding is fixed: restoreQuote calculates estimateTime from the saved fee rate.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Approve boost] --> B[Wallet retains quote and attempt]
  B --> C[Broadcast replacement once]
  C --> D[Save metadata and publish replacement]
  D -->|Slow| E[Show pending feedback]
  D -->|Failed| F[Keep exact replacement for retry]
  F --> D
  D -->|Published| G[Show success in the owning sheet]
  H[Reopen sheet] --> B
  B --> I[Restore approved fee, layout, and estimate]
Loading

Reviews (3) · Last reviewed commit: "fix: retain the approved custom Boost la..." · Reviewed by Greptile

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No findings. This PR moves the RBF attempt (approved quote, replacement txid, publication state) out of the sheet's view model into a singleton RbfBoostRepo. A reopened Boost sheet then reattaches to the same broadcast instead of starting a second one. Reviewed 18692107, full tier, static.

What I checked, and 5 candidates I ruled out

Read in full: RbfBoostRepo.kt, BoostTransactionViewModel.kt, the BoostTransactionSheet.kt and ActivityDetailScreen.kt hunks, the CoreService.kt RBF hunks, ActivityRepo.publishRbfBoost/recordRbfBoost, WalletConnectionRecovery (generation, withOperation, commitCurrentState), and bitkit-core's record_rbf_boost.
Delta since dae03d9: 18692107 only extracts estimateTimeForFeeRate and calls it from restoreQuote. That answers the Greptile point, and the recreation test now asserts the estimate.
Build: a run of BoostTransactionViewModelTest, ActivityServiceRbfTest and ActivityDetailViewModelTest through the sandboxed PR runner had not finished when I posted, so nothing here claims a local pass. CI is the gate: build-and-test and the e2e-tests-local shards (onchain_boost_receive_widgets in particular) were still pending when I last read them.

Ruled out

  • Double native bump on retry or reopen: confirm() reuses the existing Attempt under lock, and a set replacementTxId skips broadcast(). bumpFeeByRbf is covered by times(1) in the recreation test.
  • Boost after the original confirmed: broadcast() re-reads the activity and checks !current.confirmed. recordRbfBoost and publishRbfBoost both throw RbfOriginalConfirmed, and finishFailure then drops the attempt. On the core side, record_rbf_boost only flips is_boosted where confirmed = 0.
  • Old wallet's attempt leaking into a new one: admit() clears attempts when the generation changes, requireCurrent runs between every step, and commitCurrentState re-checks the generation under stateLock before Published is set.
  • Stale effect reaching a rebound sheet: SheetEffect carries the origin instance and generation, and canDeliver compares both plus wallet and txid.
  • Pending state surviving process death: listed as out of scope in the body. The attempt is in memory only, so after a restart the replacement is picked up by the normal handleOnchainTransactionReplaced path.

Not closed: runAttempt holds connectionRecovery.withOperation, and so operationMutex, across broadcast, record and publish, including the lightningService.sync() inside publishRbfBoost. The connection editor's save and recovery retry in WalletViewModel take the same mutex. So in the slow-Electrum case this PR targets, changing the Electrum server could wait behind the boost until sync returns or times out. I have not measured that, so it is a question, not a finding.

Merge confidence: 4/5, no findings, but the change was not built here, CI was still running, and the mutex hold above is unmeasured.

Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed (the review session does not report it)

@piotr-iohk
piotr-iohk requested review from a team, ben-kaufman and ovitrif and removed request for a team November 13, 2026 15:04
@piotr-iohk
piotr-iohk enabled auto-merge November 13, 2026 15:34

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

QA review

Scope: Reviewed the full PR diff against its merge base 3798a007, at 18692107.

No new actionable code findings.

A recreated Activity Detail reattaches to the same approved quote and replacement. A publication retry does not broadcast again. Success is emitted only after replacement metadata is published, and only to the sheet that still owns the effect. Reopening the sheet derives the confirmation estimate from the approved fee rate. The resolved estimate-time thread matches that code.

Compared iOS BoostSheet.swift at b35611a11a25a265f38ee98fb61188da7623db39. iOS still awaits the boost inside the sheet and does not keep a process-wide pending attempt. The shared success path still waits for publication before the success toast.

Validation: source and unit tests were inspected, not executed here. On this head, build-and-test, lint, detekt, and onchain_boost_receive_widgets succeeded. Process-death persistence remains out of scope.

Device testing: not performed in this review. The author reports J1 and manual case 1 on this head.

Ready for device testing.

Reviewer model: grok-4.7
Reasoning effort: high

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ✅ Approve — one LOW display issue; the confirmation case and focused regression tests passed.

Findings:

  • LOW — Restore the custom quote's display mode (app/src/main/java/to/bitkit/ui/sheets/BoostTransactionViewModel.kt:114). After approving a custom rate, leaving Activity Detail and reopening a pending attempt creates a view model with isDefaultMode = true. restoreQuote restores the fee but leaves that mode unchanged. The recommended-fee layout hides the approved sat/vbyte rate, and its locked Edit action cannot expose it; early publication/retry copy also calls the custom quote a recommendation. Preserve the selected mode or show the saved custom rate in the locked quote. The actual approved rate and amount remain unchanged. Verified from the restoration and both sheet layouts; this custom-mode variant was not driven on device.

QA for 1869210:

Original confirms while boost publication is pending ✅ The confirmed original remains accessible with its own TXID and fee.

Android 15, isolated local regtest, app built from this head with E2E=true E2E_BACKEND=local.

Sent 10,000 sats with a 141-sat fee, opened Boost and approved the 422-sat quote. With Electrum paused, the native bump returned a replacement TXID and pending feedback began after 2.0007 seconds. Mined the original before restoring Electrum. Publication timed out without success; reopening the send showed Confirmed, the original TXID, the 10,000-sat payment and 141-sat fee, with Boost disabled.

The recording shows the pending flow and the final original transaction. App logs independently show one native bump and no replacement-publication success marker; Bitcoin Core confirms the original in the mined block.

confirmation-race2x.2x.mp4

Original confirmation while the replacement remains unpublished, at 2× speed.

Check: boost ownership and publication regressions ✅ The pinned app built and the focused unit tests passed.

Ran assembleDevDebug and testDevDebugUnitTest for BoostTransactionViewModelTest, ActivityServiceRbfTest and ActivityDetailViewModelTest: 66 tests, no failures, errors or skips. These cover recreated view models, locked quotes and estimates, single-bump retry, slow feedback, reset/cancellation, stale effects and confirmed-original publication rejection.

J1 and manual case 1 were not repeated: the author's evidence at this exact head reports immediate replacement opening and controlled pending navigation passing, including the preserved estimate/quote, one native bump, exact replacement, metadata ordering and mined confirmation. I relied on those results for those cases. The QA source review also checked the full diff and successful CI; it did not claim a device run.

Full review of 3798a007..1869210, including repository ownership, wallet generation, publication and UI-effect callers. Process-death persistence remains out of scope. Reset and stale-effect paths were exercised by unit tests, not device cases. The custom-mode display finding is source-verified.

Twin: no matching iOS pending-attempt PR found. The iOS Boost sheet at b35611a still owns its boost locally in the sheet, so it does not match the new Android navigation persistence.


Reviewed by gpt-6.1-sol-high

@piotr-iohk
piotr-iohk disabled auto-merge November 13, 2026 16:55

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utAck

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ✅ Approve — the prior custom-layout finding is fixed; no new findings.

Findings: None. The saved quote now includes the approved mode, and reopening restores that mode alongside the locked fee rate, total fee and estimate. This fixes the custom-layout finding.

QA for 2c9dadc:

Check: custom quote restoration and boost regressions ✅ The pinned app built and all focused regression tests passed.

Built with E2E=true E2E_BACKEND=local using assembleDevDebug, then ran testDevDebugUnitTest for BoostTransactionViewModelTest, ActivityServiceRbfTest and ActivityDetailViewModelTest: 66 tests passed, with no failures, errors or skips.

The changed recreation test approves a custom quote, leaves and recreates the detail view model, verifies the restored custom mode, locked rate, total and estimate, and verifies one native bump across publication retries.

No device cases were repeated in this delta review. I relied on these passing reports:

  • Manual case 1: the Android 13 report at this exact head confirms that reopening retained the locked 4 sats/vbyte custom rate, fee and ±10-minute estimate, with one native bump, the exact replacement, publication before success and confirmation after mining.
  • J1: the author's QA evidence reports immediate replacement opening, fee, TXID, history and confirmation passing at 1869210. This delta leaves those publication and routing paths unchanged.
  • Original confirms during pending publication: the earlier recorded review passed at 1869210; this delta leaves its confirmation guards and publication code unchanged.

Coverage: contiguous delta 1869210..2c9dadc and the prior open finding, including quote capture, restoration and both sheet layouts. The custom rendering is covered by the author's device report; this pass exercised the changed view-model regression. Process-death persistence remains out of scope.

Twin: no matching iOS pending-attempt PR found. The iOS Boost sheet at ae2b65c keeps its fee/edit state in the sheet and has no shared pending-attempt restoration, so navigation persistence does not match.


Reviewed by gpt-6.1-sol-high

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utAck

@piotr-iohk
piotr-iohk merged commit f2ecb18 into master Nov 13, 2026
25 checks passed
@piotr-iohk
piotr-iohk deleted the codex/320-boost-pending branch November 13, 2026 17:31
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.

Reduce RBF publication latency while preserving success ordering Preserve pending RBF attempts across activity-detail navigation

3 participants