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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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 70da6d19fa694c5ae11212645fdfa4ea47529854 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Tue, 9 Mar 2021 10:33:28 -0300 Subject: [PATCH 27/27] Parse args in opts test, no exception = pass Resolves PR review issue https://github.com/bisq-network/bisq/pull/5274/files/304781ce92c13df187b26d1ac2ceec5f5eac3825..9c12b310199cce330977fddca6d93c9d767552f2#r590228515 --- cli/src/test/java/bisq/cli/opt/OptionParsersTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java index d853da66d6a..120aaa14c2f 100644 --- a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java +++ b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java @@ -130,6 +130,7 @@ public void testValidCreateOfferOpts() { "--" + OPT_MKT_PRICE_MARGIN + "=" + "0.0", "--" + OPT_SECURITY_DEPOSIT + "=" + "25.0" }; + new CreateOfferOptionParser(args).parse(); } // CreatePaymentAcct opt parser tests