From 00fe80ef0ed8e37b88f6ea82fd031de5afffc3c9 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sun, 6 Sep 2020 20:16:00 -0500 Subject: [PATCH 1/6] Refactor Use a stream filter instead of a if clause --- .../agent/MultipleHolderNameDetection.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/bisq/core/support/dispute/agent/MultipleHolderNameDetection.java b/core/src/main/java/bisq/core/support/dispute/agent/MultipleHolderNameDetection.java index 48025ae2669..80a64808c64 100644 --- a/core/src/main/java/bisq/core/support/dispute/agent/MultipleHolderNameDetection.java +++ b/core/src/main/java/bisq/core/support/dispute/agent/MultipleHolderNameDetection.java @@ -200,18 +200,19 @@ public void removeListener(Listener listener) { private Map> getAllDisputesByTraderMap() { Map> allDisputesByTraderMap = new HashMap<>(); - disputeManager.getDisputesAsObservableList() - .forEach(dispute -> { + disputeManager.getDisputesAsObservableList().stream() + .filter(dispute -> { Contract contract = dispute.getContract(); PaymentAccountPayload paymentAccountPayload = isBuyer(dispute) ? contract.getBuyerPaymentAccountPayload() : contract.getSellerPaymentAccountPayload(); - if (paymentAccountPayload instanceof PayloadWithHolderName) { - String traderPubKeyHash = getSigPubKeyHashAsHex(dispute); - allDisputesByTraderMap.putIfAbsent(traderPubKeyHash, new ArrayList<>()); - List disputes = allDisputesByTraderMap.get(traderPubKeyHash); - disputes.add(dispute); - } + return paymentAccountPayload instanceof PayloadWithHolderName; + }) + .forEach(dispute -> { + String traderPubKeyHash = getSigPubKeyHashAsHex(dispute); + allDisputesByTraderMap.putIfAbsent(traderPubKeyHash, new ArrayList<>()); + List disputes = allDisputesByTraderMap.get(traderPubKeyHash); + disputes.add(dispute); }); return allDisputesByTraderMap; } From a32653f79075cf2dc4d3d8a0f655d5d0ecf1f640 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sun, 6 Sep 2020 20:18:02 -0500 Subject: [PATCH 2/6] Add log in case protoOutputStream.writeEnvelope fails --- .../bisq/network/p2p/network/Connection.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/p2p/src/main/java/bisq/network/p2p/network/Connection.java b/p2p/src/main/java/bisq/network/p2p/network/Connection.java index 050978d9691..436e3d51a40 100644 --- a/p2p/src/main/java/bisq/network/p2p/network/Connection.java +++ b/p2p/src/main/java/bisq/network/p2p/network/Connection.java @@ -283,12 +283,20 @@ public void sendMessage(NetworkEnvelope networkEnvelope) { bundleSender.schedule(() -> { if (!stopped) { synchronized (lock) { - BundleOfEnvelopes current = queueOfBundles.poll(); - if (current != null && !stopped) { - if (current.getEnvelopes().size() == 1) { - protoOutputStream.writeEnvelope(current.getEnvelopes().get(0)); - } else { - protoOutputStream.writeEnvelope(current); + BundleOfEnvelopes bundle = queueOfBundles.poll(); + if (bundle != null && !stopped) { + NetworkEnvelope envelope = bundle.getEnvelopes().size() == 1 ? + bundle.getEnvelopes().get(0) : + bundle; + try { + protoOutputStream.writeEnvelope(envelope); + } catch (Throwable t) { + log.error("Sending envelope of class {} to address {} " + + "failed due {}", + envelope.getClass().getSimpleName(), + this.getPeersNodeAddressOptional(), + t.toString()); + log.error("envelope: {}", envelope); } } } From e99e6a478eebfac7b94f5b9f7bb612e44751dc61 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sun, 6 Sep 2020 20:18:50 -0500 Subject: [PATCH 3/6] Add InvalidProtocolBufferException in catch clause --- p2p/src/main/java/bisq/network/p2p/network/Connection.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/p2p/src/main/java/bisq/network/p2p/network/Connection.java b/p2p/src/main/java/bisq/network/p2p/network/Connection.java index 436e3d51a40..86aa7d4fc74 100644 --- a/p2p/src/main/java/bisq/network/p2p/network/Connection.java +++ b/p2p/src/main/java/bisq/network/p2p/network/Connection.java @@ -48,6 +48,8 @@ import bisq.common.proto.network.NetworkProtoResolver; import bisq.common.util.Utilities; +import com.google.protobuf.InvalidProtocolBufferException; + import javax.inject.Inject; import com.google.common.util.concurrent.MoreExecutors; @@ -884,7 +886,7 @@ && reportInvalidRequest(RuleViolation.WRONG_NETWORK_ID)) { log.error(e.getMessage()); e.printStackTrace(); reportInvalidRequest(RuleViolation.INVALID_CLASS); - } catch (ProtobufferException | NoClassDefFoundError e) { + } catch (ProtobufferException | NoClassDefFoundError | InvalidProtocolBufferException e) { log.error(e.getMessage()); e.printStackTrace(); reportInvalidRequest(RuleViolation.INVALID_DATA_TYPE); From 199d543c23ec26144c3e8984918cf74a95ced668 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sun, 6 Sep 2020 20:43:43 -0500 Subject: [PATCH 4/6] Remove ugly parameter The 'arePeersPaymentAccountDataBanned' method in filterManager had a parameter 'PaymentAccountFilter[] appliedPaymentAccountFilter' which was used to pass over the PaymentAccountFilter which matched a banned account. The result was only used at the fault handler (log) in the ApplyFilter task. I think this was not needed to display and if it should be done in a different way, e..g using a Consumer to pass back the value, but as the method is a validation method returning a boolean we should avoid to add side-effects like writing some property. --- .../witness/AccountAgeWitnessService.java | 7 ++---- .../java/bisq/core/filter/FilterManager.java | 25 ++++++++----------- .../trade/protocol/tasks/ApplyFilter.java | 11 +++----- .../witness/AccountAgeWitnessServiceTest.java | 2 +- 4 files changed, 17 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java index 4747e3feb48..c1a55e2bb0b 100644 --- a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java +++ b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java @@ -20,7 +20,6 @@ import bisq.core.account.sign.SignedWitness; import bisq.core.account.sign.SignedWitnessService; import bisq.core.filter.FilterManager; -import bisq.core.filter.PaymentAccountFilter; import bisq.core.locale.CurrencyUtil; import bisq.core.locale.Res; import bisq.core.offer.Offer; @@ -718,10 +717,8 @@ private boolean isNotFiltered(Dispute dispute) { filterManager.isCurrencyBanned(dispute.getContract().getOfferPayload().getCurrencyCode()) || filterManager.isPaymentMethodBanned( PaymentMethod.getPaymentMethodById(dispute.getContract().getPaymentMethodId())) || - filterManager.arePeersPaymentAccountDataBanned(dispute.getContract().getBuyerPaymentAccountPayload(), - new PaymentAccountFilter[1]) || - filterManager.arePeersPaymentAccountDataBanned(dispute.getContract().getSellerPaymentAccountPayload(), - new PaymentAccountFilter[1]) || + filterManager.arePeersPaymentAccountDataBanned(dispute.getContract().getBuyerPaymentAccountPayload()) || + filterManager.arePeersPaymentAccountDataBanned(dispute.getContract().getSellerPaymentAccountPayload()) || filterManager.isWitnessSignerPubKeyBanned( Utils.HEX.encode(dispute.getContract().getBuyerPubKeyRing().getSignaturePubKeyBytes())) || filterManager.isWitnessSignerPubKeyBanned( diff --git a/core/src/main/java/bisq/core/filter/FilterManager.java b/core/src/main/java/bisq/core/filter/FilterManager.java index 0302fc24f78..d38bd0632ee 100644 --- a/core/src/main/java/bisq/core/filter/FilterManager.java +++ b/core/src/main/java/bisq/core/filter/FilterManager.java @@ -371,24 +371,19 @@ public boolean requireUpdateToNewVersionForDAO() { return requireUpdateToNewVersion; } - public boolean arePeersPaymentAccountDataBanned(PaymentAccountPayload paymentAccountPayload, - PaymentAccountFilter[] appliedPaymentAccountFilter) { + public boolean arePeersPaymentAccountDataBanned(PaymentAccountPayload paymentAccountPayload) { return getFilter() != null && getFilter().getBannedPaymentAccounts().stream() + .filter(paymentAccountFilter -> paymentAccountFilter.getPaymentMethodId().equals( + paymentAccountPayload.getPaymentMethodId())) .anyMatch(paymentAccountFilter -> { - final boolean samePaymentMethodId = paymentAccountFilter.getPaymentMethodId().equals( - paymentAccountPayload.getPaymentMethodId()); - if (samePaymentMethodId) { - try { - Method method = paymentAccountPayload.getClass().getMethod(paymentAccountFilter.getGetMethodName()); - String result = (String) method.invoke(paymentAccountPayload); - appliedPaymentAccountFilter[0] = paymentAccountFilter; - return result.toLowerCase().equals(paymentAccountFilter.getValue().toLowerCase()); - } catch (Throwable e) { - log.error(e.getMessage()); - return false; - } - } else { + try { + Method method = paymentAccountPayload.getClass().getMethod(paymentAccountFilter.getGetMethodName()); + // We invoke getter methods (no args), e.g. getHolderName + String valueFromInvoke = (String) method.invoke(paymentAccountPayload); + return valueFromInvoke.toLowerCase().equals(paymentAccountFilter.getValue().toLowerCase()); + } catch (Throwable e) { + log.error(e.getMessage()); return false; } }); diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/ApplyFilter.java b/core/src/main/java/bisq/core/trade/protocol/tasks/ApplyFilter.java index 55e40c120fc..cbcb0ae8c22 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/ApplyFilter.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/ApplyFilter.java @@ -18,7 +18,6 @@ package bisq.core.trade.protocol.tasks; import bisq.core.filter.FilterManager; -import bisq.core.filter.PaymentAccountFilter; import bisq.core.payment.payload.PaymentAccountPayload; import bisq.core.trade.Trade; @@ -42,9 +41,8 @@ protected void run() { try { runInterceptHook(); - final NodeAddress nodeAddress = processModel.getTempTradingPeerNodeAddress(); + NodeAddress nodeAddress = processModel.getTempTradingPeerNodeAddress(); PaymentAccountPayload paymentAccountPayload = checkNotNull(processModel.getTradingPeer().getPaymentAccountPayload()); - final PaymentAccountFilter[] appliedPaymentAccountFilter = new PaymentAccountFilter[1]; FilterManager filterManager = processModel.getFilterManager(); if (nodeAddress != null && filterManager.isNodeAddressBanned(nodeAddress)) { @@ -56,13 +54,12 @@ protected void run() { } else if (trade.getOffer() != null && filterManager.isCurrencyBanned(trade.getOffer().getCurrencyCode())) { failed("Currency is banned.\n" + "Currency code=" + trade.getOffer().getCurrencyCode()); - } else if (filterManager.isPaymentMethodBanned(trade.getOffer().getPaymentMethod())) { + } else if (filterManager.isPaymentMethodBanned(checkNotNull(trade.getOffer()).getPaymentMethod())) { failed("Payment method is banned.\n" + "Payment method=" + trade.getOffer().getPaymentMethod().getId()); - } else if (filterManager.arePeersPaymentAccountDataBanned(paymentAccountPayload, appliedPaymentAccountFilter)) { + } else if (filterManager.arePeersPaymentAccountDataBanned(paymentAccountPayload)) { failed("Other trader is banned by their trading account data.\n" + - "paymentAccountPayload=" + paymentAccountPayload.getPaymentDetails() + "\n" + - "banFilter=" + appliedPaymentAccountFilter[0].toString()); + "paymentAccountPayload=" + paymentAccountPayload.getPaymentDetails()); } else if (filterManager.requireUpdateToNewVersionForTrading()) { failed("Your version of Bisq is not compatible for trading anymore. " + "Please update to the latest Bisq version at https://bisq.network/downloads."); diff --git a/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java b/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java index b37a47d057e..2b2d11e5621 100644 --- a/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java +++ b/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java @@ -218,7 +218,7 @@ public void testArbitratorSignWitness() { when(filterManager.isNodeAddressBanned(any())).thenReturn(false); when(filterManager.isCurrencyBanned(any())).thenReturn(false); when(filterManager.isPaymentMethodBanned(any())).thenReturn(false); - when(filterManager.arePeersPaymentAccountDataBanned(any(), any())).thenReturn(false); + when(filterManager.arePeersPaymentAccountDataBanned(any())).thenReturn(false); when(filterManager.isWitnessSignerPubKeyBanned(any())).thenReturn(false); when(chargeBackRisk.hasChargebackRisk(any(), any())).thenReturn(true); From 7c480babadbb57f3aee5ae06456cb808258cbd9e Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sun, 6 Sep 2020 20:46:14 -0500 Subject: [PATCH 5/6] Add missing 'git lfs pull' command at build from scratch --- docs/build.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/build.md b/docs/build.md index ca3ab064572..ea054c7936e 100644 --- a/docs/build.md +++ b/docs/build.md @@ -7,7 +7,7 @@ Bisq uses Git LFS to track certain large binary files. Follow the instructions a $ git lfs version git-lfs/2.10.0 (GitHub; darwin amd64; go 1.13.6) - + ## Clone @@ -17,8 +17,9 @@ Bisq uses Git LFS to track certain large binary files. Follow the instructions a ## Build -You do _not_ need to install Gradle to complete the following command. The `gradlew` shell script will install it for you if necessary. +You do _not_ need to install Gradle to complete the following command. The `gradlew` shell script will install it for you if necessary. Pull the lfs data first. + git lfs pull ./gradlew build If on Windows run `gradlew.bat build` instead. From a4b43f66a9447486f87fae68e6142d0526df1f9b Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sun, 6 Sep 2020 22:04:41 -0500 Subject: [PATCH 6/6] Apply codacy suggestion (this time a good one ;-)) --- core/src/main/java/bisq/core/filter/FilterManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/filter/FilterManager.java b/core/src/main/java/bisq/core/filter/FilterManager.java index d38bd0632ee..cd5bb84cf0b 100644 --- a/core/src/main/java/bisq/core/filter/FilterManager.java +++ b/core/src/main/java/bisq/core/filter/FilterManager.java @@ -381,7 +381,7 @@ public boolean arePeersPaymentAccountDataBanned(PaymentAccountPayload paymentAcc Method method = paymentAccountPayload.getClass().getMethod(paymentAccountFilter.getGetMethodName()); // We invoke getter methods (no args), e.g. getHolderName String valueFromInvoke = (String) method.invoke(paymentAccountPayload); - return valueFromInvoke.toLowerCase().equals(paymentAccountFilter.getValue().toLowerCase()); + return valueFromInvoke.equalsIgnoreCase(paymentAccountFilter.getValue()); } catch (Throwable e) { log.error(e.getMessage()); return false;