-
Notifications
You must be signed in to change notification settings - Fork 1
Geoblock Check #142
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
Geoblock Check #142
Conversation
…ouldBlockLightning is true
…lockLightning is true
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.
Overall everything looks great!
Though it looks to me the logic to determine if we should block lightning needs to compare currently connected peers vs blocktank trusted peers (see comment)
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.
Thanks for updating the logic in CoreService 👍🏻
I tried testing this for the basic and easiest test: using USA IP address with VPN.
Test Scenario: Geoblock USA
- Uninstall app
- Connect via VPN to USA IP
- Open app > create new wallet
- Go to Receive > tap
Receive on Spending Balance - 🔴 expected: see geoblock UI
actual: I see amount UI
I looked at the implementation and it seems it currently applies only when the user already has a LN channel, which is not necessarily the CJIT flow.
Test Preview:
cjitNotGeoBlocked.mp4
I dug a bit in the code and noticed that we don't currently check the explicit CJIT case:

|
Thanks! I forgot this flow |
|
CJIT test added to description |
app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveQrScreen.kt
Outdated
Show resolved
Hide resolved
|
Sorry again, fixed the navigation |
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.
Great end result 🚀 , thanks a lot for addressing all comments and shaping it up to this state 🙌🏻
Gave testing one more spin:
- 🟢 CJIT (New wallet on USA IP) - geoblock UI shown
- 🟢 Existing wallet with LN (on USA IP) - geoblock UI shown
Ready to merge 🎉
FIGMA
Related to #50
geo_block.mp4
cjit_geo_block.mp4