Skip to content

chore: fix sdk market data breaking changes#3109

Merged
CharlVS merged 22 commits intodevfrom
chore/sdk-market-data-breaking-changes
Aug 29, 2025
Merged

chore: fix sdk market data breaking changes#3109
CharlVS merged 22 commits intodevfrom
chore/sdk-market-data-breaking-changes

Conversation

@takenagain
Copy link
Copy Markdown
Contributor

@takenagain takenagain commented Aug 26, 2025

Changes

Portfolio growth, total balance,profit/loss

  • Migrated sparkline, portfolio growth, and profit/loss charts to the new SDK market data interface. This introduces coin coverage using newly added providers (CoinGecko, and CoinPaprika coming soon). KMD and PEP should now have sparklines and portfolio growth values shown.
    • NOTE: CoinGecko free API limits are restrictive, so this is not a perfect solution
  • Changed the default data source of the total balance card to the same as the coin list and coin details page.

Settings

  • Re-implemented the coin config commit hash and runtime updates

Build

  • Added dependency overrides for the SDK, which is now a git submodule in this repository, to force builds to reference the SDK via local paths to the submodule.
  • Converted the project to a dart workspace (monorepo) structure to match the SDK structure.

Summary by CodeRabbit

  • New Features

    • Added bridge analytics events for initiated, succeeded, and failed cross-chain swaps.
    • Sparkline data now sourced via a repository, improving reliability in coin details and asset lists.
  • Refactor

    • Migrated market data (prices, charts, profit/loss, portfolio growth) to SDK-based sources with higher-precision Decimal/Rational calculations.
    • Improved periodic updates and precision in market maker price/margin computations.
  • Tests

    • Removed legacy CEX tests; added/updated scaffolding for 24h change, USD balance, and recent swaps.
  • Chores

    • Introduced workspace-based dependency management and updated local overrides.
    • Updated macOS Firebase Analytics plugin registration.

@takenagain takenagain added this to the v0.9.3 Release milestone Aug 26, 2025
@takenagain takenagain self-assigned this Aug 26, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Aug 26, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbit review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Migrates CEX-based market data to KomodoDefiSdk, introduces Decimal/Rational precision in pricing/arithmetics, refactors demo data generation to SDK price history, adjusts repositories/blocs accordingly, injects SparklineRepository via DI, updates model CexPrice to a deprecated Decimal-based class, adds workspace configuration, and prunes/updates tests. Minor analytics and formatting tweaks included.

Changes

Cohort / File(s) Summary
Workspace setup
pubspec.yaml, packages/komodo_ui_kit/pubspec.yaml, packages/komodo_persistence_layer/pubspec.yaml, app_theme/pubspec.yaml, sdk
Adds workspace resolution and local path overrides; introduces komodo_wallet_build_transformer; updates SDK submodule pointer.
App DI root
lib/main.dart, lib/bloc/app_bloc_root.dart
Registers SparklineRepository in DI; rewires bloc constructors to SDK-only (PriceChartBloc), passes sdk into DemoDataCache.withDefaults, removes CEX repo params from ProfitLoss/PortfolioGrowth defaults.
Price chart via SDK
lib/bloc/cex_market_data/price_chart/price_chart_bloc.dart
Replaces CEX repository with KomodoDefiSdk; fetches fiat price history by dates; computes percentage change with Decimal/Rational; simplifies coin mapping.
Portfolio growth via SDK + precision
lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_repository.dart, lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart
Drops CEX dependency; uses sdk.marketData.fiatPriceHistory (Map<DateTime, Decimal>); periodic updates refactored; 24h change now Rational; Decimal/Rational arithmetic throughout.
Profit/loss via SDK + precision
lib/bloc/cex_market_data/profit_loss/profit_loss_repository.dart, lib/bloc/cex_market_data/profit_loss/profit_loss_calculator.dart, lib/bloc/cex_market_data/profit_loss/demo_profit_loss_repository.dart, lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart
Removes CEX repo usage; calculators take sdk and Decimal prices; public API takes AssetId; removes isCoinChartSupported; filtering now excludes only test coins.
Demo data generator migration
lib/bloc/cex_market_data/mockup/generate_demo_data.dart, lib/bloc/cex_market_data/mockup/generator.dart, lib/bloc/cex_market_data/mockup/mock_portfolio_growth_repository.dart
Switches from OHLCV/CEX to SDK price history; cache keyed by AssetId→DateTime:price; new defaultAssetIds; DemoDataCache.withDefaults now requires sdk; removes supportedCoinsDemoData.
Model and price usage updates
lib/model/cex_price.dart, lib/bloc/coins_bloc/coins_repo.dart, lib/bloc/coins_bloc/coins_state.dart, lib/views/dex/..., lib/views/market_maker_bot/..., lib/views/dex/orderbook/orderbook_table.dart, lib/views/dex/simple/form/*, lib/bloc/nft_transactions/nft_txn_repository.dart
Replaces typedef with deprecated CexPrice class using Decimal fields; caches/reads prices as Decimal; downstream UI/logic converts via .toDouble() where needed.
Precision utilities
lib/shared/utils/balances_formatter.dart, lib/views/dex/dex_helpers.dart, lib/shared/utils/extensions/sdk_extensions.dart
Switches internal math to Decimal/Rational; adjusts extension APIs to QuoteCurrency; preserves double outputs for UI.
Bridge analytics and resilience
lib/bloc/bridge_form/bridge_bloc.dart
Adds analytics events (initiated/succeeded/failed); wraps max taker volume fetch with retry/backoff; style refactors.
Sparkline DI and widgets
lib/views/wallet/coin_details/coin_details_info/charts/coin_sparkline.dart, lib/views/wallet/wallet_page/common/asset_list_item_desktop.dart, lib/views/wallet/wallet_page/common/grouped_asset_ticker_item.dart, lib/main.dart
CoinSparkline now requires AssetId and injected SparklineRepository; consumers obtain repository from context and pass it through.
Market maker precision
lib/bloc/market_maker_bot/market_maker_order_list/market_maker_bot_order_list_repository.dart, lib/bloc/market_maker_bot/market_maker_trade_form/market_maker_trade_form_state.dart, lib/views/market_maker_bot/market_maker_bot_form.dart
Price/margin math moved to Decimal/Rational; UI compares price via toDouble(); formatting changes.
Fiat form/UI styling
lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart, lib/views/fiat/fiat_inputs.dart
Import narrowing and formatting-only changes; no logic changes.
Platform plugin rename
macos/Flutter/GeneratedPluginRegistrant.swift
Updates Firebase Analytics plugin class/identifier.
Tests cleanup/migration
test_units/main.dart, test_units/tests/cex_market_data/* (deleted), test_units/tests/helpers/*, test_units/tests/utils/*, test_units/tests/helpers/total_24_change_test.dart, test_units/tests/utils/get_usd_balance_test.dart, test_units/tests/utils/get_fiat_amount_tests.dart, test_units/tests/utils/test_util.dart
Removes CEX-market-data tests/mocks; adds/retains new helpers with many tests skipped; updates test util to stop constructing CexPrice.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor UI as User/UI
  participant B as PriceChartBloc
  participant SDK as KomodoDefiSdk.marketData

  UI->>B: PriceChartStarted/CoinsSelected(period, assets)
  B->>B: Build date range (startAt..endAt, interval)
  B->>SDK: fiatPriceHistory(assetId, dates)
  SDK-->>B: Map<DateTime, Decimal>
  B->>B: Build series, compute % change (Rational)
  B-->>UI: Emit LoadSuccess(series, change)
  Note over B,SDK: Data now sourced solely via SDK
Loading
sequenceDiagram
  autonumber
  participant UI as PortfolioGrowthView
  participant G as PortfolioGrowthBloc
  participant R as PortfolioGrowthRepository
  participant SDK as KomodoDefiSdk

  UI->>G: Start (listen)
  loop periodic tick
    G->>SDK: refresh current prices
    G->>R: fetchChartData()
    R->>SDK: fiatPriceHistory(assets, dates)
    SDK-->>R: Map<DateTime, Decimal> per asset
    R-->>G: Chart data (Decimal)
    G->>G: compute 24h total/percentage (Rational)
    G-->>UI: Emit GrowthChartLoadSuccess
  end
  opt error
    G-->>UI: Emit GrowthChartLoadFailure
  end
Loading
sequenceDiagram
  autonumber
  actor U as User
  participant BB as BridgeBloc
  participant S as SDK
  participant A as Analytics

  U->>BB: Start swap
  BB->>A: Track BridgeInitiatedEventData
  BB->>S: Quote/BestOrders (with retry/backoff)
  alt success
    BB->>A: Track BridgeSucceededEventData
  else failure
    BB->>A: Track BridgeFailedEventData
  end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested labels

QA, codex

Suggested reviewers

  • smk762
  • AndrewDelaney
  • gcharang
  • CharlVS

Poem

In charts of coins I twitch my ear,
Decimal dreams are crystal-clear.
I hop from CEX to SDK light,
Rational moons guide every byte.
A sparkline trail, I bound with glee—
Precision carrots, fetched for free.
Thump-thump: deployed! 🥕✨

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/sdk-market-data-breaking-changes

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@takenagain takenagain force-pushed the chore/sdk-market-data-breaking-changes branch from ac01054 to de721c8 Compare August 27, 2025 10:11
@CharlVS CharlVS changed the base branch from chore/upgrade-flutter-3.35.1 to dev August 27, 2025 10:23
API providers already moved to SDK, with market metrics soon to follow
legacy and local import names conflicted with SDK exports when importing without the `show` option to limit imports.
Double conversion used for widget display-only purposes, and Rational/Decimal defaulted to for internal calculations (e.g. repositories an blocs)
cursor[bot]

This comment was marked as outdated.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 27, 2025

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

https://walletrc--pull-3109-merge-fchw92v4.web.app

(expires Fri, 05 Sep 2025 10:34:13 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc

@takenagain
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@takenagain takenagain marked this pull request as ready for review August 27, 2025 15:37
Copilot AI review requested due to automatic review settings August 27, 2025 15:37
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Aug 27, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses breaking changes in the SDK market data implementation by migrating from the legacy CEX market data interfaces to the SDK's market data manager. The changes remove dependencies on external CEX repositories and update price/market data handling throughout the application.

Key changes:

  • Replace legacy CexPrice class with a simplified temporary version during SDK transition
  • Update all price data access to use SDK's market data interfaces
  • Remove dependencies on external CEX repositories (Binance, etc.)
  • Modernize workspace configuration with Dart workspaces

Reviewed Changes

Copilot reviewed 50 out of 53 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
lib/model/cex_price.dart Replaces legacy CexPrice with temporary implementation using Decimal types
lib/bloc/coins_bloc/coins_repo.dart Updates price fetching to use SDK market data manager
test_units/tests/utils/test_util.dart Removes usdPrice parameter from test coin creation
lib/bloc/cex_market_data/profit_loss/*.dart Updates profit/loss calculation to use SDK instead of CEX repositories
pubspec.yaml Adds workspace configuration and updates dependency overrides
Multiple UI components Updates price display to handle Decimal types from new CexPrice
Comments suppressed due to low confidence (2)

lib/bloc/cex_market_data/profit_loss/profit_loss_calculator.dart:171

  • [nitpick] The class name uses 'Realised' (British spelling) while other parts of the codebase might use 'Realized' (American spelling). Consider consistency in spelling convention.
class RealisedProfitLossCalculator extends ProfitLossCalculator {

lib/bloc/cex_market_data/profit_loss/profit_loss_calculator.dart:180

  • [nitpick] Similar spelling inconsistency with 'UnRealised'. Also, the class name should probably be 'UnrealizedProfitLossCalculator' for better readability.
class UnRealisedProfitLossCalculator extends ProfitLossCalculator {

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

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: 13

Caution

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

⚠️ Outside diff range comments (11)
lib/bloc/market_maker_bot/market_maker_trade_form/market_maker_trade_form_state.dart (2)

91-101: Guard against divide-by-zero and Decimal/Rational division exceptions in priceFromUsd.

If relUsdPrice is zero, dividing (especially with Decimal/Rational) can throw. Convert to double first and short-circuit on 0 to avoid Infinity/NaN and crashes.

-  double? get priceFromUsd {
-    final baseUsdPrice = sellCoin.value?.usdPrice?.price;
-    final relUsdPrice = buyCoin.value?.usdPrice?.price;
-    final price = relUsdPrice != null && baseUsdPrice != null
-        ? baseUsdPrice / relUsdPrice
-        : null;
-    return price?.toDouble();
-  }
+  double? get priceFromUsd {
+    final base = sellCoin.value?.usdPrice?.price?.toDouble();
+    final rel = buyCoin.value?.usdPrice?.price?.toDouble();
+    if (base == null || rel == null || rel == 0) return null;
+    return base / rel;
+  }

153-181: Preserve tradePreImageError in copyWith when omitted
The current copyWith always assigns tradePreImageError: preImageError, so omitting that argument (defaults to null) unintentionally clears any existing error. Update it to fall back to this.tradePreImageError when no new value is provided.

• Location:

  • lib/bloc/market_maker_bot/market_maker_trade_form/market_maker_trade_form_state.dart around line 177

• Change the constructor call in copyWith from:

-      tradePreImageError: preImageError,
+      tradePreImageError: preImageError ?? this.tradePreImageError,

• If callers ever need to explicitly clear the error (set it to null), introduce a flag:

MarketMakerTradeFormState copyWith({
  …,
  MarketMakerTradeFormError? preImageError,
  bool clearTradePreImageError = false,
}) {
  return MarketMakerTradeFormState(
    …,
    tradePreImageError: clearTradePreImageError
        ? null
        : (preImageError ?? this.tradePreImageError),
    …
  );
}

This ensures the field is only overwritten when intended.

lib/bloc/market_maker_bot/market_maker_order_list/market_maker_bot_order_list_repository.dart (1)

86-94: Base amount calculation ignores maxVolume and squares the balance.

toRational() * Rational.parse(baseCoinBalance.toString()) effectively returns balance² and never uses maxVolume. This will overstate amounts.

Apply:

 Rational _getBaseAmountFromVolume(String baseCoinId, double maxVolume) {
   final baseCoin = _coinsRepository.getCoin(baseCoinId);
   final baseCoinBalance = baseCoin == null
       ? Decimal.zero
       : _coinsRepository.lastKnownBalance(baseCoin.id)?.spendable ??
             Decimal.zero;
-  return baseCoinBalance.toRational() *
-      Rational.parse(baseCoinBalance.toString());
+  final balance = baseCoinBalance.toRational();
+  final maxVol = Rational.parse(maxVolume.toString());
+  return balance < maxVol ? balance : maxVol;
 }
lib/views/dex/dex_helpers.dart (2)

372-387: Fix accumulation and preserve both USD total and per-coin amounts.

Also handle USD coin specially and repeated entries.

Apply:

-Map<String, Rational> _combineFees(
+Map<String, Rational> _combineFees(
   Coin? coin,
   TradePreimageExtendedFeeInfo fee,
   Map<String, Rational> previousValue,
 ) {
-  final feeAmount = Rational.tryParse(fee.amount) ?? Rational.zero;
-  final feeUsdAmount =
-      feeAmount * (coin?.usdPrice?.price ?? Decimal.zero).toRational();
-
-  if (feeUsdAmount > Rational.zero) {
-    previousValue['USD'] = previousValue['USD']! + feeUsdAmount;
-  } else if (feeAmount > Rational.zero) {
-    previousValue[fee.coin] = feeAmount;
-  }
+  final Rational feeAmount = Rational.tryParse(fee.amount) ?? Rational.zero;
+  final Decimal usdPrice = coin?.usdPrice?.price ?? Decimal.zero;
+  final Rational feeUsdAmount = feeAmount * usdPrice.toRational();
+  final String coinAbbr = (coin?.abbr ?? fee.coin).toUpperCase();
+
+  // Always accumulate USD equivalent when convertible
+  if (feeUsdAmount > Rational.zero) {
+    previousValue['USD'] =
+        (previousValue['USD'] ?? Rational.zero) + feeUsdAmount;
+  }
+
+  // Also keep coin-denominated breakdown (except for USD itself)
+  if (feeAmount > Rational.zero && coinAbbr != 'USD') {
+    previousValue[coinAbbr] =
+        (previousValue[coinAbbr] ?? Rational.zero) + feeAmount;
+  }
   return previousValue;
 }

429-447: Guard divide-by-zero before computing price from buy/sell.

Catching exceptions for Rational division by zero is avoidable; add an explicit zero check.

Apply:

   } else if (buyAmount != null) {
-    try {
-      price = buyAmount / sellAmount;
-      return (buyAmount, price);
-    } catch (_) {
-      return (buyAmount, null);
-    }
+    if (sellAmount == Rational.zero) {
+      return (buyAmount, null);
+    }
+    price = buyAmount / sellAmount;
+    return (buyAmount, price);
   }
lib/views/wallet/coin_details/coin_details_info/charts/coin_sparkline.dart (1)

20-26: Unreachable error branch; simplify FutureBuilder conditions.

snapshot.hasError is handled in the first if, making the next else-if unreachable.

-        if (snapshot.connectionState == ConnectionState.waiting ||
-            snapshot.hasError) {
-          return const SizedBox.shrink();
-        } else if (snapshot.hasError) {
-          return Text('Error: ${snapshot.error}');
-        } else if (!snapshot.hasData || (snapshot.data?.isEmpty ?? true)) {
-          return const SizedBox.shrink();
-        } else {
+        if (snapshot.connectionState == ConnectionState.waiting ||
+            snapshot.hasError ||
+            !snapshot.hasData ||
+            (snapshot.data?.isEmpty ?? true)) {
+          return const SizedBox.shrink();
+        } else {
lib/views/wallet/wallet_page/common/grouped_asset_ticker_item.dart (1)

125-131: Fix: NumberFormat.format on non-num (Decimal/Rational) will crash

price!.price is likely Decimal/Rational now; NumberFormat expects num. Convert to double before formatting.

Apply:

-                            final formattedPrice = price?.price != null
-                                ? priceFormatter.format(price!.price)
-                                : '';
+                            final rawPrice = price?.price?.toDouble();
+                            final formattedPrice = rawPrice != null
+                                ? priceFormatter.format(rawPrice)
+                                : '';
lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart (1)

142-153: Bug: Concurrent modification while iterating and removing coins

You remove from the same list you're iterating; this can throw ConcurrentModificationError. Use removeWhere.

Apply:

-  Future<List<Coin>> _removeUnsupportedCons(
+  Future<List<Coin>> _removeUnsupportedCoins(
     List<Coin> walletCoins,
     String fiatCoinId,
   ) async {
-    final coins = List<Coin>.of(walletCoins);
-    for (final coin in coins) {
-      if (coin.isTestCoin) {
-        coins.remove(coin);
-      }
-    }
-    return coins;
+    final coins = List<Coin>.of(walletCoins);
+    coins.removeWhere((c) => c.isTestCoin);
+    return coins;
   }

And update callers accordingly (see Lines 50-54).

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

69-90: Bug: Hardcoded percentage/value in UI

percentage: 23 and value: 50 ship placeholder data. Use the provided prop or null.

Apply:

-                      child: TrendPercentageText(
-                        percentage: 23,
+                      child: TrendPercentageText(
+                        percentage: priceChangePercentage24h ?? 0,
                         upColor: Theme.of(context).brightness == Brightness.dark
@@
-                        value: 50,
-                        valueFormatter: (value) =>
-                            NumberFormat.currency(symbol: '\$').format(value),
+                        value: null,
+                        valueFormatter: null,

87-89: Add missing intl import for NumberFormat
The AssetListItemDesktop widget uses NumberFormat but doesn’t import it, which will lead to a build-time error. Please add the following import alongside the other top-level imports:

• In lib/views/wallet/wallet_page/common/asset_list_item_desktop.dart, at the top of the file:

import 'package:intl/intl.dart';
lib/bloc/app_bloc_root.dart (1)

152-179: Ensure SparklineRepository is provided at the application root

Our searches did not find any RepositoryProvider<SparklineRepository> registration in the codebase, yet GroupedAssetTickerItem, AssetListItemDesktop, and CoinSparkline all call

RepositoryProvider.of<SparklineRepository>(context)

at runtime. Without registering the provider near the root, these widgets will throw a ProviderNotFoundException.

Please add the following (or equivalent, accounting for any required constructor parameters) to your root MultiRepositoryProvider—for example, in lib/bloc/app_bloc_root.dart (or wherever your top-level providers are defined):

     return MultiRepositoryProvider(
       providers: [
         RepositoryProvider(
           create: (_) => IpfsGatewayManager(),
           dispose: (manager) => manager.dispose(),
         ),
+        // Provide SparklineRepository so downstream widgets can read sparklines
+        RepositoryProvider(
+          create: (_) => SparklineRepository(),
+        ),
         RepositoryProvider(
           create: (_) => NftsRepo(api: mm2Api.nft, coinsRepo: coinsRepository),
         ),
         // …other providers…
       ],

– lib/bloc/app_bloc_root.dart: insert the RepositoryProvider<SparklineRepository> alongside the other root providers.
Failure to do this will cause runtime errors when any widget attempts to read the SparklineRepository from the context.

♻️ Duplicate comments (3)
lib/views/market_maker_bot/market_maker_bot_form.dart (2)

114-117: Fix nullability assertion on Decimal price before toDouble().

Using e.usdPrice!.price!.toDouble() will throw if price is null. Filter safely.

-                          (e) =>
-                              e.usdPrice != null &&
-                              e.usdPrice!.price!.toDouble() > 0,
+                          (e) => (e.usdPrice?.price?.toDouble() ?? 0) > 0,

178-181: Same null-safety issue in mobile layout filter.

Mirror the safe check here.

-                      (e) =>
-                          e.usdPrice != null &&
-                          e.usdPrice!.price!.toDouble() > 0,
+                      (e) => (e.usdPrice?.price?.toDouble() ?? 0) > 0,
lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart (1)

303-307: Remove unnecessary await on getCachedPrice if sync; unify balance type

This was flagged previously and still applies if getCachedPrice is synchronous; also align lastKnownUsdBalance type usage across methods.

Run to confirm signatures and types:

#!/bin/bash
# Verify getCachedPrice signature
rg -nP "class\s+PortfolioGrowthRepository\b.*" -C1
rg -nP "\bgetCachedPrice\s*\(" -C2

# Find lastKnownUsdBalance definition and return type
rg -nP "lastKnownUsdBalance\s*\(" -C3

# Ensure callers expect sync/async consistently
rg -nP "portfolioGrowthRepository\.getCachedPrice\s*\(" -n -C2
🧹 Nitpick comments (50)
test_units/tests/helpers/total_fee_test.dart (5)

155-155: Add a skip reason for traceability.

Prefer a descriptive skip message so CI dashboards show why this is disabled.

-  }, skip: true);
+  }, skip: 'Blocked by SDK testability gaps: need DI/public adapter for MarketDataManager.priceIfKnown.');

1-2: Drop commented-out imports; keep history in VCS.

These add noise and fall out of sync. Remove until re-enabled.

-// import 'package:komodo_defi_sdk/komodo_defi_sdk.dart' show KomodoDefiSdk;
-// import 'package:rational/rational.dart';
-// import 'package:web_dex/model/trade_preimage_extended_fee_info.dart';
-// import 'package:web_dex/views/dex/dex_helpers.dart';
-// import '../utils/test_util.dart';

Also applies to: 4-7


12-65: Replace large commented test bodies with a compact, durable reference.

The commented blocks are hard to maintain. Keep the intent as a concise block comment of expected cases until SDK mocking is ready.

-    //   final List<TradePreimageExtendedFeeInfo> info = [
-    //     ...
-    //   ];
-    //   expect(...);
-    // ...
-    // test('Total fee edge cases', () {
-    //   ...
-    // });
+    /* Legacy total_fee expectations to port to SDK-backed tests:
+       Positive:
+       - info == null -> '$0.00'
+       - [KMD 1e-8, BTC 2e-8, LTC 3e-8] @ $10   -> '$0.0000006'
+       - same @ $0.10                           -> '$0.000000006'
+       - same @ $0                              -> '0.00000001 KMD + 0.00000002 BTC + 0.00000003 LTC' (NBSP between amount and symbol)
+       Edge:
+       - [KMD 1e-14] @ $1           -> '$1e-14'
+       - same @ 1e-9                -> '$1.00000000000e-23'
+       - same @ 1e-13               -> '$1e-27'
+       - same @ 1e-30               -> '$1.00000000000e-44'
+       - same @ 1e-60               -> '$1e-74'
+       - same @ 0                   -> '1e-14 KMD' (NBSP)
+       - [BTC ~1.23456789012e+59] @ $1     -> '$1.23456789012e+59'
+       - same @ 1e-59                      -> '$1.23'
+     */

Also applies to: 67-154


9-9: Make the TODO actionable (owner + blocker).

Clarify the migration path and what’s blocking it.

-// TODO: revisit or migrate these tests to the SDK package
+// TODO(takenagain): Migrate total_fee tests to SDK-backed implementations; blocked on DI/public test seam for MarketDataManager.priceIfKnown.

If helpful, I can draft an SDK-facing adapter interface and a fake implementation to unblock these tests—want me to open a follow-up PR?


158-160: Remove redundant main(); rely on central test runner

The standalone main() in test_units/tests/helpers/total_fee_test.dart is unnecessary because testGetTotalFee() is already invoked by the central runner in test_units/main.dart (line 82). Removing it avoids double-registration and keeps test files consistent.

• Location: test_units/tests/helpers/total_fee_test.dart, lines 158–160
• Change:

- void main() {
-   testGetTotalFee();
- }
lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart (4)

190-193: Confirm return URL is absolute and accepted by providers.

If BaseFiatProvider.successUrl(...) ever returns a relative URL, some providers (e.g., Banxa/Ramp) may reject it. Verify it yields a fully-qualified HTTPS URL.


201-204: Also set status to failure when checkout URL is empty.

Currently only fiatOrderStatus is set; status remains unchanged, which can desync UI state vs. order state.

Apply this diff:

-        _log.severe('Invalid checkout URL received.');
-        return emit(state.copyWith(fiatOrderStatus: FiatOrderStatus.failed));
+        _log.severe('Invalid checkout URL received.');
+        return emit(
+          state.copyWith(
+            status: FiatFormStatus.failure,
+            fiatOrderStatus: FiatOrderStatus.failed,
+            checkoutUrl: '',
+          ),
+        );

221-224: Align failure states in catch with order status.

On submit error you clear checkoutUrl and set status: failure, but not fiatOrderStatus. Consider marking it failed for consistency with the empty-URL branch.

Apply this diff:

-      emit(state.copyWith(status: FiatFormStatus.failure, checkoutUrl: ''));
+      emit(state.copyWith(
+        status: FiatFormStatus.failure,
+        fiatOrderStatus: FiatOrderStatus.failed,
+        checkoutUrl: '',
+      ));

535-538: Consider marking status as failure in update fallback.

In _updatePaymentMethods catch you clear methods and provider error but keep status unchanged. Set FiatFormStatus.failure for consistent error handling like in _refreshPaymentMethods.

Apply this diff:

-      return state.copyWith(paymentMethods: [], providerError: () => null);
+      return state.copyWith(
+        paymentMethods: [],
+        status: FiatFormStatus.failure,
+        providerError: () => null,
+      );
lib/bloc/market_maker_bot/market_maker_trade_form/market_maker_trade_form_state.dart (4)

106-111: Use the canonical parsed value for margin.

Prefer the already-validated valueAsDouble.

-    final spreadPercentage = double.tryParse(tradeMargin.value) ?? 0;
+    final spreadPercentage = tradeMargin.valueAsDouble;

118-121: Avoid double→string→Rational precision loss.

If you must derive from a double, at least guard non-finite and fix precision; ideally build Rational directly from Decimal/Rational operands.

-  Rational? get priceFromUsdWithMarginRational {
-    final price = priceFromUsdWithMargin;
-    return price != null ? Rational.parse(price.toString()) : null;
-  }
+  Rational? get priceFromUsdWithMarginRational {
+    final price = priceFromUsdWithMargin;
+    if (price == null || !price.isFinite) return null;
+    return Rational.parse(price.toStringAsFixed(16));
+  }

If feasible, compute using the original Decimal/Rational values to preserve exactness.


188-199: Stabilize spread formatting and reuse parsed margin.

Avoid floating drift in the string; also reuse valueAsDouble.

-    final spreadPercentage = double.parse(tradeMargin.value);
-    final spread = 1 + (spreadPercentage / 100);
+    final spread = 1 + (tradeMargin.valueAsDouble / 100);
@@
-      spread: spread.toString(),
+      spread: spread.toStringAsFixed(8),

13-15: Typo in comment.

“contoller” → “controller”.

-// Usually this would be a dedicated tab contoller/ui flow bloc, but because
+// Usually this would be a dedicated tab controller/ui flow bloc, but because
sdk (1)

1-1: Sync pubspec.yaml workspace and dependency_overrides for local SDK packages

The root pubspec.yaml currently only includes:

  • A single workspace entry pointing to packages/komodo_ui_kit
  • A dependency_override for intl

To ensure your local SDK packages (komodo_defi_sdk, komodo_symbol_converter) are properly wired and imports resolve consistently during development, please update it as follows:

 workspace:
-  # - sdk/
-  - packages/komodo_ui_kit
+  - packages/**
+  - sdk/packages/**
 dependency_overrides:
-  # Temporary until Flutter's pinned version is updated
-  intl: ^0.20.2
+  intl: ^0.20.2
+  komodo_defi_sdk:
+    path: sdk/packages/komodo_defi_sdk
+  komodo_symbol_converter:
+    path: sdk/packages/komodo_symbol_converter

This will enable your IDE and Dart tooling to pick up the local SDK packages under sdk/packages/… without needing to publish interim versions.

lib/shared/utils/balances_formatter.dart (1)

32-34: Minor micro-opt: short-circuit zero amounts to avoid parse/mul.

No behavior change; avoids Rational.parse when amount is zero.

 double getFiatAmount(Coin coin, Rational amount) {
+  if (amount == Rational.zero) return 0.0;
   final Decimal usdPrice = coin.usdPrice?.price ?? Decimal.zero;
   final Rational usdPriceRational = Rational.parse(usdPrice.toString());
   return (amount * usdPriceRational).toDouble();
 }
test_units/tests/utils/get_usd_balance_test.dart (1)

22-51: Skipped test with commented body: add a reason to track migration.

Keep the skip explicit to ease cleanup when SDK/Decimal tests land.

-  }, skip: true);
+  }, skip: 'Migrating to SDK/Decimal pricing (PR #3109)');
lib/views/dex/orderbook/orderbook_table.dart (1)

66-68: Consistent Decimal→double conversion; consider formatting for display.

Optional: render the USD with formatAmt for consistency with spot price.

Example (outside this hunk):

Text('\$${formatAmt(baseUsdPrice)}', style: style)
test_units/tests/utils/get_fiat_amount_tests.dart (2)

1-7: Remove commented imports or migrate tests now.

Leaving commented imports adds noise. Either delete them or reintroduce when rewriting the tests against the SDK utilities.


9-49: Skipped test with commented assertions = no coverage. Add a tracked skip reason or port now.

This test currently provides zero signal. At minimum, add a clear skip reason; ideally, port to the SDK path and reintroduce assertions (including extreme-value Rational/Decimal cases).

Apply:

-  }, skip: true);
+  }, skip: 'Migrate to KomodoDefiSdk getFiatAmount path; rewrite with Decimal/Rational.');

I can help port these to the new SDK-based formatting helpers if you want.

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

128-131: Boundary hint defaults: confirm '1' min and localize '∞'.

Hardcoding min fallback to '1' and the infinity glyph may be misleading/non-localized. Consider deriving defaults from validation rules and localizing the symbols.

Apply:

-        : '(${minFiatAmount ?? '1'} - ${maxFiatAmount ?? '∞'})';
+        : '(${minFiatAmount ?? LocaleKeys.min.tr()} - ${maxFiatAmount ?? LocaleKeys.infinity.tr()})';

If there are no i18n keys, consider constants in a shared UI utils file.

lib/views/dex/dex_helpers.dart (3)

355-370: Formatting via toDouble() may lose precision for very large rationals.

Optional: consider formatting from Decimal to avoid double conversion when feasible.


173-176: Avoid unnecessary allocation.

No need for toList() just to get length.

Apply:

-  return (coinAbbrMap[coinAbbr] ?? [])
-      .where((abbr) => secondCoinAbbr == null || secondCoinAbbr == abbr)
-      .toList()
-      .length;
+  return (coinAbbrMap[coinAbbr] ?? [])
+      .where((abbr) => secondCoinAbbr == null || secondCoinAbbr == abbr)
+      .length;

405-408: Docstring drift.

Function is getFractionOfAmount, but docs still mention “sell amount”. Update for clarity.

lib/shared/utils/extensions/sdk_extensions.dart (1)

58-68: Avoid while(true) in async; be cancellation-friendly.*

Use a periodic stream + await-for so cancellation stops the loop promptly.

   /// Watches for price updates and maintains the cache
   Stream<double?> watchFiatPrice(
     KomodoDefiSdk sdk, {
     QuoteCurrency quoteCurrency = Stablecoin.usdt,
   }) async* {
-    while (true) {
-      final price = await getFiatPrice(sdk, quoteCurrency: quoteCurrency);
-      yield price;
-      await Future.delayed(const Duration(minutes: 1));
-    }
+    // Emit immediately, then every minute.
+    yield await getFiatPrice(sdk, quoteCurrency: quoteCurrency);
+    await for (final _ in Stream.periodic(const Duration(minutes: 1))) {
+      yield await getFiatPrice(sdk, quoteCurrency: quoteCurrency);
+    }
   }
lib/model/cex_price.dart (2)

10-15: Prefer enum byName/name over toString().split for robustness.

Using toString().split('.').last is brittle. Dart enums expose name and byName.

-CexDataProvider cexDataProvider(String string) {
-  return CexDataProvider.values.firstWhere(
-    (e) => e.toString().split('.').last == string,
-    orElse: () => CexDataProvider.unknown,
-  );
-}
+CexDataProvider cexDataProvider(String string) {
+  try {
+    return CexDataProvider.values.byName(string);
+  } catch (_) {
+    return CexDataProvider.unknown;
+  }
+}

41-52: Allow clearing nullable fields via copyWith.

With ValueGetter you cannot set price/change24h to null. Accept Decimal? to allow clearing when needed.

-  CexPrice copyWith({
-    ValueGetter<Decimal>? price,
-    ValueGetter<Decimal>? change24h,
-    DateTime? lastUpdated,
-  }) {
+  CexPrice copyWith({
+    ValueGetter<Decimal?>? price,
+    ValueGetter<Decimal?>? change24h,
+    DateTime? lastUpdated,
+  }) {
     return CexPrice(
       assetId: assetId,
-      price: price?.call() ?? this.price,
-      change24h: change24h?.call() ?? this.change24h,
+      price: price != null ? price() : this.price,
+      change24h: change24h != null ? change24h() : this.change24h,
       lastUpdated: lastUpdated ?? this.lastUpdated,
     );
   }
lib/bloc/nft_transactions/nft_txn_repository.dart (1)

102-102: Avoid firstWhere crash and O(n²); map coins by abbr.

firstWhere throws if abbr missing and is linear per lookup. Build a map once, then read safely.

-    final coins = _coinsRepo.getKnownCoins();
-    for (final abbr in coinAbbr) {
-      final coin = coins.firstWhere((c) => c.abbr == abbr);
-      _abbrToUsdPrices[abbr] = coin.usdPrice?.price?.toDouble();
-    }
+    final coins = _coinsRepo.getKnownCoins();
+    final coinsByAbbr = {for (final c in coins) c.abbr: c};
+    for (final abbr in coinAbbr) {
+      final coin = coinsByAbbr[abbr];
+      if (coin != null) {
+        _abbrToUsdPrices[abbr] = coin.usdPrice?.price?.toDouble();
+      }
+    }
test_units/tests/helpers/total_24_change_test.dart (1)

11-168: Add a meaningful skip reason and clean up commented tests

The total_24_change_test.dart file is properly imported and invoked by your test aggregator (see test_units/main.dart lines 18 and 81), so feel free to update the skip and trim the commented blocks.

• In test_units/tests/helpers/total_24_change_test.dart:

  • Replace the bare skip: true with a concise explanation.
  • Remove or relocate the large commented-out test cases (e.g. into a TODO with an issue link) to reduce noise.

Apply:

-  }, skip: true);
+  }, skip: 'Pending migration to SDK (port getTotal24Change tests to Decimal/Rational).');
pubspec.yaml (1)

209-231: YAML indentation nit and override sweep.

Minor: inconsistent indent under dragon_logs → path. Functionally fine, but fix for readability.

   dragon_charts_flutter:
     path: sdk/packages/dragon_charts_flutter
-  dragon_logs:
-      path: sdk/packages/dragon_logs
+  dragon_logs:
+    path: sdk/packages/dragon_logs

Also, a large override set can mask version conflicts. If releases need fewer overrides later, consider a dev-only overlay file (documented in repo docs) to toggle locally.

lib/bloc/cex_market_data/mockup/mock_portfolio_growth_repository.dart (1)

23-30: Constructor wiring — LGTM; consider optional DI for generator.

Super call wiring with performanceMode/sdk looks correct. For extra testability, allow injecting DemoDataCache via parameter (fallback to withDefaults(sdk)).

-  MockPortfolioGrowthRepository.withDefaults({
+  MockPortfolioGrowthRepository.withDefaults({
     required this.performanceMode,
     required super.coinsRepository,
     required super.sdk,
-  }) : super(
-         transactionHistoryRepo: MockTransactionHistoryRepo(
+    DemoDataCache? demoDataCache,
+  }) : super(
+         transactionHistoryRepo: MockTransactionHistoryRepo(
            performanceMode: performanceMode,
-           demoDataGenerator: DemoDataCache.withDefaults(sdk),
+           demoDataGenerator: (demoDataCache ?? DemoDataCache.withDefaults(sdk)),
          ),
lib/views/wallet/wallet_page/common/grouped_asset_ticker_item.dart (2)

84-86: Minor: Prefer context.read for non-listening lookup

Use context.read() to avoid unintended rebuilds if the provider updates.

Apply:

-    final sparklineRepository = RepositoryProvider.of<SparklineRepository>(
-      context,
-    );
+    final sparklineRepository = context.read<SparklineRepository>();

210-222: Optional: Localize tooltips

These tooltips are hardcoded. Consider using localization keys for consistency.

lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart (2)

97-109: Reduce error/log spam: use filtered active coins in periodic refresh

The periodic stream uses event.coins (may include test/inactive/unsupported). Reuse filtered active set to avoid repeated failures.

Apply:

-    await emit.forEach(
-      Stream<Object?>.periodic(event.updateFrequency).asyncMap(
-        (_) async => _getProfitLossChart(event, event.coins, useCache: false),
-      ),
+    final periodicCoins = await _removeInactiveCoins(
+      await _removeUnsupportedCoins(event.coins, event.fiatCoinId),
+    );
+    await emit.forEach(
+      Stream<Object?>.periodic(event.updateFrequency).asyncMap(
+        (_) async => _getProfitLossChart(event, periodicCoins, useCache: false),
+      ),

142-145: Nit: Typo in method name (_removeUnsupportedCons → _removeUnsupportedCoins)

Improves readability and consistency.

Apply rename as shown in prior diffs and update the caller at Lines 50-54.

Also applies to: 50-54

lib/views/wallet/wallet_page/common/asset_list_item_desktop.dart (1)

36-39: Minor: Prefer context.read for non-listening lookup

Use context.read() for DI retrieval.

Apply:

-    final sparklineRepository = RepositoryProvider.of<SparklineRepository>(
-      context,
-    );
+    final sparklineRepository = context.read<SparklineRepository>();
lib/bloc/bridge_form/bridge_bloc.dart (1)

435-446: Async handler should return Future (style/semantics)

It’s currently declared void async; prefer Future for clarity with Bloc handlers.

Apply this diff:

-  void _onUpdateFees(BridgeUpdateFees event, Emitter<BridgeState> emit) async {
+  Future<void> _onUpdateFees(
+    BridgeUpdateFees event,
+    Emitter<BridgeState> emit,
+  ) async {
lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart (1)

146-175: Type periodic stream explicitly to ChartData (readability)

Let inference yield Stream to make the await-for clearer.

Apply this diff:

-    final periodicUpdate = Stream<Object?>.periodic(event.updateFrequency)
+    final Stream<ChartData> periodicUpdate =
+        Stream<void>.periodic(event.updateFrequency)
         .asyncMap((_) async {
           await portfolioGrowthRepository.updatePrices();
           return _fetchPortfolioGrowthChart(event);
         });
lib/bloc/coins_bloc/coins_repo.dart (1)

542-570: Normalize cache key casing to avoid misses

Callers sometimes use .toUpperCase(); normalize keys when storing.

Apply this diff:

-          _pricesCache[asset.id.symbol.configSymbol] = CexPrice(
+          final symbolKey = asset.id.symbol.configSymbol.toUpperCase();
+          _pricesCache[symbolKey] = CexPrice(
             assetId: asset.id,
             price: fiatPrice,
             lastUpdated: DateTime.now(),
             change24h: change24h,
           );

Optionally, add a similar toUpperCase() in getCachedPrice.

lib/bloc/cex_market_data/price_chart/price_chart_bloc.dart (1)

39-46: Date points may omit the end boundary (minor)

Consider adding +1 to include endAt for closed intervals.

Apply this diff:

-              final dates = List.generate(
-                (endAt.difference(startAt).inSeconds / interval.toSeconds())
-                    .toInt(),
+              final dates = List.generate(
+                (endAt.difference(startAt).inSeconds ~/ interval.toSeconds()) + 1,
                 (index) => startAt.add(
                   Duration(seconds: index * interval.toSeconds()),
                 ),
               );
lib/bloc/cex_market_data/profit_loss/profit_loss_calculator.dart (1)

24-28: Docstring drift

Parameter docs still mention ticker String; update to reflect AssetId.

lib/bloc/cex_market_data/profit_loss/profit_loss_repository.dart (2)

129-131: Optional: guard against cache deserialization errors.

Hive can throw on incompatible adapter versions. Consider catching and purging the bad key to self-heal.

-      final ProfitLossCache? profitLossCache = await _profitLossCacheProvider
-          .get(compoundKey);
+      ProfitLossCache? profitLossCache;
+      try {
+        profitLossCache = await _profitLossCacheProvider.get(compoundKey);
+      } catch (e, st) {
+        _log.warning('Corrupt ProfitLossCache at $compoundKey, deleting', e, st);
+        await _profitLossCacheProvider.delete(compoundKey);
+        profitLossCache = null;
+      }

151-152: Optional: handle fetch errors consistently.

Mirror the portfolio-growth repo’s ignoreTransactionFetchErrors pattern to avoid bubbling transient failures.

-    final transactions = await _transactionHistoryRepo
-        .fetchCompletedTransactions(coinId);
+    final transactions = await _transactionHistoryRepo
+        .fetchCompletedTransactions(coinId)
+        .catchError((e, st) {
+          _log.warning('Error fetching transactions for ${coinId.id}', e, st);
+          return <Transaction>[];
+        });
lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_repository.dart (5)

5-11: Reduce residual CEX surface, if possible.

You only need GraphInterval(+extension) and CoinOhlc.fromConstantPrice; consider relocating or shimming these to avoid depending on komodo_cex_market_data here.


165-175: Minor: prefer try/await/catch for clarity and single stopwatch stop.

Current then/catchError calls stop twice on error paths. A straight try/catch reads cleaner.

-    final List<Transaction> transactions = await _transactionHistoryRepository
-        .fetchCompletedTransactions(coin.id)
-        .then((value) => value.toList())
-        .catchError((Object e) {
-          txStopwatch.stop();
-          _log.warning(
-            'Error fetching transactions for ${coin.id} '
-            'in ${txStopwatch.elapsedMilliseconds}ms: $e',
-          );
-          if (ignoreTransactionFetchErrors) {
-            return List<Transaction>.empty();
-          } else {
-            throw e;
-          }
-        });
+    List<Transaction> transactions = const [];
+    try {
+      transactions = (await _transactionHistoryRepository
+              .fetchCompletedTransactions(coin.id))
+          .toList();
+    } catch (e, st) {
+      _log.warning('Error fetching transactions for ${coin.id}', e, st);
+      if (!ignoreTransactionFetchErrors) rethrow;
+    } finally {
+      txStopwatch.stop();
+    }

259-263: Typo: portfolowGrowthChart → portfolioGrowthChart.

Rename for clarity.

-    final List<Point<double>> portfolowGrowthChart = _mergeTransactionsWithOhlc(
+    final List<Point<double>> portfolioGrowthChart = _mergeTransactionsWithOhlc(
       ohlcData,
       transactions,
     );

269-273: Follow-up to rename.

-        graph: portfolowGrowthChart,
+        graph: portfolioGrowthChart,

281-287: Follow-up to rename.

-      'with ${portfolowGrowthChart.length} data points',
+      'with ${portfolioGrowthChart.length} data points',
-    return portfolowGrowthChart;
+    return portfolioGrowthChart;
lib/bloc/cex_market_data/mockup/generate_demo_data.dart (3)

114-121: Avoid cast hack for nullable firstWhere; use a simple loop.

Prevents confusing types and nullability gymnastics.

-    final actualAssetIds = assetIds.isEmpty ? defaultAssetIds : assetIds;
-    final assetId = actualAssetIds.cast<AssetId?>().firstWhere(
-      (asset) =>
-          asset!.id.toLowerCase() == coinId.toLowerCase() ||
-          asset.symbol.assetConfigId.toLowerCase() == coinId.toLowerCase(),
-      orElse: () => null,
-    );
+    final actualAssetIds = assetIds.isEmpty ? defaultAssetIds : assetIds;
+    AssetId? assetId;
+    for (final asset in actualAssetIds) {
+      if (asset.id.toLowerCase() == coinId.toLowerCase() ||
+          asset.symbol.assetConfigId.toLowerCase() == coinId.toLowerCase()) {
+        assetId = asset;
+        break;
+      }
+    }

152-155: Probability for “same day” trade reuses tradeAmountFactors.

Consider a separate probability vector; coupling size and frequency can skew behavior.


198-201: Optional: guard against divide-by-zero in adjustment.

Edge case if totalBalance somehow reaches 0 before adjustment.

-    final Decimal adjustmentFactor = Decimal.parse(
-      (targetFinalBalance / totalBalance).toString(),
-    );
+    final Decimal adjustmentFactor = Decimal.parse(
+      (totalBalance == 0 ? 0 : (targetFinalBalance / totalBalance)).toString(),
+    );
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between fcd85e3 and 18dfeee.

⛔ Files ignored due to path filters (3)
  • app_theme/pubspec.lock is excluded by !**/*.lock
  • packages/komodo_ui_kit/pubspec.lock is excluded by !**/*.lock
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (50)
  • app_theme/pubspec.yaml (1 hunks)
  • lib/bloc/app_bloc_root.dart (2 hunks)
  • lib/bloc/bridge_form/bridge_bloc.dart (20 hunks)
  • lib/bloc/cex_market_data/mockup/generate_demo_data.dart (5 hunks)
  • lib/bloc/cex_market_data/mockup/generator.dart (2 hunks)
  • lib/bloc/cex_market_data/mockup/mock_portfolio_growth_repository.dart (1 hunks)
  • lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart (5 hunks)
  • lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_repository.dart (11 hunks)
  • lib/bloc/cex_market_data/price_chart/price_chart_bloc.dart (6 hunks)
  • lib/bloc/cex_market_data/profit_loss/demo_profit_loss_repository.dart (1 hunks)
  • lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart (6 hunks)
  • lib/bloc/cex_market_data/profit_loss/profit_loss_calculator.dart (10 hunks)
  • lib/bloc/cex_market_data/profit_loss/profit_loss_repository.dart (5 hunks)
  • lib/bloc/coins_bloc/coins_repo.dart (2 hunks)
  • lib/bloc/coins_bloc/coins_state.dart (2 hunks)
  • lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart (8 hunks)
  • lib/bloc/market_maker_bot/market_maker_order_list/market_maker_bot_order_list_repository.dart (3 hunks)
  • lib/bloc/market_maker_bot/market_maker_trade_form/market_maker_trade_form_state.dart (1 hunks)
  • lib/bloc/nft_transactions/nft_txn_repository.dart (1 hunks)
  • lib/main.dart (1 hunks)
  • lib/model/cex_price.dart (1 hunks)
  • lib/shared/utils/balances_formatter.dart (2 hunks)
  • lib/shared/utils/extensions/sdk_extensions.dart (3 hunks)
  • lib/views/dex/dex_helpers.dart (10 hunks)
  • lib/views/dex/orderbook/orderbook_table.dart (1 hunks)
  • lib/views/dex/simple/form/dex_fiat_amount.dart (2 hunks)
  • lib/views/dex/simple/form/exchange_info/dex_compared_to_cex.dart (1 hunks)
  • lib/views/fiat/fiat_inputs.dart (6 hunks)
  • lib/views/market_maker_bot/market_maker_bot_form.dart (7 hunks)
  • lib/views/wallet/coin_details/coin_details_info/charts/coin_sparkline.dart (1 hunks)
  • lib/views/wallet/wallet_page/common/asset_list_item_desktop.dart (4 hunks)
  • lib/views/wallet/wallet_page/common/grouped_asset_ticker_item.dart (4 hunks)
  • macos/Flutter/GeneratedPluginRegistrant.swift (1 hunks)
  • packages/komodo_persistence_layer/pubspec.yaml (1 hunks)
  • packages/komodo_ui_kit/pubspec.yaml (1 hunks)
  • pubspec.yaml (2 hunks)
  • sdk (1 hunks)
  • test_units/main.dart (4 hunks)
  • test_units/tests/cex_market_data/binance_repository_test.dart (0 hunks)
  • test_units/tests/cex_market_data/charts_test.dart (0 hunks)
  • test_units/tests/cex_market_data/generate_demo_data_test.dart (0 hunks)
  • test_units/tests/cex_market_data/mocks/mock_binance_provider.dart (0 hunks)
  • test_units/tests/cex_market_data/mocks/mock_failing_binance_provider.dart (0 hunks)
  • test_units/tests/cex_market_data/profit_loss_repository_test.dart (0 hunks)
  • test_units/tests/cex_market_data/transaction_generation.dart (0 hunks)
  • test_units/tests/helpers/total_24_change_test.dart (1 hunks)
  • test_units/tests/helpers/total_fee_test.dart (1 hunks)
  • test_units/tests/utils/get_fiat_amount_tests.dart (1 hunks)
  • test_units/tests/utils/get_usd_balance_test.dart (2 hunks)
  • test_units/tests/utils/test_util.dart (0 hunks)
💤 Files with no reviewable changes (8)
  • test_units/tests/cex_market_data/generate_demo_data_test.dart
  • test_units/tests/cex_market_data/binance_repository_test.dart
  • test_units/tests/cex_market_data/charts_test.dart
  • test_units/tests/cex_market_data/profit_loss_repository_test.dart
  • test_units/tests/cex_market_data/transaction_generation.dart
  • test_units/tests/cex_market_data/mocks/mock_binance_provider.dart
  • test_units/tests/cex_market_data/mocks/mock_failing_binance_provider.dart
  • test_units/tests/utils/test_util.dart
🧰 Additional context used
🧠 Learnings (5)
📚 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/shared/utils/balances_formatter.dart
  • lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart
  • lib/bloc/bridge_form/bridge_bloc.dart
  • lib/views/fiat/fiat_inputs.dart
  • lib/bloc/cex_market_data/price_chart/price_chart_bloc.dart
  • lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart
  • lib/views/wallet/wallet_page/common/grouped_asset_ticker_item.dart
  • lib/bloc/app_bloc_root.dart
  • lib/bloc/cex_market_data/mockup/generator.dart
  • lib/bloc/cex_market_data/profit_loss/demo_profit_loss_repository.dart
  • lib/views/wallet/wallet_page/common/asset_list_item_desktop.dart
  • pubspec.yaml
  • lib/bloc/cex_market_data/mockup/mock_portfolio_growth_repository.dart
  • lib/bloc/cex_market_data/profit_loss/profit_loss_repository.dart
📚 Learning: 2025-05-08T19:05:13.083Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2611
File: test_units/tests/system_health/http_head_time_provider_test.dart:8-24
Timestamp: 2025-05-08T19:05:13.083Z
Learning: In the Komodo Wallet project, test functions are defined in individual files under `test_units/tests/` but are executed through the central entry point in `test_units/main.dart`, so individual test files don't need their own `main()` function.

Applied to files:

  • test_units/tests/utils/get_usd_balance_test.dart
  • test_units/main.dart
  • test_units/tests/utils/get_fiat_amount_tests.dart
  • test_units/tests/helpers/total_fee_test.dart
📚 Learning: 2025-05-08T19:07:13.442Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2611
File: test_units/tests/system_health/system_clock_repository_test.dart:7-8
Timestamp: 2025-05-08T19:07:13.442Z
Learning: In the Komodo Wallet project, test files are structured to define test functions that are called from a central test runner in test_units/main.dart, rather than each test file having its own main() function.

Applied to files:

  • test_units/tests/utils/get_usd_balance_test.dart
  • test_units/main.dart
  • test_units/tests/utils/get_fiat_amount_tests.dart
  • test_units/tests/helpers/total_fee_test.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/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart
  • lib/views/fiat/fiat_inputs.dart
  • lib/bloc/cex_market_data/price_chart/price_chart_bloc.dart
  • lib/bloc/cex_market_data/portfolio_growth/portfolio_growth_bloc.dart
  • lib/views/wallet/wallet_page/common/grouped_asset_ticker_item.dart
  • lib/views/wallet/wallet_page/common/asset_list_item_desktop.dart
📚 Learning: 2025-07-23T09:31:17.738Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2976
File: lib/bloc/coins_manager/coins_manager_bloc.dart:82-83
Timestamp: 2025-07-23T09:31:17.738Z
Learning: In the CoinsManagerBloc, `state.selectedCoins` is used separately from `state.coins` to indicate whether a coin is enabled or not in the UI. The order of Set deduplication when merging coin lists doesn't affect UI behavior because selection state is tracked independently from the coin list used for filtering and sorting.

Applied to files:

  • lib/views/market_maker_bot/market_maker_bot_form.dart
🪛 GitHub Actions: takenagain is validating code guidelines 🚀
lib/main.dart

[error] 557-557: The method 'SecureStorageService' isn't defined for the type '_ConfigureDialogState'

sdk

[error] 6-6: Target of URI doesn't exist: 'package:mocktail/mocktail.dart'.


[error] 11-11: Missing concrete implementations of abstract class IBinanceProvider.fetch24hrTicker, IBinanceProvider.fetchExchangeInfo, IBinanceProvider.fetchExchangeInfoReduced, and IBinanceProvider.fetchKlines.


[error] 6-6: Target of URI doesn't exist: 'package:mocktail/mocktail.dart'.


[error] 2-2: Target of URI doesn't exist: 'package:komodo_symbol_converter/komodo_symbol_converter.dart'.


[error] 6-6: Target of URI doesn't exist: 'package:kdf_sdk_example/main.dart'.


[warning] 97-97: JsonSerializable.new annotation can only be used on classes.


[warning] 1-1: include_file_not_found: include file 'package:very_good_analysis/analysis_options.6.0.0.yaml' can't be found.

⏰ 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). (3)
  • GitHub Check: Build Desktop (windows)
  • GitHub Check: Build Mobile (Android)
  • GitHub Check: build-android-docker

- Remove local path overrides from pubspec.yaml
- Add pubspec_overrides.yaml with ./sdk package paths
cursor[bot]

This comment was marked as outdated.

@CharlVS CharlVS requested review from CharlVS and DeckerSU August 27, 2025 17:02
@cursor
Copy link
Copy Markdown

cursor bot commented Aug 27, 2025

🚨 Bugbot Trial Expired

Your Bugbot trial has expired. Please purchase a license in the Cursor dashboard to continue using Bugbot.

@DeckerSU
Copy link
Copy Markdown
Contributor

Under MacOS build I have the following error:

[!] CocoaPods could not find compatible versions for pod "Firebase/Analytics":
                 In snapshot (Podfile.lock):
                   Firebase/Analytics (= 11.10.0)

                 In Podfile:
                   firebase_analytics (from `Flutter/ephemeral/.symlinks/plugins/firebase_analytics/macos`) was resolved to 11.6.0, which depends on
                     Firebase/Analytics (= 11.15.0)


               You have either:
                * out-of-date source repos which you can update with `pod repo update` or with `pod install --repo-update`.
                * changed the constraints of dependency `Firebase/Analytics` inside your development pod `firebase_analytics`.
                  You should run `pod update Firebase/Analytics` to apply changes you've made.

I guess we should update Firebase/Analytics and commit changes to repo, with something like:

cd macos
pod repo update
pod update Firebase/Analytics
pod install
git add Podfile.lock
git commit -m "chore(macos): bump Firebase/Analytics to 11.15.0"
git push

As a result Podfile.lock will be updated.

cursor[bot]

This comment was marked as outdated.

@CharlVS CharlVS added the QA Ready for QA Testing label Aug 28, 2025
Copy link
Copy Markdown
Contributor

@DeckerSU DeckerSU left a comment

Choose a reason for hiding this comment

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

LGTM in terms of builds. I can confirm that all build pipelines (Linux, Windows, macOS) have passed successfully after f26f107. However, this review does not cover the SDK changes, balance display logic, etc.

@DeckerSU
Copy link
Copy Markdown
Contributor

Also about f092ad5 ... May be I am wrong, but:

We can see that a variable _pollSubscription of type StreamSubscription is declared. There are several calls to cancel the subscription, e.g. _pollSubscription?.cancel(), in methods like StartPeriodicPolling, StopPeriodicPolling, and close().

However, _pollSubscription is never actually assigned a value. The polling stream is started using emit.forEach(...), which does not return a StreamSubscription that can be stored. As a result, _pollSubscription always remains null, so calling _pollSubscription?.cancel() has no effect.

- update wallet overview comments
- simplify total balance data source and calculations
- use valuegetter for copyWith function
@takenagain
Copy link
Copy Markdown
Contributor Author

We can see that a variable _pollSubscription of type StreamSubscription is declared. There are several calls to cancel the subscription, e.g. _pollSubscription?.cancel(), in methods like StartPeriodicPolling, StopPeriodicPolling, and close().

However, _pollSubscription is never actually assigned a value. The polling stream is started using emit.forEach(...), which does not return a StreamSubscription that can be stored. As a result, _pollSubscription always remains null, so calling _pollSubscription?.cancel() has no effect.

Thanks for having a look and flagging the unused subscription! I've replaced the unused StreamSubscription with a simpler Timer approach in 552b7e9, and cleaned up the remaining transitional refactoring remnants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA Ready for QA Testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants