From d60c0dd3cab04c170856e4f817261615b0cc1bef Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 26 Feb 2021 15:25:31 -0300 Subject: [PATCH 01/35] Display buyer's cost in api's gettrade output The offer volume is shown so traders know how much fiat they are sending or receiving without having to call getoffer. Changed the 'Fiat Sent' and 'Fiat Received' column headers to show which fiat is being transfered, e.g., 'EUR Sent', 'EUR Received'. Adjusted apitest's trade-simulation-utils.sh to the modified gettrade output. --- apitest/scripts/trade-simulation-utils.sh | 12 +++---- .../java/bisq/cli/ColumnHeaderConstants.java | 10 +++--- cli/src/main/java/bisq/cli/TradeFormat.java | 33 +++++++++++++++---- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/apitest/scripts/trade-simulation-utils.sh b/apitest/scripts/trade-simulation-utils.sh index eb1d14e65e1..c9f03ac80f5 100755 --- a/apitest/scripts/trade-simulation-utils.sh +++ b/apitest/scripts/trade-simulation-utils.sh @@ -267,9 +267,9 @@ istradepaymentsent() { MAKER_OR_TAKER="$2" if [ "$MAKER_OR_TAKER" = "MAKER" ] then - ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $11}') - else ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $12}') + else + ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $13}') fi commandalert $? "Could not parse istradepaymentsent from trade detail." echo "$ANSWER" @@ -280,9 +280,9 @@ istradepaymentreceived() { MAKER_OR_TAKER="$2" if [ "$MAKER_OR_TAKER" = "MAKER" ] then - ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $12}') - else ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $13}') + else + ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $14}') fi commandalert $? "Could not parse istradepaymentreceived from trade detail." echo "$ANSWER" @@ -293,9 +293,9 @@ istradepayoutpublished() { MAKER_OR_TAKER="$2" if [ "$MAKER_OR_TAKER" = "MAKER" ] then - ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $13}') - else ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $14}') + else + ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $15}') fi commandalert $? "Could not parse istradepayoutpublished from trade detail." echo "$ANSWER" diff --git a/cli/src/main/java/bisq/cli/ColumnHeaderConstants.java b/cli/src/main/java/bisq/cli/ColumnHeaderConstants.java index e81e407d7b9..b45cd942f28 100644 --- a/cli/src/main/java/bisq/cli/ColumnHeaderConstants.java +++ b/cli/src/main/java/bisq/cli/ColumnHeaderConstants.java @@ -27,8 +27,8 @@ class ColumnHeaderConstants { // Table column header format specs, right padded with two spaces. In some cases // such as COL_HEADER_CREATION_DATE, COL_HEADER_VOLUME and COL_HEADER_UUID, the - // expected max data string length is accounted for. In others, the column header length - // are expected to be greater than any column value length. + // expected max data string length is accounted for. In others, column header + // lengths are expected to be greater than any column value length. static final String COL_HEADER_ADDRESS = padEnd("%-3s Address", 52, ' '); static final String COL_HEADER_AMOUNT = padEnd("BTC(min - max)", 24, ' '); static final String COL_HEADER_AVAILABLE_BALANCE = "Available Balance"; @@ -49,17 +49,17 @@ class ColumnHeaderConstants { static final String COL_HEADER_PAYMENT_METHOD = "Payment Method"; static final String COL_HEADER_PRICE = "Price in %-3s for 1 BTC"; static final String COL_HEADER_TRADE_AMOUNT = padStart("Amount(%-3s)", 12, ' '); + static final String COL_HEADER_TRADE_BUYER_COST = padEnd("Buyer Cost(%-3s)", 15, ' '); static final String COL_HEADER_TRADE_DEPOSIT_CONFIRMED = "Deposit Confirmed"; static final String COL_HEADER_TRADE_DEPOSIT_PUBLISHED = "Deposit Published"; - static final String COL_HEADER_TRADE_FIAT_SENT = "Fiat Sent"; - static final String COL_HEADER_TRADE_FIAT_RECEIVED = "Fiat Received"; + static final String COL_HEADER_TRADE_PAYMENT_SENT = padEnd("%-3s Sent", 8, ' '); + static final String COL_HEADER_TRADE_PAYMENT_RECEIVED = padEnd("%-3s Received", 12, ' '); static final String COL_HEADER_TRADE_PAYOUT_PUBLISHED = "Payout Published"; static final String COL_HEADER_TRADE_WITHDRAWN = "Withdrawn"; static final String COL_HEADER_TRADE_ROLE = "My Role"; static final String COL_HEADER_TRADE_SHORT_ID = "ID"; static final String COL_HEADER_TRADE_TX_FEE = "Tx Fee(%-3s)"; static final String COL_HEADER_TRADE_TAKER_FEE = "Taker Fee(%-3s)"; - static final String COL_HEADER_TRADE_WITHDRAWAL_TX_ID = "Withdrawal TX ID"; static final String COL_HEADER_TX_ID = "Tx ID"; static final String COL_HEADER_TX_INPUT_SUM = "Tx Inputs (BTC)"; diff --git a/cli/src/main/java/bisq/cli/TradeFormat.java b/cli/src/main/java/bisq/cli/TradeFormat.java index 1c286f03772..668f9603552 100644 --- a/cli/src/main/java/bisq/cli/TradeFormat.java +++ b/cli/src/main/java/bisq/cli/TradeFormat.java @@ -25,6 +25,7 @@ import static bisq.cli.ColumnHeaderConstants.*; import static bisq.cli.CurrencyFormat.formatOfferPrice; +import static bisq.cli.CurrencyFormat.formatOfferVolume; import static bisq.cli.CurrencyFormat.formatSatoshis; import static com.google.common.base.Strings.padEnd; @@ -54,18 +55,33 @@ public static String format(TradeInfo tradeInfo) { + takerFeeHeaderFormat.get() + COL_HEADER_TRADE_DEPOSIT_PUBLISHED + COL_HEADER_DELIMITER + COL_HEADER_TRADE_DEPOSIT_CONFIRMED + COL_HEADER_DELIMITER - + COL_HEADER_TRADE_FIAT_SENT + COL_HEADER_DELIMITER - + COL_HEADER_TRADE_FIAT_RECEIVED + COL_HEADER_DELIMITER + + COL_HEADER_TRADE_BUYER_COST + COL_HEADER_DELIMITER + + COL_HEADER_TRADE_PAYMENT_SENT + COL_HEADER_DELIMITER + + COL_HEADER_TRADE_PAYMENT_RECEIVED + COL_HEADER_DELIMITER + COL_HEADER_TRADE_PAYOUT_PUBLISHED + COL_HEADER_DELIMITER + COL_HEADER_TRADE_WITHDRAWN + COL_HEADER_DELIMITER + "%n"; String counterCurrencyCode = tradeInfo.getOffer().getCounterCurrencyCode(); String baseCurrencyCode = tradeInfo.getOffer().getBaseCurrencyCode(); - String headerLine = isTaker - ? String.format(headersFormat, counterCurrencyCode, baseCurrencyCode, baseCurrencyCode, baseCurrencyCode) - : String.format(headersFormat, counterCurrencyCode, baseCurrencyCode, baseCurrencyCode); + // The taker's output contains an extra taker tx fee column. + String headerLine = isTaker + ? String.format(headersFormat, + /* COL_HEADER_PRICE */ counterCurrencyCode, + /* COL_HEADER_TRADE_AMOUNT */ baseCurrencyCode, + /* COL_HEADER_TRADE_TX_FEE */ baseCurrencyCode, + /* COL_HEADER_TRADE_TAKER_FEE */ baseCurrencyCode, + /* COL_HEADER_TRADE_BUYER_COST */ counterCurrencyCode, + /* COL_HEADER_TRADE_PAYMENT_SENT */ counterCurrencyCode, + /* COL_HEADER_TRADE_PAYMENT_RECEIVED */ counterCurrencyCode) + : String.format(headersFormat, + /* COL_HEADER_PRICE */ counterCurrencyCode, + /* COL_HEADER_TRADE_AMOUNT */ baseCurrencyCode, + /* COL_HEADER_TRADE_TX_FEE */ baseCurrencyCode, + /* COL_HEADER_TRADE_BUYER_COST */ counterCurrencyCode, + /* COL_HEADER_TRADE_PAYMENT_SENT */ counterCurrencyCode, + /* COL_HEADER_TRADE_PAYMENT_RECEIVED */ counterCurrencyCode); String colDataFormat = "%-" + shortIdColWidth + "s" // lt justify + " %-" + (roleColWidth + COL_HEADER_DELIMITER.length()) + "s" // left @@ -75,8 +91,9 @@ public static String format(TradeInfo tradeInfo) { + takerFeeHeader.get() // rt justify + " %-" + COL_HEADER_TRADE_DEPOSIT_PUBLISHED.length() + "s" // lt justify + " %-" + COL_HEADER_TRADE_DEPOSIT_CONFIRMED.length() + "s" // lt justify - + " %-" + COL_HEADER_TRADE_FIAT_SENT.length() + "s" // lt justify - + " %-" + COL_HEADER_TRADE_FIAT_RECEIVED.length() + "s" // lt justify + + "%" + (COL_HEADER_TRADE_BUYER_COST.length() + 1) + "s" // rt justify + + " %-" + (COL_HEADER_TRADE_PAYMENT_SENT.length() - 1) + "s" // left + + " %-" + (COL_HEADER_TRADE_PAYMENT_RECEIVED.length() - 1) + "s" // left + " %-" + COL_HEADER_TRADE_PAYOUT_PUBLISHED.length() + "s" // lt justify + " %-" + COL_HEADER_TRADE_WITHDRAWN.length() + "s"; // lt justify @@ -95,6 +112,7 @@ private static String formatTradeForMaker(String format, TradeInfo tradeInfo) { formatSatoshis(tradeInfo.getTxFeeAsLong()), tradeInfo.getIsDepositPublished() ? "YES" : "NO", tradeInfo.getIsDepositConfirmed() ? "YES" : "NO", + formatOfferVolume(tradeInfo.getOffer().getVolume()), tradeInfo.getIsFiatSent() ? "YES" : "NO", tradeInfo.getIsFiatReceived() ? "YES" : "NO", tradeInfo.getIsPayoutPublished() ? "YES" : "NO", @@ -111,6 +129,7 @@ private static String formatTradeForTaker(String format, TradeInfo tradeInfo) { formatSatoshis(tradeInfo.getTakerFeeAsLong()), tradeInfo.getIsDepositPublished() ? "YES" : "NO", tradeInfo.getIsDepositConfirmed() ? "YES" : "NO", + formatOfferVolume(tradeInfo.getOffer().getVolume()), tradeInfo.getIsFiatSent() ? "YES" : "NO", tradeInfo.getIsFiatReceived() ? "YES" : "NO", tradeInfo.getIsPayoutPublished() ? "YES" : "NO", From e5291e9f45319f53f745271070bad0d92bd1af03 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 17:09:54 -0300 Subject: [PATCH 02/35] Use the logger of the gRPC service throwing an exception --- .../daemon/grpc/GrpcDisputeAgentsService.java | 2 +- .../daemon/grpc/GrpcExceptionHandler.java | 5 ++-- .../grpc/GrpcGetTradeStatisticsService.java | 5 +++- .../bisq/daemon/grpc/GrpcHelpService.java | 2 +- .../bisq/daemon/grpc/GrpcOffersService.java | 12 ++++---- .../grpc/GrpcPaymentAccountsService.java | 12 ++++---- .../bisq/daemon/grpc/GrpcPriceService.java | 2 +- .../bisq/daemon/grpc/GrpcShutdownService.java | 2 +- .../bisq/daemon/grpc/GrpcTradesService.java | 12 ++++---- .../bisq/daemon/grpc/GrpcVersionService.java | 2 +- .../bisq/daemon/grpc/GrpcWalletsService.java | 28 +++++++++---------- 11 files changed, 44 insertions(+), 40 deletions(-) diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java index dfc7f7406a5..11f4a63f3fe 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java @@ -45,7 +45,7 @@ public void registerDisputeAgent(RegisterDisputeAgentRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java index 8a1c4c2836e..38665474e95 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java @@ -26,7 +26,7 @@ import java.util.function.Predicate; -import lombok.extern.slf4j.Slf4j; +import org.slf4j.Logger; import static io.grpc.Status.INVALID_ARGUMENT; import static io.grpc.Status.UNKNOWN; @@ -37,7 +37,6 @@ * An unexpected Throwable's message will be replaced with an 'unexpected' error message. */ @Singleton -@Slf4j class GrpcExceptionHandler { private final Predicate isExpectedException = (t) -> @@ -47,7 +46,7 @@ class GrpcExceptionHandler { public GrpcExceptionHandler() { } - public void handleException(Throwable t, StreamObserver responseObserver) { + public void handleException(Logger log, Throwable t, StreamObserver responseObserver) { // Log the core api error (this is last chance to do that), wrap it in a new // gRPC StatusRuntimeException, then send it to the client in the gRPC response. log.error("", t); diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java index 2fe205011b0..3ef71627bed 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java @@ -16,6 +16,8 @@ import java.util.Optional; import java.util.stream.Collectors; +import lombok.extern.slf4j.Slf4j; + import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; import static java.util.concurrent.TimeUnit.SECONDS; @@ -24,6 +26,7 @@ import bisq.daemon.grpc.interceptor.CallRateMeteringInterceptor; import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; +@Slf4j class GrpcGetTradeStatisticsService extends GetTradeStatisticsGrpc.GetTradeStatisticsImplBase { private final CoreApi coreApi; @@ -47,7 +50,7 @@ public void getTradeStatistics(GetTradeStatisticsRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcHelpService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcHelpService.java index 60cd051ae5a..10fae5cf826 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcHelpService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcHelpService.java @@ -62,7 +62,7 @@ public void getMethodHelp(GetMethodHelpRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java index 784a366865b..c6c93430e5c 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java @@ -81,7 +81,7 @@ public void getOffer(GetOfferRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -97,7 +97,7 @@ public void getMyOffer(GetMyOfferRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -116,7 +116,7 @@ public void getOffers(GetOffersRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -135,7 +135,7 @@ public void getMyOffers(GetMyOffersRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -166,7 +166,7 @@ public void createOffer(CreateOfferRequest req, responseObserver.onCompleted(); }); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -179,7 +179,7 @@ public void cancelOffer(CancelOfferRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java index d0d0c31cfa5..90b031d4904 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java @@ -40,6 +40,8 @@ import java.util.Optional; import java.util.stream.Collectors; +import lombok.extern.slf4j.Slf4j; + import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; @@ -49,7 +51,7 @@ import bisq.daemon.grpc.interceptor.CallRateMeteringInterceptor; import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; - +@Slf4j class GrpcPaymentAccountsService extends PaymentAccountsGrpc.PaymentAccountsImplBase { private final CoreApi coreApi; @@ -72,7 +74,7 @@ public void createPaymentAccount(CreatePaymentAccountRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -88,7 +90,7 @@ public void getPaymentAccounts(GetPaymentAccountsRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -104,7 +106,7 @@ public void getPaymentMethods(GetPaymentMethodsRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -119,7 +121,7 @@ public void getPaymentAccountForm(GetPaymentAccountFormRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java index 55a0ebe2d0e..256136c35ec 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java @@ -64,7 +64,7 @@ public void getMarketPrice(MarketPriceRequest req, responseObserver.onCompleted(); }); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcShutdownService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcShutdownService.java index 3ab445f9a5e..f7bf00de7e0 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcShutdownService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcShutdownService.java @@ -53,7 +53,7 @@ public void stop(StopRequest req, responseObserver.onCompleted(); UserThread.runAfter(BisqHeadlessApp.getShutDownHandler(), 500, MILLISECONDS); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index f6bcd3d97bd..7e43aacab12 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -79,7 +79,7 @@ public void getTrade(GetTradeRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -99,7 +99,7 @@ public void takeOffer(TakeOfferRequest req, responseObserver.onCompleted(); }); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -112,7 +112,7 @@ public void confirmPaymentStarted(ConfirmPaymentStartedRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -125,7 +125,7 @@ public void confirmPaymentReceived(ConfirmPaymentReceivedRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -138,7 +138,7 @@ public void keepFunds(KeepFundsRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -151,7 +151,7 @@ public void withdrawFunds(WithdrawFundsRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java index 24c114bc03f..e585b62d617 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java @@ -63,7 +63,7 @@ public void getVersion(GetVersionRequest req, StreamObserver re responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java index 448957432cb..a183aca48b6 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java @@ -103,7 +103,7 @@ public void getBalances(GetBalancesRequest req, StreamObserver responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -117,7 +117,7 @@ public void getAddressBalance(GetAddressBalanceRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -135,7 +135,7 @@ public void getFundingAddresses(GetFundingAddressesRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -150,7 +150,7 @@ public void getUnusedBsqAddress(GetUnusedBsqAddressRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -182,7 +182,7 @@ public void onFailure(TxBroadcastException ex) { } }); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -220,7 +220,7 @@ public void onFailure(@NotNull Throwable t) { } }); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -237,7 +237,7 @@ public void getTxFeeRate(GetTxFeeRateRequest req, responseObserver.onCompleted(); }); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -254,7 +254,7 @@ public void setTxFeeRatePreference(SetTxFeeRatePreferenceRequest req, responseObserver.onCompleted(); }); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -271,7 +271,7 @@ public void unsetTxFeeRatePreference(UnsetTxFeeRatePreferenceRequest req, responseObserver.onCompleted(); }); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -286,7 +286,7 @@ public void getTransaction(GetTransactionRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -299,7 +299,7 @@ public void setWalletPassword(SetWalletPasswordRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -312,7 +312,7 @@ public void removeWalletPassword(RemoveWalletPasswordRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -325,7 +325,7 @@ public void lockWallet(LockWalletRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } @@ -338,7 +338,7 @@ public void unlockWallet(UnlockWalletRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { - exceptionHandler.handleException(cause, responseObserver); + exceptionHandler.handleException(log, cause, responseObserver); } } From e5a0a3998dbc44bfada055f3867634c84e4bede0 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 17:14:22 -0300 Subject: [PATCH 03/35] Permit some gRPC excptions to be logged only as warning A new handleExceptionAsWarning method logs warn(ex.msg) instead of the full stack trace. --- .../bisq/daemon/grpc/GrpcExceptionHandler.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java index 38665474e95..5ab763aafa5 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java @@ -46,7 +46,9 @@ class GrpcExceptionHandler { public GrpcExceptionHandler() { } - public void handleException(Logger log, Throwable t, StreamObserver responseObserver) { + public void handleException(Logger log, + Throwable t, + StreamObserver responseObserver) { // Log the core api error (this is last chance to do that), wrap it in a new // gRPC StatusRuntimeException, then send it to the client in the gRPC response. log.error("", t); @@ -55,6 +57,17 @@ public void handleException(Logger log, Throwable t, StreamObserver responseO throw grpcStatusRuntimeException; } + public void handleExceptionAsWarning(Logger log, + String calledMethod, + Throwable t, + StreamObserver responseObserver) { + // Just log a warning instead of an error with full stack trace. + log.warn(calledMethod + " -> " + t.getMessage()); + var grpcStatusRuntimeException = wrapException(t); + responseObserver.onError(grpcStatusRuntimeException); + throw grpcStatusRuntimeException; + } + private StatusRuntimeException wrapException(Throwable t) { // We want to be careful about what kinds of exception messages we send to the // client. Expected core exceptions should be wrapped in an IllegalStateException From 320e63c0a135a729b0a16d58208649c9c4d8324a Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 17:25:49 -0300 Subject: [PATCH 04/35] Log 'trade not found' a warning instead of full stack trace --- .../main/java/bisq/daemon/grpc/GrpcExceptionHandler.java | 6 +++--- .../src/main/java/bisq/daemon/grpc/GrpcTradesService.java | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java index 5ab763aafa5..a99b824808e 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java @@ -58,9 +58,9 @@ public void handleException(Logger log, } public void handleExceptionAsWarning(Logger log, - String calledMethod, - Throwable t, - StreamObserver responseObserver) { + String calledMethod, + Throwable t, + StreamObserver responseObserver) { // Just log a warning instead of an error with full stack trace. log.warn(calledMethod + " -> " + t.getMessage()); var grpcStatusRuntimeException = wrapException(t); diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index 7e43aacab12..d757a22288e 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -78,6 +78,10 @@ public void getTrade(GetTradeRequest req, .build(); responseObserver.onNext(reply); responseObserver.onCompleted(); + } catch (IllegalArgumentException cause) { + // Offer makers may call 'gettrade' many times before a trade exists. + // Log a 'trade not found' warning instead of a full stack trace. + exceptionHandler.handleExceptionAsWarning(log, "getTrade", cause, responseObserver); } catch (Throwable cause) { exceptionHandler.handleException(log, cause, responseObserver); } From 98ff6cf9efd7e6b971f97bea4e0a578f9765fef7 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 18:01:45 -0300 Subject: [PATCH 05/35] Fix test bug --- .../bisq/apitest/method/CallRateMeteringInterceptorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java b/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java index b73806ca57d..1e5c39d3f76 100644 --- a/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java +++ b/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java @@ -116,7 +116,7 @@ public static File buildInterceptorConfigFile() { "createOffer", 5, MINUTES); - builder.addCallRateMeter("GrpcOffersService", + builder.addCallRateMeter("GrpcTradesService", "takeOffer", 10, DAYS); From e8d1f03792e418903a0f9881e6b9cd8620c6e170 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 18:10:06 -0300 Subject: [PATCH 06/35] Clean up call rate meter config file in test teardown --- .../method/CallRateMeteringInterceptorTest.java | 11 ++++++++++- .../test/java/bisq/apitest/scenario/StartupTest.java | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java b/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java index 1e5c39d3f76..ee5e91e1393 100644 --- a/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java +++ b/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java @@ -20,6 +20,7 @@ import io.grpc.StatusRuntimeException; import java.io.File; +import java.io.IOException; import lombok.extern.slf4j.Slf4j; @@ -34,6 +35,7 @@ import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind; import static bisq.apitest.config.BisqAppConfig.alicedaemon; +import static bisq.common.file.FileUtil.deleteFileIfExists; import static java.util.concurrent.TimeUnit.DAYS; import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.MINUTES; @@ -53,9 +55,11 @@ public class CallRateMeteringInterceptorTest extends MethodTest { private static final GetVersionTest getVersionTest = new GetVersionTest(); + private static File callRateMeteringConfigFile; + @BeforeAll public static void setUp() { - File callRateMeteringConfigFile = buildInterceptorConfigFile(); + callRateMeteringConfigFile = buildInterceptorConfigFile(); startSupportingApps(callRateMeteringConfigFile, false, false, @@ -98,6 +102,11 @@ public void testGetVersionCall4IsAllowed() { @AfterAll public static void tearDown() { + try { + deleteFileIfExists(callRateMeteringConfigFile); + } catch (IOException ex) { + log.error(ex.getMessage()); + } tearDownScaffold(); } diff --git a/apitest/src/test/java/bisq/apitest/scenario/StartupTest.java b/apitest/src/test/java/bisq/apitest/scenario/StartupTest.java index 3010a6568da..8c31b3630c5 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/StartupTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/StartupTest.java @@ -18,6 +18,7 @@ package bisq.apitest.scenario; import java.io.File; +import java.io.IOException; import lombok.extern.slf4j.Slf4j; @@ -33,6 +34,7 @@ import static bisq.apitest.config.BisqAppConfig.arbdaemon; import static bisq.apitest.config.BisqAppConfig.seednode; import static bisq.apitest.method.CallRateMeteringInterceptorTest.buildInterceptorConfigFile; +import static bisq.common.file.FileUtil.deleteFileIfExists; import static org.junit.jupiter.api.Assertions.fail; @@ -48,10 +50,12 @@ @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class StartupTest extends MethodTest { + private static File callRateMeteringConfigFile; + @BeforeAll public static void setUp() { try { - File callRateMeteringConfigFile = buildInterceptorConfigFile(); + callRateMeteringConfigFile = buildInterceptorConfigFile(); startSupportingApps(callRateMeteringConfigFile, false, false, @@ -102,6 +106,11 @@ public void testGetCreateOfferHelp() { @AfterAll public static void tearDown() { + try { + deleteFileIfExists(callRateMeteringConfigFile); + } catch (IOException ex) { + log.error(ex.getMessage()); + } tearDownScaffold(); } } From f90d2cee57e669d882b49d9122183c996d06d642 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 18:24:57 -0300 Subject: [PATCH 07/35] Fix test bug --- .../grpc/interceptor/GrpcServiceRateMeteringConfigTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/src/test/java/bisq/daemon/grpc/interceptor/GrpcServiceRateMeteringConfigTest.java b/daemon/src/test/java/bisq/daemon/grpc/interceptor/GrpcServiceRateMeteringConfigTest.java index a6b635eaab3..2b1044e4df1 100644 --- a/daemon/src/test/java/bisq/daemon/grpc/interceptor/GrpcServiceRateMeteringConfigTest.java +++ b/daemon/src/test/java/bisq/daemon/grpc/interceptor/GrpcServiceRateMeteringConfigTest.java @@ -74,7 +74,7 @@ public static void setup() { "createOffer", 5, MINUTES); - builder.addCallRateMeter("GrpcOffersService", + builder.addCallRateMeter("GrpcTradesService", "takeOffer", 10, DAYS); From 6b2c386a7c8e6fa8d3f28b5d1fdbf8d680176109 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 21:47:52 -0300 Subject: [PATCH 08/35] Fix call rate metering interceptor bug The gRPC interceptor was not using the correct method/ratemeter map key, and failing to find a rate meter for the server call. - Fix the rate meter key lookup bug. - Disable most strict, default call rate meters in tests. Made an adjustment to the test harness' scaffold setup so an interceptor testing or disabling config file is always picked up by bob and alice daemons. - Set arbitration daemon's registerDisputeAgent rate @ 10/second, so it does not interfere with the test harness. (There is no pre-existing arb node appDataDir before that daemon starts.) Note: The test harness cannot install the custom rate metering file in an arb daemon's appDataDir before it starts because there is no dao-setup file for that node type. TODO: Adjust api simulation scripts to interceptor bug fix. --- .../test/java/bisq/apitest/ApiTestCase.java | 48 +++++++++++++++--- .../CallRateMeteringInterceptorTest.java | 50 +------------------ .../java/bisq/apitest/method/MethodTest.java | 4 ++ .../bisq/apitest/scenario/StartupTest.java | 3 +- .../daemon/grpc/GrpcDisputeAgentsService.java | 6 +-- .../CallRateMeteringInterceptor.java | 17 ++++--- 6 files changed, 62 insertions(+), 66 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/ApiTestCase.java b/apitest/src/test/java/bisq/apitest/ApiTestCase.java index fea32d2e75d..dbdbfda7634 100644 --- a/apitest/src/test/java/bisq/apitest/ApiTestCase.java +++ b/apitest/src/test/java/bisq/apitest/ApiTestCase.java @@ -17,11 +17,14 @@ package bisq.apitest; +import java.io.File; import java.io.IOException; import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; +import lombok.extern.slf4j.Slf4j; + import javax.annotation.Nullable; import org.junit.jupiter.api.TestInfo; @@ -29,15 +32,19 @@ import static bisq.apitest.config.BisqAppConfig.alicedaemon; import static bisq.apitest.config.BisqAppConfig.arbdaemon; import static bisq.apitest.config.BisqAppConfig.bobdaemon; +import static bisq.common.file.FileUtil.deleteFileIfExists; import static java.net.InetAddress.getLoopbackAddress; import static java.util.Arrays.stream; import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; import bisq.apitest.config.ApiTestConfig; import bisq.apitest.method.BitcoinCliHelper; import bisq.cli.GrpcClient; +import bisq.daemon.grpc.GrpcVersionService; +import bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig; /** * Base class for all test types: 'method', 'scenario' and 'e2e'. @@ -64,6 +71,7 @@ *

* Initial Bob balances & accounts: 10.0 BTC, 1500000.00 BSQ, USD PerfectMoney dummy */ +@Slf4j public class ApiTestCase { protected static Scaffold scaffold; @@ -79,12 +87,12 @@ public class ApiTestCase { public static void setUpScaffold(Enum... supportingApps) throws InterruptedException, ExecutionException, IOException { - scaffold = new Scaffold(stream(supportingApps).map(Enum::name) - .collect(Collectors.joining(","))) - .setUp(); - config = scaffold.config; - bitcoinCli = new BitcoinCliHelper((config)); - createGrpcClients(); + String[] params = new String[]{ + "--supportingApps", stream(supportingApps).map(Enum::name).collect(Collectors.joining(",")), + "--callRateMeteringConfigPath", defaultRateMeterInterceptorConfig().getAbsolutePath(), + "--enableBisqDebugging", "false" + }; + setUpScaffold(params); } public static void setUpScaffold(String[] params) @@ -139,4 +147,32 @@ protected final String testName(TestInfo testInfo) { ? testInfo.getTestMethod().get().getName() : "unknown test name"; } + + protected static File defaultRateMeterInterceptorConfig() { + GrpcServiceRateMeteringConfig.Builder builder = new GrpcServiceRateMeteringConfig.Builder(); + builder.addCallRateMeter(GrpcVersionService.class.getSimpleName(), + "getVersion", + 1, + SECONDS); + // Only GrpcVersionService is @VisibleForTesting, so we hardcode the class names. + builder.addCallRateMeter("GrpcDisputeAgentsService", + "registerDisputeAgent", + 6, // Allow 6/second for test harness setup + tests. + SECONDS); + String[] serviceClassNames = new String[]{ + "GrpcGetTradeStatisticsService", + "GrpcHelpService", + "GrpcOffersService", + "GrpcPaymentAccountsService", + "GrpcPriceService", + "GrpcTradesService", + "GrpcWalletsService" + }; + for (String service : serviceClassNames) { + builder.addCallRateMeter(service, "disabled", 1, MILLISECONDS); + } + File file = builder.build(); + file.deleteOnExit(); + return file; + } } diff --git a/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java b/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java index ee5e91e1393..f7a076a9f96 100644 --- a/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java +++ b/apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java @@ -19,9 +19,6 @@ import io.grpc.StatusRuntimeException; -import java.io.File; -import java.io.IOException; - import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.AfterAll; @@ -35,19 +32,9 @@ import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind; import static bisq.apitest.config.BisqAppConfig.alicedaemon; -import static bisq.common.file.FileUtil.deleteFileIfExists; -import static java.util.concurrent.TimeUnit.DAYS; -import static java.util.concurrent.TimeUnit.HOURS; -import static java.util.concurrent.TimeUnit.MINUTES; -import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; - - -import bisq.daemon.grpc.GrpcVersionService; -import bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig; - @Disabled @Slf4j @TestMethodOrder(MethodOrderer.OrderAnnotation.class) @@ -55,13 +42,9 @@ public class CallRateMeteringInterceptorTest extends MethodTest { private static final GetVersionTest getVersionTest = new GetVersionTest(); - private static File callRateMeteringConfigFile; - @BeforeAll public static void setUp() { - callRateMeteringConfigFile = buildInterceptorConfigFile(); - startSupportingApps(callRateMeteringConfigFile, - false, + startSupportingApps(false, false, bitcoind, alicedaemon); } @@ -102,37 +85,6 @@ public void testGetVersionCall4IsAllowed() { @AfterAll public static void tearDown() { - try { - deleteFileIfExists(callRateMeteringConfigFile); - } catch (IOException ex) { - log.error(ex.getMessage()); - } tearDownScaffold(); } - - public static File buildInterceptorConfigFile() { - GrpcServiceRateMeteringConfig.Builder builder = new GrpcServiceRateMeteringConfig.Builder(); - builder.addCallRateMeter(GrpcVersionService.class.getSimpleName(), - "getVersion", - 1, - SECONDS); - builder.addCallRateMeter(GrpcVersionService.class.getSimpleName(), - "shouldNotBreakAnything", - 1000, - DAYS); - // Only GrpcVersionService is @VisibleForTesting, so we hardcode the class names. - builder.addCallRateMeter("GrpcOffersService", - "createOffer", - 5, - MINUTES); - builder.addCallRateMeter("GrpcTradesService", - "takeOffer", - 10, - DAYS); - builder.addCallRateMeter("GrpcTradesService", - "withdrawFunds", - 3, - HOURS); - return builder.build(); - } } diff --git a/apitest/src/test/java/bisq/apitest/method/MethodTest.java b/apitest/src/test/java/bisq/apitest/method/MethodTest.java index 84f5eed6941..f0634fb2c44 100644 --- a/apitest/src/test/java/bisq/apitest/method/MethodTest.java +++ b/apitest/src/test/java/bisq/apitest/method/MethodTest.java @@ -71,8 +71,11 @@ public static void startSupportingApps(boolean registerDisputeAgents, boolean generateBtcBlock, Enum... supportingApps) { try { + // Disable call rate metering where there is no callRateMeteringConfigFile. + File callRateMeteringConfigFile = defaultRateMeterInterceptorConfig(); setUpScaffold(new String[]{ "--supportingApps", toNameList.apply(supportingApps), + "--callRateMeteringConfigPath", callRateMeteringConfigFile.getAbsolutePath(), "--enableBisqDebugging", "false" }); doPostStartup(registerDisputeAgents, generateBtcBlock); @@ -136,6 +139,7 @@ protected final bisq.core.payment.PaymentAccount createPaymentAccount(GrpcClient protected static void registerDisputeAgents() { arbClient.registerDisputeAgent(MEDIATOR, DEV_PRIVILEGE_PRIV_KEY); + sleep(1001); // Can call registerdisputeagent 1x per second. arbClient.registerDisputeAgent(REFUND_AGENT, DEV_PRIVILEGE_PRIV_KEY); } diff --git a/apitest/src/test/java/bisq/apitest/scenario/StartupTest.java b/apitest/src/test/java/bisq/apitest/scenario/StartupTest.java index 8c31b3630c5..8aa93675119 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/StartupTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/StartupTest.java @@ -33,7 +33,6 @@ import static bisq.apitest.config.BisqAppConfig.alicedaemon; import static bisq.apitest.config.BisqAppConfig.arbdaemon; import static bisq.apitest.config.BisqAppConfig.seednode; -import static bisq.apitest.method.CallRateMeteringInterceptorTest.buildInterceptorConfigFile; import static bisq.common.file.FileUtil.deleteFileIfExists; import static org.junit.jupiter.api.Assertions.fail; @@ -55,7 +54,7 @@ public class StartupTest extends MethodTest { @BeforeAll public static void setUp() { try { - callRateMeteringConfigFile = buildInterceptorConfigFile(); + callRateMeteringConfigFile = defaultRateMeterInterceptorConfig(); startSupportingApps(callRateMeteringConfigFile, false, false, diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java index 11f4a63f3fe..60413c1c962 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java @@ -59,9 +59,9 @@ final Optional rateMeteringInterceptor() { return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass()) .or(() -> Optional.of(CallRateMeteringInterceptor.valueOf( new HashMap<>() {{ - // You can only register mainnet dispute agents in the UI. - // Do not limit devs' ability to register test agents. - put("registerDisputeAgent", new GrpcCallRateMeter(1, SECONDS)); + // Do not limit devs' ability to test agent registration + // and call validation in regtest arbitration daemons. + put("registerDisputeAgent", new GrpcCallRateMeter(10, SECONDS)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java b/daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java index 429ed1e22c7..404c093a2f2 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java +++ b/daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java @@ -27,7 +27,6 @@ import java.util.HashMap; import java.util.Map; -import java.util.Objects; import java.util.Optional; import lombok.extern.slf4j.Slf4j; @@ -35,6 +34,7 @@ import static io.grpc.Status.PERMISSION_DENIED; import static java.lang.String.format; import static java.util.stream.Collectors.joining; +import static org.apache.commons.lang3.StringUtils.uncapitalize; @Slf4j public final class CallRateMeteringInterceptor implements ServerInterceptor { @@ -106,11 +106,16 @@ private Optional> getRateMeterKV(ServerCall } private String getRateMeterKey(ServerCall serverCall) { - // Get the rate meter map key from the full rpc service name. The key name - // is hard coded in the Grpc*Service interceptors() method. - String fullServiceName = serverCall.getMethodDescriptor().getServiceName(); - return StringUtils.uncapitalize(Objects.requireNonNull(fullServiceName) - .substring("io.bisq.protobuffer.".length())); + // Get the rate meter map key from the server call method descriptor. The + // returned key (e.g., 'createOffer') is defined in the 'serviceCallRateMeters' + // constructor argument. It is extracted from the gRPC fullMethodName, e.g., + // 'io.bisq.protobuffer.Offers/CreateOffer'. + String fullServiceMethodName = serverCall.getMethodDescriptor().getFullMethodName(); + if (fullServiceMethodName.contains("/")) + return uncapitalize(fullServiceMethodName.split("/")[1]); + else + throw new IllegalStateException("Could not extract rate meter key from " + + fullServiceMethodName + "."); } @Override From 675ce9813efa5b53f46f1545482dd8ae7a0d648d Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 21:56:19 -0300 Subject: [PATCH 09/35] Make test call rate = default call rate --- apitest/src/test/java/bisq/apitest/ApiTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apitest/src/test/java/bisq/apitest/ApiTestCase.java b/apitest/src/test/java/bisq/apitest/ApiTestCase.java index dbdbfda7634..e4377ea0e22 100644 --- a/apitest/src/test/java/bisq/apitest/ApiTestCase.java +++ b/apitest/src/test/java/bisq/apitest/ApiTestCase.java @@ -157,7 +157,7 @@ protected static File defaultRateMeterInterceptorConfig() { // Only GrpcVersionService is @VisibleForTesting, so we hardcode the class names. builder.addCallRateMeter("GrpcDisputeAgentsService", "registerDisputeAgent", - 6, // Allow 6/second for test harness setup + tests. + 10, // Same as default. SECONDS); String[] serviceClassNames = new String[]{ "GrpcGetTradeStatisticsService", From 724950926cac70e282ea4fb48bf4aaa4beb39cfe Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 21:57:53 -0300 Subject: [PATCH 10/35] No need to wait, default+test call rate > 2x / second --- apitest/src/test/java/bisq/apitest/method/MethodTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/apitest/src/test/java/bisq/apitest/method/MethodTest.java b/apitest/src/test/java/bisq/apitest/method/MethodTest.java index f0634fb2c44..26765f6ccb1 100644 --- a/apitest/src/test/java/bisq/apitest/method/MethodTest.java +++ b/apitest/src/test/java/bisq/apitest/method/MethodTest.java @@ -139,7 +139,6 @@ protected final bisq.core.payment.PaymentAccount createPaymentAccount(GrpcClient protected static void registerDisputeAgents() { arbClient.registerDisputeAgent(MEDIATOR, DEV_PRIVILEGE_PRIV_KEY); - sleep(1001); // Can call registerdisputeagent 1x per second. arbClient.registerDisputeAgent(REFUND_AGENT, DEV_PRIVILEGE_PRIV_KEY); } From 3feacf4580d2f50645e535642af928846b63ee71 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 22:25:49 -0300 Subject: [PATCH 11/35] Remove unused import --- apitest/src/test/java/bisq/apitest/ApiTestCase.java | 1 - 1 file changed, 1 deletion(-) diff --git a/apitest/src/test/java/bisq/apitest/ApiTestCase.java b/apitest/src/test/java/bisq/apitest/ApiTestCase.java index e4377ea0e22..5ef101fe31c 100644 --- a/apitest/src/test/java/bisq/apitest/ApiTestCase.java +++ b/apitest/src/test/java/bisq/apitest/ApiTestCase.java @@ -32,7 +32,6 @@ import static bisq.apitest.config.BisqAppConfig.alicedaemon; import static bisq.apitest.config.BisqAppConfig.arbdaemon; import static bisq.apitest.config.BisqAppConfig.bobdaemon; -import static bisq.common.file.FileUtil.deleteFileIfExists; import static java.net.InetAddress.getLoopbackAddress; import static java.util.Arrays.stream; import static java.util.concurrent.TimeUnit.MILLISECONDS; From 3bbefffb9c510b455cc4716819c7160383005134 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 22:38:38 -0300 Subject: [PATCH 12/35] Adjust mainnet bats test to default rate meter interceptors --- apitest/scripts/mainnet-test.sh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/apitest/scripts/mainnet-test.sh b/apitest/scripts/mainnet-test.sh index d643ee716e0..a8c3132b15e 100755 --- a/apitest/scripts/mainnet-test.sh +++ b/apitest/scripts/mainnet-test.sh @@ -53,6 +53,8 @@ } @test "test getversion" { + # Wait 1 second before calling getversion again. + sleep 1 load 'version-parser' run ./bisq-cli --password=xyz getversion [ "$status" -eq 0 ] @@ -118,6 +120,8 @@ } @test "test setwalletpassword oldpwd newpwd" { + # Wait 5 seconds before calling setwalletpassword again. + sleep 5 run ./bisq-cli --password=xyz setwalletpassword --wallet-password="a b c" --new-wallet-password="d e f" [ "$status" -eq 0 ] echo "actual output: $output" >&2 @@ -163,6 +167,8 @@ } @test "test getaddressbalance bogus address argument" { + # Wait 1 second before calling getaddressbalance again. + sleep 1 run ./bisq-cli --password=xyz getaddressbalance --address=bogus [ "$status" -eq 1 ] echo "actual output: $output" >&2 @@ -187,16 +193,22 @@ } @test "test getoffers sell eur check return status" { + # Wait 1 second before calling getoffers again. + sleep 1 run ./bisq-cli --password=xyz getoffers --direction=sell --currency-code=eur [ "$status" -eq 0 ] } @test "test getoffers buy eur check return status" { + # Wait 1 second before calling getoffers again. + sleep 1 run ./bisq-cli --password=xyz getoffers --direction=buy --currency-code=eur [ "$status" -eq 0 ] } @test "test getoffers sell gbp check return status" { + # Wait 1 second before calling getoffers again. + sleep 1 run ./bisq-cli --password=xyz getoffers --direction=sell --currency-code=gbp [ "$status" -eq 0 ] } @@ -216,3 +228,9 @@ [ "${lines[1]}" = "Usage: bisq-cli [options] [params]" ] # TODO add asserts after help text is modified for new endpoints } + +@test "test takeoffer method --help" { + run ./bisq-cli --password=xyz takeoffer --help + [ "$status" -eq 0 ] + [ "${lines[0]}" = "takeoffer" ] +} From b618776b1b562c9de4e3002c2b9306e53d5b2abb Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 22:50:14 -0300 Subject: [PATCH 13/35] Wait 3 secs after removing password (for wallet save) --- apitest/scripts/mainnet-test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apitest/scripts/mainnet-test.sh b/apitest/scripts/mainnet-test.sh index a8c3132b15e..4e75649ab28 100755 --- a/apitest/scripts/mainnet-test.sh +++ b/apitest/scripts/mainnet-test.sh @@ -141,7 +141,7 @@ [ "$status" -eq 0 ] echo "actual output: $output" >&2 [ "$output" = "wallet decrypted" ] - sleep 1 + sleep 3 } @test "test getbalance when wallet available & unlocked with 0 btc balance" { From 19aed849104bcaaa0de7fd3bb6dd4f33ed9ba267 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 27 Feb 2021 22:58:22 -0300 Subject: [PATCH 14/35] Fix getunusedbsqaddress test --- apitest/scripts/mainnet-test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apitest/scripts/mainnet-test.sh b/apitest/scripts/mainnet-test.sh index 4e75649ab28..8f2815d4d03 100755 --- a/apitest/scripts/mainnet-test.sh +++ b/apitest/scripts/mainnet-test.sh @@ -155,7 +155,7 @@ } @test "test getunusedbsqaddress" { - run ./bisq-cli --password=xyz getfundingaddresses + run ./bisq-cli --password=xyz getunusedbsqaddress [ "$status" -eq 0 ] } From 3f84246f590cfd17391c677e9aad8351c6c1665d Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sun, 28 Feb 2021 17:10:51 -0300 Subject: [PATCH 15/35] Improve interceptor's rate metering key definition and lookup This change replaces the hard coded strings used as keys in interceptor rate-metering lookup maps. Now, the fullMethodName defined in each bisq.proto.grpc.* class' io.grpc.MethodDescriptor is used, not a hard coded String. For example, the rate metering lookup key for 'GetBalances', in 'GrpcWalletsService', is the fullMethodName = SERVICE_NAME + '/' + "GetBalances", where SERVICE_NAME = "io.bisq.protobuffer.Wallets". Also adjusted affected tests, and tidy'd up interceptor logging. --- .../test/java/bisq/apitest/ApiTestCase.java | 13 ++++++-- .../daemon/grpc/GrpcDisputeAgentsService.java | 7 ++-- .../grpc/GrpcGetTradeStatisticsService.java | 7 ++-- .../bisq/daemon/grpc/GrpcHelpService.java | 7 ++-- .../bisq/daemon/grpc/GrpcOffersService.java | 16 +++++----- .../grpc/GrpcPaymentAccountsService.java | 12 +++---- .../bisq/daemon/grpc/GrpcPriceService.java | 7 ++-- .../bisq/daemon/grpc/GrpcTradesService.java | 16 +++++----- .../bisq/daemon/grpc/GrpcVersionService.java | 7 ++-- .../bisq/daemon/grpc/GrpcWalletsService.java | 32 +++++++++---------- .../CallRateMeteringInterceptor.java | 25 +++++++-------- .../grpc/interceptor/GrpcCallRateMeter.java | 5 ++- 12 files changed, 83 insertions(+), 71 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/ApiTestCase.java b/apitest/src/test/java/bisq/apitest/ApiTestCase.java index 5ef101fe31c..1ecd7e53c30 100644 --- a/apitest/src/test/java/bisq/apitest/ApiTestCase.java +++ b/apitest/src/test/java/bisq/apitest/ApiTestCase.java @@ -32,6 +32,8 @@ import static bisq.apitest.config.BisqAppConfig.alicedaemon; import static bisq.apitest.config.BisqAppConfig.arbdaemon; import static bisq.apitest.config.BisqAppConfig.bobdaemon; +import static bisq.proto.grpc.DisputeAgentsGrpc.getRegisterDisputeAgentMethod; +import static bisq.proto.grpc.GetVersionGrpc.getGetVersionMethod; import static java.net.InetAddress.getLoopbackAddress; import static java.util.Arrays.stream; import static java.util.concurrent.TimeUnit.MILLISECONDS; @@ -150,14 +152,19 @@ protected final String testName(TestInfo testInfo) { protected static File defaultRateMeterInterceptorConfig() { GrpcServiceRateMeteringConfig.Builder builder = new GrpcServiceRateMeteringConfig.Builder(); builder.addCallRateMeter(GrpcVersionService.class.getSimpleName(), - "getVersion", + getGetVersionMethod().getFullMethodName(), 1, SECONDS); - // Only GrpcVersionService is @VisibleForTesting, so we hardcode the class names. + // Only GrpcVersionService is @VisibleForTesting, so we need to + // hardcode other grpcServiceClassName parameter values used in + // builder.addCallRateMeter(...). builder.addCallRateMeter("GrpcDisputeAgentsService", - "registerDisputeAgent", + getRegisterDisputeAgentMethod().getFullMethodName(), 10, // Same as default. SECONDS); + // Define rate meters for non-existent method 'disabled', to override other grpc + // services' default rate meters -- defined in their rateMeteringInterceptor() + // methods. String[] serviceClassNames = new String[]{ "GrpcGetTradeStatisticsService", "GrpcHelpService", diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java index 60413c1c962..3fae4329930 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java @@ -2,7 +2,6 @@ import bisq.core.api.CoreApi; -import bisq.proto.grpc.DisputeAgentsGrpc; import bisq.proto.grpc.RegisterDisputeAgentReply; import bisq.proto.grpc.RegisterDisputeAgentRequest; @@ -17,6 +16,8 @@ import lombok.extern.slf4j.Slf4j; import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; +import static bisq.proto.grpc.DisputeAgentsGrpc.DisputeAgentsImplBase; +import static bisq.proto.grpc.DisputeAgentsGrpc.getRegisterDisputeAgentMethod; import static java.util.concurrent.TimeUnit.SECONDS; @@ -25,7 +26,7 @@ import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; @Slf4j -class GrpcDisputeAgentsService extends DisputeAgentsGrpc.DisputeAgentsImplBase { +class GrpcDisputeAgentsService extends DisputeAgentsImplBase { private final CoreApi coreApi; private final GrpcExceptionHandler exceptionHandler; @@ -61,7 +62,7 @@ final Optional rateMeteringInterceptor() { new HashMap<>() {{ // Do not limit devs' ability to test agent registration // and call validation in regtest arbitration daemons. - put("registerDisputeAgent", new GrpcCallRateMeter(10, SECONDS)); + put(getRegisterDisputeAgentMethod().getFullMethodName(), new GrpcCallRateMeter(10, SECONDS)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java index 3ef71627bed..553ff1b30f5 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java @@ -3,7 +3,6 @@ import bisq.core.api.CoreApi; import bisq.core.trade.statistics.TradeStatistics3; -import bisq.proto.grpc.GetTradeStatisticsGrpc; import bisq.proto.grpc.GetTradeStatisticsReply; import bisq.proto.grpc.GetTradeStatisticsRequest; @@ -19,6 +18,8 @@ import lombok.extern.slf4j.Slf4j; import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; +import static bisq.proto.grpc.GetTradeStatisticsGrpc.GetTradeStatisticsImplBase; +import static bisq.proto.grpc.GetTradeStatisticsGrpc.getGetTradeStatisticsMethod; import static java.util.concurrent.TimeUnit.SECONDS; @@ -27,7 +28,7 @@ import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; @Slf4j -class GrpcGetTradeStatisticsService extends GetTradeStatisticsGrpc.GetTradeStatisticsImplBase { +class GrpcGetTradeStatisticsService extends GetTradeStatisticsImplBase { private final CoreApi coreApi; private final GrpcExceptionHandler exceptionHandler; @@ -64,7 +65,7 @@ final Optional rateMeteringInterceptor() { return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass()) .or(() -> Optional.of(CallRateMeteringInterceptor.valueOf( new HashMap<>() {{ - put("getTradeStatistics", new GrpcCallRateMeter(1, SECONDS)); + put(getGetTradeStatisticsMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcHelpService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcHelpService.java index 10fae5cf826..2721420b771 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcHelpService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcHelpService.java @@ -21,7 +21,6 @@ import bisq.proto.grpc.GetMethodHelpReply; import bisq.proto.grpc.GetMethodHelpRequest; -import bisq.proto.grpc.HelpGrpc; import io.grpc.ServerInterceptor; import io.grpc.stub.StreamObserver; @@ -34,6 +33,8 @@ import lombok.extern.slf4j.Slf4j; import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; +import static bisq.proto.grpc.HelpGrpc.HelpImplBase; +import static bisq.proto.grpc.HelpGrpc.getGetMethodHelpMethod; import static java.util.concurrent.TimeUnit.SECONDS; @@ -42,7 +43,7 @@ import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; @Slf4j -class GrpcHelpService extends HelpGrpc.HelpImplBase { +class GrpcHelpService extends HelpImplBase { private final CoreApi coreApi; private final GrpcExceptionHandler exceptionHandler; @@ -76,7 +77,7 @@ final Optional rateMeteringInterceptor() { return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass()) .or(() -> Optional.of(CallRateMeteringInterceptor.valueOf( new HashMap<>() {{ - put("getMethodHelp", new GrpcCallRateMeter(1, SECONDS)); + put(getGetMethodHelpMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java index c6c93430e5c..54658e4c9a9 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java @@ -34,7 +34,6 @@ import bisq.proto.grpc.GetOfferRequest; import bisq.proto.grpc.GetOffersReply; import bisq.proto.grpc.GetOffersRequest; -import bisq.proto.grpc.OffersGrpc; import io.grpc.ServerInterceptor; import io.grpc.stub.StreamObserver; @@ -50,6 +49,7 @@ import static bisq.core.api.model.OfferInfo.toOfferInfo; import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; +import static bisq.proto.grpc.OffersGrpc.*; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; @@ -59,7 +59,7 @@ import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; @Slf4j -class GrpcOffersService extends OffersGrpc.OffersImplBase { +class GrpcOffersService extends OffersImplBase { private final CoreApi coreApi; private final GrpcExceptionHandler exceptionHandler; @@ -193,12 +193,12 @@ final Optional rateMeteringInterceptor() { return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass()) .or(() -> Optional.of(CallRateMeteringInterceptor.valueOf( new HashMap<>() {{ - put("getOffer", new GrpcCallRateMeter(1, SECONDS)); - put("getMyOffer", new GrpcCallRateMeter(1, SECONDS)); - put("getOffers", new GrpcCallRateMeter(1, SECONDS)); - put("getMyOffers", new GrpcCallRateMeter(1, SECONDS)); - put("createOffer", new GrpcCallRateMeter(1, MINUTES)); - put("cancelOffer", new GrpcCallRateMeter(1, MINUTES)); + put(getGetOfferMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getGetMyOfferMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getGetOffersMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getGetMyOffersMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getCreateOfferMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); + put(getCancelOfferMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java index 90b031d4904..e1df2b16cb1 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java @@ -29,7 +29,6 @@ import bisq.proto.grpc.GetPaymentAccountsRequest; import bisq.proto.grpc.GetPaymentMethodsReply; import bisq.proto.grpc.GetPaymentMethodsRequest; -import bisq.proto.grpc.PaymentAccountsGrpc; import io.grpc.ServerInterceptor; import io.grpc.stub.StreamObserver; @@ -43,6 +42,7 @@ import lombok.extern.slf4j.Slf4j; import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; +import static bisq.proto.grpc.PaymentAccountsGrpc.*; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; @@ -52,7 +52,7 @@ import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; @Slf4j -class GrpcPaymentAccountsService extends PaymentAccountsGrpc.PaymentAccountsImplBase { +class GrpcPaymentAccountsService extends PaymentAccountsImplBase { private final CoreApi coreApi; private final GrpcExceptionHandler exceptionHandler; @@ -135,10 +135,10 @@ final Optional rateMeteringInterceptor() { return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass()) .or(() -> Optional.of(CallRateMeteringInterceptor.valueOf( new HashMap<>() {{ - put("createPaymentAccount", new GrpcCallRateMeter(1, MINUTES)); - put("getPaymentAccounts", new GrpcCallRateMeter(1, SECONDS)); - put("getPaymentMethods", new GrpcCallRateMeter(1, SECONDS)); - put("getPaymentAccountForm", new GrpcCallRateMeter(1, SECONDS)); + put(getCreatePaymentAccountMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); + put(getGetPaymentAccountsMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getGetPaymentMethodsMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getGetPaymentAccountFormMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java index 256136c35ec..bec21b9c5bf 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java @@ -21,7 +21,6 @@ import bisq.proto.grpc.MarketPriceReply; import bisq.proto.grpc.MarketPriceRequest; -import bisq.proto.grpc.PriceGrpc; import io.grpc.ServerInterceptor; import io.grpc.stub.StreamObserver; @@ -34,6 +33,8 @@ import lombok.extern.slf4j.Slf4j; import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; +import static bisq.proto.grpc.PriceGrpc.PriceImplBase; +import static bisq.proto.grpc.PriceGrpc.getGetMarketPriceMethod; import static java.util.concurrent.TimeUnit.SECONDS; @@ -42,7 +43,7 @@ import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; @Slf4j -class GrpcPriceService extends PriceGrpc.PriceImplBase { +class GrpcPriceService extends PriceImplBase { private final CoreApi coreApi; private final GrpcExceptionHandler exceptionHandler; @@ -78,7 +79,7 @@ final Optional rateMeteringInterceptor() { return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass()) .or(() -> Optional.of(CallRateMeteringInterceptor.valueOf( new HashMap<>() {{ - put("getMarketPrice", new GrpcCallRateMeter(1, SECONDS)); + put(getGetMarketPriceMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index d757a22288e..8cca8d75143 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -31,7 +31,6 @@ import bisq.proto.grpc.KeepFundsRequest; import bisq.proto.grpc.TakeOfferReply; import bisq.proto.grpc.TakeOfferRequest; -import bisq.proto.grpc.TradesGrpc; import bisq.proto.grpc.WithdrawFundsReply; import bisq.proto.grpc.WithdrawFundsRequest; @@ -47,6 +46,7 @@ import static bisq.core.api.model.TradeInfo.toTradeInfo; import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; +import static bisq.proto.grpc.TradesGrpc.*; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; @@ -56,7 +56,7 @@ import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; @Slf4j -class GrpcTradesService extends TradesGrpc.TradesImplBase { +class GrpcTradesService extends TradesImplBase { private final CoreApi coreApi; private final GrpcExceptionHandler exceptionHandler; @@ -169,12 +169,12 @@ final Optional rateMeteringInterceptor() { return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass()) .or(() -> Optional.of(CallRateMeteringInterceptor.valueOf( new HashMap<>() {{ - put("getTrade", new GrpcCallRateMeter(1, SECONDS)); - put("takeOffer", new GrpcCallRateMeter(1, MINUTES)); - put("confirmPaymentStarted", new GrpcCallRateMeter(1, MINUTES)); - put("confirmPaymentReceived", new GrpcCallRateMeter(1, MINUTES)); - put("keepFunds", new GrpcCallRateMeter(1, MINUTES)); - put("withdrawFunds", new GrpcCallRateMeter(1, MINUTES)); + put(getGetTradeMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getTakeOfferMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); + put(getConfirmPaymentStartedMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); + put(getConfirmPaymentReceivedMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); + put(getKeepFundsMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); + put(getWithdrawFundsMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java index e585b62d617..ed04be4a598 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java @@ -19,7 +19,6 @@ import bisq.core.api.CoreApi; -import bisq.proto.grpc.GetVersionGrpc; import bisq.proto.grpc.GetVersionReply; import bisq.proto.grpc.GetVersionRequest; @@ -36,6 +35,8 @@ import lombok.extern.slf4j.Slf4j; import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; +import static bisq.proto.grpc.GetVersionGrpc.GetVersionImplBase; +import static bisq.proto.grpc.GetVersionGrpc.getGetVersionMethod; import static java.util.concurrent.TimeUnit.SECONDS; @@ -45,7 +46,7 @@ @VisibleForTesting @Slf4j -public class GrpcVersionService extends GetVersionGrpc.GetVersionImplBase { +public class GrpcVersionService extends GetVersionImplBase { private final CoreApi coreApi; private final GrpcExceptionHandler exceptionHandler; @@ -77,7 +78,7 @@ final Optional rateMeteringInterceptor() { return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass()) .or(() -> Optional.of(CallRateMeteringInterceptor.valueOf( new HashMap<>() {{ - put("getVersion", new GrpcCallRateMeter(1, SECONDS)); + put(getGetVersionMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java index a183aca48b6..1391a5b6976 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java @@ -51,7 +51,6 @@ import bisq.proto.grpc.UnlockWalletRequest; import bisq.proto.grpc.UnsetTxFeeRatePreferenceReply; import bisq.proto.grpc.UnsetTxFeeRatePreferenceRequest; -import bisq.proto.grpc.WalletsGrpc; import io.grpc.ServerInterceptor; import io.grpc.stub.StreamObserver; @@ -73,6 +72,7 @@ import static bisq.core.api.model.TxInfo.toTxInfo; import static bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig.getCustomRateMeteringInterceptor; +import static bisq.proto.grpc.WalletsGrpc.*; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; @@ -82,7 +82,7 @@ import bisq.daemon.grpc.interceptor.GrpcCallRateMeter; @Slf4j -class GrpcWalletsService extends WalletsGrpc.WalletsImplBase { +class GrpcWalletsService extends WalletsImplBase { private final CoreApi coreApi; private final GrpcExceptionHandler exceptionHandler; @@ -352,24 +352,24 @@ final Optional rateMeteringInterceptor() { return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass()) .or(() -> Optional.of(CallRateMeteringInterceptor.valueOf( new HashMap<>() {{ - put("getBalances", new GrpcCallRateMeter(1, SECONDS)); - put("getAddressBalance", new GrpcCallRateMeter(1, SECONDS)); - put("getFundingAddresses", new GrpcCallRateMeter(1, SECONDS)); - put("getUnusedBsqAddress", new GrpcCallRateMeter(1, SECONDS)); - put("sendBsq", new GrpcCallRateMeter(1, MINUTES)); - put("sendBtc", new GrpcCallRateMeter(1, MINUTES)); - put("getTxFeeRate", new GrpcCallRateMeter(1, SECONDS)); - put("setTxFeeRatePreference", new GrpcCallRateMeter(1, SECONDS)); - put("unsetTxFeeRatePreference", new GrpcCallRateMeter(1, SECONDS)); - put("getTransaction", new GrpcCallRateMeter(1, SECONDS)); + put(getGetBalancesMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getGetAddressBalanceMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getGetFundingAddressesMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getGetUnusedBsqAddressMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getSendBsqMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); + put(getSendBtcMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); + put(getGetTxFeeRateMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getSetTxFeeRatePreferenceMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getUnsetTxFeeRatePreferenceMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getGetTransactionMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); // Trying to set or remove a wallet password several times before the 1st attempt has time to // persist the change to disk may corrupt the wallet, so allow only 1 attempt per 5 seconds. - put("setWalletPassword", new GrpcCallRateMeter(1, SECONDS, 5)); - put("removeWalletPassword", new GrpcCallRateMeter(1, SECONDS, 5)); + put(getSetWalletPasswordMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS, 5)); + put(getRemoveWalletPasswordMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS, 5)); - put("lockWallet", new GrpcCallRateMeter(1, SECONDS)); - put("unlockWallet", new GrpcCallRateMeter(1, SECONDS)); + put(getLockWalletMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); + put(getUnlockWalletMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); }} ))); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java b/daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java index 404c093a2f2..5f1dfd5c131 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java +++ b/daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java @@ -34,7 +34,6 @@ import static io.grpc.Status.PERMISSION_DENIED; import static java.lang.String.format; import static java.util.stream.Collectors.joining; -import static org.apache.commons.lang3.StringUtils.uncapitalize; @Slf4j public final class CallRateMeteringInterceptor implements ServerInterceptor { @@ -85,16 +84,19 @@ private void handlePermissionDeniedWarningAndCloseCall(String methodName, ServerCall serverCall) throws StatusRuntimeException { String msg = getDefaultRateExceededError(methodName, rateMeter); - log.warn(StringUtils.capitalize(msg) + "."); - serverCall.close(PERMISSION_DENIED.withDescription(msg), new Metadata()); + log.warn(msg + "."); + serverCall.close(PERMISSION_DENIED.withDescription(msg.toLowerCase()), new Metadata()); } private String getDefaultRateExceededError(String methodName, GrpcCallRateMeter rateMeter) { // The derived method name may not be an exact match to CLI's method name. String timeUnitName = StringUtils.chop(rateMeter.getTimeUnit().name().toLowerCase()); - return format("the maximum allowed number of %s calls (%d/%s) has been exceeded", - methodName.toLowerCase(), + // Just print 'getversion', not the grpc method descriptor's + // full-method-name: 'io.bisq.protobuffer.getversion/getversion'. + String loggedMethodName = methodName.split("/")[1]; + return format("The maximum allowed number of %s calls (%d/%s) has been exceeded", + loggedMethodName, rateMeter.getAllowedCallsPerTimeWindow(), timeUnitName); } @@ -107,15 +109,10 @@ private Optional> getRateMeterKV(ServerCall private String getRateMeterKey(ServerCall serverCall) { // Get the rate meter map key from the server call method descriptor. The - // returned key (e.g., 'createOffer') is defined in the 'serviceCallRateMeters' - // constructor argument. It is extracted from the gRPC fullMethodName, e.g., - // 'io.bisq.protobuffer.Offers/CreateOffer'. - String fullServiceMethodName = serverCall.getMethodDescriptor().getFullMethodName(); - if (fullServiceMethodName.contains("/")) - return uncapitalize(fullServiceMethodName.split("/")[1]); - else - throw new IllegalStateException("Could not extract rate meter key from " - + fullServiceMethodName + "."); + // returned String (e.g., 'io.bisq.protobuffer.Offers/CreateOffer') will match + // a map entry key in the 'serviceCallRateMeters' constructor argument, if it + // was defined in the Grpc*Service class' rateMeteringInterceptor method. + return serverCall.getMethodDescriptor().getFullMethodName(); } @Override diff --git a/daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java b/daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java index 6cc35a6d435..73096a0336b 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java +++ b/daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java @@ -55,8 +55,11 @@ public int getCallsCount() { public String getCallsCountProgress(String calledMethodName) { String shortTimeUnitName = StringUtils.chop(timeUnit.name().toLowerCase()); + // Just print 'GetVersion has been called N times...', + // not 'io.bisq.protobuffer.GetVersion/GetVersion has been called N times...' + String loggedMethodName = calledMethodName.split("/")[1]; return format("%s has been called %d time%s in the last %s, rate limit is %d/%s", - calledMethodName, + loggedMethodName, callTimestamps.size(), callTimestamps.size() == 1 ? "" : "s", shortTimeUnitName, From 392c0f58afb4f8bc7f62fc0dba381a161c50fc32 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 1 Mar 2021 15:20:08 -0300 Subject: [PATCH 16/35] Fix CLI number opt validation, improve server-not-up msg - Fix tx-fee-rate opt validation bug. - Tell user what option value is not a number. - Append ", server may not be running" text to "io exception" exception msg. --- cli/src/main/java/bisq/cli/CliMain.java | 42 +++++++++++++++---------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index 6dab838a455..5e6e913711c 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -45,10 +45,7 @@ import static bisq.cli.CurrencyFormat.toSecurityDepositAsPct; import static bisq.cli.Method.*; import static bisq.cli.TableFormat.*; -import static bisq.cli.opts.OptLabel.OPT_HELP; -import static bisq.cli.opts.OptLabel.OPT_HOST; -import static bisq.cli.opts.OptLabel.OPT_PASSWORD; -import static bisq.cli.opts.OptLabel.OPT_PORT; +import static bisq.cli.opts.OptLabel.*; import static java.lang.String.format; import static java.lang.System.err; import static java.lang.System.exit; @@ -230,11 +227,11 @@ public static void run(String[] args) { } var address = opts.getAddress(); var amount = opts.getAmount(); - verifyStringIsValidDecimal(amount); + verifyStringIsValidDecimal(OPT_AMOUNT, amount); var txFeeRate = opts.getFeeRate(); - if (txFeeRate.isEmpty()) - verifyStringIsValidLong(txFeeRate); + if (!txFeeRate.isEmpty()) + verifyStringIsValidLong(OPT_TX_FEE_RATE, txFeeRate); var txInfo = client.sendBsq(address, amount, txFeeRate); out.printf("%s bsq sent to %s in tx %s%n", @@ -251,11 +248,11 @@ public static void run(String[] args) { } var address = opts.getAddress(); var amount = opts.getAmount(); - verifyStringIsValidDecimal(amount); + verifyStringIsValidDecimal(OPT_AMOUNT, amount); var txFeeRate = opts.getFeeRate(); - if (txFeeRate.isEmpty()) - verifyStringIsValidLong(txFeeRate); + if (!txFeeRate.isEmpty()) + verifyStringIsValidLong(OPT_TX_FEE_RATE, txFeeRate); var memo = opts.getMemo(); @@ -605,7 +602,10 @@ public static void run(String[] args) { } catch (StatusRuntimeException ex) { // Remove the leading gRPC status code (e.g. "UNKNOWN: ") from the message String message = ex.getMessage().replaceFirst("^[A-Z_]+: ", ""); - throw new RuntimeException(message, ex); + if (message.equals("io exception")) + throw new RuntimeException(message + ", server may not be running", ex); + else + throw new RuntimeException(message, ex); } } @@ -616,19 +616,27 @@ private static Method getMethodFromCmd(String methodName) { return Method.valueOf(methodName.toLowerCase()); } - private static void verifyStringIsValidDecimal(String param) { + @SuppressWarnings("SameParameterValue") + private static void verifyStringIsValidDecimal(String optionLabel, String optionValue) { try { - Double.parseDouble(param); + Double.parseDouble(optionValue); } catch (NumberFormatException ex) { - throw new IllegalArgumentException(format("'%s' is not a number", param)); + throw new IllegalArgumentException(format("--%s=%s, '%s' is not a number", + optionLabel, + optionValue, + optionValue)); } } - private static void verifyStringIsValidLong(String param) { + @SuppressWarnings("SameParameterValue") + private static void verifyStringIsValidLong(String optionLabel, String optionValue) { try { - Long.parseLong(param); + Long.parseLong(optionValue); } catch (NumberFormatException ex) { - throw new IllegalArgumentException(format("'%s' is not a number", param)); + throw new IllegalArgumentException(format("--%s=%s, '%s' is not a number", + optionLabel, + optionValue, + optionValue)); } } From 2473ff6374e70e88ab1158a56aa6fb256b2b887a Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Tue, 2 Mar 2021 13:28:51 -0300 Subject: [PATCH 17/35] Fix tx-fee-rate formatting (and math) bug in cli/CurrencyFormat Now prints ' sats/byte' to console. --- cli/src/main/java/bisq/cli/CurrencyFormat.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/src/main/java/bisq/cli/CurrencyFormat.java b/cli/src/main/java/bisq/cli/CurrencyFormat.java index 671e6149f79..9c141e569f1 100644 --- a/cli/src/main/java/bisq/cli/CurrencyFormat.java +++ b/cli/src/main/java/bisq/cli/CurrencyFormat.java @@ -38,7 +38,7 @@ public class CurrencyFormat { static final BigDecimal SATOSHI_DIVISOR = new BigDecimal(100000000); static final DecimalFormat BTC_FORMAT = new DecimalFormat("###,##0.00000000"); - static final DecimalFormat BTC_TX_FEE_FORMAT = new DecimalFormat("###,##0.00"); + static final DecimalFormat BTC_TX_FEE_FORMAT = new DecimalFormat("###,###,##0"); static final BigDecimal BSQ_SATOSHI_DIVISOR = new BigDecimal(100); static final DecimalFormat BSQ_FORMAT = new DecimalFormat("###,###,###,##0.00"); @@ -117,6 +117,6 @@ public static double toSecurityDepositAsPct(String securityDepositInput) { @SuppressWarnings("BigDecimalMethodWithoutRoundingCalled") public static String formatFeeSatoshis(long sats) { - return BTC_TX_FEE_FORMAT.format(BigDecimal.valueOf(sats).divide(SATOSHI_DIVISOR)); + return BTC_TX_FEE_FORMAT.format(BigDecimal.valueOf(sats)); } } From 8590c676700ff395d95f003b8ef0e7573a01049e Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Tue, 2 Mar 2021 14:40:36 -0300 Subject: [PATCH 18/35] Remove warning supression --- cli/src/main/java/bisq/cli/CurrencyFormat.java | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/src/main/java/bisq/cli/CurrencyFormat.java b/cli/src/main/java/bisq/cli/CurrencyFormat.java index 9c141e569f1..afa97b34300 100644 --- a/cli/src/main/java/bisq/cli/CurrencyFormat.java +++ b/cli/src/main/java/bisq/cli/CurrencyFormat.java @@ -115,7 +115,6 @@ public static double toSecurityDepositAsPct(String securityDepositInput) { } } - @SuppressWarnings("BigDecimalMethodWithoutRoundingCalled") public static String formatFeeSatoshis(long sats) { return BTC_TX_FEE_FORMAT.format(BigDecimal.valueOf(sats)); } From e0bf773564f137ec996a5c3f9eb042c970cdea1e Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Wed, 3 Mar 2021 12:43:21 -0300 Subject: [PATCH 19/35] Add link to api-beta-test-guide.md --- apitest/docs/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/apitest/docs/README.md b/apitest/docs/README.md index 625dadddc9f..1f24def3e44 100644 --- a/apitest/docs/README.md +++ b/apitest/docs/README.md @@ -3,3 +3,4 @@ - [build-run.md](build-run.md): Build and run API tests at the command line and from Intellij. - [test-categories.md](test-categories.md): How to categorize a test case as `method`, `scenario` or `e2e`. - [regtest-port-conflicts.md](regtest-port-conflicts.md): Avoid port conflicts when running multiple bitcoin-core apps in regtest mode. + - [api-beta-test-guide.md](api-beta-test-guide.md): How to run the test harness and tutorial script, and beta test the API with the new CLI. From a2000bdc336384274e7fd161b7f7f23ac9f4f188 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Wed, 3 Mar 2021 12:44:29 -0300 Subject: [PATCH 20/35] Explain how to manually register test dispute agents --- apitest/docs/api-beta-test-guide.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/apitest/docs/api-beta-test-guide.md b/apitest/docs/api-beta-test-guide.md index 4be69e61c93..dddf75b1e7b 100644 --- a/apitest/docs/api-beta-test-guide.md +++ b/apitest/docs/api-beta-test-guide.md @@ -152,6 +152,25 @@ CLI command unless you change the server’s `–apiPort=`. In `9998`, Bob’s is `9999`. When you manually test the Api using the test harness, be aware of the port numbers being used in the CLI commands, so you know which server (Bob’s or Alice’s) the CLI is sending requests to. +### Registering Test Dispute Agents + +If you ran the `trade-simulation.sh` script in your currently running test harness, dispute agents have +already been registered in the arbitration node, and you can run any of the commands described in the following +sections. + +If you have not run the `trade-simulation.sh` script against the test harness, you will need to +manually register dispute agents in the arbitration node before you can initiate a trade. Copy, paste and run +the following CLI commands to register a `mediator` and a `refundagent`. Do not change the commands' port `9997` +option (the test arbitration node's listening port). +``` +$ ./bisq-cli --password=xyz --port=9997 registerdisputeagent --dispute-agent-type=mediator + --registration-key=6ac43ea1df2a290c1c8391736aa42e4339c5cb4f110ff0257a13b63211977b7a + +$ ./bisq-cli --password=xyz --port=9997 registerdisputeagent --dispute-agent-type=refundagent + --registration-key=6ac43ea1df2a290c1c8391736aa42e4339c5cb4f110ff0257a13b63211977b7a +``` +_Note: The API cannot be used to register dispute agents on nodes connected to `mainnet`._ + ### CLI Help Useful information can be found using the CLI’s `--help` option. From cfaa539a5e5fffb2c9d732b7281769bde49516c6 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Thu, 4 Mar 2021 13:03:04 -0300 Subject: [PATCH 21/35] Fix opt validation bugs in CLI Unset empty string default values on some the createoffer method's required option declarations in CreateOfferOptionParser, and adds a check for presence of required currency-code option. - Removes default "" value from required opt direction. - Removes default "" value from required opt currency-code. - Removes default "" value from required opt amount. - Removes default "" value from required opt min-amount. - Removes default "" value from required opt fixed-price. - Removes default "" value from required opt security-deposit. - Check for required currency-code option. Unset empty string default values on some of the createoffer method's required option declarations in TakeOfferOptionParser. - Removes default "" value from required opt offer-id. - Removes default "" value from required opt payment-account. Other opt parser default values removed from: - CancelOfferOptionParser#offer-id - CreatePaymentAcctOptionParser#payment-account-form - GetAddressBalanceOptionParser#address - GetBTCMarketPriceOptionParser#currency-code - GetOfferOptionParser#offer-id - GetOffersOptionParser#direction - GetOffersOptionParser#currency-code - GetPaymentAcctFormOptionParser#payment-method-id - GetTradeOptionParser#trade-id - GetTransactionOptionParser#transaction-id - RegisterDisputeAgentOptionParser#registration-key - RegisterDisputeAgentOptionParser#dispute-agent-type - RemoveWalletPasswordOptionParser#wallet-password - SendBsqOptionParser#address - SendBsqOptionParser#amount - SendBtcOptionParser#address - SendBtcOptionParser#amount - SetTxFeeRateOptionParser#tx-fee-rate - SetWalletPasswordOptionParser#wallet-password - UnlockWalletOptionParser#wallet-password - UnlockWalletOptionParser#timeout - WithdrawFundsOptionParser#trade-id - WithdrawFundsOptionParser#address --- .../cli/opts/CancelOfferOptionParser.java | 4 +--- .../cli/opts/CreateOfferOptionParser.java | 20 +++++++++---------- .../opts/CreatePaymentAcctOptionParser.java | 4 +--- .../opts/GetAddressBalanceOptionParser.java | 4 +--- .../opts/GetBTCMarketPriceOptionParser.java | 4 +--- .../bisq/cli/opts/GetOfferOptionParser.java | 4 +--- .../bisq/cli/opts/GetOffersOptionParser.java | 7 ++----- .../opts/GetPaymentAcctFormOptionParser.java | 4 +--- .../bisq/cli/opts/GetTradeOptionParser.java | 4 +--- .../cli/opts/GetTransactionOptionParser.java | 4 +--- .../RegisterDisputeAgentOptionParser.java | 7 ++----- .../RemoveWalletPasswordOptionParser.java | 4 +--- .../bisq/cli/opts/SendBsqOptionParser.java | 6 ++---- .../bisq/cli/opts/SendBtcOptionParser.java | 6 ++---- .../cli/opts/SetTxFeeRateOptionParser.java | 4 +--- .../opts/SetWalletPasswordOptionParser.java | 3 +-- .../bisq/cli/opts/TakeOfferOptionParser.java | 7 ++----- .../cli/opts/UnlockWalletOptionParser.java | 4 +--- .../cli/opts/WithdrawFundsOptionParser.java | 9 +++++---- 19 files changed, 36 insertions(+), 73 deletions(-) diff --git a/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java index 24ebc744211..5ed33ed41fb 100644 --- a/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_OFFER_ID; -import static joptsimple.internal.Strings.EMPTY; public class CancelOfferOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec offerIdOpt = parser.accepts(OPT_OFFER_ID, "id of offer to cancel") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public CancelOfferOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java index d4d7c05c7ad..bbaf68bab36 100644 --- a/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java @@ -33,20 +33,16 @@ public class CreateOfferOptionParser extends AbstractMethodOptionParser implemen .defaultsTo(EMPTY); final OptionSpec directionOpt = parser.accepts(OPT_DIRECTION, "offer direction (buy|sell)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec currencyCodeOpt = parser.accepts(OPT_CURRENCY_CODE, "currency code (eur|usd|...)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec amountOpt = parser.accepts(OPT_AMOUNT, "amount of btc to buy or sell") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec minAmountOpt = parser.accepts(OPT_MIN_AMOUNT, "minimum amount of btc to buy or sell") - .withOptionalArg() - .defaultsTo(EMPTY); + .withOptionalArg(); final OptionSpec mktPriceMarginOpt = parser.accepts(OPT_MKT_PRICE_MARGIN, "market btc price margin (%)") .withOptionalArg() @@ -54,11 +50,10 @@ public class CreateOfferOptionParser extends AbstractMethodOptionParser implemen final OptionSpec fixedPriceOpt = parser.accepts(OPT_FIXED_PRICE, "fixed btc price") .withOptionalArg() - .defaultsTo(EMPTY); + .defaultsTo("0"); final OptionSpec securityDepositOpt = parser.accepts(OPT_SECURITY_DEPOSIT, "maker security deposit (%)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec makerFeeCurrencyCodeOpt = parser.accepts(OPT_FEE_CURRENCY, "maker fee currency code (bsq|btc)") .withOptionalArg() @@ -81,6 +76,9 @@ public CreateOfferOptionParser parse() { if (!options.has(directionOpt)) throw new IllegalArgumentException("no direction (buy|sell) specified"); + if (!options.has(currencyCodeOpt)) + throw new IllegalArgumentException("no currency code specified"); + if (!options.has(amountOpt)) throw new IllegalArgumentException("no btc amount specified"); diff --git a/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java b/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java index 21fb01bcec5..1d4a85ed497 100644 --- a/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java @@ -25,14 +25,12 @@ import static bisq.cli.opts.OptLabel.OPT_PAYMENT_ACCOUNT_FORM; import static java.lang.String.format; -import static joptsimple.internal.Strings.EMPTY; public class CreatePaymentAcctOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec paymentAcctFormPathOpt = parser.accepts(OPT_PAYMENT_ACCOUNT_FORM, "path to json payment account form") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public CreatePaymentAcctOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java index 80537ffc89d..254fed91e65 100644 --- a/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_ADDRESS; -import static joptsimple.internal.Strings.EMPTY; public class GetAddressBalanceOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec addressOpt = parser.accepts(OPT_ADDRESS, "wallet btc address") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetAddressBalanceOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java index c54f65f42e9..da66b6b41f1 100644 --- a/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_CURRENCY_CODE; -import static joptsimple.internal.Strings.EMPTY; public class GetBTCMarketPriceOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec currencyCodeOpt = parser.accepts(OPT_CURRENCY_CODE, "currency-code") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetBTCMarketPriceOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java index 600e7672c45..159f459be2d 100644 --- a/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_OFFER_ID; -import static joptsimple.internal.Strings.EMPTY; public class GetOfferOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec offerIdOpt = parser.accepts(OPT_OFFER_ID, "id of offer to get") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetOfferOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java index 29360886f87..dbc1bf83a99 100644 --- a/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java @@ -22,17 +22,14 @@ import static bisq.cli.opts.OptLabel.OPT_CURRENCY_CODE; import static bisq.cli.opts.OptLabel.OPT_DIRECTION; -import static joptsimple.internal.Strings.EMPTY; public class GetOffersOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec directionOpt = parser.accepts(OPT_DIRECTION, "offer direction (buy|sell)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec currencyCodeOpt = parser.accepts(OPT_CURRENCY_CODE, "currency code (eur|usd|...)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetOffersOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java index ef5bd5b454c..52fdfd52999 100644 --- a/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java @@ -21,14 +21,12 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_PAYMENT_METHOD_ID; -import static joptsimple.internal.Strings.EMPTY; public class GetPaymentAcctFormOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec paymentMethodIdOpt = parser.accepts(OPT_PAYMENT_METHOD_ID, "id of payment method type used by a payment account") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetPaymentAcctFormOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java index 2b7681f3c69..3ea344a06ee 100644 --- a/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java @@ -22,13 +22,11 @@ import static bisq.cli.opts.OptLabel.OPT_SHOW_CONTRACT; import static bisq.cli.opts.OptLabel.OPT_TRADE_ID; -import static joptsimple.internal.Strings.EMPTY; public class GetTradeOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec tradeIdOpt = parser.accepts(OPT_TRADE_ID, "id of trade") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec showContractOpt = parser.accepts(OPT_SHOW_CONTRACT, "show trade's json contract") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java index d4266eb9ff7..83cc4f48fa7 100644 --- a/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_TRANSACTION_ID; -import static joptsimple.internal.Strings.EMPTY; public class GetTransactionOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec txIdOpt = parser.accepts(OPT_TRANSACTION_ID, "id of transaction") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetTransactionOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java b/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java index 428555a3493..9a3a8e7525d 100644 --- a/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java @@ -22,17 +22,14 @@ import static bisq.cli.opts.OptLabel.OPT_DISPUTE_AGENT_TYPE; import static bisq.cli.opts.OptLabel.OPT_REGISTRATION_KEY; -import static joptsimple.internal.Strings.EMPTY; public class RegisterDisputeAgentOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec disputeAgentTypeOpt = parser.accepts(OPT_DISPUTE_AGENT_TYPE, "dispute agent type") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec registrationKeyOpt = parser.accepts(OPT_REGISTRATION_KEY, "registration key") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public RegisterDisputeAgentOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java b/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java index 5b9a3915941..dd1de67f057 100644 --- a/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_WALLET_PASSWORD; -import static joptsimple.internal.Strings.EMPTY; public class RemoveWalletPasswordOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec passwordOpt = parser.accepts(OPT_WALLET_PASSWORD, "bisq wallet password") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public RemoveWalletPasswordOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java b/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java index 3bffce785c4..73025a17bf0 100644 --- a/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java @@ -28,12 +28,10 @@ public class SendBsqOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec addressOpt = parser.accepts(OPT_ADDRESS, "destination bsq address") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec amountOpt = parser.accepts(OPT_AMOUNT, "amount of bsq to send") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec feeRateOpt = parser.accepts(OPT_TX_FEE_RATE, "optional tx fee rate (sats/byte)") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java b/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java index 8c3f9762019..e49e961a1f5 100644 --- a/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java @@ -29,12 +29,10 @@ public class SendBtcOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec addressOpt = parser.accepts(OPT_ADDRESS, "destination btc address") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec amountOpt = parser.accepts(OPT_AMOUNT, "amount of btc to send") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec feeRateOpt = parser.accepts(OPT_TX_FEE_RATE, "optional tx fee rate (sats/byte)") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java b/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java index 9d4b5e71b3e..f6b9f7c7a06 100644 --- a/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java @@ -21,14 +21,12 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_TX_FEE_RATE; -import static joptsimple.internal.Strings.EMPTY; public class SetTxFeeRateOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec feeRateOpt = parser.accepts(OPT_TX_FEE_RATE, "tx fee rate preference (sats/byte)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public SetTxFeeRateOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java b/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java index d55b1bf33b4..483d56f649e 100644 --- a/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java @@ -27,8 +27,7 @@ public class SetWalletPasswordOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec passwordOpt = parser.accepts(OPT_WALLET_PASSWORD, "bisq wallet password") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec newPasswordOpt = parser.accepts(OPT_NEW_WALLET_PASSWORD, "new bisq wallet password") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java index 75ef2885b04..589860cd63e 100644 --- a/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java @@ -23,17 +23,14 @@ import static bisq.cli.opts.OptLabel.OPT_FEE_CURRENCY; import static bisq.cli.opts.OptLabel.OPT_OFFER_ID; import static bisq.cli.opts.OptLabel.OPT_PAYMENT_ACCOUNT; -import static joptsimple.internal.Strings.EMPTY; public class TakeOfferOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec offerIdOpt = parser.accepts(OPT_OFFER_ID, "id of offer to take") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec paymentAccountIdOpt = parser.accepts(OPT_PAYMENT_ACCOUNT, "id of payment account used for trade") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec takerFeeCurrencyCodeOpt = parser.accepts(OPT_FEE_CURRENCY, "taker fee currency code (bsq|btc)") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java b/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java index 4446138dd37..db2f1aa2606 100644 --- a/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java @@ -22,13 +22,11 @@ import static bisq.cli.opts.OptLabel.OPT_TIMEOUT; import static bisq.cli.opts.OptLabel.OPT_WALLET_PASSWORD; -import static joptsimple.internal.Strings.EMPTY; public class UnlockWalletOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec passwordOpt = parser.accepts(OPT_WALLET_PASSWORD, "bisq wallet password") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec unlockTimeoutOpt = parser.accepts(OPT_TIMEOUT, "wallet unlock timeout (s)") .withRequiredArg() diff --git a/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java b/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java index f47432fa860..b60829e9bef 100644 --- a/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java @@ -28,12 +28,10 @@ public class WithdrawFundsOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec tradeIdOpt = parser.accepts(OPT_TRADE_ID, "id of trade") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec addressOpt = parser.accepts(OPT_ADDRESS, "destination btc address") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec memoOpt = parser.accepts(OPT_MEMO, "optional tx memo") .withOptionalArg() @@ -53,6 +51,9 @@ public WithdrawFundsOptionParser parse() { if (!options.has(tradeIdOpt)) throw new IllegalArgumentException("no trade id specified"); + if (!options.has(addressOpt)) + throw new IllegalArgumentException("no destination address specified"); + return this; } From 62ff79d718fda4bb8c4fed0e73d33b557149e45f Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:09:41 -0300 Subject: [PATCH 22/35] Update cli getoffers smoke test to posix style opts --- .../test/java/bisq/cli/GetOffersSmokeTest.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/src/test/java/bisq/cli/GetOffersSmokeTest.java b/cli/src/test/java/bisq/cli/GetOffersSmokeTest.java index 49f849e0cc7..f613aea358c 100644 --- a/cli/src/test/java/bisq/cli/GetOffersSmokeTest.java +++ b/cli/src/test/java/bisq/cli/GetOffersSmokeTest.java @@ -16,24 +16,24 @@ public class GetOffersSmokeTest { public static void main(String[] args) { out.println(">>> getoffers buy usd"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "buy", "usd"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=buy", "--currency-code=usd"}); out.println(">>> getoffers sell usd"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "sell", "usd"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=sell", "--currency-code=usd"}); out.println(">>> getoffers buy eur"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "buy", "eur"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=buy", "--currency-code=eur"}); out.println(">>> getoffers sell eur"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "sell", "eur"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=sell", "--currency-code=eur"}); out.println(">>> getoffers buy gbp"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "buy", "gbp"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=buy", "--currency-code=gbp"}); out.println(">>> getoffers sell gbp"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "sell", "gbp"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=sell", "--currency-code=gbp"}); out.println(">>> getoffers buy brl"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "buy", "brl"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=buy", "--currency-code=brl"}); out.println(">>> getoffers sell brl"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "sell", "brl"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=sell", "--currency-code=brl"}); } } From d01a7b7feec7167b66ec808529d2e63ffe248442 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:10:42 -0300 Subject: [PATCH 23/35] Improve required-argument opt validation Make sure non-empty opt values are present in the parsers' custom validation, or jsimpleopt will throw exceptions with stylistically inconsisent messages. --- .../bisq/cli/opts/CancelOfferOptionParser.java | 2 +- .../bisq/cli/opts/CreateOfferOptionParser.java | 16 +++++++++++----- .../cli/opts/CreatePaymentAcctOptionParser.java | 2 +- .../cli/opts/GetAddressBalanceOptionParser.java | 2 +- .../cli/opts/GetBTCMarketPriceOptionParser.java | 2 +- .../java/bisq/cli/opts/GetOfferOptionParser.java | 2 +- .../bisq/cli/opts/GetOffersOptionParser.java | 4 ++-- .../cli/opts/GetPaymentAcctFormOptionParser.java | 2 +- .../java/bisq/cli/opts/GetTradeOptionParser.java | 2 +- .../cli/opts/GetTransactionOptionParser.java | 2 +- .../opts/RegisterDisputeAgentOptionParser.java | 4 ++-- .../opts/RemoveWalletPasswordOptionParser.java | 2 +- .../java/bisq/cli/opts/SendBsqOptionParser.java | 4 ++-- .../java/bisq/cli/opts/SendBtcOptionParser.java | 4 ++-- .../bisq/cli/opts/SetTxFeeRateOptionParser.java | 2 +- .../cli/opts/SetWalletPasswordOptionParser.java | 2 +- .../bisq/cli/opts/TakeOfferOptionParser.java | 4 ++-- .../bisq/cli/opts/UnlockWalletOptionParser.java | 2 +- .../bisq/cli/opts/WithdrawFundsOptionParser.java | 4 ++-- 19 files changed, 35 insertions(+), 29 deletions(-) diff --git a/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java index 5ed33ed41fb..c35ffc7bfb4 100644 --- a/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java @@ -38,7 +38,7 @@ public CancelOfferOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(offerIdOpt)) + if (!options.has(offerIdOpt) || options.valueOf(offerIdOpt).isEmpty()) throw new IllegalArgumentException("no offer id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java index bbaf68bab36..42cf8ad1550 100644 --- a/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java @@ -70,22 +70,28 @@ public CreateOfferOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(paymentAccountIdOpt)) + if (!options.has(paymentAccountIdOpt) || options.valueOf(paymentAccountIdOpt).isEmpty()) throw new IllegalArgumentException("no payment account id specified"); - if (!options.has(directionOpt)) + if (!options.has(directionOpt) || options.valueOf(directionOpt).isEmpty()) throw new IllegalArgumentException("no direction (buy|sell) specified"); - if (!options.has(currencyCodeOpt)) + if (!options.has(currencyCodeOpt) || options.valueOf(currencyCodeOpt).isEmpty()) throw new IllegalArgumentException("no currency code specified"); - if (!options.has(amountOpt)) + if (!options.has(amountOpt) || options.valueOf(amountOpt).isEmpty()) throw new IllegalArgumentException("no btc amount specified"); if (!options.has(mktPriceMarginOpt) && !options.has(fixedPriceOpt)) throw new IllegalArgumentException("no market price margin or fixed price specified"); - if (!options.has(securityDepositOpt)) + if (options.has(mktPriceMarginOpt) && options.valueOf(mktPriceMarginOpt).isEmpty()) + throw new IllegalArgumentException("no market price margin specified"); + + if (options.has(fixedPriceOpt) && options.valueOf(fixedPriceOpt).isEmpty()) + throw new IllegalArgumentException("no fixed price specified"); + + if (!options.has(securityDepositOpt) || options.valueOf(securityDepositOpt).isEmpty()) throw new IllegalArgumentException("no security deposit specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java b/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java index 1d4a85ed497..907f7ca0ed5 100644 --- a/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java @@ -43,7 +43,7 @@ public CreatePaymentAcctOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(paymentAcctFormPathOpt)) + if (!options.has(paymentAcctFormPathOpt) || options.valueOf(paymentAcctFormPathOpt).isEmpty()) throw new IllegalArgumentException("no path to json payment account form specified"); Path path = Paths.get(options.valueOf(paymentAcctFormPathOpt)); diff --git a/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java index 254fed91e65..4ad693ca4c6 100644 --- a/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java @@ -38,7 +38,7 @@ public GetAddressBalanceOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(addressOpt)) + if (!options.has(addressOpt) || options.valueOf(addressOpt).isEmpty()) throw new IllegalArgumentException("no address specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java index da66b6b41f1..8d6585631bb 100644 --- a/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java @@ -38,7 +38,7 @@ public GetBTCMarketPriceOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(currencyCodeOpt)) + if (!options.has(currencyCodeOpt) || options.valueOf(currencyCodeOpt).isEmpty()) throw new IllegalArgumentException("no currency code specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java index 159f459be2d..1a849654cf7 100644 --- a/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java @@ -38,7 +38,7 @@ public GetOfferOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(offerIdOpt)) + if (!options.has(offerIdOpt) || options.valueOf(offerIdOpt).isEmpty()) throw new IllegalArgumentException("no offer id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java index dbc1bf83a99..f8a4dee839f 100644 --- a/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java @@ -42,10 +42,10 @@ public GetOffersOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(directionOpt)) + if (!options.has(directionOpt) || options.valueOf(directionOpt).isEmpty()) throw new IllegalArgumentException("no direction (buy|sell) specified"); - if (!options.has(currencyCodeOpt)) + if (!options.has(currencyCodeOpt) || options.valueOf(currencyCodeOpt).isEmpty()) throw new IllegalArgumentException("no currency code specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java index 52fdfd52999..508069c2f30 100644 --- a/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java @@ -39,7 +39,7 @@ public GetPaymentAcctFormOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(paymentMethodIdOpt)) + if (!options.has(paymentMethodIdOpt) || options.valueOf(paymentMethodIdOpt).isEmpty()) throw new IllegalArgumentException("no payment method id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java index 3ea344a06ee..1419f3ed6a6 100644 --- a/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java @@ -44,7 +44,7 @@ public GetTradeOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(tradeIdOpt)) + if (!options.has(tradeIdOpt) || options.valueOf(tradeIdOpt).isEmpty()) throw new IllegalArgumentException("no trade id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java index 83cc4f48fa7..0b245cb1560 100644 --- a/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java @@ -38,7 +38,7 @@ public GetTransactionOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(txIdOpt)) + if (!options.has(txIdOpt) || options.valueOf(txIdOpt).isEmpty()) throw new IllegalArgumentException("no tx id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java b/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java index 9a3a8e7525d..b1c8f0bba8f 100644 --- a/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java @@ -42,10 +42,10 @@ public RegisterDisputeAgentOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(disputeAgentTypeOpt)) + if (!options.has(disputeAgentTypeOpt) || options.valueOf(disputeAgentTypeOpt).isEmpty()) throw new IllegalArgumentException("no dispute agent type specified"); - if (!options.has(registrationKeyOpt)) + if (!options.has(registrationKeyOpt) || options.valueOf(registrationKeyOpt).isEmpty()) throw new IllegalArgumentException("no registration key specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java b/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java index dd1de67f057..db556a0fd89 100644 --- a/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java @@ -38,7 +38,7 @@ public RemoveWalletPasswordOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(passwordOpt)) + if (!options.has(passwordOpt) || options.valueOf(passwordOpt).isEmpty()) throw new IllegalArgumentException("no password specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java b/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java index 73025a17bf0..ad9ab87cbba 100644 --- a/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java @@ -48,10 +48,10 @@ public SendBsqOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(addressOpt)) + if (!options.has(addressOpt) || options.valueOf(addressOpt).isEmpty()) throw new IllegalArgumentException("no bsq address specified"); - if (!options.has(amountOpt)) + if (!options.has(amountOpt) || options.valueOf(amountOpt).isEmpty()) throw new IllegalArgumentException("no bsq amount specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java b/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java index e49e961a1f5..f7d8fd56835 100644 --- a/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java @@ -53,10 +53,10 @@ public SendBtcOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(addressOpt)) + if (!options.has(addressOpt) || options.valueOf(addressOpt).isEmpty()) throw new IllegalArgumentException("no btc address specified"); - if (!options.has(amountOpt)) + if (!options.has(amountOpt) || options.valueOf(amountOpt).isEmpty()) throw new IllegalArgumentException("no btc amount specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java b/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java index f6b9f7c7a06..f7ed113cb3c 100644 --- a/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java @@ -39,7 +39,7 @@ public SetTxFeeRateOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(feeRateOpt)) + if (!options.has(feeRateOpt) || options.valueOf(feeRateOpt).isEmpty()) throw new IllegalArgumentException("no tx fee rate specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java b/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java index 483d56f649e..1caa09232c1 100644 --- a/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java @@ -44,7 +44,7 @@ public SetWalletPasswordOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(passwordOpt)) + if (!options.has(passwordOpt) || options.valueOf(passwordOpt).isEmpty()) throw new IllegalArgumentException("no password specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java index 589860cd63e..67fbdd8c86d 100644 --- a/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java @@ -47,10 +47,10 @@ public TakeOfferOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(offerIdOpt)) + if (!options.has(offerIdOpt) || options.valueOf(offerIdOpt).isEmpty()) throw new IllegalArgumentException("no offer id specified"); - if (!options.has(paymentAccountIdOpt)) + if (!options.has(paymentAccountIdOpt) || options.valueOf(paymentAccountIdOpt).isEmpty()) throw new IllegalArgumentException("no payment account id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java b/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java index db2f1aa2606..2908be42bfd 100644 --- a/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java @@ -44,7 +44,7 @@ public UnlockWalletOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(passwordOpt)) + if (!options.has(passwordOpt) || options.valueOf(passwordOpt).isEmpty()) throw new IllegalArgumentException("no password specified"); if (!options.has(unlockTimeoutOpt) || options.valueOf(unlockTimeoutOpt) <= 0) diff --git a/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java b/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java index b60829e9bef..bdba6437603 100644 --- a/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java @@ -48,10 +48,10 @@ public WithdrawFundsOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(tradeIdOpt)) + if (!options.has(tradeIdOpt) || options.valueOf(tradeIdOpt).isEmpty()) throw new IllegalArgumentException("no trade id specified"); - if (!options.has(addressOpt)) + if (!options.has(addressOpt) || options.valueOf(addressOpt).isEmpty()) throw new IllegalArgumentException("no destination address specified"); return this; From 304781ce92c13df187b26d1ac2ceec5f5eac3825 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:15:01 -0300 Subject: [PATCH 24/35] Add jupiter test support to :cli subproject --- build.gradle | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/build.gradle b/build.gradle index a533c31f374..921f074addf 100644 --- a/build.gradle +++ b/build.gradle @@ -378,6 +378,17 @@ configure(project(':cli')) { implementation "ch.qos.logback:logback-classic:$logbackVersion" compileOnly "org.projectlombok:lombok:$lombokVersion" annotationProcessor "org.projectlombok:lombok:$lombokVersion" + + testImplementation "org.junit.jupiter:junit-jupiter-api:$jupiterVersion" + testImplementation "org.junit.jupiter:junit-jupiter-params:$jupiterVersion" + testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:$jupiterVersion") + testAnnotationProcessor "org.projectlombok:lombok:$lombokVersion" + testCompileOnly "org.projectlombok:lombok:$lombokVersion" + testRuntime "javax.annotation:javax.annotation-api:$javaxAnnotationVersion" + } + + test { + useJUnitPlatform() } } From 9c12b310199cce330977fddca6d93c9d767552f2 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:16:36 -0300 Subject: [PATCH 25/35] Add new cli option parser test Does not check every combination of options for every method, but we finally have an option parser test case to build on. --- .../java/bisq/cli/opt/OptionParsersTest.java | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 cli/src/test/java/bisq/cli/opt/OptionParsersTest.java diff --git a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java new file mode 100644 index 00000000000..9e7cd45cb0e --- /dev/null +++ b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java @@ -0,0 +1,160 @@ +package bisq.cli.opt; + +import org.junit.jupiter.api.Test; + +import static bisq.cli.Method.canceloffer; +import static bisq.cli.Method.createoffer; +import static bisq.cli.Method.createpaymentacct; +import static bisq.cli.opts.OptLabel.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + + + +import bisq.cli.opts.CancelOfferOptionParser; +import bisq.cli.opts.CreateOfferOptionParser; +import bisq.cli.opts.CreatePaymentAcctOptionParser; + + +public class OptionParsersTest { + + private static final String PASSWORD_OPT = "--" + OPT_PASSWORD + "=" + "xyz"; + + // CancelOffer opt parsing tests + + @Test + public void testCancelOfferWithMissingOfferIdOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + canceloffer.name() + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CancelOfferOptionParser(args).parse()); + assertEquals("no offer id specified", exception.getMessage()); + } + + @Test + public void testCancelOfferWithEmptyOfferIdOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + canceloffer.name(), + "--" + OPT_OFFER_ID + "=" // missing opt value + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CancelOfferOptionParser(args).parse()); + assertEquals("no offer id specified", exception.getMessage()); + } + + @Test + public void testCancelOfferWithMissingOfferIdValueShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + canceloffer.name(), + "--" + OPT_OFFER_ID // missing equals sign & opt value + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CancelOfferOptionParser(args).parse()); + assertEquals("Option offer-id requires an argument", exception.getMessage()); + } + + @Test + public void testValidCancelOfferOpts() { + String[] args = new String[]{ + PASSWORD_OPT, + canceloffer.name(), + "--" + OPT_OFFER_ID + "=" + "ABC-OFFER-ID" + }; + new CancelOfferOptionParser(args).parse(); + } + + // CreateOffer opt parsing tests + + @Test + public void testCreateOfferOptParserWithMissingPaymentAccountIdOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name() + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreateOfferOptionParser(args).parse()); + assertEquals("no payment account id specified", exception.getMessage()); + } + + @Test + public void testCreateOfferOptParserWithMissingDirectionOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name(), + "--" + OPT_PAYMENT_ACCOUNT + "=" + "abc-payment-acct-id-123" + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreateOfferOptionParser(args).parse()); + assertEquals("no direction (buy|sell) specified", exception.getMessage()); + } + + @Test + public void testCreateOfferOptParserWithMissingDirectionOptValueShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name(), + "--" + OPT_PAYMENT_ACCOUNT + "=" + "abc-payment-acct-id-123", + "--" + OPT_DIRECTION + "=" + "" + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreateOfferOptionParser(args).parse()); + assertEquals("no direction (buy|sell) specified", exception.getMessage()); + } + + @Test + public void testValidCreateOfferOpts() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name(), + "--" + OPT_PAYMENT_ACCOUNT + "=" + "abc-payment-acct-id-123", + "--" + OPT_DIRECTION + "=" + "BUY", + "--" + OPT_CURRENCY_CODE + "=" + "EUR", + "--" + OPT_AMOUNT + "=" + "0.125", + "--" + OPT_MKT_PRICE_MARGIN + "=" + "0.0", + "--" + OPT_SECURITY_DEPOSIT + "=" + "25.0" + }; + } + + // CreatePaymentAcct opt parser tests + + @Test + public void testCreatePaymentAcctOptParserWithMissingPaymentFormOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createpaymentacct.name() + // OPT_PAYMENT_ACCOUNT_FORM + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreatePaymentAcctOptionParser(args).parse()); + assertEquals("no path to json payment account form specified", exception.getMessage()); + } + + @Test + public void testCreatePaymentAcctOptParserWithMissingPaymentFormOptValueShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createpaymentacct.name(), + "--" + OPT_PAYMENT_ACCOUNT_FORM + "=" + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreatePaymentAcctOptionParser(args).parse()); + assertEquals("no path to json payment account form specified", exception.getMessage()); + } + + @Test + public void testCreatePaymentAcctOptParserWithInvalidPaymentFormOptValueShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createpaymentacct.name(), + "--" + OPT_PAYMENT_ACCOUNT_FORM + "=" + "/tmp/milkyway/solarsystem/mars" + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreatePaymentAcctOptionParser(args).parse()); + assertEquals("json payment account form '/tmp/milkyway/solarsystem/mars' could not be found", + exception.getMessage()); + } +} From a13ef79e6e606cefa04587e56bcff1181ecfb335 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 12:28:10 -0300 Subject: [PATCH 26/35] Handle require-arg options missing the = sign This change ensure a stylistically consistent error message is printed to the CLI console if an option is present, but without the '=' sign. --- .../cli/opts/AbstractMethodOptionParser.java | 25 ++++++++++++++++--- .../java/bisq/cli/opt/OptionParsersTest.java | 15 ++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/cli/src/main/java/bisq/cli/opts/AbstractMethodOptionParser.java b/cli/src/main/java/bisq/cli/opts/AbstractMethodOptionParser.java index 4c495c06e5e..25256eb6a99 100644 --- a/cli/src/main/java/bisq/cli/opts/AbstractMethodOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/AbstractMethodOptionParser.java @@ -17,11 +17,13 @@ package bisq.cli.opts; +import joptsimple.OptionException; import joptsimple.OptionParser; import joptsimple.OptionSet; import joptsimple.OptionSpec; import java.util.List; +import java.util.function.Function; import lombok.Getter; @@ -48,12 +50,29 @@ protected AbstractMethodOptionParser(String[] args) { } public AbstractMethodOptionParser parse() { - options = parser.parse(new ArgumentList(args).getMethodArguments()); - nonOptionArguments = (List) options.nonOptionArguments(); - return this; + try { + options = parser.parse(new ArgumentList(args).getMethodArguments()); + //noinspection unchecked + nonOptionArguments = (List) options.nonOptionArguments(); + return this; + } catch (OptionException ex) { + throw new IllegalArgumentException(cliExceptionMessageStyle.apply(ex), ex); + } } public boolean isForHelp() { return options.has(helpOpt); } + + private final Function cliExceptionMessageStyle = (ex) -> { + if (ex.getMessage() == null) + return null; + + var optionToken = "option "; + var cliMessage = ex.getMessage().toLowerCase(); + if (cliMessage.startsWith(optionToken) && cliMessage.length() > optionToken.length()) { + cliMessage = cliMessage.substring(cliMessage.indexOf(" ") + 1); + } + return cliMessage; + }; } diff --git a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java index 9e7cd45cb0e..d853da66d6a 100644 --- a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java +++ b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java @@ -54,7 +54,7 @@ public void testCancelOfferWithMissingOfferIdValueShouldThrowException() { }; Throwable exception = assertThrows(RuntimeException.class, () -> new CancelOfferOptionParser(args).parse()); - assertEquals("Option offer-id requires an argument", exception.getMessage()); + assertEquals("offer-id requires an argument", exception.getMessage()); } @Test @@ -80,6 +80,18 @@ public void testCreateOfferOptParserWithMissingPaymentAccountIdOptShouldThrowExc assertEquals("no payment account id specified", exception.getMessage()); } + @Test + public void testCreateOfferOptParserWithEmptyPaymentAccountIdOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name(), + "--" + OPT_PAYMENT_ACCOUNT + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreateOfferOptionParser(args).parse()); + assertEquals("payment-account requires an argument", exception.getMessage()); + } + @Test public void testCreateOfferOptParserWithMissingDirectionOptShouldThrowException() { String[] args = new String[]{ @@ -92,6 +104,7 @@ public void testCreateOfferOptParserWithMissingDirectionOptShouldThrowException( assertEquals("no direction (buy|sell) specified", exception.getMessage()); } + @Test public void testCreateOfferOptParserWithMissingDirectionOptValueShouldThrowException() { String[] args = new String[]{ From 99fea74e096af799d53648caa31d9c0923d0658b Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sun, 7 Mar 2021 19:51:29 -0300 Subject: [PATCH 27/35] Validate offer <-> payment-acct in createoffer This change prohibits creation of new offers with incompatible payment accounts with the api. - PaymentAccountUtil Renamed isAnyTakerPaymentAccountValidForOffer -> isAnyPaymentAccountValidForOffer. Renmaed isTakerPaymentAccountValidForOffer -> isPaymentAccountValidForOffer. Deleted commented code. - PaymentAccounts: Adjusted to PaymentAccountUtil method name changes. - OfferFilter: Adjusted to PaymentAccountUtil method name changes. - OfferBookViewModelTest: Adjusted to PaymentAccountUtil method name changes. - Add CoreOffersService#verifyPaymentAccountIsValidForOffer - ValidateCreateOfferTest, OfferTest: Added test cases. --- .../method/offer/ValidateCreateOfferTest.java | 37 ++++++++++++++++ .../java/bisq/apitest/scenario/OfferTest.java | 2 + .../java/bisq/core/api/CoreOffersService.java | 12 +++++ .../java/bisq/core/offer/OfferFilter.java | 2 +- .../bisq/core/payment/PaymentAccountUtil.java | 31 +++---------- .../bisq/core/payment/PaymentAccounts.java | 2 +- .../offerbook/OfferBookViewModelTest.java | 44 +++++++++---------- 7 files changed, 81 insertions(+), 49 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/offer/ValidateCreateOfferTest.java b/apitest/src/test/java/bisq/apitest/method/offer/ValidateCreateOfferTest.java index a979b7bf239..65f8c83f607 100644 --- a/apitest/src/test/java/bisq/apitest/method/offer/ValidateCreateOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/offer/ValidateCreateOfferTest.java @@ -30,6 +30,7 @@ import org.junit.jupiter.api.TestMethodOrder; import static bisq.core.btc.wallet.Restrictions.getDefaultBuyerSecurityDepositAsPercent; +import static java.lang.String.format; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -55,4 +56,40 @@ public void testAmtTooLargeShouldThrowException() { assertEquals("UNKNOWN: An error occurred at task: ValidateOffer", exception.getMessage()); } + + @Test + @Order(2) + public void testNoMatchingEURPaymentAccountShouldThrowException() { + PaymentAccount chfAccount = createDummyF2FAccount(aliceClient, "ch"); + @SuppressWarnings("ResultOfMethodCallIgnored") + Throwable exception = assertThrows(StatusRuntimeException.class, () -> + aliceClient.createFixedPricedOffer("buy", + "eur", + 10000000L, + 10000000L, + "40000.0000", + getDefaultBuyerSecurityDepositAsPercent(), + chfAccount.getId(), + "btc")); + String expectedError = format("UNKNOWN: cannot create EUR offer with payment account %s", chfAccount.getId()); + assertEquals(expectedError, exception.getMessage()); + } + + @Test + @Order(2) + public void testNoMatchingCADPaymentAccountShouldThrowException() { + PaymentAccount audAccount = createDummyF2FAccount(aliceClient, "au"); + @SuppressWarnings("ResultOfMethodCallIgnored") + Throwable exception = assertThrows(StatusRuntimeException.class, () -> + aliceClient.createFixedPricedOffer("buy", + "cad", + 10000000L, + 10000000L, + "63000.0000", + getDefaultBuyerSecurityDepositAsPercent(), + audAccount.getId(), + "btc")); + String expectedError = format("UNKNOWN: cannot create CAD offer with payment account %s", audAccount.getId()); + assertEquals(expectedError, exception.getMessage()); + } } diff --git a/apitest/src/test/java/bisq/apitest/scenario/OfferTest.java b/apitest/src/test/java/bisq/apitest/scenario/OfferTest.java index b01a9486ea5..c82cbaef90e 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/OfferTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/OfferTest.java @@ -42,6 +42,8 @@ public class OfferTest extends AbstractOfferTest { public void testAmtTooLargeShouldThrowException() { ValidateCreateOfferTest test = new ValidateCreateOfferTest(); test.testAmtTooLargeShouldThrowException(); + test.testNoMatchingEURPaymentAccountShouldThrowException(); + test.testNoMatchingCADPaymentAccountShouldThrowException(); } @Test diff --git a/core/src/main/java/bisq/core/api/CoreOffersService.java b/core/src/main/java/bisq/core/api/CoreOffersService.java index 63420e0bbb1..3e6c6f621a2 100644 --- a/core/src/main/java/bisq/core/api/CoreOffersService.java +++ b/core/src/main/java/bisq/core/api/CoreOffersService.java @@ -54,6 +54,7 @@ import static bisq.core.locale.CurrencyUtil.isCryptoCurrency; import static bisq.core.offer.OfferPayload.Direction; import static bisq.core.offer.OfferPayload.Direction.BUY; +import static bisq.core.payment.PaymentAccountUtil.isPaymentAccountValidForOffer; import static java.lang.String.format; import static java.util.Comparator.comparing; @@ -174,6 +175,8 @@ void createAndPlaceOffer(String currencyCode, buyerSecurityDeposit, paymentAccount); + verifyPaymentAccountIsValidForNewOffer(offer, paymentAccount); + // We don't support atm funding from external wallet to keep it simple. boolean useSavingsWallet = true; //noinspection ConstantConditions @@ -219,6 +222,15 @@ void cancelOffer(String id) { }); } + private void verifyPaymentAccountIsValidForNewOffer(Offer offer, PaymentAccount paymentAccount) { + if (!isPaymentAccountValidForOffer(offer, paymentAccount)) { + String error = String.format("cannot create %s offer with payment account %s", + offer.getOfferPayload().getCounterCurrencyCode(), + paymentAccount.getId()); + throw new IllegalStateException(error); + } + } + private void placeOffer(Offer offer, double buyerSecurityDeposit, long triggerPrice, diff --git a/core/src/main/java/bisq/core/offer/OfferFilter.java b/core/src/main/java/bisq/core/offer/OfferFilter.java index c22231de5bb..bfd37f2af12 100644 --- a/core/src/main/java/bisq/core/offer/OfferFilter.java +++ b/core/src/main/java/bisq/core/offer/OfferFilter.java @@ -134,7 +134,7 @@ public Result canTakeOffer(Offer offer, boolean isTakerApiUser) { public boolean isAnyPaymentAccountValidForOffer(Offer offer) { return user.getPaymentAccounts() != null && - PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer(offer, user.getPaymentAccounts()); + PaymentAccountUtil.isAnyPaymentAccountValidForOffer(offer, user.getPaymentAccounts()); } public boolean hasSameProtocolVersion(Offer offer) { diff --git a/core/src/main/java/bisq/core/payment/PaymentAccountUtil.java b/core/src/main/java/bisq/core/payment/PaymentAccountUtil.java index 08a96db151b..fc810b017d4 100644 --- a/core/src/main/java/bisq/core/payment/PaymentAccountUtil.java +++ b/core/src/main/java/bisq/core/payment/PaymentAccountUtil.java @@ -41,10 +41,10 @@ @Slf4j public class PaymentAccountUtil { - public static boolean isAnyTakerPaymentAccountValidForOffer(Offer offer, - Collection takerPaymentAccounts) { - for (PaymentAccount takerPaymentAccount : takerPaymentAccounts) { - if (isTakerPaymentAccountValidForOffer(offer, takerPaymentAccount)) + public static boolean isAnyPaymentAccountValidForOffer(Offer offer, + Collection paymentAccounts) { + for (PaymentAccount paymentAccount : paymentAccounts) { + if (isPaymentAccountValidForOffer(offer, paymentAccount)) return true; } return false; @@ -55,7 +55,7 @@ public static ObservableList getPossiblePaymentAccounts(Offer of AccountAgeWitnessService accountAgeWitnessService) { ObservableList result = FXCollections.observableArrayList(); result.addAll(paymentAccounts.stream() - .filter(paymentAccount -> isTakerPaymentAccountValidForOffer(offer, paymentAccount)) + .filter(paymentAccount -> isPaymentAccountValidForOffer(offer, paymentAccount)) .filter(paymentAccount -> isAmountValidForOffer(offer, paymentAccount, accountAgeWitnessService)) .collect(Collectors.toList())); return result; @@ -79,7 +79,7 @@ public static String getInfoForMismatchingPaymentMethodLimits(Offer offer, Payme "Payment method from offer: " + offer.getPaymentMethod().toString(); } - public static boolean isTakerPaymentAccountValidForOffer(Offer offer, PaymentAccount paymentAccount) { + public static boolean isPaymentAccountValidForOffer(Offer offer, PaymentAccount paymentAccount) { return new ReceiptValidator(offer, paymentAccount).isValid(); } @@ -144,23 +144,4 @@ public static Optional findPaymentAccount(PaymentAccountPayload filter(e -> e.getPaymentAccountPayload().equals(paymentAccountPayload)) .findAny(); } - - // TODO no code duplication found in UI code (added for API) - // That is optional and set to null if not supported (AltCoins,...) - /* public static String getCountryCode(PaymentAccount paymentAccount) { - if (paymentAccount instanceof CountryBasedPaymentAccount) { - Country country = ((CountryBasedPaymentAccount) paymentAccount).getCountry(); - return country != null ? country.code : null; - } else { - return null; - } - }*/ - - // TODO no code duplication found in UI code (added for API) - /*public static long getMaxTradeLimit(AccountAgeWitnessService accountAgeWitnessService, PaymentAccount paymentAccount, String currencyCode) { - if (paymentAccount != null) - return accountAgeWitnessService.getMyTradeLimit(paymentAccount, currencyCode); - else - return 0; - }*/ } diff --git a/core/src/main/java/bisq/core/payment/PaymentAccounts.java b/core/src/main/java/bisq/core/payment/PaymentAccounts.java index c27c883449d..6f871ce20f9 100644 --- a/core/src/main/java/bisq/core/payment/PaymentAccounts.java +++ b/core/src/main/java/bisq/core/payment/PaymentAccounts.java @@ -41,7 +41,7 @@ class PaymentAccounts { private final BiFunction validator; PaymentAccounts(Set accounts, AccountAgeWitnessService accountAgeWitnessService) { - this(accounts, accountAgeWitnessService, PaymentAccountUtil::isTakerPaymentAccountValidForOffer); + this(accounts, accountAgeWitnessService, PaymentAccountUtil::isPaymentAccountValidForOffer); } PaymentAccounts(Set accounts, AccountAgeWitnessService accountAgeWitnessService, diff --git a/desktop/src/test/java/bisq/desktop/main/offer/offerbook/OfferBookViewModelTest.java b/desktop/src/test/java/bisq/desktop/main/offer/offerbook/OfferBookViewModelTest.java index dc48c8c5459..68b8718e35c 100644 --- a/desktop/src/test/java/bisq/desktop/main/offer/offerbook/OfferBookViewModelTest.java +++ b/desktop/src/test/java/bisq/desktop/main/offer/offerbook/OfferBookViewModelTest.java @@ -105,46 +105,46 @@ public void testIsAnyPaymentAccountValidForOffer() { Collection paymentAccounts; paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSepaAccount("EUR", "DE", "1212324", new ArrayList<>(Arrays.asList("AT", "DE"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // empty paymentAccounts paymentAccounts = new ArrayList<>(); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer(getSEPAPaymentMethod("EUR", "AT", + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer(getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // simple cases: same payment methods // offer: alipay paymentAccount: alipay - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getAliPayAccount("CNY"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getAliPayPaymentMethod("EUR"), paymentAccounts)); // offer: ether paymentAccount: ether - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getCryptoAccount("ETH"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getBlockChainsPaymentMethod("ETH"), paymentAccounts)); // offer: sepa paymentAccount: sepa - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSepaAccount("EUR", "AT", "1212324", new ArrayList<>(Arrays.asList("AT", "DE"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // offer: nationalBank paymentAccount: nationalBank - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("EUR", "AT", "PSK"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // offer: SameBank paymentAccount: SameBank - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("EUR", "AT", "PSK"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSameBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // offer: sepa paymentAccount: sepa - diff. country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSepaAccount("EUR", "DE", "1212324", new ArrayList<>(Arrays.asList("AT", "DE"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); @@ -152,79 +152,79 @@ public void testIsAnyPaymentAccountValidForOffer() { // offer: sepa paymentAccount: sepa - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSepaAccount("EUR", "AT", "1212324", new ArrayList<>(Arrays.asList("AT", "DE"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // offer: sepa paymentAccount: nationalBank - same country, same currency // wrong method paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("EUR", "AT", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // wrong currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("USD", "US", "XXX"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // wrong country paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("EUR", "FR", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // sepa wrong country paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("EUR", "CH", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // sepa wrong currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("CHF", "DE", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // same bank paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("EUR", "AT", "PSK"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // not same bank paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("EUR", "AT", "Raika"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // same bank, wrong country paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("EUR", "DE", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // same bank, wrong currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("USD", "AT", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // spec. bank paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSpecificBanksAccount("EUR", "AT", "PSK", new ArrayList<>(Arrays.asList("PSK", "Raika"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // spec. bank, missing bank paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSpecificBanksAccount("EUR", "AT", "PSK", new ArrayList<>(FXCollections.singletonObservableList("Raika"))))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // spec. bank, wrong country paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSpecificBanksAccount("EUR", "FR", "PSK", new ArrayList<>(Arrays.asList("PSK", "Raika"))))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // spec. bank, wrong currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSpecificBanksAccount("USD", "AT", "PSK", new ArrayList<>(Arrays.asList("PSK", "Raika"))))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); //TODO add more tests From baca7001ea5081784691e20f1fcf2ad2e1fb4a84 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sun, 7 Mar 2021 20:05:39 -0300 Subject: [PATCH 28/35] Fix unnecessary use of fully qualified name 'String.format' --- core/src/main/java/bisq/core/api/CoreOffersService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/api/CoreOffersService.java b/core/src/main/java/bisq/core/api/CoreOffersService.java index 3e6c6f621a2..8a5b95cf31e 100644 --- a/core/src/main/java/bisq/core/api/CoreOffersService.java +++ b/core/src/main/java/bisq/core/api/CoreOffersService.java @@ -55,6 +55,7 @@ import static bisq.core.offer.OfferPayload.Direction; import static bisq.core.offer.OfferPayload.Direction.BUY; import static bisq.core.payment.PaymentAccountUtil.isPaymentAccountValidForOffer; +import static java.lang.String.*; import static java.lang.String.format; import static java.util.Comparator.comparing; @@ -224,7 +225,7 @@ void cancelOffer(String id) { private void verifyPaymentAccountIsValidForNewOffer(Offer offer, PaymentAccount paymentAccount) { if (!isPaymentAccountValidForOffer(offer, paymentAccount)) { - String error = String.format("cannot create %s offer with payment account %s", + String error = format("cannot create %s offer with payment account %s", offer.getOfferPayload().getCounterCurrencyCode(), paymentAccount.getId()); throw new IllegalStateException(error); From b4f4d90e05d4b3779fed765dce0a200b796aa1d4 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 14:47:30 -0300 Subject: [PATCH 29/35] Add tradeCurrencies field to Transferwise acct form These changes make tradeCurrencies a required twise acct form field. Addresses issue https://github.com/bisq-network/bisq/issues/5281 - Added comment to PaymentAccountForm about special 'tradeCurrencies' field handling. - Added support for Transferwise 'tradeCurrencies' to PaymentAccountTypeAdapter. - Added instance of isTransferwiseAccount check to PaymentAccount. - Added methods to CurrencyUtil to convert comma delimited code list to TradeCurrency list. - Added methods to ReflectionUtil for getting fields and methods on class. - Added required field check to CorePaymentAccountsService. - Added CreatePaymentAccount test cases. --- .../payment/AbstractPaymentAccountTest.java | 1 + .../payment/CreatePaymentAccountTest.java | 95 ++++++++++++++-- .../apitest/scenario/PaymentAccountTest.java | 18 ++- .../bisq/common/util/ReflectionUtils.java | 32 ++++++ .../core/api/CorePaymentAccountsService.java | 10 ++ .../core/api/model/PaymentAccountForm.java | 2 +- .../api/model/PaymentAccountTypeAdapter.java | 107 +++++++++++++++--- .../java/bisq/core/locale/CurrencyUtil.java | 28 +++++ .../bisq/core/payment/PaymentAccount.java | 4 + 9 files changed, 261 insertions(+), 36 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/AbstractPaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/AbstractPaymentAccountTest.java index 44c56f08a05..034bc3aeb02 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/AbstractPaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/AbstractPaymentAccountTest.java @@ -85,6 +85,7 @@ public class AbstractPaymentAccountTest extends MethodTest { static final String PROPERTY_NAME_SALT = "salt"; static final String PROPERTY_NAME_SORT_CODE = "sortCode"; static final String PROPERTY_NAME_STATE = "state"; + static final String PROPERTY_NAME_TRADE_CURRENCIES = "tradeCurrencies"; static final String PROPERTY_NAME_USERNAME = "userName"; static final Gson GSON = new GsonBuilder() diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index a194c4aa1f9..2cc5bfcf857 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -17,6 +17,7 @@ package bisq.apitest.method.payment; +import bisq.core.locale.TradeCurrency; import bisq.core.payment.AdvancedCashAccount; import bisq.core.payment.AliPayAccount; import bisq.core.payment.AustraliaPayid; @@ -50,8 +51,12 @@ import bisq.core.payment.payload.SameBankAccountPayload; import bisq.core.payment.payload.SpecificBanksAccountPayload; +import io.grpc.StatusRuntimeException; + import java.io.File; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; import lombok.extern.slf4j.Slf4j; @@ -65,12 +70,11 @@ import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind; import static bisq.apitest.config.BisqAppConfig.alicedaemon; -import static bisq.core.locale.CurrencyUtil.getAllAdvancedCashCurrencies; -import static bisq.core.locale.CurrencyUtil.getAllMoneyGramCurrencies; -import static bisq.core.locale.CurrencyUtil.getAllRevolutCurrencies; -import static bisq.core.locale.CurrencyUtil.getAllUpholdCurrencies; +import static bisq.core.locale.CurrencyUtil.*; import static bisq.core.payment.payload.PaymentMethod.*; +import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation; @@ -737,27 +741,98 @@ public void testCreateSwishAccount(TestInfo testInfo) { } @Test - public void testCreateTransferwiseAccount(TestInfo testInfo) { + public void testCreateTransferwiseAccountWith1TradeCurrency(TestInfo testInfo) { File emptyForm = getEmptyForm(testInfo, TRANSFERWISE_ID); verifyEmptyForm(emptyForm, TRANSFERWISE_ID, PROPERTY_NAME_EMAIL); COMPLETED_FORM_MAP.put(PROPERTY_NAME_PAYMENT_METHOD_ID, TRANSFERWISE_ID); COMPLETED_FORM_MAP.put(PROPERTY_NAME_ACCOUNT_NAME, "Transferwise Acct"); - COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jan@doe.info"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_TRADE_CURRENCIES, "eur"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jane@doe.info"); COMPLETED_FORM_MAP.put(PROPERTY_NAME_SALT, ""); String jsonString = getCompletedFormAsJsonString(); TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(aliceClient, jsonString); verifyUserPayloadHasPaymentAccountWithId(aliceClient, paymentAccount.getId()); - // As per commit 88f26f93241af698ae689bf081205d0f9dc929fa - // Do not autofill all currencies by default but keep all unselected. - // verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount); - assertEquals(0, paymentAccount.getTradeCurrencies().size()); + assertEquals(1, paymentAccount.getTradeCurrencies().size()); + List expectedTradeCurrencies = singletonList(getTradeCurrency("EUR").get()); + verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); } + @Test + public void testCreateTransferwiseAccountWith10TradeCurrencies(TestInfo testInfo) { + File emptyForm = getEmptyForm(testInfo, TRANSFERWISE_ID); + verifyEmptyForm(emptyForm, + TRANSFERWISE_ID, + PROPERTY_NAME_EMAIL); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_PAYMENT_METHOD_ID, TRANSFERWISE_ID); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_ACCOUNT_NAME, "Transferwise Acct"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_TRADE_CURRENCIES, "ars, cad, hrk, czk, eur, hkd, idr, jpy, chf, nzd"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jane@doe.info"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_SALT, ""); + String jsonString = getCompletedFormAsJsonString(); + TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(aliceClient, jsonString); + verifyUserPayloadHasPaymentAccountWithId(aliceClient, paymentAccount.getId()); + assertEquals(10, paymentAccount.getTradeCurrencies().size()); + List expectedTradeCurrencies = new ArrayList<>() {{ + add(getTradeCurrency("ARS").get()); + add(getTradeCurrency("CAD").get()); + add(getTradeCurrency("HRK").get()); + add(getTradeCurrency("CZK").get()); + add(getTradeCurrency("EUR").get()); + add(getTradeCurrency("HKD").get()); + add(getTradeCurrency("IDR").get()); + add(getTradeCurrency("JPY").get()); + add(getTradeCurrency("CHF").get()); + add(getTradeCurrency("NZD").get()); + }}; + verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); + verifyCommonFormEntries(paymentAccount); + assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); + log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + } + + @Test + public void testCreateTransferwiseAccountWithInvalidBrlTradeCurrencyShouldThrowException(TestInfo testInfo) { + File emptyForm = getEmptyForm(testInfo, TRANSFERWISE_ID); + verifyEmptyForm(emptyForm, + TRANSFERWISE_ID, + PROPERTY_NAME_EMAIL); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_PAYMENT_METHOD_ID, TRANSFERWISE_ID); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_ACCOUNT_NAME, "Transferwise Acct"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_TRADE_CURRENCIES, "eur, hkd, idr, jpy, chf, nzd, brl, gbp"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jane@doe.info"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_SALT, ""); + String jsonString = getCompletedFormAsJsonString(); + + Throwable exception = assertThrows(StatusRuntimeException.class, () -> + createPaymentAccount(aliceClient, jsonString)); + assertEquals("INVALID_ARGUMENT: BRL is not a member of valid currencies list", + exception.getMessage()); + } + + @Test + public void testCreateTransferwiseAccountWithoutTradeCurrenciesShouldThrowException(TestInfo testInfo) { + File emptyForm = getEmptyForm(testInfo, TRANSFERWISE_ID); + verifyEmptyForm(emptyForm, + TRANSFERWISE_ID, + PROPERTY_NAME_EMAIL); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_PAYMENT_METHOD_ID, TRANSFERWISE_ID); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_ACCOUNT_NAME, "Transferwise Acct"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_TRADE_CURRENCIES, ""); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jane@doe.info"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_SALT, ""); + String jsonString = getCompletedFormAsJsonString(); + + Throwable exception = assertThrows(StatusRuntimeException.class, () -> + createPaymentAccount(aliceClient, jsonString)); + assertEquals("INVALID_ARGUMENT: no trade currencies defined for transferwise payment account", + exception.getMessage()); + } + @Test public void testCreateUpholdAccount(TestInfo testInfo) { File emptyForm = getEmptyForm(testInfo, UPHOLD_ID); diff --git a/apitest/src/test/java/bisq/apitest/scenario/PaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/scenario/PaymentAccountTest.java index e3ce0a0806b..c3eb41343a9 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/PaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/PaymentAccountTest.java @@ -13,8 +13,6 @@ import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind; import static bisq.apitest.config.BisqAppConfig.alicedaemon; import static bisq.apitest.config.BisqAppConfig.seednode; -import static java.util.Objects.requireNonNull; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @@ -27,10 +25,6 @@ @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class PaymentAccountTest extends AbstractPaymentAccountTest { - // Two dummy (usd +eth) accounts are set up as defaults in regtest / dao mode, - // then we add 28 more payment accounts in testCreatePaymentAccount(). - private static final int EXPECTED_NUM_PAYMENT_ACCOUNTS = 2 + 28; - @BeforeAll public static void setUp() { try { @@ -75,14 +69,18 @@ public void testCreatePaymentAccount(TestInfo testInfo) { test.testCreateSepaAccount(testInfo); test.testCreateSpecificBanksAccount(testInfo); test.testCreateSwishAccount(testInfo); - test.testCreateTransferwiseAccount(testInfo); + + // TransferwiseAccount is only PaymentAccount with a + // tradeCurrencies field in the json form. + test.testCreateTransferwiseAccountWith1TradeCurrency(testInfo); + test.testCreateTransferwiseAccountWith10TradeCurrencies(testInfo); + test.testCreateTransferwiseAccountWithInvalidBrlTradeCurrencyShouldThrowException(testInfo); + test.testCreateTransferwiseAccountWithoutTradeCurrenciesShouldThrowException(testInfo); + test.testCreateUpholdAccount(testInfo); test.testCreateUSPostalMoneyOrderAccount(testInfo); test.testCreateWeChatPayAccount(testInfo); test.testCreateWesternUnionAccount(testInfo); - - var paymentAccounts = requireNonNull(aliceClient).getPaymentAccounts(); - assertEquals(EXPECTED_NUM_PAYMENT_ACCOUNTS, paymentAccounts.size()); } @AfterAll diff --git a/common/src/main/java/bisq/common/util/ReflectionUtils.java b/common/src/main/java/bisq/common/util/ReflectionUtils.java index e70162ecb23..c9f83a8f41a 100644 --- a/common/src/main/java/bisq/common/util/ReflectionUtils.java +++ b/common/src/main/java/bisq/common/util/ReflectionUtils.java @@ -28,9 +28,13 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import lombok.extern.slf4j.Slf4j; + +import static java.lang.String.format; import static java.util.Arrays.stream; import static org.apache.commons.lang3.StringUtils.capitalize; +@Slf4j public class ReflectionUtils { /** @@ -105,4 +109,32 @@ else if (Modifier.isPublic(field.getModifiers())) else return ""; } + + public static Field getField(String name, Class clazz) { + Optional field = stream(clazz.getDeclaredFields()) + .filter(f -> f.getName().equals(name)).findFirst(); + return field.orElseThrow(() -> + new IllegalArgumentException(format("field %s not found in class %s", + name, + clazz.getSimpleName()))); + } + + public static Method getMethod(String name, Class clazz) { + Optional method = stream(clazz.getDeclaredMethods()) + .filter(m -> m.getName().equals(name)).findFirst(); + return method.orElseThrow(() -> + new IllegalArgumentException(format("method %s not found in class %s", + name, + clazz.getSimpleName()))); + } + + public static void handleSetFieldValueError(Object object, + Field field, + ReflectiveOperationException ex) { + String errMsg = format("cannot set value of field %s, on class %s", + field.getName(), + object.getClass().getSimpleName()); + log.error(capitalize(errMsg) + ".", ex); + throw new IllegalStateException("programmer error: " + errMsg); + } } diff --git a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java index 095bf2d1f2d..b915b3ab4be 100644 --- a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java +++ b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java @@ -35,6 +35,8 @@ import lombok.extern.slf4j.Slf4j; +import static java.lang.String.format; + @Singleton @Slf4j class CorePaymentAccountsService { @@ -54,6 +56,7 @@ public CorePaymentAccountsService(AccountAgeWitnessService accountAgeWitnessServ PaymentAccount createPaymentAccount(String jsonString) { PaymentAccount paymentAccount = paymentAccountForm.toPaymentAccount(jsonString); + verifyPaymentAccountHasRequiredFields(paymentAccount); user.addPaymentAccountIfNotExists(paymentAccount); accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload()); log.info("Saved payment account with id {} and payment method {}.", @@ -82,4 +85,11 @@ String getPaymentAccountFormAsString(String paymentMethodId) { File getPaymentAccountForm(String paymentMethodId) { return paymentAccountForm.getPaymentAccountForm(paymentMethodId); } + + private void verifyPaymentAccountHasRequiredFields(PaymentAccount paymentAccount) { + // Do checks here to make sure required fields are populated. + if (paymentAccount.isTransferwiseAccount() && paymentAccount.getTradeCurrencies().isEmpty()) + throw new IllegalArgumentException(format("no trade currencies defined for %s payment account", + paymentAccount.getPaymentMethod().getDisplayString().toLowerCase())); + } } diff --git a/core/src/main/java/bisq/core/api/model/PaymentAccountForm.java b/core/src/main/java/bisq/core/api/model/PaymentAccountForm.java index dae437d265d..777e48987d5 100644 --- a/core/src/main/java/bisq/core/api/model/PaymentAccountForm.java +++ b/core/src/main/java/bisq/core/api/model/PaymentAccountForm.java @@ -136,7 +136,7 @@ public class PaymentAccountForm { "paymentMethod", "paymentMethodId", // This field will be included, but handled differently. "selectedTradeCurrency", - "tradeCurrencies", + "tradeCurrencies", // This field may be included, but handled differently. "HOLDER_NAME", "SALT" // This field will be included, but handled differently. }; diff --git a/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java b/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java index f49809a50cd..d8d2e19c8da 100644 --- a/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java +++ b/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java @@ -20,6 +20,7 @@ import bisq.core.locale.Country; import bisq.core.locale.FiatCurrency; +import bisq.core.locale.TradeCurrency; import bisq.core.payment.CountryBasedPaymentAccount; import bisq.core.payment.MoneyGramAccount; import bisq.core.payment.PaymentAccount; @@ -35,12 +36,11 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.Function; import java.util.function.Predicate; import java.lang.reflect.Constructor; @@ -50,16 +50,20 @@ import lombok.extern.slf4j.Slf4j; -import static bisq.common.util.ReflectionUtils.getSetterMethodForFieldInClassHierarchy; -import static bisq.common.util.ReflectionUtils.getVisibilityModifierAsString; -import static bisq.common.util.ReflectionUtils.isSetterOnClass; -import static bisq.common.util.ReflectionUtils.loadFieldListForClassHierarchy; +import static bisq.common.util.ReflectionUtils.*; import static bisq.common.util.Utilities.decodeFromHex; import static bisq.core.locale.CountryUtil.findCountryByCode; +import static bisq.core.locale.CurrencyUtil.getAllTransferwiseCurrencies; import static bisq.core.locale.CurrencyUtil.getCurrencyByCountryCode; +import static bisq.core.locale.CurrencyUtil.getTradeCurrencies; +import static bisq.core.locale.CurrencyUtil.getTradeCurrenciesInList; import static com.google.common.base.Preconditions.checkNotNull; import static java.lang.String.format; +import static java.util.Arrays.stream; +import static java.util.Collections.singletonList; +import static java.util.Collections.unmodifiableMap; import static java.util.Comparator.comparing; +import static java.util.stream.Collectors.toList; @Slf4j class PaymentAccountTypeAdapter extends TypeAdapter { @@ -96,7 +100,7 @@ public PaymentAccountTypeAdapter(Class paymentAccountT public PaymentAccountTypeAdapter(Class paymentAccountType, String[] excludedFields) { this.paymentAccountType = paymentAccountType; this.paymentAccountPayloadType = getPaymentAccountPayloadType(); - this.isExcludedField = (f) -> Arrays.stream(excludedFields).anyMatch(e -> e.equals(f.getName())); + this.isExcludedField = (f) -> stream(excludedFields).anyMatch(e -> e.equals(f.getName())); this.fieldSettersMap = getFieldSetterMap(); } @@ -128,6 +132,9 @@ public void write(JsonWriter out, PaymentAccount account) throws IOException { } private void writeInnerMutableFields(JsonWriter out, PaymentAccount account) { + if (account.isTransferwiseAccount()) + writeTradeCurrenciesField(out, account); + fieldSettersMap.forEach((field, value) -> { try { // Write out a json element if there is a @Setter for this field. @@ -151,6 +158,25 @@ private void writeInnerMutableFields(JsonWriter out, PaymentAccount account) { }); } + // In some cases (TransferwiseAccount), we need to include a 'tradeCurrencies' + // field in the json form, though the 'tradeCurrencies' field has no setter method in + // the PaymentAccount class hierarchy. At of time of this change, TransferwiseAccount + // is the only known exception to the rule. + private void writeTradeCurrenciesField(JsonWriter out, PaymentAccount account) { + try { + String fieldName = "tradeCurrencies"; + log.debug("Append form with non-settable field: {}", fieldName); + out.name(fieldName); + out.value("comma delimited currency code list, e.g., gbp,eur"); + } catch (Exception ex) { + String errMsg = format("cannot create a new %s json form", + account.getClass().getSimpleName()); + log.error(StringUtils.capitalize(errMsg) + ".", ex); + throw new IllegalStateException("programmer error: " + errMsg); + } + } + + @Override public PaymentAccount read(JsonReader in) throws IOException { PaymentAccount account = initNewPaymentAccount(); @@ -158,6 +184,11 @@ public PaymentAccount read(JsonReader in) throws IOException { while (in.hasNext()) { String currentFieldName = in.nextName(); + // The tradeCurrency field is common to all payment account types, + // but has no setter. + if (didReadTradeCurrenciesField(in, account, currentFieldName)) + continue; + // Some of the fields are common to all payment account types. if (didReadCommonField(in, account, currentFieldName)) continue; @@ -199,17 +230,13 @@ private void invokeSetterMethod(PaymentAccount account, Field field, JsonReader account.getClass().getSimpleName()); throw new IllegalStateException(errMsg); } - } catch (IllegalAccessException | InvocationTargetException ex) { - String errMsg = format("cannot set field value for %s on %s", - field.getName(), - account.getClass().getSimpleName()); - log.error(StringUtils.capitalize(errMsg) + ".", ex); - throw new IllegalStateException("programmer error: " + errMsg); + } catch (ReflectiveOperationException ex) { + handleSetFieldValueError(account, field, ex); } } else { throw new IllegalStateException( format("programmer error: cannot de-serialize json to a '%s' " - + " because there is no setter method for field %s.", + + " because field value cannot be set %s.", account.getClass().getSimpleName(), field.getName())); } @@ -232,7 +259,7 @@ private Map> getFieldSetterMap() { .or(() -> getSetterMethodForFieldInClassHierarchy(field, paymentAccountPayloadType)); map.put(field, setter); } - return Collections.unmodifiableMap(map); + return unmodifiableMap(map); } private List getOrderedFields() { @@ -274,6 +301,56 @@ private Long nextLongOrNull(JsonReader in) { } } + private final Predicate isCommaDelimitedCurrencyList = (s) -> s != null && s.contains(","); + private final Function> commaDelimitedCodesToList = (s) -> { + if (isCommaDelimitedCurrencyList.test(s)) + return stream(s.split(",")).map(a -> a.trim().toUpperCase()).collect(toList()); + else if (s != null && !s.isEmpty()) + return singletonList(s.trim().toUpperCase()); + else + return new ArrayList<>(); + }; + + private boolean didReadTradeCurrenciesField(JsonReader in, + PaymentAccount account, + String fieldName) { + // The PaymentAccount.tradeCurrencies field is a special case because it has + // no setter, and we add currencies to the List here. Normally, it is an + // excluded field, TransferwiseAccount excepted. + if (fieldName.equals("tradeCurrencies")) { + try { + String fieldValue = nextStringOrNull(in); + List currencyCodes = commaDelimitedCodesToList.apply(fieldValue); + + Optional> tradeCurrencies; + if (account.isTransferwiseAccount()) + tradeCurrencies = getTradeCurrenciesInList(currencyCodes, getAllTransferwiseCurrencies()); + else + tradeCurrencies = getTradeCurrencies(currencyCodes); + + if (tradeCurrencies.isPresent()) { + Method addCurrencyMethod = getMethod("addCurrency", PaymentAccount.class); + for (TradeCurrency tradeCurrency : tradeCurrencies.get()) { + addCurrencyMethod.invoke(account, tradeCurrency); + } + } else { + // Log a warning. We should not throw an exception here because the + // gson library will not pass it up to the calling Bisq class as it + // would be defined here. Do a check in a calling class to make sure + // the tradeCurrencies field is populated in the PaymentAccount + // object, if it is required for the payment account method. + log.warn("No trade currencies were found in the {} account form.", + account.getPaymentMethod().getDisplayString()); + } + return true; + } catch (ReflectiveOperationException ex) { + Field field = getField("tradeCurrencies", PaymentAccount.class); + handleSetFieldValueError(account, field, ex); + } + } + return false; + } + private boolean didReadCommonField(JsonReader in, PaymentAccount account, String fieldName) throws IOException { diff --git a/core/src/main/java/bisq/core/locale/CurrencyUtil.java b/core/src/main/java/bisq/core/locale/CurrencyUtil.java index e94127cef1b..9ed9bb1a967 100644 --- a/core/src/main/java/bisq/core/locale/CurrencyUtil.java +++ b/core/src/main/java/bisq/core/locale/CurrencyUtil.java @@ -43,6 +43,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -51,6 +52,7 @@ import lombok.extern.slf4j.Slf4j; import static com.google.common.base.Preconditions.checkArgument; +import static java.lang.String.format; @Slf4j public class CurrencyUtil { @@ -486,6 +488,32 @@ public static Optional getTradeCurrency(String currencyCode) { return Optional.empty(); } + public static Optional> getTradeCurrencies(List currencyCodes) { + List tradeCurrencies = new ArrayList<>(); + currencyCodes.stream().forEachOrdered(c -> + tradeCurrencies.add(getTradeCurrency(c).orElseThrow(() -> + new IllegalArgumentException(format("%s is not a valid trade currency code", c))))); + return tradeCurrencies.isEmpty() + ? Optional.empty() + : Optional.of(tradeCurrencies); + } + + public static Optional> getTradeCurrenciesInList(List currencyCodes, + List validCurrencies) { + Optional> tradeCurrencies = getTradeCurrencies(currencyCodes); + Consumer> validateCandidateCurrencies = (list) -> { + for (TradeCurrency tradeCurrency : list) { + if (!validCurrencies.contains(tradeCurrency)) { + throw new IllegalArgumentException( + format("%s is not a member of valid currencies list", + tradeCurrency.getCode())); + } + } + }; + tradeCurrencies.ifPresent(validateCandidateCurrencies); + return tradeCurrencies; + } + public static FiatCurrency getCurrencyByCountryCode(String countryCode) { if (countryCode.equals("XK")) return new FiatCurrency("EUR"); diff --git a/core/src/main/java/bisq/core/payment/PaymentAccount.java b/core/src/main/java/bisq/core/payment/PaymentAccount.java index a540641319d..1e481e44583 100644 --- a/core/src/main/java/bisq/core/payment/PaymentAccount.java +++ b/core/src/main/java/bisq/core/payment/PaymentAccount.java @@ -198,6 +198,10 @@ public boolean isMoneyGramAccount() { return this instanceof MoneyGramAccount; } + public boolean isTransferwiseAccount() { + return this instanceof TransferwiseAccount; + } + /** * Return an Optional of the trade currency for this payment account, or * Optional.empty() if none is found. If this payment account has a selected From de59c0a5f9a159ef06f0aa4d6399f80fcacd775c Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 15:22:28 -0300 Subject: [PATCH 30/35] Assign selected trading currency on new payment accts (api only) If a new account has a trading currency, assign it as the selected trading currency. Also fixed a payment acct console formatting issue (left justify the ccy code). --- .../payment/CreatePaymentAccountTest.java | 70 +++++++++++-------- cli/src/main/java/bisq/cli/TableFormat.java | 2 +- .../core/api/CorePaymentAccountsService.java | 11 ++- 3 files changed, 50 insertions(+), 33 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index 2cc5bfcf857..b2911aa842e 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -32,6 +32,7 @@ import bisq.core.payment.MoneyBeamAccount; import bisq.core.payment.MoneyGramAccount; import bisq.core.payment.NationalBankAccount; +import bisq.core.payment.PaymentAccount; import bisq.core.payment.PerfectMoneyAccount; import bisq.core.payment.PopmoneyAccount; import bisq.core.payment.PromptPayAccount; @@ -63,13 +64,13 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.TestMethodOrder; import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind; import static bisq.apitest.config.BisqAppConfig.alicedaemon; +import static bisq.cli.TableFormat.formatPaymentAcctTbl; import static bisq.core.locale.CurrencyUtil.*; import static bisq.core.payment.payload.PaymentMethod.*; import static java.util.Collections.singletonList; @@ -78,7 +79,7 @@ import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation; -@Disabled +// @Disabled @Slf4j @TestMethodOrder(OrderAnnotation.class) public class CreatePaymentAccountTest extends AbstractPaymentAccountTest { @@ -109,7 +110,7 @@ public void testCreateAdvancedCashAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -128,7 +129,7 @@ public void testCreateAliPayAccount(TestInfo testInfo) { verifyAccountSingleTradeCurrency("CNY", paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -150,7 +151,7 @@ public void testCreateAustraliaPayidAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_PAY_ID), paymentAccount.getPayid()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BANK_ACCOUNT_NAME), paymentAccount.getBankAccountName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -202,7 +203,7 @@ public void testCreateCashDepositAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_TAX_ID), payload.getHolderTaxId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_NATIONAL_ACCOUNT_ID), payload.getNationalAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_REQUIREMENTS), payload.getRequirements()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -247,7 +248,7 @@ public void testCreateBrazilNationalBankAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_TAX_ID), payload.getHolderTaxId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_NATIONAL_ACCOUNT_ID), payload.getNationalAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -269,7 +270,7 @@ public void testCreateChaseQuickPayAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -292,7 +293,7 @@ public void testCreateClearXChangeAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL_OR_MOBILE_NR), paymentAccount.getEmailOrMobileNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -321,7 +322,7 @@ public void testCreateF2FAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_CITY), paymentAccount.getCity()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_CONTACT), paymentAccount.getContact()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EXTRA_INFO), paymentAccount.getExtraInfo()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -344,7 +345,7 @@ public void testCreateFasterPaymentsAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SORT_CODE), paymentAccount.getSortCode()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -363,7 +364,7 @@ public void testCreateHalCashAccount(TestInfo testInfo) { verifyAccountSingleTradeCurrency("EUR", paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_MOBILE_NR), paymentAccount.getMobileNr()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -392,7 +393,7 @@ public void testCreateInteracETransferAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_QUESTION), paymentAccount.getQuestion()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ANSWER), paymentAccount.getAnswer()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -429,7 +430,7 @@ public void testCreateJapanBankAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BANK_ACCOUNT_NAME), paymentAccount.getBankAccountName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BANK_ACCOUNT_TYPE), paymentAccount.getBankAccountType()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BANK_ACCOUNT_NUMBER), paymentAccount.getBankAccountNumber()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -449,7 +450,7 @@ public void testCreateMoneyBeamAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_ID), paymentAccount.getAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -478,7 +479,7 @@ public void testCreateMoneyGramAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_COUNTRY), Objects.requireNonNull(paymentAccount.getCountry()).code); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_STATE), paymentAccount.getState()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -498,7 +499,7 @@ public void testCreatePerfectMoneyAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -520,7 +521,7 @@ public void testCreatePopmoneyAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_ID), paymentAccount.getAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -540,7 +541,7 @@ public void testCreatePromptPayAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_PROMPT_PAY_ID), paymentAccount.getPromptPayId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -559,7 +560,7 @@ public void testCreateRevolutAccount(TestInfo testInfo) { verifyAccountTradeCurrencies(getAllRevolutCurrencies(), paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_USERNAME), paymentAccount.getUserName()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -604,7 +605,7 @@ public void testCreateSameBankAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_TAX_ID), payload.getHolderTaxId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_NATIONAL_ACCOUNT_ID), payload.getNationalAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -635,7 +636,7 @@ public void testCreateSepaInstantAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BIC), paymentAccount.getBic()); // bankId == bic assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BIC), paymentAccount.getBankId()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -667,7 +668,7 @@ public void testCreateSepaAccount(TestInfo testInfo) { // bankId == bic assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BIC), paymentAccount.getBankId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -714,7 +715,7 @@ public void testCreateSpecificBanksAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), payload.getHolderName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_TAX_ID), payload.getHolderTaxId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_NATIONAL_ACCOUNT_ID), payload.getNationalAccountId()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -737,7 +738,7 @@ public void testCreateSwishAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_MOBILE_NR), paymentAccount.getMobileNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -759,7 +760,7 @@ public void testCreateTransferwiseAccountWith1TradeCurrency(TestInfo testInfo) { verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -792,7 +793,7 @@ public void testCreateTransferwiseAccountWith10TradeCurrencies(TestInfo testInfo verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -850,7 +851,7 @@ public void testCreateUpholdAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_ID), paymentAccount.getAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -872,7 +873,7 @@ public void testCreateUSPostalMoneyOrderAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_POSTAL_ADDRESS), paymentAccount.getPostalAddress()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -892,7 +893,7 @@ public void testCreateWeChatPayAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -924,11 +925,18 @@ public void testCreateWesternUnionAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_COUNTRY), Objects.requireNonNull(paymentAccount.getCountry()).code); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @AfterAll public static void tearDown() { tearDownScaffold(); } + + private void print(PaymentAccount paymentAccount) { + if (log.isDebugEnabled()) { + log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + log.debug("\n{}", formatPaymentAcctTbl(singletonList(paymentAccount.toProtoMessage()))); + } + } } diff --git a/cli/src/main/java/bisq/cli/TableFormat.java b/cli/src/main/java/bisq/cli/TableFormat.java index 8064e9f6962..dca4aa06994 100644 --- a/cli/src/main/java/bisq/cli/TableFormat.java +++ b/cli/src/main/java/bisq/cli/TableFormat.java @@ -163,7 +163,7 @@ public static String formatPaymentAcctTbl(List paymentAccounts) + padEnd(COL_HEADER_PAYMENT_METHOD, paymentMethodColWidth, ' ') + COL_HEADER_DELIMITER + COL_HEADER_UUID + COL_HEADER_DELIMITER + "\n"; String colDataFormat = "%-" + nameColWidth + "s" // left justify - + " %" + COL_HEADER_CURRENCY.length() + "s" // right justify + + " %-" + COL_HEADER_CURRENCY.length() + "s" // left justify + " %-" + paymentMethodColWidth + "s" // left justify + " %-" + COL_HEADER_UUID.length() + "s"; // left justify return headerLine diff --git a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java index b915b3ab4be..dae19ea9fae 100644 --- a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java +++ b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java @@ -44,19 +44,23 @@ class CorePaymentAccountsService { private final AccountAgeWitnessService accountAgeWitnessService; private final PaymentAccountForm paymentAccountForm; private final User user; + private final boolean isApiUser; @Inject - public CorePaymentAccountsService(AccountAgeWitnessService accountAgeWitnessService, + public CorePaymentAccountsService(CoreContext coreContext, + AccountAgeWitnessService accountAgeWitnessService, PaymentAccountForm paymentAccountForm, User user) { this.accountAgeWitnessService = accountAgeWitnessService; this.paymentAccountForm = paymentAccountForm; this.user = user; + this.isApiUser = coreContext.isApiUser(); } PaymentAccount createPaymentAccount(String jsonString) { PaymentAccount paymentAccount = paymentAccountForm.toPaymentAccount(jsonString); verifyPaymentAccountHasRequiredFields(paymentAccount); + maybeAssignSelectedTradeCurrency(paymentAccount); user.addPaymentAccountIfNotExists(paymentAccount); accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload()); log.info("Saved payment account with id {} and payment method {}.", @@ -92,4 +96,9 @@ private void verifyPaymentAccountHasRequiredFields(PaymentAccount paymentAccount throw new IllegalArgumentException(format("no trade currencies defined for %s payment account", paymentAccount.getPaymentMethod().getDisplayString().toLowerCase())); } + + private void maybeAssignSelectedTradeCurrency(PaymentAccount paymentAccount) { + if (isApiUser && paymentAccount.getSelectedTradeCurrency() == null) + paymentAccount.setSelectedTradeCurrency(paymentAccount.getTradeCurrency().orElse(null)); + } } From 75d81d9095d46ff9dba0f339cef642f065dcdfda Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 15:26:15 -0300 Subject: [PATCH 31/35] Avoid test run repetition, add warning supressions --- .../bisq/apitest/method/payment/CreatePaymentAccountTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index b2911aa842e..8f05b2a5da8 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -64,6 +64,7 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.TestMethodOrder; @@ -79,7 +80,8 @@ import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation; -// @Disabled +@SuppressWarnings({"OptionalGetWithoutIsPresent", "ConstantConditions"}) +@Disabled @Slf4j @TestMethodOrder(OrderAnnotation.class) public class CreatePaymentAccountTest extends AbstractPaymentAccountTest { From 04b52f873e4ae233c5ccfab5dc497b05197f644f Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 16:02:18 -0300 Subject: [PATCH 32/35] Fix setSelectedCurrency bug, and replace unnecessary reflection usage Commit de59c0a5f9a159ef06f0aa4d6399f80fcacd775c did not persist the assigned selected currency, this fixes the bug. --- .../payment/CreatePaymentAccountTest.java | 8 ++- .../core/api/CorePaymentAccountsService.java | 11 +--- .../api/model/PaymentAccountTypeAdapter.java | 50 +++++++++---------- 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index 8f05b2a5da8..3caef6ee391 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -758,7 +758,9 @@ public void testCreateTransferwiseAccountWith1TradeCurrency(TestInfo testInfo) { TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(aliceClient, jsonString); verifyUserPayloadHasPaymentAccountWithId(aliceClient, paymentAccount.getId()); assertEquals(1, paymentAccount.getTradeCurrencies().size()); - List expectedTradeCurrencies = singletonList(getTradeCurrency("EUR").get()); + TradeCurrency expectedCurrency = getTradeCurrency("EUR").get(); + assertEquals(expectedCurrency, paymentAccount.getSelectedTradeCurrency()); + List expectedTradeCurrencies = singletonList(expectedCurrency); verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); @@ -781,7 +783,7 @@ public void testCreateTransferwiseAccountWith10TradeCurrencies(TestInfo testInfo verifyUserPayloadHasPaymentAccountWithId(aliceClient, paymentAccount.getId()); assertEquals(10, paymentAccount.getTradeCurrencies().size()); List expectedTradeCurrencies = new ArrayList<>() {{ - add(getTradeCurrency("ARS").get()); + add(getTradeCurrency("ARS").get()); // 1st in list = selected ccy add(getTradeCurrency("CAD").get()); add(getTradeCurrency("HRK").get()); add(getTradeCurrency("CZK").get()); @@ -793,6 +795,8 @@ public void testCreateTransferwiseAccountWith10TradeCurrencies(TestInfo testInfo add(getTradeCurrency("NZD").get()); }}; verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); + TradeCurrency expectedSelectedCurrency = expectedTradeCurrencies.get(0); + assertEquals(expectedSelectedCurrency, paymentAccount.getSelectedTradeCurrency()); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); print(paymentAccount); diff --git a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java index dae19ea9fae..b915b3ab4be 100644 --- a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java +++ b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java @@ -44,23 +44,19 @@ class CorePaymentAccountsService { private final AccountAgeWitnessService accountAgeWitnessService; private final PaymentAccountForm paymentAccountForm; private final User user; - private final boolean isApiUser; @Inject - public CorePaymentAccountsService(CoreContext coreContext, - AccountAgeWitnessService accountAgeWitnessService, + public CorePaymentAccountsService(AccountAgeWitnessService accountAgeWitnessService, PaymentAccountForm paymentAccountForm, User user) { this.accountAgeWitnessService = accountAgeWitnessService; this.paymentAccountForm = paymentAccountForm; this.user = user; - this.isApiUser = coreContext.isApiUser(); } PaymentAccount createPaymentAccount(String jsonString) { PaymentAccount paymentAccount = paymentAccountForm.toPaymentAccount(jsonString); verifyPaymentAccountHasRequiredFields(paymentAccount); - maybeAssignSelectedTradeCurrency(paymentAccount); user.addPaymentAccountIfNotExists(paymentAccount); accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload()); log.info("Saved payment account with id {} and payment method {}.", @@ -96,9 +92,4 @@ private void verifyPaymentAccountHasRequiredFields(PaymentAccount paymentAccount throw new IllegalArgumentException(format("no trade currencies defined for %s payment account", paymentAccount.getPaymentMethod().getDisplayString().toLowerCase())); } - - private void maybeAssignSelectedTradeCurrency(PaymentAccount paymentAccount) { - if (isApiUser && paymentAccount.getSelectedTradeCurrency() == null) - paymentAccount.setSelectedTradeCurrency(paymentAccount.getTradeCurrency().orElse(null)); - } } diff --git a/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java b/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java index d8d2e19c8da..b566ac10b0d 100644 --- a/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java +++ b/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java @@ -318,35 +318,31 @@ private boolean didReadTradeCurrenciesField(JsonReader in, // no setter, and we add currencies to the List here. Normally, it is an // excluded field, TransferwiseAccount excepted. if (fieldName.equals("tradeCurrencies")) { - try { - String fieldValue = nextStringOrNull(in); - List currencyCodes = commaDelimitedCodesToList.apply(fieldValue); - - Optional> tradeCurrencies; - if (account.isTransferwiseAccount()) - tradeCurrencies = getTradeCurrenciesInList(currencyCodes, getAllTransferwiseCurrencies()); - else - tradeCurrencies = getTradeCurrencies(currencyCodes); - - if (tradeCurrencies.isPresent()) { - Method addCurrencyMethod = getMethod("addCurrency", PaymentAccount.class); - for (TradeCurrency tradeCurrency : tradeCurrencies.get()) { - addCurrencyMethod.invoke(account, tradeCurrency); - } - } else { - // Log a warning. We should not throw an exception here because the - // gson library will not pass it up to the calling Bisq class as it - // would be defined here. Do a check in a calling class to make sure - // the tradeCurrencies field is populated in the PaymentAccount - // object, if it is required for the payment account method. - log.warn("No trade currencies were found in the {} account form.", - account.getPaymentMethod().getDisplayString()); + String fieldValue = nextStringOrNull(in); + List currencyCodes = commaDelimitedCodesToList.apply(fieldValue); + + Optional> tradeCurrencies; + if (account.isTransferwiseAccount()) + tradeCurrencies = getTradeCurrenciesInList(currencyCodes, getAllTransferwiseCurrencies()); + else + tradeCurrencies = getTradeCurrencies(currencyCodes); + + if (tradeCurrencies.isPresent()) { + for (TradeCurrency tradeCurrency : tradeCurrencies.get()) { + account.addCurrency(tradeCurrency); } - return true; - } catch (ReflectiveOperationException ex) { - Field field = getField("tradeCurrencies", PaymentAccount.class); - handleSetFieldValueError(account, field, ex); + // For api users, define a selected currency. + account.setSelectedTradeCurrency(account.getTradeCurrency().orElse(null)); + } else { + // Log a warning. We should not throw an exception here because the + // gson library will not pass it up to the calling Bisq class as it + // would be defined here. Do a check in a calling class to make sure + // the tradeCurrencies field is populated in the PaymentAccount + // object, if it is required for the payment account method. + log.warn("No trade currencies were found in the {} account form.", + account.getPaymentMethod().getDisplayString()); } + return true; } return false; } From d6c79fca70f437e93348050f902a4bd5b06fac78 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 16:26:16 -0300 Subject: [PATCH 33/35] Filter out 'my' offers from 'available' offers --- core/src/main/java/bisq/core/api/CoreOffersService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/api/CoreOffersService.java b/core/src/main/java/bisq/core/api/CoreOffersService.java index 8a5b95cf31e..be429709c69 100644 --- a/core/src/main/java/bisq/core/api/CoreOffersService.java +++ b/core/src/main/java/bisq/core/api/CoreOffersService.java @@ -55,7 +55,6 @@ import static bisq.core.offer.OfferPayload.Direction; import static bisq.core.offer.OfferPayload.Direction.BUY; import static bisq.core.payment.PaymentAccountUtil.isPaymentAccountValidForOffer; -import static java.lang.String.*; import static java.lang.String.format; import static java.util.Comparator.comparing; @@ -103,6 +102,7 @@ public CoreOffersService(CoreContext coreContext, Offer getOffer(String id) { return offerBookService.getOffers().stream() .filter(o -> o.getId().equals(id)) + .filter(o -> !o.isMyOffer(keyRing)) .filter(o -> offerFilter.canTakeOffer(o, isApiUser).isValid()) .findAny().orElseThrow(() -> new IllegalStateException(format("offer with id '%s' not found", id))); @@ -118,6 +118,7 @@ Offer getMyOffer(String id) { List getOffers(String direction, String currencyCode) { return offerBookService.getOffers().stream() + .filter(o -> !o.isMyOffer(keyRing)) .filter(o -> offerMatchesDirectionAndCurrency(o, direction, currencyCode)) .filter(o -> offerFilter.canTakeOffer(o, isApiUser).isValid()) .sorted(priceComparator(direction)) From 2161c936f171f0a0a8106408a172200ad190771c Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 16:36:01 -0300 Subject: [PATCH 34/35] Adjust canceloffer to getmyoffer bugfix (commit d6c79fc) --- core/src/main/java/bisq/core/api/CoreOffersService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/api/CoreOffersService.java b/core/src/main/java/bisq/core/api/CoreOffersService.java index be429709c69..9942380abca 100644 --- a/core/src/main/java/bisq/core/api/CoreOffersService.java +++ b/core/src/main/java/bisq/core/api/CoreOffersService.java @@ -215,7 +215,7 @@ Offer editOffer(String offerId, } void cancelOffer(String id) { - Offer offer = getOffer(id); + Offer offer = getMyOffer(id); openOfferManager.removeOffer(offer, () -> { }, From 389abf459d87f748dc0998490bc7332ac5a7ff3c Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Tue, 9 Mar 2021 10:43:05 -0300 Subject: [PATCH 35/35] Check valid createoffer opt values with asserts --- cli/src/test/java/bisq/cli/opt/OptionParsersTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java index d853da66d6a..1f939198d69 100644 --- a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java +++ b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java @@ -130,6 +130,13 @@ public void testValidCreateOfferOpts() { "--" + OPT_MKT_PRICE_MARGIN + "=" + "0.0", "--" + OPT_SECURITY_DEPOSIT + "=" + "25.0" }; + CreateOfferOptionParser parser = new CreateOfferOptionParser(args).parse(); + assertEquals("abc-payment-acct-id-123", parser.getPaymentAccountId()); + assertEquals("BUY", parser.getDirection()); + assertEquals("EUR", parser.getCurrencyCode()); + assertEquals("0.125", parser.getAmount()); + assertEquals("0.0", parser.getMktPriceMargin()); + assertEquals("25.0", parser.getSecurityDeposit()); } // CreatePaymentAcct opt parser tests