Skip to content

Conversation

@n13
Copy link
Collaborator

@n13 n13 commented Jan 9, 2026

Moved from develop branch

@n13 n13 marked this pull request as ready for review January 11, 2026 12:33
@dewabisma
Copy link
Collaborator

Gemini 3 Pro Review


[9 tools called]

The branch move_hs_account_flow_to_main introduces the High Security (Guardian Account) feature. This appears to be a UI/Frontend implementation pass, as most of the underlying logic and data fetching are currently mocked.

Here is a thorough code review of the changes:

1. Functional Overview

The branch implements the "High Security" setup flow, allowing users to designate a "Guardian Account" to protect their funds.

  • Entry Point: Added to AccountSettingsScreen (guarded by FeatureFlags.enableHighSecurity).
  • Setup Flow: "Get Started" -> "Guardian Wizard" (Address Input) -> "Safeguard Window Wizard" (Time Picker) -> "Summary" -> "Confirmation".
  • Dashboard: "Entrusted Accounts" now appear nested under their main account in the AccountsScreen.

2. Critical Issues (Must Fix / Verify)

  • Hardcoded UI Values:

    • In mobile-app/lib/features/main/screens/high_security/high_security_summary_wizard.dart, the "Everyday Account" name and address are hardcoded to "Grain-Red-Flash-Hyper-Cloud" and a specific address. This must be replaced with the actual active account data before merging or testing with real accounts.
    • In mobile-app/lib/providers/entrusted_account_provider.dart, the provider returns dummy data if the account name starts with 'G'.
  • Mocked Services:

    • quantus_sdk/lib/src/services/high_security_service.dart: The methods setupHighSecurityAccount and getHighSecuritySetupFee are mocks (using Future.delayed).
      • getHighSecuritySetupFee returns a hardcoded fee of 1.0.
      • setupHighSecurityAccount does nothing but wait.
    • Ensure there is a follow-up task to implement the actual Substrate extrinsic calls.
  • Error Logging:

    • HighSecurityService uses print('Failed to setup: $e'); for error handling. In production code, you should use a proper logging service (e.g., Logger or a crash reporting tool) instead of print.

3. Detailed File Review

SDK (quantus_sdk/)

  • HighSecurityData: Simple, immutable data class. Looks good.
  • DatetimeFormattingService: Added formatSafeguardTime. Logic assumes inputs are strictly partitioned (e.g., if months > 0, it shows months/days/hours).
    • Nit: The logic might produce slightly odd outputs like "1 month, 0 days, 5 hr". Verify if 0 days should be omitted if it's zero but hours are non-zero.
  • HighSecurityService: As noted, this is a skeleton. getHighSecuritySetupCall throws "No Implementation".

Providers (mobile-app/lib/providers/)

  • HighSecurityFormNotifier: Clean implementation of StateNotifier for managing the multi-step form state.
  • EntrustedAccountProvider: Currently a mock. There is a TODO: Implement actual fetching.

Screens & UI (mobile-app/lib/features/main/screens/)

high_security/ (The Wizard Flow)

  • HighSecurityGuardianWizard:
    • Good validation (disables "Next" if address is empty).
    • Supports Paste and QR Scan.
  • HighSecuritySafeguardWindowWizard:
    • Calculates months/days/hours from seconds.
    • Note: It assumes 1 Month = 30 Days (86400 * 30). This is a standard approximation for duration windows but ensure this aligns with how the blockchain logic interprets time (usually block numbers or strict seconds).
  • SafeguardWindowPickerSheet:
    • Uses 3 separate CupertinoPicker widgets.
    • UX looks solid, but verify on small screens if the "Months/Days/Hours" headers align well with the columns.
  • HighSecurityConfirmationSheet:
    • Correctly fetches the (mocked) fee and displays warnings.
    • Blocks interaction (_isSubmitting) while "confirming".

AccountsScreen

  • _buildAccountListItem:
    • Now calculates entrustedNodes and renders a TreeListView if data exists.
    • Visual hierarchy is clear with the darker background for entrusted accounts.
  • AccountTag: New widget to label "Entrusted" accounts. Looks good.

4. Recommendations

  1. Refactor Hardcoding: Immediately replace the hardcoded "Grain-Red-Flash..." account info in HighSecuritySummaryWizard with ref.watch(activeAccountProvider) or pass the account through the wizard.
  2. Define Implementation Plan: Since the service layer is mocked, clarify if this PR is strictly "UI Implementation". If so, add comments or tickets linking to the backend implementation work.
  3. Cleanup: Remove the print statements in the SDK service.
  4. Date Logic: specific logic in DatetimeFormattingService.formatSafeguardTime might need a unit test to ensure edge cases (e.g., exact 1 month, or 1 month + 1 hour) format cleanly.

Copy link
Collaborator

@dewabisma dewabisma left a comment

Choose a reason for hiding this comment

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

Personally LGTM!

I don't think there is a problem even in Gemini review, just some recommendations

@n13 n13 merged commit 00bc015 into main Jan 12, 2026
1 check passed
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