Skip to content

Conversation

@praveenperera
Copy link
Contributor

@praveenperera praveenperera commented Jan 22, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved send flow state management by preserving state when returning to the wallet screen and ensuring proper cleanup at the appropriate lifecycle stage.

✏️ Tip: You can customize this high-level summary in your review settings.

@praveenperera praveenperera force-pushed the android-send-flow-clean-up-fix branch from 640a704 to 0ee2234 Compare January 22, 2026 15:54
@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

Refactored SendFlowManager cleanup timing by removing the clearing call from SelectedWalletScreen's LaunchedEffect and adding it to SendFlowContainer's DisposableEffect. This relocates cleanup from wallet screen return to container disposal, altering when the send flow state is reset.

Changes

Cohort / File(s) Summary
SendFlowManager Lifecycle Relocation
android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/SelectedWalletScreen.kt, android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.kt
Removed app?.clearSendFlowManager() from SelectedWalletScreen LaunchedEffect; added app.clearSendFlowManager() to SendFlowContainer DisposableEffect. Changes when SendFlowManager is cleared during lifecycle transitions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

android

Poem

🐰 The flow manager's cleanup tale,

Moves from screen to container's farewell,

When disposal arrives, the state goes away,

Lifecycle timing saves the day! 🌾

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Android send flow clean up fix' accurately describes the main change: refactoring SendFlowManager cleanup by moving it from SelectedWalletScreen to SendFlowContainer's disposal, which is a targeted cleanup optimization.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link

greptile-apps bot commented Jan 22, 2026

Greptile Summary

Fixed critical crash caused by premature cleanup of RustSendFlowManager and added Android product flavors for separate dev/store builds.

Key Changes

Critical Bug Fix (SendFlow lifecycle):

  • Moved app.clearSendFlowManager() from SelectedWalletScreen.kt:136 to SendFlowContainer.kt:107 in the DisposableEffect cleanup handler
  • Previously, the manager was destroyed when returning to the wallet screen, but the Rust object could still be accessed if user navigated back to send flow
  • Now cleanup only happens when SendFlowContainer is disposed, ensuring the manager lifetime matches its usage scope

Android Build Configuration:

  • Added product flavors: dev (with .dev suffix) and store (production)
  • Dev flavor allows installing development builds alongside production app
  • Updated build scripts to use assembleDevDebug/assembleDevRelease for local development
  • Store flavor uses bundleStoreRelease and assembleStoreRelease for Play Store distribution
  • Build script now generates both AAB and APK for store releases

Confidence Score: 5/5

  • This PR is safe to merge - fixes critical crash and improves build system
  • The lifecycle fix correctly addresses a use-after-free bug by moving cleanup to the proper disposal point. The product flavor changes are standard Android configuration that separates dev/store builds cleanly. All changes are well-scoped and follow Android best practices.
  • No files require special attention

Important Files Changed

Filename Overview
android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.kt Moved clearSendFlowManager() to DisposableEffect cleanup to fix crash when manager is destroyed
android/app/src/main/java/org/bitcoinppl/cove/flows/SelectedWalletFlow/SelectedWalletScreen.kt Removed premature clearSendFlowManager() call that caused use-after-free crash

Sequence Diagram

sequenceDiagram
    participant User
    participant SelectedWalletScreen
    participant SendFlowContainer
    participant SendFlowManager
    participant RustCore

    Note over User,RustCore: Before Fix (Crash Scenario)
    User->>SelectedWalletScreen: Navigate to wallet
    SelectedWalletScreen->>SendFlowManager: clearSendFlowManager()
    SendFlowManager->>RustCore: close() and destroy
    Note over SendFlowManager,RustCore: Object destroyed
    User->>SendFlowContainer: Navigate to send flow
    SendFlowContainer->>RustCore: Access destroyed object
    RustCore-->>SendFlowContainer: CRASH ❌

    Note over User,RustCore: After Fix (Proper Cleanup)
    User->>SelectedWalletScreen: Navigate to wallet
    Note over SelectedWalletScreen: No premature cleanup
    User->>SendFlowContainer: Navigate to send flow
    SendFlowContainer->>SendFlowManager: Initialize manager
    SendFlowManager->>RustCore: Create and use safely ✓
    User->>SendFlowContainer: Exit send flow
    SendFlowContainer->>SendFlowManager: DisposableEffect cleanup
    SendFlowContainer->>SendFlowManager: clearSendFlowManager()
    SendFlowManager->>RustCore: close() and destroy
    Note over SendFlowManager,RustCore: Object destroyed at proper time ✓
Loading

@praveenperera praveenperera merged commit 62dc6d2 into master Jan 22, 2026
9 checks passed
@praveenperera praveenperera deleted the android-send-flow-clean-up-fix branch January 22, 2026 19:56
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.

2 participants