From 88dd92457740effeac848b3602f288adc728af50 Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Thu, 9 Feb 2023 14:34:18 +0800 Subject: [PATCH 01/10] Clean up TransactionAwareTrade & speed it up slightly Replace the "Optional.ofNullable(...)..." constructs with more direct code using short-circuit operators, as this is shorter and a little faster. Also use "trade.get[Deposit|Payout]TxId()" instead of the code "trade.get[Deposit|Payout]TxId().getTxId()", as (upon inspection of the code) there should never be a case where the deposit/payout transaction field of a Trade object is set but the respective txID field is null (or set to an inconsistent value). Also remove a redundant 'RefundManager.getDisputesAsObservableList' method call, which was also slowing things down slightly. The minor speedups afforded by the above are important because the method 'TransactionAwareTrade.isRelatedToTransaction' is called a quadratic number of times and consequently a major bottleneck when loading the Transactions view. --- .../transactions/TransactionAwareTrade.java | 29 ++++++------------- .../TransactionAwareTradeTest.java | 4 +-- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java index d940ad5df7c..cc449231001 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java @@ -37,8 +37,6 @@ import javafx.collections.ObservableList; -import java.util.Optional; - import lombok.extern.slf4j.Slf4j; import static com.google.common.base.Preconditions.checkNotNull; @@ -73,8 +71,8 @@ public boolean isRelatedToTransaction(Transaction transaction) { Trade trade = (Trade) tradeModel; boolean isTakerOfferFeeTx = txId.equals(trade.getTakerFeeTxId()); boolean isOfferFeeTx = isOfferFeeTx(txId); - boolean isDepositTx = isDepositTx(hash); - boolean isPayoutTx = isPayoutTx(hash); + boolean isDepositTx = isDepositTx(txId); + boolean isPayoutTx = isPayoutTx(txId); boolean isDisputedPayoutTx = isDisputedPayoutTx(txId); boolean isDelayedPayoutTx = transaction.getLockTime() != 0 && isDelayedPayoutTx(txId); boolean isRefundPayoutTx = isRefundPayoutTx(trade, txId); @@ -91,36 +89,28 @@ public boolean isRelatedToTransaction(Transaction transaction) { return tradeRelated || isBsqSwapTrade; } - private boolean isPayoutTx(Sha256Hash txId) { + private boolean isPayoutTx(String txId) { if (isBsqSwapTrade()) return false; Trade trade = (Trade) tradeModel; - return Optional.ofNullable(trade.getPayoutTx()) - .map(Transaction::getTxId) - .map(hash -> hash.equals(txId)) - .orElse(false); + return txId.equals(trade.getPayoutTxId()); } - private boolean isDepositTx(Sha256Hash txId) { + private boolean isDepositTx(String txId) { if (isBsqSwapTrade()) return false; Trade trade = (Trade) tradeModel; - return Optional.ofNullable(trade.getDepositTx()) - .map(Transaction::getTxId) - .map(hash -> hash.equals(txId)) - .orElse(false); + return txId.equals(trade.getDepositTxId()); } private boolean isOfferFeeTx(String txId) { if (isBsqSwapTrade()) return false; - return Optional.ofNullable(tradeModel.getOffer()) - .map(Offer::getOfferFeePaymentTxId) - .map(paymentTxId -> paymentTxId.equals(txId)) - .orElse(false); + Offer offer = tradeModel.getOffer(); + return offer != null && txId.equals(offer.getOfferFeePaymentTxId()); } private boolean isDisputedPayoutTx(String txId) { @@ -168,7 +158,7 @@ boolean isDelayedPayoutTx(String txId) { if (parentTransaction == null) { return false; } - return isDepositTx(parentTransaction.getTxId()); + return isDepositTx(parentTransaction.getTxId().toString()); }); } @@ -177,7 +167,6 @@ private boolean isRefundPayoutTx(Trade trade, String txId) { return false; String tradeId = tradeModel.getId(); - ObservableList disputes = refundManager.getDisputesAsObservableList(); boolean isAnyDisputeRelatedToThis = refundManager.getDisputedTradeIds().contains(tradeId); diff --git a/desktop/src/test/java/bisq/desktop/main/funds/transactions/TransactionAwareTradeTest.java b/desktop/src/test/java/bisq/desktop/main/funds/transactions/TransactionAwareTradeTest.java index 711dac141c0..9659e202feb 100644 --- a/desktop/src/test/java/bisq/desktop/main/funds/transactions/TransactionAwareTradeTest.java +++ b/desktop/src/test/java/bisq/desktop/main/funds/transactions/TransactionAwareTradeTest.java @@ -66,13 +66,13 @@ public void testIsRelatedToTransactionWhenTakerOfferFeeTx() { @Test public void testIsRelatedToTransactionWhenPayoutTx() { - when(delegate.getPayoutTx().getTxId()).thenReturn(XID); + when(delegate.getPayoutTxId()).thenReturn(XID.toString()); assertTrue(trade.isRelatedToTransaction(transaction)); } @Test public void testIsRelatedToTransactionWhenDepositTx() { - when(delegate.getDepositTx().getTxId()).thenReturn(XID); + when(delegate.getDepositTxId()).thenReturn(XID.toString()); assertTrue(trade.isRelatedToTransaction(transaction)); } From c913b7a428a0a3990a34ad7b2721941db71dbe5a Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Thu, 9 Feb 2023 17:13:14 +0800 Subject: [PATCH 02/10] Cache last txId to further speed up 'isRelatedToTransaction' Use a mutable static tuple field to cache the last result of 'Sha256Hash.toString', which is used to get the ID string of the input tx, when calling 'TransactionAwareTrade.isRelatedToTransaction'. In this way, consecutive calls to 'isRelatedToTransaction' on the same input tx (over all the past trades, as done by 'TransactionsView.updateList') are sped up significantly, since hex encoding the txId is a bottleneck. --- .../main/funds/transactions/TransactionAwareTrade.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java index cc449231001..faca5917c14 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java @@ -29,6 +29,7 @@ import bisq.core.trade.model.bsq_swap.BsqSwapTrade; import bisq.common.crypto.PubKeyRing; +import bisq.common.util.Tuple2; import org.bitcoinj.core.Address; import org.bitcoinj.core.Sha256Hash; @@ -49,6 +50,9 @@ class TransactionAwareTrade implements TransactionAwareTradable { private final BtcWalletService btcWalletService; private final PubKeyRing pubKeyRing; + // As Sha256Hash.toString() is expensive, cache the last result, which will usually be next one needed. + private static Tuple2 lastTxIdTuple; + TransactionAwareTrade(TradeModel tradeModel, ArbitrationManager arbitrationManager, RefundManager refundManager, @@ -64,7 +68,11 @@ class TransactionAwareTrade implements TransactionAwareTradable { @Override public boolean isRelatedToTransaction(Transaction transaction) { Sha256Hash hash = transaction.getTxId(); - String txId = hash.toString(); + var txIdTuple = lastTxIdTuple; + if (txIdTuple == null || !txIdTuple.first.equals(hash)) { + lastTxIdTuple = txIdTuple = new Tuple2<>(hash, hash.toString()); + } + String txId = txIdTuple.second; boolean tradeRelated = false; if (tradeModel instanceof Trade) { From 3aacc99f38e29e985ff7d010b644bc50889e54bd Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Thu, 9 Feb 2023 21:16:40 +0800 Subject: [PATCH 03/10] Tidy up TransactionsView slightly 1. Expression lambda -> method reference; 2. Statement lambda -> expression lambda; 3. Field -> local variable. --- .../main/funds/transactions/TransactionsView.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java index 8c341c91fb4..55f33584a67 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java @@ -201,7 +201,7 @@ public void initialize() { dateColumn.setComparator(Comparator.comparing(TransactionsListItem::getDate)); tradeIdColumn.setComparator(Comparator.comparing(o -> o.getTradable() != null ? o.getTradable().getId() : "")); - detailsColumn.setComparator(Comparator.comparing(o -> o.getDetails())); + detailsColumn.setComparator(Comparator.comparing(TransactionsListItem::getDetails)); addressColumn.setComparator(Comparator.comparing(item -> item.getDirection() + item.getAddressString())); transactionColumn.setComparator(Comparator.comparing(TransactionsListItem::getTxId)); amountColumn.setComparator(Comparator.comparing(TransactionsListItem::getAmountAsCoin)); @@ -211,9 +211,7 @@ public void initialize() { dateColumn.setSortType(TableColumn.SortType.DESCENDING); tableView.getSortOrder().add(dateColumn); - walletChangeEventListener = wallet -> { - updateList(); - }; + walletChangeEventListener = wallet -> updateList(); keyEventEventHandler = event -> { // Not intended to be public to users as the feature is not well tested @@ -419,15 +417,13 @@ public TableCell call(TableColumn column) { return new TableCell<>() { - private HyperlinkWithIcon hyperlinkWithIcon; - @Override public void updateItem(final TransactionsListItem item, boolean empty) { super.updateItem(item, empty); if (item != null && !empty) { if (item.isDustAttackTx()) { - hyperlinkWithIcon = new HyperlinkWithIcon(item.getDetails(), AwesomeIcon.WARNING_SIGN); + var hyperlinkWithIcon = new HyperlinkWithIcon(item.getDetails(), AwesomeIcon.WARNING_SIGN); hyperlinkWithIcon.setOnAction(event -> new Popup().warning(Res.get("funds.tx.dustAttackTx.popup")).show()); setGraphic(hyperlinkWithIcon); } else { From fb78345ba31312410ec31e7f8b8a01ee34ef120d Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Thu, 9 Feb 2023 21:35:32 +0800 Subject: [PATCH 04/10] Avoid repeated Set build in transactions view load Move the line, Set tradables = tradableRepository.getAll(); to the top level of 'TransactionsView.updateList', instead of needlessly calling 'TradableRepository.getAll' (which builds a new set every invocation) for each wallet transaction being iterated over. This was causing a significant slowdown of the view load. --- .../desktop/main/funds/transactions/TransactionsView.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java index 55f33584a67..2fc2e868760 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java @@ -288,11 +288,11 @@ protected void deactivate() { } private void updateList() { + Set tradables = tradableRepository.getAll(); + List transactionsListItems = btcWalletService.getTransactions(false) .stream() .map(transaction -> { - Set tradables = tradableRepository.getAll(); - TransactionAwareTradable maybeTradable = tradables.stream() .map(tradable -> { if (tradable instanceof OpenOffer) { From 19a80d19bb36043053532a08f9cd7b89881fcf96 Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Fri, 10 Feb 2023 10:28:02 +0800 Subject: [PATCH 05/10] Short circuit txId hex encoding for BSQ swap offers Inline a local variable, to eliminate another minor Sha256Hash.toString hotspot in the Transactions view load, this time coming from 'TransactionsAwareOpenOffer.isRelatedToTransaction'. This is helpful in the case that the user has a large number of (possibly disabled) BSQ swap offers. --- .../main/funds/transactions/TransactionAwareOpenOffer.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareOpenOffer.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareOpenOffer.java index 4a4368b01a8..a355e2f849e 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareOpenOffer.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareOpenOffer.java @@ -34,9 +34,7 @@ public boolean isRelatedToTransaction(Transaction transaction) { Offer offer = delegate.getOffer(); String paymentTxId = offer.getOfferFeePaymentTxId(); - String txId = transaction.getTxId().toString(); - - return paymentTxId != null && paymentTxId.equals(txId); + return paymentTxId != null && paymentTxId.equals(transaction.getTxId().toString()); } public Tradable asTradable() { From 97ef9c13080afff81041e97c5dd6a7ad1e85a99e Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Sat, 11 Feb 2023 01:03:52 +0800 Subject: [PATCH 06/10] Place filter in front of 'isRelatedToTransaction' for speedup Use a crude Bloom filter (of sorts) to cut down the quadratic number of calls to 'TransactionAwareTradable.isRelatedToTransaction' (that is, one for each tx-tradable pair) during the Transactions view load. In this way, we may reduce the number of calls roughly 40-fold, for a Bisq instance with similar numbers of BSQ swap trades and escrow trades. (Sadly, profiling does not show a 40-fold reduction in the size of the 'isRelatedToTransaction' hotspot, likely due to the remaining calls being expensive ones involving disputed trades or unusual txs with nonzero locktime, e.g. dust attacks or funds from Electrum wallets.) To this end, partition the wallet transactions into 64 pseudo-randomly chosen buckets (with a dedicated bucket for txs which might be delayed payouts, namely those with nonzero locktime). Add an interface method, 'TransactionAwareTradable.getRelatedTransactionFilter', which returns an IntStream of all the indices of buckets where a related tx may plausibly be found. Where this is unclear, e.g. for trades involved in a dispute, just return everything (that is, the range 0..63 inclusive). Add a class, 'RelatedTransactionFilterSlices', that holds a provided list of TransactionAwareTradable instances and 64 bitsets of all the slices through their respective filters (each realised as 64-bit word instead of a streams of integers). In this way, a list of tradables plausibly related to any given tx may be quickly found by simply selecting the appropriate bitset of the 64 (by the tx bucket index). --- .../RelatedTransactionFilterSlices.java | 52 +++++++++++++++++++ .../TransactionAwareOpenOffer.java | 10 ++++ .../TransactionAwareTradable.java | 25 +++++++++ .../transactions/TransactionAwareTrade.java | 28 +++++++++- .../funds/transactions/TransactionsView.java | 37 +++++++------ 5 files changed, 134 insertions(+), 18 deletions(-) create mode 100644 desktop/src/main/java/bisq/desktop/main/funds/transactions/RelatedTransactionFilterSlices.java diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/RelatedTransactionFilterSlices.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/RelatedTransactionFilterSlices.java new file mode 100644 index 00000000000..08f168dc2e4 --- /dev/null +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/RelatedTransactionFilterSlices.java @@ -0,0 +1,52 @@ +/* + * This file is part of Bisq. + * + * Bisq is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or (at + * your option) any later version. + * + * Bisq is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public + * License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Bisq. If not, see . + */ + +package bisq.desktop.main.funds.transactions; + +import org.bitcoinj.core.Transaction; + +import java.util.Arrays; +import java.util.BitSet; +import java.util.Collection; +import java.util.List; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import static bisq.desktop.main.funds.transactions.TransactionAwareTradable.TX_FILTER_SIZE; + +public class RelatedTransactionFilterSlices { + private final List tradables; + private final BitSet[] filterSlices; + + public RelatedTransactionFilterSlices(Collection tradables) { + this.tradables = List.copyOf(tradables); + + filterSlices = new BitSet[TX_FILTER_SIZE]; + Arrays.setAll(filterSlices, i -> new BitSet(this.tradables.size())); + + IntStream.range(0, this.tradables.size()) + .forEach(j -> this.tradables.get(j).getRelatedTransactionFilter() + .forEach(i -> filterSlices[i].set(j))); + } + + public Stream getAllRelatedTradables(Transaction tx) { + int i = TransactionAwareTradable.bucketIndex(tx); + return filterSlices[i].stream() + .mapToObj(tradables::get) + .filter(t -> t.isRelatedToTransaction(tx)); + } +} diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareOpenOffer.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareOpenOffer.java index a355e2f849e..b9f14f1e8aa 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareOpenOffer.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareOpenOffer.java @@ -23,6 +23,8 @@ import org.bitcoinj.core.Transaction; +import java.util.stream.IntStream; + class TransactionAwareOpenOffer implements TransactionAwareTradable { private final OpenOffer delegate; @@ -40,4 +42,12 @@ public boolean isRelatedToTransaction(Transaction transaction) { public Tradable asTradable() { return delegate; } + + @Override + public IntStream getRelatedTransactionFilter() { + Offer offer = delegate.getOffer(); + String paymentTxId = offer.getOfferFeePaymentTxId(); + return IntStream.of(TransactionAwareTradable.bucketIndex(paymentTxId)) + .filter(i -> i >= 0); + } } diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTradable.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTradable.java index b00c283d96e..99d05de3ac3 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTradable.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTradable.java @@ -19,10 +19,35 @@ import bisq.core.trade.model.Tradable; +import org.bitcoinj.core.Sha256Hash; import org.bitcoinj.core.Transaction; +import java.util.stream.IntStream; + +import javax.annotation.Nullable; + interface TransactionAwareTradable { + int TX_FILTER_SIZE = 64; + int DELAYED_PAYOUT_TX_BUCKET_INDEX = TX_FILTER_SIZE - 1; + boolean isRelatedToTransaction(Transaction transaction); Tradable asTradable(); + + /** Returns a list of bucket indices of all transactions which might be related to this Tradable. */ + IntStream getRelatedTransactionFilter(); + + static int bucketIndex(Transaction tx) { + return tx.getLockTime() == 0 ? bucketIndex(tx.getTxId()) : DELAYED_PAYOUT_TX_BUCKET_INDEX; + } + + static int bucketIndex(Sha256Hash hash) { + int i = hash.getBytes()[31] & 255; + return i % TX_FILTER_SIZE != DELAYED_PAYOUT_TX_BUCKET_INDEX ? + i % TX_FILTER_SIZE : i / TX_FILTER_SIZE; + } + + static int bucketIndex(@Nullable String txId) { + return txId != null ? bucketIndex(Sha256Hash.wrap(txId)) : -1; + } } diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java index faca5917c14..4ffe513cf25 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java @@ -38,8 +38,11 @@ import javafx.collections.ObservableList; +import java.util.stream.IntStream; + import lombok.extern.slf4j.Slf4j; +import static bisq.desktop.main.funds.transactions.TransactionAwareTradable.bucketIndex; import static com.google.common.base.Preconditions.checkNotNull; @Slf4j @@ -128,7 +131,7 @@ private boolean isDisputedPayoutTx(String txId) { String delegateId = tradeModel.getId(); ObservableList disputes = arbitrationManager.getDisputesAsObservableList(); - boolean isAnyDisputeRelatedToThis = arbitrationManager.getDisputedTradeIds().contains(tradeModel.getId()); + boolean isAnyDisputeRelatedToThis = arbitrationManager.getDisputedTradeIds().contains(delegateId); return isAnyDisputeRelatedToThis && disputes.stream() .anyMatch(dispute -> { @@ -216,4 +219,27 @@ private boolean isBsqSwapTrade(String txId) { public Tradable asTradable() { return tradeModel; } + + @Override + public IntStream getRelatedTransactionFilter() { + if (tradeModel instanceof Trade && !arbitrationManager.getDisputedTradeIds().contains(tradeModel.getId()) && + !refundManager.getDisputedTradeIds().contains(tradeModel.getId())) { + Trade trade = (Trade) tradeModel; + String takerFeeTxId = trade.getTakerFeeTxId(); + String offerFeeTxId = trade.getOffer() != null ? trade.getOffer().getOfferFeePaymentTxId() : null; + String depositTxId = trade.getDepositTxId(); + String payoutTxId = trade.getPayoutTxId(); + return IntStream.of(DELAYED_PAYOUT_TX_BUCKET_INDEX, bucketIndex(takerFeeTxId), bucketIndex(offerFeeTxId), + bucketIndex(depositTxId), bucketIndex(payoutTxId)) + .filter(i -> i >= 0); + } else if (tradeModel instanceof BsqSwapTrade) { + BsqSwapTrade trade = (BsqSwapTrade) tradeModel; + String swapTxId = trade.getTxId(); + return IntStream.of(bucketIndex(swapTxId)) + .filter(i -> i >= 0); + } else { + // We are involved in a dispute (rare) - don't do any initial tx filtering. + return IntStream.range(0, TX_FILTER_SIZE); + } + } } diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java index 2fc2e868760..3fa3bdb4018 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java @@ -95,6 +95,7 @@ import java.util.Comparator; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -289,27 +290,29 @@ protected void deactivate() { private void updateList() { Set tradables = tradableRepository.getAll(); + var filterSlices = new RelatedTransactionFilterSlices(tradables.stream() + .map(tradable -> { + if (tradable instanceof OpenOffer) { + return new TransactionAwareOpenOffer((OpenOffer) tradable); + } else if (tradable instanceof TradeModel) { + return new TransactionAwareTrade( + (TradeModel) tradable, + arbitrationManager, + refundManager, + btcWalletService, + pubKeyRing + ); + } else { + return null; + } + }) + .filter(Objects::nonNull) + .collect(Collectors.toUnmodifiableList())); List transactionsListItems = btcWalletService.getTransactions(false) .stream() .map(transaction -> { - TransactionAwareTradable maybeTradable = tradables.stream() - .map(tradable -> { - if (tradable instanceof OpenOffer) { - return new TransactionAwareOpenOffer((OpenOffer) tradable); - } else if (tradable instanceof TradeModel) { - return new TransactionAwareTrade( - (TradeModel) tradable, - arbitrationManager, - refundManager, - btcWalletService, - pubKeyRing - ); - } else { - return null; - } - }) - .filter(tradable -> tradable != null && tradable.isRelatedToTransaction(transaction)) + TransactionAwareTradable maybeTradable = filterSlices.getAllRelatedTradables(transaction) .findAny() .orElse(null); From 2e8f03061efce2e71e6072a3e6f29b46f89fdda1 Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Sat, 11 Feb 2023 08:04:21 +0800 Subject: [PATCH 07/10] TransactionsListItem cleanup: remove unused local variable (This also avoids a redundant call to 'Tradable.getShortId', shaving off another percent or two from the Transactions view load.) --- .../desktop/main/funds/transactions/TransactionsListItem.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java index 49a32bbcf7c..c7da5056b5a 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java @@ -17,10 +17,10 @@ package bisq.desktop.main.funds.transactions; -import bisq.desktop.util.filtering.FilterableListItem; import bisq.desktop.components.indicator.TxConfidenceIndicator; import bisq.desktop.util.DisplayUtils; import bisq.desktop.util.GUIUtil; +import bisq.desktop.util.filtering.FilterableListItem; import bisq.core.btc.listeners.TxConfidenceListener; import bisq.core.btc.wallet.BsqWalletService; @@ -197,7 +197,6 @@ else if (txTypeOptional.get().equals(TxType.REIMBURSEMENT_REQUEST)) if (optionalTradable.isPresent()) { tradable = optionalTradable.get(); - String tradeId = tradable.getShortId(); if (tradable instanceof OpenOffer) { details = Res.get("funds.tx.createOfferFee"); } else if (tradable instanceof Trade) { From ee452025218c3afbdf50ba40b60ab3d7cf55db8e Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Sat, 11 Feb 2023 09:30:57 +0800 Subject: [PATCH 08/10] Remove redundant listener from TransactionsListItem Eliminate a minor quadratic time bug, caused by the unnecessary addition of a (BtcWalletService) TxConfidenceListener for each list item in the Transactions view. (Since the confidence listeners are internally held in a CopyOnWriteArraySet, this sadly runs in quadratic time, slowing down the Transactions view load a little.) The confidence listener is apparently redundant because of a set of calls to 'TransactionsListItem.cleanup' immediately upon construction of the item list, which removes all the listeners just added. (This code appears to date from at least February 2016, in commit c70df86.) (The confidence indicators are kept up to date by simply reloading the entire list upon each wallet change event.) --- .../transactions/TransactionsListItem.java | 18 ------------------ .../funds/transactions/TransactionsView.java | 2 -- 2 files changed, 20 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java index c7da5056b5a..6b67ee2d5d1 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java @@ -22,7 +22,6 @@ import bisq.desktop.util.GUIUtil; import bisq.desktop.util.filtering.FilterableListItem; -import bisq.core.btc.listeners.TxConfidenceListener; import bisq.core.btc.wallet.BsqWalletService; import bisq.core.btc.wallet.BtcWalletService; import bisq.core.btc.wallet.WalletService; @@ -59,7 +58,6 @@ @Slf4j class TransactionsListItem implements FilterableListItem { - private final BtcWalletService btcWalletService; private final CoinFormatter formatter; private String dateString; private final Date date; @@ -69,7 +67,6 @@ class TransactionsListItem implements FilterableListItem { private String details = ""; private String addressString = ""; private String direction = ""; - private TxConfidenceListener txConfidenceListener; private boolean received; private Coin amountAsCoin = Coin.ZERO; private String memo = ""; @@ -91,7 +88,6 @@ private LazyFields lazy() { // used at exportCSV TransactionsListItem() { date = null; - btcWalletService = null; txId = null; formatter = null; isDustAttackTx = false; @@ -105,7 +101,6 @@ private LazyFields lazy() { DaoFacade daoFacade, CoinFormatter formatter, long ignoreDustThreshold) { - this.btcWalletService = btcWalletService; this.formatter = formatter; this.memo = transaction.getMemo(); @@ -303,19 +298,6 @@ else if (txTypeOptional.get().equals(TxType.REIMBURSEMENT_REQUEST)) GUIUtil.updateConfidence(confidence, tooltip, txConfidenceIndicator); confirmations = confidence.getDepthInBlocks(); }}); - - txConfidenceListener = new TxConfidenceListener(txId) { - @Override - public void onTransactionConfidenceChanged(TransactionConfidence confidence) { - GUIUtil.updateConfidence(confidence, lazy().tooltip, lazy().txConfidenceIndicator); - confirmations = confidence.getDepthInBlocks(); - } - }; - btcWalletService.addTxConfidenceListener(txConfidenceListener); - } - - public void cleanup() { - btcWalletService.removeTxConfidenceListener(txConfidenceListener); } diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java index 3fa3bdb4018..39945f799cb 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsView.java @@ -279,7 +279,6 @@ protected void activate() { protected void deactivate() { filterBox.deactivate(); sortedList.comparatorProperty().unbind(); - observableList.forEach(TransactionsListItem::cleanup); btcWalletService.removeChangeEventListener(walletChangeEventListener); if (scene != null) @@ -328,7 +327,6 @@ private void updateList() { }) .collect(Collectors.toList()); // are sorted by getRecentTransactions - transactionsListItems.forEach(TransactionsListItem::cleanup); observableList.setAll(transactionsListItems); } From 37b6a4becfb8389bda4a7b6cecb57da07bab661b Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Sat, 11 Feb 2023 15:47:04 +0800 Subject: [PATCH 09/10] Cache formatters to speed up DisplayUtils.formatDateTime Make 'FormattingUtils.formatDateTime(Date, boolean)' two or three times faster by caching the 'java.text.DateFormat' objects used to format the given date & time in either the local or UTC time zone. Since these formatters are not thread safe (though they may be reused), use a ThreadLocal to store them (as a tuple, along with the current locale to make sure it hasn't changed since the last invocation). This is a minor hotspot for the Transactions view load, since the date strings in TransactionsListItem are formatted eagerly (via DisplayUtils) for the purpose of filtering the items by substring match. (Other list views seem to format dates lazily, though it's possible that there will be speedups elsewhere in the UI.) --- .../java/bisq/core/util/FormattingUtils.java | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/bisq/core/util/FormattingUtils.java b/core/src/main/java/bisq/core/util/FormattingUtils.java index d6658e71290..cf8e098b6f6 100644 --- a/core/src/main/java/bisq/core/util/FormattingUtils.java +++ b/core/src/main/java/bisq/core/util/FormattingUtils.java @@ -7,6 +7,7 @@ import bisq.core.monetary.Price; import bisq.common.util.MathUtils; +import bisq.common.util.Tuple3; import org.bitcoinj.core.Coin; import org.bitcoinj.core.Monetary; @@ -33,12 +34,15 @@ public class FormattingUtils { public static final String BTC_FORMATTER_KEY = "BTC"; - public final static String RANGE_SEPARATOR = " - "; + public static final String RANGE_SEPARATOR = " - "; private static final MonetaryFormat fiatPriceFormat = new MonetaryFormat().shift(0).minDecimals(4).repeatOptionalDecimals(0, 0); private static final MonetaryFormat altcoinFormat = new MonetaryFormat().shift(0).minDecimals(8).repeatOptionalDecimals(0, 0); private static final DecimalFormat decimalFormat = new DecimalFormat("#.#"); + private static final ThreadLocal> cachedUtcDateTimeFormatters = new ThreadLocal<>(); + private static final ThreadLocal> cachedLocalDateTimeFormatters = new ThreadLocal<>(); + public static String formatCoinWithCode(long value, MonetaryFormat coinFormat) { return formatCoinWithCode(Coin.valueOf(value), coinFormat); } @@ -183,12 +187,25 @@ public static String formatRoundedDoubleWithPrecision(double value, int precisio public static String formatDateTime(Date date, boolean useLocaleAndLocalTimezone) { Locale locale = useLocaleAndLocalTimezone ? GlobalSettings.getLocale() : Locale.US; - DateFormat dateInstance = DateFormat.getDateInstance(DateFormat.DEFAULT, locale); - DateFormat timeInstance = DateFormat.getTimeInstance(DateFormat.DEFAULT, locale); - if (!useLocaleAndLocalTimezone) { - dateInstance.setTimeZone(TimeZone.getTimeZone("UTC")); - timeInstance.setTimeZone(TimeZone.getTimeZone("UTC")); + + var formatterTuple = (useLocaleAndLocalTimezone ? + cachedLocalDateTimeFormatters : cachedUtcDateTimeFormatters).get(); + if (formatterTuple == null || !formatterTuple.first.equals(locale)) { + formatterTuple = new Tuple3<>(locale, + DateFormat.getDateInstance(DateFormat.DEFAULT, locale), + DateFormat.getTimeInstance(DateFormat.DEFAULT, locale)); + + if (useLocaleAndLocalTimezone) { + cachedLocalDateTimeFormatters.set(formatterTuple); + } else { + formatterTuple.second.setTimeZone(TimeZone.getTimeZone("UTC")); + formatterTuple.third.setTimeZone(TimeZone.getTimeZone("UTC")); + cachedUtcDateTimeFormatters.set(formatterTuple); + } } + DateFormat dateInstance = formatterTuple.second; + DateFormat timeInstance = formatterTuple.third; + return formatDateTime(date, dateInstance, timeInstance); } @@ -288,7 +305,8 @@ else if (bytes < mb) } @NotNull - public static String fillUpPlacesWithEmptyStrings(String formattedNumber, @SuppressWarnings("unused") int maxNumberOfDigits) { + public static String fillUpPlacesWithEmptyStrings(String formattedNumber, + @SuppressWarnings("unused") int maxNumberOfDigits) { //FIXME: temporary deactivate adding spaces in front of numbers as we don't use a monospace font right now. /*int numberOfPlacesToFill = maxNumberOfDigits - formattedNumber.length(); for (int i = 0; i < numberOfPlacesToFill; i++) { From 6a845007104cd3ce5d5c60888cb4b727af815078 Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Sun, 12 Feb 2023 12:35:05 +0800 Subject: [PATCH 10/10] Cache txo addresses to speed up isRefundPayoutTx in some cases In the (hopefully rare) case that the user has multiple past trades that end in arbitration, the entire wallet tx output set was scanned once for every such trade (via 'TransactionAwareTrade.isRefundPayoutTx' calls), to look for any outputs matching the payout address. This potentially causes a slowdown of the Transaction view load for each new arbitration case added. To avoid this problem, cache the last set of recipient address strings of the provided tx, as the next call to 'isRefundPayoutTx' is likely to be for the same tx. Also check that there is exactly one input (the multisig input) for any candidate delayed payout tx, to speed up 'isDelayedPayoutTx' in case the wallet contains many unusual txs with nonzero locktime. --- .../transactions/TransactionAwareTrade.java | 56 ++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java index 4ffe513cf25..b0c21a6ff9a 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java @@ -36,8 +36,11 @@ import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionOutput; +import com.google.common.collect.ImmutableSet; + import javafx.collections.ObservableList; +import java.util.Set; import java.util.stream.IntStream; import lombok.extern.slf4j.Slf4j; @@ -55,6 +58,8 @@ class TransactionAwareTrade implements TransactionAwareTradable { // As Sha256Hash.toString() is expensive, cache the last result, which will usually be next one needed. private static Tuple2 lastTxIdTuple; + // Similarly, cache the last computed set of tx receiver addresses, to speed up 'isRefundPayoutTx'. + private static Tuple2> lastReceiverAddressStringsTuple; TransactionAwareTrade(TradeModel tradeModel, ArbitrationManager arbitrationManager, @@ -156,7 +161,7 @@ boolean isDelayedPayoutTx(String txId) { if (transaction.getLockTime() == 0) return false; - if (transaction.getInputs() == null) + if (transaction.getInputs() == null || transaction.getInputs().size() != 1) return false; return transaction.getInputs().stream() @@ -178,32 +183,45 @@ private boolean isRefundPayoutTx(Trade trade, String txId) { return false; String tradeId = tradeModel.getId(); - boolean isAnyDisputeRelatedToThis = refundManager.getDisputedTradeIds().contains(tradeId); if (isAnyDisputeRelatedToThis) { - Transaction tx = btcWalletService.getTransaction(txId); - if (tx != null) { - for (TransactionOutput txo : tx.getOutputs()) { - if (btcWalletService.isTransactionOutputMine(txo)) { - try { - Address receiverAddress = txo.getScriptPubKey().getToAddress(btcWalletService.getParams()); - Contract contract = checkNotNull(trade.getContract()); - String myPayoutAddressString = contract.isMyRoleBuyer(pubKeyRing) ? - contract.getBuyerPayoutAddressString() : - contract.getSellerPayoutAddressString(); - if (receiverAddress != null && myPayoutAddressString.equals(receiverAddress.toString())) { - return true; - } - } catch (RuntimeException ignore) { - } - } - } + try { + Contract contract = checkNotNull(trade.getContract()); + String myPayoutAddressString = contract.isMyRoleBuyer(pubKeyRing) ? + contract.getBuyerPayoutAddressString() : + contract.getSellerPayoutAddressString(); + + return getReceiverAddressStrings(txId).contains(myPayoutAddressString); + } catch (RuntimeException ignore) { } } return false; } + private Set getReceiverAddressStrings(String txId) { + var tuple = lastReceiverAddressStringsTuple; + if (tuple == null || !tuple.first.equals(txId)) { + lastReceiverAddressStringsTuple = tuple = computeReceiverAddressStringsTuple(txId); + } + return tuple != null ? tuple.second : ImmutableSet.of(); + } + + private Tuple2> computeReceiverAddressStringsTuple(String txId) { + Transaction tx = btcWalletService.getTransaction(txId); + if (tx == null) { + // Clear cache if the tx isn't found, as theoretically it could be added to the wallet later. + return null; + } + Set addressStrings = tx.getOutputs().stream() + .filter(btcWalletService::isTransactionOutputMine) + .map(txo -> txo.getScriptPubKey().getToAddress(btcWalletService.getParams())) + .map(Address::toString) + .collect(ImmutableSet.toImmutableSet()); + + return new Tuple2<>(txId, addressStrings); + } + private boolean isBsqSwapTrade() { return tradeModel instanceof BsqSwapTrade; }