Skip to content

sync main#3298

Merged
ca333 merged 91 commits intomainfrom
dev
Oct 29, 2025
Merged

sync main#3298
ca333 merged 91 commits intomainfrom
dev

Conversation

@ca333
Copy link
Copy Markdown
Contributor

@ca333 ca333 commented Oct 29, 2025

changelog

Enhancements

Migrate faucet check to SDK extension (#3206)

Add realtime balance updates via SDK streaming

Reduce MM2 RPC spam via SDK cache and UI debouncing (#3253, #3238)

Add fiat value display to withdrawal amount and fee (#3274)

Add vertical layout for withdrawal form on narrow devices

Prefer cached pubkeys before RPC calls across app

Show unconfirmed transactions first in transaction list

Add Banxa iframe allow list for fiat onramp

Fixes

Show seed backup banner in mobile view

Dismiss keyboard on scroll for fiat and swap inputs

Fix alignment and spacing in active coins list

Fix text alignment and scaling for longer names in fiat onramp

Add komodo and sandbox domains to fiat onramp whitelist (#3225)

Disable overly restrictive CSP with limited platform support

Hide custom seed toggle unless BIP39 validation fails (#3260)

Fix activation state alignment (#3272)

Fix unban_pubkeys null check error (#3276)

Fix canceled ZHTLC activation toggle (#3283)

Fix transaction history list sorting

Preserve coin activation state on realtime balance updates

Isolate TransactionHistoryBloc per-coin to prevent cross-asset history bleed

Fix unnecessary activation cache invalidation

Fix decimal handling (#3297)

Fix web build error due to unsupported Platform import

Refactoring

Remove unused widget and update enum docs

Deprecate legacy fiat onramp validation

Use absolute value of netchange in withdrawal preview details

SDK Updates

Multiple SDK submodule updates with streaming improvements, type safety enhancements, and various bug fixes
Related Issues/PRs include: #3206, #3225, #3238, #3253, #3260, #3272, #3274, #3276, #3283, #3297

Summary by CodeRabbit

Release Notes

  • New Features

    • Real-time balance monitoring with immediate UI updates
    • Locale-aware decimal parsing for international number formats
    • Enhanced fiat form with keyboard dismissal on scroll
    • Improved asset activation with intelligent caching
  • Bug Fixes

    • Fixed asset activation error messaging guidance
    • Corrected iOS app bundle identifier
    • Enhanced transaction history merging and error handling
    • Improved fiat iframe security permissions
  • Performance

    • Optimized coin activation polling with configurable intervals
    • Implemented pubkey caching to reduce RPC calls
    • Extended price update intervals from 1 to 3 minutes
  • Refactor

    • Streamlined asset activation using repository-based checks
    • Added comprehensive resource cleanup and lifecycle management
    • Improved wallet import UI with dynamic custom seed phrase visibility

takenagain and others added 30 commits October 21, 2025 22:21
twas disabled by default for mobile for some reason (undocumented)
coins with long names, like BAT, would overflow and cause alignment issues.

Fixed with expanded and flex
…elay

perf(wallet): throttle price polling to 3m; switch to MapEquality to avoid Flutter import (#3238)\n\nchore: update SDK submodule pointer

chore: clean-up temp files
- Wire SDK `ActivatedAssetsCache` into activation/coins flows:
  updates across `CoinsBloc`, `AssetOverviewBloc`, custom token import, and
  `sdk_auth_activation_extension` to reuse activation state instead of re-querying.
- Debounce/polish polling in `portfolio_growth_bloc` and `profit_loss_bloc`
  to prevent overlapping requests.
- Remove duplicate activation/balance checks in maker/taker validators and forms.
- Consolidate repeated calls in `mm2_api`/`mm2_api_nft`/`rpc_native`; prefer cached values.
- Reduce startup RPCs in `app_bootstrapper`; stop background timers in
  `window_close_handler` on app close to avoid trailing calls.
- Add shared intervals in `shared/constants`; introduce
  `lib/shared/utils/activated_assets_cache.dart` for app-specific helpers.
- No UI changes; measurable reduction in RPC volume and improved responsiveness.

Refs #3238
also decrease perimeter padding and increase vertical padding
CharlVS and others added 25 commits October 28, 2025 17:11
…es\n\nEnsure CoinsBloc merges balance-change updates with existing coin state to avoid flipping active/suspended flags, which could disable Send erroneously.
…set history bleed

- Remove global TransactionHistoryBloc provider from AppBlocRoot
- Provide per-page bloc in CoinDetails and subscribe to widget.coin
- Prevents DOGE history appearing on MATIC page and similar cross-contamination
- No SDK changes required
hide custom seed toggle unless bip39 validation fails
…oins

feat(faucet): migrate hasFaucet check to the SDK extension
fix: mobile seed banner, text alignment, and input dismissal
Align coin activation state in fiat ramp page with portfolio view
…heck-error

fix(settings): prevent null-assert on Android by using disabled trailing button for unban_pubkeys
fix(withdraw): add USD fiat values alongside amounts
revert toggle on zhtlc activation config cancel
perf: eliminate RPC polling via event streaming, persistent caching, and deduplication (#3238)
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 29, 2025

Walkthrough

This PR introduces major improvements to asset activation, caching, and balance tracking across the app. It refactors coin activation logic to use a centralized cache, adds real-time balance change broadcasting, updates fiat onramp handling with improved state management and WebView configuration, adds locale-aware decimal parsing utilities, and refactors multiple UI components for better responsiveness and keyboard handling. Additionally, it updates the iOS bundle identifier, clarifies BIP39 validation messaging, and upgrades the SDK submodule.

Changes

Cohort / File(s) Summary
Translation & Configuration
assets/translations/en.json, lib/app_config/app_config.dart
Updated BIP39 seed error message to direct users to 'Use custom seed phrase / private key (WIF)' option. Modified app config to enable 'BTC-segwit' by default alongside 'KMD' and removed the public coinsWithFaucet getter.
iOS Build Configuration
ios/Runner.xcodeproj/project.pbxproj
Updated iOS app bundle identifier from com.komodoplatform.atomicdex to com.komodo.wallet across Debug, Profile, and Release build configurations.
Fiat Widget & Web Integration
assets/web_pages/fiat_widget.html, lib/views/fiat/fiat_provider_web_view_settings.dart
Added iframe allow attribute (payment, encrypted-media, microphone, camera, midi) and JSDoc formatting. Extended trusted domains to include Banxa sandbox and Komodo platform. Enabled iframe allow permissions and adjusted sandbox settings; made Ramp demo domain conditional in debug builds.
Core Bloc Architecture
lib/bloc/app_bloc_root.dart, lib/bloc/coins_bloc/coins_bloc.dart, lib/bloc/coins_bloc/coins_repo.dart, lib/bloc/coins_bloc/coins_event.dart, lib/bloc/coins_bloc/asset_coin_extension.dart
Removed TransactionHistoryBloc from AppBlocRoot. Added real-time balance change handling via CoinsBalanceChanged event and balance broadcast mechanism in CoinsRepo. Introduced activation caching and new public methods (getActivatedAssetIds, isAssetActivated). Added ZhtlcActivationCancelled exception for cancellation handling. Removed AssetListOps extension and updated CoinSupportOps to use activated assets cache.
Activation & Polling Configuration
lib/shared/constants.dart, lib/bloc/assets_overview/bloc/asset_overview_bloc.dart, lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart, lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart, lib/bloc/cex_market_data/sdk_auth_activation_extension.dart
Introduced kActivationPollingInterval constant (environment-configurable, default 2000ms) and updated all activation threshold-wait calls to use this explicit delay parameter.
Pubkey Caching & Asset Retrieval
lib/bloc/coin_addresses/bloc/coin_addresses_bloc.dart, lib/bloc/bridge_form/bridge_validator.dart, lib/bloc/taker_form/taker_validator.dart, lib/bloc/nft_receive/bloc/nft_receive_bloc.dart, lib/bloc/withdraw_form/withdraw_form_bloc.dart, lib/mm2/mm2_api/mm2_api.dart
Consistently replaced direct getPubkeys() calls with cache-first pattern: sdk.pubkeys.lastKnown(asset.id) with fallback to getPubkeys() for improved performance.
Decimal Parsing Utilities
lib/shared/utils/formatters.dart, test_units/shared/utils/formatters_test.dart
Added normalizeDecimalString(), parseLocaleAwareDecimal(), and parseLocaleAwareRational() functions to handle mixed locale formats (European, US, Russian, etc.). Includes comprehensive unit tests covering basic parsing, error conditions, and real-world locale scenarios.
Locale-Aware Amount Input
lib/views/dex/simple/form/amount_input_field.dart, lib/views/fiat/custom_fiat_input_field.dart, lib/views/market_maker_bot/coin_trade_amount_form_field.dart, lib/model/forms/fiat/fiat_amount_input.dart, lib/blocs/maker_form_bloc.dart, lib/bloc/taker_form/taker_bloc.dart, lib/bloc/withdraw_form/withdraw_form_state.dart
Replaced direct Rational.parse() and Decimal.parse() with locale-aware parsing functions across form handling and amount input fields.
Fiat Onramp Form Enhancement
lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart, lib/bloc/fiat/fiat_onramp_form/fiat_form_state.dart, lib/bloc/fiat/fiat_order_status.dart
Added coinsRepo dependency to FiatFormBloc for asset activation. Introduced isBanxaSelected getter in FiatFormState. Added FiatOrderStatus.submitting state to track in-flight order submission. Optimized pubkey retrieval and replaced direct SDK activation with CoinsRepo-based calls.
Fiat UI Components & Keyboard Management
lib/views/fiat/fiat_form.dart, lib/views/fiat/fiat_inputs.dart, lib/views/fiat/custom_fiat_input_field.dart, lib/views/fiat/fiat_asset_icon.dart, lib/views/fiat/fiat_select_button.dart
Migrated CustomFiatInputField to stateful widget with focus node and submission callback support. Added scroll-based keyboard dismissal in FiatForm and FiatInputs. Refactored FiatAssetIcon layout with expanded mode for responsive rendering. Updated FiatSelectButton label constraints and asset rendering logic.
Wallet Import & BIP39 Validation
lib/views/wallets_manager/widgets/wallet_import_by_file.dart, lib/views/wallets_manager/widgets/wallet_simple_import.dart
Enhanced BIP39 validation flow with dynamic toggle visibility for custom seed option. Added logic to reveal "Use custom seed" toggle only after validation failure or for non-BIP39 seeds in HD mode. Added seed-change listener in simple import to rebuild UI reactively.
Transaction History & Error Handling
lib/bloc/transaction_history/transaction_history_bloc.dart, lib/views/wallet/coin_details/transactions/transaction_table.dart
Refactored transaction merging to use internalId-based map instead of stream filtering, enabling deterministic ordering and incremental updates. Added specialized error handling for ActivationFailedException. Updated error display logic to parse activation-specific failures.
Withdrawal & Address UI
lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart, lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
Made WithdrawPreviewDetails responsive with LayoutBuilder-based row/column switching based on available width. Updated faucet visibility check to use coin.id.hasFaucet extension. Added AssetAmountWithFiat widget for display.
Responsive Layout & Scroll Management
lib/views/dex/simple/form/maker/maker_form_layout.dart, lib/views/dex/simple/form/taker/taker_form_layout.dart, lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart, lib/views/common/pages/page_layout.dart
Converted desktop/mobile taker form layouts to stateful widgets with scroll controllers and keyboard dismissal-on-scroll handlers. Added scroll listeners to maker form layouts. Refactored expandable coin list to use Expanded and AutoScrollText for mobile layout. Updated page layout formatting.
Resource Management & Disposal
lib/bloc/custom_token_import/bloc/custom_token_import_bloc.dart, lib/bloc/custom_token_import/data/custom_token_import_repository.dart, lib/mm2/rpc_native.dart, lib/mm2/mm2_api/mm2_api.dart, lib/mm2/mm2_api/mm2_api_nft.dart, lib/sdk/widgets/window_close_handler.dart, lib/services/initializer/app_bootstrapper.dart
Added lifecycle disposal methods to CustomTokenImportBloc and repository with HTTP client ownership tracking. Updated RPCNative and Mm2Api with HTTP client injection and dispose patterns. Added SparklineRepository to dependency registration. Enhanced window close handler to dispose Mm2Api and SparklineRepository from GetIt.
Trading & Activation Management
lib/bloc/coins_bloc/coins_manager_bloc.dart, lib/bloc/taker_form/taker_bloc.dart, lib/blocs/trading_entities_bloc.dart, lib/blocs/maker_form_bloc.dart, lib/views/dex/simple/confirm/taker_order_confirmation.dart
Added handling for ZhtlcActivationCancelled exception in coin activation with optimistic UI revert. Enhanced taker bloc with logging, error handling for max-sell-amount updates, and locale-aware amount parsing. Changed TradingEntitiesBloc polling from 1s to 10s interval. Added 1-second delay before trading entities fetch after swap starts.
New Widgets & UI Components
lib/shared/widgets/asset_amount_with_fiat.dart
Introduced AssetAmountWithFiat widget to display crypto amounts with optional fiat values, supporting auto-scroll and selectable text modes with dynamic coin symbol display.
Settings & UI Polish
lib/views/settings/widgets/security_settings/unban_pubkeys_plate.dart
Replaced inline action text with UiPrimaryButton with responsive sizing and disabled state during unbanning.
Coin Model & Extension Updates
lib/model/coin.dart
Removed public hasFaucet getter from Coin class; functionality now accessed via coin.id.hasFaucet extension.
SDK Submodule
sdk
Updated SDK submodule reference from commit 0b231266... to 74d4999c..., aligning with SDK asset activation and caching enhancements.
Main App Initialization
lib/main.dart
Disabled BalanceManager debug logging (commented out).

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant UI as UI/Form
    participant CoinsBloc
    participant CoinsRepo as CoinsRepo<br/>(Balance & Activation)
    participant SDK as KomodoDefiSdk
    
    User->>UI: Input amount
    UI->>UI: Parse with<br/>parseLocaleAwareRational()
    
    alt Cache hit
        UI->>CoinsRepo: Check isAssetActivated()
        CoinsRepo-->>UI: cached result
    else Cache miss
        UI->>CoinsRepo: Check isAssetActivated(forceRefresh=true)
        CoinsRepo->>SDK: Query asset state
        SDK-->>CoinsRepo: Activation status
        CoinsRepo-->>UI: result (cached)
    end
    
    UI->>CoinsRepo: Activate asset if needed
    CoinsRepo->>SDK: Activate asset
    SDK-->>CoinsRepo: Activation complete
    CoinsRepo->>CoinsRepo: Broadcast balance change
    CoinsRepo-->>CoinsBloc: balanceChanges stream
    CoinsBloc->>CoinsBloc: Emit CoinsBalanceChanged
    CoinsBloc-->>UI: UI rebuilds with new balance
Loading
sequenceDiagram
    actor User
    participant CoinsBloc
    participant CoinsRepo
    participant SDK as KomodoDefiSdk
    
    Note over CoinsBloc,SDK: Asset Activation Flow
    CoinsBloc->>CoinsRepo: Wait for coins threshold<br/>(with kActivationPollingInterval)
    CoinsRepo->>SDK: Query activated assets
    SDK-->>CoinsRepo: Active asset IDs
    CoinsRepo->>CoinsRepo: Cache result in<br/>activatedAssetsCache
    
    alt Asset not yet activated
        CoinsRepo->>SDK: Activate asset
        SDK-->>CoinsRepo: Activation in progress
        CoinsRepo->>CoinsRepo: Broadcast activating state
    else Asset already activated
        CoinsRepo->>CoinsRepo: Skip activation call<br/>(optimization)
    end
    
    SDK-->>CoinsRepo: Activation complete
    CoinsRepo->>CoinsRepo: Subscribe to balance changes<br/>Broadcast balance update
    CoinsRepo-->>CoinsBloc: balanceChanges stream
    CoinsBloc->>CoinsBloc: Emit CoinsBalanceChanged
Loading
sequenceDiagram
    participant FiatForm as Fiat Form UI
    participant FiatFormBloc
    participant CoinsRepo
    participant CustomFiatInputField
    
    Note over FiatForm: Fiat Amount Input with Keyboard Management
    
    FiatForm->>CustomFiatInputField: Create with focusNode
    CustomFiatInputField->>CustomFiatInputField: Init _focusNode
    
    User->>CustomFiatInputField: Enter amount
    CustomFiatInputField->>CustomFiatInputField: normalizeDecimalString()
    CustomFiatInputField->>CustomFiatInputField: Parse as Decimal
    CustomFiatInputField-->>FiatFormBloc: onTextChanged(amount)
    
    User->>CustomFiatInputField: Press submit/done
    CustomFiatInputField->>CustomFiatInputField: Unfocus
    CustomFiatInputField-->>FiatFormBloc: onSubmitted(amount)
    
    FiatForm->>FiatForm: Scroll event
    FiatForm->>FiatForm: On scroll: unfocus keyboard
    FiatForm->>FiatFormBloc: Form submit
    FiatFormBloc->>CoinsRepo: Activate coin if needed
    CoinsRepo-->>FiatFormBloc: Activation complete
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

  • lib/bloc/coins_bloc/coins_repo.dart — Major refactoring with new activation caching, balance broadcasting, and exception handling; intricate control flow in _activateZhtlcAsset and activateAssetsSync
  • lib/bloc/coins_bloc/coins_bloc.dart — Reworked balance change subscription, dynamic activation threshold logic with _fire mechanism, and replaced price comparison approach; new state management pattern
  • lib/bloc/bridge_form/bridge_validator.dart & lib/bloc/taker_form/taker_validator.dart — Heterogeneous changes across multiple validation paths with refactored error handling, pubkey caching, and localized error formatting
  • lib/views/fiat/fiat_onramp_form/fiat_form_bloc.dart — Integration of CoinsRepo dependency, activation flow changes, and WebView mode determination refactor
  • lib/views/dex/simple/form/taker/taker_form_layout.dart — Conversion to stateful widget with new scroll/keyboard management patterns and multiple control flow changes
  • lib/shared/utils/formatters.dart & test_units/shared/utils/formatters_test.dart — New locale-aware parsing logic with edge cases; comprehensive test coverage validates multiple locale formats
  • lib/bloc/transaction_history/transaction_history_bloc.dart — Substantial rework of transaction merging logic with internalId-based state-driven map, new comparator/sorting logic, and specialized error handling
  • lib/mm2/mm2_api/mm2_api_nft.dart — Refactored NFT chain activation to use activated assets cache with adjusted logic paths

Possibly related PRs

Suggested reviewers

  • smk762
  • CharlVS

Poem

🐰 Hoppin' through the code with care,
Caching coins everywhere!
Locales parsed with decimal grace,
Balance updates set the pace—
Keyboards dismissed with nimble bounds,
Activation's now more sound!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The pull request title "sync main" is vague and uses a non-descriptive term that provides no meaningful information about the actual substance of the changeset. While the PR contains extensive changes across multiple files including translation updates, UI refactoring, bloc architecture modifications, activation flow changes, and numerous widget improvements, the title fails to convey any of these specifics. It reads as a generic operation description rather than a meaningful summary of what was actually changed, similar to terms like "misc updates" that don't help reviewers understand the primary intent. Consider revising the title to reflect the primary objective of the changes. If this is a routine sync from main, consider titles like "Merge main into dev" or specific to the key features added (e.g., "Add balance change monitoring and improve fiat form UX"). The current title lacks sufficient specificity for developers scanning commit history to understand what changed.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

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.

@github-actions
Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit ebb3f74):

https://walletrc--pull-3298-merge-ckuixm3h.web.app

(expires Wed, 05 Nov 2025 14:20:27 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc

@ca333 ca333 merged commit 7bf4f27 into main Oct 29, 2025
8 of 15 checks passed
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (11)
lib/views/fiat/fiat_asset_icon.dart (1)

18-24: Remove unused parameters/fields (onTap, assetExists) or wire them in

onTap is declared required but never used; assetExists wasn’t used until the previous fix; keep assetExists only if you rely on it, otherwise drop it. Unused required params will fail flutter analyze in many configs.

Option A (minimal): Drop onTap now; keep assetExists only if needed.

   const FiatAssetIcon({
     required this.currency,
     required this.icon,
-    required this.onTap,
-    required this.assetExists,
     super.key,
     this.expanded = false,
   });
@@
   final ICurrency currency;
   final Widget icon;
-  final VoidCallback onTap;
-  final bool? assetExists;
   final bool expanded;

Then update call site in lib/views/fiat/fiat_select_button.dart:

-              icon: icon,
-              onTap: onTap,
-              assetExists: assetExists,
+              icon: icon,
+              // assetExists no longer needed if null-guard by SDK is sufficient

Option B (use them): If you want tap handling here, wrap the returned Row with InkWell(onTap: onTap, child: …) and keep onTap. Pick one path and make CI green. As per coding guidelines.

Also applies to: 26-31

lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart (2)

53-71: Guard PageStorage null and avoid key collisions; use coin.id

PageStorage.of(context) can be null outside a PageStorage scope; also abbr-based IDs can collide across chains. Use null-aware bucket and coin.id.

Apply this diff:

-    // Attempt to restore state from PageStorage using a unique key
-    _isExpanded =
-        PageStorage.of(
-              context,
-            ).readState(context, identifier: '${widget.coin.abbr}_expanded')
-            as bool? ??
-        false;
+    // Attempt to restore state from PageStorage using a unique, collision-free key
+    final bucket = PageStorage.of(context);
+    _isExpanded = (bucket?.readState(
+          context,
+          identifier: '${widget.coin.id}_expanded',
+        ) as bool?) ??
+        false;
-      // Save state to PageStorage using a unique key
-      PageStorage.of(context).writeState(
-        context,
-        _isExpanded,
-        identifier: '${widget.coin.abbr}_expanded',
-      );
+      // Save state to PageStorage using a unique key
+      final bucket = PageStorage.of(context);
+      bucket?.writeState(
+        context,
+        _isExpanded,
+        identifier: '${widget.coin.id}_expanded',
+      );

Additionally update the PageStorageKey for consistency:

-      key: PageStorageKey('coin_${widget.coin.abbr}'),
+      key: PageStorageKey('coin_${widget.coin.id}'),

1-1: Fix incorrect header comment path

The file header comment references a non-existent path that doesn't match the actual file location. Line 1 should be updated from:

// lib/src/defi/asset/coin_list_item.dart

to:

// lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart

Note: Formatting and static analysis verification could not be completed in the sandbox environment (dart and flutter tools unavailable). Recommend running locally before merging:

  • dart format lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart
  • flutter analyze lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart
lib/mm2/mm2_api/mm2_api_nft.dart (1)

135-154: Switch to nftAssetTicker() in getActiveNftChains for consistency with NFT asset IDs

The implementations confirm the inconsistency: coinAbbr() returns base tickers ('ETH', 'BNB', 'AVAX', 'MATIC', 'FTM') while nftAssetTicker() returns prefixed identifiers ('NFT_ETH', 'NFT_BNB', 'NFT_AVAX', 'NFT_MATIC', 'NFT_FTM'). Since activatedAssetsCache.getActivatedAssets() returns NFT assets with ids matching the 'NFT_' prefix format (as confirmed by enableNftChains at line 182), the comparison at line 147 will fail.

Change:

.where((c) => enabledCoinIds.contains(c.coinAbbr()))

to:

.where((c) => enabledCoinIds.contains(c.nftAssetTicker()))
lib/views/fiat/fiat_form.dart (1)

286-311: Critical: switch cases fall through; add breaks or returns.

Dart does not allow implicit fall-through. The cases for submitted/success/failed need breaks (or early returns). As written, this won’t compile.

-      case FiatOrderStatus.submitted:
+      case FiatOrderStatus.submitted:
         title = LocaleKeys.fiatPaymentSubmittedTitle.tr();
         content = LocaleKeys.fiatPaymentSubmittedMessage.tr();
         icon = const Icon(Icons.open_in_new);
+        break;
       case FiatOrderStatus.success:
         title = LocaleKeys.fiatPaymentSuccessTitle.tr();
         content = LocaleKeys.fiatPaymentSuccessMessage.tr();
         icon = const Icon(Icons.check_circle_outline);
+        break;
       case FiatOrderStatus.failed:
         title = LocaleKeys.fiatPaymentFailedTitle.tr();
         content = LocaleKeys.fiatPaymentFailedMessage.tr();
         if (state.providerError != null && state.providerError!.isNotEmpty) {
           content =
               '$content\n\n${LocaleKeys.errorDetails.tr()}: '
               '${state.providerError}';
         }
         icon = const Icon(Icons.error_outline, color: Colors.red);
+        break;
lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart (1)

117-121: Status remains loading when not signed in.

_onCoinSelected first sets status: loading, then early-returns if not signed in without resetting status, leaving the UI stuck in loading.

-      if (!await _sdk.auth.isSignedIn()) {
-        return emit(state.copyWith(selectedAssetAddress: () => null));
-      }
+      if (!await _sdk.auth.isSignedIn()) {
+        return emit(
+          state.copyWith(
+            selectedAssetAddress: () => null,
+            status: FiatFormStatus.initial,
+            fiatOrderStatus: FiatOrderStatus.initial,
+          ),
+        );
+      }
lib/blocs/maker_form_bloc.dart (2)

229-246: Cancel timers in dispose to prevent leaks and callbacks after teardown

Both _maxSellAmountTimer and _preimageDebounceTimer aren’t cancelled in dispose(). Cancel them alongside closing controllers.

   @override
   void dispose() {
+    _maxSellAmountTimer?.cancel();
+    _preimageDebounceTimer?.cancel();
     _inProgressCtrl.close();
     _showConfirmationCtrl.close();
     _sellCoinCtrl.close();
     _buyCoinCtrl.close();
     _sellAmountCtrl.close();
     _buyAmountCtrl.close();
     _priceCtrl.close();
     _isMaxActiveCtrl.close();
     _showSellCoinSelectCtrl.close();
     _showBuyCoinSelectCtrl.close();
     _formErrorsCtrl.close();
     _availableBalanceStateCtrl.close();
   }

Also applies to: 682-690


605-613: Wrap parseLocaleAwareRational in try/catch in setPriceValue

Unlike sell/buy setters, setPriceValue doesn’t catch parsing errors; invalid input will throw.

   void setPriceValue(String? priceStr) {
     priceStr ??= '';
     Rational? priceValue;
 
     if (priceStr.isEmpty) {
       priceValue = null;
     } else {
-      priceValue = parseLocaleAwareRational(priceStr);
+      try {
+        priceValue = parseLocaleAwareRational(priceStr);
+      } catch (_) {
+        priceValue = null;
+      }
     }
 
     if (priceValue == price) return;
     price = priceValue;
     _onPriceUpdated();
   }
lib/shared/widgets/asset_amount_with_fiat.dart (1)

361-372: Inconsistent sign handling between layouts

Row layout shows abs() for netChange; column layout uses raw (possibly negative) amount. Align them.

-                  AssetAmountWithFiat(
-                    assetId: assets.id,
-                    amount: preview.balanceChanges.netChange,
-                    isAutoScrollEnabled: true,
-                  ),
+                  AssetAmountWithFiat(
+                    assetId: assets.id,
+                    amount: preview.balanceChanges.netChange.abs(),
+                    isAutoScrollEnabled: true,
+                  ),
lib/bloc/custom_token_import/data/custom_token_import_repository.dart (1)

162-176: Handle non‑200 responses before jsonDecode

jsonDecode on error bodies can throw or mislead. Check statusCode and return null with a warning.

     try {
       final response = await _httpClient
           .get(url)
           .timeout(
             timeout,
             onTimeout: () {
               throw TimeoutException('Timeout fetching token data from $url');
             },
           );
-      final data = jsonDecode(response.body);
-      return data;
+      if (response.statusCode != 200) {
+        _log.warning('Token data request failed ${response.statusCode} for $url');
+        return null;
+      }
+      final data = jsonDecode(response.body) as Map<String, dynamic>;
+      return data;
     } catch (e, s) {
       _log.severe('Error fetching token data from $url', e, s);
       return null;
     }
lib/views/fiat/custom_fiat_input_field.dart (1)

23-24: Fix onChanged type mismatch with TextField

TextField.onChanged expects ValueChanged. Wrap the callback to satisfy the non‑nullable signature.

-  final void Function(String?) onTextChanged;
+  final void Function(String?) onTextChanged;
@@
-          onChanged: widget.onTextChanged,
+          onChanged: (value) => widget.onTextChanged(value),

Alternatively, change the field type to ValueChanged.

Also applies to: 96-100

♻️ Duplicate comments (1)
lib/views/dex/simple/form/maker/maker_form_layout.dart (1)

205-205: Same keyboard dismissal pattern as desktop layout.

This implementation mirrors the desktop layout's approach. The same optional suggestions from the desktop layout review apply here (mounted check and potential code duplication).

Also applies to: 210-210, 215-218

🧹 Nitpick comments (41)
lib/views/dex/simple/form/maker/maker_form_layout.dart (1)

123-124: Good UX improvement with proper lifecycle management.

The keyboard dismissal on scroll is a nice touch that enhances user experience. The listeners are correctly added in initState and removed in dispose.

Optional enhancements:

  1. Add a mounted check for defensive coding:
void _onScroll() {
  if (!mounted) return;
  FocusScope.of(context).unfocus();
}
  1. Consider extracting the duplicate _onScroll method (also in mobile layout) to reduce code duplication, perhaps via a mixin or base class.

Also applies to: 129-130, 136-139

lib/views/dex/simple/confirm/taker_order_confirmation.dart (1)

349-351: Consider adding retry logic for robustness.

The 1-second fixed delay works around a race condition with MM2/KDF swap registration, but if the delay proves insufficient under load, the fetch() call could fetch stale or incomplete data without recovery. This pattern aligns with similar timing workarounds in the codebase (e.g., analytics_lifecycle_handler uses 150ms for iOS backgrounding).

However, for better reliability, consider:

  • Adding retry logic around tradingEntitiesBloc.fetch()
  • Verifying swap registration before calling fetch (if MM2/KDF API supports it)
  • Polling with a condition check as optional future enhancement

If you're confident 1 second is sufficient under typical load conditions, the current approach is acceptable given the existing codebase patterns.

lib/views/fiat/fiat_asset_icon.dart (1)

43-58: Simplify Flexible branch

Inside the else you always have expanded == false, so fit: expanded ? FlexFit.tight : FlexFit.loose is always FlexFit.loose. Remove the conditional to reduce cognitive load.

Apply this diff:

-            : Flexible(
-                fit: expanded ? FlexFit.tight : FlexFit.loose,
-                child: _FiatCoinItemLabel(size: size, coin: coin),
-              ),
+            : Flexible(
+                fit: FlexFit.loose,
+                child: _FiatCoinItemLabel(size: size, coin: coin),
+              ),
lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart (3)

170-218: Reduce rebuild/formatting overhead; dedupe usdBalance; verify Spacer + Expanded on small widths

  • Cache NumberFormat instead of creating per-build.
  • Read usdBalance once and reuse.
  • Consider BlocSelector to narrow rebuilds to only the 24h change signal.
  • Spacer plus two Expanded can overconstrain on narrow widths; sanity-check wrap/overflow.

Suggested changes:

  1. Cache currency formatter in State:
 class _ExpandableCoinListItemState extends State<ExpandableCoinListItem> {
+  // Cache to avoid re-creating on every build
+  late final NumberFormat _usdFmt = NumberFormat.currency(symbol: '\$');
  1. Use the cached formatter and reuse balance:
-                if (widget.coin.lastKnownUsdBalance(context.sdk) != null)
-                  Text(
-                    '\$${NumberFormat("#,##0.00").format(widget.coin.lastKnownUsdBalance(context.sdk)!)}',
-                    style: theme.textTheme.headlineMedium,
-                  ),
+                final usd = widget.coin.lastKnownUsdBalance(context.sdk);
+                if (usd != null)
+                  Text(
+                    _usdFmt.format(usd),
+                    style: theme.textTheme.headlineMedium,
+                  ),
-                    final usdBalance = widget.coin.lastKnownUsdBalance(
-                      context.sdk,
-                    );
+                    final usdBalance = widget.coin.lastKnownUsdBalance(context.sdk);
@@
-                      valueFormatter: (value) =>
-                          NumberFormat.currency(symbol: '\$').format(value),
+                      valueFormatter: (value) => _usdFmt.format(value),

Optional (narrow rebuilds):

// Replace BlocBuilder with BlocSelector if feasible:
// BlocSelector<CoinsBloc, CoinsState, double?>(
//   selector: (state) => state.get24hChangeForAsset(widget.coin.id),
//   builder: (context, change24hPercent) { ... }
// )

Please also verify the mobile row with the current Spacer + two Expanded on very small devices (potential overflow).


149-166: Balance null default to 0 shows “0 TICKER”; consider placeholder

If balance is unavailable, showing 0 may mislead. Prefer '—' or a skeleton until loaded.

Example:

final bal = widget.coin.balance(context.sdk);
final balanceText = bal == null ? '—' : '${doubleToString(bal)} ${widget.coin.abbr}';
// AutoScrollText(text: balanceText, ...)

311-319: Add tooltip for copy icon (accessibility)

Provide a tooltip for screen readers and hover targets.

-              child: IconButton(
+              child: IconButton(
                 iconSize: 16,
                 icon: const Icon(Icons.copy),
                 onPressed: onCopy,
+                tooltip: LocaleKeys.copy.tr(),
                 visualDensity: VisualDensity.compact,
               ),
lib/bloc/transaction_history/transaction_history_bloc.dart (8)

32-33: Remove unused _processedTxIds; it adds noise and minor memory overhead.

The set is written to but never read. It’s safe to delete the field and its usages.

Apply this diff:

-  // TODO: Remove or move to SDK
-  final Set<String> _processedTxIds = {};
@@
-      _processedTxIds.clear();
@@
-                  _processedTxIds.add(sanitized.internalId);
@@
-            _processedTxIds.add(sanitized.internalId);

Also applies to: 67-67, 105-106, 198-198


34-37: Only record firstSeenAt for zero‑timestamp tx and prune periodically to bound memory.

Currently you store entries even for tx with real timestamps and never prune; the map can grow unbounded during long sessions. Store only for zero‑timestamp tx and prune keys once a tx gains a real timestamp.

Apply these changes:

@@
-  // Stable in-memory clock for transactions that arrive with a zero timestamp.
-  // Ensures deterministic ordering of unconfirmed and just-confirmed items.
+  // Stable in-memory clock for transactions that arrive with a zero timestamp.
+  // Ensures deterministic ordering of unconfirmed and just-confirmed items.
   final Map<String, DateTime> _firstSeenAtById = {};
@@
-                _firstSeenAtById.putIfAbsent(
-                  sanitized.internalId,
-                  () => sanitized.timestamp.millisecondsSinceEpoch != 0
-                      ? sanitized.timestamp
-                      : DateTime.now(),
-                );
+                if (sanitized.timestamp.millisecondsSinceEpoch == 0) {
+                  _firstSeenAtById.putIfAbsent(
+                    sanitized.internalId,
+                    () => DateTime.now(),
+                  );
+                }
@@
-              final updatedTransactions = byId.values.toList()
-                ..sort(_compareTransactions);
+              final updatedTransactions = byId.values.toList()
+                ..sort(_compareTransactions);
+              // Prune stored first-seen markers once tx have a real timestamp or were removed
+              _firstSeenAtById.removeWhere((id, _) {
+                final t = byId[id];
+                return t == null || t.timestamp.millisecondsSinceEpoch != 0;
+              });
@@
-            _firstSeenAtById.putIfAbsent(
-              sanitized.internalId,
-              () => sanitized.timestamp.millisecondsSinceEpoch != 0
-                  ? sanitized.timestamp
-                  : DateTime.now(),
-            );
+            if (sanitized.timestamp.millisecondsSinceEpoch == 0) {
+              _firstSeenAtById.putIfAbsent(
+                sanitized.internalId,
+                () => DateTime.now(),
+              );
+            }
@@
-            final updatedTransactions = byId.values.toList()
-              ..sort(_compareTransactions);
+            final updatedTransactions = byId.values.toList()
+              ..sort(_compareTransactions);
+            _firstSeenAtById.removeWhere((id, _) {
+              final t = byId[id];
+              return t == null || t.timestamp.millisecondsSinceEpoch != 0;
+            });

Also applies to: 96-101, 174-179, 118-121, 200-202


55-57: Localize this user‑facing error string.

Replace the hard‑coded English string with a localized key (e.g., LocaleKeys.txHistoryUnsupportedCoin.tr()), or add a new key to codegen_loader.g.dart.

Please confirm the appropriate LocaleKeys entry, and I can provide an exact diff.


110-115: Promote timestamp when it becomes known to keep ordering correct after confirmation.

When a tx transitions from unconfirmed (timestamp often 0) to confirmed (real timestamp), update timestamp in the merged entry. Otherwise the item may continue to sort using the synthetic first‑seen time.

Proposed change (adjust field name if needed in copyWith):

-                byId[sanitized.internalId] = existing.copyWith(
+                byId[sanitized.internalId] = existing.copyWith(
                   confirmations: sanitized.confirmations,
                   blockHeight: sanitized.blockHeight,
                   fee: sanitized.fee ?? existing.fee,
                   memo: sanitized.memo ?? existing.memo,
+                  timestamp: sanitized.timestamp.millisecondsSinceEpoch != 0
+                      ? sanitized.timestamp
+                      : existing.timestamp,
                 );

Repeat the same change in the single‑tx watcher merge.

Also applies to: 191-196


127-133: Log stream errors for diagnosis; keep user‑facing text localized.

Add log(...) in both onError handlers (history and watcher) to capture errors and stack traces while returning a localized message to the UI.

-            onError: (error) {
+            onError: (error, StackTrace stackTrace) {
+              log(
+                'Transactions history stream error: $error',
+                isError: true,
+                path: 'transaction_history_bloc->_onSubscribe.history',
+                trace: stackTrace,
+              );
               add(
                 TransactionHistoryFailure(
                   error: TextError(error: LocaleKeys.somethingWrong.tr()),
                 ),
               );
             },
@@
-          onError: (error) {
+          onError: (error, StackTrace stackTrace) {
+            log(
+              'Transactions watch stream error: $error',
+              isError: true,
+              path: 'transaction_history_bloc->_subscribeToNewTransactions.watch',
+              trace: stackTrace,
+            );
             String errorMessage;
             if (error is ActivationFailedException) {
               errorMessage = 'Asset activation failed: ${error.message}';
             } else {
               errorMessage = LocaleKeys.somethingWrong.tr();
             }

Also applies to: 209-220


152-157: Make specialized activation error messages localizable.

The messages beginning with “Asset activation failed” are plain strings; wrap them with a localization key to be consistent with the rest of the app.

Confirm the intended LocaleKeys mapping (e.g., LocaleKeys.assetActivationFailed) and I’ll provide an exact diff.

Also applies to: 210-215


277-279: Avoid floating‑point conversion for zero‑amount filter.

Comparing totalAmount.toDouble() == 0.0 can be unsafe (precision, large values). Prefer domain‑type checks:

  • If Decimal: totalAmount == Decimal.zero or totalAmount.compareTo(Decimal.zero) == 0.
  • If BigInt: totalAmount == BigInt.zero.
  • If custom Amount: use .isZero.

What is the exact type of tx.balanceChanges.totalAmount? I can provide a precise diff once confirmed.


282-374: Move pagination types out of the BLoC file; keep concerns separated.

Place Pagination and TransactionPagination variants in a pagination.dart (or types) module and import here. Also ensure Dart SDK constraints support sealed.

If the repo targets Dart < 3, replace sealed or gate with proper SDK constraints.

assets/translations/en.json (1)

213-214: Align adjacent strings to the new “Use custom seed phrase / private key (WIF)” wording

Line 214 still mentions “Allow custom seed”, which conflicts with Line 213 and key useCustomSeedOrWif (Line 536). Unify to avoid user confusion.

-  "walletCreationHdBip39SeedError": "Your input seed phrase is not BIP39 compliant, and can not be used in multi-address wallet mode. Please try again, or disable multi-address wallet mode and select 'Allow custom seed' to proceed.",
+  "walletCreationHdBip39SeedError": "Your input seed phrase is not BIP39 compliant and cannot be used in multi-address wallet mode. Please try again, or disable multi-address wallet mode and select 'Use custom seed phrase / private key (WIF)' to proceed.",
lib/bloc/coins_bloc/asset_coin_extension.dart (1)

264-268: Use a Set for membership lookups; DRY retrieval of activated IDs

Converting to a Set avoids O(n) contains() per coin and makes both methods clearer. Optionally factor a small helper to fetch once.

-  Future<List<Coin>> removeInactiveCoins(KomodoDefiSdk sdk) async {
-    final activeIds = await sdk.activatedAssetsCache.getActivatedAssetIds();
-    return where((coin) => activeIds.contains(coin.id)).unmodifiable().toList();
-  }
+  Future<List<Coin>> removeInactiveCoins(KomodoDefiSdk sdk) async {
+    final activeIds = Set.of(await sdk.activatedAssetsCache.getActivatedAssetIds());
+    return where((coin) => activeIds.contains(coin.id)).unmodifiable().toList();
+  }

-  Future<List<Coin>> removeActiveCoins(KomodoDefiSdk sdk) async {
-    final activeIds = await sdk.activatedAssetsCache.getActivatedAssetIds();
-    return where(
-      (coin) => !activeIds.contains(coin.id),
-    ).unmodifiable().toList();
-  }
+  Future<List<Coin>> removeActiveCoins(KomodoDefiSdk sdk) async {
+    final activeIds = Set.of(await sdk.activatedAssetsCache.getActivatedAssetIds());
+    return where((coin) => !activeIds.contains(coin.id)).unmodifiable().toList();
+  }

Also applies to: 270-276

lib/bloc/taker_form/taker_validator.dart (2)

73-101: Preimage error mapping improvements LGTM

Clearer user messages with proper coin/amount context. Consider localizing the generic fallback string for consistency.


309-318: Minor UX: localize the catch-all preimage error

Return a localized string (e.g., LocaleKeys.dexErrorMessage or a dedicated key) instead of a hardcoded English message.

-      return DataFromService(
-        error: TextError(error: 'Failed to request trade preimage'),
-      );
+      return DataFromService(
+        error: TextError(error: LocaleKeys.dexErrorMessage.tr()),
+      );
lib/views/wallets_manager/widgets/wallet_import_by_file.dart (2)

151-158: Comment vs. behavior: toggle is not hidden on HD switch.

The comment says “hide toggle on HD switch” but only resets _allowCustomSeed. Either hide it as well or update the comment to reflect behavior.

-                        // Reset custom seed usage and hide toggle on HD switch
-                        if (_isHdMode) {
-                          _allowCustomSeed = false;
-                        }
+                        // Reset custom seed usage and hide toggle on HD switch
+                        if (_isHdMode) {
+                          _allowCustomSeed = false;
+                          _showCustomSeedToggle = false; // keep UI consistent
+                        }

321-325: Nit: consider collapsing to a single expression.

Readable as-is; optionally compress to a single return for clarity.

-bool get _shouldShowCustomSeedToggle {
-  if (_allowCustomSeed) return true; // keep visible once enabled
-  if (_showCustomSeedToggle) return true; // show after first failure, even in HD
-  return false;
-}
+bool get _shouldShowCustomSeedToggle =>
+    _allowCustomSeed || _showCustomSeedToggle;
lib/views/dex/simple/form/amount_input_field.dart (1)

94-105: Add error handling for parseLocaleAwareRational.

parseLocaleAwareRational can throw a FormatException for invalid input (e.g., empty string, non-numeric characters). While currencyInputFormatters should prevent most invalid input, defensive coding suggests wrapping the parse call in a try-catch to avoid potential crashes if invalid text reaches _onDataChange.

Apply this diff to add error handling:

 void _onDataChange(Rational? value) {
   if (!mounted) return;
   final String currentText = _controller.text;
-  if (currentText.isNotEmpty && parseLocaleAwareRational(currentText) == value) return;
+  if (currentText.isNotEmpty) {
+    try {
+      if (parseLocaleAwareRational(currentText) == value) return;
+    } catch (_) {
+      // Parse failed, continue to update the field
+    }
+  }

   final String newText = value == null ? '' : formatDexAmt(value);

   _controller.value = TextEditingValue(
       text: newText,
       selection: TextSelection.collapsed(offset: newText.length),
       composing: TextRange.empty);
 }
lib/views/wallets_manager/widgets/wallet_simple_import.dart (3)

151-156: Remove the listener in dispose for safety

Explicitly remove the _seedController listener to avoid any chance of setState after dispose and for symmetry.

Apply:

   @override
   void initState() {
     super.initState();
     _seedController.addListener(_onSeedChanged);
   }

   @override
   void dispose() {
+    _seedController.removeListener(_onSeedChanged);
     _nameController.dispose();
     _seedController.dispose();
     _passwordController.dispose();
     super.dispose();
   }

157-160: Guard setState with mounted

Tiny safety net to prevent accidental setState after dispose.

 void _onSeedChanged() {
-  // Rebuild to update custom seed toggle visibility as user types
-  setState(() {});
+  if (!mounted) return;
+  setState(() {}); // Rebuild to update custom seed toggle visibility as user types
 }

436-446: Avoid heavy validation in build; compute once and narrow rebuilds

_validateBip39 is called on every build/keystroke. Consider caching the boolean in _onSeedChanged and wrapping just the toggle in a ValueListenableBuilder/AnimatedBuilder to limit rebuild scope.

If you want to keep current structure, at least short‑circuit empty/HD early (you already do) and consider debouncing validateBip39 if it’s non‑trivial.

lib/mm2/mm2_api/mm2_api.dart (1)

73-76: Make dispose synchronous (no awaits here)

dispose() is async but doesn’t await anything. Prefer a sync signature for clarity.

-Future<void> dispose() async {
-  nft.dispose();
-}
+void dispose() {
+  nft.dispose();
+}
lib/bloc/coins_manager/coins_manager_bloc.dart (2)

244-254: Localize the error message and consider a constant

Use a localized string instead of a hardcoded 'Activation canceled.' to keep UX consistent.

Example:

errorMessage: LocaleKeys.activationCanceled.tr(), // or a shared UiKit string

As per coding guidelines.


275-284: Decide whether to revert selection on general activation failures

Currently only cancellation reverts. For other failures, you log a warning and return, leaving the optimistic selection intact. If activation actually failed, this may confuse users. Consider reverting selection and emitting an error message for non‑cancellation failures too.

Would you like a patch that mirrors the cancellation branch for generic failures?

lib/mm2/mm2_api/mm2_api_nft.dart (2)

176-192: Avoid firstWhere throws; handle unknown chains gracefully

firstWhere() will throw if a chain’s asset isn’t in knownAssets. Use a safe lookup to skip unknowns and log once.

-    final inactiveChains = chains
-        .where(
-          (chain) => !activeAssets.any(
-            (asset) => asset.id.id == chain.nftAssetTicker(),
-          ),
-        )
-        .map(
-          (chain) => knownAssets.values.firstWhere(
-            (asset) => asset.id.id == chain.nftAssetTicker(),
-          ),
-        )
-        .toList();
+    final inactiveChains = <Asset>[];
+    for (final chain in chains) {
+      final ticker = chain.nftAssetTicker();
+      final isActive = activeAssets.any((a) => a.id.id == ticker);
+      if (isActive) continue;
+      final match = knownAssets.values.where((a) => a.id.id == ticker);
+      if (match.isEmpty) {
+        _log.warning('Unknown NFT chain ticker: $ticker; skipping enable.');
+        continue;
+      }
+      inactiveChains.add(match.first);
+    }

219-221: Optional: reuse a single HTTP Client

Creating a new Client per call can add overhead. Consider a shared Client with proper disposal.

Also applies to: 254-259

lib/bloc/bridge_form/bridge_validator.dart (2)

92-95: Avoid double.parse; use Rational for precision and consistency.

Parsing thresholds with double sacrifices precision. Use Rational/Decimal and the existing formatting helpers.

-        error: LocaleKeys.lowTradeVolumeError.tr(
-          args: [formatAmt(double.parse(error.threshold)), error.coin],
-        ),
+        error: LocaleKeys.lowTradeVolumeError.tr(
+          args: [formatDexAmt(Rational.parse(error.threshold)), error.coin],
+        ),

243-254: Activation check can short-circuit per asset.

Fetching the full activated set each time adds overhead. Prefer the provided per-asset check where available.

-    final activatedAssetIds = await _coinsRepo.getActivatedAssetIds();
+    // Micro-optimizations and clearer intent:
+    // final activated = await _coinsRepo.isAssetActivated(coin.id);
+    // final parentActive = parentId == null || await _coinsRepo.isAssetActivated(parentId);

If sticking to the set approach, keep it—it’s fine; the above just reduces set allocations on hot paths.

lib/views/fiat/fiat_form.dart (2)

253-275: Avoid showing an extra dialog after the WebView closes.

After awaiting WebViewDialog.show(...) for the submitted state, the code continues and triggers _showPaymentStatusDialog which shows a “submitted” dialog. Return early to avoid duplicate UX.

-      await WebViewDialog.show(
+      await WebViewDialog.show(
         context,
         url: stateSnapshot.checkoutUrl,
         mode: stateSnapshot.webViewMode,
         title: LocaleKeys.buy.tr(),
         onMessage: _onConsoleMessage,
         onCloseWindow: _onCloseWebView,
         settings: FiatProviderWebViewSettings.createSecureProviderSettings(),
       );
+      return;

331-343: Minor i18n consistency: format both min and max uniformly.

Maximum uses toString(); minimum uses toStringAsFixed(2). Consider consistent formatting (e.g., both with 2 decimals or via a formatter) to avoid mismatched precision in UI.

lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart (2)

239-246: Reuse state.isBanxaSelected for consistency.

You introduced isBanxaSelected on state; apply it here too for clarity.

-    if (isLinux || (isWeb && state.isBanxaSelected)) {
+    if (isLinux || (isWeb && state.isBanxaSelected)) {
       return WebViewDialogMode.newTab;
     } else if (isWeb) {
       return WebViewDialogMode.dialog;
     } else {
       return WebViewDialogMode.fullscreen;
     }

(Already correct; just calling out consistency. If other branches use literal 'Banxa', align them.)


357-361: Use the new isBanxaSelected helper.

Avoid providerId string literals.

-    if (state.selectedPaymentMethod.providerId != 'Banxa') {
+    if (!state.isBanxaSelected) {
       return;
     }
lib/blocs/maker_form_bloc.dart (2)

253-256: Prefer a named constant for the refresh interval

Replace the magic number 10s with a shared constant (e.g., kMaxSellAmountRefreshInterval) to keep polling unified.

-    _maxSellAmountTimer = Timer.periodic(const Duration(seconds: 10), (
+    _maxSellAmountTimer = Timer.periodic(kMaxSellAmountRefreshInterval, (
       _,
     ) async {
       await _updateMaxSellAmount();
     });

525-535: Defensive checks before placing order

makeOrder() dereferences sellCoin, buyCoin, sellAmount, price with !. Add guards/asserts to avoid crashes if called pre‑validation.

   Future<TextError?> makeOrder() async {
+    if (sellCoin == null || buyCoin == null || sellAmount == null || price == null) {
+      return TextError(error: LocaleKeys.somethingWrong.tr());
+    }
     final Map<String, dynamic>? response = await api.setprice(
       SetPriceRequest(
         base: sellCoin!.abbr,
         rel: buyCoin!.abbr,
         volume: sellAmount!,
         price: price!,
         max: isMaxActive,
       ),
     );
lib/shared/widgets/asset_amount_with_fiat.dart (2)

45-55: Validate type of priceIfKnown(assetId) to avoid operator mismatch

Ensure priceIfKnown returns Decimal. If it returns num/double, this multiply will fail at compile time.

If it’s not Decimal, coerce to doubles for display:

-    final formattedFiat = price != null
-        ? ' (${formatUsdValue((price * amount).toDouble())})'
+    final formattedFiat = price != null
+        ? ' (${formatUsdValue((price.toDouble() * amount.toDouble()))})'
         : '';

47-55: Optional: use a crypto amount formatter

amount.toString() may be noisy (scale/zeros). Consider a formatter (e.g., formatCryptoAmount/normalizeDecimalString) for user‑friendly output.

lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart (1)

457-466: Minor: label style consistency

Use theme.textTheme.labelLarge for labels in _buildRow to match column layout styling.

-        Expanded(flex: 2, child: Text(label)),
+        Expanded(
+          flex: 2,
+          child: Text(label, style: Theme.of(context).textTheme.labelLarge),
+        ),
lib/bloc/coins_bloc/coins_bloc.dart (1)

423-423: Minor: Unused stopwatch.

The stopwatch is started and stopped but the elapsed time is never logged or used. Consider either logging the elapsed time for debugging purposes or removing the stopwatch entirely.

Apply this diff to log the elapsed time:

       stopwatch.stop();
+      _log.fine('Initial balance refresh scheduling completed in ${stopwatch.elapsedMilliseconds}ms');

Or remove the unused stopwatch:

-      final stopwatch = Stopwatch()..start();
       var triggeredByThreshold = false;
       ...
-      stopwatch.stop();

Also applies to: 499-499

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26d1889 and ebb3f74.

⛔ Files ignored due to path filters (1)
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (63)
  • assets/translations/en.json (1 hunks)
  • assets/web_pages/fiat_widget.html (5 hunks)
  • ios/Runner.xcodeproj/project.pbxproj (3 hunks)
  • lib/app_config/app_config.dart (1 hunks)
  • lib/bloc/app_bloc_root.dart (0 hunks)
  • lib/bloc/assets_overview/bloc/asset_overview_bloc.dart (2 hunks)
  • lib/bloc/bridge_form/bridge_validator.dart (10 hunks)
  • lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart (2 hunks)
  • lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart (2 hunks)
  • lib/bloc/cex_market_data/sdk_auth_activation_extension.dart (3 hunks)
  • lib/bloc/coin_addresses/bloc/coin_addresses_bloc.dart (1 hunks)
  • lib/bloc/coins_bloc/asset_coin_extension.dart (1 hunks)
  • lib/bloc/coins_bloc/coins_bloc.dart (9 hunks)
  • lib/bloc/coins_bloc/coins_event.dart (1 hunks)
  • lib/bloc/coins_bloc/coins_repo.dart (18 hunks)
  • lib/bloc/coins_manager/coins_manager_bloc.dart (2 hunks)
  • lib/bloc/custom_token_import/bloc/custom_token_import_bloc.dart (1 hunks)
  • lib/bloc/custom_token_import/data/custom_token_import_repository.dart (2 hunks)
  • lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart (8 hunks)
  • lib/bloc/fiat/fiat_onramp_form/fiat_form_state.dart (1 hunks)
  • lib/bloc/fiat/fiat_order_status.dart (3 hunks)
  • lib/bloc/nft_receive/bloc/nft_receive_bloc.dart (2 hunks)
  • lib/bloc/taker_form/taker_bloc.dart (6 hunks)
  • lib/bloc/taker_form/taker_validator.dart (9 hunks)
  • lib/bloc/transaction_history/transaction_history_bloc.dart (6 hunks)
  • lib/bloc/withdraw_form/withdraw_form_bloc.dart (3 hunks)
  • lib/bloc/withdraw_form/withdraw_form_state.dart (3 hunks)
  • lib/blocs/maker_form_bloc.dart (11 hunks)
  • lib/blocs/trading_entities_bloc.dart (1 hunks)
  • lib/main.dart (2 hunks)
  • lib/mm2/mm2_api/mm2_api.dart (2 hunks)
  • lib/mm2/mm2_api/mm2_api_nft.dart (8 hunks)
  • lib/mm2/rpc_native.dart (1 hunks)
  • lib/model/coin.dart (0 hunks)
  • lib/model/forms/fiat/fiat_amount_input.dart (2 hunks)
  • lib/sdk/widgets/window_close_handler.dart (5 hunks)
  • lib/services/initializer/app_bootstrapper.dart (2 hunks)
  • lib/shared/constants.dart (1 hunks)
  • lib/shared/utils/formatters.dart (1 hunks)
  • lib/shared/widgets/asset_amount_with_fiat.dart (1 hunks)
  • lib/views/common/pages/page_layout.dart (2 hunks)
  • lib/views/dex/simple/confirm/taker_order_confirmation.dart (1 hunks)
  • lib/views/dex/simple/form/amount_input_field.dart (1 hunks)
  • lib/views/dex/simple/form/maker/maker_form_layout.dart (6 hunks)
  • lib/views/dex/simple/form/taker/taker_form_layout.dart (3 hunks)
  • lib/views/fiat/custom_fiat_input_field.dart (5 hunks)
  • lib/views/fiat/fiat_asset_icon.dart (3 hunks)
  • lib/views/fiat/fiat_form.dart (7 hunks)
  • lib/views/fiat/fiat_inputs.dart (5 hunks)
  • lib/views/fiat/fiat_page.dart (1 hunks)
  • lib/views/fiat/fiat_provider_web_view_settings.dart (3 hunks)
  • lib/views/fiat/fiat_select_button.dart (2 hunks)
  • lib/views/market_maker_bot/coin_trade_amount_form_field.dart (1 hunks)
  • lib/views/settings/widgets/security_settings/unban_pubkeys_plate.dart (2 hunks)
  • lib/views/wallet/coin_details/coin_details.dart (1 hunks)
  • lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart (3 hunks)
  • lib/views/wallet/coin_details/transactions/transaction_table.dart (1 hunks)
  • lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart (2 hunks)
  • lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart (1 hunks)
  • lib/views/wallets_manager/widgets/wallet_import_by_file.dart (5 hunks)
  • lib/views/wallets_manager/widgets/wallet_simple_import.dart (3 hunks)
  • sdk (1 hunks)
  • test_units/shared/utils/formatters_test.dart (1 hunks)
💤 Files with no reviewable changes (2)
  • lib/model/coin.dart
  • lib/bloc/app_bloc_root.dart
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart

📄 CodeRabbit inference engine (AGENTS.md)

**/*.dart: Run flutter analyze and resolve issues before committing code
Format Dart code with dart format (on changed files) before committing

Files:

  • lib/views/wallet/coin_details/transactions/transaction_table.dart
  • lib/blocs/trading_entities_bloc.dart
  • lib/bloc/custom_token_import/bloc/custom_token_import_bloc.dart
  • lib/bloc/coins_bloc/coins_event.dart
  • lib/views/fiat/fiat_page.dart
  • lib/views/dex/simple/form/maker/maker_form_layout.dart
  • lib/model/forms/fiat/fiat_amount_input.dart
  • test_units/shared/utils/formatters_test.dart
  • lib/views/dex/simple/form/amount_input_field.dart
  • lib/shared/constants.dart
  • lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart
  • lib/shared/utils/formatters.dart
  • lib/bloc/withdraw_form/withdraw_form_bloc.dart
  • lib/views/market_maker_bot/coin_trade_amount_form_field.dart
  • lib/views/fiat/fiat_asset_icon.dart
  • lib/bloc/cex_market_data/sdk_auth_activation_extension.dart
  • lib/bloc/fiat/fiat_onramp_form/fiat_form_state.dart
  • lib/bloc/nft_receive/bloc/nft_receive_bloc.dart
  • lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart
  • lib/bloc/fiat/fiat_order_status.dart
  • lib/sdk/widgets/window_close_handler.dart
  • lib/services/initializer/app_bootstrapper.dart
  • lib/views/settings/widgets/security_settings/unban_pubkeys_plate.dart
  • lib/bloc/custom_token_import/data/custom_token_import_repository.dart
  • lib/bloc/assets_overview/bloc/asset_overview_bloc.dart
  • lib/views/fiat/custom_fiat_input_field.dart
  • lib/views/wallet/coin_details/coin_details.dart
  • lib/views/wallets_manager/widgets/wallet_simple_import.dart
  • lib/bloc/withdraw_form/withdraw_form_state.dart
  • lib/mm2/mm2_api/mm2_api.dart
  • lib/bloc/transaction_history/transaction_history_bloc.dart
  • lib/bloc/coins_manager/coins_manager_bloc.dart
  • lib/views/fiat/fiat_select_button.dart
  • lib/views/wallets_manager/widgets/wallet_import_by_file.dart
  • lib/main.dart
  • lib/views/fiat/fiat_provider_web_view_settings.dart
  • lib/bloc/coins_bloc/coins_bloc.dart
  • lib/blocs/maker_form_bloc.dart
  • lib/app_config/app_config.dart
  • lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart
  • lib/bloc/taker_form/taker_validator.dart
  • lib/bloc/taker_form/taker_bloc.dart
  • lib/views/dex/simple/confirm/taker_order_confirmation.dart
  • lib/mm2/mm2_api/mm2_api_nft.dart
  • lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
  • lib/mm2/rpc_native.dart
  • lib/bloc/coin_addresses/bloc/coin_addresses_bloc.dart
  • lib/views/wallet/wallet_page/common/expandable_coin_list_item.dart
  • lib/bloc/coins_bloc/coins_repo.dart
  • lib/views/common/pages/page_layout.dart
  • lib/views/fiat/fiat_form.dart
  • lib/bloc/coins_bloc/asset_coin_extension.dart
  • lib/shared/widgets/asset_amount_with_fiat.dart
  • lib/views/fiat/fiat_inputs.dart
  • lib/views/dex/simple/form/taker/taker_form_layout.dart
  • lib/bloc/bridge_form/bridge_validator.dart
  • lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart
🧠 Learnings (3)
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.

Applied to files:

  • lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart
  • lib/views/fiat/fiat_asset_icon.dart
  • lib/views/settings/widgets/security_settings/unban_pubkeys_plate.dart
  • lib/bloc/assets_overview/bloc/asset_overview_bloc.dart
  • lib/bloc/transaction_history/transaction_history_bloc.dart
  • lib/bloc/coins_bloc/coins_bloc.dart
  • lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart
  • lib/bloc/taker_form/taker_bloc.dart
  • lib/views/wallet/coin_details/coin_details_info/coin_addresses.dart
  • lib/bloc/coins_bloc/coins_repo.dart
  • lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The `excludedAssetList` from `app_config.dart` is used in the `_filterExcludedAssets` method in `coins_state.dart`. Since `coins_state.dart` is a part file of `coins_bloc.dart`, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.

Applied to files:

  • lib/views/fiat/fiat_asset_icon.dart
  • lib/bloc/assets_overview/bloc/asset_overview_bloc.dart
  • lib/bloc/coins_bloc/coins_bloc.dart
  • lib/bloc/taker_form/taker_bloc.dart
  • lib/bloc/coins_bloc/coins_repo.dart
  • lib/bloc/coins_bloc/asset_coin_extension.dart
  • lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart
📚 Learning: 2025-09-12T00:25:47.005Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#3129
File: lib/bloc/custom_token_import/data/custom_token_import_repository.dart:205-221
Timestamp: 2025-09-12T00:25:47.005Z
Learning: In the KdfCustomTokenImportRepository class in lib/bloc/custom_token_import/data/custom_token_import_repository.dart, the copyWithProtocolData extension method intentionally drops additional protocol fields when creating custom tokens - this is by design to ensure clean configuration without unexpected carryover from platform assets.

Applied to files:

  • lib/bloc/custom_token_import/data/custom_token_import_repository.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: validate_code_guidelines
  • GitHub Check: build-linux-docker
  • GitHub Check: unit_tests
  • GitHub Check: Test web-app-linux-profile
  • GitHub Check: Test web-app-macos
  • GitHub Check: Build Mobile (iOS)
  • GitHub Check: Build Desktop (macos)
  • GitHub Check: Build Mobile (Android)
  • GitHub Check: build-android-docker
  • GitHub Check: build_and_preview
  • GitHub Check: Build Desktop (linux)
  • GitHub Check: Build Desktop (windows)

Comment on lines 30 to 32
sandbox="allow-forms allow-scripts allow-same-origin allow-popups allow-top-navigation allow-top-navigation-by-user-action"
src="">
allow="payment; encrypted-media; microphone; camera; midi" src="">
<!-- Placeholder fallback message -->
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.

🛠️ Refactor suggestion | 🟠 Major

Remove allow-top-navigation; enforce iframe URL allowlist to prevent escape and phishing

  • The iframe’s sandbox currently includes allow-top-navigation, which enables top-level navigation from the provider; this contradicts the Dart settings comment and weakens isolation.
  • targetUrl is accepted from a base64 URL param without scheme/host validation. Add a strict allowlist to mitigate loading untrusted origins with camera/mic permissions.
-  <iframe id="fiat-onramp-iframe" title="Fiat On-Ramp Widget"
-    sandbox="allow-forms allow-scripts allow-same-origin allow-popups allow-top-navigation allow-top-navigation-by-user-action"
-    allow="payment; encrypted-media; microphone; camera; midi" src="">
+  <iframe id="fiat-onramp-iframe" title="Fiat On-Ramp Widget"
+    sandbox="allow-forms allow-scripts allow-same-origin allow-popups allow-top-navigation-by-user-activation"
+    allow="payment; encrypted-media; microphone; camera" src="">

Add strict URL validation before assigning iframe src:

 function _komodoSetIframeUrlFromParams() {
   const urlParam = _komodoGetUrlParameter('fiatUrl');
 
   let targetUrl = null;
   if (urlParam) {
     try {
       targetUrl = atob(urlParam); // base64 decode the `url` parameter
     } catch (error) {
       console.error('Error decoding base64 url parameter', error);
     }
   }
 
-  if (targetUrl) {
-    document.getElementById('fiat-onramp-iframe').src = targetUrl;
+  if (targetUrl) {
+    try {
+      const urlObj = new URL(targetUrl);
+      const allowedHosts = [
+        'app.ramp.network',
+        'app.demo.ramp.network',
+        'komodo.banxa.com',
+        'komodo.banxa-sandbox.com',
+        'embed.bitrefill.com',
+        'app.komodoplatform.com',
+      ];
+      const isHttp = urlObj.protocol === 'https:' || urlObj.protocol === 'http:';
+      const hostOk = allowedHosts.some(d => urlObj.hostname === d || urlObj.hostname.endsWith('.' + d));
+      if (!isHttp || !hostOk) {
+        throw new Error('Blocked untrusted iframe origin: ' + urlObj.hostname);
+      }
+      document.getElementById('fiat-onramp-iframe').src = urlObj.toString();
+    } catch (e) {
+      console.error('Invalid or disallowed targetUrl', e);
+    }
   } else {
     console.error('No URL parameter provided');
   }
 }

Optional hardening: restrict postMessage target origins instead of "*", if you can determine the parent’s origin.

Also applies to: 49-66

);
MACOSX_DEPLOYMENT_TARGET = 15.0;
PRODUCT_BUNDLE_IDENTIFIER = com.komodoplatform.atomicdex;
PRODUCT_BUNDLE_IDENTIFIER = com.komodo.wallet;
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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify bundle identifier change impact and readiness.

Changing the iOS bundle identifier from com.komodoplatform.atomicdex to com.komodo.wallet creates a new app identity in Apple's ecosystem. This means existing users cannot update the app—they must install it as a new application, potentially losing data if migration is not handled.

Ensure the following are addressed:

  1. User data migration: Implement a strategy to migrate user data from the old app to the new one (e.g., using App Groups or iCloud storage).
  2. Apple Developer Portal updates: Update provisioning profiles, certificates, App Store Connect app record, push notifications, Sign in with Apple, associated domains, and any in-app purchases.
  3. Deep links and universal links: Update all configurations to use the new bundle identifier.
  4. Android consistency: Verify if a corresponding package name change was made for Android.
  5. Hardcoded references: Check for any hardcoded references to the old bundle identifier throughout the codebase.

Run the following script to check for hardcoded references to the old bundle identifier:

Also verify the Android package name for consistency:

Also applies to: 552-552, 587-587


🏁 Script executed:

#!/bin/bash
# Search for hardcoded references to the old bundle identifier

echo "Searching for old bundle identifier references..."
rg -n "com\.komodoplatform\.atomicdex" --type-add 'config:*.{plist,entitlements,xcconfig}' --type config

echo -e "\nSearching in Dart/Flutter files..."
rg -n "com\.komodoplatform\.atomicdex" --type dart

echo -e "\nSearching in Swift/Objective-C files..."
rg -n "com\.komodoplatform\.atomicdex" --type swift --type objc

echo -e "\nSearching in configuration files..."
rg -n "com\.komodoplatform\.atomicdex" --glob '*.json' --glob '*.yaml' --glob '*.yml'

Length of output: 280


🏁 Script executed:

#!/bin/bash
# Check Android package name in build.gradle

echo "Checking Android package name..."
fd -e gradle -x grep -Hn "applicationId"

Length of output: 115


🏁 Script executed:

#!/bin/bash
# Check for any references to the new iOS bundle ID in Android or Web files
echo "Checking if new iOS bundle ID appears in Android/Web configs..."
rg -n "com\.komodo\.wallet" --glob '*.gradle' --glob '*.json' --glob '*.yaml'

echo -e "\nLooking for package name patterns in Android files..."
rg -n "package" ./android/app/build.gradle -A2 -B2

echo -e "\nChecking Web manifest structure..."
cat ./web/manifest.json 2>/dev/null | head -20

Length of output: 221


🏁 Script executed:

#!/bin/bash
# Find and inspect Android and Web configuration files
echo "=== Finding Android build.gradle ==="
fd build.gradle -x cat {}

echo -e "\n=== Checking web/manifest.json ==="
fd manifest.json -x cat {}

echo -e "\n=== Checking if new iOS bundle ID (com.komodo.wallet) appears anywhere in non-iOS configs ==="
rg -n "com\.komodo\.wallet"

Length of output: 5338


Update Android and web configurations to match iOS bundle identifier change.

The iOS bundle identifier has been successfully changed to com.komodo.wallet across all configurations and code references. However, the Android and web configurations were not updated, creating a platform identity mismatch:

Required fixes:

  1. Android (android/app/build.gradle):

    • Line 42: Update namespace 'com.komodoplatform.atomicdex'namespace 'com.komodo.wallet'
    • Line 64: Update applicationId "com.komodoplatform.atomicdex"applicationId "com.komodo.wallet"
  2. Web (web/manifest.json line 15):

    • Update Play Store URL from com.komodoplatform.atomicdex to com.komodo.wallet

Additional concern: This is a breaking app identity change across platforms. Ensure a user data migration strategy exists (e.g., via App Groups, iCloud, or Firebase), and verify all infrastructure updates are ready (provisioning profiles, App Store Connect, certificates, Sign in with Apple, push notifications).

Also applies to: 552-552, 587-587

Comment on lines +131 to 140
log(
e.toString(),
trace: s,
path: 'bridge_validator::_getPreimageData',
isError: true,
);
return DataFromService(
error: TextError(error: 'Failed to request trade preimage'));
error: TextError(error: 'Failed to request trade preimage'),
);
}
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.

⚠️ Potential issue | 🟡 Minor

Localize and preserve context in preimage fetch failure.

Error text is hard-coded and loses localization. Also include a concise reason for better UX.

-      return DataFromService(
-        error: TextError(error: 'Failed to request trade preimage'),
-      );
+      return DataFromService(
+        error: TextError(
+          error: LocaleKeys.somethingWrong.tr(), // generic localized fallback
+        ),
+      );

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In lib/bloc/bridge_form/bridge_validator.dart around lines 131–140, replace the
hard-coded error TextError('Failed to request trade preimage') with a localized
message pulled from the app's localization resources and include a concise
reason derived from the caught exception (e.g., a short e.message or first 100
chars of e.toString()) so users see context; keep the existing log call
unchanged but ensure the DataFromService.error uses something like
Localizations.instance['failed_fetch_preimage'] (or your project's
S/AppLocalizations helper) and append the concise reason (safely truncated to
avoid leaking sensitive data).

Comment on lines +267 to 269
final cached = _sdk.pubkeys.lastKnown(asset.id);
final ownPubkeys = cached ?? await _sdk.pubkeys.getPubkeys(asset);
final ownAddresses = ownPubkeys.keys
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.

⚠️ Potential issue | 🟠 Major

Harden trading-with-self check against pubkeys fetch failures.

A transient pubkeys error will currently abort validation. Catch and degrade to “not trading with self” while logging.

-    final cached = _sdk.pubkeys.lastKnown(asset.id);
-    final ownPubkeys = cached ?? await _sdk.pubkeys.getPubkeys(asset);
+    final cached = _sdk.pubkeys.lastKnown(asset.id);
+    AssetPubkeys? ownPubkeys = cached;
+    if (ownPubkeys == null) {
+      try {
+        ownPubkeys = await _sdk.pubkeys.getPubkeys(asset);
+      } catch (e, s) {
+        log(
+          'Failed to load pubkeys for self-trade check: $e',
+          trace: s,
+          path: 'bridge_validator::_checkTradeWithSelf',
+          isError: true,
+        );
+        return false; // Don’t block the user on this non-critical check
+      }
+    }
🤖 Prompt for AI Agents
In lib/bloc/bridge_form/bridge_validator.dart around lines 267 to 269, the call
to await _sdk.pubkeys.getPubkeys(asset) can throw and currently aborts
validation; wrap the fetch in a try/catch, on success use the returned map as
ownPubkeys, on failure log the error (using the file's existing logger) and
assign an empty map for ownPubkeys so ownAddresses becomes empty (which degrades
to “not trading with self”); keep using cached when available and preserve
existing control flow otherwise.

_broadcastAsset(coin.copyWith(state: CoinState.suspended));
}

throw Exception('zcoin activaiton failed: $message');
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.

⚠️ Potential issue | 🟡 Minor

Fix typo in error message.

The error message contains a typo: "zcoin activaiton failed" should be "zcoin activation failed".

Apply this diff:

-          throw Exception('zcoin activaiton failed: $message');
+          throw Exception('zcoin activation failed: $message');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
throw Exception('zcoin activaiton failed: $message');
throw Exception('zcoin activation failed: $message');
🤖 Prompt for AI Agents
In lib/bloc/coins_bloc/coins_repo.dart around line 968, fix the typo in the
thrown Exception message by replacing "zcoin activaiton failed" with "zcoin
activation failed" so the error text is spelled correctly and remains otherwise
unchanged.

Comment on lines +81 to 89
: Flexible(
child: FiatAssetIcon(
currency: currency!,
icon: icon,
onTap: onTap,
assetExists: assetExists,
expanded: true,
),
),
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.

⚠️ Potential issue | 🟠 Major

Avoid Flexible inside FilledButton.icon’s icon slot

Flexible assumes a Flex ancestor; FilledButton.icon’s internals can change across Flutter versions. Passing Flexible here risks “Incorrect use of ParentDataWidget” and brittle layout. Let FiatAssetIcon manage its own expansion instead.

Apply this diff:

-          : Flexible(
-              child: FiatAssetIcon(
-                currency: currency!,
-                icon: icon,
-                onTap: onTap,
-                assetExists: assetExists,
-                expanded: true,
-              ),
-            ),
+          : FiatAssetIcon(
+              currency: currency!,
+              icon: icon,
+              onTap: onTap,
+              assetExists: assetExists,
+              expanded: true,
+            ),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
: Flexible(
child: FiatAssetIcon(
currency: currency!,
icon: icon,
onTap: onTap,
assetExists: assetExists,
expanded: true,
),
),
: FiatAssetIcon(
currency: currency!,
icon: icon,
onTap: onTap,
assetExists: assetExists,
expanded: true,
),
🤖 Prompt for AI Agents
In lib/views/fiat/fiat_select_button.dart around lines 81 to 89, remove the
Flexible wrapper used as the FilledButton.icon’s icon child (which can cause
“Incorrect use of ParentDataWidget” because Flexible requires a Flex ancestor);
instead pass FiatAssetIcon directly (it already has expanded: true) and if
needed apply sizing/constraining inside FiatAssetIcon or use a
SizedBox/ConstrainedBox inside that widget rather than placing Flexible here.

Comment on lines 76 to 81
@override
Widget build(BuildContext context) {
final amount = _controller.text.isNotEmpty
? Rational.parse(_controller.text)
? parseLocaleAwareRational(_controller.text)
: Rational.zero;
return ConstrainedBox(
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.

⚠️ Potential issue | 🟠 Major

Prevent build-time exceptions on malformed input.

parseLocaleAwareRational(...) can throw; a thrown FormatException during build will break the widget tree. Fallback to Rational.zero on error.

-    final amount = _controller.text.isNotEmpty
-        ? parseLocaleAwareRational(_controller.text)
-        : Rational.zero;
+    final Rational amount = () {
+      if (_controller.text.isEmpty) return Rational.zero;
+      try {
+        return parseLocaleAwareRational(_controller.text);
+      } on FormatException {
+        return Rational.zero;
+      }
+    }();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@override
Widget build(BuildContext context) {
final amount = _controller.text.isNotEmpty
? Rational.parse(_controller.text)
? parseLocaleAwareRational(_controller.text)
: Rational.zero;
return ConstrainedBox(
@override
Widget build(BuildContext context) {
final Rational amount = () {
if (_controller.text.isEmpty) return Rational.zero;
try {
return parseLocaleAwareRational(_controller.text);
} on FormatException {
return Rational.zero;
}
}();
return ConstrainedBox(
🤖 Prompt for AI Agents
In lib/views/market_maker_bot/coin_trade_amount_form_field.dart around lines 76
to 81, the call to parseLocaleAwareRational(_controller.text) can throw (e.g.,
FormatException) and will crash the build; wrap the parse in a try/catch and on
any parse error assign Rational.zero instead of letting the exception propagate
so the widget tree doesn't break; specifically, replace the direct parse with a
guarded block that tries to parse when text is not empty, catches
FormatException (and fallback to catch-all Exception if desired) and sets amount
= Rational.zero on error.

Comment on lines +76 to +83
String errorText;
if (state.error!.message.contains('Asset activation failed')) {
errorText = 'Asset activation failed for ${coin.displayName}';
} else {
errorText = LocaleKeys.connectionToServersFailing.tr(
args: [coin.displayName],
);
}
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.

⚠️ Potential issue | 🟠 Major

Use error types instead of string matching, and localize the error message.

Two concerns with this implementation:

  1. String-based error matching is brittle: Line 77 relies on matching the exact substring 'Asset activation failed' in the error message. If the error message format changes elsewhere in the codebase, this conditional will silently fail.

  2. Missing localization: Line 78 hardcodes an English error message, while the else branch (lines 80-82) properly uses LocaleKeys.connectionToServersFailing.tr() for localization. This creates an inconsistency in i18n handling.

Consider introducing an error type or code field in the error object to distinguish asset activation failures from other errors, rather than parsing message strings. Additionally, add a localization key for the asset activation error message.

Example refactor:

-          String errorText;
-          if (state.error!.message.contains('Asset activation failed')) {
-            errorText = 'Asset activation failed for ${coin.displayName}';
-          } else {
-            errorText = LocaleKeys.connectionToServersFailing.tr(
-              args: [coin.displayName],
-            );
-          }
+          final errorText = state.error!.isActivationFailure
+            ? LocaleKeys.assetActivationFailed.tr(args: [coin.displayName])
+            : LocaleKeys.connectionToServersFailing.tr(args: [coin.displayName]);

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In lib/views/wallet/coin_details/transactions/transaction_table.dart around
lines 76 to 83, replace brittle string-matching of state.error!.message with a
typed or coded error check and use a localized message key: update code to check
an explicit error type or a code property on the error (e.g., state.error is
AssetActivationException or state.error.code == 'asset_activation_failed')
instead of contains('Asset activation failed'), return the localized string via
a new LocaleKeys.assetActivationFailed.tr(args: [coin.displayName]) and ensure
any places that throw this error populate the type/code so this branch works
reliably.

Comment on lines +361 to 372
else ...[
Text(
LocaleKeys.amount.tr(),
style: Theme.of(context).textTheme.labelLarge,
),
const SizedBox(height: 4),
AssetAmountWithFiat(
assetId: assets.id,
amount: preview.balanceChanges.netChange,
isAutoScrollEnabled: true,
),
],
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.

⚠️ Potential issue | 🟠 Major

Normalize displayed withdrawal amount sign

Row path uses abs(); column path uses raw netChange. Apply abs() in the column path too to avoid “negative” confusion in confirm screen.

-                  AssetAmountWithFiat(
-                    assetId: assets.id,
-                    amount: preview.balanceChanges.netChange,
-                    isAutoScrollEnabled: true,
-                  ),
+                  AssetAmountWithFiat(
+                    assetId: assets.id,
+                    amount: preview.balanceChanges.netChange.abs(),
+                    isAutoScrollEnabled: true,
+                  ),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
else ...[
Text(
LocaleKeys.amount.tr(),
style: Theme.of(context).textTheme.labelLarge,
),
const SizedBox(height: 4),
AssetAmountWithFiat(
assetId: assets.id,
amount: preview.balanceChanges.netChange,
isAutoScrollEnabled: true,
),
],
else ...[
Text(
LocaleKeys.amount.tr(),
style: Theme.of(context).textTheme.labelLarge,
),
const SizedBox(height: 4),
AssetAmountWithFiat(
assetId: assets.id,
amount: preview.balanceChanges.netChange.abs(),
isAutoScrollEnabled: true,
),
],
🤖 Prompt for AI Agents
In lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart around lines
361 to 372, the column path currently passes preview.balanceChanges.netChange
directly into AssetAmountWithFiat causing negative values to show; change the
amount prop to use the absolute value (e.g., wrap netChange with .abs() or an
equivalent absolute-value helper) so the displayed withdrawal amount is
normalized and consistent with the row path.

Comment on lines +256 to 273
final bool isBip39 = _sdk.mnemonicValidator.validateBip39(decryptedSeed);
if (!isBip39) {
if (_isHdMode) {
setState(() {
_commonError = LocaleKeys.walletCreationHdBip39SeedError.tr();
_showCustomSeedToggle = true;
});
return;
}
if (!_allowCustomSeed) {
setState(() {
_commonError = LocaleKeys.walletCreationBip39SeedError.tr();
_showCustomSeedToggle = true;
});
return;
}
// Non-HD and custom seed allowed: continue without setting an error
}
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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Add mounted checks before setState after awaits; avoid exceptions on dispose.

Multiple awaits precede these setState calls (decrypt + BIP39). If the widget unmounts mid-flow, setState throws. Guard with if (!mounted) return.

-      final String? decryptedSeed = await encryptionTool.decryptData(
+      final String? decryptedSeed = await encryptionTool.decryptData(
         _filePasswordController.text,
         walletConfig.seedPhrase,
       );
       if (decryptedSeed == null) return;
       if (!_isValidData) return;

-      final bool isBip39 = _sdk.mnemonicValidator.validateBip39(decryptedSeed);
+      final bool isBip39 = _sdk.mnemonicValidator.validateBip39(decryptedSeed);
       if (!isBip39) {
         if (_isHdMode) {
-          setState(() {
+          if (!mounted) return;
+          setState(() {
             _commonError = LocaleKeys.walletCreationHdBip39SeedError.tr();
             _showCustomSeedToggle = true;
           });
           return;
         }
         if (!_allowCustomSeed) {
-          setState(() {
+          if (!mounted) return;
+          setState(() {
             _commonError = LocaleKeys.walletCreationBip39SeedError.tr();
             _showCustomSeedToggle = true;
           });
           return;
         }

Also apply the same mounted guard to earlier setState calls that update _filePasswordError after decrypting the file content.


🏁 Script executed:

#!/bin/bash
# Show all setState calls in this file to review async contexts
rg -n 'setState\(' lib/views/wallets_manager/widgets/wallet_import_by_file.dart -n -C2

Length of output: 2216


I need to see more context around these setState calls to verify the async flow and confirm whether mounted checks are genuinely needed.

Now I need to see the full function context to verify which setState calls actually follow await operations:

Now I need to inspect the actual function context to verify which setState calls follow awaits:

[skip_cloning_since_not_needed]


🏁 Script executed:

#!/bin/bash
# Get the full context around lines 220-320 to see the async flow
sed -n '200,320p' lib/views/wallets_manager/widgets/wallet_import_by_file.dart

Length of output: 3794


Let me verify if Flutter's analyzer already flags this issue:

Based on my verification, the review comment is accurate and well-founded. The code does have a genuine Flutter anti-pattern issue.

Key findings:

  1. Multiple setState calls follow await operations without checking the mounted property, which Flutter documentation states is how to determine if setState is legal to call.

  2. After each await point, the possibility that the widget has been unmounted needs to be checked before calling setState.

  3. The problematic setState calls occur at:

    • Lines 232–234 (after await encryptionTool.decryptData() at line 229)
    • Lines 238–240 (same await)
    • Lines 259–262 (after second await encryptionTool.decryptData() at line 245)
    • Lines 266–270 (same await)
    • Lines 315–317 (in catch block, following multiple awaits)
  4. The developer is aware of this pattern—line 288 correctly includes if (!mounted) return; before calling walletRenameDialog(), but hasn't applied it consistently to earlier setState calls.

  5. This practice is enforced by the use-setstate-synchronously linting rule, which is the standard static analysis for Flutter code quality.

Mounted guards must be added before all setState calls that follow await operations.

🤖 Prompt for AI Agents
In lib/views/wallets_manager/widgets/wallet_import_by_file.dart around lines
256–273 there are setState calls executed after await points without checking
widget mounted state; add mounted guards before those setState calls (i.e.,
check if (!mounted) return; immediately after the await and before calling
setState) for the occurrences you flagged (after await
encryptionTool.decryptData() at ~lines 229, 245 and the subsequent setState
blocks at ~232–234, ~238–240, ~259–262, ~266–270) and also add the same mounted
check before the setState in the catch block (~lines 315–317) so setState is
only called when the widget is still mounted.

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.

4 participants