Improve payment withdrawals.

app/build.gradle

@@ -212,7 +212,6 @@ android {
         buildConfigField "String[]", "LANGUAGES", "new String[]{\"" + autoResConfig().collect { s -> s.replace('-r', '_') }.join('", "') + '"}'
         buildConfigField "int", "CANONICAL_VERSION_CODE", "$canonicalVersionCode"
         buildConfigField "String", "DEFAULT_CURRENCIES", "\"EUR,AUD,GBP,CAD,CNY\""
-        buildConfigField "int[]", "MOBILE_COIN_BLACKLIST", "new int[]{98,963,53,850,7}"
         buildConfigField "String", "GIPHY_API_KEY", "\"3o6ZsYH6U6Eri53TXy\""
         buildConfigField "String", "SIGNAL_CAPTCHA_URL", "\"https://signalcaptchas.org/registration/generate.html\""
         buildConfigField "String", "RECAPTCHA_PROOF_URL", "\"https://signalcaptchas.org/challenge/generate.html\""

app/src/main/java/org/thoughtcrime/securesms/keyvalue/AccountValues.kt

@@ -30,7 +30,6 @@ internal class AccountValues internal constructor(store: KeyValueStore) : Signal
   companion object {
     private val TAG = Log.tag(AccountValues::class.java)
     private const val KEY_SERVICE_PASSWORD = "account.service_password"
-    private const val KEY_IS_REGISTERED = "account.is_registered"
     private const val KEY_REGISTRATION_ID = "account.registration_id"
     private const val KEY_FCM_ENABLED = "account.fcm_enabled"
     private const val KEY_FCM_TOKEN = "account.fcm_token"
@@ -61,6 +60,8 @@ internal class AccountValues internal constructor(store: KeyValueStore) : Signal
     const val KEY_ACI = "account.aci"
     @VisibleForTesting
     const val KEY_PNI = "account.pni"
+    @VisibleForTesting
+    const val KEY_IS_REGISTERED = "account.is_registered"
   }
 
   init {

app/src/main/java/org/thoughtcrime/securesms/keyvalue/PaymentsValues.kt

@@ -1,5 +1,6 @@
 package org.thoughtcrime.securesms.keyvalue
 
+import androidx.annotation.VisibleForTesting
 import androidx.annotation.WorkerThread
 import androidx.lifecycle.LiveData
 import androidx.lifecycle.MutableLiveData
@@ -36,7 +37,6 @@ internal class PaymentsValues internal constructor(store: KeyValueStore) : Signa
     private val TAG = Log.tag(PaymentsValues::class.java)
 
     private const val PAYMENTS_ENTROPY = "payments_entropy"
-    private const val MOB_PAYMENTS_ENABLED = "mob_payments_enabled"
     private const val MOB_LEDGER = "mob_ledger"
     private const val PAYMENTS_CURRENT_CURRENCY = "payments_current_currency"
     private const val DEFAULT_CURRENCY_CODE = "GBP"
@@ -48,6 +48,9 @@ internal class PaymentsValues internal constructor(store: KeyValueStore) : Signa
     private const val SHOW_UPDATE_PIN_INFO_CARD = "mob_payments_show_update_pin_info_card"
 
     private val LARGE_BALANCE_THRESHOLD = Money.mobileCoin(BigDecimal.valueOf(500))
+
+    @VisibleForTesting
+    const val MOB_PAYMENTS_ENABLED = "mob_payments_enabled"
   }
 
   private val liveCurrentCurrency: MutableLiveData<Currency> by lazy { MutableLiveData(currentCurrency()) }
@@ -92,16 +95,20 @@ internal class PaymentsValues internal constructor(store: KeyValueStore) : Signa
    */
   val paymentsAvailability: PaymentsAvailability
     get() {
-      if (!SignalStore.account().isRegistered ||
-        !GeographicalRestrictions.e164Allowed(Recipient.self().requireE164())
-      ) {
+      if (!SignalStore.account().isRegistered) {
         return PaymentsAvailability.NOT_IN_REGION
       }
       return if (FeatureFlags.payments()) {
         if (mobileCoinPaymentsEnabled()) {
-          PaymentsAvailability.WITHDRAW_AND_SEND
-        } else {
+          if (GeographicalRestrictions.e164Allowed(SignalStore.account().e164)) {
+            PaymentsAvailability.WITHDRAW_AND_SEND
+          } else {
+            return PaymentsAvailability.WITHDRAW_ONLY
+          }
+        } else if (GeographicalRestrictions.e164Allowed(SignalStore.account().e164)) {
           PaymentsAvailability.REGISTRATION_AVAILABLE
+        } else {
+          PaymentsAvailability.NOT_IN_REGION
         }
       } else {
         if (mobileCoinPaymentsEnabled()) {

app/src/main/java/org/thoughtcrime/securesms/payments/GeographicalRestrictions.java

@@ -1,16 +1,17 @@
 package org.thoughtcrime.securesms.payments;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
-import com.google.i18n.phonenumbers.NumberParseException;
-import com.google.i18n.phonenumbers.PhoneNumberUtil;
-
 import org.signal.core.util.logging.Log;
-import org.thoughtcrime.securesms.BuildConfig;
+import org.thoughtcrime.securesms.util.FeatureFlags;
+import org.thoughtcrime.securesms.util.Util;
 
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.stream.Collectors;
 
 public final class GeographicalRestrictions {
 
@@ -18,32 +19,23 @@ public final class GeographicalRestrictions {
 
   private GeographicalRestrictions() {}
 
-  private static final Set<Integer> BLACKLIST;
-
-  static {
-    Set<Integer> set = new HashSet<>(BuildConfig.MOBILE_COIN_BLACKLIST.length);
-
-    for (int i = 0; i < BuildConfig.MOBILE_COIN_BLACKLIST.length; i++) {
-      set.add(BuildConfig.MOBILE_COIN_BLACKLIST[i]);
-    }
-
-    BLACKLIST = Collections.unmodifiableSet(set);
-  }
-
-  public static boolean regionAllowed(int regionCode) {
-    return !BLACKLIST.contains(regionCode);
-  }
-
   public static boolean e164Allowed(@Nullable String e164) {
-    try {
-      int countryCode = PhoneNumberUtil.getInstance()
-                                       .parse(e164, null)
-                                       .getCountryCode();
-
-      return GeographicalRestrictions.regionAllowed(countryCode);
-    } catch (NumberParseException e) {
-      Log.w(TAG, e);
+    if (e164 == null) {
       return false;
     }
+
+    String bareE164 = e164.startsWith("+") ? e164.substring(1) : e164;
+
+    return parsePrefixes(FeatureFlags.paymentsCountryBlocklist())
+        .stream()
+        .noneMatch(bareE164::startsWith);
+  }
+
+  private static List<String> parsePrefixes(@NonNull String serializedList) {
+    return Arrays.stream(serializedList.split(","))
+        .map(v -> v.replaceAll(" ", ""))
+        .map(String::trim)
+        .filter(v -> !Util.isEmpty(v))
+        .collect(Collectors.toList());
   }
 }

app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java

@@ -3,7 +3,6 @@ package org.thoughtcrime.securesms.util;
 import android.text.TextUtils;
 
 import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 import androidx.annotation.WorkerThread;
 
@@ -18,7 +17,6 @@ import org.thoughtcrime.securesms.groups.SelectionLimits;
 import org.thoughtcrime.securesms.jobs.RefreshAttributesJob;
 import org.thoughtcrime.securesms.jobs.RemoteConfigRefreshJob;
 import org.thoughtcrime.securesms.keyvalue.SignalStore;
-import org.thoughtcrime.securesms.keyvalue.StoryValues;
 import org.thoughtcrime.securesms.messageprocessingalarm.MessageProcessReceiver;
 
 import java.io.IOException;
@@ -95,6 +93,7 @@ public final class FeatureFlags {
   private static final String SOFTWARE_AEC_BLOCKLIST_MODELS     = "android.calling.softwareAecBlockList";
   private static final String USE_HARDWARE_AEC_IF_OLD           = "android.calling.useHardwareAecIfOlderThanApi29";
   private static final String USE_AEC3                          = "android.calling.useAec3";
+  private static final String PAYMENTS_COUNTRY_BLOCKLIST        = "android.payments.blocklist";
 
   /**
    * We will only store remote values for flags in this set. If you want a flag to be controllable
@@ -141,7 +140,8 @@ public final class FeatureFlags {
       HARDWARE_AEC_BLOCKLIST_MODELS,
       SOFTWARE_AEC_BLOCKLIST_MODELS,
       USE_HARDWARE_AEC_IF_OLD,
-      USE_AEC3
+      USE_AEC3,
+      PAYMENTS_COUNTRY_BLOCKLIST
   );
 
   @VisibleForTesting
@@ -199,7 +199,8 @@ public final class FeatureFlags {
       HARDWARE_AEC_BLOCKLIST_MODELS,
       SOFTWARE_AEC_BLOCKLIST_MODELS,
       USE_HARDWARE_AEC_IF_OLD,
-      USE_AEC3
+      USE_AEC3,
+      PAYMENTS_COUNTRY_BLOCKLIST
   );
 
   /**
@@ -418,6 +419,11 @@ public final class FeatureFlags {
     return getBoolean(GROUP_CALL_RINGING, false);
   }
 
+  /** A comma-separated list of country codes where payments should be disabled. */
+  public static String paymentsCountryBlocklist() {
+    return getString(PAYMENTS_COUNTRY_BLOCKLIST, "98,963,53,850,7");
+  }
+
   /**
    * Whether or not to show donor badges in the UI.
    */

app/src/test/java/org/thoughtcrime/securesms/keyvalue/PaymentsValuesTest.kt

@@ -0,0 +1,158 @@
+package org.thoughtcrime.securesms.keyvalue
+
+import android.app.Application
+import androidx.test.core.app.ApplicationProvider
+import org.junit.Assert.assertEquals
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.powermock.api.mockito.PowerMockito
+import org.powermock.core.classloader.annotations.PowerMockIgnore
+import org.powermock.core.classloader.annotations.PrepareForTest
+import org.powermock.modules.junit4.rule.PowerMockRule
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+import org.thoughtcrime.securesms.dependencies.ApplicationDependencies
+import org.thoughtcrime.securesms.dependencies.MockApplicationDependencyProvider
+import org.thoughtcrime.securesms.util.FeatureFlags
+
+@RunWith(RobolectricTestRunner::class)
+@Config(manifest = Config.NONE, application = Application::class)
+@PowerMockIgnore("org.mockito.*", "org.robolectric.*", "android.*", "androidx.*", "org.powermock.*")
+@PrepareForTest(FeatureFlags::class)
+class PaymentsValuesTest {
+
+  @get:Rule
+  val powerMockRule = PowerMockRule()
+
+  @Before
+  fun setup() {
+    if (!ApplicationDependencies.isInitialized()) {
+      ApplicationDependencies.init(ApplicationProvider.getApplicationContext(), MockApplicationDependencyProvider())
+    }
+
+    PowerMockito.mockStatic(FeatureFlags::class.java)
+  }
+
+  @Test
+  fun `when unregistered, expect NOT_IN_REGION`() {
+    setupStore(
+      KeyValueDataSet().apply {
+        putBoolean(AccountValues.KEY_IS_REGISTERED, false)
+      }
+    )
+
+    assertEquals(PaymentsAvailability.NOT_IN_REGION, SignalStore.paymentsValues().paymentsAvailability)
+  }
+
+  @Test
+  fun `when flag disabled and no account, expect DISABLED_REMOTELY`() {
+    setupStore(
+      KeyValueDataSet().apply {
+        putBoolean(AccountValues.KEY_IS_REGISTERED, true)
+        putString(AccountValues.KEY_E164, "+15551234567")
+        putBoolean(PaymentsValues.MOB_PAYMENTS_ENABLED, false)
+      }
+    )
+
+    PowerMockito.`when`(FeatureFlags.payments()).thenReturn(false)
+    PowerMockito.`when`(FeatureFlags.paymentsCountryBlocklist()).thenReturn("")
+
+    assertEquals(PaymentsAvailability.DISABLED_REMOTELY, SignalStore.paymentsValues().paymentsAvailability)
+  }
+
+  @Test
+  fun `when flag disabled but has account, expect WITHDRAW_ONLY`() {
+    setupStore(
+      KeyValueDataSet().apply {
+        putBoolean(AccountValues.KEY_IS_REGISTERED, true)
+        putString(AccountValues.KEY_E164, "+15551234567")
+        putBoolean(PaymentsValues.MOB_PAYMENTS_ENABLED, true)
+      }
+    )
+
+    PowerMockito.`when`(FeatureFlags.payments()).thenReturn(false)
+    PowerMockito.`when`(FeatureFlags.paymentsCountryBlocklist()).thenReturn("")
+
+    assertEquals(PaymentsAvailability.WITHDRAW_ONLY, SignalStore.paymentsValues().paymentsAvailability)
+  }
+
+  @Test
+  fun `when flag enabled and no account, expect REGISTRATION_AVAILABLE`() {
+    setupStore(
+      KeyValueDataSet().apply {
+        putBoolean(AccountValues.KEY_IS_REGISTERED, true)
+        putString(AccountValues.KEY_E164, "+15551234567")
+        putBoolean(PaymentsValues.MOB_PAYMENTS_ENABLED, false)
+      }
+    )
+
+    PowerMockito.`when`(FeatureFlags.payments()).thenReturn(true)
+    PowerMockito.`when`(FeatureFlags.paymentsCountryBlocklist()).thenReturn("")
+
+    assertEquals(PaymentsAvailability.REGISTRATION_AVAILABLE, SignalStore.paymentsValues().paymentsAvailability)
+  }
+
+  @Test
+  fun `when flag enabled and has account, expect WITHDRAW_AND_SEND`() {
+    setupStore(
+      KeyValueDataSet().apply {
+        putBoolean(AccountValues.KEY_IS_REGISTERED, true)
+        putString(AccountValues.KEY_E164, "+15551234567")
+        putBoolean(PaymentsValues.MOB_PAYMENTS_ENABLED, true)
+      }
+    )
+
+    PowerMockito.`when`(FeatureFlags.payments()).thenReturn(true)
+    PowerMockito.`when`(FeatureFlags.paymentsCountryBlocklist()).thenReturn("")
+
+    assertEquals(PaymentsAvailability.WITHDRAW_AND_SEND, SignalStore.paymentsValues().paymentsAvailability)
+  }
+
+  @Test
+  fun `when flag enabled and no account and in the country blocklist, expect NOT_IN_REGION`() {
+    setupStore(
+      KeyValueDataSet().apply {
+        putBoolean(AccountValues.KEY_IS_REGISTERED, true)
+        putString(AccountValues.KEY_E164, "+15551234567")
+        putBoolean(PaymentsValues.MOB_PAYMENTS_ENABLED, false)
+      }
+    )
+
+    PowerMockito.`when`(FeatureFlags.payments()).thenReturn(true)
+    PowerMockito.`when`(FeatureFlags.paymentsCountryBlocklist()).thenReturn("1")
+
+    assertEquals(PaymentsAvailability.NOT_IN_REGION, SignalStore.paymentsValues().paymentsAvailability)
+  }
+
+  @Test
+  fun `when flag enabled and has account and in the country blocklist, expect WITHDRAW_ONLY`() {
+    setupStore(
+      KeyValueDataSet().apply {
+        putBoolean(AccountValues.KEY_IS_REGISTERED, true)
+        putString(AccountValues.KEY_E164, "+15551234567")
+        putBoolean(PaymentsValues.MOB_PAYMENTS_ENABLED, true)
+      }
+    )
+
+    PowerMockito.`when`(FeatureFlags.payments()).thenReturn(true)
+    PowerMockito.`when`(FeatureFlags.paymentsCountryBlocklist()).thenReturn("1")
+
+    assertEquals(PaymentsAvailability.WITHDRAW_ONLY, SignalStore.paymentsValues().paymentsAvailability)
+  }
+
+  /**
+   * Account values will overwrite some values upon first access, so this takes care of that
+   */
+  private fun setupStore(dataset: KeyValueDataSet) {
+    val store = KeyValueStore(
+      MockKeyValuePersistentStorage.withDataSet(
+        dataset.apply {
+          putString(AccountValues.KEY_ACI, "")
+        }
+      )
+    )
+    SignalStore.inject(store)
+  }
+}

app/src/test/java/org/thoughtcrime/securesms/payments/GeographicalRestrictionsTest.java

@@ -2,46 +2,52 @@ package org.thoughtcrime.securesms.payments;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 import org.signal.core.util.logging.Log;
 import org.thoughtcrime.securesms.BuildConfig;
 import org.thoughtcrime.securesms.testutil.EmptyLogger;
+import org.thoughtcrime.securesms.util.FeatureFlags;
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeFalse;
 import static org.junit.Assume.assumeTrue;
 
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(FeatureFlags.class)
 public final class GeographicalRestrictionsTest {
 
   @Before
   public void setup() {
     Log.initialize(new EmptyLogger());
+    PowerMockito.mockStatic(FeatureFlags.class);
   }
 
   @Test
-  public void bad_number_not_allowed() {
-    assertFalse(GeographicalRestrictions.e164Allowed("bad_number"));
+  public void e164Allowed_general() {
+    PowerMockito.when(FeatureFlags.paymentsCountryBlocklist()).thenReturn("");
+    assertTrue(GeographicalRestrictions.e164Allowed("+15551234567"));
+
+    PowerMockito.when(FeatureFlags.paymentsCountryBlocklist()).thenReturn("1");
+    assertFalse(GeographicalRestrictions.e164Allowed("+15551234567"));
+
+    PowerMockito.when(FeatureFlags.paymentsCountryBlocklist()).thenReturn("1,44");
+    assertFalse(GeographicalRestrictions.e164Allowed("+15551234567"));
+    assertFalse(GeographicalRestrictions.e164Allowed("+445551234567"));
+    assertTrue(GeographicalRestrictions.e164Allowed("+525551234567"));
+
+    PowerMockito.when(FeatureFlags.paymentsCountryBlocklist()).thenReturn("1 234,44");
+    assertFalse(GeographicalRestrictions.e164Allowed("+12341234567"));
+    assertTrue(GeographicalRestrictions.e164Allowed("+15551234567"));
+    assertTrue(GeographicalRestrictions.e164Allowed("+525551234567"));
+    assertTrue(GeographicalRestrictions.e164Allowed("+2345551234567"));
   }
 
   @Test
-  public void null_not_allowed() {
+  public void e164Allowed_nullNotAllowed() {
     assertFalse(GeographicalRestrictions.e164Allowed(null));
   }
-
-  @Test
-  public void uk_allowed() {
-    assertTrue(GeographicalRestrictions.e164Allowed("+441617151234"));
-  }
-
-  @Test
-  public void crimea_not_allowed() {
-    assertFalse(GeographicalRestrictions.e164Allowed("+79782222222"));
-  }
-
-  @Test
-  public void blacklist_not_allowed() {
-    for (int code : BuildConfig.MOBILE_COIN_BLACKLIST) {
-      assertFalse(GeographicalRestrictions.regionAllowed(code));
-    }
-  }
 }