diff --git a/core/src/main/java/bisq/core/btc/wallet/BisqRiskAnalysis.java b/core/src/main/java/bisq/core/btc/wallet/BisqRiskAnalysis.java index 8b665c67c9..b5d23fa29d 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BisqRiskAnalysis.java +++ b/core/src/main/java/bisq/core/btc/wallet/BisqRiskAnalysis.java @@ -60,12 +60,14 @@ // Copied from DefaultRiskAnalysis as DefaultRiskAnalysis has mostly private methods and constructor so we cannot // override it. -// The changes to DefaultRiskAnalysis are: removal of the RBF check and accept as standard an OP_RETURN outputs -// with 0 value. +// The changes to DefaultRiskAnalysis are: removal of the RBF check and removal of the relative lock-time check. // For Bisq's use cases RBF is not considered risky. Requiring a confirmation for RBF payments from a user's // external wallet to Bisq would hurt usability. The trade transaction requires anyway a confirmation and we don't see // a use case where a Bisq user accepts unconfirmed payment from untrusted peers and would not wait anyway for at least // one confirmation. +// Relative lock-times are used by claim txs for the v5 trade protocol. It's doubtful that they would realistically +// show up in any other context (maybe forced lightning channel closures spending straight to Bisq) or would ever be +// replaced once broadcast, so we deem them non-risky. /** *

The default risk analysis. Currently, it only is concerned with whether a tx/dependency is non-final or not, and @@ -122,12 +124,13 @@ private Result analyzeIsFinal() { // return Result.NON_FINAL; // } - // Relative time-locked transactions are risky too. We can't check the locks because usually we don't know the - // spent outputs (to know when they were created). - if (tx.hasRelativeLockTime()) { - nonFinal = tx; - return Result.NON_FINAL; - } + // Commented out to accept claim txs for v5 trade protocol. + // // Relative time-locked transactions are risky too. We can't check the locks because usually we don't know the + // // spent outputs (to know when they were created). + // if (tx.hasRelativeLockTime()) { + // nonFinal = tx; + // return Result.NON_FINAL; + // } if (wallet == null) return null; diff --git a/core/src/main/java/bisq/core/btc/wallet/WalletService.java b/core/src/main/java/bisq/core/btc/wallet/WalletService.java index aa498726b0..7de7ec92ab 100644 --- a/core/src/main/java/bisq/core/btc/wallet/WalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/WalletService.java @@ -43,6 +43,7 @@ import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionConfidence; import org.bitcoinj.core.TransactionInput; +import org.bitcoinj.core.TransactionOutPoint; import org.bitcoinj.core.TransactionOutput; import org.bitcoinj.core.TransactionWitness; import org.bitcoinj.core.VerificationException; @@ -65,6 +66,7 @@ import org.bitcoinj.wallet.RedeemData; import org.bitcoinj.wallet.SendRequest; import org.bitcoinj.wallet.Wallet; +import org.bitcoinj.wallet.WalletTransaction; import org.bitcoinj.wallet.listeners.WalletChangeEventListener; import org.bitcoinj.wallet.listeners.WalletCoinsReceivedEventListener; import org.bitcoinj.wallet.listeners.WalletCoinsSentEventListener; @@ -88,6 +90,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.atomic.AtomicReference; @@ -817,11 +820,33 @@ public boolean isTransactionOutputMine(TransactionOutput transactionOutput) { }*/ public Coin getValueSentFromMeForTransaction(Transaction transaction) throws ScriptException { - return transaction.getValueSentFromMe(wallet); + // Does the same thing as transaction.getValueSentFromMe(wallet), except that watched connected + // outputs don't count towards the total, only outputs with pubKeys belonging to the wallet. + long satoshis = transaction.getInputs().stream() + .flatMap(input -> getConnectedOutput(input, WalletTransaction.Pool.UNSPENT) + .or(() -> getConnectedOutput(input, WalletTransaction.Pool.SPENT)) + .or(() -> getConnectedOutput(input, WalletTransaction.Pool.PENDING)) + .filter(o -> o.isMine(wallet)) + .stream()) + .mapToLong(o -> o.getValue().value) + .sum(); + return Coin.valueOf(satoshis); + } + + private Optional getConnectedOutput(TransactionInput input, WalletTransaction.Pool pool) { + TransactionOutPoint outpoint = input.getOutpoint(); + return Optional.ofNullable(wallet.getTransactionPool(pool).get(outpoint.getHash())) + .map(tx -> tx.getOutput(outpoint.getIndex())); } public Coin getValueSentToMeForTransaction(Transaction transaction) throws ScriptException { - return transaction.getValueSentToMe(wallet); + // Does the same thing as transaction.getValueSentToMe(wallet), except that watched outputs + // don't count towards the total, only outputs with pubKeys belonging to the wallet. + long satoshis = transaction.getOutputs().stream() + .filter(o -> o.isMine(wallet)) + .mapToLong(o -> o.getValue().value) + .sum(); + return Coin.valueOf(satoshis); } diff --git a/core/src/main/java/bisq/core/trade/protocol/bisq_v5/tasks/arbitration/CreateSignedClaimTx.java b/core/src/main/java/bisq/core/trade/protocol/bisq_v5/tasks/arbitration/CreateSignedClaimTx.java index 44a1e30f82..1fe2f59bc4 100644 --- a/core/src/main/java/bisq/core/trade/protocol/bisq_v5/tasks/arbitration/CreateSignedClaimTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/bisq_v5/tasks/arbitration/CreateSignedClaimTx.java @@ -54,6 +54,8 @@ protected void run() { TradingPeer tradingPeer = processModel.getTradePeer(); TransactionOutput myWarningTxOutput = processModel.getWarningTx().getOutput(0); + // TODO: At present, the claim tx can be picked up by the payout tx listener and set as the trade payout tx. + // It's not clear we want this, or perhaps we should always consider a claim as the trade payout. AddressEntry addressEntry = processModel.getBtcWalletService().getOrCreateAddressEntry(tradeId, AddressEntry.Context.TRADE_PAYOUT); Address payoutAddress = addressEntry.getAddress(); long miningFee = StagedPayoutTxParameters.getClaimTxMiningFee(feeService.getTxFeePerVbyte().value); diff --git a/core/src/main/resources/i18n/displayStrings.properties b/core/src/main/resources/i18n/displayStrings.properties index 2d4b6cf60f..97b6e61c56 100644 --- a/core/src/main/resources/i18n/displayStrings.properties +++ b/core/src/main/resources/i18n/displayStrings.properties @@ -1194,6 +1194,10 @@ funds.tx.multiSigPayout=Multisig payout funds.tx.disputePayout=Dispute payout funds.tx.disputeLost=Lost dispute case funds.tx.collateralForRefund=Refund collateral +funds.tx.warningTx=Warning tx +funds.tx.peersWarningTx=Peer's warning tx +funds.tx.claimTx=Collateral claimed +funds.tx.peersClaimTx=Collateral claimed by peer funds.tx.timeLockedPayoutTx=Time locked payout tx funds.tx.refund=Refund from arbitration funds.tx.unknown=Unknown reason diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java index 291c349726..1f896d2053 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionAwareTrade.java @@ -149,7 +149,7 @@ boolean isDelayedPayoutTx(String txId) { return isDelayedPayoutOrWarningTx(txId) && !((Trade) tradeModel).hasV5Protocol(); } - private boolean isWarningTx(String txId) { + boolean isWarningTx(String txId) { return isDelayedPayoutOrWarningTx(txId) && ((Trade) tradeModel).hasV5Protocol(); } @@ -171,6 +171,22 @@ private boolean isDelayedPayoutOrWarningTx(Transaction transaction, @Nullable St return firstParent(this::isDepositTx, transaction, txId); } + boolean isRedirectTx(String txId) { + if (isBsqSwapTrade()) { + return false; + } + Transaction transaction = btcWalletService.getTransaction(txId); + return transaction != null && !transaction.hasRelativeLockTime() && isRedirectOrClaimTx(transaction, null); + } + + boolean isClaimTx(String txId) { + if (isBsqSwapTrade()) { + return false; + } + Transaction transaction = btcWalletService.getTransaction(txId); + return transaction != null && transaction.hasRelativeLockTime() && isRedirectOrClaimTx(transaction, null); + } + private boolean isRedirectOrClaimTx(Transaction transaction, @Nullable String txId) { if (transaction.getInputs().size() != 1) { return false; diff --git a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java index 875a1f895b..b1dce72bb3 100644 --- a/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java +++ b/desktop/src/main/java/bisq/desktop/main/funds/transactions/TransactionsListItem.java @@ -208,9 +208,9 @@ else if (txTypeOptional.get().equals(TxType.REIMBURSEMENT_REQUEST)) trade.getDepositTx().getTxId().equals(Sha256Hash.wrap(txId))) { details = Res.get("funds.tx.multiSigDeposit"); } else if (trade.getPayoutTx() != null && - trade.getPayoutTx().getTxId().equals(Sha256Hash.wrap(txId))) { + trade.getPayoutTx().getTxId().equals(Sha256Hash.wrap(txId)) && + !transactionAwareTrade.isClaimTx(txId)) { details = Res.get("funds.tx.multiSigPayout"); - if (amountAsCoin.isZero()) { initialTxConfidenceVisibility = false; } @@ -223,10 +223,10 @@ else if (txTypeOptional.get().equals(TxType.REIMBURSEMENT_REQUEST)) details = Res.get("funds.tx.disputeLost"); initialTxConfidenceVisibility = false; } - } else if (disputeState == Trade.DisputeState.REFUND_REQUEST_CLOSED || + } else if ((disputeState == Trade.DisputeState.REFUND_REQUEST_CLOSED || disputeState == Trade.DisputeState.REFUND_REQUESTED || - disputeState == Trade.DisputeState.REFUND_REQUEST_STARTED_BY_PEER) { - if (valueSentToMe.isPositive()) { + disputeState == Trade.DisputeState.REFUND_REQUEST_STARTED_BY_PEER) && !trade.hasV5Protocol()) { + if (valueSentToMe.isPositive()) { // FIXME: This will give a false positive if user is a BM. details = Res.get("funds.tx.refund"); } else { // We have spent the deposit tx outputs to the Bisq donation address to enable @@ -239,7 +239,17 @@ else if (txTypeOptional.get().equals(TxType.REIMBURSEMENT_REQUEST)) initialTxConfidenceVisibility = false; } } else { - if (transactionAwareTrade.isDelayedPayoutTx(txId)) { + if (transactionAwareTrade.isWarningTx(txId)) { + details = valueSentToMe.isPositive() + ? Res.get("funds.tx.warningTx") + : Res.get("funds.tx.peersWarningTx"); + } else if (transactionAwareTrade.isRedirectTx(txId)) { + details = Res.get("funds.tx.collateralForRefund"); + } else if (transactionAwareTrade.isClaimTx(txId)) { + details = valueSentToMe.isPositive() + ? Res.get("funds.tx.claimTx") + : Res.get("funds.tx.peersClaimTx"); + } else if (transactionAwareTrade.isDelayedPayoutTx(txId)) { details = Res.get("funds.tx.timeLockedPayoutTx"); initialTxConfidenceVisibility = false; } else { diff --git a/desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java b/desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java index 12c102d1ae..a77ed222c5 100644 --- a/desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java +++ b/desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java @@ -288,17 +288,27 @@ private void addContent() { depositTxId, depositTxIdFromTx, depositTx); } - 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) { + Transaction claimTx = trade.getClaimTx(btcWalletService); + if (trade.hasV5Protocol()) { // 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); + Transaction buyersWarningTx = trade.getBuyersWarningTx(btcWalletService); + Transaction sellersWarningTx = trade.getSellersWarningTx(btcWalletService); + Transaction buyersRedirectTx = trade.getBuyersRedirectTx(btcWalletService); + Transaction sellersRedirectTx = trade.getSellersRedirectTx(btcWalletService); + // Because the peer's warning & redirect txs could have been cleared as sensitive data for a failed or + // closed trade (as they contain the peer's fee bump address), we need to check for null here. + if (buyersWarningTx != null) { + addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.buyersWarningTxId"), buyersWarningTx.getTxId().toString()); + } + if (sellersWarningTx != null) { + addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.sellersWarningTxId"), sellersWarningTx.getTxId().toString()); + } + if (sellersRedirectTx != null) { + addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.sellersRedirectTxId"), sellersRedirectTx.getTxId().toString()); + } + if (buyersRedirectTx != null) { + addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.buyersRedirectTxId"), buyersRedirectTx.getTxId().toString()); + } if (claimTx != null) { addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.claimTxId"), claimTx.getTxId().toString()); } @@ -309,7 +319,7 @@ private void addContent() { addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.delayedPayoutTxId"), delayedPayoutTxId); } - if (trade.getPayoutTx() != null) + if (trade.getPayoutTx() != null && !trade.getPayoutTx().equals(claimTx)) addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.payoutTxId"), trade.getPayoutTx().getTxId().toString()); if (showDisputedTx)