Skip to content

sync main#3324

Merged
ca333 merged 9 commits intomainfrom
dev
Oct 31, 2025
Merged

sync main#3324
ca333 merged 9 commits intomainfrom
dev

Conversation

@ca333
Copy link
Copy Markdown
Contributor

@ca333 ca333 commented Oct 31, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • Withdrawal form now displays transaction details after successful completion.
    • Grouped coin balance summary added to orders table, showing aggregated spendable amounts and USD values.
  • Bug Fixes

    • Corrected translation for withdrawal confirmation message.
  • UI/UX

    • Updated withdrawal confirmation button label from "Confirm" to "Send" for improved clarity.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 31, 2025

Walkthrough

This PR integrates MM2 API into the withdrawal flow, replacing Trezor-progress signaling with direct broadcast via MM2. UI improvements include transaction details display post-withdrawal, grouped coin balance visualization in orders table, and an optional trailing widget parameter for coin table items. A translation key is corrected.

Changes

Cohort / File(s) Summary
Withdrawal Flow Integration
lib/bloc/withdraw_form/withdraw_form_bloc.dart, lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart
Added MM2 API dependency to WithdrawFormBloc constructor and replaced Trezor-progress streaming with single MM2 broadcast call (sendRawTransaction). Updated WithdrawForm to pass MM2 API to bloc, construct Transaction model from withdrawal result, and wire success callback through TransactionDetails closure. Changed confirmation button label from "Confirm" to "Send".
Coin & Order Table UI Enhancements
lib/views/dex/simple/form/tables/coins_table/coins_table_item.dart, lib/views/dex/simple/form/tables/orders_table/grouped_list_view.dart
Added optional trailing widget parameter to CoinsTableItem for custom rendering. Introduced new _GroupedCoinBalance widget in grouped order list to display aggregated spendable balances and USD values per coin group in expansion tile header.
Localization Fix
assets/translations/en.json
Corrected translation key "confirmSending" from "Confirm sending" to "Confirm withdrawl".

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant WithdrawForm as WithdrawForm UI
    participant Bloc as WithdrawFormBloc
    participant MM2 as MM2 API
    participant TxDetails as TransactionDetails

    User->>WithdrawForm: Initiate withdrawal
    WithdrawForm->>Bloc: Submit withdrawal (new with mm2Api)
    Bloc->>MM2: sendRawTransaction (broadcast)
    MM2-->>Bloc: Success {txHash, balanceChanges, coin, fee, ...}
    Bloc->>Bloc: Construct Transaction model
    Bloc-->>WithdrawForm: Emit success state
    WithdrawForm->>TxDetails: Display transaction details
    User->>TxDetails: Review & close
    TxDetails-->>WithdrawForm: onDone callback
    WithdrawForm->>User: Return to previous screen
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Extra attention areas:
    • lib/bloc/withdraw_form/withdraw_form_bloc.dart — Logic density in submission flow refactoring (Trezor progress removal, MM2 broadcast integration, error handling updates); validate state machine transitions and result construction.
    • lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart — Callback wiring changes and async path modifications in BlocListener; verify success flow closure timing with TransactionDetails integration.
    • lib/views/dex/simple/form/tables/orders_table/grouped_list_view.dart — New _GroupedCoinBalance widget logic for balance aggregation and USD value calculation; verify ticker derivation and rendering constraints.

Possibly related PRs

  • #3269: Adds iOS FD-monitoring error handling in the same WithdrawFormBloc submission flow.
  • #3245: Modifies WithdrawFormBloc behavior and constructor parameters.
  • #3270: Updates platform checks and FD logging in WithdrawFormBloc alongside related field/constructor changes.

Suggested labels

codex, QA

Suggested reviewers

  • takenagain
  • smk762
  • CharlVS

Poem

🐰 Hopping through withdrawals with MM2's fast track,
No Trezor progress loops, we're taking a new path back!
Grouped balances gleam in the orders' bright header,
Transaction details bloom—the UI's much better!
Thump thump!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The pull request title "sync main" is vague and generic, providing no meaningful information about the actual changes in the changeset. While the PR does involve syncing from the dev branch to main, the title fails to convey any details about the substantial modifications across five files, including major changes like MM2 integration in the withdrawal flow, UI enhancements with new widgets, and constructor signature updates. A teammate scanning the history would have no understanding of what this PR actually accomplishes from the title alone.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +69 to +72
trailing: _GroupedCoinBalance(
coins: group.value
.map((item) => getCoin(context, item))
.toList(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Deduplicate coins before summing grouped balances

The header balance for grouped coins is built from group.value.map((item) => getCoin(context, item)).toList() and then summed in _GroupedCoinBalance. group.value contains every order in the group, so the same Coin object is inserted once per order. When a coin has multiple orders, its spendable and USD amounts are added repeatedly, inflating the totals in the group header. The list should be deduplicated by coin id before summing so each variant contributes its balance only once.

Useful? React with 👍 / 👎.

@ca333 ca333 merged commit f79000c into main Oct 31, 2025
2 of 12 checks passed
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 513c264 and 1e3daa0.

📒 Files selected for processing (5)
  • assets/translations/en.json (1 hunks)
  • lib/bloc/withdraw_form/withdraw_form_bloc.dart (3 hunks)
  • lib/views/dex/simple/form/tables/coins_table/coins_table_item.dart (2 hunks)
  • lib/views/dex/simple/form/tables/orders_table/grouped_list_view.dart (3 hunks)
  • lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart (10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • lib/bloc/withdraw_form/withdraw_form_bloc.dart
  • lib/views/dex/simple/form/tables/coins_table/coins_table_item.dart
  • lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart
  • lib/views/dex/simple/form/tables/orders_table/grouped_list_view.dart
🧠 Learnings (3)
📚 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/withdraw_form/withdraw_form_bloc.dart
  • lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart
  • lib/views/dex/simple/form/tables/orders_table/grouped_list_view.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/withdraw_form/withdraw_form_bloc.dart
  • lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart
  • lib/views/dex/simple/form/tables/orders_table/grouped_list_view.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/bloc/withdraw_form/withdraw_form_bloc.dart
  • lib/views/wallet/coin_details/withdraw_form/withdraw_form.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). (10)
  • GitHub Check: Build Desktop (macos)
  • GitHub Check: Build Desktop (linux)
  • GitHub Check: Build Desktop (windows)
  • GitHub Check: Test web-app-macos
  • GitHub Check: Test web-app-linux-profile
  • GitHub Check: Build Mobile (iOS)
  • GitHub Check: Build Mobile (Android)
  • GitHub Check: validate_code_guidelines
  • GitHub Check: unit_tests
  • GitHub Check: build_and_preview

"transactionDenied": "Denied",
"coinDisableSpan1": "You can't disable {} while it has a swap in progress",
"confirmSending": "Confirm sending",
"confirmSending": "Confirm withdrawl",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix spelling in confirmation label.

“Confirm withdrawl” has a typo; please change it to “Confirm withdrawal” so the UI copy reads correctly.

-  "confirmSending": "Confirm withdrawl",
+  "confirmSending": "Confirm withdrawal",
📝 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
"confirmSending": "Confirm withdrawl",
"confirmSending": "Confirm withdrawal",
🤖 Prompt for AI Agents
In assets/translations/en.json at line 66, the confirmation label has a typo
"Confirm withdrawl"; update the string value to "Confirm withdrawal" so the UI
copy reads correctly (i.e., replace withdrawl with withdrawal).

Comment on lines +728 to 749
id: result.txHash,
internalId: result.txHash,
assetId: state.asset.id,
balanceChanges: result.balanceChanges,
// Show as unconfirmed initially
timestamp: DateTime.fromMillisecondsSinceEpoch(0),
confirmations: 0,
blockHeight: 0,
from: state.selectedSourceAddress != null
? [state.selectedSourceAddress!.address]
: <String>[],
to: [result.toAddress],
txHash: result.txHash,
fee: result.fee,
memo: state.memo,
);

return TransactionDetails(
transaction: tx,
coin: state.asset.toCoin(),
onClose: onDone,
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid showing a 1970 timestamp in the success details.

Setting timestamp to DateTime.fromMillisecondsSinceEpoch(0) makes the TransactionDetails panel render “Jan 1, 1970” for a brand-new withdrawal, which is misleading for users. Feed a real timestamp (e.g., DateTime.now()) or adjust the widget to treat missing timestamps specially so the success view reflects the actual submission time.

-          timestamp: DateTime.fromMillisecondsSinceEpoch(0),
+          timestamp: DateTime.now(),
📝 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
id: result.txHash,
internalId: result.txHash,
assetId: state.asset.id,
balanceChanges: result.balanceChanges,
// Show as unconfirmed initially
timestamp: DateTime.fromMillisecondsSinceEpoch(0),
confirmations: 0,
blockHeight: 0,
from: state.selectedSourceAddress != null
? [state.selectedSourceAddress!.address]
: <String>[],
to: [result.toAddress],
txHash: result.txHash,
fee: result.fee,
memo: state.memo,
);
return TransactionDetails(
transaction: tx,
coin: state.asset.toCoin(),
onClose: onDone,
);
id: result.txHash,
internalId: result.txHash,
assetId: state.asset.id,
balanceChanges: result.balanceChanges,
// Show as unconfirmed initially
timestamp: DateTime.now(),
confirmations: 0,
blockHeight: 0,
from: state.selectedSourceAddress != null
? [state.selectedSourceAddress!.address]
: <String>[],
to: [result.toAddress],
txHash: result.txHash,
fee: result.fee,
memo: state.memo,
);
return TransactionDetails(
transaction: tx,
coin: state.asset.toCoin(),
onClose: onDone,
);
🤖 Prompt for AI Agents
In lib/views/wallet/coin_details/withdraw_form/withdraw_form.dart around lines
728 to 749, the code creates a Transaction with timestamp set to
DateTime.fromMillisecondsSinceEpoch(0) which displays Jan 1, 1970; change the
timestamp to a real submission time (e.g., DateTime.now()) or supply null if
Transaction.timestamp is nullable and let TransactionDetails handle missing
timestamps; update the Transaction construction to pass DateTime.now() (or null)
and ensure any downstream types accept the change.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants