From 9ab659ceac8993f50eb4c83f338d6345e7f417e4 Mon Sep 17 00:00:00 2001 From: jmacxx <47253594+jmacxx@users.noreply.github.com> Date: Sat, 9 Oct 2021 08:42:11 -0500 Subject: [PATCH 1/2] Fix issue of Trade Step 1 validation done too soon Deprecate 4 states which are not used. ShareBuyerPaymentAccountMessage can arrive before deposit broadcast completes. --- core/src/main/java/bisq/core/trade/Trade.java | 8 +++---- .../core/trade/protocol/SellerProtocol.java | 2 +- ...ndsDepositTxAndDelayedPayoutTxMessage.java | 22 +++++++++---------- .../pendingtrades/PendingTradesViewModel.java | 8 +------ proto/src/main/proto/pb.proto | 8 +++---- 5 files changed, 20 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/Trade.java b/core/src/main/java/bisq/core/trade/Trade.java index de5c1949bb7..447f8b52ea2 100644 --- a/core/src/main/java/bisq/core/trade/Trade.java +++ b/core/src/main/java/bisq/core/trade/Trade.java @@ -119,10 +119,10 @@ public enum State { // DEPOSIT_TX_PUBLISHED_MSG // seller perspective - SELLER_SENT_DEPOSIT_TX_PUBLISHED_MSG(Phase.DEPOSIT_PUBLISHED), - SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG(Phase.DEPOSIT_PUBLISHED), - SELLER_STORED_IN_MAILBOX_DEPOSIT_TX_PUBLISHED_MSG(Phase.DEPOSIT_PUBLISHED), - SELLER_SEND_FAILED_DEPOSIT_TX_PUBLISHED_MSG(Phase.DEPOSIT_PUBLISHED), + @Deprecated SELLER_SENT_DEPOSIT_TX_PUBLISHED_MSG(Phase.DEPOSIT_PUBLISHED), + @Deprecated SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG(Phase.DEPOSIT_PUBLISHED), + @Deprecated SELLER_STORED_IN_MAILBOX_DEPOSIT_TX_PUBLISHED_MSG(Phase.DEPOSIT_PUBLISHED), + @Deprecated SELLER_SEND_FAILED_DEPOSIT_TX_PUBLISHED_MSG(Phase.DEPOSIT_PUBLISHED), // buyer perspective BUYER_RECEIVED_DEPOSIT_TX_PUBLISHED_MSG(Phase.DEPOSIT_PUBLISHED), diff --git a/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java b/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java index cb94485fb1f..ef5f83b0d34 100644 --- a/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java @@ -87,7 +87,7 @@ protected void handle(DelayedPayoutTxSignatureResponse message, NodeAddress peer } protected void handle(ShareBuyerPaymentAccountMessage message, NodeAddress peer) { - expect(anyPhase(Trade.Phase.DEPOSIT_PUBLISHED, Trade.Phase.DEPOSIT_CONFIRMED) + expect(anyPhase(Trade.Phase.TAKER_FEE_PUBLISHED, Trade.Phase.DEPOSIT_PUBLISHED, Trade.Phase.DEPOSIT_CONFIRMED) .with(message) .from(peer)) .setup(tasks(SellerProcessShareBuyerPaymentAccountMessage.class, diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSendsDepositTxAndDelayedPayoutTxMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSendsDepositTxAndDelayedPayoutTxMessage.java index 5fad7f71e8a..b20b9654c9c 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSendsDepositTxAndDelayedPayoutTxMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSendsDepositTxAndDelayedPayoutTxMessage.java @@ -79,16 +79,15 @@ protected TradeMailboxMessage getTradeMailboxMessage(String tradeId) { @Override protected void setStateSent() { - trade.setStateIfValidTransitionTo(Trade.State.SELLER_SENT_DEPOSIT_TX_PUBLISHED_MSG); - - processModel.getTradeManager().requestPersistence(); + // we no longer set deprecated state (Trade.State.SELLER_SENT_DEPOSIT_TX_PUBLISHED_MSG); + // see https://github.com/bisq-network/bisq/pull/5746#issuecomment-939879623 } @Override protected void setStateArrived() { - trade.setStateIfValidTransitionTo(Trade.State.SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG); + // we no longer set deprecated state (Trade.State.SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG); + // see https://github.com/bisq-network/bisq/pull/5746#issuecomment-939879623 - processModel.getTradeManager().requestPersistence(); cleanup(); // Complete is called in base class } @@ -101,9 +100,9 @@ protected void onStoredInMailbox() { @Override protected void setStateStoredInMailbox() { - trade.setStateIfValidTransitionTo(Trade.State.SELLER_STORED_IN_MAILBOX_DEPOSIT_TX_PUBLISHED_MSG); + // we no longer set deprecated state (Trade.State.SELLER_STORED_IN_MAILBOX_DEPOSIT_TX_PUBLISHED_MSG); + // see https://github.com/bisq-network/bisq/pull/5746#issuecomment-939879623 - processModel.getTradeManager().requestPersistence(); // The DepositTxAndDelayedPayoutTxMessage is a mailbox message as earlier we use only the deposit tx which can // be also received from the network once published. // Now we send the delayed payout tx as well and with that this message is mandatory for continuing the protocol. @@ -125,12 +124,11 @@ protected void onFault(String errorMessage, TradeMessage message) { @Override protected void setStateFault() { - trade.setStateIfValidTransitionTo(Trade.State.SELLER_SEND_FAILED_DEPOSIT_TX_PUBLISHED_MSG); + // we no longer set deprecated state (Trade.State.SELLER_SEND_FAILED_DEPOSIT_TX_PUBLISHED_MSG); + // see https://github.com/bisq-network/bisq/pull/5746#issuecomment-939879623 if (!trade.isDepositConfirmed()) { tryToSendAgainLater(); } - - processModel.getTradeManager().requestPersistence(); } @Override @@ -184,9 +182,9 @@ private void onMessageStateChange(MessageState newValue) { // Once we receive an ACK from our msg we know the peer has received the msg and we stop. if (newValue == MessageState.ACKNOWLEDGED) { // We treat a ACK like SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG - trade.setStateIfValidTransitionTo(Trade.State.SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG); + // we no longer set deprecated state (Trade.State.SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG); + // see https://github.com/bisq-network/bisq/pull/5746#issuecomment-939879623 - processModel.getTradeManager().requestPersistence(); cleanup(); complete(); } diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java index 67abc107fc4..21407c5fe42 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java @@ -433,15 +433,9 @@ private void onTradeStateChanged(Trade.State tradeState) { // #################### Phase DEPOSIT_PAID - case SELLER_PUBLISHED_DEPOSIT_TX: - // DEPOSIT_TX_PUBLISHED_MSG // seller perspective - case SELLER_SENT_DEPOSIT_TX_PUBLISHED_MSG: - case SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG: - case SELLER_STORED_IN_MAILBOX_DEPOSIT_TX_PUBLISHED_MSG: - case SELLER_SEND_FAILED_DEPOSIT_TX_PUBLISHED_MSG: - + case SELLER_PUBLISHED_DEPOSIT_TX: // buyer perspective case BUYER_RECEIVED_DEPOSIT_TX_PUBLISHED_MSG: diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index f2ddf37287e..3da99c00db8 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -1498,10 +1498,10 @@ message Trade { MAKER_SEND_FAILED_PUBLISH_DEPOSIT_TX_REQUEST = 6; TAKER_RECEIVED_PUBLISH_DEPOSIT_TX_REQUEST = 7; SELLER_PUBLISHED_DEPOSIT_TX = 8; - SELLER_SENT_DEPOSIT_TX_PUBLISHED_MSG = 9; - SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG = 10; - SELLER_STORED_IN_MAILBOX_DEPOSIT_TX_PUBLISHED_MSG = 11; - SELLER_SEND_FAILED_DEPOSIT_TX_PUBLISHED_MSG = 12; + SELLER_SENT_DEPOSIT_TX_PUBLISHED_MSG = 9 [deprecated = true]; + SELLER_SAW_ARRIVED_DEPOSIT_TX_PUBLISHED_MSG = 10 [deprecated = true]; + SELLER_STORED_IN_MAILBOX_DEPOSIT_TX_PUBLISHED_MSG = 11 [deprecated = true]; + SELLER_SEND_FAILED_DEPOSIT_TX_PUBLISHED_MSG = 12 [deprecated = true]; BUYER_RECEIVED_DEPOSIT_TX_PUBLISHED_MSG = 13; BUYER_SAW_DEPOSIT_TX_IN_NETWORK = 14; DEPOSIT_CONFIRMED_IN_BLOCK_CHAIN = 15; From 56b3f3fae1dade4305fed2e336ebf524683001e8 Mon Sep 17 00:00:00 2001 From: jmacxx <47253594+jmacxx@users.noreply.github.com> Date: Mon, 11 Oct 2021 14:20:17 -0500 Subject: [PATCH 2/2] Check for invalid tx chain only after deposit published --- .../main/portfolio/pendingtrades/PendingTradesView.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java index 38acf82510f..364953640ed 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java @@ -368,7 +368,8 @@ private void updateMoveTradeToFailedColumnState() { } private boolean isMaybeInvalidTrade(Trade trade) { - return trade.isTxChainInvalid() || trade.hasErrorMessage(); + return trade.hasErrorMessage() || + (Trade.Phase.DEPOSIT_PUBLISHED.ordinal() <= trade.getPhase().ordinal() && trade.isTxChainInvalid()); } private void onMoveInvalidTradeToFailedTrades(Trade trade) {