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 8 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
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,6 @@ public void testCreateTransferwiseAccount(TestInfo testInfo) {
String jsonString = getCompletedFormAsJsonString();
TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(alicedaemon, jsonString);
verifyUserPayloadHasPaymentAccountWithId(paymentAccount.getId());
verifyUserPayloadHasPaymentAccountWithId(paymentAccount.getId());
// As per commit 88f26f93241af698ae689bf081205d0f9dc929fa
// Do not autofill all currencies by default but keep all unselected.
// verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount);
Expand Down
Expand Up @@ -33,7 +33,6 @@
import static bisq.apitest.config.BisqAppConfig.alicedaemon;
import static bisq.apitest.config.BisqAppConfig.bobdaemon;
import static bisq.cli.CurrencyFormat.formatSatoshis;
import static bisq.cli.TransactionFormat.format;
import static bisq.core.trade.Trade.Phase.*;
import static bisq.core.trade.Trade.State.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -165,19 +164,4 @@ public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) {
testName(testInfo),
formatSatoshis(currentBalance.getAvailableBalance()));
}

@Test
@Order(5)
public void testGetTradeWithdrawalTx(final TestInfo testInfo) {
var trade = getTrade(bobdaemon, tradeId);
var withdrawalTxId = trade.getWithdrawalTxId();
assertNotNull(withdrawalTxId);

var txInfo = getTransaction(bobdaemon, withdrawalTxId);
assertEquals(WITHDRAWAL_TX_MEMO, txInfo.getMemo());

log.debug("{} Trade withdrawal Tx:\n{}",
testName(testInfo),
format(txInfo));
}
}
1 change: 0 additions & 1 deletion apitest/src/test/java/bisq/apitest/scenario/TradeTest.java
Expand Up @@ -60,6 +60,5 @@ public void testTakeSellBTCOffer(final TestInfo testInfo) {
test.testBobsConfirmPaymentStarted(testInfo);
test.testAlicesConfirmPaymentReceived(testInfo);
test.testBobsBtcWithdrawalToExternalAddress(testInfo);
test.testGetTradeWithdrawalTx(testInfo);
}
}
10 changes: 3 additions & 7 deletions cli/src/main/java/bisq/cli/TradeFormat.java
Expand Up @@ -58,7 +58,6 @@ public static String format(TradeInfo tradeInfo) {
+ COL_HEADER_TRADE_FIAT_RECEIVED + COL_HEADER_DELIMITER
+ COL_HEADER_TRADE_PAYOUT_PUBLISHED + COL_HEADER_DELIMITER
+ COL_HEADER_TRADE_WITHDRAWN + COL_HEADER_DELIMITER
+ (tradeInfo.getIsWithdrawn() ? COL_HEADER_TRADE_WITHDRAWAL_TX_ID + COL_HEADER_DELIMITER : "")
+ "%n";

String counterCurrencyCode = tradeInfo.getOffer().getCounterCurrencyCode();
Expand All @@ -79,8 +78,7 @@ public static String format(TradeInfo tradeInfo) {
+ " %-" + COL_HEADER_TRADE_FIAT_SENT.length() + "s" // lt justify
+ " %-" + COL_HEADER_TRADE_FIAT_RECEIVED.length() + "s" // lt justify
+ " %-" + COL_HEADER_TRADE_PAYOUT_PUBLISHED.length() + "s" // lt justify
+ " %-" + COL_HEADER_TRADE_WITHDRAWN.length() + "s" // lt justify
+ " %-" + COL_HEADER_TRADE_WITHDRAWAL_TX_ID.length() + "s"; // left
+ " %-" + COL_HEADER_TRADE_WITHDRAWN.length() + "s"; // lt justify

return headerLine +
(isTaker
Expand All @@ -100,8 +98,7 @@ private static String formatTradeForMaker(String format, TradeInfo tradeInfo) {
tradeInfo.getIsFiatSent() ? "YES" : "NO",
tradeInfo.getIsFiatReceived() ? "YES" : "NO",
tradeInfo.getIsPayoutPublished() ? "YES" : "NO",
tradeInfo.getIsWithdrawn() ? "YES" : "NO",
tradeInfo.getIsWithdrawn() ? tradeInfo.getWithdrawalTxId() : "");
tradeInfo.getIsWithdrawn() ? "YES" : "NO");
}

private static String formatTradeForTaker(String format, TradeInfo tradeInfo) {
Expand All @@ -117,7 +114,6 @@ private static String formatTradeForTaker(String format, TradeInfo tradeInfo) {
tradeInfo.getIsFiatSent() ? "YES" : "NO",
tradeInfo.getIsFiatReceived() ? "YES" : "NO",
tradeInfo.getIsPayoutPublished() ? "YES" : "NO",
tradeInfo.getIsWithdrawn() ? "YES" : "NO",
tradeInfo.getIsWithdrawn() ? tradeInfo.getWithdrawalTxId() : "");
tradeInfo.getIsWithdrawn() ? "YES" : "NO");
}
}
11 changes: 0 additions & 11 deletions core/src/main/java/bisq/core/api/model/TradeInfo.java
Expand Up @@ -47,7 +47,6 @@ public class TradeInfo implements Payload {
private final String takerFeeTxId;
private final String depositTxId;
private final String payoutTxId;
private final String withdrawalTxId;
private final long tradeAmountAsLong;
private final long tradePrice;
private final String tradingPeerNodeAddress;
Expand All @@ -74,7 +73,6 @@ public TradeInfo(TradeInfoBuilder builder) {
this.takerFeeTxId = builder.takerFeeTxId;
this.depositTxId = builder.depositTxId;
this.payoutTxId = builder.payoutTxId;
this.withdrawalTxId = builder.withdrawalTxId;
this.tradeAmountAsLong = builder.tradeAmountAsLong;
this.tradePrice = builder.tradePrice;
this.tradingPeerNodeAddress = builder.tradingPeerNodeAddress;
Expand Down Expand Up @@ -108,7 +106,6 @@ public static TradeInfo toTradeInfo(Trade trade, String role) {
.withTakerFeeTxId(trade.getTakerFeeTxId())
.withDepositTxId(trade.getDepositTxId())
.withPayoutTxId(trade.getPayoutTxId())
.withWithdrawalTxId(trade.getWithdrawalTxId())
.withTradeAmountAsLong(trade.getTradeAmountAsLong())
.withTradePrice(trade.getTradePrice().getValue())
.withTradingPeerNodeAddress(Objects.requireNonNull(
Expand Down Expand Up @@ -144,7 +141,6 @@ public bisq.proto.grpc.TradeInfo toProtoMessage() {
.setTakerFeeTxId(takerFeeTxId == null ? "" : takerFeeTxId)
.setDepositTxId(depositTxId == null ? "" : depositTxId)
.setPayoutTxId(payoutTxId == null ? "" : payoutTxId)
.setWithdrawalTxId(withdrawalTxId == null ? "" : withdrawalTxId)
.setTradeAmountAsLong(tradeAmountAsLong)
.setTradePrice(tradePrice)
.setTradingPeerNodeAddress(tradingPeerNodeAddress)
Expand Down Expand Up @@ -184,7 +180,6 @@ public static class TradeInfoBuilder {
private String takerFeeTxId;
private String depositTxId;
private String payoutTxId;
private String withdrawalTxId;
private long tradeAmountAsLong;
private long tradePrice;
private String tradingPeerNodeAddress;
Expand Down Expand Up @@ -254,11 +249,6 @@ public TradeInfoBuilder withPayoutTxId(String payoutTxId) {
return this;
}

public TradeInfoBuilder withWithdrawalTxId(String withdrawalTxId) {
this.withdrawalTxId = withdrawalTxId;
return this;
}

public TradeInfoBuilder withTradeAmountAsLong(long tradeAmountAsLong) {
this.tradeAmountAsLong = tradeAmountAsLong;
return this;
Expand Down Expand Up @@ -342,7 +332,6 @@ public String toString() {
", takerFeeTxId='" + takerFeeTxId + '\'' + "\n" +
", depositTxId='" + depositTxId + '\'' + "\n" +
", payoutTxId='" + payoutTxId + '\'' + "\n" +
", withdrawalTxId='" + withdrawalTxId + '\'' + "\n" +
", tradeAmountAsLong='" + tradeAmountAsLong + '\'' + "\n" +
", tradePrice='" + tradePrice + '\'' + "\n" +
", tradingPeerNodeAddress='" + tradingPeerNodeAddress + '\'' + "\n" +
Expand Down
7 changes: 0 additions & 7 deletions core/src/main/java/bisq/core/trade/Trade.java
Expand Up @@ -317,10 +317,6 @@ public static protobuf.Trade.TradePeriodState toProtoMessage(Trade.TradePeriodSt
@Getter
@Setter
private String payoutTxId;
@Nullable
@Getter
@Setter
private String withdrawalTxId;
@Getter
@Setter
private long tradeAmountAsLong;
Expand Down Expand Up @@ -558,7 +554,6 @@ public Message toProtoMessage() {
Optional.ofNullable(takerFeeTxId).ifPresent(builder::setTakerFeeTxId);
Optional.ofNullable(depositTxId).ifPresent(builder::setDepositTxId);
Optional.ofNullable(payoutTxId).ifPresent(builder::setPayoutTxId);
Optional.ofNullable(withdrawalTxId).ifPresent(builder::setWithdrawalTxId);
Optional.ofNullable(tradingPeerNodeAddress).ifPresent(e -> builder.setTradingPeerNodeAddress(tradingPeerNodeAddress.toProtoMessage()));
Optional.ofNullable(contract).ifPresent(e -> builder.setContract(contract.toProtoMessage()));
Optional.ofNullable(contractAsJson).ifPresent(builder::setContractAsJson);
Expand Down Expand Up @@ -592,7 +587,6 @@ public static Trade fromProto(Trade trade, protobuf.Trade proto, CoreProtoResolv
trade.setTakerFeeTxId(ProtoUtil.stringOrNullFromProto(proto.getTakerFeeTxId()));
trade.setDepositTxId(ProtoUtil.stringOrNullFromProto(proto.getDepositTxId()));
trade.setPayoutTxId(ProtoUtil.stringOrNullFromProto(proto.getPayoutTxId()));
trade.setWithdrawalTxId(ProtoUtil.stringOrNullFromProto(proto.getWithdrawalTxId()));
trade.setContract(proto.hasContract() ? Contract.fromProto(proto.getContract(), coreProtoResolver) : null);
trade.setContractAsJson(ProtoUtil.stringOrNullFromProto(proto.getContractAsJson()));
trade.setContractHash(ProtoUtil.byteArrayOrNullFromProto(proto.getContractHash()));
Expand Down Expand Up @@ -1130,7 +1124,6 @@ public String toString() {
",\n takerFeeTxId='" + takerFeeTxId + '\'' +
",\n depositTxId='" + depositTxId + '\'' +
",\n payoutTxId='" + payoutTxId + '\'' +
",\n withdrawalTxId='" + withdrawalTxId + '\'' +
",\n tradeAmountAsLong=" + tradeAmountAsLong +
",\n tradePrice=" + tradePrice +
",\n tradingPeerNodeAddress=" + tradingPeerNodeAddress +
Expand Down
1 change: 0 additions & 1 deletion core/src/main/java/bisq/core/trade/TradeManager.java
Expand Up @@ -497,7 +497,6 @@ public void onSuccess(@javax.annotation.Nullable Transaction transaction) {
onTradeCompleted(trade);
trade.setState(Trade.State.WITHDRAW_COMPLETED);
getTradeProtocol(trade).onWithdrawCompleted();
trade.setWithdrawalTxId(transaction.getTxId().toString());
requestPersistence();
resultHandler.handleResult();
}
Expand Down
27 changes: 13 additions & 14 deletions proto/src/main/proto/grpc.proto
Expand Up @@ -273,20 +273,19 @@ message TradeInfo {
string takerFeeTxId = 9;
string depositTxId = 10;
string payoutTxId = 11;
string withdrawalTxId = 12;
uint64 tradeAmountAsLong = 13;
uint64 tradePrice = 14;
string tradingPeerNodeAddress = 15;
string state = 16;
string phase = 17;
string tradePeriodState = 18;
bool isDepositPublished = 19;
bool isDepositConfirmed = 20;
bool isFiatSent = 21;
bool isFiatReceived = 22;
bool isPayoutPublished = 23;
bool isWithdrawn = 24;
string contractAsJson = 25;
uint64 tradeAmountAsLong = 12;
uint64 tradePrice = 13;
string tradingPeerNodeAddress = 14;
string state = 15;
string phase = 16;
string tradePeriodState = 17;
bool isDepositPublished = 18;
bool isDepositConfirmed = 19;
bool isFiatSent = 20;
bool isFiatReceived = 21;
bool isPayoutPublished = 22;
bool isWithdrawn = 23;
string contractAsJson = 24;
}

///////////////////////////////////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion proto/src/main/proto/pb.proto
Expand Up @@ -1450,7 +1450,6 @@ message Trade {
string counter_currency_extra_data = 37;
string asset_tx_proof_result = 38; // name of AssetTxProofResult enum
string uid = 39;
string withdrawal_tx_id = 40;
}

message BuyerAsMakerTrade {
Expand Down