diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index 9a39a72fadc..e3a340c8dca 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -152,21 +152,13 @@ public static void main(String[] args) { var stub = GetBalanceGrpc.newBlockingStub(channel).withCallCredentials(credentials); var request = GetBalanceRequest.newBuilder().build(); var reply = stub.getBalance(request); - if (reply.getBalance() == -1) { - err.println(reply.getErrorMessage()); - exit(EXIT_FAILURE); - } out.println(formatBalance(reply.getBalance())); exit(EXIT_SUCCESS); } case lockwallet: { var stub = LockWalletGrpc.newBlockingStub(channel).withCallCredentials(credentials); var request = LockWalletRequest.newBuilder().build(); - var reply = stub.lockWallet(request); - if (!reply.getSuccess()) { - err.println(reply.getErrorMessage()); - exit(EXIT_FAILURE); - } + stub.lockWallet(request); out.println("wallet locked"); exit(EXIT_SUCCESS); } @@ -190,11 +182,7 @@ public static void main(String[] args) { var request = UnlockWalletRequest.newBuilder() .setPassword(nonOptionArgs.get(1)) .setTimeout(timeout).build(); - var reply = stub.unlockWallet(request); - if (!reply.getSuccess()) { - err.println(reply.getErrorMessage()); - exit(EXIT_FAILURE); - } + stub.unlockWallet(request); out.println("wallet unlocked"); exit(EXIT_SUCCESS); } @@ -205,11 +193,7 @@ public static void main(String[] args) { } var stub = RemoveWalletPasswordGrpc.newBlockingStub(channel).withCallCredentials(credentials); var request = RemoveWalletPasswordRequest.newBuilder().setPassword(nonOptionArgs.get(1)).build(); - var reply = stub.removeWalletPassword(request); - if (!reply.getSuccess()) { - err.println(reply.getErrorMessage()); - exit(EXIT_FAILURE); - } + stub.removeWalletPassword(request); out.println("wallet decrypted"); exit(EXIT_SUCCESS); } @@ -222,11 +206,7 @@ public static void main(String[] args) { var request = (nonOptionArgs.size() == 3) ? SetWalletPasswordRequest.newBuilder().setPassword(nonOptionArgs.get(1)).setNewPassword(nonOptionArgs.get(2)).build() : SetWalletPasswordRequest.newBuilder().setPassword(nonOptionArgs.get(1)).build(); - var reply = stub.setWalletPassword(request); - if (!reply.getSuccess()) { - err.println(reply.getErrorMessage()); - exit(EXIT_FAILURE); - } + stub.setWalletPassword(request); out.println("wallet encrypted" + (nonOptionArgs.size() == 2 ? "" : " with new password")); exit(EXIT_SUCCESS); } @@ -236,11 +216,17 @@ public static void main(String[] args) { } } } catch (StatusRuntimeException ex) { - // This exception is thrown if the client-provided password credentials do not - // match the value set on the server. The actual error message is in a nested - // exception and we clean it up a bit to make it more presentable. + // The actual server error message is in a nested exception and we clean it + // up a bit to make it more presentable. Throwable t = ex.getCause() == null ? ex : ex.getCause(); - err.println("Error: " + t.getMessage().replace("UNAUTHENTICATED: ", "")); + String message = t.getMessage() + .replace("FAILED_PRECONDITION: ", "") + .replace("INTERNAL: ", "") + .replace("INVALID_ARGUMENT: ", "") + .replace("UNAUTHENTICATED: ", "") + .replace("UNAVAILABLE: ", "") + .replace("UNKNOWN: ", ""); + err.println("Error: " + message); exit(EXIT_FAILURE); } } diff --git a/core/src/main/java/bisq/core/grpc/ApiStatus.java b/core/src/main/java/bisq/core/grpc/ApiStatus.java new file mode 100644 index 00000000000..021d55412f8 --- /dev/null +++ b/core/src/main/java/bisq/core/grpc/ApiStatus.java @@ -0,0 +1,76 @@ +/* + * This file is part of Bisq. + * + * Bisq is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or (at + * your option) any later version. + * + * Bisq is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public + * License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Bisq. If not, see . + */ + +package bisq.core.grpc; + +import io.grpc.Status; + +/** + * Maps meaningful CoreApi error messages to more general purpose gRPC error Status codes. + * This keeps gRPC api out of CoreApi, and ensures the correct gRPC Status is sent to the + * client. + * + * @see gRPC Status Codes + */ +enum ApiStatus { + + INCORRECT_OLD_WALLET_PASSWORD(Status.INVALID_ARGUMENT, "incorrect old password"), + INCORRECT_WALLET_PASSWORD(Status.INVALID_ARGUMENT, "incorrect password"), + + INTERNAL(Status.INTERNAL, "internal server error"), + + OK(Status.OK, null), + + UNKNOWN(Status.UNKNOWN, "unknown"), + + WALLET_ALREADY_LOCKED(Status.FAILED_PRECONDITION, "wallet is already locked"), + + WALLET_ENCRYPTER_NOT_AVAILABLE(Status.FAILED_PRECONDITION, "wallet encrypter is not available"), + + WALLET_IS_ENCRYPTED(Status.FAILED_PRECONDITION, "wallet is encrypted with a password"), + WALLET_IS_ENCRYPTED_WITH_UNLOCK_INSTRUCTION(Status.FAILED_PRECONDITION, + "wallet is encrypted with a password; unlock it with the 'unlock \"password\" timeout' command"), + + WALLET_NOT_ENCRYPTED(Status.FAILED_PRECONDITION, "wallet is not encrypted with a password"), + WALLET_NOT_AVAILABLE(Status.UNAVAILABLE, "wallet is not available"); + + + private final Status grpcStatus; + private final String description; + + ApiStatus(Status grpcStatus, String description) { + this.grpcStatus = grpcStatus; + this.description = description; + } + + Status getGrpcStatus() { + return this.grpcStatus; + } + + String getDescription() { + return this.description; + } + + @Override + public String toString() { + return "ApiStatus{" + + "grpcStatus=" + grpcStatus + + ", description='" + description + '\'' + + '}'; + } +} + diff --git a/core/src/main/java/bisq/core/grpc/CoreApi.java b/core/src/main/java/bisq/core/grpc/CoreApi.java index 7d5a5cc3289..ddb165b5788 100644 --- a/core/src/main/java/bisq/core/grpc/CoreApi.java +++ b/core/src/main/java/bisq/core/grpc/CoreApi.java @@ -51,6 +51,7 @@ import javax.annotation.Nullable; +import static bisq.core.grpc.ApiStatus.*; import static java.util.concurrent.TimeUnit.SECONDS; /** @@ -91,18 +92,21 @@ public String getVersion() { return Version.VERSION; } - public Tuple2 getAvailableBalance() { + public Tuple2 getAvailableBalance() { if (!walletsManager.areWalletsAvailable()) - return new Tuple2<>(-1L, "Error: wallet is not available"); + return new Tuple2<>(-1L, WALLET_NOT_AVAILABLE); if (walletsManager.areWalletsEncrypted()) - return new Tuple2<>(-1L, "Error: wallet is encrypted; unlock it with the 'unlock \"password\" timeout' command"); + return new Tuple2<>(-1L, WALLET_IS_ENCRYPTED_WITH_UNLOCK_INSTRUCTION); try { long balance = balances.getAvailableBalance().get().getValue(); - return new Tuple2<>(balance, ""); + return new Tuple2<>(balance, OK); } catch (Throwable t) { - return new Tuple2<>(-1L, "Error: " + t.getLocalizedMessage()); + // TODO Derive new ApiStatus codes from server stack traces. + t.printStackTrace(); + // TODO Fix bug causing NPE thrown by getAvailableBalance(). + return new Tuple2<>(-1L, INTERNAL); } } @@ -183,74 +187,78 @@ public void placeOffer(String offerId, // Provided for automated wallet protection method testing, despite the // security risks exposed by providing users the ability to decrypt their wallets. - public Tuple2 removeWalletPassword(String password) { + public Tuple2 removeWalletPassword(String password) { if (!walletsManager.areWalletsAvailable()) - return new Tuple2<>(false, "Error: wallet is not available"); + return new Tuple2<>(false, WALLET_NOT_AVAILABLE); if (!walletsManager.areWalletsEncrypted()) - return new Tuple2<>(false, "Error: wallet is not encrypted with a password"); + return new Tuple2<>(false, WALLET_NOT_ENCRYPTED); KeyCrypterScrypt keyCrypterScrypt = walletsManager.getKeyCrypterScrypt(); if (keyCrypterScrypt == null) - return new Tuple2<>(false, "Error: wallet encrypter is not available"); + return new Tuple2<>(false, WALLET_ENCRYPTER_NOT_AVAILABLE); KeyParameter aesKey = keyCrypterScrypt.deriveKey(password); if (!walletsManager.checkAESKey(aesKey)) - return new Tuple2<>(false, "Error: incorrect password"); + return new Tuple2<>(false, INCORRECT_WALLET_PASSWORD); walletsManager.decryptWallets(aesKey); - return new Tuple2<>(true, ""); + return new Tuple2<>(true, OK); } - public Tuple2 setWalletPassword(String password, String newPassword) { + public Tuple2 setWalletPassword(String password, String newPassword) { try { if (!walletsManager.areWalletsAvailable()) - return new Tuple2<>(false, "Error: wallet is not available"); + return new Tuple2<>(false, WALLET_NOT_AVAILABLE); KeyCrypterScrypt keyCrypterScrypt = walletsManager.getKeyCrypterScrypt(); if (keyCrypterScrypt == null) - return new Tuple2<>(false, "Error: wallet encrypter is not available"); + return new Tuple2<>(false, WALLET_ENCRYPTER_NOT_AVAILABLE); if (newPassword != null && !newPassword.isEmpty()) { - // todo validate new password + // TODO Validate new password before replacing old password. if (!walletsManager.areWalletsEncrypted()) - return new Tuple2<>(false, "Error: wallet is not encrypted with a password"); + return new Tuple2<>(false, WALLET_NOT_ENCRYPTED); KeyParameter aesKey = keyCrypterScrypt.deriveKey(password); if (!walletsManager.checkAESKey(aesKey)) - return new Tuple2<>(false, "Error: incorrect old password"); + return new Tuple2<>(false, INCORRECT_OLD_WALLET_PASSWORD); walletsManager.decryptWallets(aesKey); aesKey = keyCrypterScrypt.deriveKey(newPassword); walletsManager.encryptWallets(keyCrypterScrypt, aesKey); - return new Tuple2<>(true, ""); + return new Tuple2<>(true, OK); } if (walletsManager.areWalletsEncrypted()) - return new Tuple2<>(false, "Error: wallet is already encrypted"); + return new Tuple2<>(false, WALLET_IS_ENCRYPTED); + + // TODO Validate new password. KeyParameter aesKey = keyCrypterScrypt.deriveKey(password); walletsManager.encryptWallets(keyCrypterScrypt, aesKey); - return new Tuple2<>(true, ""); + return new Tuple2<>(true, OK); } catch (Throwable t) { - return new Tuple2<>(false, "Error: " + t.getLocalizedMessage()); + // TODO Derive new ApiStatus codes from server stack traces. + t.printStackTrace(); + return new Tuple2<>(false, INTERNAL); } } - public Tuple2 lockWallet() { + public Tuple2 lockWallet() { if (tempLockWalletPassword != null) { - Tuple2 encrypted = setWalletPassword(tempLockWalletPassword, null); + Tuple2 encrypted = setWalletPassword(tempLockWalletPassword, null); tempLockWalletPassword = null; - if (!encrypted.first) + if (!encrypted.second.equals(OK)) return encrypted; - return new Tuple2<>(true, ""); + return new Tuple2<>(true, OK); } - return new Tuple2<>(false, "Error: wallet is already locked"); + return new Tuple2<>(false, WALLET_ALREADY_LOCKED); } - public Tuple2 unlockWallet(String password, long timeout) { - Tuple2 decrypted = removeWalletPassword(password); - if (!decrypted.first) + public Tuple2 unlockWallet(String password, long timeout) { + Tuple2 decrypted = removeWalletPassword(password); + if (!decrypted.second.equals(OK)) return decrypted; TimerTask timerTask = new TimerTask() { @@ -267,6 +275,6 @@ public void run() { // Cache wallet password for timeout (secs), in case // user wants to lock the wallet for timeout expires. tempLockWalletPassword = password; - return new Tuple2<>(true, ""); + return new Tuple2<>(true, OK); } } diff --git a/core/src/main/java/bisq/core/grpc/GrpcServer.java b/core/src/main/java/bisq/core/grpc/GrpcServer.java index ebedb02cc08..0a379e0b3aa 100644 --- a/core/src/main/java/bisq/core/grpc/GrpcServer.java +++ b/core/src/main/java/bisq/core/grpc/GrpcServer.java @@ -56,6 +56,7 @@ import bisq.proto.grpc.UnlockWalletRequest; import io.grpc.ServerBuilder; +import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import java.io.IOException; @@ -114,7 +115,13 @@ class GetBalanceService extends GetBalanceGrpc.GetBalanceImplBase { @Override public void getBalance(GetBalanceRequest req, StreamObserver responseObserver) { var result = coreApi.getAvailableBalance(); - var reply = GetBalanceReply.newBuilder().setBalance(result.first).setErrorMessage(result.second).build(); + if (!result.second.equals(ApiStatus.OK)) { + StatusRuntimeException ex = new StatusRuntimeException(result.second.getGrpcStatus() + .withDescription(result.second.getDescription())); + responseObserver.onError(ex); + throw ex; + } + var reply = GetBalanceReply.newBuilder().setBalance(result.first).build(); responseObserver.onNext(reply); responseObserver.onCompleted(); } @@ -191,8 +198,13 @@ class RemoveWalletPasswordService extends RemoveWalletPasswordGrpc.RemoveWalletP public void removeWalletPassword(RemoveWalletPasswordRequest req, StreamObserver responseObserver) { var result = coreApi.removeWalletPassword(req.getPassword()); - var reply = RemoveWalletPasswordReply.newBuilder() - .setSuccess(result.first).setErrorMessage(result.second).build(); + if (!result.second.equals(ApiStatus.OK)) { + StatusRuntimeException ex = new StatusRuntimeException(result.second.getGrpcStatus() + .withDescription(result.second.getDescription())); + responseObserver.onError(ex); + throw ex; + } + var reply = RemoveWalletPasswordReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); } @@ -203,8 +215,13 @@ class SetWalletPasswordService extends SetWalletPasswordGrpc.SetWalletPasswordIm public void setWalletPassword(SetWalletPasswordRequest req, StreamObserver responseObserver) { var result = coreApi.setWalletPassword(req.getPassword(), req.getNewPassword()); - var reply = SetWalletPasswordReply.newBuilder() - .setSuccess(result.first).setErrorMessage(result.second).build(); + if (!result.second.equals(ApiStatus.OK)) { + StatusRuntimeException ex = new StatusRuntimeException(result.second.getGrpcStatus() + .withDescription(result.second.getDescription())); + responseObserver.onError(ex); + throw ex; + } + var reply = SetWalletPasswordReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); } @@ -215,8 +232,13 @@ class LockWalletService extends LockWalletGrpc.LockWalletImplBase { public void lockWallet(LockWalletRequest req, StreamObserver responseObserver) { var result = coreApi.lockWallet(); - var reply = LockWalletReply.newBuilder() - .setSuccess(result.first).setErrorMessage(result.second).build(); + if (!result.second.equals(ApiStatus.OK)) { + StatusRuntimeException ex = new StatusRuntimeException(result.second.getGrpcStatus() + .withDescription(result.second.getDescription())); + responseObserver.onError(ex); + throw ex; + } + var reply = LockWalletReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); } @@ -227,8 +249,13 @@ class UnlockWalletService extends UnlockWalletGrpc.UnlockWalletImplBase { public void unlockWallet(UnlockWalletRequest req, StreamObserver responseObserver) { var result = coreApi.unlockWallet(req.getPassword(), req.getTimeout()); - var reply = UnlockWalletReply.newBuilder() - .setSuccess(result.first).setErrorMessage(result.second).build(); + if (!result.second.equals(ApiStatus.OK)) { + StatusRuntimeException ex = new StatusRuntimeException(result.second.getGrpcStatus() + .withDescription(result.second.getDescription())); + responseObserver.onError(ex); + throw ex; + } + var reply = UnlockWalletReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); } diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index 6a056b5c184..d7dbf1df327 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -53,7 +53,6 @@ message GetBalanceRequest { message GetBalanceReply { uint64 balance = 1; - string error_message = 2; // optional error message } /////////////////////////////////////////////////////////////////////////////////////////// @@ -143,8 +142,6 @@ message RemoveWalletPasswordRequest { } message RemoveWalletPasswordReply { - bool success = 1; - string error_message = 2; // optional error message } /////////////////////////////////////////////////////////////////////////////////////////// @@ -162,8 +159,6 @@ message SetWalletPasswordRequest { } message SetWalletPasswordReply { - bool success = 1; - string error_message = 2; // optional error message } /////////////////////////////////////////////////////////////////////////////////////////// @@ -179,8 +174,6 @@ message LockWalletRequest { } message LockWalletReply { - bool success = 1; - string error_message = 2; // optional error message } /////////////////////////////////////////////////////////////////////////////////////////// @@ -198,6 +191,4 @@ message UnlockWalletRequest { } message UnlockWalletReply { - bool success = 1; - string error_message = 2; // optional error message }