Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

chore(release): v0.9.0 - HD support, Unified codebase, Custom tokens, and SDK migration #2526

Open
wants to merge 61 commits into
base: main
Choose a base branch
from

Conversation

CharlVS
Copy link
Member

@CharlVS CharlVS commented Feb 11, 2025

TODO

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced a new GitHub Actions workflow for code coverage reporting.
    • Added a GitHub Actions workflow for installing Flutter dependencies.
    • Implemented a new GitHub Actions workflow for generating assets.
    • Introduced a GitHub Actions workflow for validating builds.
    • Enhanced Firebase Hosting deployment workflows for better modularity.
    • Added support for generating unit test coverage reports.
    • Introduced a new interface for managing custom token imports.
    • Added a new CoinAddressesBloc for managing coin addresses.
    • Introduced a CustomTokenImportBloc to handle custom token imports.
    • Added a FiatFormBloc for managing fiat on-ramp forms.
    • Implemented a new GitHub Action for setting up Android build environments.
    • Added a new GitHub Action for setting up iOS build environments.
    • Introduced a new GitHub Action for setting up Linux build environments.
    • Added a new GitHub Action for setting up macOS build environments.
    • Introduced a new GitHub Action for setting up Windows build environments.
    • Launched a new workflow for building desktop applications across platforms.
    • Launched a new workflow for building mobile applications for iOS and Android.
  • Improvements

    • Streamlined workflows by utilizing custom actions for asset generation and build validation.
    • Enhanced documentation for build configuration and running instructions.
    • Improved error handling and clarity in various workflows and actions.
    • Updated translation strings for better user experience.
    • Enhanced the WithdrawFormBloc to support new asset and SDK-based operations.
    • Refactored MarketMakerBotBloc for better event handling and integration with new dependencies.
    • Updated Android and iOS build configurations to support newer SDK versions.
    • Updated the Android SDK and NDK versions in the build configuration files.
    • Enhanced the BanxaFiatProvider and RampFiatProvider for better type safety and error handling.
  • Fixes

    • Resolved issues related to the handling of unsupported coins and improved validation logic.
    • Fixed bugs related to the management of coin addresses and transaction history retrieval.
    • Improved error handling in the NftWithdrawBloc and MarketMakerBotBloc.
    • Enhanced error logging and handling in the BanxaFiatProvider and RampFiatProvider.

smk762 and others added 30 commits November 8, 2024 23:22
* CI self-hosted config test #1

* Fix incorrect duplicated job ID

* CI test #2

* CI Test #3

Results of test #2: integration tests work when using GitHub-hosted Actions latest ubuntu

* CI Test #4

* Test unit/integration tests on hosted runner

Test unit/integration tests on the hosted runner to determine if the cause of the failing tests is related to differences between self-hosted and hosted environment.

* Update CI conditional statements of self-hosted runners

Update CI conditional statements that referenced variables only applicable to self-hosted runners. Change to the equivalent for hosted runners.

* Remove very_good_analysis linting dependency

Flutter is not fetching dependencies for sub-packages in the `packages` directory, so the very_good_analysis files are missing until `flutter pub get` is ran for each package

* Add composite actions for asset generation and build validation

* Remove chromedriver from unit tests workflow

* Add missing `shell` property to the `generate-assets` action

Also remove the default value, and unset the environment variable if it does not exist

* Delete the `build/` directory between builds to clear AssetManifest.bin

Rebuilds do not appear to consistently update AssetManifest.bin, even if there are new assets generated between builds (icons, coin configs)

* Add code coverage report as artefact upload

Temporary until link with 3rd-party sources like CodeCov is setup

* Only fail the coverage step if the report does not exist

The flutter test command failing or passing should not affect the test coverage step success/failure

* Fix profit/loss unit tests

Binance closing price introduces some variability in the calculated profit/loss that has to be accounted for. The margin of error can be reduced by improved price and timestamp matching, but this should suffice for now

* Fix code coverage generation action

I forgot the spaces in the bash condition :(. It should be "if [ ... ]; then"

* Revert to using `strings` before `grep` to be platform independent

macOS handles binary files differently to Linux, meaning that `grep -i ...` works with binary files on Linux systems, but not macOS

* Remove trailing spaces in zip generation command

Trailing spaces after the backslash in a multi-line command causes it to fail with "command not found"

* Prefix artefact with runner name & move coverage step to end of ui tests

* Update job names and add descriptions

* Switch build_transformer over to the new sdk repo

* Remove old build_transformer package

* Revert changes to web/src/index.html

* Add index.html to .gitignore

* Move webpack JS files out of web/src

* Revert changes to index.html once more :(

* Untrack index.html & update build transformer package

* Use default GITHUB_TOKEN to make authenticated requests

* Add test case for failing binance requests

* demo data generator: check if coin is supported before fetching

* Add validation step for build_config.json

* Fix silently failing asset manifest validation check

* Add json validity checks for coin configs

* Update commit sha of build transformer package

* Use abstract repository class in generate demo data

* Install Chrome alongside chromedriver on linux for UI tests

* Skip generate demo test data unit tests

* Bump build transformer to dev branch

* Switch UI and unit test workflows back to self-hosted runners

* Restrict coverage report to linux-based runners

* Improve npm build cross-platform compatibility

'&&' is invalid in Windows PowerShell. ';' works on Windows, macOS, and Linux

* Move Flutter and NodeJS installation to composite action

* Add github token to UI and unit test steps

Seeing 403 responses on self-hosted runners

* Fix UI integration tests on self-hosted macos

* Export logs for safaridriver in workflows

Use the --diagnose flag when starting safaridriver

* Fix failing restore wallet integration test

* Add verbose flag to integration test runner

* Fix issues with pumpAndSettle timeout

Add pumpNFrames and use that instead. This is based on the recommendation in the docs: https://api.flutter.dev/flutter/flutter_test/WidgetTester/pumpAndSettle.html

* Fix missing balance overview and back button

* Fix wallet & wallet manager integration tests

* Fix maker & taker integration tests

* Fix misc integration tests

* Fix analyse warnings and remove support page test

* Pin chromedriver to the self-hosted runner version

116.0.5845.96 with path /opt/google/chrome/chrome

* Use mock binance repository for unit testing

Using the live Binance endpoints resulted in some variance of the closing price & final calculation that caused the unit tests to fail on occasion.

* Add note about the possibility of 403 error

---------

Co-authored-by: CharlVS <[email protected]>
Co-authored-by: Francois <[email protected]>
* add enable test coins to settings

* test coins hide if toggled off in settings
…uilds (#163)

* Update Flutter to 3.24.3 (stable)

Only web build works with this configuration

* Bump Flutter version in workflows to `3.24.x`

* Remove `desktop_webview_window` dependency

* Switch to in-app webview in popup-window

* Upgrade `file_picker` to 8.1.2 to fix ios&macos build error

win32 v5 removes references to deprecated APIs, which

jonataslaw/get_cli#263

* Upgrade `url_launcher` to 6.3.0 to fix iOS build error

* Skip web defi fetch step if target is iOS

* Bump CI Flutter version to `3.24.x`
* router fix uri query parameters were lost at parse

* router add dexroute parameters

* router dex state notifyListeners for new fields

* router dex process order_type param

* router dex clean params without notifying

* router dex maker process ticker and amount params

* router dex fix typo

* dex TakerSetSellCoin event add setOnlyIfNotSet

* router dex maker consumes only their own params

* router dex allow order_type being maker by default

* router dex taker process from/to currencies

* dex repository waitOrderbookAvailability function

* router dex form wait for orderbooks before process

* router dex taker process from_amount

* router dex code cleanup

* mobile/widget layout taker form dropdown position

* maker form parse error check
* change extension to `.gz` on native platforms

* add single-file zip archive support

credit for zip.dart goes to gpt-4o
* Update Flutter to 3.24.3 (stable)

Only web build works with this configuration

* Bump Flutter version in workflows to `3.24.x`

* Remove `desktop_webview_window` dependency

* Switch to in-app webview in popup-window

* Upgrade `file_picker` to 8.1.2 to fix ios&macos build error

win32 v5 removes references to deprecated APIs, which

jonataslaw/get_cli#263

* Upgrade `url_launcher` to 6.3.0 to fix iOS build error

* Skip web defi fetch step if target is iOS

* Bump CI Flutter version to `3.24.x`

* Fix mobile coin details buttons layout

- Also fix Bitrefill button in preparation for cross-platform fiat onramp in a similar fashion

* Add fullscreen in-app-webview for native platforms

* Close the browser if redirected to web app

This is a failure condition for the `checkout_status_redirect.html` page

* Move payment status events to BLoC

The async onConsole/onMessage callback used by the `flutter_inappwebview` package is incompatible with the previous watcher implementation.

* Migrate bitrefill provider & watcher to package:web

* refactor fullscreen webview to webview dialog

* add fiat onramp html page

fixes issues with reading `onmessage` and `window.console` from an iframe or another window

* apply patch to `web_support.js` to fix web callbacks

from pichillilorenzo/flutter_inappwebview#2058

* migrate remaining fiat & bitrefill html references to package:web

* WIP: add initial fiat onramp bloc implementation

* fix arb merge issue: add arb to currency class

* fix type conversion bugs and add more error logging

add stacktrace to logs when in kDebug mode

* replace onCheckoutComplete callback with BlocListener

* improve form state management

and add default payment methods list for initial user input

* fix cross-platform compilation with conditional imports

package:web and js_interop only work on web, so use conditional exports

* fix cocoapods build warnings

* use url instead of proxy page on native platforms

only web requires the proxy page because of CORS restrictions. onConsole, and onMessage works on native platforms

* fix hive runtime init exception

`Hive.initFlutter` failed on macOS and appears to be a web-specific function that produces an exception on native platforms

* add error parsing for banxa order creation

* localize fiat error and popup messages

* fix status message parsing in wrapper html page

* move getCoinAddress to coins bloc

* update testing and setup docs

add example launch.json and iOS crash logs location
add notes about linux setup

* fix fiat amount injecting decimals

removed unnecessary string `error` field, since the status fields suffice

* fix onramp error on linux & banxa parsing bug

* fix fiat form overlapping issues on mobile

- use autoscrolltext, expand, and align
- fix fiat icon errors when scrolling quickly
- fix webview platform check

* fix flutter analyze warnings

* add fiat onramp form integration test

* bump build transformer package commit

* improve status parsing on native platforms

- parsing errors from escaped json strings on Windows
- re-enable banxa order status watching

* fix coins bloc and Hive init race condition on macos

Runtime updates Hive boxes have to be initialised on native platforms before coins bloc executes, which was not happening consistently across all platforms

* move confirmation prompt behind conditional import

`web` and `js_interop` package imports do not compile on native platforms, so they have to be hidden behind conditional imports to allow for cross-platform support
* kdf show_priv_key api

* display private keys

* private keys QR code dialog

* private keys list title

* private key dialog width

* private key share instead of copy

* Revert "private key share instead of copy"

This reverts commit a623444ea75e4d074419bcc9689585ca819beceb.

* private key clipboard warning
* Fallback to remote images CDN for missing coin icons

* Add coin icon fallback for all references to icons

Add coin icon fallback for other widgets referencing the local image icons. Now all coin icons in the app share the same widget.

* Prevent flicker for fallback coin icons

Prevent flicker for fallback coin icons by caching the status of the existence on the CDN.

* Further coin icon fallback bug fixes
… created (#184)

* fix trading bot tab order

* fix trading bot history onclick event

* show estimate trade volume while creating new order
* add system clock repository with fallback urls

* add directly connected peers rpc as fallback

* improve utc parsing and request exception handling

* revert changes to build_config
* add komodo_defi_sdk to dependencies

update required dependencies, and fix build errors

* WIP: replace mm2 classes with defi sdk

* replace kdf rpc status checks with isSignedIn

The new SDK abstracts away the KDF functionality behind an authentication class, so the previous status checks are not possible, and the isSignedIn is the closest alternative without removing the logic entirely (possibly breaking change)

* bump kdf version

* load coin assets from sdk package

komodo_defi_framework package already downloads the configs and icons, so load them from the external package instead of redownloading and loading the same assets twice

* ci: update paths in validate action

* replace dynamic index.html with static version

the drawbacks of bundling with webpack outweighed the negligible size reduction in the kdf wasm files, so it was dropped in favour of a static index.html in the sdk

* fix failing unit tests

caused by the `rational` and `decimal` package updates

* update docs

remove nodejs, api update script, and add fvm as flutter installation alternative

* fix breaking changes to rpc method return types

sdk returns different types which have to be accounted for, sometimes on a per-rpc basis

* fix validation warnings

* WIP: fix integration tests

temporarily disable suspended assets test. The `coins_config.json` is no longer editable as it is obtained from an external package, so alternative means of invalidating the electrum URLs for an asset are required

* re-enable suspended assets test on chrome

* block electrum urls in chrome for suspended assets test

* update logs location in ui-tests workflow

* fix taker order integration test

* change from sdk to komodo_defi_framework

* fix debug utils & withdraw test

* remove driver start step from ui test workflow

browser driver startup is now handled by integration test runner

* improve driver management & fix `test_withdraw` test

add and refactor integration test utility functions

* add pub get flag

flutter drive runs `flutter pub get` before each test by default, which slows down the current implementation, which runs each set of tests independently

* add profile mode step to ui-tests-on-pr

* add verbose logging to integration tests steps

some errors, like pumpAndSettle timeouts, do not produce useful stacktraces, so adding print statements is necessary unless we run the integration tests in debug mode (not recommended)

* fix cex_prices integration test

* fix theme switching on web

* fix nft and trezor RPC call type errors

type conversions missed during initial migration, but caught by integration tests

* migrate web file_loader to js_interop and package:web

* change file loader to conditional import structure

js_interop causes builds to fail once again

* fix seed file upload & add keep-running flag

safaridriver logs do not include console logs, so we have to keep the browser window open to read console logs in the event of failure

* fix intermittent test failures

* bump sdk version & add debug statements

* increase flutter drive timeout & remove tests.dart

group structure is better suited to the new integration_test format rather than flutter_drive. `await app.main` fails after the second test when in the same group

* fix merge error

* fix validation warnings & flaky dex taker test

* add port option & fix flaky dex ui test steps

* tests: add longer wait after taker/maker confirmation button click

* fix misc integration test errors when switching theme

* fix trading bot order count in tab bar

* fix analyze warnings and increase tab refresh rate

* restore missing error check

throw exception if error response is received from API. Market maker bot was failing to start/stop after this was removed as part of the sdk integration

* fix trading bot & dex order list onClick events

* fix merge issues & trade bot empty list check

list null = empty

* fix best orders and connected peers parsing

v2 uses orderaddress object for address field instead of string
Add coin icon fallback for other widgets referencing the local image icons. Now all coin icons in the app share the same widget.

Fallback to remote images CDN for missing coin icons

Further coin icon fallback bug fixes

Prevent flicker for fallback coin icons

Prevent flicker for fallback coin icons by caching the status of the existence on the CDN.
* fix zero balance issue by fallback to my_balance

* update kdf wasm version

* Remove undefined KDF build configs

* deploy preview on PR to main

* max_taker_vol fallback to my_balance

* ci branches add release/*

* max_taker_vol balance parse rational

---------

Co-authored-by: CharlVS <[email protected]>
Co-authored-by: Francois <[email protected]>
* add support for map in hd balance response

* bump kdf release hash and wasm checksum
* add komodo_defi_sdk to dependencies

update required dependencies, and fix build errors

* WIP: replace mm2 classes with defi sdk

* replace kdf rpc status checks with isSignedIn

The new SDK abstracts away the KDF functionality behind an authentication class, so the previous status checks are not possible, and the isSignedIn is the closest alternative without removing the logic entirely (possibly breaking change)

* bump kdf version

* load coin assets from sdk package

komodo_defi_framework package already downloads the configs and icons, so load them from the external package instead of redownloading and loading the same assets twice

* ci: update paths in validate action

* replace dynamic index.html with static version

the drawbacks of bundling with webpack outweighed the negligible size reduction in the kdf wasm files, so it was dropped in favour of a static index.html in the sdk

* fix failing unit tests

caused by the `rational` and `decimal` package updates

* update docs

remove nodejs, api update script, and add fvm as flutter installation alternative

* fix breaking changes to rpc method return types

sdk returns different types which have to be accounted for, sometimes on a per-rpc basis

* fix validation warnings

* WIP: fix integration tests

temporarily disable suspended assets test. The `coins_config.json` is no longer editable as it is obtained from an external package, so alternative means of invalidating the electrum URLs for an asset are required

* re-enable suspended assets test on chrome

* block electrum urls in chrome for suspended assets test

* update logs location in ui-tests workflow

* fix taker order integration test

* coin addresses bloc init

* coin addresses init list in coin details

* coin addresses separate status for creation

* create addresses cleanup

* change from sdk to komodo_defi_framework

* coin addresses realistic fake address

* coin addresses hide middle part of the address

* fix debug utils & withdraw test

* remove driver start step from ui test workflow

browser driver startup is now handled by integration test runner

* coin addresses improve ui

* coin addresses special exception type

* coin addresses copy button functionality

* coin addresses swap address tag

* coin addresses bloc hide zero balance

* coin addresses styling and hide 0 balance

* improve driver management & fix `test_withdraw` test

add and refactor integration test utility functions

* add pub get flag

flutter drive runs `flutter pub get` before each test by default, which slows down the current implementation, which runs each set of tests independently

* add profile mode step to ui-tests-on-pr

* add verbose logging to integration tests steps

some errors, like pumpAndSettle timeouts, do not produce useful stacktraces, so adding print statements is necessary unless we run the integration tests in debug mode (not recommended)

* coin addresses extract widget to file

* coin addresses QR code

* coin addresses extract widgets

* coin addresses mobile view init

* coin addresses extract widgets

* coin addresses improve mobile view layout

* coin addresses slightly larger swap address text

* coin addresses improve QR dialog

* coin addresses localize texts with existing keys

* coin addresses localize texts

* coin addresses localize texts

* coin addresses polish

* fix cex_prices integration test

* fix theme switching on web

* fix nft and trezor RPC call type errors

type conversions missed during initial migration, but caught by integration tests

* migrate web file_loader to js_interop and package:web

* change file loader to conditional import structure

js_interop causes builds to fail once again

* fix seed file upload & add keep-running flag

safaridriver logs do not include console logs, so we have to keep the browser window open to read console logs in the event of failure

* fix intermittent test failures

* bump sdk version & add debug statements

* increase flutter drive timeout & remove tests.dart

group structure is better suited to the new integration_test format rather than flutter_drive. `await app.main` fails after the second test when in the same group

* fix merge error

* fix validation warnings & flaky dex taker test

* add port option & fix flaky dex ui test steps

* tests: add longer wait after taker/maker confirmation button click

* fix misc integration test errors when switching theme

* fix trading bot order count in tab bar

* fix analyze warnings and increase tab refresh rate

* restore missing error check

throw exception if error response is received from API. Market maker bot was failing to start/stop after this was removed as part of the sdk integration

* fix trading bot & dex order list onClick events

* fix merge issues & trade bot empty list check

list null = empty

* fix best orders and connected peers parsing

v2 uses orderaddress object for address field instead of string

* replace auth methods with KomodoDefiSdk

* fix global variable initialization order and async issues

Migrating away from the global variables in blocs.dart will involve a considerable number of code changes, so leaving as-is for now

* use sdk in current wallet "bloc"

default values used for hasBackup

* move global bloc variables to repositoryprovider

async sdk/kdf initialization does not work with the current global variable structure, so moved the global repositories to repositoryprovider

* revert breaking wallet restore and import changes

* skip orders, and swaps RPCs when not logged in

also bump kdf sdk version

* fix login to existing wallet and remove onlogout deactivate

* catch and log uncaught (or async) errors

* sdk txs disable tests and mock repository

* sdk txs cex market data charts

* sdk txs transactions import change

* fix onlogout balance clearing

throw exceptions with stacktraces

* sdk txs use the new sdk Transaction model

* sdk txs use the new sdk Transaction model

* sdk txs use the new sdk Transaction model

* sdk txs use the new sdk Transaction model

* sdk txs change types import

* tests: use faucet for doc/marty if balance is insufficient

* fix maker form auth state listener

* sdk txs fee coin

* fix validation warnings and remove gha timeout at step

* sdk txs fetch from sdk

* coin details hide addresses if tx is selected

* sdk txs always show transactions if filled

* fix transactions missing one row during loading

* sdk txs getSdkAsset temp helper function

* sdk access added to CoinsRepo

* sdk replace max_maker_vol with sdk

* sdk getSdkPubkeys helper function

* add wallet metadata and disable unsupported features

- add `has_backup` and `WalletType` metadata fields to KdfUser
- disable delete wallet feature
-

* sdk addresses port

* replace current wallet stream with auth bloc

* rename coins "bloc" and move more repos to DI

* use new asset class for trezor coin activation

trezor activation responds to 'UserActionRequired' events, which are not propagated through asset manager

* migrate coins manager from legacy coins bloc

to repository and new coins bloc

* migrate legacy coins bloc to coins repostory

* remove runtime coin updates

`KomodoCoins` fetches coins from github cdn at runtime, so runtime updates are no longer necessary

* remove coin activation rpc models and migrate legacy coins bloc refs

* add activated coins metadata

* refactor: migrate fiat bloc from legacy coins bloc

* migrate remaining legacy coins bloc references to coins repo

* migrate current wallet references to auth bloc

* fix swap page validation issues

* fix market metrics and transaction history loading

* fix trading bot validation errors

coins bloc emits not refreshing the widgets at the bottom of the tree

* fix bridge page missing protocols

* fix coin activation error handling

* fix merge issues

* fix timestamp in unit test transaction generation

* sdk show pubkey addresses for all coins

* sdk addresses active swap address tag

* sdk addresses display balance

* sdk addresses polish balance display

* fix view seed and re-login flow

* fix coin activation via coins manager

activating coins were only displayed on the next balance refresh instead of immediately after activation

* add parent coin to conversion

* sdk addresses hide zero balances checkbox

* sdk addresses reload after create

* sdk remove coin addresses repository

* fix portfolio growth concurrent modification

* fix coin details tab controller error

* sdk getSdkAsset use findAssetsByTicker

* sdk addresses max 3 empty addresses

* sdk addresses split variables

* sdk txs add address

* sdk txs add address mobile

* sdk txs load 200

* fix sporadic activation in coins list

* add legacy wallet migration

* sdk asset getPubkeys

* sdk use assetsFromTicker

* pubspec upgrade

* coin details history -> lastTransactions

* coin details transactions better spacing

* coin details polish transactions section

* upgrade to flutter 3.27.0

* coin details redesign tx history section

* coin details redesign tx history section mobile

* fix new validation warnings from flutter upgrade

* fix trezor login

login does not work in debug mode because of an assert statement in the SDK

* bump flutter version in workflows

* fix macos and ios builds

* decouple balance fetching & coin activation

* sdk addresses use CantCreateNewAddressReason

* coin details remove createAddressAllowed field

* sdk localize CantCreateNewAddressReason texts

* trim unused methods from coin model

* fix coin balances persisting to another wallet login

* fix initial trezor coin activation list

* fix trezor coin add asset page

* call faucet on all addresses

should implement a more robust solution to HD wallet support for faucet

* improve login activated coins metadata storing

* upgrade sdk

---------

Co-authored-by: Francois <[email protected]>
@CharlVS CharlVS marked this pull request as ready for review February 17, 2025 01:58
@KomodoPlatform KomodoPlatform deleted a comment from coderabbitai bot Feb 17, 2025
Copy link

@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: 14

🧹 Nitpick comments (44)
lib/bloc/fiat/base_fiat_provider.dart (1)

214-214: Track the SDK migration task.

The TODO comment indicates pending work to migrate to SDK's ticker/formatted getters.

Would you like me to create an issue to track this SDK migration task?

lib/mm2/mm2.dart (3)

9-9: Avoid global singletons if multiple instances might be needed.
While having a single global mm2 instance can be convenient, it may reduce flexibility or complicate test scenarios if future needs require multiple instances of MM2 (e.g., multi-wallet contexts). Consider using a provider or a dependency injection approach for more modularity.


39-39: Consider removing or revisiting the 'Hack' comment.
Referencing a “Hack” can create confusion for future maintainers. If this workaround is still necessary, provide context or a TODO comment explaining its purpose and potential resolution path.


77-96: Revisit _assertPass override strategy.
Currently, _assertPass forces userpass to an empty string for all RPC requests. While it may be necessary for “noauth mode,” this could be error-prone if, in the future, any RPC request requires authentication. Consider adding a check or a parameter to conditionally set this.

lib/bloc/trezor_init_bloc/trezor_init_bloc.dart (4)

82-84: Log message references an outdated method name

The log message uses 'trezor_init_bloc => _loginToHiddenMode' but the method is _loginToTrezorWallet. Renaming this reference improves trace clarity and consistency.

-        path: 'trezor_init_bloc => _loginToHiddenMode',
+        path: 'trezor_init_bloc => _loginToTrezorWallet',

150-177: Centralize repeated logout flow for error conditions

Multiple code paths call _logout() upon encountering errors. Consolidating this pattern into a small helper could enhance readability and maintainability. Consider extracting an _onErrorAndLogout() method to handle error updates, emit states, and log out.


272-281: Avoid using hard-coded wallet name and password

Using walletName = 'My Trezor' and password = 'hidden-login' for all Trezor logins can pose security risks and might cause collisions if multiple Trezors share the same identifier. Consider allowing the user to specify or securely generate these values.


320-322: Rename method to indicate a boolean check

_checkAndHandleInvalidPin only returns a boolean rather than "handling" the invalid pin scenario. Rename it to something like _isInvalidPin for clarity, or incorporate actual handling logic for the invalid pin case.

lib/bloc/transaction_history/transaction_history_repo.dart (3)

6-7: Clarify the exception circumstances in doc comments.
Currently, the doc states this exception is thrown if fetching fails, but the method only throws when the asset is null. You might align the documentation with the actual code behavior or throw the exception in other failure scenarios as well.


8-11: Consider returning an empty list instead of null.
Declaring Future<List<Transaction>?> suggests nullable lists. Returning an empty list supports consistent handling downstream, minimizing null checks.


13-19: Use a 'final' class or mark it abstract if unextended.
If you do not intend to subclass or extend SdkTransactionHistoryRepository, consider declaring it final to communicate intent and enable certain compiler optimizations.

lib/blocs/current_wallet_bloc.dart (5)

15-15: Good transition to _kdfSdk.
Switching from local repos to the SDK centralizes wallet operations and aligns with the overarching migration strategy.

Also applies to: 20-20, 22-22


27-27: Check necessity of ignore comment.
The ignore comment for getters and setters might not be needed if you truly require the setter for other code. Consider removing if not essential.


34-34: Consider disposing of internal streams.
If this bloc holds streams or subscriptions from _kdfSdk, ensure they are canceled here to prevent potential memory leaks.


37-40: Unimplemented password update.
Throwing an UnimplementedError is fine as a placeholder. Consider adding a TODO or referencing an issue to ensure this is addressed long-term.


99-100: Confirming backup in _kdfSdk.
If wallet.config.hasBackup is already true, no further action is taken. This is correct, but confirm the new approach is used consistently across the app for clarity.

Also applies to: 104-104

lib/bloc/custom_token_import/bloc/custom_token_import_bloc.dart (2)

26-49: Resetting form status.
Collecting EVM-supported coin types and clearing errors is a good approach. Validate that sorting by name meets all UI/UX expectations.


61-66: Address updates.
Straightforward assignment of address. Consider validating address format to prevent submission errors down the line.

lib/bloc/custom_token_import/data/custom_token_import_repository.dart (3)

13-16: Consider adding method-level documentation for clarity.

While the interface methods are straightforward, adding doc comments clarifying expected behavior, error conditions, and parameter usage can aid future maintainers.


124-147: Handle partial data gracefully in fetchTokenInfoFromApi.

If CoinGecko returns unexpected data structures (or if an error occurs), the function currently logs and returns null. Consider surfacing more context or rethrowing a custom exception to help troubleshoot.


153-179: Consider a map-based lookup instead of a switch.

The getNetworkApiName switch is thorough but verbose. A static map (e.g., Map<CoinSubClass, String>), combined with a fallback, can simplify maintenance.

lib/bloc/withdraw_form/withdraw_form_state.dart (2)

9-35: Document the new fields for better maintainability.

Properties like isMaxAmount, isCustomFee, and isIbcTransfer could benefit from doc comments describing how they’re used or the assumptions behind them.


158-166: Address the TODO placeholders.

The TODOs (usdFeePrice, usdAmountPrice, isFeePriceExpensive) hint at planned features or incomplete logic. Consider adding skeleton implementations or clarifying next steps to avoid shipping partial features.

lib/bloc/trezor_bloc/trezor_repo.dart (2)

39-42: Refactor constructor injection if more dependencies appear.

The constructor is already configuring Mm2ApiTrezor and KomodoDefiSdk. If new dependencies emerge, consider a builder or module-based injection pattern to keep the constructor clean and flexible.


139-142: Broaden isTrezorWallet checks if additional wallet types are introduced.

Currently, it compares against WalletType.trezor. If new Trezor subtypes or multi-wallet configurations appear, consider robust logic to detect Trezor capabilities.

lib/bloc/withdraw_form/withdraw_form_bloc.dart (2)

235-248: Edge case handling in _onMaxAmountEnabled.
Consider clarifying behavior if spendable balance is zero (currently it sets amount to null string if no balance).


426-433: Cancellation is partially implemented.
The TODO hints at a need for actual cancellation if supported by _sdk.

Do you want to implement logic to cancel an in-progress withdrawal? I can help generate a pattern if the SDK supports it.

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

80-92: disableCoin function swallows errors.
Consider returning or rethrowing errors if the caller needs to handle them.


173-197: sendRawTransaction
Returns a typed response. Consider including the actual error message in error instead of always 'null response' for more clarity.

lib/bloc/coins_bloc/coins_state.dart (2)

42-42: Address the TODO comment regarding SDK migration.

The comment indicates pending SDK migration work. Consider prioritizing this migration to maintain consistency with the rest of the codebase.

Would you like me to help create an issue to track the SDK migration task?


43-52: Consider refactoring price calculation for better maintainability.

The current implementation could be more concise using null-aware operators.

-  double? getUsdPriceByAmount(String amount, String coinAbbr) {
-    final Coin? coin = coins[coinAbbr];
-    final double? parsedAmount = double.tryParse(amount);
-    final double? usdPrice = coin?.usdPrice?.price;
-
-    if (coin == null || usdPrice == null || parsedAmount == null) {
-      return null;
-    }
-    return parsedAmount * usdPrice;
+  double? getUsdPriceByAmount(String amount, String coinAbbr) {
+    final double? parsedAmount = double.tryParse(amount);
+    final double? usdPrice = coins[coinAbbr]?.usdPrice?.price;
+    return (parsedAmount != null && usdPrice != null) 
+        ? parsedAmount * usdPrice 
+        : null;
   }
lib/bloc/coins_bloc/asset_coin_extension.dart (1)

51-52: Improve null safety in contract address getter.

The nested valueOrNull calls could lead to runtime errors if any intermediate value is null. Consider using a more defensive approach.

Apply this diff to improve null safety:

-  String? get contractAddress => protocol.config
-      .valueOrNull('protocol', 'protocol_data', 'contract_address');
+  String? get contractAddress {
+    final config = protocol.config;
+    if (config == null) return null;
+    final protocolData = config.valueOrNull('protocol', 'protocol_data');
+    if (protocolData == null) return null;
+    return protocolData.valueOrNull('contract_address');
+  }
lib/bloc/cex_market_data/mockup/generate_demo_data.dart (2)

134-135: Improve decimal conversion safety.

The conversion from double to Decimal using toString() could lead to precision loss or overflow. Consider using a safer conversion method.

Apply this diff to improve decimal conversion:

-    final Decimal adjustmentFactor =
-        Decimal.parse((targetFinalBalance / totalBalance).toString());
+    final Decimal adjustmentFactor = targetFinalBalance == 0 || totalBalance == 0
+        ? Decimal.zero
+        : (Decimal.parse(targetFinalBalance.toString()) /
+            Decimal.parse(totalBalance.toString()))
+            .toDecimal(scaleOnInfinitePrecision: 18);

211-212: Optimize wallet type check.

The wallet type check can be optimized by returning early if the wallet type is null.

Apply this diff to improve readability:

-  final WalletType? walletType = (await sdk.currentWallet())?.config.type;
-  if (walletType == null) return [];
+  final wallet = await sdk.currentWallet();
+  if (wallet == null) return [];
+  final walletType = wallet.config.type;
lib/bloc/coins_manager/coins_manager_bloc.dart (2)

113-115: Consider using async coin activation.

The coin activation/deactivation is performed synchronously which could block the UI thread. Consider using async methods instead.

Apply this diff to improve performance:

-        ? _coinsRepo.activateCoinsSync(selectedCoins)
-        : _coinsRepo.deactivateCoinsSync(selectedCoins);
+        ? _coinsRepo.activateCoins(selectedCoins)
+        : _coinsRepo.deactivateCoins(selectedCoins);

83-90: Optimize coin list reset.

The coin list reset could be optimized by avoiding unnecessary state emissions.

Apply this diff to improve performance:

-    emit(CoinsManagerState.initial(coins: [], action: event.action));
-    final List<Coin> coins = await _getOriginalCoinList(
-      _coinsRepo,
-      event.action,
-      _sdk,
-    )
-      ..sort((a, b) => a.abbr.compareTo(b.abbr));
-    emit(state.copyWith(coins: coins, action: event.action));
+    final List<Coin> coins = await _getOriginalCoinList(
+      _coinsRepo,
+      event.action,
+      _sdk,
+    );
+    coins.sort((a, b) => a.abbr.compareTo(b.abbr));
+    emit(CoinsManagerState.initial(coins: coins, action: event.action));
lib/bloc/auth_bloc/auth_bloc.dart (2)

90-98: Extract duplicate error handling logic.

The error handling logic is duplicated across multiple event handlers. Consider extracting it into a reusable method.

Apply this diff to improve code reuse:

+  void _handleError(String operation, String walletName, Object error, StackTrace trace) {
+    log(
+      'Failed to $operation wallet $walletName',
+      isError: true,
+      trace: trace,
+      path: 'auth_bloc -> $operation',
+    ).ignore();
+    emit(const AuthBlocState(mode: AuthorizeMode.noLogin));
+  }

-    } catch (e, s) {
-      log(
-        'Failed to login wallet ${event.wallet.name}',
-        isError: true,
-        trace: s,
-        path: 'auth_bloc -> onLogin',
-      ).ignore();
-      emit(const AuthBlocState(mode: AuthorizeMode.noLogin));
+    } catch (e, s) {
+      _handleError('login', event.wallet.name, e, s);
     }

-    } catch (e, s) {
-      log(
-        'Failed to register wallet ${event.wallet.name}',
-        isError: true,
-        trace: s,
-        path: 'auth_bloc -> onRegister',
-      ).ignore();
-      emit(const AuthBlocState(mode: AuthorizeMode.noLogin));
+    } catch (e, s) {
+      _handleError('register', event.wallet.name, e, s);
     }

-    } catch (e, s) {
-      log(
-        'Failed to restore existing wallet ${event.wallet.name}',
-        isError: true,
-        trace: s,
-        path: 'auth_bloc -> onRestore',
-      ).ignore();
-      emit(const AuthBlocState(mode: AuthorizeMode.noLogin));
+    } catch (e, s) {
+      _handleError('restore', event.wallet.name, e, s);
     }

Also applies to: 148-156, 210-218


221-228: Improve auth state changes subscription management.

The auth state changes subscription could leak if _listenToAuthStateChanges is called multiple times. Consider using a more robust subscription management approach.

Apply this diff to improve subscription management:

   void _listenToAuthStateChanges() {
-    _authorizationSubscription?.cancel();
-    _authorizationSubscription = _kdfSdk.auth.authStateChanges.listen((user) {
+    if (_authorizationSubscription != null) return;
+    _authorizationSubscription = _kdfSdk.auth.authStateChanges.listen(
+      (user) {
       final AuthorizeMode event =
           user != null ? AuthorizeMode.logIn : AuthorizeMode.noLogin;
       add(AuthModeChanged(mode: event, currentUser: user));
-    });
+      },
+      onError: (error, stackTrace) {
+        log(
+          'Auth state changes stream error',
+          isError: true,
+          trace: stackTrace,
+          path: 'auth_bloc -> _listenToAuthStateChanges',
+        ).ignore();
+      },
+    );
   }
lib/bloc/nft_withdraw/nft_withdraw_bloc.dart (3)

43-45: Consider reordering field declarations.

The field declarations could be reordered to group related fields together. Consider placing _kdfSdk and _coinsRepository next to each other since they are both repository/SDK dependencies.

Apply this diff to improve readability:

-  final NftWithdrawRepo _repo;
-  final KomodoDefiSdk _kdfSdk;
-  final CoinsRepo _coinsRepository;
+  final KomodoDefiSdk _kdfSdk;
+  final CoinsRepo _coinsRepository;
+  final NftWithdrawRepo _repo;

258-274: Enhance error handling in address conversion.

The error handling in _onConvertAddress could be more specific. Consider catching specific exceptions and providing more descriptive error messages.

Apply this diff to improve error handling:

   try {
     final subclass = state.nft.parentCoin.type.toCoinSubClass();
     final result = await _kdfSdk.client.rpc.address.convertAddress(
       from: state.address,
       coin: subclass.ticker,
       toFormat: AddressFormat.fromCoinSubClass(subclass),
     );
     add(NftWithdrawAddressChanged(result.address));
-  } catch (e) {
+  } catch (e) when (e is ArgumentError) {
+    emit(
+      state.copyWith(
+        address: () => '',
+        addressError: () => TextError(error: 'Invalid address format: ${e.message}'),
+      ),
+    );
+  } catch (e) when (e is FormatException) {
+    emit(
+      state.copyWith(
+        address: () => '',
+        addressError: () => TextError(error: 'Address conversion failed: ${e.message}'),
+      ),
+    );
+  } catch (e) {
     emit(
       state.copyWith(
         address: () => '',
         addressError: () => TextError(error: e.toString()),
       ),
     );
   }

276-282: Add error handling to coin activation.

The _activateParentCoinIfNeeded method should include error handling to gracefully handle activation failures.

Apply this diff to add error handling:

   Future<void> _activateParentCoinIfNeeded(NftToken nft) async {
     final parentCoin = state.nft.parentCoin;
 
     if (!parentCoin.isActive) {
-      await _coinsRepository.activateCoinsSync([parentCoin]);
+      try {
+        await _coinsRepository.activateCoinsSync([parentCoin]);
+      } catch (e) {
+        log(
+          'Failed to activate parent coin: ${parentCoin.abbr}',
+          isError: true,
+        ).ignore();
+        throw StateError('Failed to activate parent coin: ${e.toString()}');
+      }
     }
   }
lib/bloc/coins_bloc/coins_bloc.dart (2)

59-62: Consider using a single timer for updates.

Multiple timers are used for different update operations. Consider consolidating them into a single timer that handles all periodic updates to reduce resource usage.

Apply this diff to consolidate timers:

-  StreamSubscription<Coin>? _enabledCoinsSubscription;
-  Timer? _updateBalancesTimer;
-  Timer? _updatePricesTimer;
-  Timer? _reActivateSuspendedTimer;
+  StreamSubscription<Coin>? _enabledCoinsSubscription;
+  Timer? _updateTimer;
+
+  static const _updateInterval = Duration(minutes: 1);
+  static const _reactivateInterval = Duration(seconds: 30);
+  DateTime _lastReactivateTime = DateTime.now();

Then update the timer initialization:

-    _updateBalancesTimer?.cancel();
-    _updateBalancesTimer = Timer.periodic(
-      const Duration(minutes: 1),
-      (timer) {
+    _updateTimer?.cancel();
+    _updateTimer = Timer.periodic(
+      _updateInterval,
+      (timer) {
+        final now = DateTime.now();
         add(CoinsBalancesRefreshed());
+        add(CoinsPricesUpdated());
+        
+        if (now.difference(_lastReactivateTime) >= _reactivateInterval) {
+          add(CoinsSuspendedReactivated());
+          _lastReactivateTime = now;
+        }
       },
     );
-
-    _reActivateSuspendedTimer?.cancel();
-    _reActivateSuspendedTimer = Timer.periodic(
-      const Duration(seconds: 30),
-      (_) => add(CoinsSuspendedReactivated()),
-    );

529-547: Improve reactivation retry logic.

The _reActivateSuspended method uses a fixed number of attempts. Consider implementing an exponential backoff strategy for retries.

Apply this diff to implement exponential backoff:

   Stream<List<Coin>> _reActivateSuspended(
     Emitter<CoinsState> emit, {
-    int attempts = 1,
+    int maxAttempts = 3,
+    Duration initialDelay = const Duration(seconds: 1),
   }) async* {
     final List<String> coinsToBeActivated = [];
+    Duration delay = initialDelay;
 
-    for (int i = 0; i < attempts; i++) {
+    for (int i = 0; i < maxAttempts; i++) {
       final List<String> suspended = state.walletCoins.values
           .where((coin) => coin.isSuspended)
           .map((coin) => coin.abbr)
           .toList();
 
       coinsToBeActivated.addAll(suspended);
       coinsToBeActivated.addAll(_getUnactivatedWalletCoins());
 
       if (coinsToBeActivated.isEmpty) return;
+      
+      if (i > 0) {
+        await Future.delayed(delay);
+        delay *= 2; // Exponential backoff
+      }
+      
       yield await _activateCoins(coinsToBeActivated, emit);
     }
   }
ios/Runner.xcodeproj/project.pbxproj (1)

416-416: Verify iOS deployment target compatibility.

The iOS deployment target has been updated to 15.6 across all configurations. This change:

  1. Aligns with the platform version in Podfile (15.5.0)
  2. May restrict the app's availability to older devices

Consider documenting this minimum version requirement in the project's README to inform developers and users.

Also applies to: 559-559, 594-594

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3b24f4b and e9abd03.

⛔ Files ignored due to path filters (4)
  • app_theme/pubspec.lock is excluded by !**/*.lock
  • assets/logo/not_found.png is excluded by !**/*.png
  • ios/Podfile.lock is excluded by !**/*.lock
  • lib/generated/codegen_loader.g.dart is excluded by !**/generated/**
📒 Files selected for processing (32)
  • analysis_options.yaml (1 hunks)
  • android/app/build.gradle (3 hunks)
  • app_theme/lib/src/dark/theme_global_dark.dart (7 hunks)
  • app_theme/lib/src/light/theme_global_light.dart (7 hunks)
  • app_theme/pubspec.yaml (1 hunks)
  • assets/translations/en.json (8 hunks)
  • ios/Podfile (1 hunks)
  • ios/Runner.xcodeproj/project.pbxproj (10 hunks)
  • ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme (2 hunks)
  • lib/bloc/analytics/analytics_repo.dart (1 hunks)
  • lib/bloc/auth_bloc/auth_bloc.dart (1 hunks)
  • lib/bloc/cex_market_data/mockup/generate_demo_data.dart (6 hunks)
  • lib/bloc/coins_bloc/asset_coin_extension.dart (1 hunks)
  • lib/bloc/coins_bloc/coins_bloc.dart (1 hunks)
  • lib/bloc/coins_bloc/coins_event.dart (1 hunks)
  • lib/bloc/coins_bloc/coins_repo.dart (1 hunks)
  • lib/bloc/coins_bloc/coins_state.dart (1 hunks)
  • lib/bloc/coins_manager/coins_manager_bloc.dart (8 hunks)
  • lib/bloc/custom_token_import/bloc/custom_token_import_bloc.dart (1 hunks)
  • lib/bloc/custom_token_import/bloc/custom_token_import_event.dart (1 hunks)
  • lib/bloc/custom_token_import/bloc/custom_token_import_state.dart (1 hunks)
  • lib/bloc/custom_token_import/data/custom_token_import_repository.dart (1 hunks)
  • lib/bloc/fiat/base_fiat_provider.dart (5 hunks)
  • lib/bloc/nft_withdraw/nft_withdraw_bloc.dart (10 hunks)
  • lib/bloc/transaction_history/transaction_history_repo.dart (1 hunks)
  • lib/bloc/trezor_bloc/trezor_repo.dart (5 hunks)
  • lib/bloc/trezor_init_bloc/trezor_init_bloc.dart (5 hunks)
  • lib/bloc/withdraw_form/withdraw_form_bloc.dart (1 hunks)
  • lib/bloc/withdraw_form/withdraw_form_state.dart (1 hunks)
  • lib/blocs/current_wallet_bloc.dart (4 hunks)
  • lib/mm2/mm2.dart (1 hunks)
  • lib/mm2/mm2_api/mm2_api.dart (17 hunks)
✅ Files skipped from review due to trivial changes (1)
  • analysis_options.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
  • app_theme/lib/src/dark/theme_global_dark.dart
  • app_theme/lib/src/light/theme_global_light.dart
  • assets/translations/en.json
🔇 Additional comments (105)
app_theme/pubspec.yaml (3)

4-5: Cosmetic Homepage Comment Change
The change on line 4 is a minor formatting update (commenting the homepage key with adjusted spacing). This doesn’t affect functionality, so no further action is required.


7-7: SDK Version Constraint Update
The Dart SDK constraint has been updated to ">=3.6.0 <4.0.0". Please verify that all project dependencies and CI workflows support this updated range to avoid compatibility issues.


8-8: Flutter Version Constraint Update
The Flutter version is now set to "^3.29.0", which aligns with newer Flutter features and improvements. Ensure that all components of the project (and any dependent repositories) are compatible with this upgraded Flutter version.

ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme (2)

3-3:

❓ Verification inconclusive

Verify Xcode version compatibility.

The scheme has been upgraded from Xcode 13.0 to Xcode 15.1.0. Please ensure that all team members have compatible Xcode versions and that CI/CD pipelines are updated accordingly.


🌐 Web query:

What is the latest stable version of Xcode and when was Xcode 15.1.0 released?

💡 Result:

The latest stable version of Xcode as of February 17, 2025, is Xcode 16.2 (16C5032a), released on December 11, 2024[3][10]. This information aligns with Apple's typical annual release cycle for major Xcode updates.

Regarding Xcode 15.1, historical records show:

  • Xcode 15.1 (without ".0") was released on December 11, 2023[3][6]
  • The version numbering convention shows subsequent updates as 15.x rather than 15.x.x (e.g., next update was Xcode 15.2)[3][6]

There is no official record of an Xcode 15.1.0 release in Apple's documentation or third-party tracking sources[3][6][8]. The standard release was labeled simply as Xcode 15.1, following Apple's typical versioning pattern where minor updates increment the second digit without additional decimal points[3][6].

Citations:


Recheck Xcode Scheme Settings and GPU Validation Configuration

  • The scheme now sets LastUpgradeVersion to "1510", which appears to represent Xcode 15.1. However, official records indicate that Apple labels this release simply as “Xcode 15.1” (without an extra trailing zero). Please confirm that the version string in the scheme file matches your intended Xcode environment and update any related CI/CD or team configurations if necessary.

  • The addition of enableGPUValidationMode="1" in the LaunchAction is intended to facilitate GPU-related debugging during development. Verify that this setting is applied only in appropriate debug contexts to prevent unintended performance impacts.


51-51:

❓ Verification inconclusive

Document the reason for enabling GPU validation mode.

GPU validation mode has been enabled, which can help catch GPU-related issues during development but may impact performance. Please document:

  1. The specific GPU-related issues this aims to address
  2. Whether this should be enabled only for specific debugging scenarios

🌐 Web query:

What is GPU Frame Capture and Validation mode in Xcode and when should it be enabled?

💡 Result:

GPU Frame Capture in Xcode is a diagnostic tool that captures detailed information about how your Metal-based application interacts with the GPU during a specific frame. This includes recording all Metal API calls, shader executions, and memory operations for analysis[1][2][6]. Validation Mode refers to Metal API Validation, which checks for proper API usage and error conditions during development[10].

Key Features of GPU Frame Capture

  • Frame Analysis: Captures full frame data including draw calls, textures, and buffers[2][4]
  • Performance Insights: Reveals timing breakdowns per command buffer/encoder and shader execution costs[8][9]
  • Dependency Visualization: Shows relationships between rendering passes and resource dependencies[2]
  • Shader Debugging: Allows inspection of compiled shader code and intermediate rendering results[7]

When to Enable These Tools

Scenario GPU Frame Capture Validation Mode
Debugging rendering artifacts Required Recommended
Profiling GPU performance Required Optional
Developing Metal features Recommended Required
Release builds Disabled Disabled
Testing API correctness Optional Required

GPU Frame Capture should be enabled when:

  1. Diagnosing visual glitches or rendering errors[7]
  2. Optimizing GPU-bound applications[2][9]
  3. Analyzing complex render pipelines with multiple passes[2]
  4. Debugging shader-related issues[1][7]

Validation Mode should be enabled during:

  1. Initial development phases[10]
  2. Testing new Metal features
  3. Troubleshooting crashes/errors in Metal API usage
  4. Ensuring resource synchronization correctness

To configure these settings:

1. Edit scheme > Run > Options
2. Set "GPU Frame Capture" to Metal
3. Enable "Metal API Validation"[10][6]

The tools require macOS/iOS devices using Metal graphics and Xcode 12+[1][4]. Captures can be triggered via UI (camera icon) or programmatically using MTLCaptureManager[8]. Note that some advanced features like command-line captures aren't supported on iOS[4][5].

Citations:


Clarify GPU Validation Mode Usage in the Runner Scheme

The current setting enableGPUValidationMode = "1" enables Metal API Validation. This mode is intended to help catch GPU-related issues during development—such as incorrect resource management, synchronization errors, and shader problems—by validating Metal API calls. Because this validation can significantly impact performance, it is recommended only for debugging and development scenarios rather than for production or performance-critical builds.

Please update the documentation (or inline comments) to clearly state:

  • The GPU-related issues (e.g., improper API usage, resource synchronization, shader errors) that this mode helps detect.
  • That GPU validation should be enabled only during development or debugging sessions and disabled in release builds.
lib/bloc/fiat/base_fiat_provider.dart (5)

6-8: LGTM! Import changes enhance type safety.

The addition of dedicated model classes and window utilities improves code organization and type safety.

Also applies to: 10-10


19-44: LGTM! Method signatures enhance type safety and maintainability.

The transition from generic types to specific model classes (ICurrency, FiatPaymentMethod, FiatPriceInfo, FiatBuyOrderInfo) improves type safety and makes the API more explicit.


99-99: LGTM! Error handling improvements.

The error handling is more robust with explicit type casting and descriptive network error messages.

Also applies to: 103-103


107-137: LGTM! Type-safe chain ID mapping.

The update to accept CryptoCurrency instead of Currency improves type safety and makes the method's purpose clearer.


248-281:

❓ Verification inconclusive

Verify security implications of URL handling.

The new URL handling methods look well-implemented, but please verify:

  1. The security of base64-encoded provider URLs
  2. The handling of postMessage events
  3. The potential for URL manipulation in the success callback

Run this script to check for any existing security measures:


🏁 Script executed:

#!/bin/bash
# Search for postMessage event handlers and security checks
rg -A 5 "window\.postMessage|addEventListener.*message" 

# Search for URL validation
rg -A 5 "validateUrl|sanitizeUrl|isValidUrl"

Length of output: 562


Security Verification Needed for URL Handling

The new static URL helper methods look well-documented and generally well-implemented, but please verify the following security aspects:

  • Provider URL Validation:
    In fiatWrapperPageUrl, the provider URL is base64-encoded before being appended to the URL. Ensure that the input provider URL is coming from a trusted source or is validated elsewhere in the codebase to prevent URL injection or unexpected behavior.

  • postMessage Handling:
    The search in lib/views/fiat/webview_dialog.dart shows usage of window.addEventListener("message", …). Confirm that the event listener properly verifies the origin of incoming messages and securely handles the message data.

  • Query Parameter Integrity:
    In successUrl, although Uri.encodeComponent is used for the account reference, double-check that this parameter is sufficiently validated upstream to prevent any potential URL manipulation.

Please perform a manual security review of these aspects in the broader context of the application before merging.

lib/mm2/mm2.dart (4)

11-23: Constructor looks good overall.
Initializing the _kdfSdk in the constructor is straightforward and readable. This design ensures the SDK is always ready for use.


31-50: Ensure concurrency safety in initialize().
The pattern using _initCompleter and _isInitializing is a standard approach to preventing multiple initializations. However, if multiple callers attempt to initialize at once, be sure that the rest of the code is thread-safe, especially regarding _kdfSdk usage in other async operations.


52-57: version() method is concise and clear.
Getting version info from the RPC and returning it as String is straightforward. The conversion from JSON to domain objects is also well-organized.


59-75: Good error handling in call().
Catching & logging the exception, then rethrowing it, allows external layers to decide how to handle the error. This helps maintain a clean separation of concerns.

lib/bloc/trezor_init_bloc/trezor_init_bloc.dart (4)

43-44: Subscription to auth state is well-managed

The subscription to _kdfSdk.auth.authStateChanges is properly canceled in close(), ensuring no memory leaks or unintended updates post-disposal. This is a good practice for reactive streams.


187-192: Guard against missing device details

This block force-unwraps event.status.details.deviceDetails! which might be risky if, for any reason, the response does not include device details. Although the success flow expects them, consider adding a null check or fallback handling to avoid potential runtime exceptions.


283-295: Handle potential errors from setWalletType and confirmSeedBackup

The calls to _kdfSdk.setWalletType(WalletType.trezor) and _kdfSdk.confirmSeedBackup() do not appear to have explicit error handling. If these methods fail, the user experience may degrade silently. Consider handling or logging any exceptions.


328-330: Stream subscription cleanup confirmed

The _authorizationSubscription.cancel() call ensures that no further authentication events propagate once the bloc is closed, preventing potential leaks or exceptions. This finalizes proper resource disposal.

lib/bloc/transaction_history/transaction_history_repo.dart (2)

3-4: Imports appear consistent with the SDK approach.
Good inclusion of komodo_defi_sdk and komodo_defi_types for streamlined transaction handling.


50-59: Check the silent fallback behavior on fetch errors.
fetch may return null on failure, which gets replaced by an empty list in fetchCompletedTransactions. Be sure you intend to silently ignore errors, or consider surfacing them to upstream callers for better troubleshooting.

lib/blocs/current_wallet_bloc.dart (6)

4-4: SDK imports look valid.
These newly added imports properly integrate KomodoDefiSdk and related functionality into the bloc code.

Also applies to: 7-7


12-12: Deprecation annotation is clear.
Informing developers to use AuthBloc or KomodoDefiSdk helps guide them to the new implementation.


57-58: addCoin result confirmation.
Setting wallet before returning true is fine, but confirm that external code checks for success or handles any partial updates if activation fails later.


69-69: removeCoin update looks consistent.
Removing the coin from the wallet config and returning true is straightforward. Ensure the caller re-renders or refreshes the UI if needed.


73-85: Retrieve and encrypt seed.
Nicely integrates with _kdfSdk to fetch mnemonic. Confirm the seedPhrase.isEmpty check is robust enough if user set a custom seed phrase previously.


89-89: Post-save backup confirmation.
Calling confirmBackup after saving helps ensure the wallet is flagged as backed up. This is a good practice for user flows.

lib/bloc/custom_token_import/bloc/custom_token_import_bloc.dart (4)

1-11: New file for custom token importing.
All these imports are relevant for custom token logic, decimal math, events, states, and repositories. They set a solid foundation for EVM-based token import flow.


12-24: Bloc structure initialization.
Creating the bloc with CustomTokenImportState.defaults() is clean. The event handlers are neatly registered, improving readability.


51-59: Conditional exit on event.network == null.
Returning early is fine. Consider logging or throwing if a network is unexpectedly null, or if you intend that scenario to be user-driven.


119-149: Import custom token flow.
Emits success or failure states clearly. Ensure the UI consistently resets or updates its view on either outcome.

lib/bloc/custom_token_import/data/custom_token_import_repository.dart (2)

25-45: Ensure robust null-safety checks.

In fetchCustomToken, consider validating convertAddressResponse and its fields before proceeding, as a null or partial response could break subsequent steps.


119-122: Check for async activation race conditions.

Ensure the synchronous activation in importCustomToken does not conflict with other concurrent asset operations (e.g., if multiple imports occur simultaneously). Consider additional synchronization or waiting for the _coinsRepo to fully finish.

lib/bloc/withdraw_form/withdraw_form_state.dart (1)

81-106: Ensure consistent copyWith usage.

Using ValueGetter<T?> is a neat approach, but it relies on the caller to pass functions. Double-check that the usage within the rest of the codebase matches the intended pattern; accidental direct invocations or forgetting the function calls can introduce subtle bugs.

lib/bloc/trezor_bloc/trezor_repo.dart (2)

90-92: Confirm backward compatibility when migrating from Coin to Asset.

enableUtxo now accepts Asset. Verify that all callers and usage sites have been updated correctly to prevent runtime mismatches.


169-182: Ensure account refresh is correct in getNewAddressStatus.

If new addresses are derived or updated, confirm that the local state (coin.accounts) does not conflict with other concurrency or stale data from _api.

lib/bloc/withdraw_form/withdraw_form_bloc.dart (21)

3-5: Imports look fine.
No immediate concerns regarding versioning or usage.


14-14: Decimal import is appropriate.
Using Decimal for precise arithmetic helps reduce floating-point errors.


17-17: Good approach introducing the _sdk field.
No concurrency or lifecycle issues spotted; usage seems straightforward.


19-48: Constructor logic is consistent.
All event handlers are registered, and initial load is triggered via add(const WithdrawFormSourcesLoadRequested()). Implementation is sound.


50-82: Solid error handling in _onSourcesLoadRequested.
Differentiates between “no addresses found” and general errors. This should help with user feedback.


84-99: Validate default fees.
Returning null for unsupported protocols is acceptable, but consider verifying the fallback fee approach (e.g., protocol.txFee ?? 10000) aligns with real network fees.


101-173: Robust address validation in _onRecipientChanged.
Mixed-case checks and fallback error handling are well-structured.


175-221: Amount parsing in _onAmountChanged is thorough.
Ensures positive amounts, checks balance, and gracefully handles parsing errors.


223-233: Selecting source address is straightforward.
Implementation is concise.


250-262: Custom fee toggle is simple and clear.
No issues found.


264-288: Fee validation logic is appropriate.
Good separation of EVM vs UTXO fees.


290-298: _validateEvmFee method is correct.
Ensures gas price and gas limit are above zero.


299-303: _validateUtxoFee checks the fee is > 0.
Straightforward.


305-310: No issues with memo handling.
Implementation is minimal and effective.


312-323: IBC transfer toggle is well-defined.
Approach is consistent with existing logic.


325-345: Sensible channel ID checks.
Gracefully handles empty channels.


347-381: Preview submission handles loading and errors cleanly.
No concerns found.


383-424: Withdrawal submission flow looks good.
Progress streaming is handled properly, and errors are surfaced clearly.


435-449: Reset logic is clean.
Resets relevant fields without unexpected side effects.


451-456: Mixed-case utility is concise.
Logic is straightforward and consistent with the EVM approach.


458-491: Address conversion routine.
Properly converts to the checksummed format, with clear error handling.

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

4-5: New SDK imports appear valid.
No version conflicts evident.


7-7: Importing CoinsRepo.
Usage is consistent across constructor and NFT instantiation.


15-16: Directly connected peers import is clean.
No issues.


44-44: SendRawTransactionResponse import is correct.
Enhances type safety.


55-55: TextError import is used for error reporting.
No concerns.


61-63: Constructor injects CoinsRepo and KomodoDefiSdk.
Aligns with the new architecture.


64-65: Private fields _sdk and _mm2 are properly initialized.
Follows the updated design.


71-74: Comment on bridging to the SDK.
No functional issues.


94-100: Deprecated getBalance adapter.
Safe approach until full migration to the SDK is complete.


102-122: _fallbackToBalanceTaker implementation
Returns a synthetic MaxTakerVolResponse on error. Logical fallback approach.


128-136: Active swaps retrieval
Error logging is consistent; fallback response is minimal.


145-156: validateAddress
Captures exceptions, logs them, and returns null if failing.


161-170: withdraw integration
Graceful fallback to null on exceptions.


203-212: getTransactionsHistory
No issues. Logs errors appropriately.


219-228: getTransactionsHistoryV2
Similar error handling strategy.


235-244: getRewardsInfo
Straightforward retrieval with logging.


249-258: getBestOrders
Returns partial error info in a map. Implementation is acceptable.


263-273: sell
Error handling logs exceptions and returns a map with 'error'.


277-287: setprice
Same pattern of returning 'error': e. Fine for now.


291-301: cancelOrder
Risks are clearly logged, fallback map is returned.


305-315: getSwapStatus
Consistent approach to logging and fallback.


317-338: getMyOrders
Conditional sign-in check and error handling. Implementation is correct.


341-342: getRawSwapData
Encodes response to JSON. Straightforward.


348-362: getMyRecentSwaps
Null check for 'error' key, then returns typed response.


367-381: getOrderStatus
No issues. Follows the same pattern.


385-399: importSwaps
Uses typed response, logs errors. Looks good.


405-423: recoverFundsOfSwap
Captures errors and logs them, returns null if failing.


430-444: getMaxTakerVolume
On error or 'error' key, uses _fallbackToBalanceTaker.


450-464: getMinTradingVol
Returns null if 'error'. No concerns noted.


468-494: getOrderbook
Returns an OrderbookResponse, logs exceptions.


497-515: getOrderBookDepth
Handles 'error' gracefully. No issues.


523-542: getTradePreimage
Uses typed result with fallback. Solid.


557-575: startStopMarketMakerBot
Throws if 'error' is present. Logging is good.


593-594: stop
Simple call to StopReq with no further logic.


600-613: showPrivKey
Ignores errors, returns null if 'error' in response. Suitable for debugging.


616-640: getDirectlyConnectedPeers
Re-throws on failure, returns a typed response if successful. Implementation is consistent.

lib/bloc/custom_token_import/bloc/custom_token_import_event.dart (6)

1-9: Abstract event class is well defined.
Equatable usage ensures efficient equality checks.


11-19: UpdateNetworkEvent:
Nullable CoinSubClass? network is properly exposed in props.


20-27: UpdateAddressEvent:
Straightforward property for address updates.


29-31: SubmitImportCustomTokenEvent:
Placeholder event for the import flow.


33-35: SubmitFetchCustomTokenEvent:
Allows fetching custom token details.


37-39: ResetFormStatusEvent:
Suitable event to reset or clear form state.

lib/bloc/analytics/analytics_repo.dart (1)

53-55: LGTM! Parameter sanitization ensures proper data types.

The implementation correctly handles non-Object values by converting them to strings, preventing potential Firebase Analytics errors.

lib/bloc/coins_bloc/coins_repo.dart (2)

97-116: Remove deprecated method getCoin.

The getCoin method is marked as deprecated. Consider removing it and updating all callers to use KomodoDefiSdk.assets or getCoinFromId instead.


327-329: Remove deprecated method getFirstPubkey.

The getFirstPubkey method is marked as deprecated. Update callers to use pubkeys.getPubkeys from the SDK and let users select from available options.

lib/bloc/coins_bloc/coins_bloc.dart (1)

51-57: Replace deprecated CurrentWalletBloc.

The class uses the deprecated CurrentWalletBloc. Consider migrating to AuthBloc or KomodoDefiSdk for wallet management.

ios/Podfile (1)

2-2:

✅ Verification successful

Verify iOS platform version compatibility.

The platform version has been significantly increased from 11.0 to 15.5.0. This change will restrict the app to only run on iOS 15.5.0 and later devices, potentially reducing the user base.

Consider if this restriction aligns with your target audience and market requirements. If broader device support is needed, you might want to consider a lower minimum version while ensuring it's still compatible with your app's features and dependencies.


🌐 Web query:

What is the current iOS adoption rate for iOS 15.5 and above?

💡 Result:

As of February 2025, iOS adoption rates for versions 15.5 and above can be inferred from recent data trends:

  1. Cumulative Adoption:

    • By October 1, 2024, 95.1% of iPhones had adopted at least iOS 15, including all subsequent updates like iOS 16, 17, and 18[8].
    • This figure represents cumulative usage over time rather than active installations of specific minor versions like iOS 15.
  2. Current Market Share:

    • Data from January 2025 shows that only ~3–4% of active devices run iOS 16 or earlier (including residual installations of iOS 15)[6][7].
    • The vast majority (~96–97%) use iOS 17 or later, implying they also meet the "15.5 or above" threshold due to Apple’s forced update policies[7][9].

Key Notes:

  • Apple’s aggressive update enforcement ensures most compatible devices automatically upgrade beyond older minor releases like iOS 15.
  • Devices stuck on older OS versions are typically legacy models incompatible with newer updates[8].

In summary: While exact figures for iOS 15.x aren’t explicitly reported in early 2025,≥95% of active iPhones likely run iOS 15.x or later due to Apple’s update ecosystem[6][7][8].

Citations:


Attention: iOS Deployment Target Confirmed

Based on current market data, nearly 95% of active iPhone users are running iOS 15.5 or later. Therefore, updating the Podfile to:

platform :ios, '15.5.0'

will have minimal impact on your user base. Please ensure that this threshold still aligns with your target audience—especially if you have any niche segments potentially using older devices.

  • File: ios/Podfile (Line 2)
  • Key Consideration: Adoption trends support this update, but double-check if legacy device support is necessary.
ios/Runner.xcodeproj/project.pbxproj (2)

267-283: LGTM! New CocoaPods resources phase added.

The addition of the [CP] Copy Pods Resources build phase is a standard CocoaPods integration step that ensures proper resource handling.


186-186:

❓ Verification inconclusive

Verify Xcode version compatibility.

The LastUpgradeCheck has been updated to 1510, which corresponds to Xcode 15.1.


🌐 Web query:

What is the latest version of Xcode and what are the minimum system requirements?

💡 Result:

The latest stable version of Xcode as of February 2025 is Xcode 16, released on September 16, 2024[1][4]. The current beta version is Xcode 16.2 beta[1].

Minimum System Requirements

macOS:

  • Requires macOS Sonoma 14.5 or later for all Xcode 16 versions[1][7].
  • Older versions of Xcode have progressively lower macOS requirements (e.g., Xcode 15 requires macOS Ventura 13.5)[3].

Hardware Compatibility:

  • Apple does not specify exact hardware requirements but ties compatibility to macOS versions[2].
  • For macOS Sonoma (required for Xcode 16), supported devices generally include:
    • Macs with Apple Silicon (M1/M2/M3 chips)
    • Intel-based Macs from late 2017 onward[2][8].

Recommended Specifications:

  • RAM: At least 8GB (functional for basic development), though 16GB is recommended for smoother performance[2][8].
  • Storage: Minimum 256GB SSD, with additional space required for simulators and SDKs (~80GB for two Xcode versions)[5].

Key Features in Xcode 16

  • Supports development for iOS 18, macOS 15, visionOS 2, and other Apple platforms[1][4].
  • Includes Swift 6 and backward compatibility with Swift versions as old as Swift 4[1].
  • Architecture support: arm64, arm64e, and legacy Intel (x86_64)[1].

For app store submissions, Apple will stop accepting apps built with Xcode versions older than v15 starting April 2025[7].

Citations:


Action: Confirm Compatibility with Current Xcode Version

  • File: ios/Runner.xcodeproj/project.pbxproj at line 186
  • Observation: The LastUpgradeCheck field is set to 1510, which indicates the project was last upgraded using Xcode 15.1.
  • Context: As of February 2025, the latest stable Xcode release is version 16 (released September 2024), which requires macOS Sonoma 14.5 or later.
  • Recommendation: Please verify that retaining the value 1510 is intentional and that the project remains fully compatible with Xcode 16. If compatibility issues arise, consider updating this field to reflect the current Xcode environment.

Comment on lines +20 to 45
Future<List<Transaction>?> fetch(AssetId assetId, {String? fromId}) async {
final asset = _sdk.assets.available[assetId];
if (asset == null) {
throw TransactionFetchException('Asset $assetId not found');
}
return coin.isErcType
? await fetchErcTransactionHistory(coin)
: await fetchTransactionHistory(
MyTxHistoryRequest(
coin: coin.abbr,
max: true,
),
);
}

Future<List<Transaction>> fetchTransactions(Coin coin) async {
final historyResponse = await fetch(coin);
final TransactionHistoryResponseResult? result = historyResponse.data;

final BaseError? responseError = historyResponse.error;
// TODO: add custom exceptions here?
if (responseError != null) {
throw TransactionFetchException('Transaction fetch error: ${responseError.message}');
} else if (result == null) {
throw TransactionFetchException('Transaction fetch result is null');
try {
final transactionHistory = await _sdk.transactions.getTransactionHistory(
asset,
pagination: fromId == null
? const PagePagination(
pageNumber: 1,
// TODO: Handle cases with more than 2000 transactions and/or
// adopt a pagination strategy. Migrate form
itemsPerPage: 2000,
)
: TransactionBasedPagination(
fromId: fromId,
itemCount: 2000,
),
);
return transactionHistory.transactions;
} catch (e) {
return null;
}

return result.transactions;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Unify exception vs. null returns in fetch.
After throwing an exception for a missing asset, the catch block swallows other errors by returning null. Consider consistently throwing TransactionFetchException here or returning a wrapped error to avoid masking different error states.

Comment on lines +68 to +117
Future<void> _onSubmitFetchCustomToken(
SubmitFetchCustomTokenEvent event,
Emitter<CustomTokenImportState> emit,
) async {
emit(state.copyWith(formStatus: FormStatus.submitting));

try {
final networkAsset = _coinsRepo.getCoin(state.network.ticker);
if (networkAsset == null) {
throw Exception('Network asset ${state.network.formatted} not found');
}

await _coinsRepo.activateCoinsSync([networkAsset]);
final tokenData =
await repository.fetchCustomToken(state.network, state.address);
await _coinsRepo.activateAssetsSync([tokenData]);

final balanceInfo = await _coinsRepo.tryGetBalanceInfo(tokenData.id);
final balance = balanceInfo.spendable;
final usdBalance =
_coinsRepo.getUsdPriceByAmount(balance.toString(), tokenData.id.id);

emit(
state.copyWith(
formStatus: FormStatus.success,
tokenData: () => tokenData,
tokenBalance: balance,
tokenBalanceUsd:
Decimal.tryParse(usdBalance?.toString() ?? '0.0') ?? Decimal.zero,
formErrorMessage: '',
),
);

await _coinsRepo.deactivateCoinsSync([tokenData.toCoin()]);
} catch (e, s) {
log(
'Error fetching custom token: ${e.toString()}',
path: 'CustomTokenImportBloc._onSubmitFetchCustomToken',
isError: true,
trace: s,
);
emit(
state.copyWith(
formStatus: FormStatus.failure,
tokenData: () => null,
formErrorMessage: e.toString(),
),
);
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fetch custom token logic.

  1. Activation and deactivation flow for custom token looks robust.
  2. The fallback to Exception might obscure typed errors from the repository.
  3. Consider thorough error handling or typed exceptions for clarity.

Comment on lines +47 to +117
Future<Asset> _createNewCoin(
String contractAddress,
CoinSubClass network,
String address,
) async {
final response = await _kdfSdk.client.rpc.utility.getTokenInfo(
contractAddress: contractAddress,
platform: network.ticker,
protocolType: CoinSubClass.erc20.formatted,
);

final platformAssets = _kdfSdk.assets.findAssetsByTicker(network.ticker);
if (platformAssets.length != 1) {
throw Exception('Platform asset not found. ${platformAssets.length} '
'results returned.');
}
final platformAsset = platformAssets.single;
final platformConfig = platformAsset.protocol.config;
final String ticker = response.info.symbol;
final tokenApi = await fetchTokenInfoFromApi(network, contractAddress);

final coinId = '$ticker-${network.ticker}';
final logoImageUrl = tokenApi?['image']?['large'] ??
tokenApi?['image']?['small'] ??
tokenApi?['image']?['thumb'];
final newCoin = Asset(
id: AssetId(
id: coinId,
name: tokenApi?['name'] ?? ticker,
symbol: AssetSymbol(
assetConfigId: '$ticker-${network.ticker}',
coinGeckoId: tokenApi?['id'],
coinPaprikaId: tokenApi?['id'],
),
chainId: AssetChainId(chainId: 0),
subClass: network,
derivationPath: '',
),
protocol: Erc20Protocol.fromJson({
'type': network.formatted,
'chain_id': 0,
'nodes': [],
'swap_contract_address':
platformConfig.valueOrNull<String>('swap_contract_address'),
'fallback_swap_contract':
platformConfig.valueOrNull<String>('fallback_swap_contract'),
'protocol': {
'protocol_data': {
'platform': network.ticker,
'contract_address': address,
},
},
'logo_image_url': logoImageUrl,
'explorer_url': platformConfig.valueOrNull<String>('explorer_url'),
'explorer_url_tx':
platformConfig.valueOrNull<String>('explorer_url_tx'),
'explorer_url_address':
platformConfig.valueOrNull<String>('explorer_url_address'),
}).copyWith(isCustomToken: true),
);

CoinIcon.registerCustomIcon(
newCoin.id.id,
NetworkImage(
tokenApi?['image']?['large'] ??
'assets/coin_icons/png/${ticker.toLowerCase()}.png',
),
);

return newCoin;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling for token creation.

In _createNewCoin, consider handling potential absent or malformed fields from the RPC response (e.g., response.info). Null checks, combined with descriptive exceptions, can help diagnose issues quickly.

Comment on lines +141 to +156
WithdrawParameters toWithdrawParameters() {
return WithdrawParameters(
asset: asset.id.id,
toAddress: recipientAddress,
amount: isMaxAmount ? null : Decimal.parse(amount),
fee: isCustomFee ? customFee : null,
from: selectedSourceAddress?.derivationPath != null
? WithdrawalSource.hdDerivationPath(
selectedSourceAddress!.derivationPath!,
)
: null,
memo: memo,
ibcTransfer: isIbcTransfer ? true : null,
isMax: isMaxAmount,
);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Validate numeric parsing in toWithdrawParameters.

Decimal.parse(amount) can throw an exception if amount is empty or invalid. Ensure that upstream validations always guarantee a valid numeric string.

Comment on lines 60 to 67
Future<void> sendPin(String pin, TrezorTask trezorTask) async {
await _api.pin(TrezorPinRequest(
pin: pin,
task: trezorTask,
));
await _api.pin(
TrezorPinRequest(
pin: pin,
task: trezorTask,
),
);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Confirm secure input handling for sendPin.

Pins are potentially sensitive data. Confirm that logging is avoided and ensure no PII or secrets are inadvertently persisted in logs or analytics.

Comment on lines +126 to +167
CoinSubClass toCoinSubClass() {
switch (this) {
case CoinType.ftm20:
return CoinSubClass.ftm20;
case CoinType.arb20:
return CoinSubClass.arbitrum;
case CoinType.slp:
return CoinSubClass.slp;
case CoinType.qrc20:
return CoinSubClass.qrc20;
case CoinType.avx20:
return CoinSubClass.avx20;
case CoinType.smartChain:
return CoinSubClass.smartChain;
case CoinType.mvr20:
return CoinSubClass.moonriver;
case CoinType.etc:
return CoinSubClass.ethereumClassic;
case CoinType.hco20:
return CoinSubClass.hecoChain;
case CoinType.hrc20:
return CoinSubClass.hrc20;
case CoinType.iris:
return CoinSubClass.tendermintToken;
case CoinType.cosmos:
return CoinSubClass.tendermint;
case CoinType.ubiq:
return CoinSubClass.ubiq;
case CoinType.bep20:
return CoinSubClass.bep20;
case CoinType.plg20:
return CoinSubClass.matic;
case CoinType.utxo:
return CoinSubClass.utxo;
case CoinType.sbch:
return CoinSubClass.smartBch;
case CoinType.erc20:
return CoinSubClass.erc20;
case CoinType.krc20:
return CoinSubClass.krc20;
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add default case for unsupported coin types.

The toCoinSubClass() method is missing a default case, which could lead to runtime errors when an unsupported coin type is provided.

Apply this diff to improve error handling:

     case CoinType.krc20:
       return CoinSubClass.krc20;
+    default:
+      throw UnsupportedError('Unsupported coin type: $this');
   }
📝 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
CoinSubClass toCoinSubClass() {
switch (this) {
case CoinType.ftm20:
return CoinSubClass.ftm20;
case CoinType.arb20:
return CoinSubClass.arbitrum;
case CoinType.slp:
return CoinSubClass.slp;
case CoinType.qrc20:
return CoinSubClass.qrc20;
case CoinType.avx20:
return CoinSubClass.avx20;
case CoinType.smartChain:
return CoinSubClass.smartChain;
case CoinType.mvr20:
return CoinSubClass.moonriver;
case CoinType.etc:
return CoinSubClass.ethereumClassic;
case CoinType.hco20:
return CoinSubClass.hecoChain;
case CoinType.hrc20:
return CoinSubClass.hrc20;
case CoinType.iris:
return CoinSubClass.tendermintToken;
case CoinType.cosmos:
return CoinSubClass.tendermint;
case CoinType.ubiq:
return CoinSubClass.ubiq;
case CoinType.bep20:
return CoinSubClass.bep20;
case CoinType.plg20:
return CoinSubClass.matic;
case CoinType.utxo:
return CoinSubClass.utxo;
case CoinType.sbch:
return CoinSubClass.smartBch;
case CoinType.erc20:
return CoinSubClass.erc20;
case CoinType.krc20:
return CoinSubClass.krc20;
}
}
CoinSubClass toCoinSubClass() {
switch (this) {
case CoinType.ftm20:
return CoinSubClass.ftm20;
case CoinType.arb20:
return CoinSubClass.arbitrum;
case CoinType.slp:
return CoinSubClass.slp;
case CoinType.qrc20:
return CoinSubClass.qrc20;
case CoinType.avx20:
return CoinSubClass.avx20;
case CoinType.smartChain:
return CoinSubClass.smartChain;
case CoinType.mvr20:
return CoinSubClass.moonriver;
case CoinType.etc:
return CoinSubClass.ethereumClassic;
case CoinType.hco20:
return CoinSubClass.hecoChain;
case CoinType.hrc20:
return CoinSubClass.hrc20;
case CoinType.iris:
return CoinSubClass.tendermintToken;
case CoinType.cosmos:
return CoinSubClass.tendermint;
case CoinType.ubiq:
return CoinSubClass.ubiq;
case CoinType.bep20:
return CoinSubClass.bep20;
case CoinType.plg20:
return CoinSubClass.matic;
case CoinType.utxo:
return CoinSubClass.utxo;
case CoinType.sbch:
return CoinSubClass.smartBch;
case CoinType.erc20:
return CoinSubClass.erc20;
case CoinType.krc20:
return CoinSubClass.krc20;
default:
throw UnsupportedError('Unsupported coin type: $this');
}
}

Comment on lines +9 to +13
ProtocolData? protocolData;
protocolData = ProtocolData(
platform: id.parentId?.id ?? '',
contractAddress: '',
);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Initialize contract address from protocol data.

The ProtocolData is initialized with an empty contract address, which could cause issues. Consider initializing it with the actual contract address from the protocol data.

Apply this diff to fix the issue:

-    protocolData = ProtocolData(
-      platform: id.parentId?.id ?? '',
-      contractAddress: '',
-    );
+    protocolData = ProtocolData(
+      platform: id.parentId?.id ?? '',
+      contractAddress: protocol.config.valueOrNull('protocol', 'protocol_data', 'contract_address') ?? '',
+    );
📝 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
ProtocolData? protocolData;
protocolData = ProtocolData(
platform: id.parentId?.id ?? '',
contractAddress: '',
);
ProtocolData? protocolData;
protocolData = ProtocolData(
platform: id.parentId?.id ?? '',
contractAddress: protocol.config.valueOrNull('protocol', 'protocol_data', 'contract_address') ?? '',
);

Comment on lines +96 to +98
default:
return CoinType.utxo;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Throw error for unsupported coin subclasses.

The default case returns CoinType.utxo which might not be appropriate for all unsupported coin subclasses. Consider throwing an error to prevent silent failures.

Apply this diff to improve error handling:

-      default:
-        return CoinType.utxo;
+      default:
+        throw UnsupportedError('Unsupported coin subclass: $this');
📝 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
default:
return CoinType.utxo;
}
default:
throw UnsupportedError('Unsupported coin subclass: $this');
}

* build: fix missing asset error

assets declared by dependency packages appear to be accessible from the calling/parent package

* ci(flutter): upgrade pinned version to 3.29

* docs: upgrade pinned flutter version to 3.29

* build(android): upgrade NDK version to v27 (latest stable)
Copy link

@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: 0

🧹 Nitpick comments (2)
.github/actions/flutter-deps/action.yml (1)

12-20: Prepare Build Directory Step – Consider Adding Strict Error Handling:
The cleanup commands (invoking flutter clean and removing build-related directories) are effective in ensuring a clean environment.
A good-to-have improvement would be to add an set -e at the beginning of the shell script to immediately terminate the step if any command fails. This minimizes the risk of proceeding with a partially cleaned environment. For example:

-      run: |
-        flutter clean
-        rm -rf build/*
-        rm -rf web/src/mm2/*
-        rm -rf web/src/kdfi/*
-        rm -rf web/dist/*
+      run: |
+        set -e
+        flutter clean
+        rm -rf build/*
+        rm -rf web/src/mm2/*
+        rm -rf web/src/kdfi/*
+        rm -rf web/dist/*
docs/FLUTTER_VERSION.md (1)

12-15: Specify Language for Fenced Code Block

The fenced code block starting on line 11 lacks a language specifier, which is triggering markdown lint warnings. Please add an appropriate language identifier (e.g., bash or sh) after the opening triple backticks to improve clarity and lint compliance.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9abd03 and 6b06f15.

📒 Files selected for processing (5)
  • .devcontainer/komodo-wallet-android-dev.dockerfile (1 hunks)
  • .docker/komodo-wallet-android.dockerfile (1 hunks)
  • .github/actions/flutter-deps/action.yml (1 hunks)
  • android/app/build.gradle (3 hunks)
  • docs/FLUTTER_VERSION.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • android/app/build.gradle
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/FLUTTER_VERSION.md

11-11: Fenced code blocks should have a language specified
null

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: build_and_preview
  • GitHub Check: validate_code_guidelines
  • GitHub Check: build_and_deploy
🔇 Additional comments (6)
.github/actions/flutter-deps/action.yml (3)

1-2: Clear Naming and Description:
The action name ("Flutter Dependencies") and description ("Installs Flutter and any other dependencies required for the build") are clear and appropriately describe the purpose of the action.


3-5: Composite Action Declaration:
The use of a composite action with runs: using: "composite" and a proper steps definition is correct. It clearly communicates that multiple steps are executed as part of this action.


6-11: Stable Flutter Installation Step:
The step uses subosito/flutter-action@v2 with the specified flutter-version: "3.29.x" and channel: "stable", which is appropriate for ensuring that the correct version of Flutter is installed. There are no issues seen here.

.docker/komodo-wallet-android.dockerfile (1)

15-15: Update Flutter Version Environment Variable

The Flutter version has been updated successfully from "3.22.3" to "3.29.0". This change aligns with the overall upgrade across the codebase. Please verify that the updated tag exists in the Flutter repository and that the build process works as expected with this version.

.devcontainer/komodo-wallet-android-dev.dockerfile (1)

5-5: Update Flutter Version in Dev Container

The Flutter version environment variable has been correctly updated to "3.29.0". This change is consistent with other repository files and should ensure that development and build environments use the latest stable version.

docs/FLUTTER_VERSION.md (1)

7-7: Update Documented Current Flutter Version

The documented current version has been updated to 3.29.0. This reflects the intended upgrade and ensures that documentation is in sync with the project configuration.

5960abfcc01b067e994504218a7f64a3c142ac08
* fix(asset): add missing segwit flag to coin conversion function

* fix(activation): skip coin activation if not signed in

coin activation fails with an error message if there is no wallet/user signed in

* fix(text-form-field): add a default fill colour to UiTextFormField

fixes the lacking contrast with the input fields, as a lack of fill colour results in an input field with the same colour as the background (dark and light modes)
Copy link

@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: 2

♻️ Duplicate comments (1)
lib/bloc/coins_bloc/coins_repo.dart (1)

342-351: ⚠️ Potential issue

Replace deprecated getFirstPubkey method.

The method is marked as deprecated with a comment suggesting to use SDK pubkeys.getPubkeys instead.

Since this method is still being used in the codebase (e.g., line 141), it needs to be replaced. Here's how to refactor the usage:

-final coinAddress = await getFirstPubkey(coin.abbr);
+final asset = _kdfSdk.assets.findAssetsByTicker(coin.abbr).single;
+final pubkeys = await _kdfSdk.pubkeys.getPubkeys(asset);
+final coinAddress = pubkeys.keys.isNotEmpty ? pubkeys.keys.first.address : null;
🧹 Nitpick comments (3)
lib/bloc/coins_bloc/coins_repo.dart (3)

45-47: Address TODO comment regarding Trezor bloc refactoring.

The TODO comment indicates that the Trezor bloc needs refactoring to handle pin/password input events. This architectural issue should be addressed.

Would you like me to help design a proper repository pattern for handling Trezor-related operations?


364-373: Consider adding retry mechanism for price fetching.

The price fetching logic could benefit from a retry mechanism with exponential backoff for better resilience.

 Future<Map<String, CexPrice>?> fetchCurrentPrices() async {
+  const maxRetries = 3;
+  const initialDelay = Duration(seconds: 1);
+  
+  for (int i = 0; i < maxRetries; i++) {
+    if (i > 0) {
+      await Future.delayed(initialDelay * (1 << i));  // Exponential backoff
+    }
     final Map<String, CexPrice>? prices =
         await _updateFromMain() ?? await _updateFromFallback();
 
     if (prices != null) {
       _pricesCache = prices;
+      return _pricesCache;
     }
+  }
 
-    return _pricesCache;
+  return null;  // Return null after all retries fail
 }

534-570: Enhance error handling in withdraw method.

The withdraw method's error handling could be improved by adding more specific error types and logging.

 Future<BlocResponse<WithdrawDetails, BaseError>> withdraw(
   WithdrawRequest request,
 ) async {
   Map<String, dynamic>? response;
   try {
     response = await _mm2.call(request) as Map<String, dynamic>?;
   } catch (e, s) {
+    final errorMessage = 'Error withdrawing ${request.params.coin}: $e';
     log(
-      'Error withdrawing ${request.params.coin}: $e',
+      errorMessage,
       path: 'api => withdraw',
       trace: s,
       isError: true,
     ).ignore();
+    return BlocResponse(
+      error: TextError(
+        error: e is TimeoutException
+          ? LocaleKeys.withdrawalTimeout.tr()
+          : LocaleKeys.somethingWrong.tr(),
+        details: errorMessage,
+      ),
+    );
   }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6b06f15 and 8fa543a.

📒 Files selected for processing (2)
  • lib/bloc/coins_bloc/asset_coin_extension.dart (1 hunks)
  • lib/bloc/coins_bloc/coins_repo.dart (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/bloc/coins_bloc/asset_coin_extension.dart
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: build_and_preview
  • GitHub Check: validate_code_guidelines
  • GitHub Check: build_and_deploy
🔇 Additional comments (2)
lib/bloc/coins_bloc/coins_repo.dart (2)

29-41: LGTM! Constructor properly initializes dependencies.

The constructor correctly initializes the required dependencies and sets up the broadcast stream controller with proper listener management.


291-292: Remove duplicate deprecated method usage.

This is another instance of using the deprecated activateAsset method with an ignore comment.

Comment on lines +241 to +242
// ignore: deprecated_member_use
final progress = await _kdfSdk.assets.activateAsset(assets.single).last;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove deprecated method usage.

The code is using a deprecated activateAsset method with an explicit ignore comment.

Consider migrating to the new activation method in the SDK. If you need help with the migration, I can assist with the implementation.

Comment on lines +433 to 492
Future<Map<String, CexPrice>?> _updateFromFallback() async {
final List<String> ids = (await getEnabledCoins())
.map((c) => c.coingeckoId ?? '')
.toList()
..removeWhere((id) => id.isEmpty);
final Uri fallbackUri = Uri.parse(
'https://api.coingecko.com/api/v3/simple/price?ids='
'${ids.join(',')}&vs_currencies=usd',
);

http.Response res;
String body;
try {
res = await http.get(fallbackUri);
body = res.body;
} catch (e, s) {
log(
'Error updating price from fallback: $e',
path: 'cex_services => _updateFromFallback => http.get',
trace: s,
isError: true,
).ignore();
return null;
}

Map<String, dynamic>? json;
try {
json = jsonDecode(body) as Map<String, dynamic>?;
} catch (e, s) {
log(
'Error parsing of update price from fallback response: $e',
path: 'cex_services => _updateFromFallback => jsonDecode',
trace: s,
isError: true,
).ignore();
}

if (json == null) return null;
final Map<String, CexPrice> prices = {};

for (final MapEntry<String, dynamic> entry in json.entries) {
final coingeckoId = entry.key;
final pricesData = entry.value as Map<String, dynamic>? ?? {};
if (coingeckoId == 'test-coin') continue;

// Coins with the same coingeckoId supposedly have same usd price
// (e.g. KMD == KMD-BEP20)
final Iterable<Coin> samePriceCoins =
(getKnownCoins()).where((coin) => coin.coingeckoId == coingeckoId);

for (final Coin coin in samePriceCoins) {
prices[coin.abbr] = CexPrice(
ticker: coin.abbr,
price: double.parse(pricesData['usd'].toString()),
);
}
}

return prices;
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add rate limiting for Coingecko API calls.

The fallback price fetching mechanism using Coingecko's API should implement rate limiting to avoid hitting API limits.

+class RateLimiter {
+  final Duration interval;
+  DateTime? _lastCall;
+
+  RateLimiter({this.interval = const Duration(seconds: 1)});
+
+  Future<void> throttle() async {
+    if (_lastCall != null) {
+      final waitTime = interval - DateTime.now().difference(_lastCall!);
+      if (waitTime.isNegative == false) {
+        await Future.delayed(waitTime);
+      }
+    }
+    _lastCall = DateTime.now();
+  }
+}

 Future<Map<String, CexPrice>?> _updateFromFallback() async {
+  static final _rateLimiter = RateLimiter(interval: Duration(seconds: 2));
+  await _rateLimiter.throttle();
+
   final List<String> ids = (await getEnabledCoins())
       .map((c) => c.coingeckoId ?? '')
       .toList()

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

* build(docker): remove kdf build step and download flutter from sdk archive

* build: remove kdf build step from devcontainer and update flutter install

* build: fix devcontainer workspace folder error in CodeSpaces

* build(devcontainer): extract flutter to /opt rather than workspaces
* docs(flutter-setup): update setup steps to match the official guide

Flutter documentation recommends downloading via VSCode or via the SDK Archive (tar/zip files) rather than cloning the git repository

* docs(project-setup): update the referenced IDE and tool versions

* docs: address coderabbitai nitpick comments

* docs(fvm): remove unix-specific instructions, and reduce duplication
Copy link

@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: 0

🧹 Nitpick comments (7)
docs/FLUTTER_VERSION.md (1)

19-21: Clarify Pinning Recommendation
The note advising against pinning the Flutter version is helpful. It might be beneficial to briefly explain why isolation is insufficient when managing multiple Flutter versions.

.devcontainer/Dockerfile (3)

6-7: Merge PATH Environment Variables
The Dockerfile sets the PATH twice—first adding /opt/flutter/bin and then /android-ndk/bin. This may result in the latter overriding the first. Consider merging them into a single statement (e.g., ENV PATH=$PATH:/opt/flutter/bin:/android-ndk/bin) to ensure both paths are preserved.


30-37: Avoid sudo Usage in Dockerfile RUN Commands
The installation step uses sudo (e.g., sudo chown ...), which can lead to unpredictable behavior in Dockerfiles. Since the build is typically executed as root, it is preferable to remove sudo or switch to a tool like gosu when a user change is required.


72-90: Consider Removing sudo in Subsequent RUN Commands
The RUN block starting at line 72 continues to use sudo for commands like apt-get update and chown. Running these commands as root (or using an alternative like gosu) would simplify the build process and improve predictability.

🧰 Tools
🪛 Hadolint (2.12.0)

[error] 72-72: Do not use sudo as it leads to unpredictable behavior. Use a tool like gosu to enforce root

(DL3004)

.docker/komodo-wallet-android.dockerfile (1)

13-18: Flutter SDK Installation Steps:
The RUN command effectively downloads the Flutter SDK, extracts it to the designated home directory, cleans up the archive, and configures Flutter by disabling analytics and accepting Android licenses. For enhanced debuggability, you might consider adding error checking or splitting long chained commands (using &&), so that pinpointing failures is easier.

docs/INSTALL_FLUTTER.md (1)

47-47: Typographical Note:
Consider using “log out” (two words) instead of “logout” for clarity. Additionally, you may want to review the punctuation before “if” to ensure it aligns with standard style conventions.

🧰 Tools
🪛 LanguageTool

[misspelling] ~47-~47: Did you mean the verb “log out” instead of the noun ‘logout’?
Context: ...er\bin as its value. You might need to logout and re-login (or source the shell confi...

(LOG_IN)


[typographical] ~47-~47: Usually, there’s no comma before “if”.
Context: ... (or source the shell configuration file, if applicable) to make changes apply. On ...

(IF_NO_COMMA)

docs/MULTIPLE_FLUTTER_VERSIONS.md (1)

1-4: FVM Section Introduction:
The introduction clearly explains the benefits of using FVM for managing multiple Flutter SDK versions. However, the self-reference (See [MULTIPLE_FLUTTER_VERSIONS.md](MULTIPLE_FLUTTER_VERSIONS.md)) is redundant since the user is already viewing this file. Consider removing it for clarity.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8fa543a and 92cb33e.

📒 Files selected for processing (15)
  • .devcontainer/Dockerfile (1 hunks)
  • .devcontainer/dev-setup.sh (0 hunks)
  • .devcontainer/devcontainer.json (1 hunks)
  • .devcontainer/komodo-wallet-android-dev.dockerfile (0 hunks)
  • .docker/android-sdk.dockerfile (1 hunks)
  • .docker/build.sh (1 hunks)
  • .docker/kdf-android.dockerfile (0 hunks)
  • .docker/komodo-wallet-android.dockerfile (1 hunks)
  • .dockerignore (1 hunks)
  • docs/BUILD_RUN_APP.md (2 hunks)
  • docs/FIREBASE_SETUP.md (1 hunks)
  • docs/FLUTTER_VERSION.md (1 hunks)
  • docs/INSTALL_FLUTTER.md (2 hunks)
  • docs/MULTIPLE_FLUTTER_VERSIONS.md (1 hunks)
  • docs/PROJECT_SETUP.md (2 hunks)
💤 Files with no reviewable changes (3)
  • .devcontainer/dev-setup.sh
  • .docker/kdf-android.dockerfile
  • .devcontainer/komodo-wallet-android-dev.dockerfile
✅ Files skipped from review due to trivial changes (2)
  • docs/FIREBASE_SETUP.md
  • .dockerignore
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/PROJECT_SETUP.md
🧰 Additional context used
🪛 LanguageTool
docs/INSTALL_FLUTTER.md

[misspelling] ~47-~47: Did you mean the verb “log out” instead of the noun ‘logout’?
Context: ...er\bin as its value. You might need to logout and re-login (or source the shell confi...

(LOG_IN)


[typographical] ~47-~47: Usually, there’s no comma before “if”.
Context: ... (or source the shell configuration file, if applicable) to make changes apply. On ...

(IF_NO_COMMA)

🪛 Hadolint (2.12.0)
.devcontainer/Dockerfile

[error] 72-72: Do not use sudo as it leads to unpredictable behavior. Use a tool like gosu to enforce root

(DL3004)

🔇 Additional comments (24)
.devcontainer/devcontainer.json (5)

4-4: Update Dockerfile Reference.
The dockerFile property now references "Dockerfile", which streamlines the build configuration. Ensure that the Dockerfile is appropriately configured at the repository root and includes all necessary instructions for the development environment.


6-8: Refined Workspace and Initialization Commands.
The updated "workspaceFolder" explicitly sets the working directory, and the new "postCreateCommand" applies correct ownership to ensure proper permissions. Additionally, replacing the previous post-attach script with "flutter pub get" in "postAttachCommand" simplifies dependency management. Verify that these changes are compatible across all development setups.


10-13: Enhanced Run Arguments for Device Access.
Adding "--privileged" along with the USB device volume mapping ("-v", "/dev/bus/usb:/dev/bus/usb") facilitates direct hardware access, which is critical for mobile and Flutter development. While useful, please ensure that running with privileged mode in the development container does not introduce unintended security risks.


14-17: Port Forwarding Configuration.
The addition of the forwardPorts property with ports 8081 and 5037 is well-suited for enabling live reload, debugging, and potentially ADB connectivity. Please verify that these forwarded ports do not conflict with host processes.


20-23: Streamlined VSCode Extensions.
The reduction of the VSCode extensions list to only include "Dart-Code.dart-code" and "Dart-Code.flutter" helps focus the development environment on core Flutter tooling. Confirm that any functionality provided by the removed extensions is either unnecessary or provided elsewhere in your workflow.

.docker/build.sh (3)

19-23: OS Check Update for macOS
The updated comparison using = for string equality is POSIX-compliant and works well for detecting macOS.


25-27: Update Android SDK Image Version
The Docker build command for the Android SDK image now tags it as komodo/android-sdk:35, reflecting the upgrade from version 34 to 35. Ensure that all related configurations (e.g., in Gradle files) are updated accordingly.


29-33: Streamline Flutter Build Command
The run command has been simplified by removing the flutter pub get && flutter analyze sequence. Please verify that this streamlined command still triggers all necessary build steps for asset downloads and analysis.

.docker/android-sdk.dockerfile (1)

28-32: Upgrade Android Build Tools and NDK Versions
The environment variables ANDROID_PLATFORM_VERSION, ANDROID_BUILD_TOOLS_VERSION, and ANDROID_NDK_VERSION have been updated to 35, 35.0.1, and 27.2.12479018 respectively. This change aligns with newer Android SDK capabilities.

docs/FLUTTER_VERSION.md (2)

7-8: Update Flutter Version Documentation
The documented current version has been updated from 3.22.3 to 3.29.0, which clearly informs users of the new stable release.


11-13: Update Git Checkout Command
The instructions now recommend using git checkout 3.29.0 to switch to the updated Flutter version. This is clear and consistent with the version bump.

docs/BUILD_RUN_APP.md (2)

110-121: Enhance Windows Desktop Prerequisites Documentation
The added prerequisites for Windows desktop builds—including Visual Studio Community 17.13.0 with the Desktop development with C++ workload and NuGet CLI installation via winget—provide clearer setup instructions for Windows users.


182-210: Improve Robustness of PKG_CONFIG_PATH Setup on Linux
The updated Linux instructions now include installation of additional dependencies and modifications to PKG_CONFIG_PATH. Currently, the snippet checks only if PKG_CONFIG_PATH is empty before appending a fixed value. Consider enhancing this logic by verifying and appending missing paths instead of overwriting partial configurations.

.docker/komodo-wallet-android.dockerfile (1)

1-6: Environment and Base Image Update:
The update to the base image (komodo/android-sdk:35) along with the new environment variables (FLUTTER_VERSION set to "3.29.0", and HOME set to "/home/komodo") is clear and in line with the updated Android SDK requirements. The adjustment of the PATH variable to include Flutter’s binary directory is correctly implemented.

docs/INSTALL_FLUTTER.md (5)

9-10: Enhanced Installation Options:
The introduction of multiple installation options—using VS Code, the SDK Archive, or FVM—provides comprehensive guidance for various user preferences. This clarity is beneficial for new users.


11-14: VS Code Installation Section Clarity:
The new "Use VS Code to install" section effectively recommends the Flutter extension for VS Code (or Android Studio/IntelliJ), streamlining the process for beginners.


15-21: Download and Install Section:
The "Download and install" section is detailed—with links for Windows, Mac, and Linux—and clearly instructs users on how to obtain and set up the Flutter SDK manually. The structured layout aids readability.


40-46: Windows PATH Update Instructions:
The Windows instructions, laid out as a bulleted list, clearly describe the steps to modify the PATH environment variable via the system settings. This approach is user-friendly.


49-51: Verification Reminder:
The tip to verify the PATH setup by running which flutter on macOS and Linux is a helpful addition for troubleshooting.

docs/MULTIPLE_FLUTTER_VERSIONS.md (5)

6-9: macOS and Linux Instructions:
The instructions under the "macOS and Linux" section are clear and informative. The description of FVM’s role and the consolidated approach for local and global SDK management are well presented.


12-17: Bash Code Block for FVM Installation:
The bash code block that installs FVM using the installation script and then configures the Flutter version for the current directory is succinct and directly addresses user needs.


19-21: Section Separator and Windows Introduction:
The use of a separator (----) effectively distinguishes the sections, and the introduction to the Windows section is clear and sets up the following instructions well.


25-29: Chocolatey Installation Instructions:
The instructions for installing Chocolatey via a PowerShell command are detailed and provide a necessary prerequisite for Windows users, ensuring a smoother FVM installation process.


31-37: Windows FVM Installation Instructions:
The PowerShell code block for installing FVM using Chocolatey, followed by commands to configure and check the Flutter setup, is clear and effective.

CharlVS and others added 5 commits February 26, 2025 15:31
…dows (#2549)

* feature: remove `bip39` package and migrate to SDK

Remove `bip39` package and migrate seed validation to SDK

* fix: Remove seed from registration

- Remove the seed parameter from registration since KDF is responsible for generating seeds.
- Update to reference the correct bloc event.
- Change default wallet type to HD.
Upgrade the KDF SDK version for bug fixes.
* Fix some grammar in the EULA/ToC

* rm surplus `-`
* ci: add initial mobile and desktop workflows

* ci: fix apple-actions action name

* fix macos and ios builds

* change xcode version to latest stable

* fix merge issue and job issues

* fix android build

- remove mm2 linking in cpp, build.gradle, and MainActivity

* add debug builds on error & fix ios build order

* docs: add windows dependencies

* upgrade to flutter 3.27.0

* ci: add github token to platform build step

* ci: let flutter pub errors through for debugging

errors at the flutter pub get step currently cause the ci steps to fail with an exit code, but no logs pointing to the cause

* build(windows): add flutter build artefact changes

* ci: fix code validation warnings

* remove unnecessary asset declarations

missing folders result in build "errors" that act like warnings, but cause the builds on windows to fail

* ci: fix missing desktop archives

* ci: remove web validation step for mobile & desktop workflows

validation step only supports web for now

* ci: fix desktop build output path mismatch

* build(desktop): move executable copy step to kdf plugin

* build(android): bump jvm and gradle to latest stable versions

* chore(deps): bump sdk version

* chore(deps): fix iOS build by updating firebase packages

* build(android): upgrade jvm, kotlin, and video_player

* build(android): upgrade gradle plugin and kotlin language versions

gradle v8.8 and kotlin language v2.1.10

* ci: switch ui & unit test workflows to hosted runners

align with changes previously on dev/main, seeing as self-hosted runners are disabled for the time being

* ci(ios): add xcode simulator download to setup script

* ci(desktop): use the default platform for asset fetching/generation

* build: remove api section from build_config as it is not referenced

The build config is now primarily used in the SDK komodo_defi_framework package, and not in this repository

* ci: add repository check to certificate and signing setup steps

Copy over behaviour from komodo-wallet-desktop repository

* ci: fix coderabbitai nitpick comments and split signing documentation

* chore(docs): Clarify/expand Flutter version instructions

* chore(deps): update firebase for ios and macos

* ci(generate-assets): refactor build platform to build command input

reduce ci build time

* ci(ios): fix build error by defaulting to building web first

---------

Co-authored-by: Francois <[email protected]>
Co-authored-by: CharlVS <[email protected]>
Copy link

@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: 1

🧹 Nitpick comments (9)
docs/FLUTTER_VERSION.md (1)

13-28: Alternative Pinning Section:
The alternative section is well written. It clearly explains why pinning (i.e. locking to a specific Flutter version) is not recommended, and the command snippet (switching via “git checkout 3.29.0”) is up-to-date. Consider a brief troubleshooting note for users who may still face issues after switching versions.

.github/actions/releases/setup-windows/action.yml (2)

5-11: Input Parameters:
The inputs for the Base64-encoded PFX certificate and its password are descriptive and correctly marked as not required. You might consider adding additional validation or usage examples in the documentation for clarity.


30-38: Windows Code Signing Step:
The conditional step for setting up codesigning is well structured. The script decodes the certificate, converts the provided password to a secure string, imports the certificate, and removes the temporary file. To further improve resilience, you might add error handling (for example, checking if the Import-PfxCertificate command fails) to report issues during the import process.

.github/workflows/mobile-builds.yml (1)

59-65: Asset Generation Step and Trailing Spaces

The step to fetch packages and generate assets via the custom action is well implemented.

Note: Line 63 contains trailing spaces. Although this does not affect functionality, please remove them to comply with YAML linting standards.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 63-63: trailing spaces

(trailing-spaces)

docs/SIGN_RELEASE.md (2)

5-11: Android Keystore Generation

The instructions for generating a keystore with keytool are clear. For added clarity, consider briefly explaining key parameters to help new users.


102-112: Linux Signing Instructions

The Linux section covers GPG key generation and exporting correctly. For completeness, you might consider adding best practices for GPG key storage and usage.

.github/actions/validate-build/action.yml (1)

31-46: AssetManifest.bin and Coin Icon Check.
The script validates the presence of AssetManifest.bin and verifies that it includes the expected coin icon (assets/coin_icons/png/kmd.png). Note that the inner check (lines 42–45) appears redundant; consider streamlining that block to avoid duplication.

.github/actions/generate-assets/action.yml (1)

29-30: Remove Trailing Whitespace.
There are trailing spaces on these lines. For cleaner YAML formatting and to avoid potential linting issues, please remove any trailing spaces.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 29-29: trailing spaces

(trailing-spaces)


[error] 30-30: trailing spaces

(trailing-spaces)

.github/workflows/ui-tests-on-pr.yml (1)

95-105: Coverage Report Generation is Temporarily Disabled with Clear Documentation

The commented-out "Generate coverage report" block (lines 95–105) includes explanatory comments regarding issues with Hive and storage providers. This is helpful for future reference once the integration test coverage is fixed. Consider tracking this in an issue if not already done.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 92cb33e and 21b82ba.

⛔ Files ignored due to path filters (1)
  • ios/Podfile.lock is excluded by !**/*.lock
📒 Files selected for processing (25)
  • .github/actions/flutter-deps/action.yml (1 hunks)
  • .github/actions/generate-assets/action.yml (1 hunks)
  • .github/actions/releases/setup-android/action.yml (1 hunks)
  • .github/actions/releases/setup-ios/action.yml (1 hunks)
  • .github/actions/releases/setup-linux/action.yml (1 hunks)
  • .github/actions/releases/setup-macos/action.yml (1 hunks)
  • .github/actions/releases/setup-windows/action.yml (1 hunks)
  • .github/actions/validate-build/action.yml (1 hunks)
  • .github/workflows/desktop-builds.yml (1 hunks)
  • .github/workflows/mobile-builds.yml (1 hunks)
  • .github/workflows/ui-tests-on-pr.yml (1 hunks)
  • .github/workflows/unit-tests-on-pr.yml (1 hunks)
  • .gitignore (3 hunks)
  • android/app/build.gradle (2 hunks)
  • android/app/src/main/cpp/CMakeLists.txt (0 hunks)
  • android/app/src/main/cpp/mm2_native.cpp (0 hunks)
  • android/app/src/main/java/com/komodoplatform/atomicdex/MainActivity.java (0 hunks)
  • android/gradle/wrapper/gradle-wrapper.properties (1 hunks)
  • android/settings.gradle (1 hunks)
  • app_build/build_config.json (1 hunks)
  • docs/BUILD_RELEASE.md (2 hunks)
  • docs/FLUTTER_VERSION.md (1 hunks)
  • docs/MULTIPLE_FLUTTER_VERSIONS.md (1 hunks)
  • docs/SIGN_RELEASE.md (1 hunks)
  • lib/bloc/analytics/analytics_repo.dart (2 hunks)
💤 Files with no reviewable changes (3)
  • android/app/src/main/cpp/mm2_native.cpp
  • android/app/src/main/cpp/CMakeLists.txt
  • android/app/src/main/java/com/komodoplatform/atomicdex/MainActivity.java
✅ Files skipped from review due to trivial changes (1)
  • docs/BUILD_RELEASE.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/bloc/analytics/analytics_repo.dart
🧰 Additional context used
🧠 Learnings (1)
android/app/build.gradle (1)
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2526
File: android/app/build.gradle:28-28
Timestamp: 2025-02-17T06:53:06.737Z
Learning: Android 15 (API Level 35) was released on October 15, 2024, making it a valid target for Android applications.
🪛 YAMLlint (1.35.1)
.github/workflows/mobile-builds.yml

[error] 63-63: trailing spaces

(trailing-spaces)

.github/actions/generate-assets/action.yml

[error] 29-29: trailing spaces

(trailing-spaces)


[error] 30-30: trailing spaces

(trailing-spaces)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Build Mobile (Android)
  • GitHub Check: Build Desktop (windows)
  • GitHub Check: Build Mobile (iOS)
🔇 Additional comments (111)
android/settings.gradle (1)

21-24: Update Gradle and Kotlin plugin versions.
The Android application plugin version is updated to “8.8.0” and the Kotlin Android plugin to “2.1.10”, with helpful comments linking to release notes. Please verify that these versions are compatible with all modules (e.g. the corresponding changes in the app’s build files).

android/gradle/wrapper/gradle-wrapper.properties (1)

3-4: Upgrade Gradle distribution version.
The distribution URL has been updated to “gradle-8.12.1-bin.zip” and the checksum adjusted accordingly. Ensure that the new checksum correctly matches the distribution file from Gradle’s release.

app_build/build_config.json (1)

3-7: Update coins configuration for build time fetching and repository URL.
A new property "fetch_at_build_enabled": true has been added under the coins section, and the repository URL for coin assets is updated from the GitHub raw URL to “https://komodoplatform.github.io/coins”. This aligns the configuration with the intended asset hosting location.

.github/actions/releases/setup-linux/action.yml (4)

2-12: Introduce “Setup Linux Build Environment” action.
The action is defined with two optional inputs for GPG key and key ID. The description clearly states the purpose. No issues detected in the input definitions.


16-24: Install system dependencies step looks robust.
The step installs a comprehensive list of Linux packages (including tools like ninja-build, libgtk-3-dev, clang, etc.). Consider verifying that all required libraries (e.g. for Flutter and code signing) are present on your target runners.


25-30: Ensure Flutter tool setup via “Install Linux build dependencies.”
The use of “flutter pub get” and “flutter doctor” is a good minimal check that Flutter is set up correctly.


31-39: Conditional Linux code signing step.
This step conditionally imports a provided GPG key and sets up the GPG agent. The conditional using if: inputs.gpg-key != '' && 'KomodoPlatform/komodo-wallet' == github.repository is clear. Verify that the destination paths (like ~/.devscripts) and configuration (e.g. “allow-preset-passphrase”) meet your security requirements.

.github/actions/releases/setup-macos/action.yml (7)

1-4: New “Setup macOS Build Environment” action header.
The action is introduced with a clear name and description to configure Flutter and its dependencies on macOS.


6-33: Input parameters are comprehensive and well defined.
Inputs for Xcode version, certificate (P12) file and password, bundle ID, provisioning profile type, and App Store Connect credentials are specified. The use of defaults (e.g. "latest-stable" for Xcode version and "com.komodo.wallet" for bundle-id) is appropriate.


37-41: Xcode setup step is correctly implemented.
Using the external action maxim-lobanov/setup-xcode@v1 with an input based on xcode-version ensures that the proper Xcode version is installed.


42-47: Configure Xcode command line tools.
The shell step switches to the proper developer directory and accepts the Xcode license. This is critical for non-interactive build environments.


48-54: Installation of macOS build dependencies.
The step runs flutter pub get and then enters the macos folder to perform a pod install, which is standard for Flutter macOS projects.


55-64: Import macOS certificate action.
This step conditionally invokes the action apple-actions/import-codesign-certs@v3 to import signing certificates. Using the github.run_id as both keychain and keychain-password is an interesting choice; please check that this meets your security and reproducibility standards.


65-73: Download macOS provisioning profile.
The action downloads the provisioning profile using inputs for bundle-id, profile type, and App Store Connect credentials. The condition ensures this only runs when the issuer ID is provided and the repository matches, which is appropriate.

docs/FLUTTER_VERSION.md (1)

1-12: Clarity & Updated Version Information:
The “Flutter Version Management” document now clearly states the supported version (Flutter 3.29.0) and outlines the recommended approach of using a version manager. The section hierarchy and the call to the guide on multiple Flutter versions help guide users to the detailed documentation.

.github/actions/releases/setup-windows/action.yml (4)

1-4: Workflow Metadata & Description:
The workflow name “Setup Windows Build Environment” and its description clearly reflect the purpose of configuring the Flutter environment and dependencies for Windows builds.


16-18: Visual Studio Setup Step:
The step using microsoft/[email protected] is straightforward and is an appropriate method to ensure Visual Studio is ready for builds.


19-23: Windows SDK Installation:
Using GuillaumeFalourd/setup-windows10-sdk-action@v2 with the given sdk-version is correctly implemented to install the required Windows SDK.


24-29: Build Dependencies Installation:
The PowerShell step running flutter pub get and flutter doctor covers the essential dependency checks. It’s concise yet effective in initializing the build environment.

.github/actions/releases/setup-ios/action.yml (6)

1-4: Workflow Metadata & Description:
The workflow “Setup iOS Build Environment” is clearly named and described to configure Xcode and manage iOS signing requirements. This change lays a good foundation for automating iOS build setups.


5-33: Input Parameters:
All inputs—including the Xcode version, certificate details, bundle-id, profile type, and App Store Connect credentials—are clearly defined with appropriate defaults. This will allow users to flexibly configure the iOS build environment.


34-41: Xcode Setup Step:
Using the maxim-lobanov/setup-xcode@v1 action to set up the specified Xcode version (defaulting to “latest-stable”) is an optimal choice. This ensures the build environment meets current Xcode requirements.


42-52: Install iOS Dependencies:
The added step downloads the iOS platform via xcodebuild -downloadPlatform iOS, runs flutter pub get, and then changes directory to ios for pod install. This sequence helps mitigate issues with missing simulator platforms in Xcode 15 and ensures CocoaPods dependencies are correctly installed.


53-59: Import iOS Certificates:
The conditional step for importing iOS certificates using apple-actions/import-codesign-certs@v3 is implemented correctly. The condition ensures that this step runs only if the certificate is provided and if the repository matches “KomodoPlatform/komodo-wallet”.


60-69: Download Provisioning Profile:
This step downloads the provisioning profile using apple-actions/download-provisioning-profiles@v1 conditioned on a provided issuer ID. All necessary inputs (bundle-id, profile-type, issuer-id, api-key-id, api-private-key) are passed correctly. This configuration should streamline provisioning profile management for iOS builds.

.github/workflows/mobile-builds.yml (5)

1-10: Workflow Metadata and Trigger Configuration

The workflow defines its name, run name, and triggers (pull_request, workflow_dispatch, release) clearly. Verify that the branch patterns and release types match your intended deployment strategies.


31-37: Checkout and Dependencies Setup

The checkout step using actions/checkout@v4 and the subsequent installation of Flutter dependencies via the custom action are correctly implemented.


38-49: Platform-specific iOS Environment Setup

The iOS environment is properly configured with secrets for the P12 file, password, bundle ID, and related App Store credentials. Ensure that the values (e.g. "IOS_APP_STORE") match your deployment requirements.


50-58: Platform-specific Android Environment Setup

The Android environment setup correctly references the keystore credentials via secrets. No issues are identified in this segment.


66-77: Build and Artifact Upload Steps

The build step runs using the matrix variable for the build command, and the artifact upload is configured with a five‐day retention period. Everything appears to be in order.

.github/actions/flutter-deps/action.yml (3)

1-5: Action Metadata and Composite Configuration

The action's metadata (name and description) and the composite run configuration are clearly defined.


6-11: Stable Flutter Installation Step

The step uses subosito/flutter-action@v2 with a fixed version ("3.29.x") from the stable channel. This is a robust choice for consistent Flutter installs.


12-20: Prepare Build Directory Step

The bash step cleans previous build artifacts and removes specific directories (e.g. web/src/mm2/*, web/src/kdfi/*, web/dist/*). Verify that these paths correctly reflect your project structure.

.github/workflows/unit-tests-on-pr.yml (8)

1-4: Unit Test Workflow Metadata

The workflow name and personalized run name using ${{ github.actor }} are well configured to trigger on the proper pull request events.


10-12: Job Configuration

The unit_tests job is set to run on ubuntu-latest with an appropriate timeout. This configuration is straightforward and effective.


15-17: Code Checkout Step

The checkout step using actions/checkout@v4 is correctly implemented.


18-20: Flutter Dependencies Installation

The custom action to install Flutter and its dependencies ensures consistency in your test environment.


21-25: Asset Generation Step

The asset generation step uses the custom action with the GitHub token passed securely. The configuration meets the requirements.


26-28: Build Validation Step

The use of the custom action for validating the build encapsulates the necessary checks, contributing to a cleaner workflow.


29-37: Unit Test Execution Step

The testing step uses the provided environment variable and runs the tests on test_units/main.dart. Confirm the file path is correct for your unit tests.


38-44: Unit Test Coverage Report Generation

Generating the coverage report via the custom action is well executed, with an explicit test_file parameter ensuring the proper test run.

android/app/build.gradle (5)

25-27: Android Build Configuration Header

The initial section that sets the namespace and Flutter version details is correctly configured.


28-35: SDK and Language Version Updates

The compileSdk is now updated to 35, and the compileOptions have been shifted to use JavaVersion.VERSION_21 for both source and target compatibility. Similarly, the Kotlin jvmTarget is now set to '21'.

Verification Required: Double-check that all your dependencies and Gradle plugins are compatible with Java 21 since this is an uncommon jump compared to the more standard versions.


36-38: Source Sets Configuration

The modification of the sourceSets (adding src/main/kotlin) improves Kotlin integration; this is standard and acceptable.


40-46: Application Default Configuration

The defaultConfig block properly defines the applicationId, minSdkVersion (28), and updates the targetSdkVersion to 35. Versioning via Flutter properties is handled correctly.


47-52: NDK Version Update

The ndkVersion is updated to 27.2.12479018, aligning with recent stable releases. Ensure your native components remain fully compatible with this version.

docs/SIGN_RELEASE.md (12)

1-4: New Signing Release Documentation

This new file introduces a comprehensive guide for signing builds across platforms. The overall structure—with distinct sections for Android, iOS/macOS, Windows, and Linux—is clear and well organized.


12-16: Convert Keystore to Base64

The provided base64 conversion step is accurate. Ensure that file permissions and security aspects are appropriately managed after conversion.


17-22: Keystore Validation

Using keytool -list to validate the keystore is a solid approach. You might add common troubleshooting hints for potential errors.


32-35: Android Signing Resources

The reference link to the Android Signing Guide is valuable for further details.


36-55: iOS/macOS Signing Instructions

The guide covers the necessary steps—from creating an Apple Developer account to exporting certificates. The instructions are clear and logically ordered.


56-64: iOS/macOS Signing Secrets Example

The YAML snippet for iOS/macOS secrets (IOS_P12_BASE64, IOS_P12_PASSWORD, etc.) is comprehensive. Ensure the private keys and certificates are maintained securely.


65-73: iOS/macOS Signing Documentation

The provided links to external resources enhance this section. They offer good additional guidance for users who may need more details.


74-83: Windows Code Signing Instructions

The Windows section correctly outlines the process to export and convert a PFX certificate for signing. The validation command using signtool is also well explained.


84-95: Windows Signing Secrets Example

The YAML example for Windows signing is clear. As with other secret examples, verify that placeholder values are replaced before production use.


96-101: Windows Signing Resources

The documentation links for Windows code signing provide useful additional context.


113-121: Linux Signing Validation

The validation command using GPG is clear. Confirm that instructions include any prerequisites for installing GPG on various distributions.


122-179: Summary of GitHub Workflow Secrets

The summary tables categorizing secrets for iOS, macOS, Android, Windows, and Linux provide a clear resource for managing CI/CD secrets. This greatly increases the maintainability of your build pipelines.

.github/workflows/desktop-builds.yml (6)

1-10: Workflow Metadata and Triggers.
The workflow is clearly defined with proper triggers (pull requests, manual dispatch, and release creation), and the run‐name provides good context.


18-34: Matrix Strategy and Platform Configuration.
The matrix correctly enumerates macOS, Windows, and Linux configurations with appropriate runner images, build commands, artifact paths, and artifact names. This setup will ensure each desktop build is run under its designated environment.


36-41: Checkout and Dependency Installation.
Using actions/checkout@v4 followed by invoking the local flutter-deps composite action ensures that the repository is checked out and the Flutter environment is properly set up before any platform-specific actions occur.


42-70: Platform-Specific Setup Steps.
The conditional steps for macOS (with signing details), Linux, and Windows are all well implemented. The use of secrets in each platform’s configuration appears secure and meets best practices. Verify that the secret names match what’s expected in your CI/CD settings.


71-76: Asset Generation Action Invocation.
The step that calls the generate-assets action with appropriate inputs (GitHub token and build command) is clear and modular. This promotes reusability in the workflow.


77-88: Build Execution and Artifact Upload.
Running the build command via an environment variable and then uploading artifacts (with a defined retention period) is implemented correctly. This helps ensure that the correct build output is captured and archived.

.github/actions/releases/setup-android/action.yml (5)

1-4: Action Identification and Description.
The action is well named ("Setup Android Build Environment") and includes a concise description to explain its purpose.


5-21: Input Parameters Configuration.
All inputs (including java-version, keystore-base64, key-alias, store-password, and key-password) are clearly defined with appropriate descriptions and a default for the Java version. This makes the action flexible for different Android build scenarios.


23-32: JDK Setup Step.
The step using actions/setup-java@v4 is properly configured with the input variable for Java version, temurin for distribution, and caching enabled for Gradle. This should help reduce build times.


33-35: Android SDK Installation.
Using the android-actions/setup-android@v3 action correctly establishes the Android SDK environment.


36-46: Android Keystore Setup.
The conditional step that decodes the Base64 keystore and writes out the android/key.properties file is clear and robust. Removing any previous version of the file with rm -f helps avoid duplication issues.

.github/actions/validate-build/action.yml (6)

1-3: Action Metadata and Description.
The action is aptly named "Validate build" and the description clearly states its purpose—to check for all necessary build files.


10-23: WebAssembly File Validation.
The script checks for the existence of a wasm file in the expected directory and provides directory listings for debugging when missing. This is valuable for diagnosing build issues with the web target.


25-29: Index.html Comparison.
Using cmp -s to ensure that the generated build/web/index.html matches the source web/index.html is a strict but effective check for build consistency.


48-57: build_config.json Validation.
The action ensures that app_build/build_config.json exists and is valid JSON using jq. This helps catch any formatting errors early in the build process.


59-68: coins.json Validation.
The script confirms the existence and JSON structural integrity of $COIN_ASSETS_DIR/config/coins.json. This is critical for proper coin asset handling.


70-79: coins_config.json Validation.
Similarly, checking for both the presence and valid JSON formatting of $COIN_ASSETS_DIR/config/coins_config.json ensures that the necessary configuration is in place.

.github/actions/generate-assets/action.yml (8)

1-3: Action Definition and Metadata.
The composite action is clearly defined with the name "Generates assets" and a useful description that explains its role in transforming and generating assets.


4-11: Input Parameters Configuration.
The inputs for GITHUB_TOKEN (required) and BUILD_COMMAND (with a default value) are precisely defined. This allows the action to be flexible while ensuring that critical parameters are provided.


12-14: Composite Runner Configuration.
The action correctly sets up the composite runner with the using: "composite" method.


15-19: Environment Variable Setup for Secure Token Usage.
Mapping GITHUB_API_PUBLIC_READONLY_TOKEN directly from the input variable optimizes secure token usage in the subsequent build steps.


20-27: Token Validation Logic.
The run script checks if the GitHub token is provided and prints the appropriate message. The conditional unsetting of the variable when not provided is a good touch to ensure no empty values affect downstream commands.


32-34: Preliminary Flutter Build Execution.
By running flutter pub get and the build command once (and allowing failure via || true), the action pre-downloads coin icons and configuration files. This preliminary step is clever and sets up the second build execution.


36-42: Final Build Execution and Output Capture.
The script captures the output and exit status from running the build command a second time. This is an effective way to halt the process upon encountering genuine errors while allowing known expected failures during the first run.


43-49: Error Handling and Completion Confirmation.
If the build exit status is non-zero, the script prints the output and exits with the given status. The final echo statement confirms successful asset generation.

.gitignore (5)

1-3: Excluding First-Party SDK Files.
Adding the sdk/ entry is a solid change to ensure that generated or bundled SDK artifacts are not committed to version control.


41-42: Ignoring DevTools Configuration Files.
The addition of devtools_options.yaml helps prevent unwanted local configuration files from being tracked.


44-45: Refined Web Ignore Patterns.
Ignoring web/index.html and all files under web/src/ prevents build artifacts or source files that are regenerated during deployment from being committed. Confirm that these files are automatically rebuilt as needed.


81-86: Excluding Environment and Build Caches.
The new ignore entries for venv/, .fvm/, and /macos/build keep development caches and build directories out of version control, which is ideal for maintaining a clean repository.


40-50: Asset Tracking Adjustments.
The removal of ignore rules for asset configuration directories (e.g. assets/config/) and coin icon directories (e.g. assets/coin_icons/) ensures that these critical assets are tracked by Git and available to the project.

docs/MULTIPLE_FLUTTER_VERSIONS.md (12)

1-3: Title & Introduction are Clear and Informative

The updated title "Managing Multiple Flutter Versions" clearly reflects the content, and the introductory paragraph on lines 1–3 concisely describes the purpose of the document.


5-7: List of Recommended Tools is Well-Structured

The list introducing Flutter Sidekick and FVM (Flutter Version Manager) is clear and helps set expectations for readers with different expertise.


8-11: Pre-Installation Instructions Outlined Effectively

The "Before You Begin: Remove Existing Flutter Installations" section (lines 8–11) provides necessary context to avoid conflicts. This sets a good foundation for the subsequent installation steps.


12-22: macOS Uninstallation Steps are Clear

The instructions for macOS (lines 12–22), including removal via Git and Homebrew, are well-explained. The code block format for the commands enhances readability.


24-32: Windows Uninstallation Instructions are Detailed

The Windows section (lines 24–32) efficiently explains both manual uninstallation and the winget approach. This provides users with multiple options depending on their installation method.


34-45: Linux Uninstallation Guidelines are Comprehensive

The Linux instructions (lines 34–45) cover both Git-based and package-manager installations, along with guidance on updating the PATH. These details will help avoid configuration conflicts.


47-67: Flutter Sidekick Section is User-Friendly

The "Option 1: Flutter Sidekick" section (lines 47–67) is well-organized. The step-by-step installation instructions, including setting the default version and updating the PATH, are clear and accessible for beginners.


73-81: FVM Overview Provides Clarity on Advanced Usage

The introductory part of "Option 2: FVM Command Line" (lines 73–81) succinctly explains the advantages of FVM for CI/CD and non-GUI environments.


82-109: macOS & Linux FVM Setup is Accurate and Detailed

The FVM installation and configuration instructions for macOS/Linux (lines 82–109) are comprehensive—from installing via a script to configuring the shell PATH and verifying Flutter. This section should serve as a reliable guide for users on these platforms.


110-137: Windows FVM Setup Provides Clear Guidance

The step-by-step instructions for installing FVM on Windows (lines 110–137) are thorough and include specific commands (e.g., using Chocolatey). The details regarding PATH configuration are particularly helpful.


138-148: Project-Specific Flutter Version Instructions are Straightforward

The section on enforcing a project-specific Flutter version (lines 138–148) is clear. Explaining the purpose of the generated .fvmrc file helps users understand the benefits of this configuration.


150-170: VS Code Integration Section Enhances Developer Experience

The "Using with VS Code" section (lines 150–170) is well-detailed. The instructions for installing the Flutter FVM extension and configuring the .vscode/settings.json file are clear. This ensures that users can seamlessly integrate Flutter version management with their IDE.

.github/workflows/ui-tests-on-pr.yml (11)

1-4: Workflow Header Update is Effective

The updated workflow name "UI Integration tests on PR" and the dynamic run-name (including the GitHub actor) improve clarity and traceability of test runs. These changes set a positive tone for the CI process.


16-29: Matrix Configuration for web-app-linux-profile is Well-Defined

The new matrix entry for "web-app-linux-profile" (lines 16–29) clearly specifies parameters for the Chrome-based tests, including browser, display mode, resolution, and driver log paths. This granularity is beneficial for targeted testing scenarios.


30-40: Matrix Entry for web-app-macos is Clear and Contextualized

The matrix entry for "web-app-macos" (lines 30–40) specifies appropriate settings for Safari, including comments noting that display and resolution settings do not affect safaridriver. This contextual guidance is useful.


45-54: Conditional Setup for Chrome and Chromedriver is Appropriately Scoped

The step "Install Chrome and chromedriver" (lines 45–54) uses a conditional check for Chrome-based tests. The inclusion of parameters like chrome-version and automatic dependency installation is precise and should streamline the test environment setup.


55-62: Safaridriver Enablement Step is Necessary for macOS

The conditional step to enable safaridriver (lines 55–62) is correctly gated for Safari testing. The use of the defaults write commands, along with a fallback echo statement on failure, provides a robust setup for macOS environments.


63-65: Flutter Dependencies Step Modularizes the Setup

The "Install Flutter and dependencies" step (lines 63–65) leverages a dedicated action. This modularity should help maintain the workflow and allows for centralized updates to Flutter dependency handling.


66-70: Separation of Package Fetching and Asset Generation Improves Readability

The "Fetch packages and generate assets" step (lines 66–70) is clearly delineated from the Flutter dependencies installation, which improves the maintainability of the workflow by isolating asset generation logic.


71-73: Build Validation Action Enhances Confidence in the Release Process

The "Validate build" step (lines 71–73) encapsulates build-validation into a reusable action. This abstraction aligns with best practices by decoupling validation logic from testing logic.


74-86: Integration Test Execution is Dynamic and Configurable

The integration tests step (lines 74–86) uses matrix variables effectively to pass dynamic parameters (such as display mode, resolution, browser, and mode) to the test runner. This design supports diverse testing environments and configurations.


88-94: Driver Logs Upload Step is Well-Implemented

The "Upload driver logs" step (lines 88–94) correctly uses an artifact upload action and conditionally warns if no files are found. This ensures that logs are captured for troubleshooting without halting the workflow unnecessarily.


106-109: Fail Workflow Step Ensures Test Reliability

The final step "Fail workflow if tests failed" (lines 106–109) appropriately terminates the workflow when tests do not pass. This explicit check reinforces the reliability of the CI process.

Comment on lines +23 to +30
Example secrets:

```yaml
ANDROID_KEYSTORE_BASE64: "/u3+7QAAAAIAAAABAAAAAQAHa29tb2RvAAABjK6LSU8AAAUBMIIE..."
ANDROID_KEY_ALIAS: "komodo"
ANDROID_STORE_PASSWORD: "your-keystore-password"
ANDROID_KEY_PASSWORD: "your-key-password"
```
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Android Signing Secrets Example

The YAML example with keys (ANDROID_KEYSTORE_BASE64, ANDROID_KEY_ALIAS, etc.) is informative. Make sure to replace any placeholder values with actual secrets and add a comment to warn against committing sensitive information.

CharlVS and others added 2 commits March 3, 2025 01:26
#2556)

Role KDF SDK to` af79b7d09ab3f3b45ff6767b9556c514f09b25ce ` to fix Windows crashes for devices with Nvidia GPUs. Bug report to be filed with Nvidia.
…eated wallets (#2553)

* fix(auth): store wallet backup status value instead of constant

* fix(show-seed): replace texteditingcontroller with initial value

focus/text controller handling logic change in the uitextformfield resulted in this small regression

* refactor(current-wallet-bloc): replace final references with SDK

required to move the seed backup to auth bloc to fix issue with banner not disappearing

* refactor(auth): move remaining current wallet functions to auth bloc

* fix(auth): add backup confirmed to manual seed confirmation step
Copy link

@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: 2

🧹 Nitpick comments (14)
lib/bloc/coins_bloc/coins_bloc.dart (4)

74-105: Authenticate pubkey retrieval failures more granularly.

The _onCoinsPubkeysRequested method logs a generic error for any exception. In production, certain errors (e.g., network failure vs. invalid coin references) might require different handling or user messages. Consider extending the catch block to handle known SDK exceptions more transparently before falling back to a generic error.


106-119: Evaluate periodic triggers and resource usage.

A periodic timer is set up in _onCoinsStarted to fetch coin prices every minute. Make sure this frequency balances real-time needs versus resource consumption, particularly if the operation is cost-intensive or if many coins are tracked.


322-373: Review session management consistency.

_onLogin and _onLogout flush caches and modify in-memory states. Ensure that external references (UI views, other blocs) are notified or invalidated accordingly to prevent showing stale data or introducing race conditions if these events happen in rapid succession.


375-415: Combine activation results for collective error handling.

When _activateCoins processes multiple coins, partial success may leave some coins active and others not. Currently, each result is emitted incrementally. Consider returning a combined result object capturing success/failure for each coin to allow more comprehensive feedback or error display in the UI.

lib/bloc/security_settings/security_settings_state.dart (2)

4-18: Ensure consistent naming and ordering of enum values.

The new enum values seedShow, seedConfirm, and seedSuccess are logically consistent. Confirm that future steps—e.g., passwordUpdate—are not needed in the immediate flow or that the comment-out is intentional. Overly commented-out code blocks can lead to confusion.


22-44: Validate default states for new fields.

showSeedWords and isSeedSaved are crucial for controlling security-sensitive flows. Confirm their default values (both false in initialState) match the intended user flow. If there's any scenario where a newly created account is considered “seedSaved,” consider adjusting accordingly or adding logic in the UI.

lib/bloc/nft_receive/bloc/nft_receive_bloc.dart (2)

3-4: Check import order consistency.

The new imports for KomodoDefiSdk and CoinsRepo appear correct. However, ensure your coding guidelines do not require grouping of external/package imports together and local project imports in a separate group.


14-26: Clarify fallback behaviors on coin not found.

In _onInitial, if _coinsRepo.getCoin(abbr) returns null, the code simply emits NftReceiveFailure. Consider including a more descriptive failure message or logs about the missing coin to facilitate debugging.

lib/bloc/app_bloc_root.dart (3)

118-118: Offer assistance with the TODO comment.

The comment says “SDK Port needed, not sure about this part.” If you have any pending tasks or open questions regarding the migration to KomodoDefiSdk, consider creating a dedicated issue/ticket to track or clarify the port strategy.

Would you like help drafting an outline or sample code for the missing port?


326-337: Debug initialization is clear but can be cleaner.

Placing debug-related logic (wallet fetching, auth testing) under the kDebugMode flag is sensible. However, consider extracting it into a dedicated method or file to avoid clutter here as the debug logic grows.


363-364: Refine concurrency in _precacheCoinIcons.

When _precacheCoinIcons detects a new request mid-operation, it completes the current Completer abruptly. This may lead to partial precaching for the earlier request. Consider a more robust approach to queue or cancel the existing operation gracefully.

Also applies to: 391-403

lib/bloc/security_settings/security_settings_bloc.dart (2)

38-41: Method signature updated to async, but no awaits used.

The method has been changed to return Future<void> and marked as async, but there are no await expressions inside it. While this doesn't cause any functional issues, it's unnecessary to mark a method as async if it doesn't need to wait for any asynchronous operations.

If there are no plans to add asynchronous operations to this method in the future, consider changing it back to:

-  Future<void> _onShowSeedWords(
+  void _onShowSeedWords(
     ShowSeedWordsEvent event,
     Emitter<SecuritySettingsState> emit,
-  ) async {
+  ) {

85-88: Method signature updated to async without awaits.

Similar to _onShowSeedWords, this method has been changed to return Future<void> and marked as async, but contains no await expressions. This is unnecessary if no asynchronous operations are being performed.

If there are no plans to add asynchronous operations, consider changing it back to:

-  Future<void> _onSeedCopied(
+  void _onSeedCopied(
     ShowSeedCopiedEvent event,
     Emitter<SecuritySettingsState> emit,
-  ) async {
+  ) {
lib/bloc/auth_bloc/auth_bloc.dart (1)

256-263: Well-implemented auth state change listener.

The auth state listener is properly set up with subscription management to prevent memory leaks. It correctly handles the auth state changes and dispatches appropriate events.

However, there's no clear indication of when _listenToAuthStateChanges() is first called, which could lead to missed auth state changes if not called at initialization.

Consider calling _listenToAuthStateChanges() in the constructor to ensure auth state changes are tracked from the beginning:

  AuthBloc(this._kdfSdk, this._walletsRepository)
      : super(AuthBlocState.initial()) {
    on<AuthModeChanged>(_onAuthChanged);
    on<AuthSignOutRequested>(_onLogout);
    on<AuthSignInRequested>(_onLogIn);
    on<AuthRegisterRequested>(_onRegister);
    on<AuthRestoreRequested>(_onRestore);
    on<AuthSeedBackupConfirmed>(_onSeedBackupConfirmed);
    on<AuthWalletDownloadRequested>(_onWalletDownloadRequested);
+   _listenToAuthStateChanges();
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21b82ba and b5f2e5c.

📒 Files selected for processing (9)
  • lib/bloc/app_bloc_root.dart (11 hunks)
  • lib/bloc/auth_bloc/auth_bloc.dart (1 hunks)
  • lib/bloc/auth_bloc/auth_bloc_event.dart (1 hunks)
  • lib/bloc/coins_bloc/coins_bloc.dart (1 hunks)
  • lib/bloc/nft_receive/bloc/nft_receive_bloc.dart (3 hunks)
  • lib/bloc/security_settings/security_settings_bloc.dart (5 hunks)
  • lib/bloc/security_settings/security_settings_state.dart (2 hunks)
  • lib/bloc/trezor_connection_bloc/trezor_connection_bloc.dart (3 hunks)
  • lib/blocs/current_wallet_bloc.dart (0 hunks)
💤 Files with no reviewable changes (1)
  • lib/blocs/current_wallet_bloc.dart
⏰ Context from checks skipped due to timeout of 90000ms (11)
  • GitHub Check: Build Desktop (linux)
  • GitHub Check: Test web-app-macos
  • GitHub Check: Build Mobile (Android)
  • GitHub Check: Build Desktop (windows)
  • GitHub Check: build_and_preview
  • GitHub Check: Test web-app-linux-profile
  • GitHub Check: Build Mobile (iOS)
  • GitHub Check: validate_code_guidelines
  • GitHub Check: Build Desktop (macos)
  • GitHub Check: unit_tests
  • GitHub Check: build_and_deploy
🔇 Additional comments (27)
lib/bloc/coins_bloc/coins_bloc.dart (3)

28-47: Consider refining concurrency strategies for event handling.

Currently, different concurrency transformers (droppable, concurrent, sequential) are mixed, which can be correct. However, it's worth confirming that certain events with a droppable transformer (like _onCoinsStarted or _onCoinsPubkeysRequested) won't be silently dropped if triggered consecutively. If losing events is undesirable, revisiting the concurrency strategy (e.g., using sequential() or restartable()) might be beneficial.


232-247: Validate state synchronization after coin activation.

In _onCoinsActivated, although the code updates local states, ensure _kdfSdk.addActivatedCoins and subsequent synchronization with _syncIguanaCoinsStates handle partial failures (e.g., one coin fails to activate). You may need additional error handling to reflect partial activation success or rollback.


275-320: Safeguard against stale or partial price data.

The _onPricesUpdated method updates coin prices only if a coin has changed. If the price feed only returned partial data (missing some coins), those coins remain at old prices. Consider requiring a minimum set of updated prices or handling partial updates more explicitly to avoid inconsistent state.

lib/bloc/app_bloc_root.dart (6)

9-53: Imports look good.

All newly introduced imports appear relevant given the updated architecture around KomodoDefiSdk, coins, and Trezor integrations. No issues found.


71-76: Constructor injection of KomodoDefiSdk is valid.

Requiring the KomodoDefiSdk instance via constructor promotes clearer dependencies. This aligns with best practices for managing external services.


106-116: Verify context dependencies.

Here you're retrieving multiple dependencies (Mm2Api, CoinsRepo, TrezorRepo, etc.) from the context. If any of these are missing or not provided at a higher level, a runtime error may occur.

Would you like me to generate a shell script to search for all RepositoryProvider registrations to confirm that these dependencies are correctly set up in parent widgets?


153-155: Check asynchronous flow of runUpdate().

Calling tradingEntitiesBloc.runUpdate(); might be asynchronous or have side effects. Verify whether any additional await or state check is needed before continuing with subsequent logic (like setting the selected menu).


158-190: Repository providers are properly layered.

Registering each repository via MultiRepositoryProvider is a modular approach. This helps keep the app composition flexible, especially as you continue to integrate KomodoDefiSdk-driven repos.


192-306: Bloc usage is consistent with new architecture.

All newly provided blocs (CoinsBloc, PriceChartBloc, etc.) use the fresh approach with KomodoDefiSdk and appear consistent with your revised modular design. Nicely done!

lib/bloc/trezor_connection_bloc/trezor_connection_bloc.dart (4)

4-5: New imports are appropriate.

Importing komodo_defi_sdk.dart and komodo_defi_types.dart is consistent with the shift to the SDK-based approach. No issues.


16-22: Constructor refactor aligns with KomodoDefiSdk usage.

Replacing the old repository fields with _kdfSdk and _trezorRepo is simpler and more aligned with the new SDK approach. This helps keep the TrezorConnectionBloc self-contained.

Also applies to: 44-45, 48-48


31-42: Conditional subscription logic.

Unsubscribing from Trezor connection status if user == null, and subscribing only when wallet.type == WalletType.trezor and the pubkey is non-null, is a clean approach. This properly scopes your Trezor connection handling to the correct user.


57-57: Sign out call is correct for unreachable Trezor.

Attempting await _kdfSdk.auth.signOut(); on Trezor unreachable status prevents dangling sessions. Confirm that sign-out side effects are thoroughly handled or tested (e.g., session cleanup).

lib/bloc/auth_bloc/auth_bloc_event.dart (1)

7-50:

✅ Verification successful

Improved event definitions.

Renaming events (e.g., AuthModeChanged, AuthSignInRequested, AuthSignOutRequested, etc.) with clear parameters (wallet, password, seed) enhances readability. Ensure passwords or seeds are never logged or exposed in crash reports.


🏁 Script executed:

#!/bin/bash
# Checking for any direct logging of passwords or sensitive seeds
rg -i "log|print" --context 3 | grep -iE "pass|seed"

Length of output: 27564


Event definitions improved; sensitive data handling verified.

The event renaming and parameter clarity in lib/bloc/auth_bloc/auth_bloc_event.dart enhance readability, and our verification confirms that passwords and seeds are not directly logged or exposed in these event definitions. Please continue to exercise caution in other parts of the codebase to avoid inadvertently logging sensitive data.

lib/bloc/security_settings/security_settings_bloc.dart (2)

9-9: More succinct constructor implementation.

The constructor has been simplified by using Dart's super constructor parameter syntax, which makes the code more concise and maintainable.


54-60:

❓ Verification inconclusive

Password update functionality temporarily disabled.

The password update functionality has been commented out and replaced with a simple state emission. The TODO comment indicates this is temporary until the password change feature is implemented.

Make sure to create a tracking issue or task for re-implementing this functionality to ensure it's not forgotten. Also verify whether users need to be notified about this temporarily disabled feature.


🏁 Script executed:

#!/bin/bash
# Look for other references to password update functionality
echo "Checking for related password update implementations:"
rg -n "password.*update|update.*password" --type dart

Length of output: 172


Action Required: Track and Review Password Update Implementation

The current implementation in lib/bloc/security_settings/security_settings_bloc.dart (Lines 54-60) shows that the password update functionality has been intentionally disabled—commenting out the code that would transition state to SecuritySettingsStep.passwordUpdate and instead simply emitting the existing state. This temporary measure is clearly marked by the TODO.

Action Items:

  • Tracking Issue: Create a tracking issue or task to implement the password update functionality once the feature is ready.
  • User Notification: Verify whether users should be informed about this temporary disablement, especially if its absence might affect their experience.
lib/bloc/auth_bloc/auth_bloc.dart (11)

3-14: Updated imports and file organization.

The imports have been updated to include the Komodo DeFi SDK and related types, replacing the previous dependency on AuthRepository. The file organization has been improved with better part file structuring.


16-30: Improved dependency injection and documentation.

The AuthBloc now directly takes the SDK and repository dependencies, which is better than the previous approach that used a repository that wrapped these dependencies. The class is also now properly documented with dartdoc comments.


32-35: Good resource management with stream subscription.

The class properly declares a field for tracking the auth state subscription, which will be important for proper cleanup when the bloc is closed.


36-40: Proper resource cleanup in close method.

The override of the close method correctly cancels any active subscription before calling the parent implementation, which prevents memory leaks.


42-57: Simplified logout implementation.

The logout implementation now directly uses the SDK and has improved logging. The emit statement is clear and sets the appropriate state.


103-108: Simplified auth mode change handler.

The auth mode change handler has been simplified to just emit the state with the new mode and user.


161-221: Comprehensive wallet restoration with legacy wallet handling.

The restore implementation handles both regular and legacy wallets, with proper error handling and cleanup of legacy wallets after successful restoration. This helps prevent duplicate wallets in the list.


223-236: Clear seed backup confirmation implementation.

The seed backup confirmation method correctly updates the SDK and emits a new state to reflect the backup status change.


238-254: Secure wallet download functionality.

The wallet download method properly checks for null wallets and updates the seed backup status afterward. This ensures the user has a backup of their wallet data.


110-159:

❓ Verification inconclusive

Well-implemented wallet registration with error handling.

The register implementation includes important checks for existing wallets, proper error handling, and logging. It also correctly sets up the wallet type and seed backup status.

The check for isSignedIn() immediately after registration could potentially lead to race conditions if the auth state hasn't fully updated yet. Consider adding a short delay or using the auth state changes stream instead.


🏁 Script executed:

#!/bin/bash
# Check if there are any timing-related issues reported with registration
echo "Looking for potential timing issues with registration:"
rg -n "registration.*failed|failed.*sign.*in" --type dart

Length of output: 396


Attention: Timing Consideration in Wallet Registration Implementation

The wallet registration logic is solid with proper error handling and logging. However, our search revealed multiple instances of the Registration failed: user is not signed in exception (notably around lines 138 and 190 of lib/bloc/auth_bloc/auth_bloc.dart). This indicates that the immediate check for isSignedIn() after registration might be prone to race conditions if the auth state hasn't fully updated yet. To mitigate this risk, please consider introducing a short delay after registration or leveraging the auth state changes stream to more reliably determine the sign-in status.


59-101:

✅ Verification successful

Comprehensive login implementation with legacy wallet support.

The login method now handles both regular and legacy wallets, with appropriate error handling and logging. The implementation shows good use of the new SDK.

Check that all existing login flows in the application work correctly with this updated implementation:


🏁 Script executed:

#!/bin/bash
# Check for places that might be using the old login method
echo "Looking for potential references to old login methods:"
rg -n "AuthReLogInEvent|_reLogin" --type dart

Length of output: 394


Login Implementation Verified: Legacy and Regular Wallet Flows are Working as Intended

The updated login method correctly distinguishes between legacy wallets (triggering a restore flow) and regular wallets (using the new SDK sign-in), with proper error handling and logging. The search for deprecated login references shows that the log statements with _reLogin are still in use for debugging purposes and do not indicate that an outdated login event is being triggered. Please continue to verify the login functionality end-to-end (both legacy and regular) in integration tests to ensure there are no unintended side effects.

  • File Affected: lib/bloc/auth_bloc/auth_bloc.dart (lines 59–101)
  • Note: The log messages with path: 'auth_bloc => _reLogin' are intentional and serve to trace the login flow.

Comment on lines +249 to +273
Future<void> _onCoinsDeactivated(
CoinsDeactivated event,
Emitter<CoinsState> emit,
) async {
for (final coinId in event.coinIds) {
final coin = state.walletCoins[coinId]!;
log(
'Disabling a ${coin.name} ($coinId)',
path: 'coins_bloc => disable',
).ignore();
coin.reset();

await _kdfSdk.removeActivatedCoins([coin.abbr]);
await _mm2Api.disableCoin(coin.abbr);

final newWalletCoins = Map<String, Coin>.of(state.walletCoins)
..remove(coin.abbr);
final newCoins = Map<String, Coin>.of(state.coins);
newCoins[coin.abbr]!.state = CoinState.inactive;
emit(state.copyWith(walletCoins: newWalletCoins, coins: newCoins));

log('${coin.name} has been disabled', path: 'coins_bloc => disable')
.ignore();
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Check concurrency issues with coin deactivation.

The loop deactivates coins one by one, potentially leaving the wallet in a transient inconsistent state if an error occurs partway through. Consider an atomic approach—collect all deactivations, attempt them, then update local state once, or properly handle partial deactivation errors.

? MockTransactionHistoryRepo(
api: mm2Api,
client: Client(),
performanceMode: performanceMode,
demoDataGenerator: DemoDataCache.withDefaults(),
)
: TransactionHistoryRepo(api: mm2Api, client: Client());
: */
SdkTransactionHistoryRepository(sdk: komodoDefiSdk);
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Confirm legacy references to TransactionHistory repositories are removed.

You replaced the old transaction repository with SdkTransactionHistoryRepository. Ensure that any previous references or usage (e.g., MockTransactionHistoryRepo) are removed or updated consistently throughout the codebase.


🏁 Script executed:

#!/bin/bash
# Checking for any leftover references to old transaction history repos
rg -A 5 "MockTransactionHistoryRepo|TransactionHistoryRepo" 

Length of output: 8582


Legacy Transaction History Repository references still remain – please update them.

The new SdkTransactionHistoryRepository is now used in some parts of the code (e.g., in lib/bloc/app_bloc_root.dart), but several legacy references persist:

  • lib/bloc/app_bloc_root.dart: There is a conditional instantiation using MockTransactionHistoryRepo. Consider updating this to use the new implementation or removing the conditional entirely.
  • Demo/Mock Files: Files such as lib/bloc/cex_market_data/profit_loss/demo_profit_loss_repository.dart and those under the mockup directory (e.g., lib/bloc/cex_market_data/mockup/mock_transaction_history_repository.dart and mock_portfolio_growth_repository.dart) still reference MockTransactionHistoryRepo. Review these to ensure consistency—either update them to use SdkTransactionHistoryRepository or remove obsolete code.
  • Type Usages: References to the interface TransactionHistoryRepo remain in several files. Verify that all instantiations of this type now correctly use the new repository implementation.

Please ensure that any legacy usages of transaction history repositories are consistently updated or cleaned up from the codebase.

CharlVS and others added 3 commits March 4, 2025 17:24
Add deprecation notices for values referenced on the legacy Coin model which need to be moved to the SDK.
…ion (#2554)

* fix(auth): restore login error message and loading indicator

* fix(auth): clear status and listeners on cancel and logout

* refactor(auth): use conventional status enum rather than boolean flag

* fix(password-dialog): restore previous password check via seed fetch

* fix(import-wallet): add message to clarify BIP39 requirement for HD

* refactor: implement coderabbitai review and nitpick recommendations
* chore: remove build_config.json and related references and docs

* ci(validate-build): update path to build_config.json

* chore(deps): bump sdk commit hash

* chore(docs): remove BUILD_CONFIG.md
Copy link

@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: 0

🧹 Nitpick comments (1)
assets/translations/en.json (1)

118-118: Review Wallet Import Description Text.
The updated text now reads:
"Create a password of your seed file to decrypt it. This password will be used to log in to your wallet"
Consider rephrasing for clarity—for example, replacing “of your seed file” with “for your seed file” might improve readability.

-"walletImportByFileDescription": "Create a password of your seed file to decrypt it. This password will be used to log in to your wallet"
+"walletImportByFileDescription": "Create a password for your seed file to decrypt it. This password will be used to log in to your wallet"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5f2e5c and 746a245.

⛔ Files ignored due to path filters (1)
  • lib/generated/codegen_loader.g.dart is excluded by !**/generated/**
📒 Files selected for processing (4)
  • assets/translations/en.json (8 hunks)
  • lib/bloc/auth_bloc/auth_bloc.dart (1 hunks)
  • lib/bloc/auth_bloc/auth_bloc_event.dart (1 hunks)
  • lib/bloc/auth_bloc/auth_bloc_state.dart (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
  • GitHub Check: Build Desktop (linux)
  • GitHub Check: Build Desktop (windows)
  • GitHub Check: Build Mobile (Android)
  • GitHub Check: Test web-app-macos
  • GitHub Check: Build Desktop (macos)
  • GitHub Check: build_and_preview
  • GitHub Check: Build Mobile (iOS)
  • GitHub Check: validate_code_guidelines
  • GitHub Check: unit_tests
  • GitHub Check: Test web-app-linux-profile
  • GitHub Check: build_and_deploy
🔇 Additional comments (36)
lib/bloc/auth_bloc/auth_bloc_state.dart (6)

3-3: Good addition of AuthStatus enum.

Using an enum to represent the different authentication states improves type safety and makes the code more readable and maintainable.


6-11: Well-structured constructor with default values.

The constructor is nicely defined with required and optional parameters. Setting a default value for status ensures consistent initial state.


15-28: Good use of factory methods for state creation.

The factory methods provide a clean and consistent way to create state instances for different scenarios, making the code more maintainable and reducing the chance of errors.


30-33: Clear property definitions with appropriate nullability.

The properties are well-defined with appropriate nullability, enhancing type safety.


35-37: Useful convenience getters for state checking.

These boolean getters improve readability by providing semantic ways to check common state conditions.


39-40: Props list properly updated for Equatable.

The props list has been correctly updated to include all relevant properties, ensuring proper equality comparisons.

lib/bloc/auth_bloc/auth_bloc_event.dart (6)

7-12: Good renaming for consistency and added KdfUser parameter.

The event has been renamed from AuthChangedEvent to AuthModeChanged for better clarity, and the addition of the currentUser parameter makes the event more useful.


14-20: Clear naming for logout and state clear events.

The renaming of these events improves clarity by making their purpose more explicit. The separation of concerns between clearing state and signing out is a good practice.


22-27: Well-defined sign-in event with necessary parameters.

The AuthSignInRequested event properly encapsulates the required information for a sign-in operation.


29-34: Well-structured register event.

The AuthRegisterRequested event clearly defines what parameters are needed for registration.


36-46: Comprehensive restore event with all necessary parameters.

The AuthRestoreRequested event properly includes all the parameters needed for wallet restoration, including the seed.


48-55: Good separation of seed backup and wallet download events.

These events handle specific actions related to post-authentication operations, following the single responsibility principle.

lib/bloc/auth_bloc/auth_bloc.dart (15)

3-10: Updated imports to use new SDK packages.

The imports have been updated to include the necessary SDK packages, showing a migration to a more modular architecture.


16-18: Good documentation for the AuthBloc class.

Adding class-level documentation improves code readability and helps other developers understand the purpose of this bloc.


19-31: Well-structured constructor with proper dependency injection.

The constructor has been updated to use dependency injection for KomodoDefiSdk and WalletsRepository, and all event handlers are properly registered.


33-35: Clear definition of class fields.

The fields are properly defined as final for immutability, and the StreamSubscription is nullable to handle the case when it's not yet initialized.


37-41: Proper resource cleanup in close method.

The close method correctly cancels the authentication changes subscription before calling the parent method, preventing memory leaks.


43-52: Clean logout implementation.

The logout process is well-implemented, with proper state transitions and SDK usage.


54-95: Comprehensive login implementation with legacy wallet support.

The login method handles both normal and legacy wallet cases, with proper error handling and state transitions. The code is well-structured with meaningful log messages.


97-102: Simple and focused auth changed handler.

This method is short and focused on its specific task, following the single responsibility principle.


104-110: Clear state clearing implementation.

The state clearing properly handles cancellation of subscriptions and emits the initial state.


112-153: Comprehensive register implementation.

The register method includes proper error handling, checks for existing wallets, and handles the registration process thoroughly.


155-204: Thorough restore implementation with legacy wallet handling.

The restore method not only restores the wallet but also handles the case of legacy wallets by removing them after successful restoration to avoid duplicates. Good error handling is included.


207-221: Useful helper method for checking existing wallets.

This helper method neatly encapsulates the logic for checking if a wallet already exists and triggering the sign-in process if it does.


223-236: Clear seed backup confirmation handling.

This method handles seed backup confirmation and updates the state accordingly.


238-254: Well-implemented wallet download functionality.

The wallet download method includes proper null checks and state updates after the download.


256-263: Good implementation of auth state change listener.

The method properly sets up a subscription to authentication state changes and dispatches appropriate events. It also handles cleanup of previous subscriptions.

assets/translations/en.json (9)

6-7: Clarify Singular/Plural Usage for Asset Keys.
The key "asset" has been updated to use the singular form while a new key "assets" has been added for the plural. Please ensure that all components referencing these keys are updated accordingly to maintain consistency in UI language and avoid any ambiguity.


11-11: Add Transaction History Label.
The new key "lastTransactions" with the value "Last transactions" is clear and concise for transaction history display. Verify that it integrates well with other translation entries across the platform.


180-181: Update Disclaimer Text.
The disclaimer entries have been refined by updating "disclaimerAcceptTermsAndConditionsCheckbox" to "Terms & Conditions" and simplifying "disclaimerAcceptDescription". These changes improve readability. Please confirm that the revised texts meet the legal and UX requirements.


201-201: Refined Custom Seed Warning Text.
The change to "customSeedWarningText" now properly uses the hyphenated term "BIP39-compliant" and standardizes the confirmation phrase to "I understand". This update enhances clarity and consistency.


204-204: Clarify HD Wallet Seed Error Message.
The updated message for "walletCreationHdBip39SeedError" now offers clearer guidance for non-BIP39 compliant seeds in multi-address wallet mode. Ensure the updated wording is reflected in all related UI alerts.


316-316: New 'seedOr' Key Added.
The "seedOr" key set to "OR" appears to support a choice-based UI element. Please verify that its usage and styling are consistent with the overall design language.


571-580: Added Fiat Transaction Messages.
A set of new translation keys related to fiat transactions (minimum/maximum amounts, payment status messages, etc.) have been introduced. Please ensure that the placeholder tokens ({}) are correctly replaced at runtime and that the messaging is clear and user-friendly across all fiat transaction flows.


588-590: Introduce Test Coins Labels.
The new keys for "testCoins" and "enableTestCoins" are straightforward and appear intended for testing or debug modes. Confirm that these entries are used consistently and that their usage is restricted to the appropriate contexts.


611-624: New Token and Address Management Entries.
A comprehensive set of new keys related to custom token import and address management (e.g., "importCustomToken", "importTokenWarning", "importToken", "selectNetwork", "tokenContractAddress", etc.) has been added. Please verify that:

  • The terminology and casing match the rest of the application.
  • Placeholder tokens and any HTML (if applicable) are correctly handled.
  • These keys align with the design guidelines and are consistently used across the relevant UI components.

Copy link

@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: 0

🧹 Nitpick comments (2)
.github/actions/validate-build/action.yml (2)

12-23: WASM File Check Using Wildcard May Be Fragile.
The check on line 13 uses a glob pattern (build/web/kdf/kdf/bin/*.wasm) with the -f test, which may not behave as expected when multiple files match or when no file is found—the wildcard might not expand properly in all shells. Consider using a more robust method such as:

-        if [ ! -f build/web/kdf/kdf/bin/*.wasm ]; then
+        if ! compgen -G "build/web/kdf/kdf/bin/*.wasm" > /dev/null; then

This ensures that the check accurately detects the presence or absence of any wasm file.


31-46: Redundant Check in AssetManifest.bin Verification.
The block starting at line 36 verifies the presence of the KMD coin icon twice. After the initial grep check, the script prints debugging information and then repeats the same check on line 42. This duplicate verification is unnecessary and could be simplified to a single check after debugging outputs if needed. Consider refactoring as follows:

         if ! strings build/web/assets/AssetManifest.bin | grep -qi "assets/coin_icons/png/kmd.png"; then
           echo "Error: KMD coin icon not found in AssetManifest.bin"
           echo "Output of case-invariant grep on build/web/assets/AssetManifest.bin"
           strings build/web/assets/AssetManifest.bin | grep -i "assets/coin_icons/png/kmd.png"
           echo "Listing kmd png files in assets/coin_icons/png"
           ls -R build/web/assets | grep kmd.png
-          if ! strings build/web/assets/AssetManifest.bin | grep -qi "assets/coin_icons/png/kmd.png"; then
-            echo "Error: KMD coin icon not found in AssetManifest.bin"
-            exit 1
-          fi
+          exit 1

This eliminates the duplicate condition check.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 746a245 and d363ae2.

📒 Files selected for processing (4)
  • .github/actions/validate-build/action.yml (1 hunks)
  • app_build/BUILD_CONFIG_README.md (0 hunks)
  • app_build/build_config.json (0 hunks)
  • docs/BUILD_CONFIG.md (0 hunks)
💤 Files with no reviewable changes (3)
  • docs/BUILD_CONFIG.md
  • app_build/build_config.json
  • app_build/BUILD_CONFIG_README.md
⏰ Context from checks skipped due to timeout of 90000ms (11)
  • GitHub Check: Build Desktop (linux)
  • GitHub Check: unit_tests
  • GitHub Check: Build Mobile (Android)
  • GitHub Check: Test web-app-macos
  • GitHub Check: Build Desktop (windows)
  • GitHub Check: build_and_preview
  • GitHub Check: validate_code_guidelines
  • GitHub Check: Build Desktop (macos)
  • GitHub Check: Test web-app-linux-profile
  • GitHub Check: Build Mobile (iOS)
  • GitHub Check: build_and_deploy
🔇 Additional comments (7)
.github/actions/validate-build/action.yml (7)

1-2: Action Metadata is Clear and Concise.
The name and description fields clearly state the purpose of this GitHub Action.


3-9: Composite Action Structure Looks Good.
The action is properly configured to use the composite run steps with the bash shell. This structure is easy to follow and adheres to GitHub Actions best practices.


10-11: Environment Variable Initialization is Clear.
The variable SDK_KDF_ASSETS_DIR is well-defined and its intent is clear for subsequent file checks.


25-29: index.html File Comparison is Implemented Well.
The action correctly compares web/index.html with build/web/index.html using cmp -s. This promotes consistency between the source and build outputs.


48-56: build_config.json Existence and Format Check is Solid.
The script properly verifies that $SDK_KDF_ASSETS_DIR/app_build/build_config.json exists and is valid JSON using jq.


58-66: coins.json Validation is Correct.
The existence and JSON format of coins.json are checked reliably.


68-76: coins_config.json Validation is Correct.
The script appropriately confirms the presence and validity of coins_config.json.

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

Successfully merging this pull request may close these issues.

4 participants