From b1146fdd12ad89e94e38a8853e316f964824d67c Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 12 Jun 2020 15:16:14 -0300 Subject: [PATCH 1/9] Rename CoreWalletService -> CoreWalletsService This change fixes the ambiguity in the original class name, which implied it was a btc wallet service, not a bsq and btc wallets service. --- ...alletService.java => CoreWalletsService.java} | 4 ++-- .../java/bisq/core/grpc/GrpcWalletService.java | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) rename core/src/main/java/bisq/core/grpc/{CoreWalletService.java => CoreWalletsService.java} (98%) diff --git a/core/src/main/java/bisq/core/grpc/CoreWalletService.java b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java similarity index 98% rename from core/src/main/java/bisq/core/grpc/CoreWalletService.java rename to core/src/main/java/bisq/core/grpc/CoreWalletsService.java index ff9383c55d4..65a6133488e 100644 --- a/core/src/main/java/bisq/core/grpc/CoreWalletService.java +++ b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java @@ -19,7 +19,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; @Slf4j -class CoreWalletService { +class CoreWalletsService { private final Balances balances; private final WalletsManager walletsManager; @@ -31,7 +31,7 @@ class CoreWalletService { private KeyParameter tempAesKey; @Inject - public CoreWalletService(Balances balances, WalletsManager walletsManager) { + public CoreWalletsService(Balances balances, WalletsManager walletsManager) { this.balances = balances; this.walletsManager = walletsManager; } diff --git a/core/src/main/java/bisq/core/grpc/GrpcWalletService.java b/core/src/main/java/bisq/core/grpc/GrpcWalletService.java index 92d4cc8b81f..0ce59bb4c09 100644 --- a/core/src/main/java/bisq/core/grpc/GrpcWalletService.java +++ b/core/src/main/java/bisq/core/grpc/GrpcWalletService.java @@ -20,17 +20,17 @@ class GrpcWalletService extends WalletGrpc.WalletImplBase { - private final CoreWalletService walletService; + private final CoreWalletsService walletsService; @Inject - public GrpcWalletService(CoreWalletService walletService) { - this.walletService = walletService; + public GrpcWalletService(CoreWalletsService walletsService) { + this.walletsService = walletsService; } @Override public void getBalance(GetBalanceRequest req, StreamObserver responseObserver) { try { - long result = walletService.getAvailableBalance(); + long result = walletsService.getAvailableBalance(); var reply = GetBalanceReply.newBuilder().setBalance(result).build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -45,7 +45,7 @@ public void getBalance(GetBalanceRequest req, StreamObserver re public void setWalletPassword(SetWalletPasswordRequest req, StreamObserver responseObserver) { try { - walletService.setWalletPassword(req.getPassword(), req.getNewPassword()); + walletsService.setWalletPassword(req.getPassword(), req.getNewPassword()); var reply = SetWalletPasswordReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -60,7 +60,7 @@ public void setWalletPassword(SetWalletPasswordRequest req, public void removeWalletPassword(RemoveWalletPasswordRequest req, StreamObserver responseObserver) { try { - walletService.removeWalletPassword(req.getPassword()); + walletsService.removeWalletPassword(req.getPassword()); var reply = RemoveWalletPasswordReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -75,7 +75,7 @@ public void removeWalletPassword(RemoveWalletPasswordRequest req, public void lockWallet(LockWalletRequest req, StreamObserver responseObserver) { try { - walletService.lockWallet(); + walletsService.lockWallet(); var reply = LockWalletReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -90,7 +90,7 @@ public void lockWallet(LockWalletRequest req, public void unlockWallet(UnlockWalletRequest req, StreamObserver responseObserver) { try { - walletService.unlockWallet(req.getPassword(), req.getTimeout()); + walletsService.unlockWallet(req.getPassword(), req.getTimeout()); var reply = UnlockWalletReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); From ec66b14986bb0a9f3369009aafc541add4e65f03 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 12 Jun 2020 15:56:19 -0300 Subject: [PATCH 2/9] Add rpc wallet(s) protection tests This commit includes the following changes: * New tests for methods `lockwallet`, `unlockwallet`, `removewalletpassword`, and `setwalletpassword`. * New `getbalance` method error handing tests to verify error message correctness when wallet is locked. * Update to `getversion` method test -- now expects `1.3.4`. * Check for new `[params]` column header in help text. --- cli/test.sh | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 87 insertions(+), 5 deletions(-) diff --git a/cli/test.sh b/cli/test.sh index 94aae7d25b6..ba81d598604 100755 --- a/cli/test.sh +++ b/cli/test.sh @@ -48,17 +48,99 @@ run ./bisq-cli --password="xyz" getversion [ "$status" -eq 0 ] echo "actual output: $output" >&2 - [ "$output" = "1.3.2" ] + [ "$output" = "1.3.4" ] } @test "test getversion" { run ./bisq-cli --password=xyz getversion [ "$status" -eq 0 ] echo "actual output: $output" >&2 - [ "$output" = "1.3.2" ] + [ "$output" = "1.3.4" ] } -@test "test getbalance (available & unlocked wallet with 0 btc balance)" { +@test "test setwalletpassword \"a b c\"" { + run ./bisq-cli --password=xyz setwalletpassword "a b c" + [ "$status" -eq 0 ] + echo "actual output: $output" >&2 + [ "$output" = "wallet encrypted" ] + sleep 1 +} + +@test "test unlockwallet without password & timeout args" { + run ./bisq-cli --password=xyz unlockwallet + [ "$status" -eq 1 ] + echo "actual output: $output" >&2 + [ "$output" = "Error: no password specified" ] +} + +@test "test unlockwallet without timeout arg" { + run ./bisq-cli --password=xyz unlockwallet "a b c" + [ "$status" -eq 1 ] + echo "actual output: $output" >&2 + [ "$output" = "Error: no unlock timeout specified" ] +} + + +@test "test unlockwallet \"a b c\" 8" { + run ./bisq-cli --password=xyz unlockwallet "a b c" 8 + [ "$status" -eq 0 ] + echo "actual output: $output" >&2 + [ "$output" = "wallet unlocked" ] +} + +@test "test getbalance while wallet unlocked for 8s" { + run ./bisq-cli --password=xyz getbalance + [ "$status" -eq 0 ] + echo "actual output: $output" >&2 + [ "$output" = "0.00000000" ] + sleep 8 +} + +@test "test unlockwallet \"a b c\" 6" { + run ./bisq-cli --password=xyz unlockwallet "a b c" 6 + [ "$status" -eq 0 ] + echo "actual output: $output" >&2 + [ "$output" = "wallet unlocked" ] +} + +@test "test lockwallet before unlockwallet timeout=6s expires" { + run ./bisq-cli --password=xyz lockwallet + [ "$status" -eq 0 ] + echo "actual output: $output" >&2 + [ "$output" = "wallet locked" ] +} + +@test "test setwalletpassword incorrect old pwd error" { + run ./bisq-cli --password=xyz setwalletpassword "z z z" "d e f" + [ "$status" -eq 1 ] + echo "actual output: $output" >&2 + [ "$output" = "Error: incorrect old password" ] +} + +@test "test setwalletpassword oldpwd newpwd" { + run ./bisq-cli --password=xyz setwalletpassword "a b c" "d e f" + [ "$status" -eq 0 ] + echo "actual output: $output" >&2 + [ "$output" = "wallet encrypted with new password" ] + sleep 1 +} + +@test "test getbalance wallet locked error" { + run ./bisq-cli --password=xyz getbalance + [ "$status" -eq 1 ] + echo "actual output: $output" >&2 + [ "$output" = "Error: wallet is locked" ] +} + +@test "test removewalletpassword" { + run ./bisq-cli --password=xyz removewalletpassword "d e f" + [ "$status" -eq 0 ] + echo "actual output: $output" >&2 + [ "$output" = "wallet decrypted" ] + sleep 1 +} + +@test "test getbalance when wallet available & unlocked with 0 btc balance" { run ./bisq-cli --password=xyz getbalance [ "$status" -eq 0 ] echo "actual output: $output" >&2 @@ -69,7 +151,7 @@ run ./bisq-cli [ "$status" -eq 1 ] [ "${lines[0]}" = "Bisq RPC Client" ] - [ "${lines[1]}" = "Usage: bisq-cli [options] " ] + [ "${lines[1]}" = "Usage: bisq-cli [options] [params]" ] # TODO add asserts after help text is modified for new endpoints } @@ -77,6 +159,6 @@ run ./bisq-cli --help [ "$status" -eq 0 ] [ "${lines[0]}" = "Bisq RPC Client" ] - [ "${lines[1]}" = "Usage: bisq-cli [options] " ] + [ "${lines[1]}" = "Usage: bisq-cli [options] [params]" ] # TODO add asserts after help text is modified for new endpoints } From 85c96764fb3adaa25e8366d547be9d05e635f1bc Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 13 Jun 2020 19:59:45 -0300 Subject: [PATCH 3/9] Add rpc method 'getfundingaddresses' This addresses task #1 in issue https://github.com/bisq-network/bisq/issues/4257. This new gRPC WalletService method displays the BTC wallet's list of receiving addresses. The balance and number of confirmations for the most recent transaction is displayed to the right of each address. Instead of returning a gRPC data structure to the client, the service method returns a formatted String. If the BTC wallet has no unused addresses, one will be created and included in the returned list, and it can be used to fund the wallet. The new method required injection of the BtcWalletService into CoreWalletsService, and the usual boilerplate changes to grpc.proto, CliMain, and GrpcWalletService. Some of the next PRs (for #4257) will require some common functionality within CoreWalletsService, so these additional changes were included: * a private, class level formatSatoshis function * a public getNumConfirmationsForMostRecentTransaction method * a public getAddressBalance method * a private getAddressEntry method A unit test that verifies a successful return status was added to cli/test.sh. --- cli/src/main/java/bisq/cli/CliMain.java | 10 +- cli/test.sh | 5 + .../bisq/core/grpc/CoreWalletsService.java | 94 ++++++++++++++++++- .../bisq/core/grpc/GrpcWalletService.java | 17 ++++ proto/src/main/proto/grpc.proto | 9 ++ 5 files changed, 133 insertions(+), 2 deletions(-) diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index e5463e11cfb..ad59befae77 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -18,6 +18,7 @@ package bisq.cli; import bisq.proto.grpc.GetBalanceRequest; +import bisq.proto.grpc.GetFundingAddressesRequest; import bisq.proto.grpc.GetVersionGrpc; import bisq.proto.grpc.GetVersionRequest; import bisq.proto.grpc.LockWalletRequest; @@ -58,6 +59,7 @@ public class CliMain { private enum Method { getversion, getbalance, + getfundingaddresses, lockwallet, unlockwallet, removewalletpassword, @@ -152,6 +154,12 @@ public static void run(String[] args) { out.println(btcBalance); return; } + case getfundingaddresses: { + var request = GetFundingAddressesRequest.newBuilder().build(); + var reply = walletService.getFundingAddresses(request); + out.println(reply.getFundingAddressesInfo()); + return; + } case lockwallet: { var request = LockWalletRequest.newBuilder().build(); walletService.lockWallet(request); @@ -201,7 +209,7 @@ public static void run(String[] args) { } default: { throw new RuntimeException(format("unhandled method '%s'", method)); - } + } } } catch (StatusRuntimeException ex) { // Remove the leading gRPC status code (e.g. "UNKNOWN: ") from the message diff --git a/cli/test.sh b/cli/test.sh index ba81d598604..be2e67bc46f 100755 --- a/cli/test.sh +++ b/cli/test.sh @@ -147,6 +147,11 @@ [ "$output" = "0.00000000" ] } +@test "test getfundingaddresses" { + run ./bisq-cli --password=xyz getfundingaddresses + [ "$status" -eq 0 ] +} + @test "test help displayed on stderr if no options or arguments" { run ./bisq-cli [ "$status" -eq 1 ] diff --git a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java index 65a6133488e..b38fae120eb 100644 --- a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java @@ -1,21 +1,33 @@ package bisq.core.grpc; import bisq.core.btc.Balances; +import bisq.core.btc.model.AddressEntry; +import bisq.core.btc.wallet.BtcWalletService; import bisq.core.btc.wallet.WalletsManager; +import org.bitcoinj.core.Address; +import org.bitcoinj.core.TransactionConfidence; import org.bitcoinj.crypto.KeyCrypterScrypt; import javax.inject.Inject; import org.spongycastle.crypto.params.KeyParameter; +import java.text.DecimalFormat; + +import java.math.BigDecimal; + +import java.util.List; +import java.util.Optional; import java.util.Timer; import java.util.TimerTask; +import java.util.function.Function; import lombok.extern.slf4j.Slf4j; import javax.annotation.Nullable; +import static java.lang.String.format; import static java.util.concurrent.TimeUnit.SECONDS; @Slf4j @@ -23,6 +35,7 @@ class CoreWalletsService { private final Balances balances; private final WalletsManager walletsManager; + private final BtcWalletService btcWalletService; @Nullable private TimerTask lockTask; @@ -30,10 +43,19 @@ class CoreWalletsService { @Nullable private KeyParameter tempAesKey; + private final BigDecimal satoshiDivisor = new BigDecimal(100000000); + private final DecimalFormat btcFormat = new DecimalFormat("###,##0.00000000"); + @SuppressWarnings("BigDecimalMethodWithoutRoundingCalled") + private final Function formatSatoshis = (sats) -> + btcFormat.format(BigDecimal.valueOf(sats).divide(satoshiDivisor)); + @Inject - public CoreWalletsService(Balances balances, WalletsManager walletsManager) { + public CoreWalletsService(Balances balances, + WalletsManager walletsManager, + BtcWalletService btcWalletService) { this.balances = balances; this.walletsManager = walletsManager; + this.btcWalletService = btcWalletService; } public long getAvailableBalance() { @@ -50,6 +72,64 @@ public long getAvailableBalance() { return balance.getValue(); } + public long getAddressBalance(String addressString) { + Address address = getAddressEntry(addressString).getAddress(); + return btcWalletService.getBalanceForAddress(address).value; + } + + public String getFundingAddresses() { + if (!walletsManager.areWalletsAvailable()) + throw new IllegalStateException("wallet is not yet available"); + + if (walletsManager.areWalletsEncrypted() && tempAesKey == null) + throw new IllegalStateException("wallet is locked"); + + // TODO populate a List> to avoid repeated calls to + // fundingAddress.getAddressString() and getAddressBalance(addressString) + List fundingAddresses = btcWalletService.getAvailableAddressEntries(); + + // Create a new address with a zero balance if no addresses exist. + if (fundingAddresses.size() == 0) { + btcWalletService.getFreshAddressEntry(); + fundingAddresses = btcWalletService.getAvailableAddressEntries(); + } + + // Check to see if at least one of the existing addresses has a 0 balance. + boolean hasZeroBalance = false; + for (AddressEntry fundingAddress : fundingAddresses) { + if (getAddressBalance(fundingAddress.getAddressString()) == 0) { + hasZeroBalance = true; + break; + } + } + if (!hasZeroBalance) { + // None of the existing addresses have a zero balance, create a new one. + btcWalletService.getFreshAddressEntry(); + fundingAddresses = btcWalletService.getAvailableAddressEntries(); + } + + StringBuilder addressInfoBuilder = new StringBuilder(); + fundingAddresses.forEach(a -> { + var addressString = a.getAddressString(); + var satoshiBalance = getAddressBalance(addressString); + var btcBalance = formatSatoshis.apply(satoshiBalance); + var numConfirmations = getNumConfirmationsForMostRecentTransaction(addressString); + String addressInfo = "" + addressString + + " balance: " + btcBalance + + ((satoshiBalance > 0) ? (" confirmations: " + numConfirmations) : "") + + "\n"; + addressInfoBuilder.append(addressInfo); + }); + + return addressInfoBuilder.toString().trim(); + } + + public int getNumConfirmationsForMostRecentTransaction(String addressString) { + Address address = getAddressEntry(addressString).getAddress(); + TransactionConfidence confidence = btcWalletService.getConfidenceForAddress(address); + return confidence == null ? 0 : confidence.getDepthInBlocks(); + } + public void setWalletPassword(String password, String newPassword) { if (!walletsManager.areWalletsAvailable()) throw new IllegalStateException("wallet is not yet available"); @@ -156,4 +236,16 @@ private KeyCrypterScrypt getKeyCrypterScrypt() { throw new IllegalStateException("wallet encrypter is not available"); return keyCrypterScrypt; } + + private AddressEntry getAddressEntry(String addressString) { + Optional addressEntry = + btcWalletService.getAddressEntryListAsImmutableList().stream() + .filter(e -> addressString.equals(e.getAddressString())) + .findFirst(); + + if (!addressEntry.isPresent()) + throw new IllegalStateException(format("address %s not found in wallet", addressString)); + + return addressEntry.get(); + } } diff --git a/core/src/main/java/bisq/core/grpc/GrpcWalletService.java b/core/src/main/java/bisq/core/grpc/GrpcWalletService.java index 0ce59bb4c09..62373fd1be9 100644 --- a/core/src/main/java/bisq/core/grpc/GrpcWalletService.java +++ b/core/src/main/java/bisq/core/grpc/GrpcWalletService.java @@ -2,6 +2,8 @@ import bisq.proto.grpc.GetBalanceReply; import bisq.proto.grpc.GetBalanceRequest; +import bisq.proto.grpc.GetFundingAddressesReply; +import bisq.proto.grpc.GetFundingAddressesRequest; import bisq.proto.grpc.LockWalletReply; import bisq.proto.grpc.LockWalletRequest; import bisq.proto.grpc.RemoveWalletPasswordReply; @@ -40,6 +42,21 @@ public void getBalance(GetBalanceRequest req, StreamObserver re throw ex; } } + + @Override + public void getFundingAddresses(GetFundingAddressesRequest req, + StreamObserver responseObserver) { + try { + String result = walletsService.getFundingAddresses(); + var reply = GetFundingAddressesReply.newBuilder().setFundingAddressesInfo(result).build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } catch (IllegalStateException cause) { + var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); + responseObserver.onError(ex); + throw ex; + } + } @Override public void setWalletPassword(SetWalletPasswordRequest req, diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index b8db4c6d24b..9e85dbe371e 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -119,6 +119,8 @@ message PlaceOfferReply { service Wallet { rpc GetBalance (GetBalanceRequest) returns (GetBalanceReply) { } + rpc GetFundingAddresses (GetFundingAddressesRequest) returns (GetFundingAddressesReply) { + } rpc SetWalletPassword (SetWalletPasswordRequest) returns (SetWalletPasswordReply) { } rpc RemoveWalletPassword (RemoveWalletPasswordRequest) returns (RemoveWalletPasswordReply) { @@ -136,6 +138,13 @@ message GetBalanceReply { uint64 balance = 1; } +message GetFundingAddressesRequest { +} + +message GetFundingAddressesReply { + string fundingAddressesInfo = 1; +} + message SetWalletPasswordRequest { string password = 1; string newPassword = 2; From 2e415de4adaf54a9f9c926fa4557af95e6fdfbe4 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sun, 14 Jun 2020 13:05:37 -0300 Subject: [PATCH 4/9] Replace duplicate code in getFundingAddresses Cleaned up the method body and improved the returned string's formatting. Also added a line for this method in the CLI help text. --- cli/src/main/java/bisq/cli/CliMain.java | 1 + .../bisq/core/grpc/CoreWalletsService.java | 51 +++++++++++-------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index ad59befae77..36bb55add62 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -230,6 +230,7 @@ private static void printHelp(OptionParser parser, PrintStream stream) { stream.format("%-19s%-30s%s%n", "------", "------", "------------"); stream.format("%-19s%-30s%s%n", "getversion", "", "Get server version"); stream.format("%-19s%-30s%s%n", "getbalance", "", "Get server wallet balance"); + stream.format("%-19s%-30s%s%n", "getfundingaddresses", "", "Get BTC funding addresses"); stream.format("%-19s%-30s%s%n", "lockwallet", "", "Remove wallet password from memory, locking the wallet"); stream.format("%-19s%-30s%s%n", "unlockwallet", "password timeout", "Store wallet password in memory for timeout seconds"); diff --git a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java index b38fae120eb..64d70e15c73 100644 --- a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java @@ -5,6 +5,8 @@ import bisq.core.btc.wallet.BtcWalletService; import bisq.core.btc.wallet.WalletsManager; +import bisq.common.util.Tuple3; + import org.bitcoinj.core.Address; import org.bitcoinj.core.TransactionConfidence; import org.bitcoinj.crypto.KeyCrypterScrypt; @@ -22,6 +24,7 @@ import java.util.Timer; import java.util.TimerTask; import java.util.function.Function; +import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; @@ -84,39 +87,43 @@ public String getFundingAddresses() { if (walletsManager.areWalletsEncrypted() && tempAesKey == null) throw new IllegalStateException("wallet is locked"); - // TODO populate a List> to avoid repeated calls to - // fundingAddress.getAddressString() and getAddressBalance(addressString) - List fundingAddresses = btcWalletService.getAvailableAddressEntries(); - - // Create a new address with a zero balance if no addresses exist. - if (fundingAddresses.size() == 0) { + // Create a new funding address if none exists. + if (btcWalletService.getAvailableAddressEntries().size() == 0) btcWalletService.getFreshAddressEntry(); - fundingAddresses = btcWalletService.getAvailableAddressEntries(); - } - // Check to see if at least one of the existing addresses has a 0 balance. + // Populate a list of Tuple3 + List> addrBalanceConfirms = + btcWalletService.getAvailableAddressEntries().stream() + .map(a -> new Tuple3<>(a.getAddressString(), + getAddressBalance(a.getAddressString()), + getNumConfirmationsForMostRecentTransaction(a.getAddressString()))) + .collect(Collectors.toList()); + + // Check to see if at least one of the existing addresses has a zero balance. boolean hasZeroBalance = false; - for (AddressEntry fundingAddress : fundingAddresses) { - if (getAddressBalance(fundingAddress.getAddressString()) == 0) { + for (Tuple3 abc : addrBalanceConfirms) { + if (abc.second == 0) { hasZeroBalance = true; break; } } if (!hasZeroBalance) { - // None of the existing addresses have a zero balance, create a new one. - btcWalletService.getFreshAddressEntry(); - fundingAddresses = btcWalletService.getAvailableAddressEntries(); + // None of the existing addresses have a zero balance, create a new address. + addrBalanceConfirms.add( + new Tuple3<>(btcWalletService.getFreshAddressEntry().getAddressString(), + 0L, + 0)); } + // Iterate the list of Tuple3 objects + // and build the formatted info string. StringBuilder addressInfoBuilder = new StringBuilder(); - fundingAddresses.forEach(a -> { - var addressString = a.getAddressString(); - var satoshiBalance = getAddressBalance(addressString); - var btcBalance = formatSatoshis.apply(satoshiBalance); - var numConfirmations = getNumConfirmationsForMostRecentTransaction(addressString); - String addressInfo = "" + addressString - + " balance: " + btcBalance - + ((satoshiBalance > 0) ? (" confirmations: " + numConfirmations) : "") + addrBalanceConfirms.forEach(a -> { + var btcBalance = formatSatoshis.apply(a.second); + var numConfirmations = getNumConfirmationsForMostRecentTransaction(a.first); + String addressInfo = "" + a.first + + " balance: " + format("%13s", btcBalance) + + ((a.second > 0) ? (" confirmations: " + format("%6d", numConfirmations)) : "") + "\n"; addressInfoBuilder.append(addressInfo); }); From b1228e5ea72211c9010943db1aa8bf46675417a6 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sun, 14 Jun 2020 14:23:47 -0300 Subject: [PATCH 5/9] Add rpc method 'getaddressbalance' This addresses task 2 in issue 4257 https://github.com/bisq-network/bisq/issues/4257 This new gRPC Wallet service method displays the balance and number of confimirmations of the most recent transaction for the given BTC wallet address. The new method required the usual boilerplate changes to grpc.proto, CliMain, and GrpcWalletService. Two unit tests to check error msgs was added to cli/test.sh. --- cli/src/main/java/bisq/cli/CliMain.java | 13 +++++++++++++ cli/test.sh | 14 ++++++++++++++ .../bisq/core/grpc/CoreWalletsService.java | 9 +++++++++ .../bisq/core/grpc/GrpcWalletService.java | 19 ++++++++++++++++++- proto/src/main/proto/grpc.proto | 10 ++++++++++ 5 files changed, 64 insertions(+), 1 deletion(-) diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index 36bb55add62..ab6f39fa41c 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -17,6 +17,7 @@ package bisq.cli; +import bisq.proto.grpc.GetAddressBalanceRequest; import bisq.proto.grpc.GetBalanceRequest; import bisq.proto.grpc.GetFundingAddressesRequest; import bisq.proto.grpc.GetVersionGrpc; @@ -59,6 +60,7 @@ public class CliMain { private enum Method { getversion, getbalance, + getaddressbalance, getfundingaddresses, lockwallet, unlockwallet, @@ -154,6 +156,16 @@ public static void run(String[] args) { out.println(btcBalance); return; } + case getaddressbalance: { + if (nonOptionArgs.size() < 2) + throw new IllegalArgumentException("no address specified"); + + var request = GetAddressBalanceRequest.newBuilder() + .setAddress(nonOptionArgs.get(1)).build(); + var reply = walletService.getAddressBalance(request); + out.println(reply.getAddressBalanceInfo()); + return; + } case getfundingaddresses: { var request = GetFundingAddressesRequest.newBuilder().build(); var reply = walletService.getFundingAddresses(request); @@ -230,6 +242,7 @@ private static void printHelp(OptionParser parser, PrintStream stream) { stream.format("%-19s%-30s%s%n", "------", "------", "------------"); stream.format("%-19s%-30s%s%n", "getversion", "", "Get server version"); stream.format("%-19s%-30s%s%n", "getbalance", "", "Get server wallet balance"); + stream.format("%-19s%-30s%s%n", "getaddressbalance", "", "Get server wallet address balance"); stream.format("%-19s%-30s%s%n", "getfundingaddresses", "", "Get BTC funding addresses"); stream.format("%-19s%-30s%s%n", "lockwallet", "", "Remove wallet password from memory, locking the wallet"); stream.format("%-19s%-30s%s%n", "unlockwallet", "password timeout", diff --git a/cli/test.sh b/cli/test.sh index be2e67bc46f..875cb0a8a27 100755 --- a/cli/test.sh +++ b/cli/test.sh @@ -152,6 +152,20 @@ [ "$status" -eq 0 ] } +@test "test getaddressbalance missing address argument" { + run ./bisq-cli --password=xyz getaddressbalance + [ "$status" -eq 1 ] + echo "actual output: $output" >&2 + [ "$output" = "Error: no address specified" ] +} + +@test "test getaddressbalance bogus address argument" { + run ./bisq-cli --password=xyz getaddressbalance bogus + [ "$status" -eq 1 ] + echo "actual output: $output" >&2 + [ "$output" = "Error: address bogus not found in wallet" ] +} + @test "test help displayed on stderr if no options or arguments" { run ./bisq-cli [ "$status" -eq 1 ] diff --git a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java index 64d70e15c73..f9e3b7d1c60 100644 --- a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java @@ -80,6 +80,15 @@ public long getAddressBalance(String addressString) { return btcWalletService.getBalanceForAddress(address).value; } + public String getAddressBalanceInfo(String addressString) { + var satoshiBalance = getAddressBalance(addressString); + var btcBalance = formatSatoshis.apply(satoshiBalance); + var numConfirmations = getNumConfirmationsForMostRecentTransaction(addressString); + return addressString + + " balance: " + format("%13s", btcBalance) + + ((numConfirmations > 0) ? (" confirmations: " + format("%6d", numConfirmations)) : ""); + } + public String getFundingAddresses() { if (!walletsManager.areWalletsAvailable()) throw new IllegalStateException("wallet is not yet available"); diff --git a/core/src/main/java/bisq/core/grpc/GrpcWalletService.java b/core/src/main/java/bisq/core/grpc/GrpcWalletService.java index 62373fd1be9..0343960f394 100644 --- a/core/src/main/java/bisq/core/grpc/GrpcWalletService.java +++ b/core/src/main/java/bisq/core/grpc/GrpcWalletService.java @@ -1,5 +1,7 @@ package bisq.core.grpc; +import bisq.proto.grpc.GetAddressBalanceReply; +import bisq.proto.grpc.GetAddressBalanceRequest; import bisq.proto.grpc.GetBalanceReply; import bisq.proto.grpc.GetBalanceRequest; import bisq.proto.grpc.GetFundingAddressesReply; @@ -42,7 +44,22 @@ public void getBalance(GetBalanceRequest req, StreamObserver re throw ex; } } - + + @Override + public void getAddressBalance(GetAddressBalanceRequest req, + StreamObserver responseObserver) { + try { + String result = walletsService.getAddressBalanceInfo(req.getAddress()); + var reply = GetAddressBalanceReply.newBuilder().setAddressBalanceInfo(result).build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } catch (IllegalStateException cause) { + var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); + responseObserver.onError(ex); + throw ex; + } + } + @Override public void getFundingAddresses(GetFundingAddressesRequest req, StreamObserver responseObserver) { diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index 9e85dbe371e..41b490b9b53 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -119,6 +119,8 @@ message PlaceOfferReply { service Wallet { rpc GetBalance (GetBalanceRequest) returns (GetBalanceReply) { } + rpc GetAddressBalance (GetAddressBalanceRequest) returns (GetAddressBalanceReply) { + } rpc GetFundingAddresses (GetFundingAddressesRequest) returns (GetFundingAddressesReply) { } rpc SetWalletPassword (SetWalletPasswordRequest) returns (SetWalletPasswordReply) { @@ -138,6 +140,14 @@ message GetBalanceReply { uint64 balance = 1; } +message GetAddressBalanceRequest { + string address = 1; +} + +message GetAddressBalanceReply { + string addressBalanceInfo = 1; +} + message GetFundingAddressesRequest { } From a7542e98bf62d86bc3486e1d4d925a9a0a64d8c6 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 15 Jun 2020 14:11:51 -0300 Subject: [PATCH 6/9] Add rpc method 'createpaymentacct' This addresses task 4 in issue 4257. https://github.com/bisq-network/bisq/issues/4257 This PR should be reviewed/merged after PR 4304. https://github.com/bisq-network/bisq/pull/4304 This new gRPC PaymentAccounts service method creates a dummy PerfectMoney payment account for the given name, number and fiat currency code, as part of the required "simplest possible trading API" (for demo). An implementation supporting all payment methods is not in the scope. Changes specific to the new rpc method implementation follow: * New createpaymentacct method + help text was added to CliMain. Help text formatting was also changed to make room for larger method names and argument lists. * The PaymentAccount proto service def was renamed PaymentAccounts to avoid a name collision, and the new rpc CreatePaymentAccount was made part of the newly named PaymentAccounts service def. * New GrpcPaymentAccountsService (gRPC boilerplate) and CorePaymentAccountsService (method implementations) classes were added. * The gRPC GetPaymentAccountsService stub was moved from GrpcServer to the new GrpcPaymentAccountsService class, and GrpcPaymentAccountsService is injected into GrpcServer. * A new createpaymentacct unit test was added to the bats test suite (checks for successful return status code). Maybe bit out of scope, some small changes were made towards making sure the entire API is defined in CoreApi, which is used as a pass-through object to the new CorePaymentAccountsService. In the next PR, similar refactoring will be done to make CoreApi the pass-through object for all of the existing CoreWalletsService methods. (CoreWalletsService will be injected into CoreApi.) In the future, all Grpc*Service implementations will call core services through CoreApi, for the sake of consistency. --- cli/src/main/java/bisq/cli/CliMain.java | 47 ++++++++++++--- cli/test.sh | 5 ++ .../src/main/java/bisq/core/grpc/CoreApi.java | 11 +++- .../core/grpc/CorePaymentAccountsService.java | 57 +++++++++++++++++++ .../core/grpc/GrpcPaymentAccountsService.java | 46 +++++++++++++++ .../main/java/bisq/core/grpc/GrpcServer.java | 26 ++------- proto/src/main/proto/grpc.proto | 15 ++++- 7 files changed, 173 insertions(+), 34 deletions(-) create mode 100644 core/src/main/java/bisq/core/grpc/CorePaymentAccountsService.java create mode 100644 core/src/main/java/bisq/core/grpc/GrpcPaymentAccountsService.java diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index ab6f39fa41c..c7c16a4376f 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -17,12 +17,14 @@ package bisq.cli; +import bisq.proto.grpc.CreatePaymentAccountRequest; import bisq.proto.grpc.GetAddressBalanceRequest; import bisq.proto.grpc.GetBalanceRequest; import bisq.proto.grpc.GetFundingAddressesRequest; import bisq.proto.grpc.GetVersionGrpc; import bisq.proto.grpc.GetVersionRequest; import bisq.proto.grpc.LockWalletRequest; +import bisq.proto.grpc.PaymentAccountsGrpc; import bisq.proto.grpc.RemoveWalletPasswordRequest; import bisq.proto.grpc.SetWalletPasswordRequest; import bisq.proto.grpc.UnlockWalletRequest; @@ -58,6 +60,7 @@ public class CliMain { private enum Method { + createpaymentacct, getversion, getbalance, getaddressbalance, @@ -135,6 +138,7 @@ public static void run(String[] args) { })); var versionService = GetVersionGrpc.newBlockingStub(channel).withCallCredentials(credentials); + var paymentAccountsService = PaymentAccountsGrpc.newBlockingStub(channel).withCallCredentials(credentials); var walletService = WalletGrpc.newBlockingStub(channel).withCallCredentials(credentials); try { @@ -172,6 +176,30 @@ public static void run(String[] args) { out.println(reply.getFundingAddressesInfo()); return; } + case createpaymentacct: { + if (nonOptionArgs.size() < 2) + throw new IllegalArgumentException("no account name specified"); + + var accountName = nonOptionArgs.get(1); + + if (nonOptionArgs.size() < 3) + throw new IllegalArgumentException("no account number specified"); + + var accountNumber = nonOptionArgs.get(2); + + if (nonOptionArgs.size() < 4) + throw new IllegalArgumentException("no fiat currency specified"); + + var fiatCurrencyCode = nonOptionArgs.get(3).toUpperCase(); + + var request = CreatePaymentAccountRequest.newBuilder() + .setAccountName(accountName) + .setAccountNumber(accountNumber) + .setFiatCurrencyCode(fiatCurrencyCode).build(); + paymentAccountsService.createPaymentAccount(request); + out.println(format("payment account %s saved", accountName)); + return; + } case lockwallet: { var request = LockWalletRequest.newBuilder().build(); walletService.lockWallet(request); @@ -238,16 +266,17 @@ private static void printHelp(OptionParser parser, PrintStream stream) { stream.println(); parser.printHelpOn(stream); stream.println(); - stream.format("%-19s%-30s%s%n", "Method", "Params", "Description"); - stream.format("%-19s%-30s%s%n", "------", "------", "------------"); - stream.format("%-19s%-30s%s%n", "getversion", "", "Get server version"); - stream.format("%-19s%-30s%s%n", "getbalance", "", "Get server wallet balance"); - stream.format("%-19s%-30s%s%n", "getaddressbalance", "", "Get server wallet address balance"); - stream.format("%-19s%-30s%s%n", "getfundingaddresses", "", "Get BTC funding addresses"); - stream.format("%-19s%-30s%s%n", "lockwallet", "", "Remove wallet password from memory, locking the wallet"); - stream.format("%-19s%-30s%s%n", "unlockwallet", "password timeout", + stream.format("%-22s%-50s%s%n", "Method", "Params", "Description"); + stream.format("%-22s%-50s%s%n", "------", "------", "------------"); + stream.format("%-22s%-50s%s%n", "getversion", "", "Get server version"); + stream.format("%-22s%-50s%s%n", "getbalance", "", "Get server wallet balance"); + stream.format("%-22s%-50s%s%n", "getaddressbalance", "address", "Get server wallet address balance"); + stream.format("%-22s%-50s%s%n", "getfundingaddresses", "", "Get BTC funding addresses"); + stream.format("%-22s%-50s%s%n", "createpaymentacct", "account name, account number, currency code", "Create PerfectMoney dummy account"); + stream.format("%-22s%-50s%s%n", "lockwallet", "", "Remove wallet password from memory, locking the wallet"); + stream.format("%-22s%-50s%s%n", "unlockwallet", "password timeout", "Store wallet password in memory for timeout seconds"); - stream.format("%-19s%-30s%s%n", "setwalletpassword", "password [newpassword]", + stream.format("%-22s%-50s%s%n", "setwalletpassword", "password [newpassword]", "Encrypt wallet with password, or set new password on encrypted wallet"); stream.println(); } catch (IOException ex) { diff --git a/cli/test.sh b/cli/test.sh index 875cb0a8a27..79754d188bb 100755 --- a/cli/test.sh +++ b/cli/test.sh @@ -166,6 +166,11 @@ [ "$output" = "Error: address bogus not found in wallet" ] } +@test "test createpaymentacct PerfectMoneyDummy 0123456789 USD" { + run ./bisq-cli --password=xyz createpaymentacct PerfectMoneyDummy 0123456789 USD + [ "$status" -eq 0 ] +} + @test "test help displayed on stderr if no options or arguments" { run ./bisq-cli [ "$status" -eq 1 ] diff --git a/core/src/main/java/bisq/core/grpc/CoreApi.java b/core/src/main/java/bisq/core/grpc/CoreApi.java index a0671f4d3b0..8d45f31d5d3 100644 --- a/core/src/main/java/bisq/core/grpc/CoreApi.java +++ b/core/src/main/java/bisq/core/grpc/CoreApi.java @@ -47,6 +47,7 @@ */ @Slf4j public class CoreApi { + private final CorePaymentAccountsService paymentAccountsService; private final OfferBookService offerBookService; private final TradeStatisticsManager tradeStatisticsManager; private final CreateOfferService createOfferService; @@ -54,11 +55,13 @@ public class CoreApi { private final User user; @Inject - public CoreApi(OfferBookService offerBookService, + public CoreApi(CorePaymentAccountsService paymentAccountsService, + OfferBookService offerBookService, TradeStatisticsManager tradeStatisticsManager, CreateOfferService createOfferService, OpenOfferManager openOfferManager, User user) { + this.paymentAccountsService = paymentAccountsService; this.offerBookService = offerBookService; this.tradeStatisticsManager = tradeStatisticsManager; this.createOfferService = createOfferService; @@ -78,8 +81,12 @@ public List getOffers() { return offerBookService.getOffers(); } + public void createPaymentAccount(String accountName, String accountNumber, String fiatCurrencyCode) { + paymentAccountsService.createPaymentAccount(accountName, accountNumber, fiatCurrencyCode); + } + public Set getPaymentAccounts() { - return user.getPaymentAccounts(); + return paymentAccountsService.getPaymentAccounts(); } public void placeOffer(String currencyCode, diff --git a/core/src/main/java/bisq/core/grpc/CorePaymentAccountsService.java b/core/src/main/java/bisq/core/grpc/CorePaymentAccountsService.java new file mode 100644 index 00000000000..db2d3be4a03 --- /dev/null +++ b/core/src/main/java/bisq/core/grpc/CorePaymentAccountsService.java @@ -0,0 +1,57 @@ +package bisq.core.grpc; + +import bisq.core.account.witness.AccountAgeWitnessService; +import bisq.core.locale.FiatCurrency; +import bisq.core.payment.PaymentAccount; +import bisq.core.payment.PaymentAccountFactory; +import bisq.core.payment.PerfectMoneyAccount; +import bisq.core.payment.payload.PaymentMethod; +import bisq.core.user.User; + +import bisq.common.config.Config; + +import javax.inject.Inject; + +import java.util.Set; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class CorePaymentAccountsService { + + private final Config config; + private final AccountAgeWitnessService accountAgeWitnessService; + private final User user; + + @Inject + public CorePaymentAccountsService(Config config, + AccountAgeWitnessService accountAgeWitnessService, + User user) { + this.config = config; + this.accountAgeWitnessService = accountAgeWitnessService; + this.user = user; + } + + public void createPaymentAccount(String accountName, String accountNumber, String fiatCurrencyCode) { + // Create and persist a PerfectMoney dummy payment account. There is no guard + // against creating accounts with duplicate names & numbers, only the uuid and + // creation date are unique. + PaymentMethod dummyPaymentMethod = PaymentMethod.getDummyPaymentMethod(PaymentMethod.PERFECT_MONEY_ID); + PaymentAccount paymentAccount = PaymentAccountFactory.getPaymentAccount(dummyPaymentMethod); + paymentAccount.init(); + paymentAccount.setAccountName(accountName); + ((PerfectMoneyAccount) paymentAccount).setAccountNr(accountNumber); + paymentAccount.setSingleTradeCurrency(new FiatCurrency(fiatCurrencyCode)); + user.addPaymentAccount(paymentAccount); + + // Don't do this on mainnet until thoroughly tested. + if (config.baseCurrencyNetwork.isRegtest()) + accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload()); + + log.info("Payment account {} saved", paymentAccount.getId()); + } + + public Set getPaymentAccounts() { + return user.getPaymentAccounts(); + } +} diff --git a/core/src/main/java/bisq/core/grpc/GrpcPaymentAccountsService.java b/core/src/main/java/bisq/core/grpc/GrpcPaymentAccountsService.java new file mode 100644 index 00000000000..f2a9abf0bbb --- /dev/null +++ b/core/src/main/java/bisq/core/grpc/GrpcPaymentAccountsService.java @@ -0,0 +1,46 @@ +package bisq.core.grpc; + +import bisq.core.payment.PaymentAccount; + +import bisq.proto.grpc.CreatePaymentAccountReply; +import bisq.proto.grpc.CreatePaymentAccountRequest; +import bisq.proto.grpc.GetPaymentAccountsReply; +import bisq.proto.grpc.GetPaymentAccountsRequest; +import bisq.proto.grpc.PaymentAccountsGrpc; + +import io.grpc.stub.StreamObserver; + +import javax.inject.Inject; + +import java.util.stream.Collectors; + + +public class GrpcPaymentAccountsService extends PaymentAccountsGrpc.PaymentAccountsImplBase { + + private final CoreApi coreApi; + + @Inject + public GrpcPaymentAccountsService(CoreApi coreApi) { + this.coreApi = coreApi; + } + + @Override + public void createPaymentAccount(CreatePaymentAccountRequest req, + StreamObserver responseObserver) { + coreApi.createPaymentAccount(req.getAccountName(), req.getAccountNumber(), req.getFiatCurrencyCode()); + var reply = CreatePaymentAccountReply.newBuilder().build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } + + @Override + public void getPaymentAccounts(GetPaymentAccountsRequest req, + StreamObserver responseObserver) { + var tradeStatistics = coreApi.getPaymentAccounts().stream() + .map(PaymentAccount::toProtoMessage) + .collect(Collectors.toList()); + var reply = GetPaymentAccountsReply.newBuilder().addAllPaymentAccounts(tradeStatistics).build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } +} diff --git a/core/src/main/java/bisq/core/grpc/GrpcServer.java b/core/src/main/java/bisq/core/grpc/GrpcServer.java index 2b3543572b1..2c4f766b3c6 100644 --- a/core/src/main/java/bisq/core/grpc/GrpcServer.java +++ b/core/src/main/java/bisq/core/grpc/GrpcServer.java @@ -18,7 +18,6 @@ package bisq.core.grpc; import bisq.core.offer.Offer; -import bisq.core.payment.PaymentAccount; import bisq.core.trade.handlers.TransactionResultHandler; import bisq.core.trade.statistics.TradeStatistics2; @@ -27,9 +26,6 @@ import bisq.proto.grpc.GetOffersGrpc; import bisq.proto.grpc.GetOffersReply; import bisq.proto.grpc.GetOffersRequest; -import bisq.proto.grpc.GetPaymentAccountsGrpc; -import bisq.proto.grpc.GetPaymentAccountsReply; -import bisq.proto.grpc.GetPaymentAccountsRequest; import bisq.proto.grpc.GetTradeStatisticsGrpc; import bisq.proto.grpc.GetTradeStatisticsReply; import bisq.proto.grpc.GetTradeStatisticsRequest; @@ -60,14 +56,17 @@ public class GrpcServer { private final Server server; @Inject - public GrpcServer(Config config, CoreApi coreApi, GrpcWalletService walletService) { + public GrpcServer(Config config, + CoreApi coreApi, + GrpcPaymentAccountsService paymentAccountsService, + GrpcWalletService walletService) { this.coreApi = coreApi; this.server = ServerBuilder.forPort(config.apiPort) .addService(new GetVersionService()) .addService(new GetTradeStatisticsService()) .addService(new GetOffersService()) - .addService(new GetPaymentAccountsService()) .addService(new PlaceOfferService()) + .addService(paymentAccountsService) .addService(walletService) .intercept(new PasswordAuthInterceptor(config.apiPassword)) .build(); @@ -125,21 +124,6 @@ public void getOffers(GetOffersRequest req, StreamObserver respo } } - class GetPaymentAccountsService extends GetPaymentAccountsGrpc.GetPaymentAccountsImplBase { - @Override - public void getPaymentAccounts(GetPaymentAccountsRequest req, - StreamObserver responseObserver) { - - var tradeStatistics = coreApi.getPaymentAccounts().stream() - .map(PaymentAccount::toProtoMessage) - .collect(Collectors.toList()); - - var reply = GetPaymentAccountsReply.newBuilder().addAllPaymentAccounts(tradeStatistics).build(); - responseObserver.onNext(reply); - responseObserver.onCompleted(); - } - } - class PlaceOfferService extends PlaceOfferGrpc.PlaceOfferImplBase { @Override public void placeOffer(PlaceOfferRequest req, StreamObserver responseObserver) { diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index 41b490b9b53..66c4eb2ada8 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -72,14 +72,25 @@ message GetOffersReply { } /////////////////////////////////////////////////////////////////////////////////////////// -// PaymentAccount +// PaymentAccounts /////////////////////////////////////////////////////////////////////////////////////////// -service GetPaymentAccounts { +service PaymentAccounts { + rpc CreatePaymentAccount (CreatePaymentAccountRequest) returns (CreatePaymentAccountReply) { + } rpc GetPaymentAccounts (GetPaymentAccountsRequest) returns (GetPaymentAccountsReply) { } } +message CreatePaymentAccountRequest { + string accountName = 1; + string accountNumber = 2; + string fiatCurrencyCode = 3; +} + +message CreatePaymentAccountReply { +} + message GetPaymentAccountsRequest { } From bac3ed5697b4b854bb6c12128cee5aca1f64d09f Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 15 Jun 2020 16:43:26 -0300 Subject: [PATCH 7/9] Call core wallets service methods from CoreApi This change is a refactoring of the gRPC Wallets service for the purpose of making CoreApi the entry point to all core implementations. These changes should have been made in PR 4295. See https://github.com/bisq-network/bisq/pull/4295 The gRPC Wallet proto def name was changed to Wallets because this service manages BSQ and BTC wallets, and GrpcWalletService was changed to GrpcWalletsService for the same reason. This PR should be reviewed/merged after PR 4308. See https://github.com/bisq-network/bisq/pull/4308 This PR's branch was created from the PR 4308 branch. --- cli/src/main/java/bisq/cli/CliMain.java | 18 +++--- .../src/main/java/bisq/core/grpc/CoreApi.java | 62 ++++++++++++++++--- .../main/java/bisq/core/grpc/GrpcServer.java | 2 +- ...etService.java => GrpcWalletsService.java} | 24 +++---- proto/src/main/proto/grpc.proto | 4 +- 5 files changed, 78 insertions(+), 32 deletions(-) rename core/src/main/java/bisq/core/grpc/{GrpcWalletService.java => GrpcWalletsService.java} (86%) diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index c7c16a4376f..dee11cc7dd7 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -28,7 +28,7 @@ import bisq.proto.grpc.RemoveWalletPasswordRequest; import bisq.proto.grpc.SetWalletPasswordRequest; import bisq.proto.grpc.UnlockWalletRequest; -import bisq.proto.grpc.WalletGrpc; +import bisq.proto.grpc.WalletsGrpc; import io.grpc.ManagedChannelBuilder; import io.grpc.StatusRuntimeException; @@ -139,7 +139,7 @@ public static void run(String[] args) { var versionService = GetVersionGrpc.newBlockingStub(channel).withCallCredentials(credentials); var paymentAccountsService = PaymentAccountsGrpc.newBlockingStub(channel).withCallCredentials(credentials); - var walletService = WalletGrpc.newBlockingStub(channel).withCallCredentials(credentials); + var walletsService = WalletsGrpc.newBlockingStub(channel).withCallCredentials(credentials); try { switch (method) { @@ -151,7 +151,7 @@ public static void run(String[] args) { } case getbalance: { var request = GetBalanceRequest.newBuilder().build(); - var reply = walletService.getBalance(request); + var reply = walletsService.getBalance(request); var satoshiBalance = reply.getBalance(); var satoshiDivisor = new BigDecimal(100000000); var btcFormat = new DecimalFormat("###,##0.00000000"); @@ -166,13 +166,13 @@ public static void run(String[] args) { var request = GetAddressBalanceRequest.newBuilder() .setAddress(nonOptionArgs.get(1)).build(); - var reply = walletService.getAddressBalance(request); + var reply = walletsService.getAddressBalance(request); out.println(reply.getAddressBalanceInfo()); return; } case getfundingaddresses: { var request = GetFundingAddressesRequest.newBuilder().build(); - var reply = walletService.getFundingAddresses(request); + var reply = walletsService.getFundingAddresses(request); out.println(reply.getFundingAddressesInfo()); return; } @@ -202,7 +202,7 @@ public static void run(String[] args) { } case lockwallet: { var request = LockWalletRequest.newBuilder().build(); - walletService.lockWallet(request); + walletsService.lockWallet(request); out.println("wallet locked"); return; } @@ -222,7 +222,7 @@ public static void run(String[] args) { var request = UnlockWalletRequest.newBuilder() .setPassword(nonOptionArgs.get(1)) .setTimeout(timeout).build(); - walletService.unlockWallet(request); + walletsService.unlockWallet(request); out.println("wallet unlocked"); return; } @@ -231,7 +231,7 @@ public static void run(String[] args) { throw new IllegalArgumentException("no password specified"); var request = RemoveWalletPasswordRequest.newBuilder().setPassword(nonOptionArgs.get(1)).build(); - walletService.removeWalletPassword(request); + walletsService.removeWalletPassword(request); out.println("wallet decrypted"); return; } @@ -243,7 +243,7 @@ public static void run(String[] args) { var hasNewPassword = nonOptionArgs.size() == 3; if (hasNewPassword) requestBuilder.setNewPassword(nonOptionArgs.get(2)); - walletService.setWalletPassword(requestBuilder.build()); + walletsService.setWalletPassword(requestBuilder.build()); out.println("wallet encrypted" + (hasNewPassword ? " with new password" : "")); return; } diff --git a/core/src/main/java/bisq/core/grpc/CoreApi.java b/core/src/main/java/bisq/core/grpc/CoreApi.java index 8d45f31d5d3..610f0d7d8dd 100644 --- a/core/src/main/java/bisq/core/grpc/CoreApi.java +++ b/core/src/main/java/bisq/core/grpc/CoreApi.java @@ -48,6 +48,7 @@ @Slf4j public class CoreApi { private final CorePaymentAccountsService paymentAccountsService; + private final CoreWalletsService walletsService; private final OfferBookService offerBookService; private final TradeStatisticsManager tradeStatisticsManager; private final CreateOfferService createOfferService; @@ -56,12 +57,14 @@ public class CoreApi { @Inject public CoreApi(CorePaymentAccountsService paymentAccountsService, + CoreWalletsService walletsService, OfferBookService offerBookService, TradeStatisticsManager tradeStatisticsManager, CreateOfferService createOfferService, OpenOfferManager openOfferManager, User user) { this.paymentAccountsService = paymentAccountsService; + this.walletsService = walletsService; this.offerBookService = offerBookService; this.tradeStatisticsManager = tradeStatisticsManager; this.createOfferService = createOfferService; @@ -73,20 +76,52 @@ public String getVersion() { return Version.VERSION; } - public List getTradeStatistics() { - return new ArrayList<>(tradeStatisticsManager.getObservableTradeStatisticsSet()); + /////////////////////////////////////////////////////////////////////////////////////////// + // Wallets + /////////////////////////////////////////////////////////////////////////////////////////// + + public long getAvailableBalance() { + return walletsService.getAvailableBalance(); } - public List getOffers() { - return offerBookService.getOffers(); + public long getAddressBalance(String addressString) { + return walletsService.getAddressBalance(addressString); } - public void createPaymentAccount(String accountName, String accountNumber, String fiatCurrencyCode) { - paymentAccountsService.createPaymentAccount(accountName, accountNumber, fiatCurrencyCode); + public String getAddressBalanceInfo(String addressString) { + return walletsService.getAddressBalanceInfo(addressString); } - public Set getPaymentAccounts() { - return paymentAccountsService.getPaymentAccounts(); + public String getFundingAddresses() { + return walletsService.getFundingAddresses(); + } + + public void setWalletPassword(String password, String newPassword) { + walletsService.setWalletPassword(password, newPassword); + } + + public void lockWallet() { + walletsService.lockWallet(); + } + + public void unlockWallet(String password, long timeout) { + walletsService.unlockWallet(password, timeout); + } + + public void removeWalletPassword(String password) { + walletsService.removeWalletPassword(password); + } + + public List getTradeStatistics() { + return new ArrayList<>(tradeStatisticsManager.getObservableTradeStatisticsSet()); + } + + public int getNumConfirmationsForMostRecentTransaction(String addressString) { + return walletsService.getNumConfirmationsForMostRecentTransaction(addressString); + } + + public List getOffers() { + return offerBookService.getOffers(); } public void placeOffer(String currencyCode, @@ -152,4 +187,15 @@ public void placeOffer(String offerId, log::error); } + /////////////////////////////////////////////////////////////////////////////////////////// + // PaymentAccounts + /////////////////////////////////////////////////////////////////////////////////////////// + + public void createPaymentAccount(String accountName, String accountNumber, String fiatCurrencyCode) { + paymentAccountsService.createPaymentAccount(accountName, accountNumber, fiatCurrencyCode); + } + + public Set getPaymentAccounts() { + return paymentAccountsService.getPaymentAccounts(); + } } diff --git a/core/src/main/java/bisq/core/grpc/GrpcServer.java b/core/src/main/java/bisq/core/grpc/GrpcServer.java index 2c4f766b3c6..6fa6dad9faf 100644 --- a/core/src/main/java/bisq/core/grpc/GrpcServer.java +++ b/core/src/main/java/bisq/core/grpc/GrpcServer.java @@ -59,7 +59,7 @@ public class GrpcServer { public GrpcServer(Config config, CoreApi coreApi, GrpcPaymentAccountsService paymentAccountsService, - GrpcWalletService walletService) { + GrpcWalletsService walletService) { this.coreApi = coreApi; this.server = ServerBuilder.forPort(config.apiPort) .addService(new GetVersionService()) diff --git a/core/src/main/java/bisq/core/grpc/GrpcWalletService.java b/core/src/main/java/bisq/core/grpc/GrpcWalletsService.java similarity index 86% rename from core/src/main/java/bisq/core/grpc/GrpcWalletService.java rename to core/src/main/java/bisq/core/grpc/GrpcWalletsService.java index 0343960f394..e7ec5629100 100644 --- a/core/src/main/java/bisq/core/grpc/GrpcWalletService.java +++ b/core/src/main/java/bisq/core/grpc/GrpcWalletsService.java @@ -14,7 +14,7 @@ import bisq.proto.grpc.SetWalletPasswordRequest; import bisq.proto.grpc.UnlockWalletReply; import bisq.proto.grpc.UnlockWalletRequest; -import bisq.proto.grpc.WalletGrpc; +import bisq.proto.grpc.WalletsGrpc; import io.grpc.Status; import io.grpc.StatusRuntimeException; @@ -22,19 +22,19 @@ import javax.inject.Inject; -class GrpcWalletService extends WalletGrpc.WalletImplBase { +class GrpcWalletsService extends WalletsGrpc.WalletsImplBase { - private final CoreWalletsService walletsService; + private final CoreApi coreApi; @Inject - public GrpcWalletService(CoreWalletsService walletsService) { - this.walletsService = walletsService; + public GrpcWalletsService(CoreApi coreApi) { + this.coreApi = coreApi; } @Override public void getBalance(GetBalanceRequest req, StreamObserver responseObserver) { try { - long result = walletsService.getAvailableBalance(); + long result = coreApi.getAvailableBalance(); var reply = GetBalanceReply.newBuilder().setBalance(result).build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -49,7 +49,7 @@ public void getBalance(GetBalanceRequest req, StreamObserver re public void getAddressBalance(GetAddressBalanceRequest req, StreamObserver responseObserver) { try { - String result = walletsService.getAddressBalanceInfo(req.getAddress()); + String result = coreApi.getAddressBalanceInfo(req.getAddress()); var reply = GetAddressBalanceReply.newBuilder().setAddressBalanceInfo(result).build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -64,7 +64,7 @@ public void getAddressBalance(GetAddressBalanceRequest req, public void getFundingAddresses(GetFundingAddressesRequest req, StreamObserver responseObserver) { try { - String result = walletsService.getFundingAddresses(); + String result = coreApi.getFundingAddresses(); var reply = GetFundingAddressesReply.newBuilder().setFundingAddressesInfo(result).build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -79,7 +79,7 @@ public void getFundingAddresses(GetFundingAddressesRequest req, public void setWalletPassword(SetWalletPasswordRequest req, StreamObserver responseObserver) { try { - walletsService.setWalletPassword(req.getPassword(), req.getNewPassword()); + coreApi.setWalletPassword(req.getPassword(), req.getNewPassword()); var reply = SetWalletPasswordReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -94,7 +94,7 @@ public void setWalletPassword(SetWalletPasswordRequest req, public void removeWalletPassword(RemoveWalletPasswordRequest req, StreamObserver responseObserver) { try { - walletsService.removeWalletPassword(req.getPassword()); + coreApi.removeWalletPassword(req.getPassword()); var reply = RemoveWalletPasswordReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -109,7 +109,7 @@ public void removeWalletPassword(RemoveWalletPasswordRequest req, public void lockWallet(LockWalletRequest req, StreamObserver responseObserver) { try { - walletsService.lockWallet(); + coreApi.lockWallet(); var reply = LockWalletReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); @@ -124,7 +124,7 @@ public void lockWallet(LockWalletRequest req, public void unlockWallet(UnlockWalletRequest req, StreamObserver responseObserver) { try { - walletsService.unlockWallet(req.getPassword(), req.getTimeout()); + coreApi.unlockWallet(req.getPassword(), req.getTimeout()); 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 66c4eb2ada8..1c7ca532b0a 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -124,10 +124,10 @@ message PlaceOfferReply { } /////////////////////////////////////////////////////////////////////////////////////////// -// Wallet +// Wallets /////////////////////////////////////////////////////////////////////////////////////////// -service Wallet { +service Wallets { rpc GetBalance (GetBalanceRequest) returns (GetBalanceReply) { } rpc GetAddressBalance (GetAddressBalanceRequest) returns (GetAddressBalanceReply) { From 258d1801d2d5cecd06c60e60e31fa8b2ede744e0 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Tue, 16 Jun 2020 12:48:41 -0300 Subject: [PATCH 8/9] Factor duplicate unlocked wallet checks into new method Response to comment in PR 4299: https://github.com/bisq-network/bisq/pull/4299#discussion_r440769032 This PR should be reviewed/merged after PR 4309. https://github.com/bisq-network/bisq/pull/4309 --- .../main/java/bisq/core/grpc/CoreWalletsService.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java index f9e3b7d1c60..be44122ab2e 100644 --- a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java @@ -65,8 +65,7 @@ public long getAvailableBalance() { if (!walletsManager.areWalletsAvailable()) throw new IllegalStateException("wallet is not yet available"); - if (walletsManager.areWalletsEncrypted() && tempAesKey == null) - throw new IllegalStateException("wallet is locked"); + verifyEncryptedWalletIsUnlocked(); var balance = balances.getAvailableBalance().get(); if (balance == null) @@ -93,8 +92,7 @@ public String getFundingAddresses() { if (!walletsManager.areWalletsAvailable()) throw new IllegalStateException("wallet is not yet available"); - if (walletsManager.areWalletsEncrypted() && tempAesKey == null) - throw new IllegalStateException("wallet is locked"); + verifyEncryptedWalletIsUnlocked(); // Create a new funding address if none exists. if (btcWalletService.getAvailableAddressEntries().size() == 0) @@ -246,6 +244,12 @@ private void verifyWalletIsAvailableAndEncrypted() { throw new IllegalStateException("wallet is not encrypted with a password"); } + // Throws a RuntimeException if wallets are encrypted and locked. + private void verifyEncryptedWalletIsUnlocked() { + if (walletsManager.areWalletsEncrypted() && tempAesKey == null) + throw new IllegalStateException("wallet is locked"); + } + private KeyCrypterScrypt getKeyCrypterScrypt() { KeyCrypterScrypt keyCrypterScrypt = walletsManager.getKeyCrypterScrypt(); if (keyCrypterScrypt == null) From c5134e14c28663e16b07a1eaf1ab63e4428f1f5a Mon Sep 17 00:00:00 2001 From: Dominykas Mostauskis Date: Thu, 18 Jun 2020 15:00:23 +0200 Subject: [PATCH 9/9] Replace Tuple3 with memoization --- .../bisq/core/grpc/CoreWalletsService.java | 89 +++++++++++-------- 1 file changed, 53 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java index be44122ab2e..26a64e50696 100644 --- a/core/src/main/java/bisq/core/grpc/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/grpc/CoreWalletsService.java @@ -30,6 +30,10 @@ import javax.annotation.Nullable; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.LoadingCache; + import static java.lang.String.format; import static java.util.concurrent.TimeUnit.SECONDS; @@ -88,6 +92,20 @@ public String getAddressBalanceInfo(String addressString) { + ((numConfirmations > 0) ? (" confirmations: " + format("%6d", numConfirmations)) : ""); } + /** + * Memoization stores the results of expensive function calls and returns + * the cached result when the same input occurs again. + * + * Resulting LoadingCache is used by calling `.get(input I)` or + * `.getUnchecked(input I)`, depending on whether or not `f` can return null. + * That's because CacheLoader throws an exception on null output from `f`. + */ + private static LoadingCache memoize(Function f) { + // f::apply is used, because Guava 20.0 Function doesn't yet extend + // Java Function. + return CacheBuilder.newBuilder().build(CacheLoader.from(f::apply)); + } + public String getFundingAddresses() { if (!walletsManager.areWalletsAvailable()) throw new IllegalStateException("wallet is not yet available"); @@ -98,44 +116,43 @@ public String getFundingAddresses() { if (btcWalletService.getAvailableAddressEntries().size() == 0) btcWalletService.getFreshAddressEntry(); - // Populate a list of Tuple3 - List> addrBalanceConfirms = - btcWalletService.getAvailableAddressEntries().stream() - .map(a -> new Tuple3<>(a.getAddressString(), - getAddressBalance(a.getAddressString()), - getNumConfirmationsForMostRecentTransaction(a.getAddressString()))) - .collect(Collectors.toList()); - - // Check to see if at least one of the existing addresses has a zero balance. - boolean hasZeroBalance = false; - for (Tuple3 abc : addrBalanceConfirms) { - if (abc.second == 0) { - hasZeroBalance = true; - break; - } - } - if (!hasZeroBalance) { - // None of the existing addresses have a zero balance, create a new address. - addrBalanceConfirms.add( - new Tuple3<>(btcWalletService.getFreshAddressEntry().getAddressString(), - 0L, - 0)); + List addressStrings = + btcWalletService + .getAvailableAddressEntries() + .stream() + .map(addressEntry -> addressEntry.getAddressString()) + .collect(Collectors.toList()); + + // getAddressBalance is memoized, because we'll map it over addresses twice. + // To get the balances, we'll be using .getUnchecked, because we know that + // this::getAddressBalance cannot return null. + var balances = memoize(this::getAddressBalance); + + boolean noAddressHasZeroBalance = + addressStrings.stream() + .allMatch(addressString -> balances.getUnchecked(addressString) != 0); + + if (noAddressHasZeroBalance) { + var newZeroBalanceAddress = btcWalletService.getFreshAddressEntry(); + addressStrings.add(newZeroBalanceAddress.getAddressString()); } - // Iterate the list of Tuple3 objects - // and build the formatted info string. - StringBuilder addressInfoBuilder = new StringBuilder(); - addrBalanceConfirms.forEach(a -> { - var btcBalance = formatSatoshis.apply(a.second); - var numConfirmations = getNumConfirmationsForMostRecentTransaction(a.first); - String addressInfo = "" + a.first - + " balance: " + format("%13s", btcBalance) - + ((a.second > 0) ? (" confirmations: " + format("%6d", numConfirmations)) : "") - + "\n"; - addressInfoBuilder.append(addressInfo); - }); - - return addressInfoBuilder.toString().trim(); + String fundingAddressTable = + addressStrings.stream() + .map(addressString -> { + var balance = balances.getUnchecked(addressString); + var stringFormattedBalance = formatSatoshis.apply(balance); + var numConfirmations = + getNumConfirmationsForMostRecentTransaction(addressString); + String addressInfo = + "" + addressString + + " balance: " + format("%13s", stringFormattedBalance) + + ((balance > 0) ? (" confirmations: " + format("%6d", numConfirmations)) : ""); + return addressInfo; + }) + .collect(Collectors.joining("\n")); + + return fundingAddressTable; } public int getNumConfirmationsForMostRecentTransaction(String addressString) {