-
Notifications
You must be signed in to change notification settings - Fork 0
Geoblock improvements #233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
CJIT is not blocked, fixing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a centralized geoblocking management system through a new GeoService singleton, replacing the previous scattered isGeoBlocked property in AppViewModel. The implementation adds support for disabling geoblocking via the GEO environment variable and enforces geoblocking restrictions by filtering out Blocktank LSP channels when users are in restricted regions, while still allowing Lightning operations through external peer channels.
Key changes:
- Created
GeoServiceas single source of truth for geoblocking state - Added
GEOenvironment variable support with compilation-time and runtime configuration - Implemented LSP channel filtering throughout the app to restrict Blocktank usage when geoblocked
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Bitkit/Services/GeoService.swift | New singleton service managing geoblocking state with async status checking |
| Bitkit/Services/CoreService.swift | Added environment variable check to bypass geo API when GEO=false |
| Bitkit/Constants/Env.swift | Added isGeoblockingEnabled configuration with compilation and runtime support |
| Bitkit/ViewModels/AppViewModel.swift | Removed isGeoBlocked property, delegated to GeoService |
| Bitkit/ViewModels/WalletViewModel.swift | Added methods to calculate non-LSP inbound capacity and check for non-LSP channels |
| Bitkit/Services/LightningService.swift | Added LSP peer filtering and external peer validation for geoblocked users |
| Bitkit/Services/TransferService.swift | Added validation to block LSP transfers when geoblocked |
| Bitkit/Views/Wallets/SavingsWalletView.swift | Updated to use GeoService for transfer button visibility |
| Bitkit/Views/Wallets/Receive/ReceiveQr.swift | Implemented LSP channel filtering for tab visibility and CJIT onboarding |
| Bitkit/Views/Wallets/Receive/ReceiveEdit.swift | Updated inbound capacity calculation to exclude LSP channels when geoblocked |
| Bitkit/Views/Transfer/FundingOptions.swift | Replaced AppViewModel geo checks with GeoService |
| Bitkit/Views/Onboarding/OnboardingSlider.swift | Updated disclaimer text to use GeoService |
| Bitkit.xcodeproj/project.pbxproj | Added GeoService to build targets and configured GEO compilation flag |
| .github/workflows/e2e-tests.yml | Set GEO=false for E2E tests to bypass geoblocking |
|
Drafted to investigate CI |
…to the compilation conditions instead of overriding them
|
@ovitrif @ben-kaufman Ci Ready |
The geoblocked screen apparently was replaced by this error toss on the latest design I'll add a refresh on geoblock method |
|
@ben-kaufman fixed |
ovitrif
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
Tested:
- No Lightning channels
- Saving -> Transfer to spending button not visible 🟢
- Qr sheet -> Spending balance -> click on receive on spending balance button -> display a toast error 🟢
- Only Blocktant channels
- Saving -> Transfer to spending button not visible 🟢
- Qr sheet -> Should not display Unified QR 🟢
- Qr sheet -> Spending balance -> click on receive on spending balance button -> display a toast error 🟢
- Scan lightning QR code -> should not be able to send
todo
- Blocktank + external peer channels
- Saving -> Transfer to spending button not visible
todo - Qr sheet -> Should display Unified QR
todo - Qr sheet -> Spending balance -> Should display qr
todo - Scan Lightning QR code -> should be able to send according the outbound with the external node
todo
|
|
||
| private var hasUsableChannels: Bool { | ||
| if GeoService.shared.isGeoBlocked { | ||
| return wallet.hasNonLspChannels() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really a requirement? If the user purchased a channel (when not geo-blocked) do we have to block usage of it when they move to geo-blocked area? Current Bitkit doesn't do this I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how it works in bitkit-android AFAICR.
If the user purchased a channel (when not geo-blocked) do we have to block usage of it when they move to geo-blocked area?
I'm not sure we have to block in this specific situation, you're raising a fair point. But then again, I guess it's more fragile legally than simply blocking everything. And judging by our approach to legal, by using Occam's razor I would favor to just block it for safety reason, better to err. on this side.
But if we agree to NOT block, then I think it's best if we move this logic to bitkit-core and then both apps can use the same logic.
| func tabContent(for tab: ReceiveTab) -> some View { | ||
| VStack(spacing: 0) { | ||
| if tab == .spending && wallet.channelCount == 0 && cjitInvoice == nil { | ||
| if showingCjitOnboarding { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this (or something here) is breaking the tab transition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc. @jvsena42


Description
This PR adds setup to disable geoblocking checks entirely via GEO bool environment variable with value true or false.
Also fixes some flows when geoblocked
When using GEO=false, geoblocking checks via API are bypassed.
Defaults to true, preserving current default functionality.
Linked Issues/Tasks
Closes #182
Figma
Screenshot / Video
Test cases:
isGeoblockingEnabled = falsemakes the app ignore geoblock checkNo Lightning channels
no-ln-channels.mp4
Only Blocktant channels
only-lsp-channels.mp4
Blocktank + external peer channels
lsp-and-external.mp4
onboarding.mp4