Skip to content

Commit

Permalink
Fix UI+log errors/warnings in happy path of v5 trade protocol
Browse files Browse the repository at this point in the history 
Add checks that we're not running the v5 protocol, everywhere a missing
delayed payout tx would cause errors or warnings to appear in either the
logs or the Pending Trades or Trade Details views, for a v5 trade that
completes normally.

Also add both the buyer's & seller's redirect & warning txs to the Trade
Details view, in place of the missing DPT, as well as the claim tx if
it's present. (The latter is created & signed at the point of use.) Add
suitable 'get*(BtcWalletService)' methods to 'Trade' for that purpose.
  • Loading branch information
@stejbac
stejbac committed Jul 24, 2024
1 parent a79d1c3 commit f94ad35
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 18 deletions.
5 changes: 5 additions & 0 deletions core/src/main/java/bisq/core/trade/bisq_v1/TradeDataValidation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ public static void validateDelayedPayoutTx(Trade trade,
@Nullable Consumer<String> addressConsumer)
throws DisputeValidation.AddressException, MissingTxException,
InvalidTxException, InvalidLockTimeException, InvalidAmountException {
// No delayedPayoutTx to validate if v5 protocol
if (trade.hasV5Protocol()) {
return;
}

String errorMsg;
if (delayedPayoutTx == null) {
errorMsg = "DelayedPayoutTx must not be null";
Expand Down
29 changes: 29 additions & 0 deletions core/src/main/java/bisq/core/trade/model/bisq_v1/BuyerTrade.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import bisq.network.p2p.NodeAddress;

import org.bitcoinj.core.Coin;
import org.bitcoinj.core.Transaction;

import lombok.extern.slf4j.Slf4j;

Expand Down Expand Up @@ -93,4 +94,32 @@ public Coin getPayoutAmount() {
public boolean confirmPermitted() {
return !getDisputeState().isArbitrated();
}

@Nullable
@Override
public Transaction getBuyersWarningTx(BtcWalletService btcWalletService) {
byte[] finalizedWarningTx = getProcessModel().getFinalizedWarningTx();
return finalizedWarningTx != null ? btcWalletService.getTxFromSerializedTx(finalizedWarningTx) : null;
}

@Nullable
@Override
public Transaction getSellersWarningTx(BtcWalletService btcWalletService) {
byte[] finalizedWarningTx = getProcessModel().getTradePeer().getFinalizedWarningTx();
return finalizedWarningTx != null ? btcWalletService.getTxFromSerializedTx(finalizedWarningTx) : null;
}

@Nullable
@Override
public Transaction getBuyersRedirectTx(BtcWalletService btcWalletService) {
byte[] finalizedRedirectTx = getProcessModel().getFinalizedRedirectTx();
return finalizedRedirectTx != null ? btcWalletService.getTxFromSerializedTx(finalizedRedirectTx) : null;
}

@Nullable
@Override
public Transaction getSellersRedirectTx(BtcWalletService btcWalletService) {
byte[] finalizedRedirectTx = getProcessModel().getTradePeer().getFinalizedRedirectTx();
return finalizedRedirectTx != null ? btcWalletService.getTxFromSerializedTx(finalizedRedirectTx) : null;
}
}
29 changes: 29 additions & 0 deletions core/src/main/java/bisq/core/trade/model/bisq_v1/SellerTrade.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import bisq.network.p2p.NodeAddress;

import org.bitcoinj.core.Coin;
import org.bitcoinj.core.Transaction;

import lombok.extern.slf4j.Slf4j;

Expand Down Expand Up @@ -111,5 +112,33 @@ public boolean confirmPermitted() {
return false;
}
}

@Nullable
@Override
public Transaction getBuyersWarningTx(BtcWalletService btcWalletService) {
byte[] finalizedWarningTx = getProcessModel().getTradePeer().getFinalizedWarningTx();
return finalizedWarningTx != null ? btcWalletService.getTxFromSerializedTx(finalizedWarningTx) : null;
}

@Nullable
@Override
public Transaction getSellersWarningTx(BtcWalletService btcWalletService) {
byte[] finalizedWarningTx = getProcessModel().getFinalizedWarningTx();
return finalizedWarningTx != null ? btcWalletService.getTxFromSerializedTx(finalizedWarningTx) : null;
}

@Nullable
@Override
public Transaction getBuyersRedirectTx(BtcWalletService btcWalletService) {
byte[] finalizedRedirectTx = getProcessModel().getTradePeer().getFinalizedRedirectTx();
return finalizedRedirectTx != null ? btcWalletService.getTxFromSerializedTx(finalizedRedirectTx) : null;
}

@Nullable
@Override
public Transaction getSellersRedirectTx(BtcWalletService btcWalletService) {
byte[] finalizedRedirectTx = getProcessModel().getFinalizedRedirectTx();
return finalizedRedirectTx != null ? btcWalletService.getTxFromSerializedTx(finalizedRedirectTx) : null;
}
}

42 changes: 35 additions & 7 deletions core/src/main/java/bisq/core/trade/model/bisq_v1/Trade.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,10 @@ public static protobuf.Trade.TradePeriodState toProtoMessage(Trade.TradePeriodSt
private final long tradeTxFeeAsLong;
@Getter
private final long takerFeeAsLong;
@Getter
private final ProcessModel processModel;

// Mutable
@Getter
private ProcessModel processModel;
@Nullable
@Getter
@Setter
Expand Down Expand Up @@ -478,7 +478,6 @@ protected Trade(Offer offer,


// taker
@SuppressWarnings("NullableProblems")
protected Trade(Offer offer,
Coin amount,
Coin txFee,
Expand Down Expand Up @@ -670,7 +669,7 @@ public Transaction getDelayedPayoutTx() {
// If called from a not initialized trade (or a closed or failed trade)
// we need to pass the btcWalletService
@Nullable
public Transaction getDelayedPayoutTx(BtcWalletService btcWalletService) {
public Transaction getDelayedPayoutTx(@Nullable BtcWalletService btcWalletService) {
if (delayedPayoutTx == null) {
if (btcWalletService == null) {
log.warn("btcWalletService is null. You might call that method before the tradeManager has " +
Expand All @@ -679,7 +678,9 @@ public Transaction getDelayedPayoutTx(BtcWalletService btcWalletService) {
}

if (delayedPayoutTxBytes == null) {
log.warn("delayedPayoutTxBytes are null");
if (!hasV5Protocol()) {
log.warn("delayedPayoutTxBytes are null");
}
return null;
}

Expand All @@ -688,6 +689,24 @@ public Transaction getDelayedPayoutTx(BtcWalletService btcWalletService) {
return delayedPayoutTx;
}

@Nullable
public abstract Transaction getBuyersWarningTx(BtcWalletService btcWalletService);

@Nullable
public abstract Transaction getSellersWarningTx(BtcWalletService btcWalletService);

@Nullable
public abstract Transaction getBuyersRedirectTx(BtcWalletService btcWalletService);

@Nullable
public abstract Transaction getSellersRedirectTx(BtcWalletService btcWalletService);

@Nullable
public Transaction getClaimTx(BtcWalletService btcWalletService) {
byte[] signedClaimTx = processModel.getSignedClaimTx();
return signedClaimTx != null ? btcWalletService.getTxFromSerializedTx(signedClaimTx) : null;
}

public void addAndPersistChatMessage(ChatMessage chatMessage) {
if (!chatMessages.contains(chatMessage)) {
chatMessages.add(chatMessage);
Expand Down Expand Up @@ -722,7 +741,7 @@ public void maybeClearSensitiveData() {
change += "processModel;";
}
if (contractAsJson != null) {
String edited = contract.sanitizeContractAsJson(contractAsJson);
String edited = Contract.sanitizeContractAsJson(contractAsJson);
if (!edited.equals(contractAsJson)) {
contractAsJson = edited;
change += "contractAsJson;";
Expand Down Expand Up @@ -1059,7 +1078,10 @@ public boolean isTxChainInvalid() {
getTakerFeeTxId() == null ||
getDepositTxId() == null ||
getDepositTx() == null ||
getDelayedPayoutTxBytes() == null;
getDelayedPayoutTxBytes() == null && (processModel.getFinalizedWarningTx() == null ||
processModel.getFinalizedRedirectTx() == null ||
processModel.getTradePeer().getFinalizedWarningTx() == null ||
processModel.getTradePeer().getFinalizedRedirectTx() == null);
}

public byte[] getArbitratorBtcPubKey() {
Expand All @@ -1085,6 +1107,11 @@ public boolean isUsingLegacyBurningMan() {
return processModel.getBurningManSelectionHeight() == 0;
}

public boolean hasV5Protocol() {
// TODO: We should try to be able to correctly determine earlier in the protocol whether it is v5.
return processModel.getFinalizedWarningTx() != null;
}

/**
* @return Fee rate per Vbyte based on tradeTxFeeAsLong (deposit tx) and the max. expected size of the deposit tx.
*/
Expand Down Expand Up @@ -1182,6 +1209,7 @@ public String toString() {
",\n errorMessage='" + errorMessage + '\'' +
",\n counterCurrencyTxId='" + counterCurrencyTxId + '\'' +
",\n counterCurrencyExtraData='" + counterCurrencyExtraData + '\'' +
",\n sellerConfirmedPaymentReceiptDate=" + new Date(sellerConfirmedPaymentReceiptDate) +
",\n assetTxProofResult='" + assetTxProofResult + '\'' +
",\n chatMessages=" + chatMessages +
",\n tradeTxFee=" + tradeTxFee +
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/resources/i18n/displayStrings.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,11 @@ shared.arbitrator=Arbitrator
shared.refundAgent=Arbitrator
shared.refundAgentForSupportStaff=Refund agent
shared.delayedPayoutTxId=Delayed payout transaction ID
shared.buyersWarningTxId=BTC buyer's warning transaction ID
shared.sellersWarningTxId=BTC seller's warning transaction ID
shared.buyersRedirectTxId=BTC buyer's redirection transaction ID
shared.sellersRedirectTxId=BTC seller's redirection transaction ID
shared.claimTxId=Claim transaction ID
shared.delayedPayoutTxReceiverAddress=Delayed payout transaction sent to
shared.unconfirmedTransactionsLimitReached=You have too many unconfirmed transactions at the moment. Please try again later.
shared.numItemsLabel=Number of entries: {0}
Expand Down
23 changes: 20 additions & 3 deletions desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,26 @@ private void addContent() {
depositTxId, depositTxIdFromTx, depositTx);
}

Transaction delayedPayoutTx = trade.getDelayedPayoutTx(btcWalletService);
String delayedPayoutTxString = delayedPayoutTx != null ? delayedPayoutTx.getTxId().toString() : null;
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.delayedPayoutTxId"), delayedPayoutTxString);
Transaction buyersWarningTx = trade.getBuyersWarningTx(btcWalletService);
Transaction sellersWarningTx = trade.getSellersWarningTx(btcWalletService);
Transaction buyersRedirectTx = trade.getBuyersRedirectTx(btcWalletService);
Transaction sellersRedirectTx = trade.getSellersRedirectTx(btcWalletService);
if (buyersWarningTx != null && sellersWarningTx != null && buyersRedirectTx != null && sellersRedirectTx != null) {
// v5 trade protocol
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.buyersWarningTxId"), buyersWarningTx.getTxId().toString());
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.sellersWarningTxId"), sellersWarningTx.getTxId().toString());
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.sellersRedirectTxId"), sellersRedirectTx.getTxId().toString());
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.buyersRedirectTxId"), buyersRedirectTx.getTxId().toString());
Transaction claimTx = trade.getClaimTx(btcWalletService);
if (claimTx != null) {
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.claimTxId"), claimTx.getTxId().toString());
}
} else {
// v4 trade protocol
Transaction delayedPayoutTx = trade.getDelayedPayoutTx(btcWalletService);
String delayedPayoutTxId = delayedPayoutTx != null ? delayedPayoutTx.getTxId().toString() : null;
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.delayedPayoutTxId"), delayedPayoutTxId);
}

if (trade.getPayoutTx() != null)
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.payoutTxId"),
Expand Down
14 changes: 6 additions & 8 deletions desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import bisq.core.alert.PrivateNotificationManager;
import bisq.core.locale.Res;
import bisq.core.offer.bisq_v1.OfferPayload;
import bisq.core.provider.mempool.FeeValidationStatus;
import bisq.core.support.dispute.mediation.MediationResultState;
import bisq.core.support.messages.ChatMessage;
import bisq.core.support.traderchat.TradeChatSession;
Expand Down Expand Up @@ -131,7 +130,6 @@ public interface ChatCallback {
@FXML
TableColumn<PendingTradesListItem, PendingTradesListItem> priceColumn, volumeColumn, amountColumn, avatarColumn,
marketColumn, roleColumn, paymentMethodColumn, tradeIdColumn, dateColumn, chatColumn, moveTradeToFailedColumn;
private FilteredList<PendingTradesListItem> filteredList;
private SortedList<PendingTradesListItem> sortedList;
private TradeSubView selectedSubView;
private EventHandler<KeyEvent> keyEventEventHandler;
Expand Down Expand Up @@ -275,7 +273,7 @@ public void initialize() {
@Override
protected void activate() {
ObservableList<PendingTradesListItem> list = model.dataModel.list;
filteredList = new FilteredList<>(list);
FilteredList<PendingTradesListItem> filteredList = new FilteredList<>(list);
sortedList = new SortedList<>(filteredList);
sortedList.comparatorProperty().bind(tableView.comparatorProperty());
tableView.setItems(sortedList);
Expand Down Expand Up @@ -387,7 +385,7 @@ private void onMoveInvalidTradeToFailedTrades(Trade trade) {

private void onShowInfoForInvalidTrade(Trade trade) {
new Popup().width(900).attention(Res.get("portfolio.pending.failedTrade.info.popup",
getInvalidTradeDetails(trade)))
getInvalidTradeDetails(trade)))
.show();
}

Expand All @@ -411,7 +409,7 @@ private String getInvalidTradeDetails(Trade trade) {
return Res.get("portfolio.pending.failedTrade.missingDepositTx");
}

if (trade.getDelayedPayoutTx() == null) {
if (!trade.hasV5Protocol() && trade.getDelayedPayoutTx() == null) {
return isMyRoleBuyer ?
Res.get("portfolio.pending.failedTrade.buyer.existingDepositTxButMissingDelayedPayoutTx") :
Res.get("portfolio.pending.failedTrade.seller.existingDepositTxButMissingDelayedPayoutTx");
Expand Down Expand Up @@ -727,11 +725,11 @@ public void updateItem(final PendingTradesListItem item, boolean empty) {
try {
String volume = VolumeUtil.formatVolumeWithCode(item.getTrade().getVolume());
setGraphic(new AutoTooltipLabel(volume));
} catch (Throwable ignore) {
log.debug(ignore.toString()); // Stupidity to make Codacy happy
} catch (Throwable ignore) { //NOPMD
}
} else
} else {
setGraphic(null);
}
}
};
}
Expand Down

0 comments on commit f94ad35

Please sign in to comment.