Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle LN Address username conflicts on new wallet registration flow #344

Merged
merged 29 commits into from
Jan 29, 2025
Merged

Handle LN Address username conflicts on new wallet registration flow #344

merged 29 commits into from
Jan 29, 2025

Conversation

erdemyerebasmaz
Copy link
Collaborator

@erdemyerebasmaz erdemyerebasmaz commented Jan 27, 2025 •
edited
Loading

Fixes #260
Fixes #263
Fixes #318
Fixes #319

This PR introduces significant refactoring of the WebhookCubit, replacing it with the newly created LnAddressCubit. The changes are aimed at improving the management of Lightning Address setup and username customization, as well as enhancing the user experience when handling username conflicts.

Depends on:

Changelist:

  • Store LN Address Setup status on secure storage
  • Renamed secure storage functions for consistency with the Cubit
  • Utilize profile username only when registering a new wallet
  • Implemented retry and randomization logic to handle username conflicts during registration.

UI-related:

  • Created a new username formatter and text input formatter to handle username formatting & validation
  • Added a loading indicator on the "Done" button in the Update LN Address Username bottom sheet while the LnAddressCubit is loading.
  • Separated the status of LN Address registration from the LN Address username update status to allow more control over error handling
    • Username conflict errors are now shown on the Update LN Address Username bottom sheet
    • Update LN Address Username bottom sheet isn't dismissed on conflict errors

Misc:

  • Added a factory class for creating instances of LnAddressCubit to improve dependency management.

dangeross and others added 15 commits January 27, 2025 08:00
@dangeross @erdemyerebasmaz
Register user profile name as username for lightning address
58187b8
@dangeross @erdemyerebasmaz
Decouple users Display Name from their LN Address username
7b86b7a
@erdemyerebasmaz
Allow registering with a custom username
81866b8
@erdemyerebasmaz
Save both LNURLp & LN Address on WebhookState
a0f6d34
@erdemyerebasmaz
Display LN Address on "Receive via LN Address" page
beddf91 
- Check if lnAddress is not empty instead of relying on isLnAddress param
- Display LN Address below action buttons
- Set Copy actions value to LN Address
- Open options menu on clicking LN Address
- Open a bottom sheet to customize LN Address on clicking customize option

Misc:
- Rename DestinationActions's filename
@erdemyerebasmaz
Add TODO comments for LN Address
c527e33
@erdemyerebasmaz
Refactor WebhookCubit
f937e79 
Split WebhookCubit into two services, one for Webhook related logic one for LNURL service related logic.

Other changes on ChangeLnAddressUsernameBottomSheet
- fix: Use keyboard for email address
- fix: remove input formatter
- fix: trim the username value
@erdemyerebasmaz
Differentiate webhook & lnurlpay service errors
1bab410 
Add toString to WebhookState
@erdemyerebasmaz
Wait for update to finish before popping ln address sheet
6c158c0 
and show flushbars.

Remove loader.
@erdemyerebasmaz
convert username to lowercase and trim whitespace
5ae1521
@erdemyerebasmaz
Return lnurl data from updateLnAddressUsername
8714a29
@erdemyerebasmaz
fix: correct the error & title value for webhook registration errors
978c595 
misc: change ordering of params of emitted states for readability.
@erdemyerebasmaz
Store & use the LN Address username when refreshing webhooks
3e0b7a6
@erdemyerebasmaz
Store LN Address Username on secure storage
7119707
@erdemyerebasmaz
[WIP] Refactor WebhookCubit & replace it with LnAddressCubit
5bfd299 
Fixes #318

- Store Ln Address setup information on secure storage
- Change secure storage fn names for consistency
- Display loader for Done button on Update LN Address Username bottom sheet while cubit is loading
- Decouple LN Address & LN Address username update status
- Show conflict errors on Update LN Address Username bottom sheet
- Add retry & randomizer logic for username conflicts on registration
- Create LN username formatter & text input formatter
- Create LnAddressCubitFactory
@erdemyerebasmaz erdemyerebasmaz changed the base branch from main to ln_address January 27, 2025 19:32
@erdemyerebasmaz
Copy link
Collaborator Author

Made additional changes to include username length validation, fixes on username sanitization, and consistent naming across the app & lnurl service for maintainability.

Additionally, error handling in WebhookService is improved, and retry logic in LnUrlPayService is refactored for reusability and the LnAddressCubit is reorganized for better readability.

Changelist:

  • Validate username length for LN Address 104edb8
  • chore: renaming to be consistent with LNURL service 395c566
  • chore: rename webhook registration status on secure storage 69beaaa
  • fix: convert sanitized username to lowercase & remove all whitespace 0a97d35
  • add error handling on WebhookService 4d3eab3
  • refactor LnUrlPayService, extract retry logic to a function 99799e6
  • refactor LnAddressCubit to improve readability be54971

@erdemyerebasmaz erdemyerebasmaz marked this pull request as ready for review January 28, 2025 12:19
@erdemyerebasmaz erdemyerebasmaz changed the title [WIP] WebhookCubit overhaul - Handle LN Address username conflicts on new wallet registration flow WebhookCubit overhaul - Handle LN Address username conflicts on new wallet registration flow Jan 28, 2025
danielgranhao
danielgranhao reviewed Jan 28, 2025
View reviewed changes
Copy link
Contributor

@danielgranhao danielgranhao left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple of comments

lib/cubit/ln_address/ln_address_cubit.dart Outdated Show resolved Hide resolved
packages/breez_preferences/lib/src/breez_preferences.dart Outdated Show resolved Hide resolved
@erdemyerebasmaz erdemyerebasmaz force-pushed the ln_address_cubit_rework branch from fec023b to b00a981 Compare January 28, 2025 23:43
@erdemyerebasmaz
fix: use the same timestamp for generating the signature & registrati…
2e1c848 
…on request
@erdemyerebasmaz
refactor: Remove unused values on BreezPreferences
718b17d 
- Created a getter for SharedPreferences instance
  - Used getters for get functions
- Added comments to section keys
- Renamed keys & their fn's for consistency

This change will cause a regression as the key value has changed for _kBugReportBehavior.
@erdemyerebasmaz erdemyerebasmaz force-pushed the ln_address_cubit_rework branch from b00a981 to 718b17d Compare January 28, 2025 23:48
@erdemyerebasmaz erdemyerebasmaz force-pushed the ln_address_cubit_rework branch from 6173f92 to 72701f4 Compare January 29, 2025 00:54
@erdemyerebasmaz
Copy link
Collaborator Author

erdemyerebasmaz commented Jan 29, 2025 •
edited
Loading

Addressed feedback & made additional changes to ease the review process & code maintainability.

Feedback changes:

Changelist:

TODO:

  • Add docstrings for LnUrlPayService & WebhookService

dangeross
dangeross approved these changes Jan 29, 2025
View reviewed changes
Copy link
Contributor

@dangeross dangeross left a comment

Choose a reason for hiding this comment

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

LGTM

@erdemyerebasmaz erdemyerebasmaz changed the title WebhookCubit overhaul - Handle LN Address username conflicts on new wallet registration flow Handle LN Address username conflicts on new wallet registration flow Jan 29, 2025
@erdemyerebasmaz erdemyerebasmaz changed the base branch from ln_address to main January 29, 2025 11:15
This was referenced Jan 29, 2025
Closed
Closed
@erdemyerebasmaz erdemyerebasmaz merged commit f320d24 into main Jan 29, 2025
1 check passed
@erdemyerebasmaz erdemyerebasmaz deleted the ln_address_cubit_rework branch January 29, 2025 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielgranhao danielgranhao danielgranhao approved these changes

@dangeross dangeross dangeross approved these changes

@roeierez roeierez Awaiting requested review from roeierez

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@erdemyerebasmaz @dangeross @danielgranhao