Conversation
fixes "setState() or markNeedsBuild() called when widget tree was locked. This ScreenshotSensitivity widget cannot be marked as needing to build because the framework is locked." and "DartError: setState() or markNeedsBuild() called during build. This ScreenshotSensitivity widget cannot be marked as needing to build because the framework is already in the process of building widgets. A widget can be marked as needing to be built during the build phase only if one of its ancestors is currently building. This exception is allowed because the framework builds parent widgets before children, which means a dirty descendant will always be built. Otherwise, the framework might not visit this widget during this build phase."
|
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 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. WalkthroughAdds ARRR/ZHTLC activation infrastructure: a new ArrrActivationService with config/result/status types, DI bootstrap, and UI handlers. Updates coins activation flow to route ZHTLC assets through the service. Introduces configuration dialog/handler and an activation status bar. Wraps key pages with the handler. Updates translations and excluded assets. Adjusts screenshot sensitivity notifications. Updates SDK submodule branch. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Wallet/Dex/Bridge UI
participant CoinsRepo
participant ArrrSvc as ArrrActivationService
participant Handler as ZhtlcConfigurationHandler
participant Dialog as ZhtlcConfigurationDialog
participant SDK as SDK (ARRR/ZHTLC)
User->>UI: Activate ZHTLC asset
UI->>CoinsRepo: activateAssetsSync(assets)
alt Asset is ZHTLC
CoinsRepo->>ArrrSvc: activateArrr(asset)
alt Needs configuration
ArrrSvc-->>Handler: configurationRequests(asset, requiredSettings)
Handler->>Dialog: confirmZhtlcConfiguration(asset)
Dialog-->>Handler: ZhtlcUserConfig or cancel
alt User provided config
Handler->>ArrrSvc: submitConfiguration(assetId, config)
else Cancelled/error
Handler->>ArrrSvc: cancelConfiguration(assetId)
ArrrSvc-->>CoinsRepo: error
end
end
ArrrSvc->>SDK: start activation
SDK-->>ArrrSvc: progress events
ArrrSvc-->>UI: activationStatuses[asset] updated
ArrrSvc-->>CoinsRepo: success/error
CoinsRepo->>CoinsRepo: subscribeToBalanceUpdates(asset[, parent])
UI-->>UI: ZhtlcActivationStatusBar polls statuses
else Non-ZHTLC asset
CoinsRepo->>CoinsRepo: existing activation path
end
sequenceDiagram
participant ArrrSvc as ArrrActivationService
participant StatusBar as ZhtlcActivationStatusBar
rect rgba(225,245,254,0.4)
note over ArrrSvc,StatusBar: Periodic UI refresh
StatusBar->>StatusBar: Timer tick (1s)
StatusBar->>ArrrSvc: activationStatuses
ArrrSvc-->>StatusBar: Map<AssetId, Status>
StatusBar->>StatusBar: filter recent in-progress/completed/errors
StatusBar-->>StatusBar: render progress/message
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
lib/views/wallet/wallet_page/common/zhtlc/zhtlc_activation_status_bar.dart (1)
25-51: Refresh the cached statuses before the first frameRight now we only populate
_cachedStatusesafter the first one-second timer tick, so a status bar that should already be visible renders empty momentarily. Grabbing the latest snapshot ininitStatebefore starting the timer keeps the UI accurate from the very first frame. Something along these lines would do it:void initState() { super.initState(); + _cachedStatuses = widget.activationService.activationStatuses; _startPeriodicRefresh(); }lib/services/arrr_activation/arrr_activation_service.dart (1)
267-288: Avoid silently losing configuration requests when no listeners are readyIf no UI listener attaches within 10 seconds, we still emit to
_configRequestController; broadcast streams drop events when no one is subscribed, so the request disappears and the activation waits 15 minutes before timing out. Consider short‑circuiting with an immediate error (or queueing until a listener arrives) instead of fire‑and‑forget to a listener-less stream.lib/services/arrr_activation/arrr_config.dart (1)
53-66: Ensure deterministic equality for required settingsUsing
other.requiredSettings == requiredSettingscompares list references, not contents. Two logically identicalArrrActivationResultNeedsConfiginstances with different list instances will compare unequal, hurting caching/UI diffing. ApplyListEquality(frompackage:collection) or wrap withUnmodifiableListViewplus value equality.lib/bloc/app_bloc_root.dart (1)
26-26: Remove unused import (if not needed here)The ArrrActivationService import appears unused in this file. Consider removing to avoid warnings; it’s already imported in main.dart for the part that registers it.
Based on learnings
-import 'package:web_dex/services/arrr_activation/arrr_activation_service.dart';lib/services/initializer/app_bootstrapper.dart (1)
44-46: Registering ArrrActivationService: minor cleanup and lifecycle
- Remove extra whitespace in constructor call.
- If ArrrActivationService holds resources (streams/timers), register a dispose callback with GetIt.
- final arrrActivationService = ArrrActivationService( kdfSdk); - GetIt.I.registerSingleton<ArrrActivationService>(arrrActivationService); + final arrrActivationService = ArrrActivationService(kdfSdk); + // If ArrrActivationService exposes dispose/close, wire it here: + GetIt.I.registerSingleton<ArrrActivationService>( + arrrActivationService, + // dispose: (s) => s.dispose(), + );If the service needs async initialization (e.g., preloading Zcash params), consider registerSingletonAsync with dependsOn to avoid race conditions.
.gitmodules (1)
4-4: Pin SDK submodule to specific commit SHABranch bugfix/zhltc-activation-fixes exists upstream (HEAD 0d6b12ca533014fb658a07a7f8dbe7db07d6d733). Update the superproject to reference this SHA for reproducibility and optionally remove the
branchhint from .gitmodules.lib/views/dex/dex_page.dart (1)
20-21: Decouple ZhtlcConfigurationHandler into a shared module
The handler currently lives in lib/views/wallet/wallet_page/common/zhtlc; move it to lib/views/common/zhtlc or shared/widgets to avoid DEX→wallet coupling. DexPage is only wrapped here—no duplicate dialogs.
📜 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 (19)
.gitmodules(1 hunks)assets/translations/en.json(1 hunks)lib/app_config/app_config.dart(0 hunks)lib/bloc/app_bloc_root.dart(1 hunks)lib/bloc/coins_bloc/coins_repo.dart(6 hunks)lib/main.dart(1 hunks)lib/services/arrr_activation/arrr_activation_service.dart(1 hunks)lib/services/arrr_activation/arrr_config.dart(1 hunks)lib/services/initializer/app_bootstrapper.dart(1 hunks)lib/shared/screenshot/screenshot_sensitivity.dart(2 hunks)lib/views/bridge/bridge_page.dart(6 hunks)lib/views/dex/dex_page.dart(3 hunks)lib/views/wallet/coins_manager/coins_manager_page.dart(2 hunks)lib/views/wallet/wallet_page/common/zhtlc/zhtlc_activation_status_bar.dart(1 hunks)lib/views/wallet/wallet_page/common/zhtlc/zhtlc_configuration_dialog.dart(1 hunks)lib/views/wallet/wallet_page/common/zhtlc/zhtlc_configuration_handler.dart(1 hunks)lib/views/wallet/wallet_page/wallet_main/active_coins_list.dart(2 hunks)lib/views/wallet/wallet_page/wallet_main/wallet_main.dart(12 hunks)sdk(1 hunks)
💤 Files with no reviewable changes (1)
- lib/app_config/app_config.dart
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
Applied to files:
lib/bloc/app_bloc_root.dartlib/views/wallet/coins_manager/coins_manager_page.dartlib/views/wallet/wallet_page/wallet_main/wallet_main.dartlib/bloc/coins_bloc/coins_repo.dart
📚 Learning: 2025-09-02T15:57:12.405Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#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/views/wallet/wallet_page/wallet_main/wallet_main.dart
📚 Learning: 2025-08-31T19:24:53.317Z
Learnt from: CR
PR: KomodoPlatform/komodo-wallet#0
File: AGENTS.md:0-0
Timestamp: 2025-08-31T19:24:53.317Z
Learning: Use BLoC where applicable and follow BLoC conventions
Applied to files:
lib/views/wallet/wallet_page/wallet_main/wallet_main.dart
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The `excludedAssetList` from `app_config.dart` is used in the `_filterExcludedAssets` method in `coins_state.dart`. Since `coins_state.dart` is a part file of `coins_bloc.dart`, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Applied to files:
lib/views/wallet/wallet_page/wallet_main/wallet_main.dart
📚 Learning: 2025-08-31T19:24:53.317Z
Learnt from: CR
PR: KomodoPlatform/komodo-wallet#0
File: AGENTS.md:0-0
Timestamp: 2025-08-31T19:24:53.317Z
Learning: For Komodo DeFi Flutter SDK repository, reference `/KDF_API_DOCUMENTATION.md` and applicable RPCs, data structures, and notes when implementing RPC-related features
Applied to files:
lib/bloc/coins_bloc/coins_repo.dart
🔇 Additional comments (5)
sdk (1)
1-1: Submodule pointer update looks good.Thanks for pinning the sdk submodule to the new commit; the ref change is clear and isolated.
lib/main.dart (1)
31-31: LGTM: import ensures part file sees ArrrActivationServiceImporting arrr_activation_service at the library level is needed so the part file (app_bootstrapper.dart) can use it. Good.
Based on learnings
Also applies to: 36-36
lib/views/wallet/coins_manager/coins_manager_page.dart (1)
11-13: LGTM: Handler integration and constructor cleanup
- Good use of ZhtlcConfigurationHandler around the page.
- super.key constructor style is idiomatic.
Also applies to: 16-20, 34-61
assets/translations/en.json (1)
763-784: Validate ZHTLC i18n keys and placeholders
- No other locale files under assets/translations — cross-locale coverage N/A.
- lib/generated/codegen_loader.g.dart already declares all ZHTLC LocaleKeys.
- Confirm each
{}placeholder (e.g. in zhtlcConfigureTitle, zhtlcErrorSettingUpZcash) aligns with code usage.lib/views/bridge/bridge_page.dart (1)
19-19: Approve ZHTLC handler usage Confirmed a single ZhtlcConfigurationHandler per page—no nested wrappers in Bridge, DEX, Coins Manager, or Wallet.
lib/views/wallet/wallet_page/common/zhtlc/zhtlc_configuration_dialog.dart
Outdated
Show resolved
Hide resolved
lib/views/wallet/wallet_page/common/zhtlc/zhtlc_configuration_handler.dart
Show resolved
Hide resolved
|
Visit the preview URL for this PR (updated for commit c6e7621): https://walletrc--pull-3158-merge-9wqosmg9.web.app (expires Thu, 09 Oct 2025 18:47:37 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
- cache heavy operations that aren't expected to change regularly. - add debounce on search input
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive ZHTLC (zero hash time locked contracts) asset support to the wallet application, enabling users to activate and manage ZHTLC assets like ARRR (Pirate Chain) with guided configuration.
Key changes include:
- Guided ZHTLC asset activation with automatic Zcash parameter detection and download
- Global configuration dialog system across Wallet, DEX, and Bridge pages
- Real-time activation status display for improved user experience
Reviewed Changes
Copilot reviewed 18 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/services/arrr_activation/ | New service layer for ZHTLC activation with configuration management |
| lib/views/wallet/wallet_page/common/zhtlc/ | New UI components for ZHTLC configuration dialogs and status display |
| lib/bloc/coins_bloc/coins_repo.dart | Integration of ZHTLC activation service into coin repository |
| lib/views/wallet/wallet_page/wallet_main/ | Integration of ZHTLC handlers and status display |
| assets/translations/en.json | Added ZHTLC-specific translation strings |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lib/views/wallet/wallet_page/common/zhtlc/zhtlc_configuration_handler.dart
Outdated
Show resolved
Hide resolved
|
In Web, HD mode:
Can we please add to this warning - After switching to portfoilio and back into ARRR wallet 
General request for task based activations - while activation is in progress:
Minor theme fail - black text under "loading addresses" is hard to see, should be white in dark mode & viceversa. User should not have to await loading of addresses here, as ARRR does not allow creation of additional addresses in app. Related (though this applies to HD login also when coins are singular with no option to add more) #2804 I was stuck here for a while after attempting to create a transaction, then having the form drop out due to me opening the dev console, and reentering the form again. Eventually address showed, assuming after background tx generation completed. 
This is the display while the transaction is being generated. There is no spinner, nor any other indication of an operation in progress. Some links related for context:
Suggest spinner + After some patience and uncertainty of progress, I was rewarded with the confirmation screen: 
This should include the memo field (among other transaction metadata). This is a global, not z coin specific. Issues for this exist at [1] [2] After clicking "send", it spins again It is unclear if exiting this screen will result in a failed transaction (it shouldnt, but this is not clear in UI). This is especially confusing with ARRR, where there is no transaction history in wallet page - user must wait and see if balance changes. It appears that https://komodoplatform.com/en/docs/komodo-defi-framework/api/v20/wallet/tx/zhtlc_tx_history/ has not been implemented. Eventually, balance change indicates withdrawal was successful, despite the UX requiring faith rather than offering guidance. As suspected, post withdraw, some previously hidden balance was revealed also. Balance grew to 1.9 after it was at 1, and I withdrew 0.1 - the "bonus" would be from tx prior to sync settings being awakened by the withdrawal. Unable to view orderbook - Despite this, the form was prefilled, and the "Swap now" button active, so it was clicked to see what would happen. 
Confirm button begins its spin... I have some concerns that these waits, and the consequences of navigating away, may negatively impact UX when slow actions need to be repeated due to initial impatience or unawareness, and abandoned operations may continue to block subsequent actions until the abandoned is complete. Still waiting for confirm button to stop spinning spin... lets go get a coffee... and mow the lawn... and it's still spinning after 30 minutes. Being as I am notoriously impatient, this was the moment I gave up, and found out the "back" button wasn't working either. SummaryCrititcal
High priority, low effort UX fixes
High priority, medium effort UX fixes with global scope
Medium priority, medium effort UX fixes with global scope (will delegate to issues)
|
With ZHTLC enabled, the `get_enabled_coins` RPC takes long - not yet sure why - and the taker form calls the validator regularly, resulting in long loading times and input delays
|
Most recent commit not building, tests below based on b351e11 Confirmed extra warning text on sync params input ✔️ 
Can we please reduce the font a little here? 
Still a bit of stealth text here (balance in black to the right of the blue icon) |
|
Triggered by either attempting orderbook request during ARRR activation loop, or due to other Komodo-Wallet tabs open from previously. 
Retry after tab reload, activation message row, but no ARRR row (previously ARR was seen and clickable during activation). 
Though its preferable to hide / disable the ARRR row until activation completes (to avoid premature action conflicts), after waiting 5 mins, I came to the conclusion that this activation is likely not progressing. Log files confirm it stuck, with repeating logs for |
|
A swap was initiated (on web), and negotiated successfully. I navigated away while it was "sending taker fee" and the GUI became locked soon after (assuming due to ARRR transaction generation, as ARRR was the taker coin). After around 30 seconds, the gui was unblocked and I was able to resume navigation without further issues. I tried to also create a ARRR buy with KMD as the taker coin. First attempt failed to negotiate (possibly a maker side issue), but the second started as expected. Swap where ARRR was taker coin failed at Swap where ARRR was maker coin successful after 25 min: |
fetching tx history of activating coins wrecks havok on startup, especially for ARRR
This logic was mistakenly not copied over from the normal coin activation function
|
Confirmed critical functionality completed (activation, withdraw, trade, privkey, tx history).
- ARRR taker sells failing at `TakerPaymentSent` stage (3 fails, no success so far).
|
@smk762 you mention swaps working, so does that mean that maker orders are working? For critical issues, can you please also test on native build so we can isolate if only wasm or not? |
So far only taker tested. Where ARRR is sell coin, it has failed. Where ARRR is buy coin, it succeeds. Desktop tests scheduled for today. So far tests have been in HD also - Iguana tests also scheduled for today. Initial tests on Iguana / Linux Desktop show much faster operations. Confirmed A parallel web instance, using a different seed, was launched in Iguana mode for maker trade counterparty. ARRR once again failed to activate (NoSuchCoin), likely due to user entering ARRR wallet page before activation process was complete. I cant be certain of this, but there appears to be a strong correlation between this action and the resulting error based on prior test runs. A second activation attempt in web where user remained in portfolio page during the whole (3-5 minute) process was successful. Suggest:
I was unable to proceed with Desktop DEX tests due to unexpected geoblock on the build (via CI, though had same building local yesterday). 
Assumed due to missing |
|
I almost gave up on it, when a distant ship appeared on the horizon and made it into port. Linux Desktop Iguana Maker -> Brave Web Iguana Taker ✔️ Attempting to select as taker on desktop has led to "swap now" button spinning for a few minutes. Retried in a new session. After selecting KMD (sell) and ARRR (buy) matching my Web Maker, the following error was seen immediately after clicking the "Swap Now" button: |
…hing tx history of unactivated assets (#3162) * fix(wallet): add resilience to delisted coins for wallet coin metadata * fix(version-info): ensure that apiversion and commit has persist * perf(market-metrics): add backoff strategy for periodic updates TODO: ensure that the portfolio growth event is only fired once to avoid restarting the periodic updates on every user input (likely the case currently) * refactor(market-metrics): extract common coin filtering for test coin removal * refactor(market-metrics): extract calculation functions into extension * perf(coins-bloc): add initialisation status tracking for startup * refactor: add loop exits, update comments, remove unused event member
also cap to the available sell amount
ARRR should only appear in the wallet page once activation succeeds or fails (5 attempts with exponential backoff), so its appearance indicates a failure of the initial activation attempts. Possible activation race conditions further mitigated in 3537231.
API key can be sent in DM and added to the build command,
I pushed a fix for this alongside the additional text to the sync status banner in b85c889 and a82e022. DEX buy and sell coin selection and filtering are improved in 3537231. |
|
Confirmed no ARRR row in portfolio while activating. ✔️ When activation was at 97% I entered the DEX tab and was able to select ARRR from the sell list, before activation had completed. Upon returning to portfolio page, it was back down to 1%, then showed json error in red at bottom of activation status section, then status section disappeared, ARRR row appeared, I entered, and saw red. 
In same session, after manually "disabling" ARRR, activattion was re-attempted, with status oscillating between 1% and 9% before red json error reappeared - it's a "file already open" error - more detail in video. Seems its attempting to activate while an activation is already in progress. vokoscreenNG-2025-10-01_20-38-02.mp4 |
|
Linux Desktop testing on hold due to login issue reported in internal chat (affects all PRs) |
@smk762 does that mean that web is signed off, or at least that you've completed your full cycle of testing on it? |
Unresolved activation issues were still presenting regardless of platform at prior test. Videos in last comment show this in web. Some tests were conducted on desktop before the login issue presented, generally it performs better than web (faster to sync/generate etc). Retested most recent commits in web:
vokoscreenNG-2025-10-01_23-59-48.mp4Recommend:
Web has wasm related issue for withdrawals which might also be cause of swap fails. Linked fix did not resolve issue, but is KDF bug, not wallet. |
getWalletAssets was used to improve form validation speed to fix hitching and lengthy delays, but it is not correct and allowed for form submissions with inactive coins, resulting in unexpected error messages
Some minor UI overflows in mobile, will open separate issues for these. Also encountered freeze/crash on android mobile during swaps. If user relaunch/login fast enough, the swap will complete. Logs offer no insight, they just stop. |
Summary by CodeRabbit
New Features
UI
Bug Fixes
Chores