<p><b>@sqrrm</b> commented on this pull request.</p>
<p>I have some comments, haven't tested it yet. I think it's basically good though.</p><hr>
<p>In <a href="https://github.com/bisq-network/bisq/pull/3566#discussion_r343118576">core/src/main/java/bisq/core/app/BisqSetup.java</a>:</p>
<pre style='color:#555'>> - if (lockedUpFundsHandler != null) {
- lockedUpFundsHandler.accept(message);
+ // We check if there are locked up funds in failed or closed trades
+ try {
+ Set<String> setOfAllTradeIds = tradeManager.getSetOfFailedOrClosedTradeIdsFromLockedInFunds();
+ btcWalletService.getAddressEntriesForTrade().stream()
+ .filter(e -> setOfAllTradeIds.contains(e.getOfferId()) &&
+ e.getContext() == AddressEntry.Context.MULTI_SIG)
+ .forEach(e -> {
+ Coin balance = e.getCoinLockedInMultiSig();
+ if (balance.isPositive()) {
+ String message = Res.get("popup.warning.lockedUpFunds",
+ formatter.formatCoinWithCode(balance), e.getAddressString(), e.getOfferId());
+ log.warn(message);
+ if (lockedUpFundsHandler != null) {
+ lockedUpFundsHandler.accept(message);
</pre>
<p>Is there something preventing automatic opening of disputes here, rather than asking the trader to do it manually?</p>
<hr>
<p>In <a href="https://github.com/bisq-network/bisq/pull/3566#discussion_r343126322">core/src/main/java/bisq/core/app/BisqSetup.java</a>:</p>
<pre style='color:#555'>> + String details = null;
+ if (txId.equals(trade.getDepositTxId())) {
+ details = Res.get("popup.warning.trade.txRejected.deposit");
+ }
+ if (txId.equals(trade.getOffer().getOfferFeePaymentTxId()) || txId.equals(trade.getTakerFeeTxId())) {
+ details = Res.get("popup.warning.trade.txRejected.tradeFee");
+ }
+
+ if (details != null) {
+ // We delay to avoid concurrent modification exceptions
+ String finalDetails = details;
+ UserThread.runAfter(() -> {
+ trade.setErrorMessage(newValue.getMessage());
+ rejectedTxErrorMessageHandler.accept(Res.get("popup.warning.trade.txRejected",
+ finalDetails, trade.getShortId(), txId));
+ tradeManager.addTradeToFailedTrades(trade);
</pre>
<p>It would be nice to avoid losing the offer fee tx and revert this to an active offer.</p>
<hr>
<p>In <a href="https://github.com/bisq-network/bisq/pull/3566#discussion_r343128398">core/src/main/java/bisq/core/btc/setup/WalletsSetup.java</a>:</p>
<pre style='color:#555'>> @@ -221,6 +225,20 @@ protected void onSetupCompleted() {
peerGroup.addBlocksDownloadedEventListener((peer, block, filteredBlock, blocksLeft) -> {
blocksDownloadedFromPeer.set(peer);
});
+
+ // Need to be Threading.SAME_THREAD executor otherwise BitcoinJ will skip that listener
+ peerGroup.addPreMessageReceivedEventListener(Threading.SAME_THREAD, (peer, message) -> {
+ if (message instanceof RejectMessage) {
+ UserThread.execute(() -> {
+ RejectMessage rejectMessage = (RejectMessage) message;
+ String msg = rejectMessage.toString();
+ log.error(msg);
</pre>
<p>I think this should be warning log level</p>
<hr>
<p>In <a href="https://github.com/bisq-network/bisq/pull/3566#discussion_r343128982">core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java</a>:</p>
<pre style='color:#555'>> @@ -672,7 +672,13 @@ public void resetAddressEntriesForOpenOffer(String offerId) {
public void resetAddressEntriesForPendingTrade(String offerId) {
swapTradeEntryToAvailableEntry(offerId, AddressEntry.Context.MULTI_SIG);
- // Don't swap TRADE_PAYOUT as it might be still open in the last trade step to be used for external transfer
+ // We swap also TRADE_PAYOUT to be sure all is cleaned up. There might be cases where a user cannot send the funds
+ // to an external wallet directly in the last step of the trade, but the funds are in the Bisq wallet anyway and
+ // the dealing with the external wallet is pure UI thing. The user can move the funds to the wallet and then
+ // send out the funds to the external wallet. As this cleanup is a rare situation and most users do not use
+ // the feature to send out the funds we prefer that strategy (if we keep the address entry it might cause
+ // complications in some edge cases after a SPV resync).
+ swapTradeEntryToAvailableEntry(offerId, AddressEntry.Context.TRADE_PAYOUT);
</pre>
<p>Is there a risk of address reuse by re-labeling this address?</p>
<hr>
<p>In <a href="https://github.com/bisq-network/bisq/pull/3566#discussion_r343134448">core/src/main/java/bisq/core/trade/TradeManager.java</a>:</p>
<pre style='color:#555'>> - log.warn("We found a closed trade with locked up funds. " +
- "That should never happen. trade ID=" + e.getId());
- return e.getId();
+ tradesIdSet.addAll(closedTradableManager.getTradesStreamWithFundsLockedIn()
+ .map(trade -> {
+ Transaction depositTx = trade.getDepositTx();
+ if (depositTx != null) {
+ TransactionConfidence confidence = btcWalletService.getConfidenceForTxId(depositTx.getHashAsString());
+ if (confidence != null && confidence.getConfidenceType() != TransactionConfidence.ConfidenceType.BUILDING) {
+ tradeTxException.set(new TradeTxException(Res.get("error.closedTradeWithUnconfirmedDepositTx", trade.getShortId())));
+ } else {
+ log.warn("We found a closed trade with locked up funds. " +
+ "That should never happen. trade ID=" + trade.getId());
+ }
+ } else {
+ tradeTxException.set(new TradeTxException(Res.get("error.closedTradeWithNoDepositTx", trade.getShortId())));
</pre>
<p>This means there are no funds locked in, should it still be part of the tradesIdSet representing locked in funds?</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/bisq-network/bisq/pull/3566?email_source=notifications&email_token=AJFFTNUD4T2CGUBG6XUULZ3QSLLKJA5CNFSM4JJPDRB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCKP453Y#pullrequestreview-312463087">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNUEQP5WQTQMQNXUWCTQSLLKJANCNFSM4JJPDRBQ">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AJFFTNTXOPNZSEHCC2IG763QSLLKJA5CNFSM4JJPDRB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCKP453Y.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/3566?email_source=notifications\u0026email_token=AJFFTNUD4T2CGUBG6XUULZ3QSLLKJA5CNFSM4JJPDRB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCKP453Y#pullrequestreview-312463087",
"url": "https://github.com/bisq-network/bisq/pull/3566?email_source=notifications\u0026email_token=AJFFTNUD4T2CGUBG6XUULZ3QSLLKJA5CNFSM4JJPDRB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCKP453Y#pullrequestreview-312463087",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>