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.

Sign up for GitHub

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

Avoid NPE due to null fee while wrapping tx in TxInfo #5302

Merged
merged 10 commits into from
Mar 19, 2021
Merged

Avoid NPE due to null fee while wrapping tx in TxInfo #5302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5c097d3
Adjust protos for takeoffer error handing
ghubstan Mar 10, 2021
067a5c7
Inject CoreContext into OpenOfferManager, adjust test
ghubstan Mar 10, 2021
d0590d9
Adjust grpc & core services to new takeoffer error handling
ghubstan Mar 10, 2021
697cf66
Convert AvailabilityResult to error msg when takeoffer fails
ghubstan Mar 10, 2021
d4d989d
Add missing grpc exception catch blocks
ghubstan Mar 10, 2021
48ecb5b
Derive AvailabilityResult from Task errorMessage
ghubstan Mar 10, 2021
4a68773
Merge branch 'master' into 01-fix-takeoffer-err-handling
ghubstan Mar 14, 2021
64d4c31
Avoid NPE due to null fee while wrapping tx in TxInfo
ghubstan Mar 14, 2021
1d56d79
Refactor takeoffer reply proto & ErrorMessageHandler
ghubstan Mar 15, 2021
98c6a8e
Merge branch '01-fix-takeoffer-err-handling' into 02-modify-txinfo
ghubstan Mar 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 35 additions & 27 deletions apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,41 +132,49 @@ public void testAlicesConfirmPaymentStarted(final TestInfo testInfo) {
@Test
@Order(3)
public void testBobsConfirmPaymentReceived(final TestInfo testInfo) {
var trade = bobClient.getTrade(tradeId);
bobClient.confirmPaymentReceived(trade.getTradeId());
sleep(3000);

trade = bobClient.getTrade(tradeId);
// Note: offer.state == available
assertEquals(AVAILABLE.name(), trade.getOffer().getState());
EXPECTED_PROTOCOL_STATUS.setState(SELLER_SAW_ARRIVED_PAYOUT_TX_PUBLISHED_MSG)
.setPhase(PAYOUT_PUBLISHED)
.setPayoutPublished(true)
.setFiatReceived(true);
verifyExpectedProtocolStatus(trade);
logTrade(log, testInfo, "Bob's view after confirming fiat payment received", trade);
try {
var trade = bobClient.getTrade(tradeId);
bobClient.confirmPaymentReceived(trade.getTradeId());
sleep(3000);

trade = bobClient.getTrade(tradeId);
// Note: offer.state == available
assertEquals(AVAILABLE.name(), trade.getOffer().getState());
EXPECTED_PROTOCOL_STATUS.setState(SELLER_SAW_ARRIVED_PAYOUT_TX_PUBLISHED_MSG)
.setPhase(PAYOUT_PUBLISHED)
.setPayoutPublished(true)
.setFiatReceived(true);
verifyExpectedProtocolStatus(trade);
logTrade(log, testInfo, "Bob's view after confirming fiat payment received", trade);
} catch (StatusRuntimeException e) {
fail(e);
}
}

@Test
@Order(4)
public void testAlicesKeepFunds(final TestInfo testInfo) {
genBtcBlocksThenWait(1, 1000);
try {
genBtcBlocksThenWait(1, 1000);

var trade = aliceClient.getTrade(tradeId);
logTrade(log, testInfo, "Alice's view before keeping funds", trade);
var trade = aliceClient.getTrade(tradeId);
logTrade(log, testInfo, "Alice's view before keeping funds", trade);

aliceClient.keepFunds(tradeId);
aliceClient.keepFunds(tradeId);

genBtcBlocksThenWait(1, 1000);
genBtcBlocksThenWait(1, 1000);

trade = aliceClient.getTrade(tradeId);
EXPECTED_PROTOCOL_STATUS.setState(BUYER_RECEIVED_PAYOUT_TX_PUBLISHED_MSG)
.setPhase(PAYOUT_PUBLISHED);
verifyExpectedProtocolStatus(trade);
logTrade(log, testInfo, "Alice's view after keeping funds", trade);
BtcBalanceInfo currentBalance = aliceClient.getBtcBalances();
log.debug("{} Alice's current available balance: {} BTC",
testName(testInfo),
formatSatoshis(currentBalance.getAvailableBalance()));
trade = aliceClient.getTrade(tradeId);
EXPECTED_PROTOCOL_STATUS.setState(BUYER_RECEIVED_PAYOUT_TX_PUBLISHED_MSG)
.setPhase(PAYOUT_PUBLISHED);
verifyExpectedProtocolStatus(trade);
logTrade(log, testInfo, "Alice's view after keeping funds", trade);
BtcBalanceInfo currentBalance = aliceClient.getBtcBalances();
log.debug("{} Alice's current available balance: {} BTC",
testName(testInfo),
formatSatoshis(currentBalance.getAvailableBalance()));
} catch (StatusRuntimeException e) {
fail(e);
}
}
}
72 changes: 40 additions & 32 deletions apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,42 +134,50 @@ public void testBobsConfirmPaymentStarted(final TestInfo testInfo) {
@Test
@Order(3)
public void testAlicesConfirmPaymentReceived(final TestInfo testInfo) {
var trade = aliceClient.getTrade(tradeId);
aliceClient.confirmPaymentReceived(trade.getTradeId());
sleep(3000);

trade = aliceClient.getTrade(tradeId);
assertEquals(OFFER_FEE_PAID.name(), trade.getOffer().getState());
EXPECTED_PROTOCOL_STATUS.setState(SELLER_SAW_ARRIVED_PAYOUT_TX_PUBLISHED_MSG)
.setPhase(PAYOUT_PUBLISHED)
.setPayoutPublished(true)
.setFiatReceived(true);
verifyExpectedProtocolStatus(trade);
logTrade(log, testInfo, "Alice's view after confirming fiat payment received", trade);
try {
var trade = aliceClient.getTrade(tradeId);
aliceClient.confirmPaymentReceived(trade.getTradeId());
sleep(3000);

trade = aliceClient.getTrade(tradeId);
assertEquals(OFFER_FEE_PAID.name(), trade.getOffer().getState());
EXPECTED_PROTOCOL_STATUS.setState(SELLER_SAW_ARRIVED_PAYOUT_TX_PUBLISHED_MSG)
.setPhase(PAYOUT_PUBLISHED)
.setPayoutPublished(true)
.setFiatReceived(true);
verifyExpectedProtocolStatus(trade);
logTrade(log, testInfo, "Alice's view after confirming fiat payment received", trade);
} catch (StatusRuntimeException e) {
fail(e);
}
}

@Test
@Order(4)
public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) {
genBtcBlocksThenWait(1, 1000);

var trade = bobClient.getTrade(tradeId);
logTrade(log, testInfo, "Bob's view before withdrawing funds to external wallet", trade);

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

genBtcBlocksThenWait(1, 1000);

trade = bobClient.getTrade(tradeId);
EXPECTED_PROTOCOL_STATUS.setState(WITHDRAW_COMPLETED)
.setPhase(WITHDRAWN)
.setWithdrawn(true);
verifyExpectedProtocolStatus(trade);
logTrade(log, testInfo, "Bob's view after withdrawing funds to external wallet", trade);
BtcBalanceInfo currentBalance = bobClient.getBtcBalances();
log.debug("{} Bob's current available balance: {} BTC",
testName(testInfo),
formatSatoshis(currentBalance.getAvailableBalance()));
try {
genBtcBlocksThenWait(1, 1000);

var trade = bobClient.getTrade(tradeId);
logTrade(log, testInfo, "Bob's view before withdrawing funds to external wallet", trade);

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

genBtcBlocksThenWait(1, 1000);

trade = bobClient.getTrade(tradeId);
EXPECTED_PROTOCOL_STATUS.setState(WITHDRAW_COMPLETED)
.setPhase(WITHDRAWN)
.setWithdrawn(true);
verifyExpectedProtocolStatus(trade);
logTrade(log, testInfo, "Bob's view after withdrawing funds to external wallet", trade);
BtcBalanceInfo currentBalance = bobClient.getBtcBalances();
log.debug("{} Bob's current available balance: {} BTC",
testName(testInfo),
formatSatoshis(currentBalance.getAvailableBalance()));
} catch (StatusRuntimeException e) {
fail(e);
}
}
}
55 changes: 34 additions & 21 deletions cli/src/main/java/bisq/cli/GrpcClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import bisq.proto.grpc.SetTxFeeRatePreferenceRequest;
import bisq.proto.grpc.SetWalletPasswordRequest;
import bisq.proto.grpc.StopRequest;
import bisq.proto.grpc.TakeOfferReply;
import bisq.proto.grpc.TakeOfferRequest;
import bisq.proto.grpc.TradeInfo;
import bisq.proto.grpc.TxFeeRateInfo;
Expand All @@ -67,9 +68,13 @@
import java.util.List;
import java.util.stream.Collectors;

import lombok.extern.slf4j.Slf4j;

import static java.util.Comparator.comparing;


@SuppressWarnings("ResultOfMethodCallIgnored")
@Slf4j
public final class GrpcClient {

private final GrpcStubs grpcStubs;
Expand Down Expand Up @@ -181,23 +186,23 @@ public TxInfo getTransaction(String txId) {
}

public OfferInfo createFixedPricedOffer(String direction,
String currencyCode,
long amount,
long minAmount,
String fixedPrice,
double securityDeposit,
String paymentAcctId,
String makerFeeCurrencyCode) {
return createOffer(direction,
currencyCode,
amount,
minAmount,
false,
fixedPrice,
0.00,
securityDeposit,
paymentAcctId,
makerFeeCurrencyCode);
String currencyCode,
long amount,
long minAmount,
String fixedPrice,
double securityDeposit,
String paymentAcctId,
String makerFeeCurrencyCode) {
return createOffer(direction,
currencyCode,
amount,
minAmount,
false,
fixedPrice,
0.00,
securityDeposit,
paymentAcctId,
makerFeeCurrencyCode);
}

public OfferInfo createMarketBasedPricedOffer(String direction,
Expand Down Expand Up @@ -289,8 +294,8 @@ public List<OfferInfo> getMyOffers(String direction, String currencyCode) {
}

public List<OfferInfo> getMyOffersSortedByDate(String direction, String currencyCode) {
var offers = getMyOffers(direction, currencyCode);
return offers.isEmpty() ? offers : sortOffersByDate(offers);
var offers = getMyOffers(direction, currencyCode);
return offers.isEmpty() ? offers : sortOffersByDate(offers);
}

public OfferInfo getMostRecentOffer(String direction, String currencyCode) {
Expand All @@ -305,13 +310,21 @@ private List<OfferInfo> sortOffersByDate(List<OfferInfo> offerInfoList) {
.collect(Collectors.toList());
}

public TradeInfo takeOffer(String offerId, String paymentAccountId, String takerFeeCurrencyCode) {
public TakeOfferReply getTakeOfferReply(String offerId, String paymentAccountId, String takerFeeCurrencyCode) {
var request = TakeOfferRequest.newBuilder()
.setOfferId(offerId)
.setPaymentAccountId(paymentAccountId)
.setTakerFeeCurrencyCode(takerFeeCurrencyCode)
.build();
return grpcStubs.tradesService.takeOffer(request).getTrade();
return grpcStubs.tradesService.takeOffer(request);
}

public TradeInfo takeOffer(String offerId, String paymentAccountId, String takerFeeCurrencyCode) {
var reply = getTakeOfferReply(offerId, paymentAccountId, takerFeeCurrencyCode);
if (reply.hasTrade())
return reply.getTrade();
else
throw new IllegalStateException(reply.getAvailabilityResultDescription());
}

public TradeInfo getTrade(String tradeId) {
Expand Down
7 changes: 5 additions & 2 deletions core/src/main/java/bisq/core/api/CoreApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import bisq.common.app.Version;
import bisq.common.config.Config;
import bisq.common.handlers.ErrorMessageHandler;
import bisq.common.handlers.ResultHandler;

import org.bitcoinj.core.Coin;
Expand Down Expand Up @@ -224,12 +225,14 @@ public void getMarketPrice(String currencyCode, Consumer<Double> resultHandler)
public void takeOffer(String offerId,
String paymentAccountId,
String takerFeeCurrencyCode,
Consumer<Trade> resultHandler) {
Consumer<Trade> resultHandler,
ErrorMessageHandler errorMessageHandler) {
Offer offer = coreOffersService.getOffer(offerId);
coreTradesService.takeOffer(offer,
paymentAccountId,
takerFeeCurrencyCode,
resultHandler);
resultHandler,
errorMessageHandler);
}

public void confirmPaymentStarted(String tradeId) {
Expand Down
10 changes: 5 additions & 5 deletions core/src/main/java/bisq/core/api/CoreTradesService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import bisq.core.user.User;
import bisq.core.util.validation.BtcAddressValidator;

import bisq.common.handlers.ErrorMessageHandler;

import org.bitcoinj.core.Coin;

import javax.inject.Inject;
Expand Down Expand Up @@ -86,7 +88,8 @@ public CoreTradesService(CoreContext coreContext,
void takeOffer(Offer offer,
String paymentAccountId,
String takerFeeCurrencyCode,
Consumer<Trade> resultHandler) {
Consumer<Trade> resultHandler,
ErrorMessageHandler errorMessageHandler) {
coreWalletsService.verifyWalletsAreAvailable();
coreWalletsService.verifyEncryptedWalletIsUnlocked();

Expand Down Expand Up @@ -114,10 +117,7 @@ void takeOffer(Offer offer,
useSavingsWallet,
coreContext.isApiUser(),
resultHandler::accept,
errorMessage -> {
log.error(errorMessage);
throw new IllegalStateException(errorMessage);
}
errorMessageHandler
);
}

Expand Down
Loading