From 0656c57d393e3313050a762effefa010347da970 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 30 Oct 2020 11:07:18 -0300 Subject: [PATCH 1/5] Add null checks in PendingTradesViewModel These changes were requested in review of PR https://github.com/bisq-network/bisq/pull/4699 --- .../main/portfolio/pendingtrades/PendingTradesViewModel.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java index 6a9b4dff06c..fc08e6bc233 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java @@ -215,18 +215,22 @@ String getMarketLabel(PendingTradesListItem item) { } public String getRemainingTradeDurationAsWords() { + checkNotNull(dataModel.getTrade(), "model's trade must not be null"); return tradeUtil.getRemainingTradeDurationAsWords(dataModel.getTrade()); } public double getRemainingTradeDurationAsPercentage() { + checkNotNull(dataModel.getTrade(), "model's trade must not be null"); return tradeUtil.getRemainingTradeDurationAsPercentage(dataModel.getTrade()); } public String getDateForOpenDispute() { + checkNotNull(dataModel.getTrade(), "model's trade must not be null"); return DisplayUtils.formatDateTime(tradeUtil.getDateForOpenDispute(dataModel.getTrade())); } public boolean showWarning() { + checkNotNull(dataModel.getTrade(), "model's trade must not be null"); Date halfTradePeriodDate = tradeUtil.getHalfTradePeriodDate(dataModel.getTrade()); return halfTradePeriodDate != null && new Date().after(halfTradePeriodDate); } From f764e9feb07f1cae45309b6271ef84862ee7d8b4 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 30 Oct 2020 11:15:39 -0300 Subject: [PATCH 2/5] Simplify TradeUtil#getPaymentMethodNameWithCountryCode These changes were requested in review of PR https://github.com/bisq-network/bisq/pull/4699 --- core/src/main/java/bisq/core/trade/TradeUtil.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/TradeUtil.java b/core/src/main/java/bisq/core/trade/TradeUtil.java index cd4fd074aad..a026f6ab982 100644 --- a/core/src/main/java/bisq/core/trade/TradeUtil.java +++ b/core/src/main/java/bisq/core/trade/TradeUtil.java @@ -164,14 +164,13 @@ public String getMarketDescription(Trade trade) { } public String getPaymentMethodNameWithCountryCode(Trade trade) { - String paymentMethodDescription = ""; - if (trade != null) { - Offer offer = trade.getOffer(); - checkNotNull(offer); - checkNotNull(offer.getPaymentMethod()); - paymentMethodDescription = offer.getPaymentMethodNameWithCountryCode(); - } - return paymentMethodDescription; + if (trade == null) + return ""; + + Offer offer = trade.getOffer(); + checkNotNull(offer); + checkNotNull(offer.getPaymentMethod()); + return offer.getPaymentMethodNameWithCountryCode(); } /** From 3cd3bf0e3ed3aed626fe38ef1b1da29cbc761c61 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 30 Oct 2020 11:52:37 -0300 Subject: [PATCH 3/5] Fix grammar / typo This change resolves issue found in PR review. See https://github.com/bisq-network/bisq/pull/4703#discussion_r515076872 --- cli/src/main/java/bisq/cli/TableFormat.java | 4 ++-- cli/src/main/java/bisq/cli/TradeFormat.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/src/main/java/bisq/cli/TableFormat.java b/cli/src/main/java/bisq/cli/TableFormat.java index e9868f336e4..8336fff9ba1 100644 --- a/cli/src/main/java/bisq/cli/TableFormat.java +++ b/cli/src/main/java/bisq/cli/TableFormat.java @@ -63,7 +63,7 @@ static String formatAddressBalanceTbl(List addressBalanceInf static String formatOfferTable(List offerInfo, String fiatCurrency) { - // Some column values might be longer than header, so we need to calculated them. + // Some column values might be longer than header, so we need to calculate them. int paymentMethodColWidth = getLengthOfLongestColumn( COL_HEADER_PAYMENT_METHOD.length(), offerInfo.stream() @@ -100,7 +100,7 @@ static String formatOfferTable(List offerInfo, String fiatCurrency) { } static String formatPaymentAcctTbl(List paymentAccounts) { - // Some column values might be longer than header, so we need to calculated them. + // Some column values might be longer than header, so we need to calculate them. int nameColWidth = getLengthOfLongestColumn( COL_HEADER_NAME.length(), paymentAccounts.stream().map(PaymentAccount::getAccountName) diff --git a/cli/src/main/java/bisq/cli/TradeFormat.java b/cli/src/main/java/bisq/cli/TradeFormat.java index ab9075fe341..2a28c1dccc4 100644 --- a/cli/src/main/java/bisq/cli/TradeFormat.java +++ b/cli/src/main/java/bisq/cli/TradeFormat.java @@ -33,7 +33,7 @@ public class TradeFormat { @VisibleForTesting public static String format(TradeInfo tradeInfo) { - // Some column values might be longer than header, so we need to calculated them. + // Some column values might be longer than header, so we need to calculate them. int shortIdColWidth = Math.max(COL_HEADER_TRADE_SHORT_ID.length(), tradeInfo.getShortId().length()); int roleColWidth = Math.max(COL_HEADER_TRADE_ROLE.length(), tradeInfo.getRole().length()); From c27d5a6da87d05a0d1700c174929ad0a9af1b154 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 31 Oct 2020 13:32:14 -0300 Subject: [PATCH 4/5] Use Bisq's timer in API's 'unlockwallet timeout(s)' This change was requested in https://github.com/chimp1984/bisq/commit/961703ecea62df62946ab001ec28205662688ccd This replaces and closes PR https://github.com/bisq-network/bisq/pull/4558 --- .../bisq/core/api/CoreWalletsService.java | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/bisq/core/api/CoreWalletsService.java b/core/src/main/java/bisq/core/api/CoreWalletsService.java index 29394aad8ac..fc15ec5062a 100644 --- a/core/src/main/java/bisq/core/api/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/api/CoreWalletsService.java @@ -23,6 +23,9 @@ import bisq.core.btc.wallet.BtcWalletService; import bisq.core.btc.wallet.WalletsManager; +import bisq.common.Timer; +import bisq.common.UserThread; + import org.bitcoinj.core.Address; import org.bitcoinj.core.TransactionConfidence; import org.bitcoinj.crypto.KeyCrypterScrypt; @@ -37,8 +40,6 @@ import java.util.List; import java.util.Optional; -import java.util.Timer; -import java.util.TimerTask; import java.util.function.Function; import java.util.stream.Collectors; @@ -57,7 +58,7 @@ class CoreWalletsService { private final BtcWalletService btcWalletService; @Nullable - private TimerTask lockTask; + private Timer lockTimer; @Nullable private KeyParameter tempAesKey; @@ -190,29 +191,22 @@ void unlockWallet(String password, long timeout) { if (!walletsManager.checkAESKey(tempAesKey)) throw new IllegalStateException("incorrect password"); - if (lockTask != null) { - // The user is overriding a prior unlock timeout. Cancel the existing - // lock TimerTask to prevent it from calling lockWallet() before or after the - // new timer task does. - lockTask.cancel(); - // Avoid the synchronized(lock) overhead of an unnecessary lockTask.cancel() - // call the next time 'unlockwallet' is called. - lockTask = null; + if (lockTimer != null) { + // The user has called unlockwallet again, before the prior unlockwallet + // timeout has expired. He's overriding it with a new timeout value. + // Remove the existing lock timer to prevent it from calling lockwallet + // before or after the new one does. + lockTimer.stop(); + lockTimer = null; } - lockTask = new TimerTask() { - @Override - public void run() { - if (tempAesKey != null) { - // Do not try to lock wallet after timeout if the user has already - // done so via 'lockwallet' - log.info("Locking wallet after {} second timeout expired.", timeout); - tempAesKey = null; - } + lockTimer = UserThread.runAfter(() -> { + if (tempAesKey != null) { + // The unlockwallet timeout has expired; re-lock the wallet. + log.info("Locking wallet after {} second timeout expired.", timeout); + tempAesKey = null; } - }; - Timer timer = new Timer("Lock Wallet Timer"); - timer.schedule(lockTask, SECONDS.toMillis(timeout)); + }, timeout, SECONDS); } // Provided for automated wallet protection method testing, despite the From fcdfc687e45d1d613838d7189aa4e8edd41228e2 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 31 Oct 2020 15:44:14 -0300 Subject: [PATCH 5/5] Verify closedTradableManager.getTradableById return value is a Trade instance This change requested at https://github.com/bisq-network/bisq/pull/4711#discussion_r515520278 --- core/src/main/java/bisq/core/api/CoreTradesService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/api/CoreTradesService.java b/core/src/main/java/bisq/core/api/CoreTradesService.java index 1b58334466b..dbc6927f452 100644 --- a/core/src/main/java/bisq/core/api/CoreTradesService.java +++ b/core/src/main/java/bisq/core/api/CoreTradesService.java @@ -21,6 +21,7 @@ import bisq.core.btc.wallet.BtcWalletService; import bisq.core.offer.Offer; import bisq.core.offer.takeoffer.TakeOfferModel; +import bisq.core.trade.Tradable; import bisq.core.trade.Trade; import bisq.core.trade.TradeManager; import bisq.core.trade.TradeUtil; @@ -199,7 +200,8 @@ private Optional getOpenTrade(String tradeId) { } private Optional getClosedTrade(String tradeId) { - return closedTradableManager.getTradableById(tradeId).map(value -> (Trade) value); + Optional tradable = closedTradableManager.getTradableById(tradeId); + return tradable.filter((t) -> t instanceof Trade).map(value -> (Trade) value); } private boolean isFollowingBuyerProtocol(Trade trade) {