0.9.3.zzz Release candidate #3407
Conversation
…ansaction signing - Replace manual sendRawTransaction() call with SDK executeWithdrawal() - Remove unused mm2Api dependency and import - Simplifies code by using proper SDK abstraction - Maintains existing behavior while following SDK best practices - Ensures transaction is only signed once during preview phase
Updates SDK to commit 6576c4c which includes: - New executeWithdrawal() method for preview-then-execute workflow - Deprecation of direct withdraw() method - Prevention of duplicate transaction signing
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: charl <[email protected]>
- Updated the window close handler to manage exit behavior on Linux, ensuring graceful shutdown without GTK cleanup issues. - Added a delete-event signal handler to intercept window close events on Linux, allowing for proper dialog handling before exiting. - Introduced a stub for the exit function on the web platform to prevent unsupported calls.
- Removed unused flutter_view member from the MyApplication struct and related functions to clean up the code. - This change simplifies the application structure and addresses potential memory management concerns.
- Added a set of priority ticker symbols for unauthenticated users. - Implemented sorting logic in `sortAssetIdsWithPriority` to prioritize these tickers. - Updated `AssetsList` to sort asset groups by priority tickers first, followed by others. - Adjusted comments in `wallet_main.dart` to reflect the new sorting behavior.
…rting logic - Deleted the set of priority ticker symbols for unauthenticated users. - Removed the `sortAssetIdsWithPriority` function that utilized these tickers for sorting. - Updated comments to reflect the removal of the priority sorting feature.
add user confirm for manual withdraw input when equivalent to max
…-exit fix(linux): workaround for crash on application exit due to Flutter implicit view removal
fix: uuid copy / swap export / translations
feat: Add prioritized coins display for unauthenticated users
…hout-navigation fix(tx-history): timestamp update without navigation
…-vis removes timer on password visibility
…withdraws refactor(withdraw): use SDK executeWithdrawal() to avoid duplicate transaction signing
use latest KDF commit on `sia-rc-cleanup`
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaced many hardcoded UI strings with localized keys, added Sia support and legacy CoinType↔CoinSubClass mappings, migrated multiple surfaces from CoinType → CoinSubClass, switched withdrawal broadcast to an SDK streaming flow with optional txJson, added JSON save paths, simplified password-visibility, adjusted desktop close handling, and bumped SDK submodule. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Withdraw Form
participant WithdrawBloc
participant SDK
User->>UI: submit withdrawal (preview)
UI->>WithdrawBloc: submit(preview)
WithdrawBloc->>SDK: executeWithdrawal(preview)
SDK-->>WithdrawBloc: preview result (WithdrawDetails with txHex/txJson)
User->>UI: confirm submit
UI->>WithdrawBloc: submit(final)
WithdrawBloc->>SDK: executeWithdrawal(submit)
SDK-->>WithdrawBloc: stream(progress/events)
alt success
SDK-->>WithdrawBloc: WithdrawalStatus.complete
WithdrawBloc-->>UI: success/result
else error
SDK-->>WithdrawBloc: WithdrawalStatus.error / exception
WithdrawBloc-->>UI: error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import 'dart:io' show exit | ||
| if (dart.library.html) 'window_close_handler_exit_stub.dart' show exit; |
There was a problem hiding this comment.
The new conditional import at the top of window_close_handler.dart uses import 'dart:io' show exit if (dart.library.html) 'window_close_handler_exit_stub.dart' show exit;, which is not valid Dart syntax—combinators must follow the conditional URI, not precede it. As written the file will not compile on any platform, preventing the app from building until the import is rewritten to a valid conditional form (e.g., import 'dart:io' if (dart.library.html) 'window_close_handler_exit_stub.dart' show exit;).
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/views/dex/entity_details/swap/swap_details.dart (1)
10-13: Update the outdated comment.The comment states "This version uses static strings instead of translation keys for the 'Swap UUID' label," but the actual implementation in
trading_details_coin_pair.dart(lines 88-90) now uses localized keys (LocaleKeys.orderUuidandLocaleKeys.swapUuid).Apply this diff to update the comment:
-/// SwapDetails shows the basic information of a DEX swap including coin pairs, -/// timing and progress steps. It now includes the swap UUID with a copy -/// button so users can easily copy it. This version uses static strings -/// instead of translation keys for the "Swap UUID" label. +/// SwapDetails shows the basic information of a DEX swap including coin pairs, +/// timing and progress steps. It delegates UUID display and optional content +/// below the UUID to the TradingDetailsCoinPair widget.lib/services/file_loader/mobile/file_loader_native_ios.dart (1)
46-69: Fix inconsistent filename usage in share operation.Line 62 uses the hardcoded
'$fileName.txt'instead of the computedsuggestedName. This defeats the purpose of lines 51-53, which preserve the original extension if present.Apply this diff:
await SharePlus.instance.share( ShareParams( files: [ XFile( file.path, - name: '$fileName.txt', + name: suggestedName, mimeType: 'text/plain', ) ], sharePositionOrigin: _getSharePositionOrigin(), ), );
♻️ Duplicate comments (1)
lib/services/file_loader/mobile/file_loader_native_android.dart (1)
50-71: LGTM!The JSON file saving implementation correctly pretty-prints JSON data with a fallback to the original data if parsing fails.
🧹 Nitpick comments (12)
lib/views/dex/entity_details/trading_details_coin_pair.dart (1)
39-40: Consider removing redundant variable assignments.These local variable assignments shadow the instance fields without adding clarity or functionality.
Apply this diff to simplify:
- final String? swapId = this.swapId; - final bool isOrder = this.isOrder; -Then update line 81 and lines 88-90 to use the fields directly:
- if (swapId != null) + if (this.swapId != null) Row( mainAxisAlignment: MainAxisAlignment.spaceAround, children: [ Flexible( child: CopiedText( - key: Key('uuid-${swapId}'), - text: isOrder - ? LocaleKeys.orderUuid.tr(args: [swapId]) - : LocaleKeys.swapUuid.tr(args: [swapId]), - copiedValue: swapId, + key: Key('uuid-${this.swapId}'), + text: this.isOrder + ? LocaleKeys.orderUuid.tr(args: [this.swapId!]) + : LocaleKeys.swapUuid.tr(args: [this.swapId!]), + copiedValue: this.swapId!,And line 101:
- if (swapId != null && belowUuid != null) ...[ + if (this.swapId != null && belowUuid != null) ...[lib/bloc/transaction_history/transaction_history_bloc.dart (1)
34-36: Consider removing unused_firstSeenAtByIdmap.The
_firstSeenAtByIdmap is populated with first-seen timestamps for stable ordering (lines 96-101, 177-182), but it's never consumed—_compareTransactions(line 251) doesn't reference it. This map accumulates entries in memory without serving any purpose.If deterministic ordering of zero-timestamp transactions isn't currently needed, consider removing this map to reduce memory overhead:
- // Stable in-memory clock for transactions that arrive with a zero timestamp. - // Ensures deterministic ordering of unconfirmed and just-confirmed items. - final Map<String, DateTime> _firstSeenAtById = {};And remove the corresponding
putIfAbsentcalls at lines 96-101 and 177-182.Alternatively, if stable ordering is intended, integrate
_firstSeenAtByIdinto_compareTransactionsto fulfill its stated purpose.Also applies to: 96-101, 177-182
lib/services/file_loader/file_loader_web.dart (1)
59-89: Consider extracting the JSON pretty-print logic to a shared utility.The implementation is correct, but this JSON formatting logic is duplicated across all platform-specific file loaders (web, Android, iOS, desktop). Consider extracting the pretty-print logic to a shared utility function to reduce duplication and improve maintainability.
Example shared utility:
// In a shared utils file String formatJsonData(String data) { try { final dynamic decoded = json.decode(data); return const JsonEncoder.withIndent(' ').convert(decoded); } catch (_) { return data; } }Then use it in each platform implementation:
- String prettyData = data; - try { - final dynamic decoded = json.decode(data); - prettyData = const JsonEncoder.withIndent(' ').convert(decoded); - } catch (_) {} - - final dataArray = web.TextEncoder().encode(prettyData); + final prettyData = formatJsonData(data); + final dataArray = web.TextEncoder().encode(prettyData);lib/blocs/kmd_rewards_bloc.dart (1)
44-49: New txHex null guard correctly prevents invalid send, consider loggingGuarding against
withdrawDetails.txHex == nullbefore constructingSendRawTransactionRequestis the right way to avoid sending an invalid request / tripping the new assertion. As a small improvement, if this state is unexpected in normal flow, consider also logging it to aid debugging if users ever hit this genericsomethingWrongpath.lib/sdk/widgets/window_close_handler.dart (1)
1-3: Linux exit workaround and conditional import look sound; update comment to match behaviorThe conditional import of
exitwith a web stub plus the Linux‑specific branch that schedulesexit(0)after_handleWindowClose()completes is a reasonable way to avoid GTK cleanup issues while still disposing the SDK first. The overall flow for desktop/web/mobile platforms remains coherent.Minor nit: the comment about “manually trigger exit via SystemNavigator” no longer matches the actual Linux path (which now calls
exit(0)); it would be good to update that to avoid future confusion.Also applies to: 21-22, 52-83, 171-177
lib/shared/utils/utils.dart (1)
35-77: Clipboard feedback path improved; consider guarding global fallbackUsing
ScaffoldMessenger.maybeOf(context) ?? ScaffoldMessenger.of(scaffoldKey.currentContext!)is an improvement over always callingScaffoldMessenger.of(context), since it allows a root messenger fallback. There’s still a small risk of a runtime error if both messengers are unavailable (e.g.,scaffoldKey.currentContextis null or not attached).If you want this helper to be fully defensive, you could safely bail out when no messenger is found, for example:
- final scaffoldMessenger = - ScaffoldMessenger.maybeOf(context) ?? - ScaffoldMessenger.of(scaffoldKey.currentContext!); - scaffoldMessenger.showSnackBar( + final scaffoldMessenger = + ScaffoldMessenger.maybeOf(context) ?? + (scaffoldKey.currentContext != null + ? ScaffoldMessenger.maybeOf(scaffoldKey.currentContext!) + : null); + if (scaffoldMessenger == null) { + log('copyToClipBoard: no ScaffoldMessenger available', isError: true); + return; + } + scaffoldMessenger.showSnackBar( SnackBar( // ... ), );Not mandatory, but it would make this utility safer in edge contexts.
lib/bloc/coins_manager/coins_manager_bloc.dart (1)
359-363: Type filter now keyed off CoinSubClass; behavior looks correctUsing
coin.id.subClassto match againststate.selectedCoinTypescorrectly aligns the filter with the new selection type system. This will exclude any coins whosesubClassis not explicitly selected, which seems desirable; just ensure all filterable protocols expose the expectedsubClassvalues.lib/bloc/coins_bloc/asset_coin_extension.dart (1)
65-68: Sia mappings look good; consider surfacing lossy CoinSubClass→CoinType fallbacks
- Adding
CoinSubClass.sia ⇄ CoinType.siain both extensions is symmetric and should make Sia behave like other explicitly modeled protocols.- Grouping
moonbeam,ewt,rskSmartBitcoin, andunknownunderCoinType.utxois clearly documented as a legacy escape hatch, but it’s a lossy mapping. If these ever hit legacy flows, a debug log or assert could help detect unexpected usage instead of silently treating them as UTXO.Also applies to: 77-78, 115-121, 151-155, 165-166
lib/bloc/auth_bloc/trezor_auth_mixin.dart (1)
68-69: Trezor auth messaging localization is wired correctly; consider treatment of SDK cancellation textReplacing hardcoded English strings with
LocaleKeys.*.tr()across all Trezor states keeps the control flow intact while making messages localizable. The error state still prefersauthState.errorwhen present, which is good; for thecancelledstatus you now always show a generic localized message, dropping any SDK-provided text. That’s fine UX‑wise, but if the SDK ever returns more specific cancellation reasons you may want to log or surface them somewhere for diagnostics.Please double‑check that all new
LocaleKeys.trezor*keys are present in every supported locale so this path never falls back to missing‑key placeholders at runtime.Also applies to: 74-76, 80-83, 86-88, 90-93, 101-103, 108-110, 126-128, 158-160, 172-174, 190-192, 204-206
lib/views/wallet/coins_manager/coins_manager_filters_dropdown.dart (1)
101-107: Dynamic CoinSubClass filter derivation looks correct; consider small refinementsDeriving
listTypesfrom the underlying coins via_buildFilterSubClassesand usingList<CoinSubClass>for bothselectedCoinTypesandlistTypesis a solid move toward SDK‑driven, protocol‑level filters, and the wallet‑type switch (iguana/hdwallet →coinsBloc.state.coins.values, trezor →bloc.state.coins, others → empty list) reads as intentional. Two minor follow‑ups: (1) returningconst []for Metamask/Keplr/null wallets effectively disables protocol filters there—please double‑check that this is desired; and (2)listTypes.indexOf(type)inside themapmakes selection ofisFirstO(n²) in the worst case and could be cheaply replaced by iterating with an index (e.g.,asMap().entries) if this list grows.Also applies to: 142-173
lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart (1)
200-205: suppressPreviewError plumbing works; consider extending suppression windowThreading
suppressPreviewErrorfrom_WithdrawFormStateintoWithdrawFormContent, then intoWithdrawFormFillSection, and finally gating the preview error display withif (state.hasPreviewError && !suppressPreviewError)cleanly isolates this UX tweak from the bloc state. One subtle UX nuance: suppression is active only while the confirmation dialog is open; after the user agrees and you re‑submit the preview, the previous error can briefly reappear until the new preview result arrives—if you want a smoother experience, you could keepsuppressPreviewErrortrue until eitherpreviewErrorchanges again or the step advances away fromfill.Also applies to: 210-220, 252-256, 569-573, 689-689
assets/translations/en.json (1)
66-66: New and updated English strings look good; add key for the new withdrawal confirmation dialogThe spelling fix for
confirmSending, the more generic"Download as file"label, the new feedback validation/default type strings, the expanded Trezor status/auth messages, and the swap export/UUID labels are all consistent with the existing copy style and should integrate cleanly with the corresponding UI changes. In light of the new hard‑coded dialog inwithdraw_form.dart(“Since you're sending your full amount, the network fee will be deducted from the amount. Do you agree?”), it would be good to introduce a dedicated key here (e.g., a title and body for that confirmation) and switch the widget to useLocaleKeys.*.tr()for full localization coverage.Also applies to: 151-151, 323-323, 385-395, 530-530, 541-543
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
lib/generated/codegen_loader.g.dartis excluded by!**/generated/**
📒 Files selected for processing (37)
assets/translations/en.json(6 hunks)lib/app_config/app_config.dart(1 hunks)lib/bloc/auth_bloc/auth_bloc.dart(1 hunks)lib/bloc/auth_bloc/trezor_auth_mixin.dart(7 hunks)lib/bloc/coins_bloc/asset_coin_extension.dart(5 hunks)lib/bloc/coins_manager/coins_manager_bloc.dart(2 hunks)lib/bloc/coins_manager/coins_manager_event.dart(1 hunks)lib/bloc/coins_manager/coins_manager_state.dart(2 hunks)lib/bloc/transaction_history/transaction_history_bloc.dart(2 hunks)lib/bloc/withdraw_form/withdraw_form_bloc.dart(1 hunks)lib/blocs/kmd_rewards_bloc.dart(1 hunks)lib/mm2/mm2_api/rpc/send_raw_transaction/send_raw_transaction_request.dart(2 hunks)lib/model/coin_type.dart(2 hunks)lib/model/coin_utils.dart(1 hunks)lib/model/withdraw_details/withdraw_details.dart(5 hunks)lib/sdk/widgets/window_close_handler.dart(5 hunks)lib/sdk/widgets/window_close_handler_exit_stub.dart(1 hunks)lib/services/feedback/custom_feedback_form.dart(1 hunks)lib/services/feedback/feedback_service.dart(2 hunks)lib/services/file_loader/file_loader_native_desktop.dart(3 hunks)lib/services/file_loader/file_loader_web.dart(3 hunks)lib/services/file_loader/mobile/file_loader_native_android.dart(3 hunks)lib/services/file_loader/mobile/file_loader_native_ios.dart(4 hunks)lib/shared/utils/utils.dart(13 hunks)lib/shared/widgets/password_visibility_control.dart(1 hunks)lib/views/dex/entity_details/swap/swap_details.dart(2 hunks)lib/views/dex/entity_details/swap/swap_details_page.dart(1 hunks)lib/views/dex/entity_details/trading_details_coin_pair.dart(4 hunks)lib/views/settings/widgets/general_settings/show_swap_data.dart(1 hunks)lib/views/settings/widgets/security_settings/security_settings_page.dart(6 hunks)lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart(8 hunks)lib/views/wallet/coins_manager/coins_manager_filters_dropdown.dart(5 hunks)lib/views/wallet/coins_manager/coins_manager_selected_types_list.dart(5 hunks)lib/views/wallet/wallet_page/common/assets_list.dart(2 hunks)lib/views/wallet/wallet_page/wallet_main/wallet_main.dart(1 hunks)linux/my_application.cc(3 hunks)sdk(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 3109
File: lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart:50-54
Timestamp: 2025-08-27T22:24:30.511Z
Learning: During SDK migration for CEX market data in Komodo Wallet, functionality like coin filtering for profit/loss charts was moved to the SDK. The app-level filtering was kept as a safety net until SDK behavior could be confirmed, particularly for test coin filtering.
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
Applied to files:
lib/views/wallet/wallet_page/wallet_main/wallet_main.dartlib/bloc/coins_manager/coins_manager_state.dartlib/views/dex/entity_details/trading_details_coin_pair.dartlib/services/feedback/feedback_service.dartlib/views/settings/widgets/security_settings/security_settings_page.dartlib/bloc/auth_bloc/auth_bloc.dartlib/bloc/withdraw_form/withdraw_form_bloc.dartlib/bloc/coins_manager/coins_manager_bloc.dartlib/services/file_loader/file_loader_native_desktop.dartlib/views/wallet/coins_manager/coins_manager_selected_types_list.dartlib/views/wallet/wallet_page/common/assets_list.dartlib/services/file_loader/mobile/file_loader_native_android.dartlib/bloc/coins_bloc/asset_coin_extension.dartlib/views/wallet/coin_details/withdraw_form/withdraw_form.dartlib/views/wallet/coins_manager/coins_manager_filters_dropdown.dart
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The `excludedAssetList` from `app_config.dart` is used in the `_filterExcludedAssets` method in `coins_state.dart`. Since `coins_state.dart` is a part file of `coins_bloc.dart`, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Applied to files:
lib/views/wallet/wallet_page/wallet_main/wallet_main.dartlib/bloc/coins_manager/coins_manager_state.dartlib/app_config/app_config.dartlib/views/dex/entity_details/trading_details_coin_pair.dartlib/views/settings/widgets/security_settings/security_settings_page.dartlib/bloc/auth_bloc/auth_bloc.dartlib/bloc/coins_manager/coins_manager_bloc.dartlib/views/wallet/coins_manager/coins_manager_selected_types_list.dartlib/views/wallet/wallet_page/common/assets_list.dartlib/bloc/coins_bloc/asset_coin_extension.dartlib/views/wallet/coin_details/withdraw_form/withdraw_form.dartlib/views/wallet/coins_manager/coins_manager_filters_dropdown.dart
📚 Learning: 2025-09-02T15:57:12.405Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 3125
File: lib/views/fiat/fiat_inputs.dart:61-62
Timestamp: 2025-09-02T15:57:12.405Z
Learning: The Debouncer class is available from the 'package:komodo_ui/komodo_ui.dart' import in the Komodo Wallet codebase.
Applied to files:
lib/shared/widgets/password_visibility_control.dartlib/services/feedback/feedback_service.dartlib/views/wallet/coins_manager/coins_manager_selected_types_list.dartlib/views/wallet/wallet_page/common/assets_list.dartlib/views/wallet/coins_manager/coins_manager_filters_dropdown.dart
📚 Learning: 2025-07-23T09:31:17.738Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 2976
File: lib/bloc/coins_manager/coins_manager_bloc.dart:82-83
Timestamp: 2025-07-23T09:31:17.738Z
Learning: In the CoinsManagerBloc, `state.selectedCoins` is used separately from `state.coins` to indicate whether a coin is enabled or not in the UI. The order of Set deduplication when merging coin lists doesn't affect UI behavior because selection state is tracked independently from the coin list used for filtering and sorting.
Applied to files:
lib/bloc/coins_manager/coins_manager_state.dartlib/app_config/app_config.dartlib/views/settings/widgets/security_settings/security_settings_page.dartlib/bloc/coins_manager/coins_manager_bloc.dartlib/views/wallet/coins_manager/coins_manager_selected_types_list.dartlib/bloc/coins_manager/coins_manager_event.dartlib/views/wallet/coins_manager/coins_manager_filters_dropdown.dart
📚 Learning: 2025-05-01T21:00:36.970Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 2608
File: lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart:2-3
Timestamp: 2025-05-01T21:00:36.970Z
Learning: It's acceptable to use unconditional `dart:io` imports in the Komodo wallet codebase when the usage is guarded by `!kIsWeb` conditional checks that prevent the platform-specific code from being executed in web environments.
Applied to files:
lib/services/feedback/feedback_service.dartlib/bloc/auth_bloc/auth_bloc.dartlib/services/file_loader/mobile/file_loader_native_ios.dartlib/bloc/coins_manager/coins_manager_bloc.dartlib/services/file_loader/file_loader_native_desktop.dartlib/services/file_loader/file_loader_web.dartlib/views/wallet/wallet_page/common/assets_list.dartlib/services/file_loader/mobile/file_loader_native_android.dartlib/views/wallet/coin_details/withdraw_form/withdraw_form.dart
📚 Learning: 2025-09-12T00:25:02.472Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 3129
File: lib/bloc/custom_token_import/data/custom_token_import_repository.dart:85-87
Timestamp: 2025-09-12T00:25:02.472Z
Learning: In the KdfCustomTokenImportRepository class in lib/bloc/custom_token_import/data/custom_token_import_repository.dart, the ERC20 implementation expects and requires integer chain IDs, so int.parse() on formattedChainId is acceptable without hex handling.
Applied to files:
lib/blocs/kmd_rewards_bloc.dartlib/bloc/coins_manager/coins_manager_bloc.dart
📚 Learning: 2025-05-08T19:07:13.442Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 2611
File: test_units/tests/system_health/system_clock_repository_test.dart:7-8
Timestamp: 2025-05-08T19:07:13.442Z
Learning: In the Komodo Wallet project, test files are structured to define test functions that are called from a central test runner in test_units/main.dart, rather than each test file having its own main() function.
Applied to files:
lib/bloc/coins_manager/coins_manager_bloc.dartlib/views/wallet/wallet_page/common/assets_list.dart
📚 Learning: 2025-05-08T19:05:13.083Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 2611
File: test_units/tests/system_health/http_head_time_provider_test.dart:8-24
Timestamp: 2025-05-08T19:05:13.083Z
Learning: In the Komodo Wallet project, test functions are defined in individual files under `test_units/tests/` but are executed through the central entry point in `test_units/main.dart`, so individual test files don't need their own `main()` function.
Applied to files:
lib/bloc/coins_manager/coins_manager_bloc.dartlib/views/wallet/wallet_page/common/assets_list.dart
📚 Learning: 2025-09-12T00:25:47.005Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 3129
File: lib/bloc/custom_token_import/data/custom_token_import_repository.dart:205-221
Timestamp: 2025-09-12T00:25:47.005Z
Learning: In the KdfCustomTokenImportRepository class in lib/bloc/custom_token_import/data/custom_token_import_repository.dart, the copyWithProtocolData extension method intentionally drops additional protocol fields when creating custom tokens - this is by design to ensure clean configuration without unexpected carryover from platform assets.
Applied to files:
lib/bloc/coins_manager/coins_manager_bloc.dart
📚 Learning: 2025-08-27T22:24:30.511Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 3109
File: lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart:50-54
Timestamp: 2025-08-27T22:24:30.511Z
Learning: During SDK migration for CEX market data in Komodo Wallet, functionality like coin filtering for profit/loss charts was moved to the SDK. The app-level filtering was kept as a safety net until SDK behavior could be confirmed, particularly for test coin filtering.
Applied to files:
lib/views/wallet/coins_manager/coins_manager_filters_dropdown.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Build Mobile (Android)
- GitHub Check: Build Mobile (iOS)
- GitHub Check: validate_code_guidelines
- GitHub Check: Test web-app-linux-profile
- GitHub Check: Test web-app-macos
- GitHub Check: Build Desktop (windows)
- GitHub Check: Build Desktop (linux)
- GitHub Check: Build Desktop (macos)
- GitHub Check: build_and_preview
- GitHub Check: unit_tests
- GitHub Check: build-linux-docker
🔇 Additional comments (48)
lib/views/settings/widgets/security_settings/security_settings_page.dart (5)
94-98: LGTM: Formatting improvement.The multi-line formatting of the
BlocProvidercreation improves readability without any behavioral changes.
115-116: LGTM: Formatting improvement.The callback formatting enhances readability without affecting behavior.
206-207: LGTM: Formatting improvement.The variable declaration formatting improves code readability without behavioral changes.
224-224: LGTM: CoinSubClass migration applied correctly.The filtering logic has been successfully updated to use
coin.id.subClass != CoinSubClass.siainstead of the legacyCoinTypeapproach. This change aligns with the repository-wide migration toCoinSubClassrepresentation and maintains the correct behavior of excluding Sia coins from the seed phrase export flow.
316-319: LGTM: Formatting improvement.The error message assignment formatting enhances readability of the ternary operator without affecting behavior.
lib/views/dex/entity_details/swap/swap_details.dart (2)
15-20: LGTM!The addition of the optional
belowUuidparameter provides a clean way to inject additional UI content below the UUID display, improving composability.
51-51: LGTM!Correctly passes the
belowUuidwidget toTradingDetailsCoinPairfor rendering.lib/views/dex/entity_details/trading_details_coin_pair.dart (4)
6-9: LGTM!Localization imports added correctly to support translatable UUID labels.
24-24: LGTM!The
belowUuidparameter is properly added to the constructor and field declaration, maintaining consistency with the refactoring inSwapDetails.Also applies to: 32-32
88-90: LGTM!Localization keys are used correctly with the
.tr()method and appropriate arguments for order vs. swap UUID labels.
101-109: LGTM!The conditional rendering properly checks both
swapIdandbelowUuidbefore rendering the additional content, with appropriate null safety using the!operator.lib/views/dex/entity_details/swap/swap_details_page.dart (1)
55-76: LGTM!The refactoring successfully moves the export button into
SwapDetailsvia thebelowUuidparameter, improving encapsulation. The state management correctly handles the loading state with_isExporting, showing a spinner and disabling the button during export operations.lib/bloc/transaction_history/transaction_history_bloc.dart (2)
110-118: LGTM! Timestamp update fixes the "Now" label persistence issue.The addition of
timestamp: sanitized.timestampcorrectly ensures that when a transaction gets confirmed, its timestamp updates from zero (displayed as "Now") to the actual block timestamp. This is consistent with updating other confirmation-related fields likeconfirmationsandblockHeight.
193-201: LGTM! Consistent timestamp update in new transactions path.This mirrors the fix in the historical subscription path, ensuring timestamp updates are applied uniformly across both code paths when transactions get confirmed.
lib/services/file_loader/file_loader_web.dart (2)
1-1: LGTM!The
dart:convertimport is necessary for JSON encoding and decoding operations in the new_saveAsJsonFilemethod.
21-25: LGTM!The routing logic correctly handles JSON files with case-insensitive extension checking and appropriate method delegation.
lib/services/file_loader/mobile/file_loader_native_android.dart (2)
6-6: LGTM!The
pathpackage import and routing logic correctly handle JSON file detection with case-insensitive extension checking.Also applies to: 22-26
39-42: LGTM!The suggested name logic improves UX by ensuring files have appropriate extensions when none is provided.
lib/services/file_loader/mobile/file_loader_native_ios.dart (1)
2-2: LGTM!The
dart:convertimport and routing logic correctly handle JSON file detection with appropriate method delegation.Also applies to: 34-38
lib/services/file_loader/file_loader_native_desktop.dart (3)
1-1: LGTM!The imports and routing logic correctly support JSON-specific file saving with case-insensitive extension checking.
Also applies to: 5-5, 20-24
55-58: LGTM!The suggested name logic ensures files have appropriate extensions, improving the save dialog UX.
64-81: LGTM!The JSON file saving implementation correctly pretty-prints JSON data with a graceful fallback to the original content if parsing fails. The comment at line 76 helpfully explains the fallback behavior.
lib/bloc/auth_bloc/auth_bloc.dart (1)
17-18: Localization imports correctly exposed for part filesExposing
easy_localizationandcodegen_loader.g.dartat the parent level is the right pattern so thetrezor_auth_mixin.dartpart can useLocaleKeys/tr()without its own imports. This matches the established part/import approach in this repo.Based on learnings, part files share the parent’s imports.
lib/shared/widgets/password_visibility_control.dart (1)
32-45: Simplified password visibility toggle is correctThe InkWell onTap toggle cleanly flips
_isObscuredand reuses the existing_setObscureTocallback flow. No regressions visible and behavior remains predictable.linux/my_application.cc (1)
3-3: Linux delete-event hook is correctly wired and non-intrusiveThe new
on_window_delete_eventcallback uses the right GTK signature, returnsFALSEto propagate the event, and is connected alongside the existing"destroy"handler. This should integrate cleanly with Flutter’s window‑close handling without changing default behavior or introducing leaks.Also applies to: 31-46, 111-115
lib/mm2/mm2_api/rpc/send_raw_transaction/send_raw_transaction_request.dart (1)
4-11: txHex/txJson support and assertion look consistent and safeAllowing either
txHexortxJsonwith an assertion on at least one being non-null, and emitting only non-null fields intoJson(), cleanly extends the request format without breaking existing call sites. No issues spotted.Also applies to: 17-18, 25-27, 36-37
lib/views/settings/widgets/general_settings/show_swap_data.dart (1)
106-106: Export button now correctly uses localized textSwitching the export button label to
LocaleKeys.exportSwapData.tr()is consistent with the rest of this widget and the app’s localization approach, and keeps user-facing text translatable.sdk (1)
1-1: Verify SDK submodule change manually—automated verification is limited by sandbox constraints.The SDK submodule exists with the commit pointer updated to 2da6d29f3db70c654281b565ab58dc76bc39694a (distinct from the previous 8fcf2079...), but the sandbox environment uses a shallow clone with blob:none filtering, preventing access to the new commit's details, changelog, and branch information.
Since this is a release candidate with dependent app-level changes (CoinSubClass refactoring, withdrawal execution migration, Sia support), you must manually verify:
- The new SDK commit is on a stable/release branch (not development)
- The commit includes no breaking changes affecting coin type handling or withdrawal execution
- All SDK dependencies in pubspec.yaml (equatable, uuid, flutter_bloc, get_it) are compatible with the new SDK version
lib/services/feedback/custom_feedback_form.dart (1)
177-177: LGTM! Localization improvement.The validation message is now properly localized, aligning with the broader localization effort across the feedback UI.
lib/model/coin_utils.dart (1)
146-147: LGTM! Sia coin type support added.The new case follows the established naming pattern and correctly integrates Sia coin support into the type name mapping.
lib/views/wallet/wallet_page/wallet_main/wallet_main.dart (1)
426-426: LGTM! Helpful documentation.The comment clarifies the priority-based sorting behavior for unauthenticated users, improving code maintainability.
lib/views/wallet/wallet_page/common/assets_list.dart (2)
3-3: LGTM! Necessary import for priority sorting.The import provides access to
unauthenticatedUserPriorityTickersused in the sorting logic.
99-124: LGTM! Priority sorting correctly implemented.The two-tier sorting logic correctly handles:
- Priority tickers first in the order specified by
unauthenticatedUserPriorityTickers- Non-priority tickers sorted alphabetically
- All edge cases covered with proper comparisons
lib/services/feedback/feedback_service.dart (1)
7-8: LGTM! Default feedback type now localized.The feedback type fallback is now properly localized, aligning with the broader localization improvements across the feedback system.
Also applies to: 91-91
lib/model/coin_type.dart (2)
1-6: LGTM! Helpful migration guidance.The documentation clearly marks
CoinTypeas a legacy compatibility layer and guides developers toward using SDK metadata, supporting the ongoing migration toCoinSubClass.
30-32: LGTM! Sia variant correctly added.The new
siaenum variant is properly documented as legacy glue for the Sia protocol, consistent with the file's compatibility layer purpose.lib/sdk/widgets/window_close_handler_exit_stub.dart (1)
1-6: LGTM! Web platform stub correctly implemented.The stub provides a consistent API surface for web builds while throwing an appropriate error if mistakenly called. The implementation relies on proper
kIsWebguards in calling code, as documented in the comment.lib/app_config/app_config.dart (1)
107-119: LGTM! Priority tickers well-defined.The constant provides a clear, ordered list of priority tickers for unauthenticated users. The selection includes major cryptocurrencies (BTC, ETH, BNB) along with ecosystem-relevant coins (KMD) and popular alternatives (LTC, DASH, ZEC, DOGE), which makes sense for initial user exposure.
lib/bloc/coins_manager/coins_manager_state.dart (1)
17-21: CoinSubClass migration for selectedCoinTypes is consistentThe switch of
selectedCoinTypestoList<CoinSubClass>plus the updatedcopyWithand doc comment is internally consistent and matches the broader CoinSubClass‑based filtering model.initial()andpropsare aligned, and this doesn’t interfere with the existingselectedCoinsvscoinsseparation.
Based on learnings,selectedCoinscontinues to represent enablement independently of filtering types.Also applies to: 52-65
lib/bloc/coins_manager/coins_manager_event.dart (1)
18-21: Event payload type now matches CoinSubClass‑based filteringUsing
CoinSubClassforCoinsManagerCoinTypeSelect.typeplus the new doc comment cleanly aligns the event with the updated state and UI filters; no issues spotted.lib/shared/utils/utils.dart (2)
390-466: Sia wiring in protocol utilities looks consistent; verify explorer usage vs history supportAdding
CoinType.siato:
getProtocolColorwith a dedicated color,hasTxHistorySupport(returningtrue), andgetNativeExplorerUrlByCoin’s legacy branchis coherent with enabling Sia across the app.
Note that
getNativeExplorerUrlByCoinasserts!hasSupportbased onhasTxHistorySupport(coin), so under normal flows Sia (which now has history support) should not be routed through this helper. That’s fine as long as Sia explorers go via protocol/SDK helpers (AssetExplorerUrlsetc.), and this switch case remains only to keep the exhaustive switch compiling.Please confirm Sia is not passed into
getNativeExplorerUrlByCoinin practice; otherwise the assert will trip in debug.Also applies to: 468-503
637-687: Refactor around confirmBeforeDisablingCoin improves clarity and reuseResolving
TradingEntitiesBlocviaRepositoryProvider.ofonce and reusing it forhasSwap,openOrdersCount, and cancellation simplifies the logic and avoids repeated lookups. The behavior around active swaps, open orders, and final disable confirmation remains unchanged.lib/bloc/coins_manager/coins_manager_bloc.dart (1)
186-191: CoinSubClass-based toggle logic is consistent with previous behaviorSwitching
newTypestoList<CoinSubClass>while keeping the contains/remove/append pattern preserves the old toggle semantics and avoids duplicates. Reads clean and matches the updated state shape.lib/views/wallet/coins_manager/coins_manager_selected_types_list.dart (1)
6-7: UI migration to CoinSubClass is consistent; color/label handling looks safeThe selector, label text (
type.formatted), smart chain comparison, andprotocolColorresolution are all correctly updated toCoinSubClass, with a sensible fallback tothemeData.colorScheme.primarywhentype.coloris null. This keeps the widget aligned with the new bloc state while guarding against missing protocol colors.Also applies to: 19-21, 40-42, 67-68, 70-72, 88-89, 91-93
lib/model/withdraw_details/withdraw_details.dart (1)
5-7: Call sites verified; assertion-only guard in release mode remains a design concernVerification confirms all direct
WithdrawDetails(...)constructor calls pass at least one oftxHexortxJson:
fromJson(line 30): passes bothtxHexandtxJsonfrom inputempty()(line 48): passestxHex: ''(non-null empty string)fromTrezorJson(line 91): passes bothtxHexandtxJsonfrom inputThe call sites are structurally sound. However, the original concern about the assertion-only guard in release builds remains valid: if the backend mistakenly provides both fields as null, the
assert(txHex != null || txJson != null)will not prevent instantiation in release mode. InfromJsonandfromTrezorJson, you're relying on backend input validity with only debug-time protection. If that's an acceptable risk for your backend contract, the current implementation is safe; otherwise, consider a runtime guard for defense-in-depth.lib/views/wallet/coins_manager/coins_manager_filters_dropdown.dart (2)
6-14: Imports updated to match CoinSubClass usageSwitching to
komodo_defi_typesandcoin.dartforCoinSubClass-based filtering, and dropping the older coin type/extension imports, is consistent with the broader migration and keeps this widget aligned with the new model layer.
184-189: _DropdownItem migration to CoinSubClass is consistentTaking
CoinSubClassdirectly (final CoinSubClass type;,Function(CoinSubClass) onTap) and displayingtype.formattedkeeps the UI aligned with the new model and avoids redundant conversions, while preserving the existing key format (filter-item-${type.name.toLowerCase()}) for tests/automation.Also applies to: 223-225
lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart (1)
717-721: ZHTLC preview delay note gating matches intentConditionally showing
ZhtlcPreviewDelayNoteonly whenstate.asset.id.subClass == CoinSubClass.zhtlcandstate.isSendingkeeps this warning specific to ZHTLC withdrawals and avoids cluttering other protocols’ flows.
| Future<void> _saveAsJsonFile({ | ||
| required String fileName, | ||
| required String data, | ||
| }) async { | ||
| final directory = await getApplicationDocumentsDirectory(); | ||
| final String suggestedName = | ||
| path.extension(fileName).isEmpty ? '$fileName.json' : fileName; | ||
| final filePath = path.join(directory.path, suggestedName); | ||
|
|
||
| String prettyData = data; | ||
| try { | ||
| final dynamic decoded = json.decode(data); | ||
| prettyData = const JsonEncoder.withIndent(' ').convert(decoded); | ||
| } catch (_) {} | ||
|
|
||
| final File file = File(filePath); | ||
| await file.writeAsString(prettyData); | ||
|
|
||
| await Share.shareXFiles([XFile(file.path)]); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify inconsistent Share API usage.
Line 89 uses Share.shareXFiles while lines 57 and 105 use SharePlus.instance.share. This inconsistency should be verified:
- The
Share.shareXFilescall doesn't include thesharePositionOriginparameter, which could affect iPad/tablet UX where the share sheet needs a source location. - Consider using the same API pattern for consistency.
Run the following script to check Share API usage patterns:
If the inconsistency is unintentional, apply this diff:
- await Share.shareXFiles([XFile(file.path)]);
+ await SharePlus.instance.share(
+ ShareParams(
+ files: [XFile(file.path)],
+ sharePositionOrigin: _getSharePositionOrigin(),
+ ),
+ );🏁 Script executed:
#!/bin/bash
# Description: Check Share API usage patterns in iOS file loader
# Search for all Share API calls in the file
rg -n "Share\." lib/services/file_loader/mobile/file_loader_native_ios.dartLength of output: 51
🏁 Script executed:
#!/bin/bash
# Check lines 50-110 to see context around claimed lines 57, 105 and actual line 89
wc -l lib/services/file_loader/mobile/file_loader_native_ios.dart
echo "=== Lines 50-60 ==="
sed -n '50,60p' lib/services/file_loader/mobile/file_loader_native_ios.dart
echo "=== Lines 85-95 ==="
sed -n '85,95p' lib/services/file_loader/mobile/file_loader_native_ios.dart
echo "=== Lines 100-110 ==="
sed -n '100,110p' lib/services/file_loader/mobile/file_loader_native_ios.dartLength of output: 1097
[/web_search]
🌐 Web query:
Share.shareXFiles deprecated share_plus Flutter iPad sharePositionOrigin
💡 Result:
Short answer: share.shareXFiles (and the other static Share.* methods) are deprecated. Use SharePlus.instance.share(...) with a ShareParams object (pass files via ShareParams.files). On iPad you must provide sharePositionOrigin (a non‑zero Rect) to avoid crashes/unresponsive UI.
Example migration:
- Old:
Share.shareXFiles(files, text: '...', subject: '...'); - New:
final box = context.findRenderObject() as RenderBox?;
final rect = box!.localToGlobal(Offset.zero) & box.size;
await SharePlus.instance.share(
ShareParams(
files: files,
text: '...',
subject: '...',
sharePositionOrigin: rect,
),
);
Sources: share_plus package docs (migration notes + iPad note) and related GitHub/issue discussions. [1][2]
References
[1] share_plus package (migration & iPad sharePositionOrigin).
[2] GitHub issue discussing sharePositionOrigin requirement on iPad.
Migrate from deprecated Share API and add iPad support parameter.
Line 89 uses the deprecated Share.shareXFiles API. This should be migrated to SharePlus.instance.share with ShareParams and include the sharePositionOrigin parameter, which is required on iPad to prevent crashes or unresponsive UI.
Other methods in this file (around lines 58 and 106) already use the correct pattern. Align line 89 with them:
- await Share.shareXFiles([XFile(file.path)]);
+ await SharePlus.instance.share(
+ ShareParams(
+ files: [XFile(file.path)],
+ sharePositionOrigin: _getSharePositionOrigin(),
+ ),
+ );Ensure _getSharePositionOrigin() is available (it should already exist in the file if other methods use it).
📝 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.
| Future<void> _saveAsJsonFile({ | |
| required String fileName, | |
| required String data, | |
| }) async { | |
| final directory = await getApplicationDocumentsDirectory(); | |
| final String suggestedName = | |
| path.extension(fileName).isEmpty ? '$fileName.json' : fileName; | |
| final filePath = path.join(directory.path, suggestedName); | |
| String prettyData = data; | |
| try { | |
| final dynamic decoded = json.decode(data); | |
| prettyData = const JsonEncoder.withIndent(' ').convert(decoded); | |
| } catch (_) {} | |
| final File file = File(filePath); | |
| await file.writeAsString(prettyData); | |
| await Share.shareXFiles([XFile(file.path)]); | |
| } | |
| Future<void> _saveAsJsonFile({ | |
| required String fileName, | |
| required String data, | |
| }) async { | |
| final directory = await getApplicationDocumentsDirectory(); | |
| final String suggestedName = | |
| path.extension(fileName).isEmpty ? '$fileName.json' : fileName; | |
| final filePath = path.join(directory.path, suggestedName); | |
| String prettyData = data; | |
| try { | |
| final dynamic decoded = json.decode(data); | |
| prettyData = const JsonEncoder.withIndent(' ').convert(decoded); | |
| } catch (_) {} | |
| final File file = File(filePath); | |
| await file.writeAsString(prettyData); | |
| await SharePlus.instance.share( | |
| ShareParams( | |
| files: [XFile(file.path)], | |
| sharePositionOrigin: _getSharePositionOrigin(), | |
| ), | |
| ); | |
| } |
🤖 Prompt for AI Agents
In lib/services/file_loader/mobile/file_loader_native_ios.dart around lines 71
to 90, the code still calls the deprecated Share.shareXFiles on line 89; replace
that call with SharePlus.instance.share using a ShareParams object that includes
the file(s) (e.g. files: [XFile(file.path)]) and the sharePositionOrigin
parameter by calling the existing _getSharePositionOrigin() helper so iPad
presentation works correctly; ensure you import/use SharePlus.instance.share and
construct ShareParams consistently with the other methods in this file.
| bool _suppressPreviewError = false; | ||
| late final _mm2Api = context.read<Mm2Api>(); |
There was a problem hiding this comment.
🧩 Analysis chain
Preview error → “max amount” flow is sound; localize dialog and consider narrowing listener scope
The new listener that reacts to previewError by checking state.isMaxAmount, parsing state.amount into Decimal, and comparing it to spendable with a tiny epsilon is a sensible way to detect “full balance” attempts and offer an automatic switch to max‑amount with a re‑preview. Two refinements:
- The dialog body text (
'Since you're sending your full amount, the network fee will be deducted from the amount. Do you agree?') is hard‑coded and not localized, unlike the surrounding strings—this should go through a newLocaleKeys.*entry in the translations file. - To avoid this prompt ever firing outside the fill step, you might want to include a step guard in
listenWhen(e.g., also checkcurr.step == WithdrawFormStep.fill) so any future reuse ofpreviewErrorin later steps doesn’t accidentally trigger the “deduct fee from full amount” flow.
Also applies to: 83-138
---
🏁 Script executed:
```shell
#!/bin/bash
# View the withdraw_form.dart file around the mentioned lines (55-56, 83-138)
cat lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart | head -n 150 | tail -n +45
Length of output: 4100
🏁 Script executed:
#!/bin/bash
# Search for dialog text about "Since you're sending your full amount" to verify localization
rg -n "Since you're sending your full amount" lib/views/wallet/coin_details/withdraw_form/Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for WithdrawFormStep enum to verify .fill exists
rg -n "enum WithdrawFormStep\|class WithdrawFormStep" lib/ -A 10Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for LocaleKeys usage in withdraw form context
rg -n "LocaleKeys\." lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart | head -20Length of output: 1159
🏁 Script executed:
#!/bin/bash
# Check for listenWhen usage in previewError listeners
rg -nB2 -A8 "listenWhen.*previewError\|previewError.*listenWhen" lib/views/wallet/coin_details/withdraw_form/Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Search more broadly for WithdrawFormStep definition
rg -n "WithdrawFormStep" lib/views/wallet/coin_details/withdraw_form/ | head -30
Length of output: 1095
🏁 Script executed:
#!/bin/bash
# Find where WithdrawFormStep is defined (likely a model/bloc file)
fd -e dart | xargs rg -l "enum WithdrawFormStep\|class WithdrawFormStep" lib/ 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for step property access to confirm it's used elsewhere as reference
rg -n "\.step\s*==" lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart | head -10Length of output: 168
🏁 Script executed:
#!/bin/bash
# Look for WithdrawFormStep.fill usage in codebase
rg -n "WithdrawFormStep\.fill" lib/Length of output: 474
Localize hard-coded dialog text and add step guard to preview error listener
The hard-coded dialog content at lines 115–117 ('Since you\'re sending your full amount, the network fee will be deducted from the amount. Do you agree?') is inconsistent with the localized title and buttons; this text should use a new LocaleKeys.* entry in the translations file.
Additionally, the listenWhen condition at line 84 lacks a step guard. Since other listeners in this file check the step (e.g., curr.step == WithdrawFormStep.success at line 141), consider adding && curr.step == WithdrawFormStep.fill to the condition to prevent this "deduct fee from full amount" flow from inadvertently triggering if previewError is reused in later steps.
sdk
Outdated
| @@ -1 +1 @@ | |||
| Subproject commit 8fcf2079d82dae7bd92f00652c430553ec8adfef | |||
| Subproject commit 2da6d29f3db70c654281b565ab58dc76bc39694a | |||
There was a problem hiding this comment.
Ensure PR description is finalized before release.
The PR description states "Changelog TBAI" (To Be Added In?), indicating the changelog is incomplete. For a release candidate, the PR description and changelog should be comprehensive and finalized.
Please complete the PR description with a comprehensive changelog documenting:
- All localization changes (Trezor auth, feedback/swap UI).
- CoinSubClass refactoring details and migration notes.
- Withdrawal execution changes (SDK-based streaming).
- New Sia coin support and any related changes.
- Any breaking changes or migration requirements for users/integrators.
|
Visit the preview URL for this PR (updated for commit 7c0f789): https://walletrc--pull-3407-merge-5j5ser05.web.app (expires Mon, 08 Dec 2025 10:04:31 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
fix balance address/total sync
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/bloc/coins_bloc/coins_bloc.dart(1 hunks)lib/bloc/coins_bloc/coins_repo.dart(1 hunks)sdk(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 3109
File: lib/bloc/cex_market_data/profit_loss/profit_loss_bloc.dart:50-54
Timestamp: 2025-08-27T22:24:30.511Z
Learning: During SDK migration for CEX market data in Komodo Wallet, functionality like coin filtering for profit/loss charts was moved to the SDK. The app-level filtering was kept as a safety net until SDK behavior could be confirmed, particularly for test coin filtering.
📚 Learning: 2025-07-23T09:31:17.738Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 2976
File: lib/bloc/coins_manager/coins_manager_bloc.dart:82-83
Timestamp: 2025-07-23T09:31:17.738Z
Learning: In the CoinsManagerBloc, `state.selectedCoins` is used separately from `state.coins` to indicate whether a coin is enabled or not in the UI. The order of Set deduplication when merging coin lists doesn't affect UI behavior because selection state is tracked independently from the coin list used for filtering and sorting.
Applied to files:
lib/bloc/coins_bloc/coins_repo.dartlib/bloc/coins_bloc/coins_bloc.dart
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
Applied to files:
lib/bloc/coins_bloc/coins_bloc.dart
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
Repo: KomodoPlatform/komodo-wallet PR: 2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The `excludedAssetList` from `app_config.dart` is used in the `_filterExcludedAssets` method in `coins_state.dart`. Since `coins_state.dart` is a part file of `coins_bloc.dart`, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Applied to files:
lib/bloc/coins_bloc/coins_bloc.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: validate_code_guidelines
- GitHub Check: unit_tests
- GitHub Check: Test web-app-macos
- GitHub Check: Test web-app-linux-profile
- GitHub Check: build-linux-docker
- GitHub Check: Build Mobile (iOS)
- GitHub Check: Build Desktop (linux)
- GitHub Check: Build Mobile (Android)
- GitHub Check: Build Desktop (windows)
- GitHub Check: Build Desktop (macos)
- GitHub Check: build_and_preview
🔇 Additional comments (2)
lib/bloc/coins_bloc/coins_bloc.dart (1)
159-169: LGTM! Keying migration from abbr to id-based keys is consistent.The change from
coin.abbrtocoin.id.idfor map keying is internally consistent within this method and aligns with the broader CoinSubClass migration mentioned in the PR objectives.lib/bloc/coins_bloc/coins_repo.dart (1)
778-787: LGTM! Balance broadcast addition improves consistency for non-streaming assets.The addition of
_broadcastBalanceChange(updatedCoin)ensures that non-streaming assets emit real-time balance changes through the same path as streaming assets. This is correctly positioned before the yield and is appropriately gated by the balance-changed check (lines 771-787), preventing unnecessary broadcasts.Since this method is described as "mostly a fallback" (line 740-742) for assets not using the SDK's streaming balance watchers, the broadcast addition fills a gap without causing duplicates for streaming assets.
| _log.warning( | ||
| 'Coin ${coin.abbr} not found in wallet coins, skipping update', | ||
| ); |
There was a problem hiding this comment.
Update the warning message to use the new key format.
The warning message on line 162 still references coin.abbr, but the method now uses coin.id.id as the key. This could cause confusion during debugging.
Apply this diff to use the consistent key format:
_log.warning(
- 'Coin ${coin.abbr} not found in wallet coins, skipping update',
+ 'Coin ${key} not found in wallet coins, skipping update',
);📝 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.
| _log.warning( | |
| 'Coin ${coin.abbr} not found in wallet coins, skipping update', | |
| ); | |
| _log.warning( | |
| 'Coin ${key} not found in wallet coins, skipping update', | |
| ); |
🤖 Prompt for AI Agents
In lib/bloc/coins_bloc/coins_bloc.dart around lines 161 to 163, the warning log
still references coin.abbr while the code uses coin.id.id as the key; update the
warning message to reference the new key format (coin.id.id) so the log reads
that the coin with key coin.id.id was not found in wallet coins and is being
skipped.
Draft PR — Komodo Wallet 0.9.3.zzz Release Candidate
Overview
This release candidate rolls the current
devbranch intomainfor v0.9.3.zzz. It focuses on delivering Sia withdrawal support, safer withdrawal UX, refreshed translations, and multiple desktop/platform fixes while staying aligned with the latestkomodo_defi_sdkcommits for preview/execution parity. Per request, it also folds in the broader feature narrative captured in the v0.9.3 release notes comment thread from KomodoPlatform/komodo-wallet#3164 so reviewers have a single document. The combined notes below can be pasted into KomodoPlatform/komodo-wallet#3407 once ready for review, with additional checkboxes or reviewer requests appended as needed.Highlights
🔐 New Capabilities
CoinSubClassso new SDK coin families become selectable without wallet-side changes._saveAsJsonFileto Android, iOS, desktop, and web file loaders, enabling consistent swap/order export across platforms.🎨 UX & Wallet Flow Improvements
💻 Platform & SDK Updates
executeWithdrawal()path, eliminating duplicate transaction signing and keeping preview signatures authoritative.delete-eventhandling so GTK closes cleanly, removes the unusedflutter_viewpointer, and ensures exit dialogs behave consistently.🐛 Bug Fixes & Cleanup
Carryover from v0.9.3 Release Notes (PR #3164)
These items were already reviewed inside the 0.9.3 release thread but remain part of the payload that is rolling forward with this RC. Keeping them here ensures SEC/QA have a complete picture without cross-referencing older comments.
🚀 New Features
🎨 UI & UX Enhancements
⚙️ Performance & Reliability
max_maker_volfixes, and explicit bloc typing merged earlier remain active.🐛 Bug Fixes
isLegacyWallet, sanitizes names, ensures uniqueness, and avoids duplicate imports.📋 Compliance & Operational Notes
Recommended QA / Regression Focus
Release Checklist
CHANGELOG.mdincludes the v0.9.3.zzz entry and references Sia withdrawals + desktop fixes.sdk/submodule hash matches the expected tag/commit for this RC.assets/translations/*.json) have been regenerated/bundled.flutter analyze(or CI equivalent) on touched packages once the branch is frozen.docs/MANUAL_TESTING_DEBUGGING.mdwith emphasis on withdraw/swap flows.Full Changelog
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.