[bisq-network/bisq] PaymentAccount object removal from ObservableSet issue (#3572)

[Julian Knutsen]

This has nothing to do with implementing the hashCode correctly.

This brings up one of the dangers of modifying elements while they are inside Hash containers. The hash bucket for an item is computed at insert time by calling item.hashCode().

But, if the item is later changed such that hashCode() returns a different value, the remove will look for the item in the bucket specified by the hashCode() return value at removal time. This explains why just moving them all to a map and deleting also worked. Also, why after restart it works.

The "fix" is to ensure the hashCode is stable before inserting it into the HashSet. The attached patch works, but I would recommend testing it more or auditing the other users that do something similar. For example, the code is almost duplicated in AltCoinAccountsDataModel.

![revolut-add](https://user-images.githubusercontent.com/8082291/68917075-8381f680-071d-11ea-921a-4f08348c522e.JPG)
![revolut-remove](https://user-images.githubusercontent.com/8082291/68917079-85e45080-071d-11ea-996b-254c540857be.JPG)

```
diff --git a/desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsDataModel.java b/desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsDataModel.java
index 568b3d8c2..ed281def8 100644
--- a/desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsDataModel.java
+++ b/desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsDataModel.java
@@ -106,7 +106,6 @@ class FiatAccountsDataModel extends ActivatableDataModel {
     ///////////////////////////////////////////////////////////////////////////////////////////
 
     public void onSaveNewAccount(PaymentAccount paymentAccount) {
-        user.addPaymentAccount(paymentAccount);
         TradeCurrency singleTradeCurrency = paymentAccount.getSingleTradeCurrency();
         List<TradeCurrency> tradeCurrencies = paymentAccount.getTradeCurrencies();
         if (singleTradeCurrency != null) {
@@ -128,6 +127,8 @@ class FiatAccountsDataModel extends ActivatableDataModel {
             });
         }
 
+        user.addPaymentAccount(paymentAccount);
+
         accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload());
     }
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3572#issuecomment-554208750
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191114/c1e50ec6/attachment.html>