fix(withdraw): revert temporary IBC channel field type workaround#2938
fix(withdraw): revert temporary IBC channel field type workaround#2938
Conversation
TODO: bump SDK commit hash once changes are merged in 80d2bc5#diff-7281b7248c6013f241f51bcba3a811e1caade2f41936a141f6531f6849d1a632
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes simplify the IBC channel input handling by removing format validation and only requiring the field to be non-empty. The translation file is updated accordingly, the input widget is refactored for simplicity, and the IBC channel is now parsed as an integer rather than a string. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant IbcChannelField
participant WithdrawFormBloc
participant WithdrawFormState
User->>IbcChannelField: Enter IBC channel (digits only)
IbcChannelField->>WithdrawFormBloc: onIbcChannelChanged(channel)
WithdrawFormBloc->>WithdrawFormBloc: Check if channel is empty
alt Channel is empty
WithdrawFormBloc->>WithdrawFormState: Emit error "IBC channel is required"
else Channel is not empty
WithdrawFormBloc->>WithdrawFormState: Emit channel, clear error
end
WithdrawFormState->>WithdrawFormState: Parse channel as integer for parameters
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Visit the preview URL for this PR (updated for commit 7dded73): https://walletrc--pull-2938-merge-njib9hxn.web.app (expires Wed, 23 Jul 2025 13:26:22 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/bloc/withdraw_form/withdraw_form_bloc.dart (1)
372-388: Consider adding basic numeric validation for better error handling.The simplified validation only checks for empty input, but since the state layer uses
int.tryParse(), invalid numeric input could result in a null value being passed to the API. Consider adding a basic numeric validation check to provide better user feedback.void _onIbcChannelChanged( WithdrawFormIbcChannelChanged event, Emitter<WithdrawFormState> emit, ) { if (event.channel.isEmpty) { emit( state.copyWith( ibcChannel: () => event.channel, ibcChannelError: () => TextError(error: 'Channel ID is required'), ), ); return; } + + // Validate that the input is numeric + if (int.tryParse(event.channel) == null) { + emit( + state.copyWith( + ibcChannel: () => event.channel, + ibcChannelError: () => TextError(error: 'Channel ID must be a number'), + ), + ); + return; + } emit( state.copyWith( ibcChannel: () => event.channel, ibcChannelError: () => null, ), ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
lib/generated/codegen_loader.g.dartis excluded by!**/generated/**packages/komodo_ui_kit/pubspec.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
assets/translations/en.json(1 hunks)lib/bloc/withdraw_form/withdraw_form_bloc.dart(1 hunks)lib/bloc/withdraw_form/withdraw_form_state.dart(1 hunks)lib/views/wallet/coin_details/withdraw_form/widgets/fill_form/fields/fields.dart(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
lib/views/wallet/coin_details/withdraw_form/widgets/fill_form/fields/fields.dart (3)
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The `excludedAssetList` from `app_config.dart` is used in the `_filterExcludedAssets` method in `coins_state.dart`. Since `coins_state.dart` is a part file of `coins_bloc.dart`, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#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.
⏰ 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 Desktop (linux)
- GitHub Check: unit_tests
- GitHub Check: Build Desktop (macos)
- GitHub Check: Build Desktop (windows)
- GitHub Check: validate_code_guidelines
- GitHub Check: Test web-app-linux-profile
- GitHub Check: Build Mobile (iOS)
- GitHub Check: Test web-app-macos
- GitHub Check: Build Mobile (Android)
- GitHub Check: build_and_preview
- GitHub Check: build-android-docker
🔇 Additional comments (4)
assets/translations/en.json (1)
687-687: LGTM: Hint text updated to match numeric-only input format.The change from "channel-141" to "141" aligns with the simplified IBC channel input that now accepts only digits, removing the need for the "channel-" prefix.
lib/bloc/withdraw_form/withdraw_form_state.dart (1)
189-189: LGTM: Correctly implements the revert to integer type for IBC channel.The change from a trimmed string to
int.tryParse(ibcChannel!.trim())properly converts the input to an integer while handling parsing failures gracefully by returning null.lib/views/wallet/coin_details/withdraw_form/widgets/fill_form/fields/fields.dart (2)
6-6: LGTM: Appropriate import added for input filtering.The import for
FilteringTextInputFormatteris correctly added to support the digits-only input restriction.
583-588: LGTM: Widget simplification improves performance.The refactor from
StatefulWidgettoStatelessWidgetwithBlocBuilderis a good architectural improvement that reduces complexity and improves performance.
lib/views/wallet/coin_details/withdraw_form/widgets/fill_form/fields/fields.dart
Outdated
Show resolved
Hide resolved
lib/views/wallet/coin_details/withdraw_form/widgets/fill_form/fields/fields.dart
Outdated
Show resolved
Hide resolved
smk762
left a comment
smk762
left a comment
There was a problem hiding this comment.
Successful IBC transfer confirmed sent from ATOM -> ATOM-OSMO_IBC (we need to add this to the coins file!)
3 participants
Reverts the temporary workaround in 80d2bc5
Changes the
ibc_source_channeltype fromStringtointand removes string-based validationSummary by CodeRabbit
New Features
Bug Fixes
Refactor
Style