[bisq-network/bisq] Speed up deposit and transactions view loads (#5120)

[chimp1984]

@chimp1984 requested changes on this pull request.

I think the getTransactionConfidence method is not correct. Otherwise all looks good. Was not aware of the guava Multimap framework...Could be used in many places...

> @@ -408,18 +430,15 @@ public TransactionConfidence getConfidenceForTxId(String txId) {
         return null;
     }
 
-    protected TransactionConfidence getTransactionConfidence(Transaction tx, Address address) {
-        List<TransactionConfidence> transactionConfidenceList = getOutputsWithConnectedOutputs(tx)
-                .stream()
-                .filter(WalletService::isOutputScriptConvertibleToAddress)
-                .filter(output -> address != null && address.equals(getAddressFromOutput(output)))
-                .map(o -> tx.getConfidence())
-                .collect(Collectors.toList());
-        return getMostRecentConfidence(transactionConfidenceList);
+    private TransactionConfidence getTransactionConfidence(Transaction tx, Address address) {

Can we add a @Nullable annotation (was before as well nullable as getMostRecentConfidence is nullable).

> @@ -408,18 +430,15 @@ public TransactionConfidence getConfidenceForTxId(String txId) {
         return null;
     }
 
-    protected TransactionConfidence getTransactionConfidence(Transaction tx, Address address) {
-        List<TransactionConfidence> transactionConfidenceList = getOutputsWithConnectedOutputs(tx)
-                .stream()
-                .filter(WalletService::isOutputScriptConvertibleToAddress)
-                .filter(output -> address != null && address.equals(getAddressFromOutput(output)))
-                .map(o -> tx.getConfidence())
-                .collect(Collectors.toList());
-        return getMostRecentConfidence(transactionConfidenceList);
+    private TransactionConfidence getTransactionConfidence(Transaction tx, Address address) {
+        boolean matchesAddress = getOutputsWithConnectedOutputs(tx).stream()
+                .anyMatch(output -> address != null && address.equals(getAddressFromOutput(output)));
+
+        return matchesAddress ? getMostRecentConfidence(List.of(tx.getConfidence())) : null;

I think we have an issue here:
Assuming there was the same address used as connected output of an input and as the output of the given tx.
We had then 2 different txs in the previous version and the getMostRecentConfidence took the most recent one (the outputs as the connected output of the input could not be newer). Now we drop any of the 2 txs by `anyMatch` and use only the one of the tx (tx output). This would be correct in case we had 2 addresses connected to that tx.
But if the address was only used in a connected output of the input we would take the wrong txConfidence as we would take the actual tx and not the tx associated with the connected output of the input.

-- 
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/5120#pullrequestreview-578626998
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20210128/1e0d383e/attachment.htm>