seller signs payout tx unless illegal state, avoid recreating payout tx

core/src/main/java/haveno/core/support/dispute/DisputeManager.java

@@ -901,8 +901,7 @@ public abstract class DisputeManager<T extends DisputeList<Dispute>> extends Sup
                 if (updateState) {
                     trade.getProcessModel().setUnsignedPayoutTx(payoutTx);
                     trade.getSelf().setUpdatedMultisigHex(trade.getWallet().exportMultisigHex());
-                    trade.setPayoutTx(payoutTx);
+                    trade.updatePayout(payoutTx);
-                    trade.setPayoutTxHex(payoutTx.getTxSet().getMultisigTxHex());
                     if (trade.getBuyer().getUpdatedMultisigHex() != null && trade.getBuyer().getUnsignedPayoutTxHex() == null) trade.getBuyer().setUnsignedPayoutTxHex(payoutTx.getTxSet().getMultisigTxHex());
                     if (trade.getSeller().getUpdatedMultisigHex() != null && trade.getSeller().getUnsignedPayoutTxHex() == null) trade.getSeller().setUnsignedPayoutTxHex(payoutTx.getTxSet().getMultisigTxHex());
                 }

core/src/main/java/haveno/core/support/dispute/arbitration/ArbitrationManager.java

@@ -62,6 +62,7 @@ import haveno.core.support.dispute.messages.DisputeClosedMessage;
 import haveno.core.support.dispute.messages.DisputeOpenedMessage;
 import haveno.core.support.messages.ChatMessage;
 import haveno.core.support.messages.SupportMessage;
+import haveno.core.trade.BuyerTrade;
 import haveno.core.trade.ClosedTradableManager;
 import haveno.core.trade.Contract;
 import haveno.core.trade.HavenoUtils;
@@ -86,11 +87,9 @@ import monero.wallet.model.MoneroTxWallet;
 import java.io.IOException;
 import java.math.BigInteger;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -433,19 +432,16 @@ public final class ArbitrationManager extends DisputeManager<ArbitrationDisputeL
         // check daemon connection
         trade.verifyDaemonConnection();
 
-        // determine if we already signed dispute payout tx
+        // adapt from 1.0.6 to 1.0.7 which changes field usage
-        // TODO: better way, such as by saving signed dispute payout tx hex in designated field instead of shared payoutTxHex field?
+        // TODO: remove after future updates to allow old trades to clear
-        Set<String> nonSignedDisputePayoutTxHexes = new HashSet<String>();
+        if (trade.getPayoutTxHex() != null && trade.getBuyer().getPaymentSentMessage() != null && trade.getPayoutTxHex().equals(trade.getBuyer().getPaymentSentMessage().getPayoutTxHex())) {
-        if (trade.getTradePeer().getPaymentSentMessage() != null) nonSignedDisputePayoutTxHexes.add(trade.getTradePeer().getPaymentSentMessage().getPayoutTxHex());
+            log.warn("Nullifying payout tx hex after 1.0.7 update {} {}", trade.getClass().getSimpleName(), trade.getShortId());
-        if (trade.getTradePeer().getPaymentReceivedMessage() != null) {
+            if (trade instanceof BuyerTrade) trade.getSelf().setUnsignedPayoutTxHex(trade.getPayoutTxHex());
-            nonSignedDisputePayoutTxHexes.add(trade.getTradePeer().getPaymentReceivedMessage().getUnsignedPayoutTxHex());
+            trade.setPayoutTxHex(null);
-            nonSignedDisputePayoutTxHexes.add(trade.getTradePeer().getPaymentReceivedMessage().getSignedPayoutTxHex());
         }
-        boolean signed = trade.getPayoutTxHex() != null && !nonSignedDisputePayoutTxHexes.contains(trade.getPayoutTxHex());
 
         // sign arbitrator-signed payout tx
-        if (signed) disputeTxSet.setMultisigTxHex(trade.getPayoutTxHex());
+        if (trade.getPayoutTxHex() == null) {
-        else {
             MoneroMultisigSignResult result = multisigWallet.signMultisigTxHex(unsignedPayoutTxHex);
             if (result.getSignedMultisigTxHex() == null) throw new RuntimeException("Error signing arbitrator-signed payout tx");
             String signedMultisigTxHex = result.getSignedMultisigTxHex();
@@ -468,6 +464,8 @@ public final class ArbitrationManager extends DisputeManager<ArbitrationDisputeL
                 if (feeDiff > XmrWalletService.MINER_FEE_TOLERANCE) throw new RuntimeException("Miner fee is not within " + (XmrWalletService.MINER_FEE_TOLERANCE * 100) + "% of estimated fee, expected " + feeEstimate + " but was " + arbitratorSignedPayoutTx.getFee());
                 log.info("Payout tx fee {} is within tolerance, diff %={}", arbitratorSignedPayoutTx.getFee(), feeDiff);
             }
+        } else {
+            disputeTxSet.setMultisigTxHex(trade.getPayoutTxHex());
         }
 
         // submit fully signed payout tx to the network
@@ -485,8 +483,7 @@ public final class ArbitrationManager extends DisputeManager<ArbitrationDisputeL
         }
 
         // update state
-        trade.setPayoutTx(disputeTxSet.getTxs().get(0)); // TODO (woodser): is trade.payoutTx() mutually exclusive from dispute payout tx?
+        trade.updatePayout(disputeTxSet.getTxs().get(0));
-        trade.setPayoutTxId(disputeTxSet.getTxs().get(0).getHash());
         trade.setPayoutState(Trade.PayoutState.PAYOUT_PUBLISHED);
         dispute.setDisputePayoutTxId(disputeTxSet.getTxs().get(0).getHash());
         return disputeTxSet;

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

@@ -482,7 +482,7 @@ public abstract class Trade implements Tradable, Model {
     @Nullable
     @Getter
     @Setter
-    private String payoutTxHex;
+    private String payoutTxHex; // signed payout tx hex
     @Getter
     @Setter
     private String payoutTxKey;
@@ -1230,8 +1230,9 @@ public abstract class Trade implements Tradable, Model {
 
             // sign tx
             MoneroMultisigSignResult result = wallet.signMultisigTxHex(payoutTxHex);
-            if (result.getSignedMultisigTxHex() == null) throw new RuntimeException("Error signing payout tx");
+            if (result.getSignedMultisigTxHex() == null) throw new IllegalArgumentException("Error signing payout tx, signed multisig hex is null");
             payoutTxHex = result.getSignedMultisigTxHex();
+            setPayoutTxHex(payoutTxHex);
 
             // describe result
             describedTxSet = wallet.describeMultisigTxSet(payoutTxHex);
@@ -1247,8 +1248,8 @@ public abstract class Trade implements Tradable, Model {
         }
 
         // update trade state
-        setPayoutTx(payoutTx);
+        updatePayout(payoutTx);
-        setPayoutTxHex(payoutTxHex);
+        requestPersistence();
 
         // submit payout tx
         if (publish) {
@@ -1692,7 +1693,7 @@ public abstract class Trade implements Tradable, Model {
         getVolumeProperty().set(getVolume());
     }
 
-    public void setPayoutTx(MoneroTxWallet payoutTx) {
+    public void updatePayout(MoneroTxWallet payoutTx) {
 
         // set payout tx fields
         this.payoutTx = payoutTx;
@@ -2467,7 +2468,7 @@ public abstract class Trade implements Tradable, Model {
                     // check for outgoing txs (appears after wallet submits payout tx or on payout confirmed)
                     for (MoneroTxWallet tx : txs) {
                         if (tx.isOutgoing() && !tx.isFailed()) {
-                            setPayoutTx(tx);
+                            updatePayout(tx);
                             setPayoutStatePublished();
                             if (tx.isConfirmed()) setPayoutStateConfirmed();
                             if (!tx.isLocked()) setPayoutStateUnlocked();

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

@@ -63,7 +63,7 @@ public class BuyerPreparePaymentSentMessage extends TradeTask {
             runInterceptHook();
 
             // skip if payout tx already created
-            if (trade.getPayoutTxHex() != null) {
+            if (trade.getSelf().getUnsignedPayoutTxHex() != null) {
               log.warn("Skipping preparation of payment sent message because payout tx is already created for {} {}", trade.getClass().getSimpleName(), trade.getShortId());
               complete();
               return;
@@ -85,8 +85,8 @@ public class BuyerPreparePaymentSentMessage extends TradeTask {
                 // create payout tx
                 log.info("Buyer creating unsigned payout tx for {} {} ", trade.getClass().getSimpleName(), trade.getShortId());
                 MoneroTxWallet payoutTx = trade.createPayoutTx();
-                trade.setPayoutTx(payoutTx);
+                trade.updatePayout(payoutTx);
-                trade.setPayoutTxHex(payoutTx.getTxSet().getMultisigTxHex());
+                trade.getSelf().setUnsignedPayoutTxHex(payoutTx.getTxSet().getMultisigTxHex());
             }
 
             complete();

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

@@ -120,7 +120,7 @@ public abstract class BuyerSendPaymentSentMessage extends SendMailboxMessageTask
                     trade.getCounterCurrencyTxId(),
                     trade.getCounterCurrencyExtraData(),
                     deterministicId,
-                    trade.getPayoutTxHex(),
+                    trade.getSelf().getUnsignedPayoutTxHex(),
                     trade.getSelf().getUpdatedMultisigHex(),
                     trade.getSelf().getPaymentAccountKey(),
                     trade.getTradePeer().getAccountAgeWitness()

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

@@ -45,7 +45,6 @@ import haveno.core.trade.messages.PaymentReceivedMessage;
 import haveno.core.trade.messages.PaymentSentMessage;
 import haveno.core.util.Validator;
 import lombok.extern.slf4j.Slf4j;
-import org.apache.commons.lang3.StringUtils;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -132,6 +131,14 @@ public class ProcessPaymentReceivedMessage extends TradeTask {
 
     private void processPayoutTx(PaymentReceivedMessage message) {
 
+        // adapt from 1.0.6 to 1.0.7 which changes field usage
+        // TODO: remove after future updates to allow old trades to clear
+        if (trade.getPayoutTxHex() != null && trade.getBuyer().getPaymentSentMessage() != null && trade.getPayoutTxHex().equals(trade.getBuyer().getPaymentSentMessage().getPayoutTxHex())) {
+            log.warn("Nullifying payout tx hex after 1.0.7 update {} {}", trade.getClass().getSimpleName(), trade.getShortId());
+            if (trade instanceof BuyerTrade) trade.getSelf().setUnsignedPayoutTxHex(trade.getPayoutTxHex());
+            trade.setPayoutTxHex(null);
+        }
+
         // update wallet
         trade.importMultisigHex();
         trade.syncAndPollWallet();
@@ -160,11 +167,11 @@ public class ProcessPaymentReceivedMessage extends TradeTask {
                     try {
                         PaymentSentMessage paymentSentMessage = (trade.isArbitrator() ? trade.getBuyer() : trade.getArbitrator()).getPaymentSentMessage();
                         if (paymentSentMessage == null) throw new RuntimeException("Process model does not have payment sent message for " + trade.getClass().getSimpleName() + " " + trade.getId());
-                        if (StringUtils.equals(trade.getPayoutTxHex(), paymentSentMessage.getPayoutTxHex())) { // unsigned
+                        if (trade.getPayoutTxHex() == null) { // unsigned
                             log.info("{} {} verifying, signing, and publishing payout tx", trade.getClass().getSimpleName(), trade.getId());
                             trade.processPayoutTx(message.getUnsignedPayoutTxHex(), true, true);
                         } else {
-                            log.info("{} {} re-verifying and publishing payout tx", trade.getClass().getSimpleName(), trade.getId());
+                            log.info("{} {} re-verifying and publishing signed payout tx", trade.getClass().getSimpleName(), trade.getId());
                             trade.processPayoutTx(trade.getPayoutTxHex(), false, true);
                         }
                     } catch (Exception e) {

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

@@ -49,7 +49,6 @@ public class ProcessPaymentSentMessage extends TradeTask {
 
             // update state from message
             trade.getBuyer().setPaymentSentMessage(message);
-            trade.setPayoutTxHex(message.getPayoutTxHex());
             trade.getBuyer().setUpdatedMultisigHex(message.getUpdatedMultisigHex());
             trade.getSeller().setAccountAgeWitness(message.getSellerAccountAgeWitness());
             String counterCurrencyTxId = message.getCounterCurrencyTxId();

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

@@ -42,25 +42,45 @@ public class SellerPreparePaymentReceivedMessage extends TradeTask {
             // handle first time preparation
             if (trade.getArbitrator().getPaymentReceivedMessage() == null) {
 
-                // import multisig hex
+                // adapt from 1.0.6 to 1.0.7 which changes field usage
-                trade.importMultisigHex();
+                // TODO: remove after future updates to allow old trades to clear
-
+                if (trade.getPayoutTxHex() != null && trade.getPayoutTxHex().equals(trade.getBuyer().getPaymentSentMessage().getPayoutTxHex())) {
-                // verify, sign, and publish payout tx if given. otherwise create payout tx
+                    log.warn("Nullifying payout tx hex after 1.0.7 update {} {}", trade.getClass().getSimpleName(), trade.getShortId());
-                if (trade.getPayoutTxHex() != null) {
+                    trade.setPayoutTxHex(null);
-                    try {
-                        log.info("Seller verifying, signing, and publishing payout tx for trade {}", trade.getId());
-                        trade.processPayoutTx(trade.getPayoutTxHex(), true, true);
-                    } catch (Exception e) {
-                        log.warn("Error verifying, signing, and publishing payout tx for trade {}: {}. Creating unsigned payout tx", trade.getId(), e.getMessage());
-                        createUnsignedPayoutTx();
                 }
+
+                // import multisig hex unless already signed
+                if (trade.getPayoutTxHex() == null) {
+                    trade.importMultisigHex();
+                }
+
+                // verify, sign, and publish payout tx if given
+                if (trade.getBuyer().getPaymentSentMessage().getPayoutTxHex() != null) {
+                    try {
+                        if (trade.getPayoutTxHex() == null) {
+                            log.info("Seller verifying, signing, and publishing payout tx for trade {}", trade.getId());
+                            trade.processPayoutTx(trade.getBuyer().getPaymentSentMessage().getPayoutTxHex(), true, true);
                         } else {
+                            log.warn("Seller publishing previously signed payout tx for trade {}", trade.getId());
+                            trade.processPayoutTx(trade.getPayoutTxHex(), false, true);
+                        }
+                    } catch (IllegalArgumentException | IllegalStateException e) {
+                        log.warn("Illegal state or argument verifying, signing, and publishing payout tx for {} {}: {}. Creating new unsigned payout tx", trade.getClass().getSimpleName(), trade.getId(), e.getMessage());
+                        createUnsignedPayoutTx();
+                    } catch (Exception e) {
+                        log.warn("Error verifying, signing, and publishing payout tx for trade {}: {}", trade.getId(), e.getMessage());
+                        throw e;
+                    }
+                }
+                
+                // otherwise create unsigned payout tx
+                else if (trade.getSelf().getUnsignedPayoutTxHex() == null) {
                     createUnsignedPayoutTx();
                 }
             } else if (trade.getArbitrator().getPaymentReceivedMessage().getSignedPayoutTxHex() != null && !trade.isPayoutPublished()) {
 
                 // republish payout tx from previous message
-                log.info("Seller re-verifying and publishing payout tx for trade {}", trade.getId());
+                log.info("Seller re-verifying and publishing signed payout tx for trade {}", trade.getId());
                 trade.processPayoutTx(trade.getArbitrator().getPaymentReceivedMessage().getSignedPayoutTxHex(), false, true);
             }
 
@@ -80,7 +100,7 @@ public class SellerPreparePaymentReceivedMessage extends TradeTask {
     private void createUnsignedPayoutTx() {
         log.info("Seller creating unsigned payout tx for trade {}", trade.getId());
         MoneroTxWallet payoutTx = trade.createPayoutTx();
-        trade.setPayoutTx(payoutTx);
+        trade.updatePayout(payoutTx);
-        trade.setPayoutTxHex(payoutTx.getTxSet().getMultisigTxHex());
+        trade.getSelf().setUnsignedPayoutTxHex(payoutTx.getTxSet().getMultisigTxHex());
     }
 }

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

@@ -50,7 +50,7 @@ import haveno.network.p2p.NodeAddress;
 import lombok.EqualsAndHashCode;
 import lombok.extern.slf4j.Slf4j;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkArgument;
 
 @Slf4j
 @EqualsAndHashCode(callSuper = true)
@@ -85,7 +85,6 @@ public abstract class SellerSendPaymentReceivedMessage extends SendMailboxMessag
 
     @Override
     protected TradeMailboxMessage getTradeMailboxMessage(String tradeId) {
-        checkNotNull(trade.getPayoutTxHex(), "Payout tx must not be null");
         if (getReceiver().getPaymentReceivedMessage() == null) {
 
             // sign account witness
@@ -104,14 +103,15 @@ public abstract class SellerSendPaymentReceivedMessage extends SendMailboxMessag
                     tradeId,
                     processModel.getMyNodeAddress(),
                     deterministicId,
-                    trade.isPayoutPublished() ? null : trade.getPayoutTxHex(), // unsigned
+                    trade.getPayoutTxHex() == null ? trade.getSelf().getUnsignedPayoutTxHex() : null, // unsigned // TODO: phase in after next update to clear old style trades
-                    trade.isPayoutPublished() ? trade.getPayoutTxHex() : null, // signed
+                    trade.getPayoutTxHex() == null ? null : trade.getPayoutTxHex(), // signed
                     trade.getSelf().getUpdatedMultisigHex(),
                     trade.getState().ordinal() >= Trade.State.SELLER_SAW_ARRIVED_PAYMENT_RECEIVED_MSG.ordinal(), // informs to expect payout
                     trade.getTradePeer().getAccountAgeWitness(),
                     signedWitness,
                     getReceiver() == trade.getArbitrator() ? trade.getBuyer().getPaymentSentMessage() : null // buyer already has payment sent message
             );
+            checkArgument(message.getUnsignedPayoutTxHex() != null || message.getSignedPayoutTxHex() != null, "PaymentReceivedMessage does not include payout tx hex");
 
             // sign message
             try {