Prevent taking of offers with unequal bank account types (excl. SEPA) (…

…#3673)

* Use strict stubbing for ReceiptValidatorTest to avoid confusion

Remove redundant stubs from the MoneyGram and Western Union tests and
ensure that all such stubs result in failure. In particular, the 'offer'
mock is never accessed directly by ReceiptValidator.

* Prevent taking of offers with unequal bank account types

Use stricter criteria when deciding which of the taker's accounts (if
any) are valid for a given offer. Specifically, prevent National Bank
accounts from being used to take Same / Specific Bank(s) offers, so the
three payment method types can never being mixed.

This prevents an error on the trading peer when the trade starts, due to
enforcement of equal maker & taker payment method IDs (except for SEPA)
in the Contract payload constructor.

This partially addresses #3602, where the erroneous peer response causes
the taker to be presented with a confusing timeout.

core/src/main/java/bisq/core/payment/ReceiptValidator.java

@@ -66,10 +66,15 @@ boolean isValid() {
             return true;
         }
 
+        // Aside from Sepa or Sepa Instant, payment methods need to match
+        if (!isEqualPaymentMethods) {
+            return false;
+        }
+
         if (predicates.isOfferRequireSameOrSpecificBank(offer, account)) {
             return predicates.isMatchingBankId(offer, account);
         }
 
-        return isEqualPaymentMethods;
+        return true;
     }
 }

core/src/test/java/bisq/core/payment/ReceiptValidatorTest.java

@@ -18,17 +18,18 @@
 package bisq.core.payment;
 
 import bisq.core.offer.Offer;
-import bisq.core.payment.payload.PaymentMethod;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.junit.MockitoJUnitRunner;
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
 public class ReceiptValidatorTest {
     private ReceiptValidator validator;
     private PaymentAccount account;
@@ -43,6 +44,11 @@ public void setUp() {
         this.validator = new ReceiptValidator(offer, account, predicates);
     }
 
+    @After
+    public void tearDown() {
+        verifyZeroInteractions(offer);
+    }
+
     @Test
     public void testIsValidWhenCurrencyDoesNotMatch() {
         when(predicates.isMatchingCurrency(offer, account)).thenReturn(false);
@@ -169,12 +175,59 @@ public void testIsValidWhenNationalBankAccount() {
     }
 
     @Test
-    public void testIsValidWhenWesternUnionAccount() {
-        account = mock(WesternUnionAccount.class);
+    // Same or Specific Bank offers can't be taken by National Bank accounts. TODO: Consider partially relaxing to allow Specific Banks.
+    public void testIsValidWhenNationalBankAccountAndOfferIsNot() {
+        account = mock(NationalBankAccount.class);
+
+        when(predicates.isMatchingCurrency(offer, account)).thenReturn(true);
+        when(predicates.isEqualPaymentMethods(offer, account)).thenReturn(false);
+        when(predicates.isMatchingCountryCodes(offer, account)).thenReturn(true);
+        when(predicates.isMatchingSepaOffer(offer, account)).thenReturn(false);
+        when(predicates.isMatchingSepaInstant(offer, account)).thenReturn(false);
+
+        assertFalse(new ReceiptValidator(offer, account, predicates).isValid());
+
+        verify(predicates, never()).isOfferRequireSameOrSpecificBank(offer, account);
+        verify(predicates, never()).isMatchingBankId(offer, account);
+    }
+
+    @Test
+    // National or Same Bank offers can't be taken by Specific Banks accounts. TODO: Consider partially relaxing to allow National Bank.
+    public void testIsValidWhenSpecificBanksAccountAndOfferIsNot() {
+        account = mock(SpecificBanksAccount.class);
+
+        when(predicates.isMatchingCurrency(offer, account)).thenReturn(true);
+        when(predicates.isEqualPaymentMethods(offer, account)).thenReturn(false);
+        when(predicates.isMatchingCountryCodes(offer, account)).thenReturn(true);
+        when(predicates.isMatchingSepaOffer(offer, account)).thenReturn(false);
+        when(predicates.isMatchingSepaInstant(offer, account)).thenReturn(false);
+
+        assertFalse(new ReceiptValidator(offer, account, predicates).isValid());
 
-        PaymentMethod.WESTERN_UNION = mock(PaymentMethod.class);
+        verify(predicates, never()).isOfferRequireSameOrSpecificBank(offer, account);
+        verify(predicates, never()).isMatchingBankId(offer, account);
+    }
 
-        when(offer.getPaymentMethod()).thenReturn(PaymentMethod.WESTERN_UNION);
+    @Test
+    // National or Specific Bank offers can't be taken by Same Bank accounts.
+    public void testIsValidWhenSameBankAccountAndOfferIsNot() {
+        account = mock(SameBankAccount.class);
+
+        when(predicates.isMatchingCurrency(offer, account)).thenReturn(true);
+        when(predicates.isEqualPaymentMethods(offer, account)).thenReturn(false);
+        when(predicates.isMatchingCountryCodes(offer, account)).thenReturn(true);
+        when(predicates.isMatchingSepaOffer(offer, account)).thenReturn(false);
+        when(predicates.isMatchingSepaInstant(offer, account)).thenReturn(false);
+
+        assertFalse(new ReceiptValidator(offer, account, predicates).isValid());
+
+        verify(predicates, never()).isOfferRequireSameOrSpecificBank(offer, account);
+        verify(predicates, never()).isMatchingBankId(offer, account);
+    }
+
+    @Test
+    public void testIsValidWhenWesternUnionAccount() {
+        account = mock(WesternUnionAccount.class);
 
         when(predicates.isMatchingCurrency(offer, account)).thenReturn(true);
         when(predicates.isEqualPaymentMethods(offer, account)).thenReturn(true);
@@ -202,17 +255,14 @@ public void testIsValidWhenWesternIrregularAccount() {
     public void testIsValidWhenMoneyGramAccount() {
         account = mock(MoneyGramAccount.class);
 
-        PaymentMethod.MONEY_GRAM = mock(PaymentMethod.class);
-
-        when(offer.getPaymentMethod()).thenReturn(PaymentMethod.MONEY_GRAM);
-
         when(predicates.isMatchingCurrency(offer, account)).thenReturn(true);
         when(predicates.isEqualPaymentMethods(offer, account)).thenReturn(true);
-        when(predicates.isMatchingCountryCodes(offer, account)).thenReturn(false);
-        when(predicates.isMatchingSepaOffer(offer, account)).thenReturn(false);
-        when(predicates.isMatchingSepaInstant(offer, account)).thenReturn(false);
-        when(predicates.isOfferRequireSameOrSpecificBank(offer, account)).thenReturn(false);
 
         assertTrue(new ReceiptValidator(offer, account, predicates).isValid());
+
+        verify(predicates, never()).isMatchingCountryCodes(offer, account);
+        verify(predicates, never()).isMatchingSepaOffer(offer, account);
+        verify(predicates, never()).isMatchingSepaInstant(offer, account);
+        verify(predicates, never()).isOfferRequireSameOrSpecificBank(offer, account);
     }
 }