Skip to content

Commit

Permalink
Return void vs Tuple2 from CoreWalletService#setWalletPassword
Browse files Browse the repository at this point in the history
Like the change in commit 163061a, setWalletPassword now throws an
IllegalStateException with an appropriate message if the wallet password
cannot be set.

Also deletes unused StatusApi enums.
  • Loading branch information
ghubstan committed May 3, 2020
1 parent 163061a commit 9164579
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 28 deletions.
7 changes: 0 additions & 7 deletions core/src/main/java/bisq/core/grpc/ApiStatus.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,14 @@
*/
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_NOT_ENCRYPTED(Status.FAILED_PRECONDITION, "wallet is not encrypted with a password"),
WALLET_NOT_AVAILABLE(Status.UNAVAILABLE, "wallet is not available");

Expand Down
22 changes: 8 additions & 14 deletions core/src/main/java/bisq/core/grpc/CoreWalletService.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,51 +50,45 @@ public long getAvailableBalance() {
return balance.getValue();
}

public Tuple2<Boolean, ApiStatus> setWalletPassword(String password, String newPassword) {
public void setWalletPassword(String password, String newPassword) {
try {
if (!walletsManager.areWalletsAvailable())
return new Tuple2<>(false, WALLET_NOT_AVAILABLE);
throw new IllegalStateException("wallet is not yet available");

KeyCrypterScrypt keyCrypterScrypt = walletsManager.getKeyCrypterScrypt();
if (keyCrypterScrypt == null)
return new Tuple2<>(false, WALLET_ENCRYPTER_NOT_AVAILABLE);
throw new IllegalStateException("wallet encrypter is not available");

if (newPassword != null && !newPassword.isEmpty()) {
// TODO Validate new password before replacing old password.
if (!walletsManager.areWalletsEncrypted())
return new Tuple2<>(false, WALLET_NOT_ENCRYPTED);
throw new IllegalStateException("wallet is not encrypted with a password");

KeyParameter aesKey = keyCrypterScrypt.deriveKey(password);
if (!walletsManager.checkAESKey(aesKey))
return new Tuple2<>(false, INCORRECT_OLD_WALLET_PASSWORD);
throw new IllegalStateException("incorrect old password");

walletsManager.decryptWallets(aesKey);
aesKey = keyCrypterScrypt.deriveKey(newPassword);
walletsManager.encryptWallets(keyCrypterScrypt, aesKey);
return new Tuple2<>(true, OK);

This comment has been minimized.

Copy link
@cbeams

cbeams May 3, 2020

Contributor

Shouldn't a return; statement remain in place here?

}

if (walletsManager.areWalletsEncrypted())
return new Tuple2<>(false, WALLET_IS_ENCRYPTED);
throw new IllegalStateException("wallet is encrypted with a password");

// TODO Validate new password.
KeyParameter aesKey = keyCrypterScrypt.deriveKey(password);
walletsManager.encryptWallets(keyCrypterScrypt, aesKey);
return new Tuple2<>(true, OK);
} catch (Throwable t) {
// TODO Derive new ApiStatus codes from server stack traces.
t.printStackTrace();
return new Tuple2<>(false, INTERNAL);
throw new IllegalStateException(t);
}
}

public Tuple2<Boolean, ApiStatus> lockWallet() {
if (tempLockWalletPassword != null) {
Tuple2<Boolean, ApiStatus> encrypted = setWalletPassword(tempLockWalletPassword, null);
setWalletPassword(tempLockWalletPassword, null);
tempLockWalletPassword = null;
if (!encrypted.second.equals(OK))
return encrypted;

return new Tuple2<>(true, OK);
}
return new Tuple2<>(false, WALLET_ALREADY_LOCKED);
Expand Down
14 changes: 7 additions & 7 deletions core/src/main/java/bisq/core/grpc/GrpcWalletService.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,16 @@ public void getBalance(GetBalanceRequest req, StreamObserver<GetBalanceReply> re
@Override
public void setWalletPassword(SetWalletPasswordRequest req,
StreamObserver<SetWalletPasswordReply> responseObserver) {
var result = walletService.setWalletPassword(req.getPassword(), req.getNewPassword());
if (!result.second.equals(ApiStatus.OK)) {
StatusRuntimeException ex = new StatusRuntimeException(result.second.getGrpcStatus()
.withDescription(result.second.getDescription()));
try {
walletService.setWalletPassword(req.getPassword(), req.getNewPassword());
var reply = SetWalletPasswordReply.newBuilder().build();
responseObserver.onNext(reply);
responseObserver.onCompleted();
} catch (IllegalStateException cause) {
var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage()));
responseObserver.onError(ex);
throw ex;
}
var reply = SetWalletPasswordReply.newBuilder().build();
responseObserver.onNext(reply);
responseObserver.onCompleted();
}

@Override
Expand Down

0 comments on commit 9164579

Please sign in to comment.