Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor gRPC service error handling #4960

Merged
merged 28 commits into from
Dec 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
dc6144d
Refactor BtcWalletService to let api override fee rates
ghubstan Dec 4, 2020
900d498
Merge branch 'master' into 02-refactor-completePreparedSendBsqTx
ghubstan Dec 4, 2020
159d4cc
Add optional txFeeRate parameter to api sendbsq
ghubstan Dec 4, 2020
2842070
Merge branch 'master' into 03-add-txFeeRate-param
ghubstan Dec 8, 2020
6c9f0c2
Add new api method 'sendbtc' and test
ghubstan Dec 9, 2020
144c5a8
Merge branch 'master' into 04-add-sendbtc-impl
ghubstan Dec 9, 2020
bd66008
Support tx memo field for btc withdrawals from api
ghubstan Dec 9, 2020
478c8f4
Remove unused imports
ghubstan Dec 9, 2020
259bad6
Merge branch 'master' into 05-use-memo-tx-field
ghubstan Dec 10, 2020
150e2f6
Use Bisq's UserThread.executor in gRPC server
ghubstan Dec 11, 2020
6aa385e
Append nullable withdrawalTxId field to Trade proto message
ghubstan Dec 12, 2020
5522d0c
Add new api method gettransaction
ghubstan Dec 14, 2020
a0f1c22
Merge branch 'master' into 08-scratch
ghubstan Dec 14, 2020
0384642
Adjust create TransferwiseAccount test
ghubstan Dec 14, 2020
4be87a6
Disable method test to avoid repetition
ghubstan Dec 14, 2020
a341173
Merge branch 'master' into 09-scratch
ghubstan Dec 15, 2020
e6c6d3b
Add new CoreApiExceptionHandler to gRPC services
ghubstan Dec 16, 2020
1cd47fd
Merge branch 'master' into 09-refactor-grpc-error-handling
ghubstan Dec 16, 2020
c60605f
Fix class level comment
ghubstan Dec 16, 2020
f7c1103
Rename gRPC exception handler class
ghubstan Dec 16, 2020
672eb79
Revert "Append nullable withdrawalTxId field to Trade proto message"
ghubstan Dec 21, 2020
bdde24a
Ajust TradeInfo to reverting 6aa385e494eb8fa870257c76e078108607503d03
ghubstan Dec 21, 2020
64c2ac5
Adjust grpc.proto to reverting 6aa385e494eb8fa870257c76e078108607503d03
ghubstan Dec 21, 2020
97dcac2
Adjust TradeFormat to reverting 6aa385e494eb8fa870257c76e078108607503d03
ghubstan Dec 21, 2020
3a770f4
Adjust TakeSellBTCOfferTest to reverting 6aa385e494eb8fa870257c76e078…
ghubstan Dec 21, 2020
4aa4270
Adjust TradeTest to reverting 6aa385e494eb8fa870257c76e078108607503d03
ghubstan Dec 21, 2020
1507a2c
Resolve file conflict w/ master
ghubstan Dec 21, 2020
8ee3a15
Merge branch '08-gettransaction' into 09-refactor-grpc-error-handling
ghubstan Dec 21, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
73 changes: 66 additions & 7 deletions apitest/src/test/java/bisq/apitest/method/MethodTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import bisq.proto.grpc.GetPaymentAccountsRequest;
import bisq.proto.grpc.GetPaymentMethodsRequest;
import bisq.proto.grpc.GetTradeRequest;
import bisq.proto.grpc.GetTransactionRequest;
import bisq.proto.grpc.GetTxFeeRateRequest;
import bisq.proto.grpc.GetUnusedBsqAddressRequest;
import bisq.proto.grpc.KeepFundsRequest;
Expand All @@ -48,10 +49,12 @@
import bisq.proto.grpc.RegisterDisputeAgentRequest;
import bisq.proto.grpc.RemoveWalletPasswordRequest;
import bisq.proto.grpc.SendBsqRequest;
import bisq.proto.grpc.SendBtcRequest;
import bisq.proto.grpc.SetTxFeeRatePreferenceRequest;
import bisq.proto.grpc.SetWalletPasswordRequest;
import bisq.proto.grpc.TakeOfferRequest;
import bisq.proto.grpc.TradeInfo;
import bisq.proto.grpc.TxInfo;
import bisq.proto.grpc.UnlockWalletRequest;
import bisq.proto.grpc.UnsetTxFeeRatePreferenceRequest;
import bisq.proto.grpc.WithdrawFundsRequest;
Expand Down Expand Up @@ -160,8 +163,26 @@ protected final GetUnusedBsqAddressRequest createGetUnusedBsqAddressRequest() {
return GetUnusedBsqAddressRequest.newBuilder().build();
}

protected final SendBsqRequest createSendBsqRequest(String address, String amount) {
return SendBsqRequest.newBuilder().setAddress(address).setAmount(amount).build();
protected final SendBsqRequest createSendBsqRequest(String address,
String amount,
String txFeeRate) {
return SendBsqRequest.newBuilder()
.setAddress(address)
.setAmount(amount)
.setTxFeeRate(txFeeRate)
.build();
}

protected final SendBtcRequest createSendBtcRequest(String address,
String amount,
String txFeeRate,
String memo) {
return SendBtcRequest.newBuilder()
.setAddress(address)
.setAmount(amount)
.setTxFeeRate(txFeeRate)
.setMemo(memo)
.build();
}

protected final GetFundingAddressesRequest createGetFundingAddressesRequest() {
Expand Down Expand Up @@ -208,10 +229,13 @@ protected final KeepFundsRequest createKeepFundsRequest(String tradeId) {
.build();
}

protected final WithdrawFundsRequest createWithdrawFundsRequest(String tradeId, String address) {
protected final WithdrawFundsRequest createWithdrawFundsRequest(String tradeId,
String address,
String memo) {
return WithdrawFundsRequest.newBuilder()
.setTradeId(tradeId)
.setAddress(address)
.setMemo(memo)
.build();
}

Expand Down Expand Up @@ -247,9 +271,36 @@ protected final String getUnusedBsqAddress(BisqAppConfig bisqAppConfig) {
return grpcStubs(bisqAppConfig).walletsService.getUnusedBsqAddress(createGetUnusedBsqAddressRequest()).getAddress();
}

protected final void sendBsq(BisqAppConfig bisqAppConfig, String address, String amount) {
protected final TxInfo sendBsq(BisqAppConfig bisqAppConfig,
String address,
String amount) {
return sendBsq(bisqAppConfig, address, amount, "");
}

protected final TxInfo sendBsq(BisqAppConfig bisqAppConfig,
String address,
String amount,
String txFeeRate) {
//noinspection ResultOfMethodCallIgnored
grpcStubs(bisqAppConfig).walletsService.sendBsq(createSendBsqRequest(address, amount));
return grpcStubs(bisqAppConfig).walletsService.sendBsq(createSendBsqRequest(address,
amount,
txFeeRate))
.getTxInfo();
}

protected final TxInfo sendBtc(BisqAppConfig bisqAppConfig, String address, String amount) {
return sendBtc(bisqAppConfig, address, amount, "", "");
}

protected final TxInfo sendBtc(BisqAppConfig bisqAppConfig,
String address,
String amount,
String txFeeRate,
String memo) {
//noinspection ResultOfMethodCallIgnored
return grpcStubs(bisqAppConfig).walletsService.sendBtc(
createSendBtcRequest(address, amount, txFeeRate, memo))
.getTxInfo();
}

protected final String getUnusedBtcAddress(BisqAppConfig bisqAppConfig) {
Expand Down Expand Up @@ -354,8 +405,11 @@ protected final void keepFunds(BisqAppConfig bisqAppConfig, String tradeId) {
}

@SuppressWarnings("ResultOfMethodCallIgnored")
protected final void withdrawFunds(BisqAppConfig bisqAppConfig, String tradeId, String address) {
var req = createWithdrawFundsRequest(tradeId, address);
protected final void withdrawFunds(BisqAppConfig bisqAppConfig,
String tradeId,
String address,
String memo) {
var req = createWithdrawFundsRequest(tradeId, address, memo);
grpcStubs(bisqAppConfig).tradesService.withdrawFunds(req);
}

Expand All @@ -379,6 +433,11 @@ protected final TxFeeRateInfo unsetTxFeeRate(BisqAppConfig bisqAppConfig) {
grpcStubs(bisqAppConfig).walletsService.unsetTxFeeRatePreference(req).getTxFeeRateInfo());
}

protected final TxInfo getTransaction(BisqAppConfig bisqAppConfig, String txId) {
var req = GetTransactionRequest.newBuilder().setTxId(txId).build();
return grpcStubs(bisqAppConfig).walletsService.getTransaction(req).getTxInfo();
}

// Static conveniences for test methods and test case fixture setups.

protected static RegisterDisputeAgentRequest createRegisterDisputeAgentRequest(String disputeAgentType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,12 @@

import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind;
import static bisq.apitest.config.BisqAppConfig.alicedaemon;
import static bisq.core.locale.CurrencyUtil.*;
import static bisq.core.locale.CurrencyUtil.getAllAdvancedCashCurrencies;
import static bisq.core.locale.CurrencyUtil.getAllMoneyGramCurrencies;
import static bisq.core.locale.CurrencyUtil.getAllRevolutCurrencies;
import static bisq.core.locale.CurrencyUtil.getAllUpholdCurrencies;
import static bisq.core.payment.payload.PaymentMethod.*;
import static org.junit.Assert.assertEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation;

Expand Down Expand Up @@ -746,7 +749,10 @@ public void testCreateTransferwiseAccount(TestInfo testInfo) {
String jsonString = getCompletedFormAsJsonString();
TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(alicedaemon, jsonString);
verifyUserPayloadHasPaymentAccountWithId(paymentAccount.getId());
verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount);
// As per commit 88f26f93241af698ae689bf081205d0f9dc929fa
// Do not autofill all currencies by default but keep all unselected.
// verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount);
assertEquals(0, paymentAccount.getTradeCurrencies().size());
verifyCommonFormEntries(paymentAccount);
assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail());
log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,11 @@ protected final void logTrade(Logger log,
TestInfo testInfo,
String description,
TradeInfo trade) {
log.info(String.format("%s %s%n%s",
testName(testInfo),
description.toUpperCase(),
format(trade)));
if (log.isDebugEnabled()) {
log.debug(String.format("%s %s%n%s",
testName(testInfo),
description.toUpperCase(),
format(trade)));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public class TakeSellBTCOfferTest extends AbstractTradeTest {
// Maker and Taker fees are in BTC.
private static final String TRADE_FEE_CURRENCY_CODE = "btc";

private static final String WITHDRAWAL_TX_MEMO = "Bob's trade withdrawal";

@Test
@Order(1)
public void testTakeAlicesSellOffer(final TestInfo testInfo) {
Expand Down Expand Up @@ -147,7 +149,7 @@ public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) {
logTrade(log, testInfo, "Bob's view before withdrawing funds to external wallet", trade);

String toAddress = bitcoinCli.getNewBtcAddress();
withdrawFunds(bobdaemon, tradeId, toAddress);
withdrawFunds(bobdaemon, tradeId, toAddress, WITHDRAWAL_TX_MEMO);

genBtcBlocksThenWait(1, 2250);

Expand All @@ -158,7 +160,7 @@ public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) {
verifyExpectedProtocolStatus(trade);
logTrade(log, testInfo, "Bob's view after withdrawing funds to external wallet", trade);
BtcBalanceInfo currentBalance = getBtcBalances(bobdaemon);
log.info("{} Bob's current available balance: {} BTC",
log.debug("{} Bob's current available balance: {} BTC",
testName(testInfo),
formatSatoshis(currentBalance.getAvailableBalance()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void testInitialBsqBalances(final TestInfo testInfo) {
@Order(3)
public void testSendBsqAndCheckBalancesBeforeGeneratingBtcBlock(final TestInfo testInfo) {
String bobsBsqAddress = getUnusedBsqAddress(bobdaemon);
sendBsq(alicedaemon, bobsBsqAddress, SEND_BSQ_AMOUNT);
sendBsq(alicedaemon, bobsBsqAddress, SEND_BSQ_AMOUNT, "100");
sleep(2000);

BsqBalanceInfo alicesBsqBalances = getBsqBalances(alicedaemon);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package bisq.apitest.method.wallet;

import bisq.proto.grpc.BtcBalanceInfo;
import bisq.proto.grpc.TxInfo;

import lombok.extern.slf4j.Slf4j;

Expand All @@ -20,6 +21,8 @@
import static bisq.cli.TableFormat.formatBtcBalanceInfoTbl;
import static java.util.Collections.singletonList;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation;


Expand All @@ -31,6 +34,8 @@
@TestMethodOrder(OrderAnnotation.class)
public class BtcWalletTest extends MethodTest {

private static final String TX_MEMO = "tx memo";

// All api tests depend on the DAO / regtest environment, and Bob & Alice's wallets
// are initialized with 10 BTC during the scaffolding setup.
private static final bisq.core.api.model.BtcBalanceInfo INITIAL_BTC_BALANCES =
Expand Down Expand Up @@ -92,6 +97,50 @@ public void testFundAlicesBtcWallet(final TestInfo testInfo) {
formatBtcBalanceInfoTbl(btcBalanceInfo));
}

@Test
@Order(3)
public void testAliceSendBTCToBob(TestInfo testInfo) {
String bobsBtcAddress = getUnusedBtcAddress(bobdaemon);
log.debug("Sending 5.5 BTC From Alice to Bob @ {}", bobsBtcAddress);

TxInfo txInfo = sendBtc(alicedaemon,
bobsBtcAddress,
"5.50",
"100",
TX_MEMO);
assertTrue(txInfo.getIsPending());

// Note that the memo is not set on the tx yet.
assertTrue(txInfo.getMemo().isEmpty());
genBtcBlocksThenWait(1, 3000);

// Fetch the tx and check for confirmation and memo.
txInfo = getTransaction(alicedaemon, txInfo.getTxId());
assertFalse(txInfo.getIsPending());
assertEquals(TX_MEMO, txInfo.getMemo());

BtcBalanceInfo alicesBalances = getBtcBalances(alicedaemon);
log.debug("{} Alice's BTC Balances:\n{}",
testName(testInfo),
formatBtcBalanceInfoTbl(alicesBalances));
bisq.core.api.model.BtcBalanceInfo alicesExpectedBalances =
bisq.core.api.model.BtcBalanceInfo.valueOf(700000000,
0,
700000000,
0);
verifyBtcBalances(alicesExpectedBalances, alicesBalances);

BtcBalanceInfo bobsBalances = getBtcBalances(bobdaemon);
log.debug("{} Bob's BTC Balances:\n{}",
testName(testInfo),
formatBtcBalanceInfoTbl(bobsBalances));
// We cannot (?) predict the exact tx size and calculate how much in tx fees were
// deducted from the 5.5 BTC sent to Bob, but we do know Bob should have something
// between 15.49978000 and 15.49978100 BTC.
assertTrue(bobsBalances.getAvailableBalance() >= 1549978000);
assertTrue(bobsBalances.getAvailableBalance() <= 1549978100);
}

@AfterAll
public static void tearDown() {
tearDownScaffold();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public void testBtcWalletFunding(final TestInfo testInfo) {

btcWalletTest.testInitialBtcBalances(testInfo);
btcWalletTest.testFundAlicesBtcWallet(testInfo);
btcWalletTest.testAliceSendBTCToBob(testInfo);
}

@Test
Expand Down