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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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,