From edee8c5a23af0962999cb4b53ea6b4931cebb6c4 Mon Sep 17 00:00:00 2001 From: woodser Date: Thu, 11 Jan 2024 10:22:38 -0500 Subject: [PATCH 1/2] fix payment sent message state property after improper shut down --- .../main/java/haveno/core/trade/Trade.java | 26 +++++++++++++++++++ .../core/trade/protocol/TradeProtocol.java | 2 +- .../pendingtrades/PendingTradesViewModel.java | 6 ++--- .../steps/buyer/BuyerStep2View.java | 11 ++++---- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/haveno/core/trade/Trade.java b/core/src/main/java/haveno/core/trade/Trade.java index 591de631b92..bc497394441 100644 --- a/core/src/main/java/haveno/core/trade/Trade.java +++ b/core/src/main/java/haveno/core/trade/Trade.java @@ -32,6 +32,7 @@ import haveno.core.api.XmrConnectionService; import haveno.core.monetary.Price; import haveno.core.monetary.Volume; +import haveno.core.network.MessageState; import haveno.core.offer.Offer; import haveno.core.offer.OfferDirection; import haveno.core.payment.payload.PaymentAccountPayload; @@ -693,6 +694,13 @@ public void initialize(ProcessModelServiceProvider serviceProvider) { xmrWalletService.addWalletListener(idlePayoutSyncer); } + // TODO: trader's payment sent message state property can become unsynced (after improper shut down?) + MessageState expectedState = getPaymentSentMessageState(); + if (!isArbitrator() && expectedState != null && expectedState != processModel.getPaymentSentMessageStateProperty().get()) { + log.warn("Updating unexpected payment sent message state for {} {}, expected={}, actual={}", getClass().getSimpleName(), getId(), expectedState, processModel.getPaymentSentMessageStateProperty().get()); + processModel.getPaymentSentMessageStateProperty().set(expectedState); + } + // trade is initialized isInitialized = true; @@ -1569,6 +1577,24 @@ public String getRole() { throw new IllegalArgumentException("Trade is not buyer, seller, or arbitrator"); } + public MessageState getPaymentSentMessageState() { + if (isPaymentReceived()) return MessageState.ACKNOWLEDGED; + if (processModel.getPaymentSentMessageStateProperty().get() == MessageState.ACKNOWLEDGED) return MessageState.ACKNOWLEDGED; + switch (state) { + case BUYER_SENT_PAYMENT_SENT_MSG: + case BUYER_SAW_ARRIVED_PAYMENT_SENT_MSG: + return MessageState.SENT; + case BUYER_STORED_IN_MAILBOX_PAYMENT_SENT_MSG: + return MessageState.STORED_IN_MAILBOX; + case SELLER_RECEIVED_PAYMENT_SENT_MSG: + return MessageState.ARRIVED; + case BUYER_SEND_FAILED_PAYMENT_SENT_MSG: + return MessageState.FAILED; + default: + return null; + } + } + public String getPeerRole(TradePeer peer) { if (peer == getBuyer()) return "Buyer"; if (peer == getSeller()) return "Seller"; diff --git a/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java b/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java index f04f9f1fb44..446215cbfa5 100644 --- a/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java +++ b/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java @@ -770,7 +770,7 @@ private PubKeyRing getPeersPubKeyRing(NodeAddress address) { trade.setMyNodeAddress(); // TODO: this is a hack to update my node address before verifying the message TradePeer peer = trade.getTradePeer(address); if (peer == null) { - log.warn("Cannot get peer's pub key ring because peer is not maker, taker, or arbitrator. Their address might have changed: " + peer); + log.warn("Cannot get peer's pub key ring because peer is not maker, taker, or arbitrator. Their address might have changed: " + address); return null; } return peer.getPubKeyRing(); diff --git a/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java b/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java index 1cddb827d40..73db3c3541b 100644 --- a/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java +++ b/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java @@ -191,14 +191,12 @@ public void onSelectedItemChanged(PendingTradesListItem selectedItem) { }); messageStateSubscription = EasyBind.subscribe(trade.getProcessModel().getPaymentSentMessageStateProperty(), this::onMessageStateChanged); } - } } public void setMessageStateProperty(MessageState messageState) { - // ARRIVED is set internally after ACKNOWLEDGED, otherwise warn if subsequent states received - if ((messageStateProperty.get() == MessageState.ACKNOWLEDGED && messageState != MessageState.ARRIVED) || messageStateProperty.get() == MessageState.ARRIVED) { - log.warn("We have already an ACKNOWLEDGED/ARRIVED message received. " + + if (messageStateProperty.get() == MessageState.ACKNOWLEDGED) { + log.warn("We have already an ACKNOWLEDGED message received. " + "We would not expect any other message after that. Received messageState={}", messageState); return; } diff --git a/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java b/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java index 85a74d1518f..6990d40343c 100644 --- a/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java +++ b/desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java @@ -22,7 +22,6 @@ import haveno.common.app.DevEnv; import haveno.common.util.Tuple4; import haveno.core.locale.Res; -import haveno.core.network.MessageState; import haveno.core.offer.Offer; import haveno.core.payment.PaymentAccount; import haveno.core.payment.PaymentAccountUtil; @@ -158,7 +157,7 @@ public void activate() { case BUYER_SAW_ARRIVED_PAYMENT_SENT_MSG: busyAnimation.play(); statusLabel.setText(Res.get("shared.sendingConfirmation")); - model.setMessageStateProperty(MessageState.SENT); + model.setMessageStateProperty(trade.getPaymentSentMessageState()); timeoutTimer = UserThread.runAfter(() -> { busyAnimation.stop(); statusLabel.setText(Res.get("shared.sendingConfirmationAgain")); @@ -167,25 +166,25 @@ public void activate() { case BUYER_STORED_IN_MAILBOX_PAYMENT_SENT_MSG: busyAnimation.stop(); statusLabel.setText(Res.get("shared.messageStoredInMailbox")); - model.setMessageStateProperty(MessageState.STORED_IN_MAILBOX); + model.setMessageStateProperty(trade.getPaymentSentMessageState()); break; case SELLER_RECEIVED_PAYMENT_SENT_MSG: busyAnimation.stop(); statusLabel.setText(Res.get("shared.messageArrived")); - model.setMessageStateProperty(MessageState.ARRIVED); + model.setMessageStateProperty(trade.getPaymentSentMessageState()); break; case BUYER_SEND_FAILED_PAYMENT_SENT_MSG: // We get a popup and the trade closed, so we dont need to show anything here busyAnimation.stop(); statusLabel.setText(""); - model.setMessageStateProperty(MessageState.FAILED); + model.setMessageStateProperty(trade.getPaymentSentMessageState()); break; default: log.warn("Unexpected case: State={}, tradeId={} ", state.name(), trade.getId()); busyAnimation.stop(); statusLabel.setText(Res.get("shared.sendingConfirmationAgain")); break; - } + } } }); } From a2aa321d021518a7745311843b5fcac58ed34b92 Mon Sep 17 00:00:00 2001 From: woodser Date: Fri, 12 Jan 2024 06:42:21 -0500 Subject: [PATCH 2/2] do not re-complete task to send deposits confirmed message --- .../core/trade/protocol/tasks/SendDepositsConfirmedMessage.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/haveno/core/trade/protocol/tasks/SendDepositsConfirmedMessage.java b/core/src/main/java/haveno/core/trade/protocol/tasks/SendDepositsConfirmedMessage.java index 5fb80339358..02b9200f2dd 100644 --- a/core/src/main/java/haveno/core/trade/protocol/tasks/SendDepositsConfirmedMessage.java +++ b/core/src/main/java/haveno/core/trade/protocol/tasks/SendDepositsConfirmedMessage.java @@ -54,7 +54,7 @@ protected void run() { // skip if already acked by receiver if (ackedByReceiver()) { - complete(); + if (!isCompleted()) complete(); return; }