From de144143e5153cc1fc52304a0858d8bc07147967 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sat, 21 Dec 2019 16:17:12 -0500 Subject: [PATCH 1/4] Add button for shutdown If a trader has a pending trade with an unconfirmed tx and is doing a spv resync, the deposit tx is not found. They get displayed a popup asking to restart and if problem continues to move trade to failed trades. In regtest testing the missing deposit tx was always received from the network at a restart. So this commit adds another button as default button to shut down Bisq so that the user do first that activity instead of moving the trade to failed trades. It is not guaranteed that the trader will receive the missing deposit tx at restart, but at least it is better as the state before. We should find a way how to distinguish a valid unconfirmed tx from an invalid one but it is not clear yet how we can do that and if it is feasible with doing that with BitcoinJ only. It might require a external service to look up the tx if it is in the mem pool, but even that will never gie a 100% certainty. --- core/src/main/resources/i18n/displayStrings.properties | 1 + desktop/src/main/java/bisq/desktop/main/MainViewModel.java | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/core/src/main/resources/i18n/displayStrings.properties b/core/src/main/resources/i18n/displayStrings.properties index ef5a0206206..9d256bd0290 100644 --- a/core/src/main/resources/i18n/displayStrings.properties +++ b/core/src/main/resources/i18n/displayStrings.properties @@ -2606,6 +2606,7 @@ popup.warning.trade.depositTxNull=The trade with ID {0} has no deposit transacti Please restart the application and if the problem remains move the trade to failed trades and report the problem to \ the Bisq support channel at the Bisq Keybase team. popup.warning.trade.depositTxNull.moveToFailedTrades=Move to failed trades +popup.warning.trade.depositTxNull.shutDown=Shut down Bisq popup.info.securityDepositInfo=To ensure both traders follow the trade protocol, both traders need to pay a security \ deposit.\n\nThis deposit is kept in your trade wallet until your trade has been successfully completed, and then it's \ diff --git a/desktop/src/main/java/bisq/desktop/main/MainViewModel.java b/desktop/src/main/java/bisq/desktop/main/MainViewModel.java index aed88e56701..5d9668fc174 100644 --- a/desktop/src/main/java/bisq/desktop/main/MainViewModel.java +++ b/desktop/src/main/java/bisq/desktop/main/MainViewModel.java @@ -383,8 +383,10 @@ private void setupHandlers() { if (c.wasAdded()) { c.getAddedSubList().forEach(trade -> { new Popup().warning(Res.get("popup.warning.trade.depositTxNull", trade.getShortId())) - .actionButtonText(Res.get("popup.warning.trade.depositTxNull.moveToFailedTrades")) - .onAction(() -> tradeManager.addTradeToFailedTrades(trade)) + .actionButtonText(Res.get("popup.warning.trade.depositTxNull.shutDown")) + .onAction(() -> BisqApp.getShutDownHandler().run()) + .secondaryActionButtonText(Res.get("popup.warning.trade.depositTxNull.moveToFailedTrades")) + .onSecondaryAction(() -> tradeManager.addTradeToFailedTrades(trade)) .show(); }); } From 292ac30f8570a88aec7c1cdd338f5995f94c39b8 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sat, 21 Dec 2019 16:59:20 -0500 Subject: [PATCH 2/4] Handle unconfirmed BSQ change at spv resync In case we had an unconfirmed change output we reset the unconfirmedBsqChangeOutputList so that after a SPV resync we do not have any dangling BSQ utxos in that list which would cause an incorrect BSQ balance state after the SPV resync. --- core/src/main/java/bisq/core/app/BisqSetup.java | 9 +++++++++ .../UnconfirmedBsqChangeOutputListService.java | 12 ++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/bisq/core/app/BisqSetup.java b/core/src/main/java/bisq/core/app/BisqSetup.java index 4520b74d05a..2936e158da3 100644 --- a/core/src/main/java/bisq/core/app/BisqSetup.java +++ b/core/src/main/java/bisq/core/app/BisqSetup.java @@ -33,6 +33,7 @@ import bisq.core.dao.governance.asset.AssetService; import bisq.core.dao.governance.voteresult.VoteResultException; import bisq.core.dao.governance.voteresult.VoteResultService; +import bisq.core.dao.state.unconfirmed.UnconfirmedBsqChangeOutputListService; import bisq.core.filter.FilterManager; import bisq.core.locale.Res; import bisq.core.notifications.MobileNotificationService; @@ -170,6 +171,7 @@ default void onRequestWalletPassword() { private final ClockWatcher clockWatcher; private final FeeService feeService; private final DaoSetup daoSetup; + private final UnconfirmedBsqChangeOutputListService unconfirmedBsqChangeOutputListService; private final EncryptionService encryptionService; private final KeyRing keyRing; private final BisqEnvironment bisqEnvironment; @@ -256,6 +258,7 @@ public BisqSetup(P2PNetworkSetup p2PNetworkSetup, ClockWatcher clockWatcher, FeeService feeService, DaoSetup daoSetup, + UnconfirmedBsqChangeOutputListService unconfirmedBsqChangeOutputListService, EncryptionService encryptionService, KeyRing keyRing, BisqEnvironment bisqEnvironment, @@ -302,6 +305,7 @@ public BisqSetup(P2PNetworkSetup p2PNetworkSetup, this.clockWatcher = clockWatcher; this.feeService = feeService; this.daoSetup = daoSetup; + this.unconfirmedBsqChangeOutputListService = unconfirmedBsqChangeOutputListService; this.encryptionService = encryptionService; this.keyRing = keyRing; this.bisqEnvironment = bisqEnvironment; @@ -448,6 +452,11 @@ private void maybeReSyncSPVChain() { if (preferences.isResyncSpvRequested()) { try { walletsSetup.reSyncSPVChain(); + + // In case we had an unconfirmed change output we reset the unconfirmedBsqChangeOutputList so that + // after a SPV resync we do not have any dangling BSQ utxos in that list which would cause an incorrect + // BSQ balance state after the SPV resync. + unconfirmedBsqChangeOutputListService.onSpvResync(); } catch (IOException e) { log.error(e.toString()); e.printStackTrace(); diff --git a/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputListService.java b/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputListService.java index e153c2045f3..432f3c86959 100644 --- a/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputListService.java +++ b/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputListService.java @@ -148,8 +148,11 @@ public void onCommitTx(Transaction tx, TxType txType, Wallet wallet) { } public void onReorganize() { - unconfirmedBsqChangeOutputList.clear(); - persist(); + reset(); + } + + public void onSpvResync() { + reset(); } public void onTransactionConfidenceChanged(Transaction tx) { @@ -191,6 +194,11 @@ private void removeConnectedOutputsOfInputsOfTx(Transaction tx) { }); } + private void reset() { + unconfirmedBsqChangeOutputList.clear(); + persist(); + } + private void persist() { storage.queueUpForSave(); } From 29a8b10cdc5f7e1b64123a52dec0bc46f07963df Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sat, 21 Dec 2019 17:51:18 -0500 Subject: [PATCH 3/4] Improve popup text --- .../main/resources/i18n/displayStrings.properties | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/core/src/main/resources/i18n/displayStrings.properties b/core/src/main/resources/i18n/displayStrings.properties index 9d256bd0290..7fde0c5adb2 100644 --- a/core/src/main/resources/i18n/displayStrings.properties +++ b/core/src/main/resources/i18n/displayStrings.properties @@ -2602,9 +2602,16 @@ popup.warning.openOfferWithInvalidMakerFeeTx=The maker fee transaction for offer Please go to \"Settings/Network info\" and do a SPV resync.\n\ For further help please contact the Bisq support channel at the Bisq Keybase team. -popup.warning.trade.depositTxNull=The trade with ID {0} has no deposit transaction set.\n\ - Please restart the application and if the problem remains move the trade to failed trades and report the problem to \ - the Bisq support channel at the Bisq Keybase team. +popup.warning.trade.depositTxNull=The trade with ID ''{0}'' has no deposit transaction set.\n\n\ + Please restart the application to see if the problem still exist.\n\n\ + If so, please open the trade details popup by clicking on the trade ID and open a block explorer for both the \ + maker fee transaction and the taker fee transaction (by clicking on both transaction IDs). If any of those \ + transactions is not found in the block explorer it is likely because it was an invalid transaction.\n\n\ + If this was the case, please report the problem in the Bisq support channel at the Bisq Keybase team. If you are \ + sure the problem is caused by an invalid transaction move the trade to failed trades.\n\n\ + No funds have left your wallet in that case. If your trade fee tx was valid you have lost the trade fee and you can \ + request for reimbursement at the support repository on Github (https://github.com/bisq-network/support/issues).\n\n\ + Make a SPV resync at ''Settings/Network'' to clean up your wallet from any invalid transactions! popup.warning.trade.depositTxNull.moveToFailedTrades=Move to failed trades popup.warning.trade.depositTxNull.shutDown=Shut down Bisq From 47647a98f88b9ffcfacc440fe137b4f17c4277d8 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 23 Dec 2019 13:19:48 -0500 Subject: [PATCH 4/4] Improve text --- .../resources/i18n/displayStrings.properties | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/core/src/main/resources/i18n/displayStrings.properties b/core/src/main/resources/i18n/displayStrings.properties index 7fde0c5adb2..a2b7e0ec917 100644 --- a/core/src/main/resources/i18n/displayStrings.properties +++ b/core/src/main/resources/i18n/displayStrings.properties @@ -2603,15 +2603,17 @@ popup.warning.openOfferWithInvalidMakerFeeTx=The maker fee transaction for offer For further help please contact the Bisq support channel at the Bisq Keybase team. popup.warning.trade.depositTxNull=The trade with ID ''{0}'' has no deposit transaction set.\n\n\ - Please restart the application to see if the problem still exist.\n\n\ - If so, please open the trade details popup by clicking on the trade ID and open a block explorer for both the \ - maker fee transaction and the taker fee transaction (by clicking on both transaction IDs). If any of those \ - transactions is not found in the block explorer it is likely because it was an invalid transaction.\n\n\ - If this was the case, please report the problem in the Bisq support channel at the Bisq Keybase team. If you are \ - sure the problem is caused by an invalid transaction move the trade to failed trades.\n\n\ - No funds have left your wallet in that case. If your trade fee tx was valid you have lost the trade fee and you can \ - request for reimbursement at the support repository on Github (https://github.com/bisq-network/support/issues).\n\n\ - Make a SPV resync at ''Settings/Network'' to clean up your wallet from any invalid transactions! + Please restart the application to see if the problem still exists.\n\n\ + If it does, please open the trade details popup by clicking on the trade ID. Then click on the transaction IDs for \ + the maker fee transaction and the taker fee transaction to view them on a block explorer. A transaction \ + that cannot be found in a block explorer is probably an invalid transaction.\n\n\ + If this happens, please report it in the #support channel on the Bisq Keybase (https://keybase.io/team/bisq). \ + If your trade fee transaction is invalid, no funds have left your wallet, you can move the trade to failed trades,\ + and do an SPV resync for your funds to reappear (see how below).\n\n\ + If your trade fee transaction is valid, the fee amount is lost, and you can make a \ + request for reimbursement on the support repository on GitHub (https://github.com/bisq-network/support/issues).\n\n\ + In both cases, please do an SPV resync from the ''Settings/Network'' screen to clean your wallet of any lingering issues! + popup.warning.trade.depositTxNull.moveToFailedTrades=Move to failed trades popup.warning.trade.depositTxNull.shutDown=Shut down Bisq