mirror of
https://github.com/bisq-network/bisq.git
synced 2025-02-25 07:27:18 +01:00
Optimise TransactionAwareTrade.isRelatedToTransaction
Attempt to remove a bottleneck during the transactions view load, as revealed by JProfiler, by optimising the code to determine if any given transaction and trade are related. Since both these sets will tend to grow linearly with time, causing quadratic slowdown of TransactionsView, try to alleviate (without completely fixing) the problem. To do this, add a cached set of disputed trade IDs to DisputeListService so that TransactionAwareTradable.is(Dispute|RefundPayout)Tx can be made O(1) in the common case that the given trade is not involved in any dispute. Also avoid calling Sha256Hash::toString by passing tx IDs as Sha256Hash objects directly to is(Deposit|Payout)Tx, and short circuit an expensive call BtcWalletService.getTransaction in isDelayedPayoutTx, in the common case, by pre-checking the transaction locktime. This also fixes a bug in isRefundPayoutTx whereby it incorrectly returns false if there are >1 disputes in the list returned by RefundManager but the given trade is not involved in the last one.
This commit is contained in:
parent
97779f10d8
commit
db26a1fe2d
4 changed files with 47 additions and 42 deletions
|
@ -34,8 +34,10 @@ import javafx.beans.property.SimpleIntegerProperty;
|
||||||
import javafx.collections.ObservableList;
|
import javafx.collections.ObservableList;
|
||||||
|
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
|
import java.util.HashSet;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
import java.util.Set;
|
||||||
import java.util.function.Consumer;
|
import java.util.function.Consumer;
|
||||||
import java.util.stream.Collectors;
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
|
@ -53,6 +55,8 @@ public abstract class DisputeListService<T extends DisputeList<Dispute>> impleme
|
||||||
private final Map<String, Subscription> disputeIsClosedSubscriptionsMap = new HashMap<>();
|
private final Map<String, Subscription> disputeIsClosedSubscriptionsMap = new HashMap<>();
|
||||||
@Getter
|
@Getter
|
||||||
private final IntegerProperty numOpenDisputes = new SimpleIntegerProperty();
|
private final IntegerProperty numOpenDisputes = new SimpleIntegerProperty();
|
||||||
|
@Getter
|
||||||
|
private final Set<String> disputedTradeIds = new HashSet<>();
|
||||||
|
|
||||||
|
|
||||||
///////////////////////////////////////////////////////////////////////////////////////////
|
///////////////////////////////////////////////////////////////////////////////////////////
|
||||||
|
@ -154,6 +158,7 @@ public abstract class DisputeListService<T extends DisputeList<Dispute>> impleme
|
||||||
disputeIsClosedSubscriptionsMap.get(id).unsubscribe();
|
disputeIsClosedSubscriptionsMap.get(id).unsubscribe();
|
||||||
disputeIsClosedSubscriptionsMap.remove(id);
|
disputeIsClosedSubscriptionsMap.remove(id);
|
||||||
}
|
}
|
||||||
|
disputedTradeIds.remove(dispute.getTradeId());
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
addedList.forEach(dispute -> {
|
addedList.forEach(dispute -> {
|
||||||
|
@ -168,6 +173,7 @@ public abstract class DisputeListService<T extends DisputeList<Dispute>> impleme
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
disputeIsClosedSubscriptionsMap.put(id, disputeStateSubscription);
|
disputeIsClosedSubscriptionsMap.put(id, disputeStateSubscription);
|
||||||
|
disputedTradeIds.add(dispute.getTradeId());
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -69,6 +69,7 @@ import java.security.KeyPair;
|
||||||
import java.util.Date;
|
import java.util.Date;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
|
import java.util.Set;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
import java.util.concurrent.TimeUnit;
|
import java.util.concurrent.TimeUnit;
|
||||||
import java.util.stream.Collectors;
|
import java.util.stream.Collectors;
|
||||||
|
@ -231,6 +232,9 @@ public abstract class DisputeManager<T extends DisputeList<Dispute>> extends Sup
|
||||||
return disputeListService.getDisputeList();
|
return disputeListService.getDisputeList();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public Set<String> getDisputedTradeIds() {
|
||||||
|
return disputeListService.getDisputedTradeIds();
|
||||||
|
}
|
||||||
|
|
||||||
///////////////////////////////////////////////////////////////////////////////////////////
|
///////////////////////////////////////////////////////////////////////////////////////////
|
||||||
// API
|
// API
|
||||||
|
|
|
@ -36,13 +36,11 @@ import org.bitcoinj.core.TransactionOutput;
|
||||||
import javafx.collections.ObservableList;
|
import javafx.collections.ObservableList;
|
||||||
|
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
import java.util.concurrent.atomic.AtomicBoolean;
|
|
||||||
|
|
||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
|
|
||||||
import static com.google.common.base.Preconditions.checkNotNull;
|
import static com.google.common.base.Preconditions.checkNotNull;
|
||||||
|
|
||||||
|
|
||||||
@Slf4j
|
@Slf4j
|
||||||
class TransactionAwareTrade implements TransactionAwareTradable {
|
class TransactionAwareTrade implements TransactionAwareTradable {
|
||||||
private final Trade trade;
|
private final Trade trade;
|
||||||
|
@ -65,32 +63,31 @@ class TransactionAwareTrade implements TransactionAwareTradable {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public boolean isRelatedToTransaction(Transaction transaction) {
|
public boolean isRelatedToTransaction(Transaction transaction) {
|
||||||
String txId = transaction.getTxId().toString();
|
Sha256Hash hash = transaction.getTxId();
|
||||||
|
String txId = hash.toString();
|
||||||
|
|
||||||
boolean isTakerOfferFeeTx = txId.equals(trade.getTakerFeeTxId());
|
boolean isTakerOfferFeeTx = txId.equals(trade.getTakerFeeTxId());
|
||||||
boolean isOfferFeeTx = isOfferFeeTx(txId);
|
boolean isOfferFeeTx = isOfferFeeTx(txId);
|
||||||
boolean isDepositTx = isDepositTx(txId);
|
boolean isDepositTx = isDepositTx(hash);
|
||||||
boolean isPayoutTx = isPayoutTx(txId);
|
boolean isPayoutTx = isPayoutTx(hash);
|
||||||
boolean isDisputedPayoutTx = isDisputedPayoutTx(txId);
|
boolean isDisputedPayoutTx = isDisputedPayoutTx(txId);
|
||||||
boolean isDelayedPayoutTx = isDelayedPayoutTx(txId);
|
boolean isDelayedPayoutTx = transaction.getLockTime() != 0 && isDelayedPayoutTx(txId);
|
||||||
boolean isRefundPayoutTx = isRefundPayoutTx(txId);
|
boolean isRefundPayoutTx = isRefundPayoutTx(txId);
|
||||||
|
|
||||||
return isTakerOfferFeeTx || isOfferFeeTx || isDepositTx || isPayoutTx ||
|
return isTakerOfferFeeTx || isOfferFeeTx || isDepositTx || isPayoutTx ||
|
||||||
isDisputedPayoutTx || isDelayedPayoutTx || isRefundPayoutTx;
|
isDisputedPayoutTx || isDelayedPayoutTx || isRefundPayoutTx;
|
||||||
}
|
}
|
||||||
|
|
||||||
private boolean isPayoutTx(String txId) {
|
private boolean isPayoutTx(Sha256Hash txId) {
|
||||||
return Optional.ofNullable(trade.getPayoutTx())
|
return Optional.ofNullable(trade.getPayoutTx())
|
||||||
.map(Transaction::getTxId)
|
.map(Transaction::getTxId)
|
||||||
.map(Sha256Hash::toString)
|
|
||||||
.map(hash -> hash.equals(txId))
|
.map(hash -> hash.equals(txId))
|
||||||
.orElse(false);
|
.orElse(false);
|
||||||
}
|
}
|
||||||
|
|
||||||
private boolean isDepositTx(String txId) {
|
private boolean isDepositTx(Sha256Hash txId) {
|
||||||
return Optional.ofNullable(trade.getDepositTx())
|
return Optional.ofNullable(trade.getDepositTx())
|
||||||
.map(Transaction::getTxId)
|
.map(Transaction::getTxId)
|
||||||
.map(Sha256Hash::toString)
|
|
||||||
.map(hash -> hash.equals(txId))
|
.map(hash -> hash.equals(txId))
|
||||||
.orElse(false);
|
.orElse(false);
|
||||||
}
|
}
|
||||||
|
@ -104,9 +101,11 @@ class TransactionAwareTrade implements TransactionAwareTradable {
|
||||||
|
|
||||||
private boolean isDisputedPayoutTx(String txId) {
|
private boolean isDisputedPayoutTx(String txId) {
|
||||||
String delegateId = trade.getId();
|
String delegateId = trade.getId();
|
||||||
|
|
||||||
ObservableList<Dispute> disputes = arbitrationManager.getDisputesAsObservableList();
|
ObservableList<Dispute> disputes = arbitrationManager.getDisputesAsObservableList();
|
||||||
return disputes.stream()
|
|
||||||
|
boolean isAnyDisputeRelatedToThis = arbitrationManager.getDisputedTradeIds().contains(trade.getId());
|
||||||
|
|
||||||
|
return isAnyDisputeRelatedToThis && disputes.stream()
|
||||||
.anyMatch(dispute -> {
|
.anyMatch(dispute -> {
|
||||||
String disputePayoutTxId = dispute.getDisputePayoutTxId();
|
String disputePayoutTxId = dispute.getDisputePayoutTxId();
|
||||||
boolean isDisputePayoutTx = txId.equals(disputePayoutTxId);
|
boolean isDisputePayoutTx = txId.equals(disputePayoutTxId);
|
||||||
|
@ -139,22 +138,20 @@ class TransactionAwareTrade implements TransactionAwareTradable {
|
||||||
if (parentTransaction == null) {
|
if (parentTransaction == null) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
return isDepositTx(parentTransaction.getTxId().toString());
|
return isDepositTx(parentTransaction.getTxId());
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
private boolean isRefundPayoutTx(String txId) {
|
private boolean isRefundPayoutTx(String txId) {
|
||||||
String tradeId = trade.getId();
|
String tradeId = trade.getId();
|
||||||
ObservableList<Dispute> disputes = refundManager.getDisputesAsObservableList();
|
ObservableList<Dispute> disputes = refundManager.getDisputesAsObservableList();
|
||||||
AtomicBoolean isRefundTx = new AtomicBoolean(false);
|
|
||||||
AtomicBoolean isDisputeRelatedToThis = new AtomicBoolean(false);
|
boolean isAnyDisputeRelatedToThis = refundManager.getDisputedTradeIds().contains(tradeId);
|
||||||
disputes.forEach(dispute -> {
|
|
||||||
String disputeTradeId = dispute.getTradeId();
|
if (isAnyDisputeRelatedToThis) {
|
||||||
isDisputeRelatedToThis.set(tradeId.equals(disputeTradeId));
|
|
||||||
if (isDisputeRelatedToThis.get()) {
|
|
||||||
Transaction tx = btcWalletService.getTransaction(txId);
|
Transaction tx = btcWalletService.getTransaction(txId);
|
||||||
if (tx != null) {
|
if (tx != null) {
|
||||||
tx.getOutputs().forEach(txo -> {
|
for (TransactionOutput txo : tx.getOutputs()) {
|
||||||
if (btcWalletService.isTransactionOutputMine(txo)) {
|
if (btcWalletService.isTransactionOutputMine(txo)) {
|
||||||
try {
|
try {
|
||||||
Address receiverAddress = txo.getScriptPubKey().getToAddress(btcWalletService.getParams());
|
Address receiverAddress = txo.getScriptPubKey().getToAddress(btcWalletService.getParams());
|
||||||
|
@ -163,18 +160,15 @@ class TransactionAwareTrade implements TransactionAwareTradable {
|
||||||
contract.getBuyerPayoutAddressString() :
|
contract.getBuyerPayoutAddressString() :
|
||||||
contract.getSellerPayoutAddressString();
|
contract.getSellerPayoutAddressString();
|
||||||
if (receiverAddress != null && myPayoutAddressString.equals(receiverAddress.toString())) {
|
if (receiverAddress != null && myPayoutAddressString.equals(receiverAddress.toString())) {
|
||||||
isRefundTx.set(true);
|
return true;
|
||||||
}
|
}
|
||||||
} catch (Throwable ignore) {
|
} catch (RuntimeException ignore) {
|
||||||
}
|
|
||||||
|
|
||||||
}
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
}
|
||||||
|
}
|
||||||
return isRefundTx.get() && isDisputeRelatedToThis.get();
|
}
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|
|
@ -28,7 +28,7 @@ import org.bitcoinj.core.Transaction;
|
||||||
|
|
||||||
import javafx.collections.FXCollections;
|
import javafx.collections.FXCollections;
|
||||||
|
|
||||||
import java.util.Collections;
|
import java.util.Set;
|
||||||
|
|
||||||
import org.junit.Before;
|
import org.junit.Before;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
@ -41,8 +41,6 @@ import static org.mockito.Mockito.when;
|
||||||
public class TransactionAwareTradeTest {
|
public class TransactionAwareTradeTest {
|
||||||
private static final Sha256Hash XID = Sha256Hash.wrap("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef");
|
private static final Sha256Hash XID = Sha256Hash.wrap("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef");
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
private Transaction transaction;
|
private Transaction transaction;
|
||||||
private ArbitrationManager arbitrationManager;
|
private ArbitrationManager arbitrationManager;
|
||||||
private Trade delegate;
|
private Trade delegate;
|
||||||
|
@ -95,7 +93,10 @@ public class TransactionAwareTradeTest {
|
||||||
when(dispute.getTradeId()).thenReturn(tradeId);
|
when(dispute.getTradeId()).thenReturn(tradeId);
|
||||||
|
|
||||||
when(arbitrationManager.getDisputesAsObservableList())
|
when(arbitrationManager.getDisputesAsObservableList())
|
||||||
.thenReturn(FXCollections.observableArrayList(Collections.singleton(dispute)));
|
.thenReturn(FXCollections.observableArrayList(Set.of(dispute)));
|
||||||
|
|
||||||
|
when(arbitrationManager.getDisputedTradeIds())
|
||||||
|
.thenReturn(Set.of(tradeId));
|
||||||
|
|
||||||
when(delegate.getId()).thenReturn(tradeId);
|
when(delegate.getId()).thenReturn(tradeId);
|
||||||
|
|
||||||
|
|
Loading…
Add table
Reference in a new issue