feat(geo): bouncer for privacy coin blocking#3150
Conversation
Co-authored-by: charl <charl@vanstaden.info>
|
Cursor Agent can help with this pull request. Just |
|
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. ✨ Finishing touches🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
Co-authored-by: charl <charl@vanstaden.info>
Co-authored-by: charl <charl@vanstaden.info>
CharlVS
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: charl <charl@vanstaden.info>
…es; remove field - Replace AppGeoStatus(tradingEnabled: ...) with disallowedFeatures-based construction - Remove tradingEnabled from TradingStatusLoadSuccess; compute isEnabled from disallowedFeatures - Update TradingStatusBloc emission accordingly - Set DisallowedFeature.trading in repository failure paths - Remove unused _computeTradingEnabled helper
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…g; use DisallowedFeature.trading\n\n- Add conservative guard to treat missing disallowed_features as trading blocked\n- Return AppGeoStatus with DisallowedFeature.trading when features are absent\n- Confirm no remaining references to deprecated 'blocked' field
There was a problem hiding this comment.
Pull Request Overview
This PR updates the geoblocking logic to use new disallowed_features and disallowed_assets fields from the bouncer endpoint instead of the legacy blocked flag. It adds functionality to include/exclude blocked assets in private key export and maintains backward compatibility.
Key changes:
- Refactored trading status system to use structured geo status with disallowed features and assets
- Added toggle for including blocked assets in private key export
- Updated all trading status checks to use new state structure
Reviewed Changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| lib/bloc/trading_status/trading_status_state.dart | Replaced legacy TradingEnabled/TradingDisabled states with TradingStatusLoadSuccess containing disallowed features and assets |
| lib/bloc/trading_status/trading_status_repository.dart | Complete rewrite to fetch structured geo status with disallowed features and assets parsing |
| lib/bloc/trading_status/trading_status_bloc.dart | Updated to emit new state structure with disallowed features and assets |
| lib/bloc/trading_status/disallowed_feature.dart | New enum for parsing disallowed features from API response |
| lib/views/settings/widgets/security_settings/private_key_settings/private_key_show.dart | Added toggle and filtering logic for blocked assets in private key export |
| lib/views/settings/widgets/security_settings/security_settings_page.dart | Integration to pass blocked asset symbols to private key export |
| Multiple UI files | Updated trading status checks from is TradingEnabled to .isEnabled property |
| lib/shared/constants.dart | Updated bouncer API URL to include version path |
| assets/translations/en.json | Added translation for blocked assets toggle |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…cer rules - Adjust clock warning banner behavior - Update security settings screens for key viewing flow - Align bridge and DEX confirmations/buttons with privacy coin restrictions - Update market maker bot forms and wallet coin details buttons These changes support privacy coin blocking bouncer logic and align UI flows accordingly.
…618 and resolve ui_text_form_field conflict
…Revert formatting-only changes and resolve conflicts by keeping current branch content where conflicts occurred.
…3153) * refactor(trading-status): split provider and repository * feat(trading-status): add watcher service and coins repo/bloc filtering * refactor(trading-status): improve status watch stream handling
…and resolve conflicts in coins bloc/repo; keep TradingStatusService filtering and SDK wallet coin IDs for prices/activation
|
Using Dubai VPN:
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The above still applies with the following modifications:
|
…tform/komodo-wallet into cursor/update-bouncer-for-privacy-coin-blocking-b618
- Fix private key export to filter blocked assets before fetching keys - Fix race condition in CoinsBloc by waiting for initial trading status - Filter blocked assets from default coins during wallet creation/restoration - Prevent blocked assets from being added to wallet metadata - Apply filtering in Trezor wallet setup flow This ensures geo-blocked privacy coins (LTC, LTC-segwit, FIRO) are completely excluded from: - Wallet metadata and config - Coin activation on login - Private key export - Pre-login coins display - All user-facing features Fixes issues reported in PR #3150
Document potential future improvements to reduce startup time: - Start with unrestricted state, apply filters when API responds - Populate coins immediately, filter reactively on status updates - Add all coins to metadata, remove blocked ones post-confirmation Trade-off: Faster startup (~100-500ms improvement) vs. brief exposure of potentially blocked assets during initial API call. These optimizations would require: - Reactive UI updates to handle dynamic asset removal - Coordinated metadata cleanup when trading status changes - Handling edge cases for manually re-added blocked coins
|
Visit the preview URL for this PR (updated for commit bf9ef01): https://walletrc--pull-3150-merge-v1zqkz4f.web.app (expires Tue, 07 Oct 2025 15:11:53 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The private key fetch operation was filtering out geo-blocked assets before they reached the PrivateKeyShow component, making the 'Include blocked assets' toggle ineffective. Now only NFT assets are filtered during fetch, allowing the UI component to control blocked asset visibility through the toggle.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Replace context.read with context.watch for TradingStatusBloc state access to enable automatic UI rebuilds when geo-blocking or other trading status restrictions change. This allows filtered asset and protocol lists to update dynamically without manual intervention.
- Wrapped build methods with BlocBuilder<TradingStatusBloc> in coins_table_content, orders_table_content, and coin_selection_and_amount_input - Fixed bug where filtered coin/order lists didn't update when geo-blocking status changed - Added comprehensive documentation of BLoC anti-patterns in table_utils - Documented tech debt: utility functions using context.read in build methods - Recommended refactor: move filtering logic into respective Blocs following SOLID principles The issue was caused by using context.read<TradingStatusBloc>() which doesn't establish a subscription to state changes. Now widgets properly rebuild when trading status updates.
The FEEDBACK_API_KEY was only being passed to builds when both Trello credentials and Cloudflare credentials were present. This caused CI builds to disable trading when Trello credentials were missing, even if the FEEDBACK_API_KEY was available. Changed the condition to only require FEEDBACK_API_KEY and FEEDBACK_PRODUCTION_URL to be present, independent of Trello configuration. This allows CI builds to properly enable trading based on geo-status checks when the feedback API credentials are configured. Updated both the GitHub Actions workflow and Docker build script to use the corrected logic.
…tion" This reverts commit 59afc91.
Previously, canTradeAssets treated null asset IDs as blocked, causing trading to incorrectly show as disabled when coins weren't selected yet in forms (taker, maker, bridge, market maker bot). The fix filters out null values using whereType<AssetId>() before checking if assets are blocked, so trading status is only determined by the general trading feature flag and actually selected assets. Also fixed props getter to include isEnabled for proper state comparison.
smk762
left a comment
smk762
left a comment
There was a problem hiding this comment.
Filtering applied as expected in blocked region.
Unable to test in desktop due to login issue reported in internal chat, but as that issue affects all current PRs, it is not isolated to this one and outside scope..
CharlVS
deleted the
cursor/update-bouncer-for-privacy-coin-blocking-b618
branch
Update the geoblocking logic to utilise
disallowed_featuresanddisallowed_assetsfrom the bouncer endpoint, and add a toggle to include blocked assets in private key exports.The bouncer endpoint now provides
disallowed_featuresanddisallowed_assetsto explicitly control trading and asset access by region. This PR updates the application to accommodate the new response structure, ensuring trading is blocked based onTRADINGindisallowed_features, and allows users to export private keys fordisallowed_assets. It also maintains backward compatibility with theblockedflag and respects theGEO_BLOCK=disabledenvironment variable.Testing
Note
Adopts new bouncer disallowed_features/assets to drive trading/asset geo-blocking via a TradingStatusService, filters blocked assets across app flows, and adds a toggle to include blocked assets in private key exports.
TradingStatusService,TradingStatusApiProvider,AppGeoStatus, andDisallowedFeatureto consume bouncerdisallowed_features/disallowed_assetswith polling and backoff.TradingStatusRepositoryto parse new API, support streaming watch, and honorGEO_BLOCK=disabled.TradingStatusBlocwith load-success state, per-asset checks, and a watch event.TradingStatusServiceinmain.dart; inject intoCoinsRepo,CoinsBloc,AuthBloc.CoinsBloc/Repo: wait for initial status and filter blocked assets in known coins, activation, balances, price fetches.AuthBloc/Trezor flows: filter default/restored coins to exclude blocked assets before adding to metadata.state.isEnabledorcanTradeAssets(...)in DEX, Bridge, MMBot, menus, wallet pages, and clock banner; filter lists (coins, orders, tickers) accordingly.geoBlockerApiUrlto/v1/endpoint.includeBlockedAssetsi18n key and generated code.Written by Cursor Bugbot for commit bf9ef01. This will update automatically on new commits. Configure here.