Make Transactions view display correct types & amounts for staged txs

Ensure 'TransactionsListItem' recognises warning, redirect & claim txs
and displays appropriate details messages for them. Redirect txs are
made to show the same "Refund collateral" details message as delayed
payout txs and don't distinguish between the user's or peer's tx,
whereas warning & claim tx details do distinguish between them.

Also ensure the correct amounts are displayed in the Transaction view,
when watched scripts are present in the wallet, by changing
'WalletService::getValueSent(To|From)MeForTransaction' not to include
watched outputs or inputs in their respective sums.

Ensure claim txs broadcast by the peer are correctly linked to the trade
and display correctly in the Transactions view, by changing
'BisqRiskAnalysis' not to deem txs with a relative lock time as risky,
as that interferes with the v5 trade protocol.

Finally, make the Trade Details window resilient to missing peer's
redirect & warning tx from old trades, which could be cleared out as
sensitive data, and prevent it from incorrectly displaying the claim tx
as the multisig payout tx (and similarly for the Transactions view).

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.
 
 /**
  * <p>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;

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<TransactionOutput> 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);
     }
 
 

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);

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

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;

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 {

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)