add validation and self correct recorded security deposits

Author: woodser

core/src/main/java/haveno/core/trade/Trade.java

@@ -2695,10 +2695,18 @@ private void doPollWallet() {
                 setStateDepositsSeen();
 
                 // set actual security deposits
-                if (getBuyer().getSecurityDeposit().longValueExact() == 0) {
-                    BigInteger buyerSecurityDeposit = hasBuyerAsTakerWithoutDeposit() ? BigInteger.ZERO : ((MoneroTxWallet) getBuyer().getDepositTx()).getIncomingAmount();
-                    BigInteger sellerSecurityDeposit = ((MoneroTxWallet) getSeller().getDepositTx()).getIncomingAmount().subtract(getAmount());
+                if (getBuyer().getDepositTx() != null) {
+                    BigInteger buyerSecurityDeposit = ((MoneroTxWallet) getBuyer().getDepositTx()).getIncomingAmount();
+                    if (!getBuyer().getSecurityDeposit().equals(BigInteger.ZERO) && !getBuyer().getSecurityDeposit().equals(buyerSecurityDeposit)) {
+                        log.warn("Overwriting buyer security deposit for {} {}, old={}, new={}", getClass().getSimpleName(), getShortId(), getBuyer().getSecurityDeposit(), buyerSecurityDeposit);
+                    }
                     getBuyer().setSecurityDeposit(buyerSecurityDeposit);
+                }
+                if (getSeller().getDepositTx() != null) {
+                    BigInteger sellerSecurityDeposit = ((MoneroTxWallet) getSeller().getDepositTx()).getIncomingAmount().subtract(getAmount());
+                    if (!getSeller().getSecurityDeposit().equals(BigInteger.ZERO) && !getSeller().getSecurityDeposit().equals(sellerSecurityDeposit)) {
+                        log.warn("Overwriting seller security deposit for {} {}, old={}, new={}", getClass().getSimpleName(), getShortId(), getSeller().getSecurityDeposit(), sellerSecurityDeposit);
+                    }
                     getSeller().setSecurityDeposit(sellerSecurityDeposit);
                 }
 

core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessDepositRequest.java

@@ -120,9 +120,8 @@ private void processDepositRequest() {
         // verify deposit tx
         boolean isFromBuyerAsTakerWithoutDeposit = isFromBuyer && isFromTaker && trade.hasBuyerAsTakerWithoutDeposit();
         if (!isFromBuyerAsTakerWithoutDeposit) {
-            MoneroTx verifiedTx;
             try {
-                verifiedTx = trade.getXmrWalletService().verifyDepositTx(
+                MoneroTx verifiedTx = trade.getXmrWalletService().verifyDepositTx(
                         offer.getId(),
                         tradeFee,
                         trade.getProcessModel().getTradeFeeAddress(),
@@ -133,15 +132,22 @@ private void processDepositRequest() {
                         request.getDepositTxHex(),
                         request.getDepositTxKey(),
                         null);
+
+                // TODO: it seems a deposit tx had 0 fee once?
+                if (BigInteger.ZERO.equals(verifiedTx.getFee())) {
+                    String errorMessage = "Deposit transaction from " + (isFromTaker ? "taker" : "maker") + " has 0 fee for trade " + trade.getId() + ". This should never happen.";
+                    log.warn(errorMessage + "\n" + verifiedTx);
+                    throw new RuntimeException(errorMessage);
+                }
+
+                // update trade state
+                sender.setSecurityDeposit(sender.getSecurityDeposit().subtract(verifiedTx.getFee())); // subtract mining fee from security deposit
+                sender.setDepositTxFee(verifiedTx.getFee());
+                sender.setDepositTxHex(request.getDepositTxHex());
+                sender.setDepositTxKey(request.getDepositTxKey());
             } catch (Exception e) {
                 throw new RuntimeException("Error processing deposit tx from " + (isFromTaker ? "taker " : "maker ") + sender.getNodeAddress() + ", offerId=" + offer.getId() + ": " + e.getMessage());
             }
-    
-            // update trade state
-            sender.setSecurityDeposit(sender.getSecurityDeposit().subtract(verifiedTx.getFee())); // subtract mining fee from security deposit
-            sender.setDepositTxFee(verifiedTx.getFee());
-            sender.setDepositTxHex(request.getDepositTxHex());
-            sender.setDepositTxKey(request.getDepositTxKey());
         }
 
         // update trade state

core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessReserveTx.java

@@ -67,9 +67,8 @@ protected void run() {
                 BigInteger penaltyFee = HavenoUtils.multiply(isFromMaker ? offer.getAmount() : trade.getAmount(), offer.getPenaltyFeePct());
                 BigInteger tradeFee = isFromMaker ? offer.getMaxMakerFee() : trade.getTakerFee();
                 BigInteger sendAmount =  isFromBuyer ? BigInteger.ZERO : isFromMaker ? offer.getAmount() : trade.getAmount(); // maker reserve tx is for offer amount
-                MoneroTx verifiedTx;
                 try {
-                    verifiedTx = trade.getXmrWalletService().verifyReserveTx(
+                    MoneroTx verifiedTx = trade.getXmrWalletService().verifyReserveTx(
                         offer.getId(),
                         penaltyFee,
                         tradeFee,
@@ -80,16 +79,23 @@ protected void run() {
                         request.getReserveTxHex(),
                         request.getReserveTxKey(),
                         null);
+
+                    // TODO: it seems a deposit tx had 0 fee once?
+                    if (BigInteger.ZERO.equals(verifiedTx.getFee())) {
+                        String errorMessage = "Reserve transaction from " + (isFromMaker ? "maker" : "taker") + " has 0 fee for trade " + trade.getId() + ". This should never happen.";
+                        log.warn(errorMessage + "\n" + verifiedTx);
+                        throw new RuntimeException(errorMessage);
+                    }
+
+                    // save reserve tx to model
+                    sender.setSecurityDeposit(sender.getSecurityDeposit().subtract(verifiedTx.getFee())); // subtract mining fee from security deposit
+                    sender.setReserveTxHash(request.getReserveTxHash());
+                    sender.setReserveTxHex(request.getReserveTxHex());
+                    sender.setReserveTxKey(request.getReserveTxKey());
                 } catch (Exception e) {
                     log.error(ExceptionUtils.getStackTrace(e));
                     throw new RuntimeException("Error processing reserve tx from " + (isFromMaker ? "maker " : "taker ") + processModel.getTempTradePeerNodeAddress() + ", offerId=" + offer.getId() + ": " + e.getMessage());
                 }
-
-                // save reserve tx to model
-                sender.setSecurityDeposit(sender.getSecurityDeposit().subtract(verifiedTx.getFee())); // subtract mining fee from security deposit
-                sender.setReserveTxHash(request.getReserveTxHash());
-                sender.setReserveTxHex(request.getReserveTxHex());
-                sender.setReserveTxKey(request.getReserveTxKey());
             }
 
             // persist trade