Skip to content

feat: add paper wallet functionality #1141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jun 3, 2025

Conversation

detherminal
Copy link
Contributor

@detherminal detherminal commented May 25, 2025

  • Adds functionality to import paper wallets by scanning QRs, in the right top part of the coin selection menu (as discussed with rehrar).
  • Adds respective functions to each coin for specific functionality.
  • Imports the paper wallets automatically.
  • Adds Monero gift wallet functionality by scanning and checking out the balance to a new secure wallet if the parameter txid is provided.

@detherminal detherminal changed the title feat: add paper wallet functionalşi feat: add paper wallet functionality May 25, 2025
Copy link
Member

@sneurlax sneurlax left a comment

Choose a reason for hiding this comment

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

See notes:

  • scanning should respect user's Tor preferences and use Tor if enabled and connected,
  • scanning should respect user's node preferences and use preferred nodes other than the default if selected,
  • the wallet is swept without confirmation from the user. no txs should be made nor wallets deleted without user confirmation,
  • the loading indicator should match the widgets and pattern used elsewhere in the app

@julian-CStack
Copy link
Collaborator

@sneurlax may have covered some of these points:

  • The wallet creation logic is in a coin metadata class, which doesn’t feel like the right place for it—it makes the structure harder to follow.
  • It’s using some custom loading spinner inside a bottom sheet, instead of the animated Stack Wallet loader we use everywhere else. Not sure why that change was made.
  • WidgetRef is being passed around outside of a widget, which goes against typical usage and could lead to problems. (It's possible this is done already in other places but I would like to avoid it going forward).
  • Monero scans for transactions without respecting Tor settings,.
  • The temporary Monero wallet isn’t being closed or deleted on successful import and transfer. It will get deleted but only on next app restart and will take up resources until then.
  • There’s also no real error or failure handling—no dialogs or user feedback if something goes wrong. (Another thing we have to work on in other areas of the codebase)

@detherminal detherminal marked this pull request as draft June 2, 2025 21:40
@detherminal detherminal marked this pull request as ready for review June 2, 2025 21:48
@detherminal
Copy link
Contributor Author

Added a dialog box for both mnemonic display and verify for the new wallet. Fixed other things as Tor, node choice and loading screen.

@detherminal
Copy link
Contributor Author

@julian-CStack @sneurlax

Copy link
Collaborator

@julian-CStack julian-CStack left a comment

Choose a reason for hiding this comment

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

I'm probably missing some things as its been a long day

@detherminal detherminal requested a review from julian-CStack June 3, 2025 00:37
@detherminal
Copy link
Contributor Author

Moved the logic to a standalone function until wallet creation service is implemented.
Refactored SnackBar into app-wide FlushBar.
Added more error handling and cleaned.

@detherminal
Copy link
Contributor Author

@julian-CStack @sneurlax Sorry for ping.

@julian-CStack julian-CStack changed the base branch from staging to paper-wallet-import June 3, 2025 15:18
@julian-CStack
Copy link
Collaborator

While there have been improvements since the opening of this PR, the UX is still not great. The lack of error/failure messages and processing happening without indication in the UI as well as inconsistent mnemonic verification (ui and flow) and wallet creation flow still requires some work. So for now I will merge this into a separate branch until we have time to go over it again. You are welcome to continue working on it and create another PR with changes to https://github.com/cypherstack/stack_wallet/tree/paper-wallet-import

@julian-CStack julian-CStack merged commit 0b6039b into cypherstack:paper-wallet-import Jun 3, 2025
1 check failed
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.

3 participants